feat: inject app service for charm retrieval into legacy state

sinanawad · Dec 20, 2024 · a02322b · a02322b
1 parent 4f3f676
commit a02322b
Show file tree

Hide file tree

Showing 44 changed files with 335 additions and 342 deletions.
diff --git a/apiserver/common/presence.go b/apiserver/common/presence.go
@@ -4,7 +4,6 @@
 package common
 
 import (
-	"github.com/juju/errors"
 	"github.com/juju/names/v5"
 
 	"github.com/juju/juju/core/presence"
@@ -31,21 +30,6 @@ func (c *ModelPresenceContext) machinePresence(machine MachineStatusGetter) (boo
 
 func (c *ModelPresenceContext) unitPresence(unit UnitStatusGetter) (bool, error) {
 	agent := names.NewUnitTag(unit.Name()).String()
-	if !unit.ShouldBeAssigned() {
-		sidecar, err := unit.IsSidecar()
-		if err != nil {
-			return false, errors.Trace(err)
-		}
-		if !sidecar {
-			// Units in CAAS models rely on the operator pings.
-			// These are for the application itself.
-			appName, err := names.UnitApplication(unit.Name())
-			if err != nil {
-				return false, errors.Trace(err)
-			}
-			agent = names.NewApplicationTag(appName).String()
-		}
-	}
 	status, err := c.Presence.AgentStatus(agent)
 	return status == presence.Alive, err
 }
diff --git a/apiserver/common/unitstatus.go b/apiserver/common/unitstatus.go
@@ -29,7 +29,6 @@ type UnitStatusGetter interface {
 	ShouldBeAssigned() bool
 	Name() string
 	Life() state.Life
-	IsSidecar() (bool, error)
 }
 
 // UnitStatus returns the unit agent and workload status for a given

diff --git a/apiserver/facades/agent/instancemutater/instancemutater_test.go b/apiserver/facades/agent/instancemutater/instancemutater_test.go
@@ -251,10 +251,6 @@ func (s *InstanceMutaterAPICharmProfilingInfoSuite) setup(c *gc.C) *gomock.Contr
 	return ctrl
 }
 
-func (s *InstanceMutaterAPICharmProfilingInfoSuite) TestCharmShimNilInnerProfile(c *gc.C) {
-	c.Assert(instancemutater.NewEmptyCharmShim().LXDProfile(), gc.DeepEquals, lxdprofile.Profile{})
-}
-
 func (s *InstanceMutaterAPICharmProfilingInfoSuite) TestCharmProfilingInfo(c *gc.C) {
 	defer s.setup(c).Finish()
 

diff --git a/apiserver/facades/agent/instancemutater/package_test.go b/apiserver/facades/agent/instancemutater/package_test.go
@@ -13,7 +13,6 @@ import (
 	apiservererrors "github.com/juju/juju/apiserver/errors"
 	"github.com/juju/juju/apiserver/facade"
 	loggertesting "github.com/juju/juju/internal/logger/testing"
-	"github.com/juju/juju/state"
 )
 
 //go:generate go run go.uber.org/mock/mockgen -typed -package mocks -destination mocks/instancemutater_mock.go github.com/juju/juju/apiserver/facades/agent/instancemutater InstanceMutatorWatcher,InstanceMutaterState,Machine,Unit,Application,Charm,MachineService
@@ -63,13 +62,3 @@ func NewTestLxdProfileWatcher(c *gc.C, machine Machine, backend InstanceMutaterS
 	c.Assert(err, jc.ErrorIsNil)
 	return w
 }
-
-// NewEmptyCharmShim returns a charm shim that satisfies the Charm indirection.
-// CAUTION. This is only suitable for testing scenarios where members of the
-// inner charm document have zero values.
-// Calls relying on the inner state reference will cause a nil-reference panic.
-func NewEmptyCharmShim() *charmShim {
-	return &charmShim{
-		Charm: &state.Charm{},
-	}
-}
diff --git a/apiserver/facades/agent/instancemutater/shim.go b/apiserver/facades/agent/instancemutater/shim.go
@@ -50,7 +50,7 @@ func (s *instanceMutaterStateShim) Charm(curl string) (Charm, error) {
 		return nil, errors.Trace(err)
 	}
 	return &charmShim{
-		Charm: ch,
+		CharmRefFull: ch,
 	}, nil
 }
 
@@ -67,19 +67,22 @@ func (s instanceMutaterStateShim) Machine(machineId string) (Machine, error) {
 // charmShim is used as a shim for a state Charm to enable better
 // mock testing.
 type charmShim struct {
-	*state.Charm
+	state.CharmRefFull
 }
 
 func (s *charmShim) LXDProfile() lxdprofile.Profile {
-	profile := s.Charm.LXDProfile()
-	if profile == nil {
-		return lxdprofile.Profile{}
-	}
-	return lxdprofile.Profile{
-		Config:      profile.Config,
-		Description: profile.Description,
-		Devices:     profile.Devices,
-	}
+	// TODO(nvinuesa): LXDProfile is not implemented yet.
+	// See https://warthogs.atlassian.net/browse/JUJU-7076
+	// profile := s.CharmRefFull.LXDProfile()
+	// if profile == nil {
+	// 	return lxdprofile.Profile{}
+	// }
+	// return lxdprofile.Profile{
+	// 	Config:      profile.Config,
+	// 	Description: profile.Description,
+	// 	Devices:     profile.Devices,
+	// }
+	return lxdprofile.Profile{}
 }
 
 // unitShim is used as a shim for a state Unit to enable better

diff --git a/apiserver/facades/agent/uniter/newlxdprofile.go b/apiserver/facades/agent/uniter/newlxdprofile.go
@@ -110,19 +110,22 @@ type lxdProfileMachineV2 struct {
 }
 
 type lxdProfileCharmV2 struct {
-	*state.Charm
+	state.CharmRefFull
 }
 
 func (c *lxdProfileCharmV2) LXDProfile() lxdprofile.Profile {
-	profile := c.Charm.LXDProfile()
-	if profile == nil {
-		return lxdprofile.Profile{}
-	}
-	return lxdprofile.Profile{
-		Config:      profile.Config,
-		Description: profile.Description,
-		Devices:     profile.Devices,
-	}
+	// TODO(nvinuesa): IsUploaded is not implemented yet.
+	// See https://warthogs.atlassian.net/browse/JUJU-6845
+	// profile := c.Charm.LXDProfile()
+	// if profile == nil {
+	// 	return lxdprofile.Profile{}
+	// }
+	// return lxdprofile.Profile{
+	// 	Config:      profile.Config,
+	// 	Description: profile.Description,
+	// 	Devices:     profile.Devices,
+	// }
+	return lxdprofile.Profile{}
 }
 
 // NewExternalLXDProfileAPIv2 can be used for API registration.

diff --git a/apiserver/facades/client/application/application.go b/apiserver/facades/client/application/application.go
@@ -977,7 +977,7 @@ func (api *APIBase) setCharmWithAgentValidation(
 func (api *APIBase) applicationSetCharm(
 	ctx context.Context,
 	params setCharmParams,
-	newCharm Charm,
+	newCharm state.CharmRefFull,
 	newOrigin *state.CharmOrigin,
 ) error {
 	appConfig, appSchema, charmSettings, appDefaults, err := parseCharmSettings(newCharm.Config(), params.AppName, params.ConfigSettingsStrings, params.ConfigSettingsYAML, environsconfig.NoDefaults)

diff --git a/apiserver/facades/client/application/backend.go b/apiserver/facades/client/application/backend.go
@@ -101,7 +101,9 @@ type Charm interface {
 	Config() *charm.Config
 	Actions() *charm.Actions
 	Revision() int
-	IsUploaded() bool
+	// TODO(nvinuesa): IsUploaded is not implemented yet.
+	// See https://warthogs.atlassian.net/browse/JUJU-6845
+	// IsUploaded() bool
 	URL() string
 }
 
@@ -283,23 +285,23 @@ func (s stateShim) AddCharmMetadata(info state.CharmInfo) (Charm, error) {
 	if err != nil {
 		return nil, err
 	}
-	return stateCharmShim{Charm: c}, nil
+	return stateCharmShim{CharmRefFull: c}, nil
 }
 
 func (s stateShim) UpdateUploadedCharm(info state.CharmInfo) (services.UploadedCharm, error) {
 	c, err := s.State.UpdateUploadedCharm(info)
 	if err != nil {
 		return nil, err
 	}
-	return stateCharmShim{Charm: c}, nil
+	return stateCharmShim{CharmRefFull: c}, nil
 }
 
 func (s stateShim) PrepareCharmUpload(curl string) (services.UploadedCharm, error) {
 	c, err := s.State.PrepareCharmUpload(curl)
 	if err != nil {
 		return nil, err
 	}
-	return stateCharmShim{Charm: c}, nil
+	return stateCharmShim{CharmRefFull: c}, nil
 }
 
 type remoteApplicationShim struct {
@@ -345,7 +347,7 @@ func (s stateShim) Charm(curl string) (Charm, error) {
 	if err != nil {
 		return nil, err
 	}
-	return stateCharmShim{Charm: ch}, nil
+	return stateCharmShim{CharmRefFull: ch}, nil
 }
 
 func (s stateShim) Model() (Model, error) {
@@ -475,7 +477,7 @@ func (a stateApplicationShim) SetCharm(
 }
 
 type stateCharmShim struct {
-	*state.Charm
+	state.CharmRefFull
 }
 
 type stateMachineShim struct {

diff --git a/apiserver/facades/client/application/deployrepository.go b/apiserver/facades/client/application/deployrepository.go
@@ -116,14 +116,6 @@ func (api *DeployFromRepositoryAPI) DeployFromRepository(ctx context.Context, ar
 	// Queue async charm download.
 	// AddCharmMetadata returns no error if the charm
 	// has already been queue'd or downloaded.
-	ch, err := api.state.AddCharmMetadata(state.CharmInfo{
-		Charm: dt.charm,
-		ID:    dt.charmURL.String(),
-	})
-	if err != nil {
-		return params.DeployFromRepositoryInfo{}, nil, []error{errors.Trace(err)}
-	}
-
 	stOrigin, err := StateCharmOrigin(dt.origin)
 	if err != nil {
 		return params.DeployFromRepositoryInfo{}, nil, []error{errors.Trace(err)}
@@ -139,8 +131,8 @@ func (api *DeployFromRepositoryAPI) DeployFromRepository(ctx context.Context, ar
 	_, addApplicationErr := api.state.AddApplication(state.AddApplicationArgs{
 		ApplicationConfig: dt.applicationConfig,
 		AttachStorage:     dt.attachStorage,
-		Charm:             ch,
-		CharmURL:          ch.URL(),
+		Charm:             dt.charm,
+		CharmURL:          dt.charmURL.String(),
 		CharmConfig:       dt.charmSettings,
 		CharmOrigin:       stOrigin,
 		Constraints:       dt.constraints,
@@ -161,7 +153,7 @@ func (api *DeployFromRepositoryAPI) DeployFromRepository(ctx context.Context, ar
 			}
 			unitArgs[i].UnitName = unitName
 		}
-		_, addApplicationErr = api.applicationService.CreateApplication(ctx, dt.applicationName, ch, dt.origin, applicationservice.AddApplicationArgs{
+		_, addApplicationErr = api.applicationService.CreateApplication(ctx, dt.applicationName, dt.charm, dt.origin, applicationservice.AddApplicationArgs{
 			ReferenceName: dt.charmURL.Name,
 			Storage:       dt.storage,
 			// We always have download info for a charm from the charmhub store.

diff --git a/apiserver/facades/client/charms/client_test.go b/apiserver/facades/client/charms/client_test.go
@@ -241,8 +241,8 @@ func (s *charmsMockSuite) TestAddCharmCharmhub(c *gc.C) {
 	}, nil)
 
 	s.state.EXPECT().AddCharmMetadata(gomock.Any()).DoAndReturn(
-		func(ci state.CharmInfo) (*state.Charm, error) {
-			c.Check(ci.ID, gc.DeepEquals, curl.String())
+		func(ci state.CharmInfo) (state.CharmRefFull, error) {
+			c.Check(ci.ID, gc.DeepEquals, curl)
 			// Check that the essential metadata matches what
 			// the repository returned. We use pointer checks here.
 			c.Check(ci.Charm.Meta(), gc.Equals, expMeta)

diff --git a/apiserver/facades/client/charms/interface.go b/apiserver/facades/client/charms/interface.go
@@ -27,15 +27,15 @@ func (s stateShim) UpdateUploadedCharm(charmInfo state.CharmInfo) (services.Uplo
 	if err != nil {
 		return nil, errors.Trace(err)
 	}
-	return stateCharmShim{Charm: ch}, nil
+	return stateCharmShim{CharmRefFull: ch}, nil
 }
 
 func (s stateShim) PrepareCharmUpload(curl string) (services.UploadedCharm, error) {
 	ch, err := s.State.PrepareCharmUpload(curl)
 	if err != nil {
 		return nil, errors.Trace(err)
 	}
-	return stateCharmShim{Charm: ch}, nil
+	return stateCharmShim{CharmRefFull: ch}, nil
 }
 
 func (s stateShim) Application(name string) (interfaces.Application, error) {
@@ -68,11 +68,14 @@ func (s stateApplicationShim) AllUnits() ([]interfaces.Unit, error) {
 }
 
 type stateCharmShim struct {
-	*state.Charm
+	state.CharmRefFull
 }
 
 func (s stateCharmShim) IsUploaded() bool {
-	return s.Charm.IsUploaded()
+	// TODO(nvinuesa): IsUploaded is not implemented yet.
+	// See https://warthogs.atlassian.net/browse/JUJU-6845
+	// return s.CharmRefFull.IsUploaded()
+	return false
 }
 
 // StoreCharm represents a store charm.

diff --git a/apiserver/facades/client/charms/interfaces/state.go b/apiserver/facades/client/charms/interfaces/state.go
@@ -12,9 +12,9 @@ import (
 )
 
 type BackendState interface {
-	AddCharmMetadata(state.CharmInfo) (*state.Charm, error)
+	AddCharmMetadata(state.CharmInfo) (state.CharmRefFull, error)
 	Application(string) (Application, error)
-	Charm(curl string) (*state.Charm, error)
+	Charm(curl string) (state.CharmRefFull, error)
 	ControllerTag() names.ControllerTag
 	UpdateUploadedCharm(info state.CharmInfo) (services.UploadedCharm, error)
 	PrepareCharmUpload(curl string) (services.UploadedCharm, error)

diff --git a/apiserver/facades/client/charms/mocks/state_mock.go b/apiserver/facades/client/charms/mocks/state_mock.go