From 413270ff3dbd2e552846665f8f5c9283acc7e915 Mon Sep 17 00:00:00 2001 From: wallyworld Date: Wed, 11 Sep 2024 09:49:26 +1000 Subject: [PATCH 1/3] feat(units): wireup facades to get app/unit life from dqlite Application and unit life is written to dqlite and so can be read from there. The facades which used to get the life value from mongo now use the domain service. As with the EnsureDead and Remove apiserver plugins which relied on a generic FindEntity, the LifeGetter plugin is removed from facades and bespoke code used instead. --- api/agent/agent/facade.go | 2 +- api/agent/caasapplication/client.go | 8 +- api/agent/caasapplication/client_test.go | 7 +- api/common/charms/common.go | 2 +- .../caasapplicationprovisioner/client.go | 2 +- api/controller/caasfirewaller/client.go | 13 +- apiserver/common/life.go | 5 +- apiserver/errors/errors.go | 18 +- apiserver/facades/agent/agent/agent.go | 25 ++- apiserver/facades/agent/agent/agent_test.go | 1 + apiserver/facades/agent/agent/register.go | 12 ++ .../agent/caasapplication/application.go | 35 ++-- .../agent/caasapplication/application_test.go | 29 ++-- .../agent/caasapplication/mock_test.go | 20 --- .../facades/agent/caasapplication/state.go | 2 - apiserver/facades/agent/deployer/deployer.go | 65 +++++++- .../facades/agent/deployer/deployer_test.go | 12 +- .../agent/deployer/mocks/domain_mock.go | 78 +++++++++ .../instancemutater/lxdprofilewatcher.go | 3 + apiserver/facades/agent/uniter/register.go | 5 +- apiserver/facades/agent/uniter/service.go | 3 + apiserver/facades/agent/uniter/uniter.go | 42 ++++- apiserver/facades/agent/uniter/uniter_test.go | 18 +- .../facades/client/application/application.go | 18 +- .../application/application_unit_test.go | 10 +- .../facades/client/application/backend.go | 2 - .../deployrepository_mocks_test.go | 38 ----- .../application/mocks/application_mock.go | 76 --------- .../facades/client/application/service.go | 10 ++ .../client/application/service_mock_test.go | 79 +++++++++ .../facades/client/controller/controller.go | 12 ++ .../client/controller/controller_test.go | 1 + .../facades/client/controller/export_test.go | 1 + .../facades/client/controller/register.go | 9 + .../caasapplicationprovisioner/provisioner.go | 47 +++++- .../caasapplicationprovisioner/service.go | 3 + .../service_mock_test.go | 31 ++++ .../common_service_mocks_test.go | 122 ++++++++++++++ .../controller/caasfirewaller/firewaller.go | 73 ++++++-- .../caasfirewaller/firewaller_test.go | 6 +- .../controller/caasfirewaller/mock_test.go | 6 - .../controller/caasfirewaller/package_test.go | 3 +- .../controller/caasfirewaller/register.go | 1 + .../caasfirewaller/service_mocks_test.go | 156 ++++++++---------- .../crossmodelrelations.go | 1 + .../controller/firewaller/firewaller.go | 104 ++++++++---- .../firewaller/firewaller_base_test.go | 2 +- .../controller/firewaller/firewaller_test.go | 6 + .../firewaller/firewaller_unit_test.go | 6 + .../controller/firewaller/interface.go | 5 + .../controller/firewaller/package_test.go | 2 +- .../facades/controller/firewaller/register.go | 6 + .../firewaller/service_mock_test.go | 67 +++++++- .../controller/migrationmaster/facade.go | 4 + .../controller/migrationmaster/facade_test.go | 3 + .../migrationmaster/mocks/backend.go | 67 +++++++- .../migrationmaster/package_test.go | 2 +- .../controller/migrationmaster/register.go | 6 + .../controller/migrationmaster/service.go | 6 + .../migrationtarget/domain_mock_test.go | 67 +++++++- .../migrationtarget/migrationtarget.go | 12 +- .../migrationtarget/migrationtarget_test.go | 3 + .../migrationtarget/package_test.go | 2 +- .../controller/migrationtarget/register.go | 6 + apiserver/facades/schema.json | 10 +- cmd/containeragent/initialize/command.go | 3 +- cmd/containeragent/initialize/command_test.go | 6 +- domain/application/errors/errors.go | 6 +- domain/application/service/application.go | 44 ++++- domain/application/service_test.go | 58 ++++++- domain/life/life.go | 16 ++ domain/life/life_test.go | 7 + internal/migration/interface.go | 7 +- internal/migration/migration_mock_test.go | 67 +++++++- internal/migration/package_test.go | 17 +- internal/migration/precheck.go | 41 +++-- internal/migration/precheck_test.go | 151 ++++++++++------- .../caasapplicationprovisioner/application.go | 9 +- .../application_test.go | 2 +- .../worker/caasapplicationprovisioner/ops.go | 18 +- .../caasapplicationprovisioner/worker.go | 5 +- .../caasfirewaller/applicationworker.go | 4 +- internal/worker/caasfirewaller/worker.go | 6 +- internal/worker/caasfirewaller/worker_test.go | 3 +- internal/worker/deployer/deployer.go | 2 +- internal/worker/firewaller/firewaller.go | 2 +- .../worker/uniter/relation/statemanager.go | 5 +- .../uniter/relation/statemanager_test.go | 4 +- rpc/params/apierror.go | 20 +++ rpc/params/apierror_test.go | 2 + state/cleanup.go | 10 +- state/cleanup_test.go | 4 + 92 files changed, 1519 insertions(+), 498 deletions(-) create mode 100644 apiserver/facades/controller/caasfirewaller/common_service_mocks_test.go diff --git a/api/agent/agent/facade.go b/api/agent/agent/facade.go index 032e8941570..8d6fbb0913d 100644 --- a/api/agent/agent/facade.go +++ b/api/agent/agent/facade.go @@ -76,7 +76,7 @@ func (facade *connFacade) Life(ctx context.Context, entity names.Tag) (Life, err return "", errors.Errorf("expected 1 result, got %d", len(results.Entities)) } if err := results.Entities[0].Error; err != nil { - if params.IsCodeNotFoundOrCodeUnauthorized(err) { + if params.IsCodeNotFoundOrCodeUnauthorized(err) || params.IsCodeUnitNotFound(err) { return "", ErrDenied } return "", errors.Trace(err) diff --git a/api/agent/caasapplication/client.go b/api/agent/caasapplication/client.go index 580a2683250..9ffcb2a079f 100644 --- a/api/agent/caasapplication/client.go +++ b/api/agent/caasapplication/client.go @@ -6,10 +6,10 @@ package caasapplication import ( "context" - "github.com/juju/errors" "github.com/juju/names/v5" "github.com/juju/juju/api/base" + applicationerrors "github.com/juju/juju/domain/application/errors" "github.com/juju/juju/rpc/params" ) @@ -52,10 +52,10 @@ func (c *Client) UnitIntroduction(ctx context.Context, podName string, podUUID s return nil, err } if err := result.Error; err != nil { - if params.IsCodeAlreadyExists(err) { - return nil, errors.AlreadyExists + if params.IsCodeUnitAlreadyExists(err) { + return nil, applicationerrors.UnitAlreadyExists } else if params.IsCodeNotAssigned(err) { - return nil, errors.NotAssigned + return nil, applicationerrors.UnitNotAssigned } return nil, err } diff --git a/api/agent/caasapplication/client_test.go b/api/agent/caasapplication/client_test.go index 93be65c5df5..13cea1badb1 100644 --- a/api/agent/caasapplication/client_test.go +++ b/api/agent/caasapplication/client_test.go @@ -14,6 +14,7 @@ import ( "github.com/juju/juju/api/agent/caasapplication" basetesting "github.com/juju/juju/api/base/testing" + applicationerrors "github.com/juju/juju/domain/application/errors" "github.com/juju/juju/rpc/params" ) @@ -90,12 +91,12 @@ func (s *provisionerSuite) TestUnitIntroductionFailAlreadyExists(c *gc.C) { c.Assert(args.PodUUID, gc.Equals, "pod-uuid") c.Assert(result, gc.FitsTypeOf, ¶ms.CAASUnitIntroductionResult{}) *(result.(*params.CAASUnitIntroductionResult)) = params.CAASUnitIntroductionResult{ - Error: ¶ms.Error{Code: params.CodeAlreadyExists}, + Error: ¶ms.Error{Code: params.CodeUnitAlreadyExists}, } return nil }) _, err := client.UnitIntroduction(context.Background(), "pod-name", "pod-uuid") - c.Assert(err, jc.ErrorIs, errors.AlreadyExists) + c.Assert(err, jc.ErrorIs, applicationerrors.UnitAlreadyExists) c.Assert(called, jc.IsTrue) } @@ -117,7 +118,7 @@ func (s *provisionerSuite) TestUnitIntroductionFailNotAssigned(c *gc.C) { return nil }) _, err := client.UnitIntroduction(context.Background(), "pod-name", "pod-uuid") - c.Assert(err, jc.ErrorIs, errors.NotAssigned) + c.Assert(err, jc.ErrorIs, applicationerrors.UnitNotAssigned) c.Assert(called, jc.IsTrue) } diff --git a/api/common/charms/common.go b/api/common/charms/common.go index dc503d7f684..5bf235f63f8 100644 --- a/api/common/charms/common.go +++ b/api/common/charms/common.go @@ -53,7 +53,7 @@ func (c *ApplicationCharmInfoClient) ApplicationCharmInfo(ctx context.Context, a args := params.Entity{Tag: names.NewApplicationTag(appName).String()} var info params.Charm if err := c.facade.FacadeCall(ctx, "ApplicationCharmInfo", args, &info); err != nil { - return nil, errors.Trace(err) + return nil, params.TranslateWellKnownError(err) } return convertCharm(&info) } diff --git a/api/controller/caasapplicationprovisioner/client.go b/api/controller/caasapplicationprovisioner/client.go index b6f98d70328..d80b91bacbe 100644 --- a/api/controller/caasapplicationprovisioner/client.go +++ b/api/controller/caasapplicationprovisioner/client.go @@ -409,7 +409,7 @@ func (c *Client) RemoveUnit(ctx context.Context, unitName string) error { return err } resultErr := result.OneError() - if params.IsCodeNotFound(resultErr) { + if params.IsCodeUnitNotFound(resultErr) { return nil } return resultErr diff --git a/api/controller/caasfirewaller/client.go b/api/controller/caasfirewaller/client.go index 61c0f60347d..cd24e3f5a50 100644 --- a/api/controller/caasfirewaller/client.go +++ b/api/controller/caasfirewaller/client.go @@ -159,7 +159,7 @@ func (c *Client) Life(ctx context.Context, appName string) (life.Value, error) { return "", errors.Errorf("expected 1 result, got %d", n) } if err := results.Results[0].Error; err != nil { - return "", maybeNotFound(err) + return "", params.TranslateWellKnownError(err) } return results.Results[0].Life, nil } @@ -197,16 +197,7 @@ func (c *Client) IsExposed(ctx context.Context, appName string) (bool, error) { return false, errors.Errorf("expected 1 result, got %d", n) } if err := results.Results[0].Error; err != nil { - return false, maybeNotFound(err) + return false, params.TranslateWellKnownError(err) } return results.Results[0].Result, nil } - -// maybeNotFound returns an error satisfying errors.IsNotFound -// if the supplied error has a CodeNotFound error. -func maybeNotFound(err *params.Error) error { - if err == nil || !params.IsCodeNotFound(err) { - return err - } - return errors.NewNotFound(err, "") -} diff --git a/apiserver/common/life.go b/apiserver/common/life.go index 89e1bc9a06b..2b77eb4820f 100644 --- a/apiserver/common/life.go +++ b/apiserver/common/life.go @@ -30,7 +30,8 @@ func NewLifeGetter(st state.EntityFinder, getCanRead GetAuthFunc) *LifeGetter { } } -func (lg *LifeGetter) oneLife(tag names.Tag) (life.Value, error) { +// OneLife returns the life of the specified entity. +func (lg *LifeGetter) OneLife(tag names.Tag) (life.Value, error) { entity0, err := lg.st.FindEntity(tag) if err != nil { return "", err @@ -62,7 +63,7 @@ func (lg *LifeGetter) Life(ctx context.Context, args params.Entities) (params.Li } err = apiservererrors.ErrPerm if canRead(tag) { - result.Results[i].Life, err = lg.oneLife(tag) + result.Results[i].Life, err = lg.OneLife(tag) } result.Results[i].Error = apiservererrors.ServerError(err) } diff --git a/apiserver/errors/errors.go b/apiserver/errors/errors.go index 1c957b03683..781d3b8379a 100644 --- a/apiserver/errors/errors.go +++ b/apiserver/errors/errors.go @@ -121,10 +121,14 @@ func ServerErrorAndStatus(err error) (*params.Error, int) { params.CodeSecretRevisionNotFound, params.CodeSecretConsumerNotFound, params.CodeSecretBackendNotFound, - params.CodeApplicationNotFound: + params.CodeApplicationNotFound, + params.CodeUnitNotFound: status = http.StatusNotFound case params.CodeBadRequest, - params.CodeScalingStateInconsistent: + params.CodeScalingStateInconsistent, + params.CodeApplicationAlreadyExists, + params.CodeApplicationIsDead, + params.CodeUnitAlreadyExists: status = http.StatusBadRequest case params.CodeMethodNotAllowed: status = http.StatusMethodNotAllowed @@ -237,6 +241,16 @@ func ServerError(err error) *params.Error { code = params.CodeApplicationNotFound case errors.Is(err, applicationerrors.ScalingStateInconsistent): code = params.CodeScalingStateInconsistent + case errors.Is(err, applicationerrors.UnitNotAssigned): + code = params.CodeNotAssigned + case errors.Is(err, applicationerrors.ApplicationAlreadyExists): + code = params.CodeApplicationAlreadyExists + case errors.Is(err, applicationerrors.ApplicationIsDead): + code = params.CodeApplicationIsDead + case errors.Is(err, applicationerrors.UnitAlreadyExists): + code = params.CodeUnitAlreadyExists + case errors.Is(err, applicationerrors.UnitNotFound): + code = params.CodeUnitNotFound case errors.Is(err, errors.MethodNotAllowed): code = params.CodeMethodNotAllowed case errors.Is(err, errors.NotImplemented): diff --git a/apiserver/facades/agent/agent/agent.go b/apiserver/facades/agent/agent/agent.go index 8f6c2c0b73b..830412a0c27 100644 --- a/apiserver/facades/agent/agent/agent.go +++ b/apiserver/facades/agent/agent/agent.go @@ -40,6 +40,7 @@ type AgentAPI struct { credentialService CredentialService controllerConfigService ControllerConfigService + applicationService ApplicationService st *state.State auth facade.Authorizer resources facade.Resources @@ -56,6 +57,7 @@ func NewAgentAPI( credentialService common.CredentialService, rebootMachineService common.MachineRebootService, modelConfigService common.ModelConfigService, + applicationService ApplicationService, watcherRegistry facade.WatcherRegistry, ) (*AgentAPI, error) { getCanChange := func() (common.AuthFunc, error) { @@ -85,6 +87,7 @@ func NewAgentAPI( ), credentialService: credentialService, controllerConfigService: controllerConfigService, + applicationService: applicationService, st: st, auth: auth, resources: resources, @@ -101,6 +104,21 @@ func (api *AgentAPI) GetEntities(ctx context.Context, args params.Entities) para results.Entities[i].Error = apiservererrors.ServerError(err) continue } + // Allow only for the owner agent. + // Note: having a bulk API call for this is utter madness, given that + // this check means we can only ever return a single object. + if !api.auth.AuthOwner(tag) { + results.Entities[i].Error = apiservererrors.ServerError(apiservererrors.ErrPerm) + continue + } + // Handle units using the domain service. + // Eventually all entities will be supported via dqlite. + if tag.Kind() == names.UnitTagKind { + lifeValue, err := api.applicationService.GetUnitLife(ctx, tag.Id()) + results.Entities[i].Life = lifeValue + results.Entities[i].Error = apiservererrors.ServerError(err) + continue + } result, err := api.getEntity(tag) result.Error = apiservererrors.ServerError(err) results.Entities[i] = result @@ -109,13 +127,6 @@ func (api *AgentAPI) GetEntities(ctx context.Context, args params.Entities) para } func (api *AgentAPI) getEntity(tag names.Tag) (result params.AgentGetEntitiesResult, err error) { - // Allow only for the owner agent. - // Note: having a bulk API call for this is utter madness, given that - // this check means we can only ever return a single object. - if !api.auth.AuthOwner(tag) { - err = apiservererrors.ErrPerm - return - } entity0, err := api.st.FindEntity(tag) if err != nil { return diff --git a/apiserver/facades/agent/agent/agent_test.go b/apiserver/facades/agent/agent/agent_test.go index 08b508d1319..249703b3cb8 100644 --- a/apiserver/facades/agent/agent/agent_test.go +++ b/apiserver/facades/agent/agent/agent_test.go @@ -89,6 +89,7 @@ func (s *agentSuite) agentAPI(c *gc.C, auth facade.Authorizer, credentialService nil, nil, nil, + nil, ) } diff --git a/apiserver/facades/agent/agent/register.go b/apiserver/facades/agent/agent/register.go index 10360c50e64..98bf53ff53e 100644 --- a/apiserver/facades/agent/agent/register.go +++ b/apiserver/facades/agent/agent/register.go @@ -10,7 +10,10 @@ import ( apiservererrors "github.com/juju/juju/apiserver/errors" "github.com/juju/juju/apiserver/facade" "github.com/juju/juju/core/credential" + "github.com/juju/juju/core/life" "github.com/juju/juju/core/watcher" + "github.com/juju/juju/domain/application/service" + "github.com/juju/juju/internal/storage" ) // Register is called to expose a package of facades onto a given registry. @@ -24,6 +27,11 @@ type CredentialService interface { WatchCredential(ctx context.Context, key credential.Key) (watcher.NotifyWatcher, error) } +// ApplicationService provides access to the application service. +type ApplicationService interface { + GetUnitLife(ctx context.Context, name string) (life.Value, error) +} + // NewAgentAPIV3 returns an object implementing version 3 of the Agent API // with the given authorizer representing the currently logged in client. func NewAgentAPIV3(ctx facade.ModelContext) (*AgentAPI, error) { @@ -42,6 +50,10 @@ func NewAgentAPIV3(ctx facade.ModelContext) (*AgentAPI, error) { ctx.ServiceFactory().Credential(), ctx.ServiceFactory().Machine(), ctx.ServiceFactory().Config(), + ctx.ServiceFactory().Application(service.ApplicationServiceParams{ + StorageRegistry: storage.NotImplementedProviderRegistry{}, + Secrets: service.NotImplementedSecretService{}, + }), ctx.WatcherRegistry(), ) } diff --git a/apiserver/facades/agent/caasapplication/application.go b/apiserver/facades/agent/caasapplication/application.go index c6561665a76..e323dfb749c 100644 --- a/apiserver/facades/agent/caasapplication/application.go +++ b/apiserver/facades/agent/caasapplication/application.go @@ -19,6 +19,7 @@ import ( "github.com/juju/juju/apiserver/facade" "github.com/juju/juju/caas" "github.com/juju/juju/controller" + "github.com/juju/juju/core/life" "github.com/juju/juju/core/logger" "github.com/juju/juju/core/network" "github.com/juju/juju/core/paths" @@ -37,6 +38,8 @@ type ControllerConfigService interface { type ApplicationService interface { RegisterCAASUnit(ctx context.Context, appName string, unit applicationservice.RegisterCAASUnitParams) error CAASUnitTerminating(ctx context.Context, appName string, unitNum int, broker applicationservice.Broker) (bool, error) + GetApplicationLife(ctx context.Context, appName string) (life.Value, error) + GetUnitLife(ctx context.Context, unitName string) (life.Value, error) } // Facade defines the API methods on the CAASApplication facade. @@ -107,12 +110,13 @@ func (f *Facade) UnitIntroduction(ctx context.Context, args params.CAASUnitIntro f.logger.Debugf("introducing pod %q (%q)", args.PodName, args.PodUUID) - application, err := f.state.Application(tag.Name) + appName := tag.Name + appLife, err := f.applicationService.GetApplicationLife(ctx, appName) if err != nil { return errResp(err) } - if application.Life() != state.Alive { + if appLife != life.Alive { return errResp(errors.NotProvisionedf("application")) } @@ -133,7 +137,7 @@ func (f *Facade) UnitIntroduction(ctx context.Context, args params.CAASUnitIntro if err != nil { return errResp(err) } - n := fmt.Sprintf("%s/%d", application.Name(), ord) + n := fmt.Sprintf("%s/%d", appName, ord) upsert.UnitName = &n upsert.OrderedId = ord upsert.OrderedScale = true @@ -142,7 +146,7 @@ func (f *Facade) UnitIntroduction(ctx context.Context, args params.CAASUnitIntro } // Find the pod/unit in the provider. - caasApp := f.broker.Application(application.Name(), caas.DeploymentStateful) + caasApp := f.broker.Application(appName, caas.DeploymentStateful) pods, err := caasApp.Units() if err != nil { return errResp(err) @@ -176,13 +180,7 @@ func (f *Facade) UnitIntroduction(ctx context.Context, args params.CAASUnitIntro passwordHash := password.AgentPasswordHash(pass) upsert.PasswordHash = &passwordHash - // TODO(units) - remove dual write to state - _, err = application.UpsertCAASUnit(upsert) - if err != nil { - return errResp(err) - } - - if err := f.applicationService.RegisterCAASUnit(ctx, application.Name(), applicationservice.RegisterCAASUnitParams{ + if err := f.applicationService.RegisterCAASUnit(ctx, appName, applicationservice.RegisterCAASUnitParams{ UnitName: *upsert.UnitName, ProviderId: upsert.ProviderId, Address: upsert.Address, @@ -194,6 +192,16 @@ func (f *Facade) UnitIntroduction(ctx context.Context, args params.CAASUnitIntro return errResp(err) } + // TODO(units) - remove dual write to state + application, err := f.state.Application(tag.Name) + if err != nil { + return errResp(err) + } + _, err = application.UpsertCAASUnit(upsert) + if err != nil { + return errResp(err) + } + controllerConfig, err := f.controllerConfigService.ControllerConfig(ctx) if err != nil { return errResp(err) @@ -269,12 +277,11 @@ func (f *Facade) UnitTerminating(ctx context.Context, args params.Entity) (param return params.CAASUnitTerminationResult{}, apiservererrors.ErrPerm } - // TODO(units): should be in service but we don't keep life up to date yet - unit, err := f.state.Unit(unitTag.Id()) + unitLife, err := f.applicationService.GetUnitLife(ctx, unitTag.Id()) if err != nil { return errResp(err) } - if unit.Life() != state.Alive { + if unitLife != life.Alive { return params.CAASUnitTerminationResult{WillRestart: false}, nil } diff --git a/apiserver/facades/agent/caasapplication/application_test.go b/apiserver/facades/agent/caasapplication/application_test.go index ff850ad1a89..56e04c9cd29 100644 --- a/apiserver/facades/agent/caasapplication/application_test.go +++ b/apiserver/facades/agent/caasapplication/application_test.go @@ -38,6 +38,8 @@ type CAASApplicationSuite struct { st *mockState clock *testclock.Clock broker *mockBroker + + applicationService *service.WatchableService } func (s *CAASApplicationSuite) SetUpTest(c *gc.C) { @@ -56,7 +58,7 @@ func (s *CAASApplicationSuite) SetUpTest(c *gc.C) { // upserting of units. serviceFactory := s.DefaultModelServiceFactory(c) unitName := "gitlab/0" - applicationService := serviceFactory.Application(service.ApplicationServiceParams{ + s.applicationService = serviceFactory.Application(service.ApplicationServiceParams{ StorageRegistry: provider.CommonStorageProviders(), Secrets: service.NotImplementedSecretService{}, }) @@ -70,7 +72,7 @@ func (s *CAASApplicationSuite) SetUpTest(c *gc.C) { }, } - _, err := applicationService.CreateApplication( + _, err := s.applicationService.CreateApplication( context.Background(), "gitlab", &stubCharm{}, origin, service.AddApplicationArgs{ ReferenceName: "gitlab", }, service.AddUnitArg{ @@ -86,7 +88,7 @@ func (s *CAASApplicationSuite) SetUpTest(c *gc.C) { s.authorizer, s.st, s.st, s.ControllerServiceFactory(c).ControllerConfig(), - applicationService, + s.applicationService, s.broker, s.clock, loggertesting.WrapCheckLog(c), @@ -131,11 +133,11 @@ func (s *CAASApplicationSuite) TestAddUnit(c *gc.C) { s.st.CheckCallNames(c, "Model", "Application", "APIHostPortsForAgents") s.st.CheckCall(c, 1, "Application", "gitlab") - s.st.app.CheckCallNames(c, "Life", "Name", "Name", "UpsertCAASUnit", "Name") + s.st.app.CheckCallNames(c, "UpsertCAASUnit") mc := jc.NewMultiChecker() mc.AddExpr("_.AddUnitParams.PasswordHash", gc.Not(gc.IsNil)) - c.Assert(s.st.app.Calls()[3].Args[0], mc, state.UpsertCAASUnitParams{ + c.Assert(s.st.app.Calls()[0].Args[0], mc, state.UpsertCAASUnitParams{ AddUnitParams: state.AddUnitParams{ ProviderId: strPtr("gitlab-0"), UnitName: strPtr("gitlab/0"), @@ -169,7 +171,7 @@ func (s *CAASApplicationSuite) TestAddUnitNotNeeded(c *gc.C) { s.st.CheckCallNames(c, "Model", "Application") s.st.CheckCall(c, 1, "Application", "gitlab") - s.st.app.CheckCallNames(c, "Life", "Name", "Name", "UpsertCAASUnit") + s.st.app.CheckCallNames(c, "UpsertCAASUnit") } func (s *CAASApplicationSuite) TestReuseUnitByName(c *gc.C) { @@ -202,11 +204,11 @@ func (s *CAASApplicationSuite) TestReuseUnitByName(c *gc.C) { s.st.CheckCallNames(c, "Model", "Application", "APIHostPortsForAgents") s.st.CheckCall(c, 1, "Application", "gitlab") - s.st.app.CheckCallNames(c, "Life", "Name", "Name", "UpsertCAASUnit", "Name") + s.st.app.CheckCallNames(c, "UpsertCAASUnit") mc := jc.NewMultiChecker() mc.AddExpr("_.AddUnitParams.PasswordHash", gc.Not(gc.IsNil)) - c.Assert(s.st.app.Calls()[3].Args[0], mc, state.UpsertCAASUnitParams{ + c.Assert(s.st.app.Calls()[0].Args[0], mc, state.UpsertCAASUnitParams{ AddUnitParams: state.AddUnitParams{ ProviderId: strPtr("gitlab-0"), UnitName: strPtr("gitlab/0"), @@ -240,7 +242,7 @@ func (s *CAASApplicationSuite) TestDontReuseDeadUnitByName(c *gc.C) { s.st.CheckCallNames(c, "Model", "Application") s.st.CheckCall(c, 1, "Application", "gitlab") - s.st.app.CheckCallNames(c, "Life", "Name", "Name", "UpsertCAASUnit") + s.st.app.CheckCallNames(c, "UpsertCAASUnit") } func (s *CAASApplicationSuite) TestFindByProviderID(c *gc.C) { @@ -333,7 +335,8 @@ func (s *CAASApplicationSuite) TestDyingApplication(c *gc.C) { PodUUID: "gitlab-uuid", } - s.st.app.life = state.Dying + err := s.applicationService.DestroyApplication(context.Background(), "gitlab") + c.Assert(err, jc.ErrorIsNil) results, err := s.facade.UnitIntroduction(context.Background(), args) c.Assert(err, jc.ErrorIsNil) @@ -345,7 +348,8 @@ func (s *CAASApplicationSuite) TestMissingArgUUID(c *gc.C) { PodName: "gitlab-0", } - s.st.app.life = state.Dying + err := s.applicationService.DestroyApplication(context.Background(), "gitlab") + c.Assert(err, jc.ErrorIsNil) results, err := s.facade.UnitIntroduction(context.Background(), args) c.Assert(err, jc.ErrorIsNil) @@ -357,7 +361,8 @@ func (s *CAASApplicationSuite) TestMissingArgName(c *gc.C) { PodUUID: "gitlab-uuid", } - s.st.app.life = state.Dying + err := s.applicationService.DestroyApplication(context.Background(), "gitlab") + c.Assert(err, jc.ErrorIsNil) results, err := s.facade.UnitIntroduction(context.Background(), args) c.Assert(err, jc.ErrorIsNil) diff --git a/apiserver/facades/agent/caasapplication/mock_test.go b/apiserver/facades/agent/caasapplication/mock_test.go index 3f40ef62e38..543d4869a01 100644 --- a/apiserver/facades/agent/caasapplication/mock_test.go +++ b/apiserver/facades/agent/caasapplication/mock_test.go @@ -38,10 +38,6 @@ func newMockState() *mockState { controllerTag: names.NewControllerTag("ffffffff-ffff-ffff-ffff-ffffffffffff"), tag: names.NewModelTag("ffffffff-ffff-ffff-ffff-ffffffffffff"), }, - app: mockApplication{ - name: "gitlab", - life: state.Alive, - }, controllerConfig: jujucontroller.Config{ jujucontroller.CACertKey: jtesting.CACert, }, @@ -130,25 +126,9 @@ func (st *mockModel) Tag() names.Tag { type mockApplication struct { testing.Stub - life state.Life - name string unit *mockUnit } -func (*mockApplication) Tag() names.Tag { - return names.NewApplicationTag("gitlab") -} - -func (a *mockApplication) Life() state.Life { - a.MethodCall(a, "Life") - return a.life -} - -func (a *mockApplication) Name() string { - a.MethodCall(a, "Name") - return a.name -} - func (a *mockApplication) UpsertCAASUnit(args state.UpsertCAASUnitParams) (caasapplication.Unit, error) { a.MethodCall(a, "UpsertCAASUnit", args) return a.unit, a.NextErr() diff --git a/apiserver/facades/agent/caasapplication/state.go b/apiserver/facades/agent/caasapplication/state.go index ec2fd47878e..605b502dc00 100644 --- a/apiserver/facades/agent/caasapplication/state.go +++ b/apiserver/facades/agent/caasapplication/state.go @@ -39,8 +39,6 @@ type Model interface { // Application provides the subset of application state // required by the CAAS application facade. type Application interface { - Life() state.Life - Name() string UpsertCAASUnit(args state.UpsertCAASUnitParams) (Unit, error) } diff --git a/apiserver/facades/agent/deployer/deployer.go b/apiserver/facades/agent/deployer/deployer.go index 9e41a0411b9..9da469e0039 100644 --- a/apiserver/facades/agent/deployer/deployer.go +++ b/apiserver/facades/agent/deployer/deployer.go @@ -14,6 +14,7 @@ import ( "github.com/juju/juju/apiserver/facade" "github.com/juju/juju/controller" "github.com/juju/juju/core/leadership" + "github.com/juju/juju/core/life" "github.com/juju/juju/core/objectstore" "github.com/juju/juju/rpc/params" "github.com/juju/juju/state" @@ -29,17 +30,19 @@ type ControllerConfigGetter interface { // ApplicationService removes a unit from the dqlite database. type ApplicationService interface { + GetUnitLife(context.Context, string) (life.Value, error) + EnsureUnitDead(context.Context, string, leadership.Revoker) error RemoveUnit(context.Context, string, leadership.Revoker) error } // DeployerAPI provides access to the Deployer API facade. type DeployerAPI struct { *common.PasswordChanger - *common.LifeGetter *common.APIAddresser *common.UnitsWatcher *common.StatusSetter + canRead func(tag names.Tag) bool canWrite func(tag names.Tag) bool controllerConfigGetter ControllerConfigGetter @@ -85,21 +88,21 @@ func NewDeployerAPI( getCanWatch := func() (common.AuthFunc, error) { return authorizer.AuthOwner, nil } - canWrite, err := getAuthFunc() + auth, err := getAuthFunc() if err != nil { return nil, errors.Trace(err) } return &DeployerAPI{ PasswordChanger: common.NewPasswordChanger(st, getAuthFunc), - LifeGetter: common.NewLifeGetter(st, getAuthFunc), APIAddresser: common.NewAPIAddresser(systemState, resources), UnitsWatcher: common.NewUnitsWatcher(st, resources, getCanWatch), StatusSetter: common.NewStatusSetter(st, getAuthFunc), controllerConfigGetter: controllerConfigGetter, applicationService: applicationService, leadershipRevoker: leadershipRevoker, - canWrite: canWrite, + canRead: auth, + canWrite: auth, store: store, st: st, resources: resources, @@ -153,6 +156,31 @@ func (d *DeployerAPI) APIAddresses(ctx context.Context) (result params.StringsRe return d.APIAddresser.APIAddresses(ctx, controllerConfig) } +// Life returns the life of the specified units. +func (d *DeployerAPI) Life(ctx context.Context, args params.Entities) (params.LifeResults, error) { + result := params.LifeResults{ + Results: make([]params.LifeResult, len(args.Entities)), + } + if len(args.Entities) == 0 { + return result, nil + } + for i, entity := range args.Entities { + tag, err := names.ParseTag(entity.Tag) + if err != nil { + result.Results[i].Error = apiservererrors.ServerError(apiservererrors.ErrPerm) + continue + } + if !d.canRead(tag) { + result.Results[i].Error = apiservererrors.ServerError(apiservererrors.ErrPerm) + continue + } + lifeValue, err := d.applicationService.GetUnitLife(ctx, tag.Id()) + result.Results[i].Life = lifeValue + result.Results[i].Error = apiservererrors.ServerError(err) + } + return result, nil +} + // getAllUnits returns a list of all principal and subordinate units // assigned to the given machine. func getAllUnits(st *state.State, tag names.Tag) ([]string, error) { @@ -190,22 +218,41 @@ func (d *DeployerAPI) Remove(ctx context.Context, args params.Entities) (params. continue } - if err = d.applicationService.RemoveUnit(ctx, tag.Id(), d.leadershipRevoker); err != nil { - result.Results[i].Error = apiservererrors.ServerError(err) - continue - } - // TODO(units) - remove me. // Dual write to state. + // We need to set the unit life to Dead in state **first** + // because the life watcher is currently looking at state + // not dqlite. unit, err := d.st.Unit(tag.Id()) if err != nil { + if errors.Is(err, errors.NotFound) { + err = apiservererrors.ErrPerm + } result.Results[i].Error = apiservererrors.ServerError(err) continue } + if unit.Life() == state.Alive { + result.Results[i].Error = apiservererrors.ServerError(errors.Errorf("cannot remove unit %q: still alive", tag.Id())) + continue + } if err := unit.EnsureDead(); err != nil { result.Results[i].Error = apiservererrors.ServerError(err) continue } + + // Given the way dual write works, we need this for now. + if err = d.applicationService.EnsureUnitDead(ctx, tag.Id(), d.leadershipRevoker); err != nil { + result.Results[i].Error = apiservererrors.ServerError(err) + continue + } + // This is the call we will keep once mongo is removed. + // We will need to remove the alive check. + if err = d.applicationService.RemoveUnit(ctx, tag.Id(), d.leadershipRevoker); err != nil { + result.Results[i].Error = apiservererrors.ServerError(err) + continue + } + + // TODO(units) - remove me. if err := unit.Remove(d.store); err != nil { result.Results[i].Error = apiservererrors.ServerError(err) continue diff --git a/apiserver/facades/agent/deployer/deployer_test.go b/apiserver/facades/agent/deployer/deployer_test.go index e15f5e7d761..20f43de2562 100644 --- a/apiserver/facades/agent/deployer/deployer_test.go +++ b/apiserver/facades/agent/deployer/deployer_test.go @@ -23,6 +23,7 @@ import ( "github.com/juju/juju/apiserver/facades/agent/deployer/mocks" apiservertesting "github.com/juju/juju/apiserver/testing" "github.com/juju/juju/core/leadership" + "github.com/juju/juju/core/life" "github.com/juju/juju/core/network" "github.com/juju/juju/core/status" coretesting "github.com/juju/juju/internal/testing" @@ -260,6 +261,10 @@ func (s *deployerSuite) TestLife(c *gc.C) { defer s.setupMocks(c).Finish() s.makeDeployerAPI(c) + s.applicationService.EXPECT().GetUnitLife(gomock.Any(), "mysql/0").Return(life.Alive, nil) + s.applicationService.EXPECT().GetUnitLife(gomock.Any(), "logging/0").Return(life.Dead, nil) + s.applicationService.EXPECT().GetUnitLife(gomock.Any(), "logging/0").Return("", apiservererrors.ErrPerm) + err := s.subordinate0.EnsureDead() c.Assert(err, jc.ErrorIsNil) err = s.subordinate0.Refresh() @@ -312,17 +317,12 @@ func (s *deployerSuite) TestRemove(c *gc.C) { s.makeDeployerAPI(c) gomock.InOrder( - s.applicationService.EXPECT().RemoveUnit(gomock.Any(), "mysql/0", gomock.Any()). - Return(errors.New(`cannot remove unit "mysql/0": still alive`)), - s.applicationService.EXPECT().RemoveUnit(gomock.Any(), "logging/0", gomock.Any()). - Return(errors.New(`cannot remove unit "logging/0": still alive`)), + s.applicationService.EXPECT().EnsureUnitDead(gomock.Any(), "logging/0", gomock.Any()), s.applicationService.EXPECT().RemoveUnit(gomock.Any(), "logging/0", gomock.Any()). DoAndReturn(func(ctx context.Context, unitName string, revoker leadership.Revoker) error { appName, _ := names.UnitApplication(unitName) return revoker.RevokeLeadership(appName, unitName) }), - s.applicationService.EXPECT().RemoveUnit(gomock.Any(), "logging/0", gomock.Any()). - Return(apiservererrors.ErrPerm), ) c.Assert(s.principal0.Life(), gc.Equals, state.Alive) diff --git a/apiserver/facades/agent/deployer/mocks/domain_mock.go b/apiserver/facades/agent/deployer/mocks/domain_mock.go index 50fd45ba4ac..2d285408ea7 100644 --- a/apiserver/facades/agent/deployer/mocks/domain_mock.go +++ b/apiserver/facades/agent/deployer/mocks/domain_mock.go @@ -15,6 +15,7 @@ import ( controller "github.com/juju/juju/controller" leadership "github.com/juju/juju/core/leadership" + life "github.com/juju/juju/core/life" gomock "go.uber.org/mock/gomock" ) @@ -103,6 +104,83 @@ func (m *MockApplicationService) EXPECT() *MockApplicationServiceMockRecorder { return m.recorder } +// EnsureUnitDead mocks base method. +func (m *MockApplicationService) EnsureUnitDead(arg0 context.Context, arg1 string, arg2 leadership.Revoker) error { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "EnsureUnitDead", arg0, arg1, arg2) + ret0, _ := ret[0].(error) + return ret0 +} + +// EnsureUnitDead indicates an expected call of EnsureUnitDead. +func (mr *MockApplicationServiceMockRecorder) EnsureUnitDead(arg0, arg1, arg2 any) *MockApplicationServiceEnsureUnitDeadCall { + mr.mock.ctrl.T.Helper() + call := mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "EnsureUnitDead", reflect.TypeOf((*MockApplicationService)(nil).EnsureUnitDead), arg0, arg1, arg2) + return &MockApplicationServiceEnsureUnitDeadCall{Call: call} +} + +// MockApplicationServiceEnsureUnitDeadCall wrap *gomock.Call +type MockApplicationServiceEnsureUnitDeadCall struct { + *gomock.Call +} + +// Return rewrite *gomock.Call.Return +func (c *MockApplicationServiceEnsureUnitDeadCall) Return(arg0 error) *MockApplicationServiceEnsureUnitDeadCall { + c.Call = c.Call.Return(arg0) + return c +} + +// Do rewrite *gomock.Call.Do +func (c *MockApplicationServiceEnsureUnitDeadCall) Do(f func(context.Context, string, leadership.Revoker) error) *MockApplicationServiceEnsureUnitDeadCall { + c.Call = c.Call.Do(f) + return c +} + +// DoAndReturn rewrite *gomock.Call.DoAndReturn +func (c *MockApplicationServiceEnsureUnitDeadCall) DoAndReturn(f func(context.Context, string, leadership.Revoker) error) *MockApplicationServiceEnsureUnitDeadCall { + c.Call = c.Call.DoAndReturn(f) + return c +} + +// GetUnitLife mocks base method. +func (m *MockApplicationService) GetUnitLife(arg0 context.Context, arg1 string) (life.Value, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "GetUnitLife", arg0, arg1) + ret0, _ := ret[0].(life.Value) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// GetUnitLife indicates an expected call of GetUnitLife. +func (mr *MockApplicationServiceMockRecorder) GetUnitLife(arg0, arg1 any) *MockApplicationServiceGetUnitLifeCall { + mr.mock.ctrl.T.Helper() + call := mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetUnitLife", reflect.TypeOf((*MockApplicationService)(nil).GetUnitLife), arg0, arg1) + return &MockApplicationServiceGetUnitLifeCall{Call: call} +} + +// MockApplicationServiceGetUnitLifeCall wrap *gomock.Call +type MockApplicationServiceGetUnitLifeCall struct { + *gomock.Call +} + +// Return rewrite *gomock.Call.Return +func (c *MockApplicationServiceGetUnitLifeCall) Return(arg0 life.Value, arg1 error) *MockApplicationServiceGetUnitLifeCall { + c.Call = c.Call.Return(arg0, arg1) + return c +} + +// Do rewrite *gomock.Call.Do +func (c *MockApplicationServiceGetUnitLifeCall) Do(f func(context.Context, string) (life.Value, error)) *MockApplicationServiceGetUnitLifeCall { + c.Call = c.Call.Do(f) + return c +} + +// DoAndReturn rewrite *gomock.Call.DoAndReturn +func (c *MockApplicationServiceGetUnitLifeCall) DoAndReturn(f func(context.Context, string) (life.Value, error)) *MockApplicationServiceGetUnitLifeCall { + c.Call = c.Call.DoAndReturn(f) + return c +} + // RemoveUnit mocks base method. func (m *MockApplicationService) RemoveUnit(arg0 context.Context, arg1 string, arg2 leadership.Revoker) error { m.ctrl.T.Helper() diff --git a/apiserver/facades/agent/instancemutater/lxdprofilewatcher.go b/apiserver/facades/agent/instancemutater/lxdprofilewatcher.go index ace95b0e60b..1aa4590b01d 100644 --- a/apiserver/facades/agent/instancemutater/lxdprofilewatcher.go +++ b/apiserver/facades/agent/instancemutater/lxdprofilewatcher.go @@ -171,6 +171,9 @@ func (w *machineLXDProfileWatcher) loop() error { } } case units := <-unitWatcher.Changes(): + // TODO(units) - use service to read unit info + // We could read life from dqlite but don't yet + // support getting all the other attributes. w.logger.Debugf("unit changes on %v: %v", w.machine.Id(), units) for _, unitName := range units { u, err := w.backend.Unit(unitName) diff --git a/apiserver/facades/agent/uniter/register.go b/apiserver/facades/agent/uniter/register.go index fc4a0715eeb..5f76b4f5276 100644 --- a/apiserver/facades/agent/uniter/register.go +++ b/apiserver/facades/agent/uniter/register.go @@ -97,6 +97,7 @@ func newUniterAPIWithServices( accessApplication := applicationAccessor(authorizer, st) accessMachine := machineAccessor(authorizer, st) accessCloudSpec := cloudSpecAccessor(authorizer, st) + accessUnitOrApplication := common.AuthAny(accessUnit, accessApplication) m, err := st.Model() if err != nil { @@ -113,8 +114,6 @@ func newUniterAPIWithServices( return nil, errors.Trace(err) } - accessUnitOrApplication := common.AuthAny(accessUnit, accessApplication) - modelInfo, err := modelInfoService.GetModelInfo(stdCtx) if err != nil { return nil, errors.Trace(err) @@ -135,7 +134,6 @@ func newUniterAPIWithServices( } logger := context.Logger().Child("uniter") return &UniterAPI{ - LifeGetter: common.NewLifeGetter(st, accessUnitOrApplication), AgentEntityWatcher: common.NewAgentEntityWatcher(st, resources, accessUnitOrApplication), APIAddresser: common.NewAPIAddresser(systemState, resources), ModelWatcher: common.NewModelWatcher(modelConfigService, context.WatcherRegistry()), @@ -164,6 +162,7 @@ func newUniterAPIWithServices( leadershipRevoker: leadershipRevoker, accessUnit: accessUnit, accessApplication: accessApplication, + accessUnitOrApplication: accessUnitOrApplication, accessMachine: accessMachine, accessCloudSpec: accessCloudSpec, cloudSpecer: cloudSpec, diff --git a/apiserver/facades/agent/uniter/service.go b/apiserver/facades/agent/uniter/service.go index 81006c7bb07..64b634247bd 100644 --- a/apiserver/facades/agent/uniter/service.go +++ b/apiserver/facades/agent/uniter/service.go @@ -10,6 +10,7 @@ import ( "github.com/juju/juju/controller" "github.com/juju/juju/core/credential" "github.com/juju/juju/core/leadership" + "github.com/juju/juju/core/life" coremachine "github.com/juju/juju/core/machine" "github.com/juju/juju/core/model" "github.com/juju/juju/core/network" @@ -53,6 +54,8 @@ type CredentialService interface { // ApplicationService provides access to the application service. type ApplicationService interface { + GetApplicationLife(ctx context.Context, unitName string) (life.Value, error) + GetUnitLife(ctx context.Context, unitName string) (life.Value, error) EnsureUnitDead(ctx context.Context, unitName string, leadershipRevoker leadership.Revoker) error DeleteUnit(ctx context.Context, unitName string) error DestroyUnit(ctx context.Context, unitName string) error diff --git a/apiserver/facades/agent/uniter/uniter.go b/apiserver/facades/agent/uniter/uniter.go index 0542f128724..4b0d12c1cf3 100644 --- a/apiserver/facades/agent/uniter/uniter.go +++ b/apiserver/facades/agent/uniter/uniter.go @@ -38,7 +38,6 @@ import ( // UniterAPI implements the latest version (v18) of the Uniter API. type UniterAPI struct { - *common.LifeGetter *StatusAPI *common.AgentEntityWatcher *common.APIAddresser @@ -65,6 +64,7 @@ type UniterAPI struct { leadershipRevoker leadership.Revoker accessUnit common.GetAuthFunc accessApplication common.GetAuthFunc + accessUnitOrApplication common.GetAuthFunc accessMachine common.GetAuthFunc containerBrokerFunc caas.NewContainerBrokerFunc *StorageAPI @@ -1078,6 +1078,44 @@ func (u *UniterAPI) RelationsStatus(ctx context.Context, args params.Entities) ( return result, nil } +// Life returns the life status of the specified units. +func (u *UniterAPI) Life(ctx context.Context, args params.Entities) (params.LifeResults, error) { + result := params.LifeResults{ + Results: make([]params.LifeResult, len(args.Entities)), + } + if len(args.Entities) == 0 { + return result, nil + } + canRead, err := u.accessUnitOrApplication() + if err != nil { + return params.LifeResults{}, errors.Trace(err) + } + for i, entity := range args.Entities { + tag, err := names.ParseTag(entity.Tag) + if err != nil { + result.Results[i].Error = apiservererrors.ServerError(apiservererrors.ErrPerm) + continue + } + if !canRead(tag) { + result.Results[i].Error = apiservererrors.ServerError(apiservererrors.ErrPerm) + continue + } + var lifeValue life.Value + switch tag.Kind() { + case names.ApplicationTagKind: + lifeValue, err = u.applicationService.GetApplicationLife(ctx, tag.Id()) + case names.UnitTagKind: + lifeValue, err = u.applicationService.GetUnitLife(ctx, tag.Id()) + default: + result.Results[i].Error = apiservererrors.ServerError(apiservererrors.ErrPerm) + continue + } + result.Results[i].Life = lifeValue + result.Results[i].Error = apiservererrors.ServerError(err) + } + return result, nil +} + // Refresh retrieves the latest values for attributes on this unit. func (u *UniterAPI) Refresh(ctx context.Context, args params.Entities) (params.UnitRefreshResults, error) { result := params.UnitRefreshResults{ @@ -1098,6 +1136,7 @@ func (u *UniterAPI) Refresh(ctx context.Context, args params.Entities) (params.U } err = apiservererrors.ErrPerm if canRead(tag) { + // TODO(units) - read unit details from dqlite var unit *state.Unit if unit, err = u.getUnit(tag); err == nil { result.Results[i].Life = life.Value(unit.Life().String()) @@ -2218,6 +2257,7 @@ func (u *UniterAPI) goalStateRelations(appName, principalName string, allRelatio // and stores the goal state status in UnitsGoalState. func (u *UniterAPI) goalStateUnits(app *state.Application, principalName string) (params.UnitsGoalState, error) { + // TODO(units) - add service method for AllUnits allUnits, err := app.AllUnits() if err != nil { return nil, err diff --git a/apiserver/facades/agent/uniter/uniter_test.go b/apiserver/facades/agent/uniter/uniter_test.go index a7f39a93d57..fabaa6f08c8 100644 --- a/apiserver/facades/agent/uniter/uniter_test.go +++ b/apiserver/facades/agent/uniter/uniter_test.go @@ -29,11 +29,13 @@ import ( "github.com/juju/juju/core/life" "github.com/juju/juju/core/network" "github.com/juju/juju/core/status" + "github.com/juju/juju/domain/application/service" "github.com/juju/juju/environs/config" "github.com/juju/juju/internal/charm" loggertesting "github.com/juju/juju/internal/logger/testing" "github.com/juju/juju/internal/password" _ "github.com/juju/juju/internal/secrets/provider/all" + "github.com/juju/juju/internal/storage" coretesting "github.com/juju/juju/internal/testing" "github.com/juju/juju/internal/testing/factory" "github.com/juju/juju/internal/uuid" @@ -215,6 +217,13 @@ func (s *uniterSuite) TestLife(c *gc.C) { c.Assert(err, jc.ErrorIsNil) c.Assert(relStatus.Status, gc.Equals, status.Joining) + // We need to dual write to dqlite. + sf := s.ServiceFactorySuite.ServiceFactoryGetter(c).FactoryForModel(s.ServiceFactorySuite.ControllerModelUUID) + applicationService := sf.Application(service.ApplicationServiceParams{ + StorageRegistry: storage.NotImplementedProviderRegistry{}, + Secrets: service.NotImplementedSecretService{}, + }) + // Make the wordpressUnit dead. err = s.wordpressUnit.EnsureDead() c.Assert(err, jc.ErrorIsNil) @@ -222,19 +231,26 @@ func (s *uniterSuite) TestLife(c *gc.C) { c.Assert(err, jc.ErrorIsNil) c.Assert(s.wordpressUnit.Life(), gc.Equals, state.Dead) + err = applicationService.EnsureUnitDead(context.Background(), "wordpress/0", s.leadershipRevoker) + c.Assert(err, jc.ErrorIsNil) + c.Assert(s.wordpressUnit.Life(), gc.Equals, state.Dead) + // Add another unit, so the service will stay dying when we // destroy it later. extraUnit, err := s.wordpress.AddUnit(state.AddUnitParams{}) c.Assert(err, jc.ErrorIsNil) c.Assert(extraUnit, gc.NotNil) - // Make the wordpress service dying. + // Make the wordpress application dying. err = s.wordpress.Destroy(s.store) c.Assert(err, jc.ErrorIsNil) err = s.wordpress.Refresh() c.Assert(err, jc.ErrorIsNil) c.Assert(s.wordpress.Life(), gc.Equals, state.Dying) + err = applicationService.DestroyApplication(context.Background(), "wordpress") + c.Assert(err, jc.ErrorIsNil) + args := params.Entities{Entities: []params.Entity{ {Tag: "unit-mysql-0"}, {Tag: "unit-wordpress-0"}, diff --git a/apiserver/facades/client/application/application.go b/apiserver/facades/client/application/application.go index 2b6bece7623..6771dbafd2b 100644 --- a/apiserver/facades/client/application/application.go +++ b/apiserver/facades/client/application/application.go @@ -2321,6 +2321,12 @@ func (api *APIBase) ApplicationsInfo(ctx context.Context, in params.Entities) (p out[i].Error = apiservererrors.ServerError(err) continue } + appLife, err := api.applicationService.GetApplicationLife(ctx, tag.Name) + if err != nil { + out[i].Error = apiservererrors.ServerError(err) + continue + } + app, err := api.backend.Application(tag.Name) if err != nil { out[i].Error = apiservererrors.ServerError(err) @@ -2369,7 +2375,7 @@ func (api *APIBase) ApplicationsInfo(ctx context.Context, in params.Entities) (p Principal: app.IsPrincipal(), Exposed: app.IsExposed(), Remote: app.IsRemote(), - Life: app.Life().String(), + Life: string(appLife), EndpointBindings: bindingsMap, ExposedEndpoints: exposedEndpoints, } @@ -2585,7 +2591,7 @@ func (api *APIBase) UnitsInfo(ctx context.Context, in params.Entities) (params.U continue } for _, unit := range units { - result, err := api.unitResultForUnit(unit) + result, err := api.unitResultForUnit(ctx, unit) if err != nil { results = append(results, params.UnitInfoResult{Error: apiservererrors.ServerError(err)}) continue @@ -2625,7 +2631,7 @@ func (api *APIBase) unitsFromTag(tag string) ([]Unit, error) { } // Builds a *params.UnitResult describing the unit argument. -func (api *APIBase) unitResultForUnit(unit Unit) (*params.UnitResult, error) { +func (api *APIBase) unitResultForUnit(ctx context.Context, unit Unit) (*params.UnitResult, error) { app, err := api.backend.Application(unit.ApplicationName()) if err != nil { return nil, err @@ -2639,13 +2645,17 @@ func (api *APIBase) unitResultForUnit(unit Unit) (*params.UnitResult, error) { if err != nil { return nil, err } + unitLife, err := api.applicationService.GetUnitLife(ctx, unit.Name()) + if err != nil { + return nil, err + } result := ¶ms.UnitResult{ Tag: unit.Tag().String(), WorkloadVersion: workloadVersion, Machine: machineId, Charm: *curl, - Life: unit.Life().String(), + Life: string(unitLife), } if machineId != "" { machine, err := api.backend.Machine(machineId) diff --git a/apiserver/facades/client/application/application_unit_test.go b/apiserver/facades/client/application/application_unit_test.go index 0bb2aa286b3..bcb29a4accb 100644 --- a/apiserver/facades/client/application/application_unit_test.go +++ b/apiserver/facades/client/application/application_unit_test.go @@ -31,6 +31,7 @@ import ( "github.com/juju/juju/core/constraints" "github.com/juju/juju/core/crossmodel" "github.com/juju/juju/core/instance" + "github.com/juju/juju/core/life" corelogger "github.com/juju/juju/core/logger" "github.com/juju/juju/core/machine" "github.com/juju/juju/core/model" @@ -38,6 +39,7 @@ import ( "github.com/juju/juju/core/objectstore" "github.com/juju/juju/core/status" jujuversion "github.com/juju/juju/core/version" + applicationerrors "github.com/juju/juju/domain/application/errors" applicationservice "github.com/juju/juju/domain/application/service" storageerrors "github.com/juju/juju/domain/storage/errors" "github.com/juju/juju/environs/config" @@ -290,8 +292,8 @@ func (s *ApplicationSuite) expectApplication(ctrl *gomock.Controller, name strin app.EXPECT().IsPrincipal().Return(true).AnyTimes() app.EXPECT().IsExposed().Return(false).AnyTimes() app.EXPECT().IsRemote().Return(false).AnyTimes() - app.EXPECT().Life().Return(state.Alive).AnyTimes() app.EXPECT().Constraints().Return(constraints.MustParse("arch=amd64 mem=4G cores=1 root-disk=8G"), nil).AnyTimes() + s.applicationService.EXPECT().GetApplicationLife(gomock.Any(), name).Return(life.Alive, nil).AnyTimes() return app } @@ -858,7 +860,7 @@ func (s *ApplicationSuite) expectUnit(ctrl *gomock.Controller, name string) *moc unit.EXPECT().ApplicationName().Return(appName).AnyTimes() unit.EXPECT().AssignedMachineId().Return(machineId, nil).AnyTimes() unit.EXPECT().WorkloadVersion().Return("666", nil).AnyTimes() - unit.EXPECT().Life().Return(state.Alive).AnyTimes() + s.applicationService.EXPECT().GetUnitLife(gomock.Any(), name).Return(life.Alive, nil).AnyTimes() return unit } @@ -2782,7 +2784,7 @@ func (s *ApplicationSuite) TestApplicationsInfoMany(c *gc.C) { s.backend.EXPECT().Application("postgresql").Return(app, nil).MinTimes(1) // wordpress - s.backend.EXPECT().Application("wordpress").Return(nil, errors.NotFoundf(`application "wordpress"`)) + s.applicationService.EXPECT().GetApplicationLife(gomock.Any(), "wordpress").Return("", applicationerrors.ApplicationNotFound) s.networkService.EXPECT().GetAllSpaces(gomock.Any()).Times(2) entities := []params.Entity{{Tag: "application-postgresql"}, {Tag: "application-wordpress"}, {Tag: "unit-postgresql-0"}} @@ -2800,7 +2802,7 @@ func (s *ApplicationSuite) TestApplicationsInfoMany(c *gc.C) { "juju-info": "myspace", }, }) - c.Assert(result.Results[1].Error, gc.ErrorMatches, `application "wordpress" not found`) + c.Assert(result.Results[1].Error, gc.ErrorMatches, `application not found`) c.Assert(result.Results[2].Error, gc.ErrorMatches, `"unit-postgresql-0" is not a valid application tag`) } diff --git a/apiserver/facades/client/application/backend.go b/apiserver/facades/client/application/backend.go index 00f770df032..c56126d1fdc 100644 --- a/apiserver/facades/client/application/backend.go +++ b/apiserver/facades/client/application/backend.go @@ -90,7 +90,6 @@ type Application interface { IsExposed() bool IsPrincipal() bool IsRemote() bool - Life() state.Life SetCharm(state.SetCharmConfig, objectstore.ObjectStore) error SetConstraints(constraints.Value) error MergeExposeSettings(map[string]state.ExposedEndpoint) error @@ -181,7 +180,6 @@ type Unit interface { Destroy(objectstore.ObjectStore) error DestroyOperation(objectstore.ObjectStore) *state.DestroyUnitOperation IsPrincipal() bool - Life() state.Life Resolve(retryHooks bool) error AgentTools() (*tools.Tools, error) diff --git a/apiserver/facades/client/application/deployrepository_mocks_test.go b/apiserver/facades/client/application/deployrepository_mocks_test.go index f736721ab09..598894dc633 100644 --- a/apiserver/facades/client/application/deployrepository_mocks_test.go +++ b/apiserver/facades/client/application/deployrepository_mocks_test.go @@ -1945,44 +1945,6 @@ func (c *MockApplicationIsRemoteCall) DoAndReturn(f func() bool) *MockApplicatio return c } -// Life mocks base method. -func (m *MockApplication) Life() state.Life { - m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "Life") - ret0, _ := ret[0].(state.Life) - return ret0 -} - -// Life indicates an expected call of Life. -func (mr *MockApplicationMockRecorder) Life() *MockApplicationLifeCall { - mr.mock.ctrl.T.Helper() - call := mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Life", reflect.TypeOf((*MockApplication)(nil).Life)) - return &MockApplicationLifeCall{Call: call} -} - -// MockApplicationLifeCall wrap *gomock.Call -type MockApplicationLifeCall struct { - *gomock.Call -} - -// Return rewrite *gomock.Call.Return -func (c *MockApplicationLifeCall) Return(arg0 state.Life) *MockApplicationLifeCall { - c.Call = c.Call.Return(arg0) - return c -} - -// Do rewrite *gomock.Call.Do -func (c *MockApplicationLifeCall) Do(f func() state.Life) *MockApplicationLifeCall { - c.Call = c.Call.Do(f) - return c -} - -// DoAndReturn rewrite *gomock.Call.DoAndReturn -func (c *MockApplicationLifeCall) DoAndReturn(f func() state.Life) *MockApplicationLifeCall { - c.Call = c.Call.DoAndReturn(f) - return c -} - // MergeBindings mocks base method. func (m *MockApplication) MergeBindings(arg0 *state.Bindings, arg1 bool) error { m.ctrl.T.Helper() diff --git a/apiserver/facades/client/application/mocks/application_mock.go b/apiserver/facades/client/application/mocks/application_mock.go index 231a1ced34d..1ec5fdcb409 100644 --- a/apiserver/facades/client/application/mocks/application_mock.go +++ b/apiserver/facades/client/application/mocks/application_mock.go @@ -2424,44 +2424,6 @@ func (c *MockApplicationIsRemoteCall) DoAndReturn(f func() bool) *MockApplicatio return c } -// Life mocks base method. -func (m *MockApplication) Life() state.Life { - m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "Life") - ret0, _ := ret[0].(state.Life) - return ret0 -} - -// Life indicates an expected call of Life. -func (mr *MockApplicationMockRecorder) Life() *MockApplicationLifeCall { - mr.mock.ctrl.T.Helper() - call := mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Life", reflect.TypeOf((*MockApplication)(nil).Life)) - return &MockApplicationLifeCall{Call: call} -} - -// MockApplicationLifeCall wrap *gomock.Call -type MockApplicationLifeCall struct { - *gomock.Call -} - -// Return rewrite *gomock.Call.Return -func (c *MockApplicationLifeCall) Return(arg0 state.Life) *MockApplicationLifeCall { - c.Call = c.Call.Return(arg0) - return c -} - -// Do rewrite *gomock.Call.Do -func (c *MockApplicationLifeCall) Do(f func() state.Life) *MockApplicationLifeCall { - c.Call = c.Call.Do(f) - return c -} - -// DoAndReturn rewrite *gomock.Call.DoAndReturn -func (c *MockApplicationLifeCall) DoAndReturn(f func() state.Life) *MockApplicationLifeCall { - c.Call = c.Call.DoAndReturn(f) - return c -} - // MergeBindings mocks base method. func (m *MockApplication) MergeBindings(arg0 *state.Bindings, arg1 bool) error { m.ctrl.T.Helper() @@ -4352,44 +4314,6 @@ func (c *MockUnitIsPrincipalCall) DoAndReturn(f func() bool) *MockUnitIsPrincipa return c } -// Life mocks base method. -func (m *MockUnit) Life() state.Life { - m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "Life") - ret0, _ := ret[0].(state.Life) - return ret0 -} - -// Life indicates an expected call of Life. -func (mr *MockUnitMockRecorder) Life() *MockUnitLifeCall { - mr.mock.ctrl.T.Helper() - call := mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Life", reflect.TypeOf((*MockUnit)(nil).Life)) - return &MockUnitLifeCall{Call: call} -} - -// MockUnitLifeCall wrap *gomock.Call -type MockUnitLifeCall struct { - *gomock.Call -} - -// Return rewrite *gomock.Call.Return -func (c *MockUnitLifeCall) Return(arg0 state.Life) *MockUnitLifeCall { - c.Call = c.Call.Return(arg0) - return c -} - -// Do rewrite *gomock.Call.Do -func (c *MockUnitLifeCall) Do(f func() state.Life) *MockUnitLifeCall { - c.Call = c.Call.Do(f) - return c -} - -// DoAndReturn rewrite *gomock.Call.DoAndReturn -func (c *MockUnitLifeCall) DoAndReturn(f func() state.Life) *MockUnitLifeCall { - c.Call = c.Call.DoAndReturn(f) - return c -} - // Name mocks base method. func (m *MockUnit) Name() string { m.ctrl.T.Helper() diff --git a/apiserver/facades/client/application/service.go b/apiserver/facades/client/application/service.go index b5c2d366839..fbbde7639a8 100644 --- a/apiserver/facades/client/application/service.go +++ b/apiserver/facades/client/application/service.go @@ -12,6 +12,7 @@ import ( "github.com/juju/juju/core/assumes" corecharm "github.com/juju/juju/core/charm" "github.com/juju/juju/core/crossmodel" + "github.com/juju/juju/core/life" "github.com/juju/juju/core/machine" "github.com/juju/juju/core/network" applicationservice "github.com/juju/juju/domain/application/service" @@ -79,6 +80,15 @@ type ApplicationService interface { // returning an error satisfying [applicationerrors.UnitNotFoundError] // if the unit doesn't exist. DestroyUnit(ctx context.Context, name string) error + + // GetApplicationLife looks up the life of the specified application, returning an error + // satisfying [applicationerrors.ApplicationNotFoundError] if the application is not found. + GetApplicationLife(ctx context.Context, name string) (life.Value, error) + + // GetUnitLife looks up the life of the specified unit, returning an error + // satisfying [applicationerrors.UnitNotFoundError] if the unit is not found. + GetUnitLife(ctx context.Context, name string) (life.Value, error) + // GetSupportedFeatures returns the set of features that the model makes // available for charms to use. GetSupportedFeatures(ctx context.Context) (assumes.FeatureSet, error) diff --git a/apiserver/facades/client/application/service_mock_test.go b/apiserver/facades/client/application/service_mock_test.go index 1c6eb49f370..4dfc37b5b97 100644 --- a/apiserver/facades/client/application/service_mock_test.go +++ b/apiserver/facades/client/application/service_mock_test.go @@ -17,6 +17,7 @@ import ( assumes "github.com/juju/juju/core/assumes" charm "github.com/juju/juju/core/charm" crossmodel "github.com/juju/juju/core/crossmodel" + life "github.com/juju/juju/core/life" machine "github.com/juju/juju/core/machine" network "github.com/juju/juju/core/network" service "github.com/juju/juju/domain/application/service" @@ -352,6 +353,45 @@ func (c *MockApplicationServiceDestroyUnitCall) DoAndReturn(f func(context.Conte return c } +// GetApplicationLife mocks base method. +func (m *MockApplicationService) GetApplicationLife(arg0 context.Context, arg1 string) (life.Value, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "GetApplicationLife", arg0, arg1) + ret0, _ := ret[0].(life.Value) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// GetApplicationLife indicates an expected call of GetApplicationLife. +func (mr *MockApplicationServiceMockRecorder) GetApplicationLife(arg0, arg1 any) *MockApplicationServiceGetApplicationLifeCall { + mr.mock.ctrl.T.Helper() + call := mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetApplicationLife", reflect.TypeOf((*MockApplicationService)(nil).GetApplicationLife), arg0, arg1) + return &MockApplicationServiceGetApplicationLifeCall{Call: call} +} + +// MockApplicationServiceGetApplicationLifeCall wrap *gomock.Call +type MockApplicationServiceGetApplicationLifeCall struct { + *gomock.Call +} + +// Return rewrite *gomock.Call.Return +func (c *MockApplicationServiceGetApplicationLifeCall) Return(arg0 life.Value, arg1 error) *MockApplicationServiceGetApplicationLifeCall { + c.Call = c.Call.Return(arg0, arg1) + return c +} + +// Do rewrite *gomock.Call.Do +func (c *MockApplicationServiceGetApplicationLifeCall) Do(f func(context.Context, string) (life.Value, error)) *MockApplicationServiceGetApplicationLifeCall { + c.Call = c.Call.Do(f) + return c +} + +// DoAndReturn rewrite *gomock.Call.DoAndReturn +func (c *MockApplicationServiceGetApplicationLifeCall) DoAndReturn(f func(context.Context, string) (life.Value, error)) *MockApplicationServiceGetApplicationLifeCall { + c.Call = c.Call.DoAndReturn(f) + return c +} + // GetSupportedFeatures mocks base method. func (m *MockApplicationService) GetSupportedFeatures(arg0 context.Context) (assumes.FeatureSet, error) { m.ctrl.T.Helper() @@ -391,6 +431,45 @@ func (c *MockApplicationServiceGetSupportedFeaturesCall) DoAndReturn(f func(cont return c } +// GetUnitLife mocks base method. +func (m *MockApplicationService) GetUnitLife(arg0 context.Context, arg1 string) (life.Value, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "GetUnitLife", arg0, arg1) + ret0, _ := ret[0].(life.Value) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// GetUnitLife indicates an expected call of GetUnitLife. +func (mr *MockApplicationServiceMockRecorder) GetUnitLife(arg0, arg1 any) *MockApplicationServiceGetUnitLifeCall { + mr.mock.ctrl.T.Helper() + call := mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetUnitLife", reflect.TypeOf((*MockApplicationService)(nil).GetUnitLife), arg0, arg1) + return &MockApplicationServiceGetUnitLifeCall{Call: call} +} + +// MockApplicationServiceGetUnitLifeCall wrap *gomock.Call +type MockApplicationServiceGetUnitLifeCall struct { + *gomock.Call +} + +// Return rewrite *gomock.Call.Return +func (c *MockApplicationServiceGetUnitLifeCall) Return(arg0 life.Value, arg1 error) *MockApplicationServiceGetUnitLifeCall { + c.Call = c.Call.Return(arg0, arg1) + return c +} + +// Do rewrite *gomock.Call.Do +func (c *MockApplicationServiceGetUnitLifeCall) Do(f func(context.Context, string) (life.Value, error)) *MockApplicationServiceGetUnitLifeCall { + c.Call = c.Call.Do(f) + return c +} + +// DoAndReturn rewrite *gomock.Call.DoAndReturn +func (c *MockApplicationServiceGetUnitLifeCall) DoAndReturn(f func(context.Context, string) (life.Value, error)) *MockApplicationServiceGetUnitLifeCall { + c.Call = c.Call.DoAndReturn(f) + return c +} + // SetApplicationScale mocks base method. func (m *MockApplicationService) SetApplicationScale(arg0 context.Context, arg1 string, arg2 int) error { m.ctrl.T.Helper() diff --git a/apiserver/facades/client/controller/controller.go b/apiserver/facades/client/controller/controller.go index df323e860aa..cabe7b1357a 100644 --- a/apiserver/facades/client/controller/controller.go +++ b/apiserver/facades/client/controller/controller.go @@ -28,6 +28,7 @@ import ( "github.com/juju/juju/apiserver/facade" corecontroller "github.com/juju/juju/controller" "github.com/juju/juju/core/leadership" + "github.com/juju/juju/core/life" corelogger "github.com/juju/juju/core/logger" coremigration "github.com/juju/juju/core/migration" coremodel "github.com/juju/juju/core/model" @@ -87,6 +88,10 @@ type ModelService interface { GetModelUsers(ctx context.Context, modelUUID coremodel.UUID) ([]coremodel.ModelUserInfo, error) } +type ApplicationService interface { + GetApplicationLife(ctx context.Context, name string) (life.Value, error) +} + // ModelConfigService provides access to the model configuration. type ModelConfigService interface { // ModelConfig returns the current config for the model. @@ -128,6 +133,7 @@ type ControllerAPI struct { controllerConfigService ControllerConfigService accessService ControllerAccessService modelService ModelService + applicationServiceGetter func(coremodel.UUID) ApplicationService modelConfigServiceGetter func(coremodel.UUID) ModelConfigService proxyService ProxyService modelExporter func(coremodel.UUID, facade.LegacyStateExporter) ModelExporter @@ -161,6 +167,7 @@ func NewControllerAPI( upgradeService UpgradeService, accessService ControllerAccessService, modelService ModelService, + applicationServiceGetter func(coremodel.UUID) ApplicationService, modelConfigServiceGetter func(coremodel.UUID) ModelConfigService, proxyService ProxyService, modelExporter func(coremodel.UUID, facade.LegacyStateExporter) ModelExporter, @@ -210,6 +217,7 @@ func NewControllerAPI( credentialService: credentialService, upgradeService: upgradeService, cloudService: cloudService, + applicationServiceGetter: applicationServiceGetter, accessService: accessService, modelService: modelService, modelConfigServiceGetter: modelConfigServiceGetter, @@ -741,6 +749,7 @@ func (c *ControllerAPI) initiateOneMigration(ctx context.Context, spec params.Mi if err != nil { return "", errors.Trace(err) } + applicationService := c.applicationServiceGetter(coremodel.UUID(hostedState.ModelUUID())) if err := runMigrationPrechecks( ctx, hostedState.State, systemState, @@ -750,6 +759,7 @@ func (c *ControllerAPI) initiateOneMigration(ctx context.Context, spec params.Mi c.credentialService, c.upgradeService, c.modelService, + applicationService, c.modelExporter, c.store, leaders, @@ -895,6 +905,7 @@ var runMigrationPrechecks = func( credentialService common.CredentialService, upgradeService UpgradeService, modelService ModelService, + applicationService ApplicationService, modelExporter func(coremodel.UUID, facade.LegacyStateExporter) ModelExporter, store objectstore.ObjectStore, leaders map[string]string, @@ -914,6 +925,7 @@ var runMigrationPrechecks = func( cloudspec.MakeCloudSpecGetterForModel(st, cloudService, credentialService), credentialService, upgradeService, + applicationService, ); err != nil { return errors.Annotate(err, "source prechecks failed") } diff --git a/apiserver/facades/client/controller/controller_test.go b/apiserver/facades/client/controller/controller_test.go index 6b93dd40d71..20ae5f0cff0 100644 --- a/apiserver/facades/client/controller/controller_test.go +++ b/apiserver/facades/client/controller/controller_test.go @@ -980,6 +980,7 @@ func (s *accessSuite) controllerAPI(c *gc.C) *controller.ControllerAPI { nil, nil, nil, + nil, ) c.Assert(err, jc.ErrorIsNil) diff --git a/apiserver/facades/client/controller/export_test.go b/apiserver/facades/client/controller/export_test.go index 85e9afd0df0..7d7f101a7ca 100644 --- a/apiserver/facades/client/controller/export_test.go +++ b/apiserver/facades/client/controller/export_test.go @@ -28,6 +28,7 @@ func SetPrecheckResult(p patcher, err error) { credentialService common.CredentialService, upgradeService UpgradeService, modelService ModelService, + applicationService ApplicationService, modelExporter func(model.UUID, facade.LegacyStateExporter) ModelExporter, store objectstore.ObjectStore, leaders map[string]string) error { diff --git a/apiserver/facades/client/controller/register.go b/apiserver/facades/client/controller/register.go index bdb5b5600c1..4e5d393d0cc 100644 --- a/apiserver/facades/client/controller/register.go +++ b/apiserver/facades/client/controller/register.go @@ -12,6 +12,8 @@ import ( "github.com/juju/juju/apiserver/facade" "github.com/juju/juju/core/model" + "github.com/juju/juju/domain/application/service" + "github.com/juju/juju/internal/storage" ) // Register is called to expose a package of facades onto a given registry. @@ -45,6 +47,12 @@ func makeControllerAPI(stdCtx context.Context, ctx facade.MultiModelContext) (*C modelConfigServiceGetter := func(modelID model.UUID) ModelConfigService { return ctx.ServiceFactoryForModel(modelID).Config() } + applicationServiceGetter := func(modelID model.UUID) ApplicationService { + return ctx.ServiceFactoryForModel(modelID).Application(service.ApplicationServiceParams{ + StorageRegistry: storage.NotImplementedProviderRegistry{}, + Secrets: service.NotImplementedSecretService{}, + }) + } return NewControllerAPI( stdCtx, @@ -62,6 +70,7 @@ func makeControllerAPI(stdCtx context.Context, ctx facade.MultiModelContext) (*C serviceFactory.Upgrade(), serviceFactory.Access(), serviceFactory.Model(), + applicationServiceGetter, modelConfigServiceGetter, serviceFactory.Proxy(), func(modelUUID model.UUID, legacyState facade.LegacyStateExporter) ModelExporter { diff --git a/apiserver/facades/controller/caasapplicationprovisioner/provisioner.go b/apiserver/facades/controller/caasapplicationprovisioner/provisioner.go index cfa3ae8f897..d5a443a89ed 100644 --- a/apiserver/facades/controller/caasapplicationprovisioner/provisioner.go +++ b/apiserver/facades/controller/caasapplicationprovisioner/provisioner.go @@ -28,6 +28,7 @@ import ( "github.com/juju/juju/controller" coreapplication "github.com/juju/juju/core/application" "github.com/juju/juju/core/leadership" + "github.com/juju/juju/core/life" corelogger "github.com/juju/juju/core/logger" "github.com/juju/juju/core/model" "github.com/juju/juju/core/network" @@ -58,11 +59,12 @@ import ( type APIGroup struct { *common.PasswordChanger - *common.LifeGetter *common.AgentEntityWatcher + *API + charmInfoAPI *charmscommon.CharmInfoAPI appCharmInfoAPI *charmscommon.ApplicationCharmInfoAPI - *API + lifeCanRead common.GetAuthFunc } type NewResourceOpenerFunc func(appName string) (resources.Opener, error) @@ -185,16 +187,54 @@ func NewStateCAASApplicationProvisionerAPI(ctx facade.ModelContext) (*APIGroup, apiGroup := &APIGroup{ PasswordChanger: common.NewPasswordChanger(st, common.AuthFuncForTagKind(names.ApplicationTagKind)), - LifeGetter: common.NewLifeGetter(st, lifeCanRead), AgentEntityWatcher: common.NewAgentEntityWatcher(st, ctx.Resources(), common.AuthFuncForTagKind(names.ApplicationTagKind)), charmInfoAPI: commonCharmsAPI, appCharmInfoAPI: appCharmInfoAPI, + lifeCanRead: lifeCanRead, API: api, } return apiGroup, nil } +// Life returns the life status of every supplied app or unit, where available. +func (a *APIGroup) Life(ctx context.Context, args params.Entities) (params.LifeResults, error) { + result := params.LifeResults{ + Results: make([]params.LifeResult, len(args.Entities)), + } + if len(args.Entities) == 0 { + return result, nil + } + canRead, err := a.lifeCanRead() + if err != nil { + return params.LifeResults{}, errors.Trace(err) + } + for i, entity := range args.Entities { + tag, err := names.ParseTag(entity.Tag) + if err != nil { + result.Results[i].Error = apiservererrors.ServerError(apiservererrors.ErrPerm) + continue + } + if !canRead(tag) { + result.Results[i].Error = apiservererrors.ServerError(apiservererrors.ErrPerm) + continue + } + var lifeValue life.Value + switch tag.Kind() { + case names.ApplicationTagKind: + lifeValue, err = a.applicationService.GetApplicationLife(ctx, tag.Id()) + case names.UnitTagKind: + lifeValue, err = a.applicationService.GetUnitLife(ctx, tag.Id()) + default: + result.Results[i].Error = apiservererrors.ServerError(apiservererrors.ErrPerm) + continue + } + result.Results[i].Life = lifeValue + result.Results[i].Error = apiservererrors.ServerError(err) + } + return result, nil +} + // CharmInfo returns information about the requested charm. func (a *APIGroup) CharmInfo(ctx context.Context, args params.CharmURL) (params.Charm, error) { return a.charmInfoAPI.CharmInfo(ctx, args) @@ -872,6 +912,7 @@ func (a *API) UpdateApplicationsUnits(ctx context.Context, args params.UpdateApp result.Results[i].Error = apiservererrors.ServerError(err) continue } + // app, err := a.state.Application(appTag.Id()) if err != nil { result.Results[i].Error = apiservererrors.ServerError(err) diff --git a/apiserver/facades/controller/caasapplicationprovisioner/service.go b/apiserver/facades/controller/caasapplicationprovisioner/service.go index be818ffad4b..c1d463754a6 100644 --- a/apiserver/facades/controller/caasapplicationprovisioner/service.go +++ b/apiserver/facades/controller/caasapplicationprovisioner/service.go @@ -8,6 +8,7 @@ import ( "github.com/juju/juju/controller" "github.com/juju/juju/core/leadership" + "github.com/juju/juju/core/life" "github.com/juju/juju/core/model" "github.com/juju/juju/core/watcher" "github.com/juju/juju/domain/application/service" @@ -45,6 +46,8 @@ type ApplicationService interface { SetApplicationScalingState(ctx context.Context, name string, scaleTarget int, scaling bool) error GetApplicationScalingState(ctx context.Context, name string) (service.ScalingState, error) GetApplicationScale(ctx context.Context, name string) (int, error) + GetApplicationLife(ctx context.Context, name string) (life.Value, error) + GetUnitLife(ctx context.Context, name string) (life.Value, error) DestroyUnit(ctx context.Context, name string) error RemoveUnit(ctx context.Context, unitName string, leadershipRevoker leadership.Revoker) error } diff --git a/apiserver/facades/controller/caasapplicationprovisioner/service_mock_test.go b/apiserver/facades/controller/caasapplicationprovisioner/service_mock_test.go index 183802d0bd9..98b8582322b 100644 --- a/apiserver/facades/controller/caasapplicationprovisioner/service_mock_test.go +++ b/apiserver/facades/controller/caasapplicationprovisioner/service_mock_test.go @@ -15,6 +15,7 @@ import ( controller "github.com/juju/juju/controller" leadership "github.com/juju/juju/core/leadership" + life "github.com/juju/juju/core/life" model "github.com/juju/juju/core/model" watcher "github.com/juju/juju/core/watcher" service "github.com/juju/juju/domain/application/service" @@ -203,6 +204,21 @@ func (mr *MockApplicationServiceMockRecorder) DestroyUnit(arg0, arg1 any) *gomoc return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "DestroyUnit", reflect.TypeOf((*MockApplicationService)(nil).DestroyUnit), arg0, arg1) } +// GetApplicationLife mocks base method. +func (m *MockApplicationService) GetApplicationLife(arg0 context.Context, arg1 string) (life.Value, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "GetApplicationLife", arg0, arg1) + ret0, _ := ret[0].(life.Value) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// GetApplicationLife indicates an expected call of GetApplicationLife. +func (mr *MockApplicationServiceMockRecorder) GetApplicationLife(arg0, arg1 any) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetApplicationLife", reflect.TypeOf((*MockApplicationService)(nil).GetApplicationLife), arg0, arg1) +} + // GetApplicationScale mocks base method. func (m *MockApplicationService) GetApplicationScale(arg0 context.Context, arg1 string) (int, error) { m.ctrl.T.Helper() @@ -233,6 +249,21 @@ func (mr *MockApplicationServiceMockRecorder) GetApplicationScalingState(arg0, a return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetApplicationScalingState", reflect.TypeOf((*MockApplicationService)(nil).GetApplicationScalingState), arg0, arg1) } +// GetUnitLife mocks base method. +func (m *MockApplicationService) GetUnitLife(arg0 context.Context, arg1 string) (life.Value, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "GetUnitLife", arg0, arg1) + ret0, _ := ret[0].(life.Value) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// GetUnitLife indicates an expected call of GetUnitLife. +func (mr *MockApplicationServiceMockRecorder) GetUnitLife(arg0, arg1 any) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetUnitLife", reflect.TypeOf((*MockApplicationService)(nil).GetUnitLife), arg0, arg1) +} + // RemoveUnit mocks base method. func (m *MockApplicationService) RemoveUnit(arg0 context.Context, arg1 string, arg2 leadership.Revoker) error { m.ctrl.T.Helper() diff --git a/apiserver/facades/controller/caasfirewaller/common_service_mocks_test.go b/apiserver/facades/controller/caasfirewaller/common_service_mocks_test.go new file mode 100644 index 00000000000..70fbd3d8743 --- /dev/null +++ b/apiserver/facades/controller/caasfirewaller/common_service_mocks_test.go @@ -0,0 +1,122 @@ +// Code generated by MockGen. DO NOT EDIT. +// Source: github.com/juju/juju/apiserver/common/charms (interfaces: CharmService) +// +// Generated by this command: +// +// mockgen -typed -package caasfirewaller_test -destination common_service_mocks_test.go github.com/juju/juju/apiserver/common/charms CharmService +// + +// Package caasfirewaller_test is a generated GoMock package. +package caasfirewaller_test + +import ( + context "context" + reflect "reflect" + + charm "github.com/juju/juju/core/charm" + charm0 "github.com/juju/juju/domain/application/charm" + charm1 "github.com/juju/juju/internal/charm" + gomock "go.uber.org/mock/gomock" +) + +// MockCharmService is a mock of CharmService interface. +type MockCharmService struct { + ctrl *gomock.Controller + recorder *MockCharmServiceMockRecorder +} + +// MockCharmServiceMockRecorder is the mock recorder for MockCharmService. +type MockCharmServiceMockRecorder struct { + mock *MockCharmService +} + +// NewMockCharmService creates a new mock instance. +func NewMockCharmService(ctrl *gomock.Controller) *MockCharmService { + mock := &MockCharmService{ctrl: ctrl} + mock.recorder = &MockCharmServiceMockRecorder{mock} + return mock +} + +// EXPECT returns an object that allows the caller to indicate expected use. +func (m *MockCharmService) EXPECT() *MockCharmServiceMockRecorder { + return m.recorder +} + +// GetCharm mocks base method. +func (m *MockCharmService) GetCharm(arg0 context.Context, arg1 charm.ID) (charm1.Charm, charm0.CharmOrigin, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "GetCharm", arg0, arg1) + ret0, _ := ret[0].(charm1.Charm) + ret1, _ := ret[1].(charm0.CharmOrigin) + ret2, _ := ret[2].(error) + return ret0, ret1, ret2 +} + +// GetCharm indicates an expected call of GetCharm. +func (mr *MockCharmServiceMockRecorder) GetCharm(arg0, arg1 any) *MockCharmServiceGetCharmCall { + mr.mock.ctrl.T.Helper() + call := mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetCharm", reflect.TypeOf((*MockCharmService)(nil).GetCharm), arg0, arg1) + return &MockCharmServiceGetCharmCall{Call: call} +} + +// MockCharmServiceGetCharmCall wrap *gomock.Call +type MockCharmServiceGetCharmCall struct { + *gomock.Call +} + +// Return rewrite *gomock.Call.Return +func (c *MockCharmServiceGetCharmCall) Return(arg0 charm1.Charm, arg1 charm0.CharmOrigin, arg2 error) *MockCharmServiceGetCharmCall { + c.Call = c.Call.Return(arg0, arg1, arg2) + return c +} + +// Do rewrite *gomock.Call.Do +func (c *MockCharmServiceGetCharmCall) Do(f func(context.Context, charm.ID) (charm1.Charm, charm0.CharmOrigin, error)) *MockCharmServiceGetCharmCall { + c.Call = c.Call.Do(f) + return c +} + +// DoAndReturn rewrite *gomock.Call.DoAndReturn +func (c *MockCharmServiceGetCharmCall) DoAndReturn(f func(context.Context, charm.ID) (charm1.Charm, charm0.CharmOrigin, error)) *MockCharmServiceGetCharmCall { + c.Call = c.Call.DoAndReturn(f) + return c +} + +// GetCharmID mocks base method. +func (m *MockCharmService) GetCharmID(arg0 context.Context, arg1 charm0.GetCharmArgs) (charm.ID, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "GetCharmID", arg0, arg1) + ret0, _ := ret[0].(charm.ID) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// GetCharmID indicates an expected call of GetCharmID. +func (mr *MockCharmServiceMockRecorder) GetCharmID(arg0, arg1 any) *MockCharmServiceGetCharmIDCall { + mr.mock.ctrl.T.Helper() + call := mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetCharmID", reflect.TypeOf((*MockCharmService)(nil).GetCharmID), arg0, arg1) + return &MockCharmServiceGetCharmIDCall{Call: call} +} + +// MockCharmServiceGetCharmIDCall wrap *gomock.Call +type MockCharmServiceGetCharmIDCall struct { + *gomock.Call +} + +// Return rewrite *gomock.Call.Return +func (c *MockCharmServiceGetCharmIDCall) Return(arg0 charm.ID, arg1 error) *MockCharmServiceGetCharmIDCall { + c.Call = c.Call.Return(arg0, arg1) + return c +} + +// Do rewrite *gomock.Call.Do +func (c *MockCharmServiceGetCharmIDCall) Do(f func(context.Context, charm0.GetCharmArgs) (charm.ID, error)) *MockCharmServiceGetCharmIDCall { + c.Call = c.Call.Do(f) + return c +} + +// DoAndReturn rewrite *gomock.Call.DoAndReturn +func (c *MockCharmServiceGetCharmIDCall) DoAndReturn(f func(context.Context, charm0.GetCharmArgs) (charm.ID, error)) *MockCharmServiceGetCharmIDCall { + c.Call = c.Call.DoAndReturn(f) + return c +} diff --git a/apiserver/facades/controller/caasfirewaller/firewaller.go b/apiserver/facades/controller/caasfirewaller/firewaller.go index 9e3b7e23907..5c35dda5f7d 100644 --- a/apiserver/facades/controller/caasfirewaller/firewaller.go +++ b/apiserver/facades/controller/caasfirewaller/firewaller.go @@ -14,19 +14,31 @@ import ( charmscommon "github.com/juju/juju/apiserver/common/charms" apiservererrors "github.com/juju/juju/apiserver/errors" "github.com/juju/juju/apiserver/facade" + "github.com/juju/juju/core/life" "github.com/juju/juju/core/network" + "github.com/juju/juju/domain/application" + applicationcharm "github.com/juju/juju/domain/application/charm" + "github.com/juju/juju/internal/charm" "github.com/juju/juju/rpc/params" "github.com/juju/juju/state/watcher" ) +// ApplicationService provides access to the application service. +type ApplicationService interface { + GetCharmByApplicationName(context.Context, string) (charm.Charm, applicationcharm.CharmOrigin, application.Platform, error) + GetApplicationLife(context.Context, string) (life.Value, error) + GetUnitLife(context.Context, string) (life.Value, error) +} type Facade struct { - *common.LifeGetter *common.AgentEntityWatcher resources facade.Resources state CAASFirewallerState charmInfoAPI *charmscommon.CharmInfoAPI appCharmInfoAPI *charmscommon.ApplicationCharmInfoAPI accessModel common.GetAuthFunc + accessUnit common.GetAuthFunc + + applicationService ApplicationService } // CharmInfo returns information about the requested charm. @@ -112,32 +124,71 @@ func NewFacade( st CAASFirewallerState, commonCharmsAPI *charmscommon.CharmInfoAPI, appCharmInfoAPI *charmscommon.ApplicationCharmInfoAPI, + applicationService ApplicationService, ) (*Facade, error) { if !authorizer.AuthController() { return nil, apiservererrors.ErrPerm } accessApplication := common.AuthFuncForTagKind(names.ApplicationTagKind) + accessUnit := common.AuthAny( + common.AuthFuncForTagKind(names.ApplicationTagKind), + common.AuthFuncForTagKind(names.UnitTagKind), + ) return &Facade{ accessModel: common.AuthFuncForTagKind(names.ModelTagKind), - LifeGetter: common.NewLifeGetter( - st, common.AuthAny( - common.AuthFuncForTagKind(names.ApplicationTagKind), - common.AuthFuncForTagKind(names.UnitTagKind), - ), - ), + accessUnit: accessUnit, AgentEntityWatcher: common.NewAgentEntityWatcher( st, resources, accessApplication, ), - resources: resources, - state: st, - charmInfoAPI: commonCharmsAPI, - appCharmInfoAPI: appCharmInfoAPI, + resources: resources, + state: st, + charmInfoAPI: commonCharmsAPI, + appCharmInfoAPI: appCharmInfoAPI, + applicationService: applicationService, }, nil } +// Life returns the life status of the specified units. +func (f *Facade) Life(ctx context.Context, args params.Entities) (params.LifeResults, error) { + result := params.LifeResults{ + Results: make([]params.LifeResult, len(args.Entities)), + } + if len(args.Entities) == 0 { + return result, nil + } + canRead, err := f.accessUnit() + if err != nil { + return params.LifeResults{}, errors.Trace(err) + } + for i, entity := range args.Entities { + tag, err := names.ParseTag(entity.Tag) + if err != nil { + result.Results[i].Error = apiservererrors.ServerError(apiservererrors.ErrPerm) + continue + } + if !canRead(tag) { + result.Results[i].Error = apiservererrors.ServerError(apiservererrors.ErrPerm) + continue + } + var lifeValue life.Value + switch tag.Kind() { + case names.ApplicationTagKind: + lifeValue, err = f.applicationService.GetApplicationLife(ctx, tag.Id()) + case names.UnitTagKind: + lifeValue, err = f.applicationService.GetUnitLife(ctx, tag.Id()) + default: + result.Results[i].Error = apiservererrors.ServerError(apiservererrors.ErrPerm) + continue + } + result.Results[i].Life = lifeValue + result.Results[i].Error = apiservererrors.ServerError(err) + } + return result, nil +} + // WatchOpenedPorts returns a new StringsWatcher for each given // model tag. func (f *Facade) WatchOpenedPorts(ctx context.Context, args params.Entities) (params.StringsWatchResults, error) { diff --git a/apiserver/facades/controller/caasfirewaller/firewaller_test.go b/apiserver/facades/controller/caasfirewaller/firewaller_test.go index ac05b7d010d..fc0c33fdb49 100644 --- a/apiserver/facades/controller/caasfirewaller/firewaller_test.go +++ b/apiserver/facades/controller/caasfirewaller/firewaller_test.go @@ -20,7 +20,6 @@ import ( "github.com/juju/juju/core/network" coretesting "github.com/juju/juju/internal/testing" "github.com/juju/juju/rpc/params" - "github.com/juju/juju/state" statetesting "github.com/juju/juju/state/testing" ) @@ -57,7 +56,6 @@ func (s *firewallerSuite) SetUpTest(c *gc.C) { appExposedWatcher := statetesting.NewMockNotifyWatcher(s.appExposedChanges) s.st = &mockState{ application: mockApplication{ - life: state.Alive, watcher: appExposedWatcher, }, applicationsWatcher: statetesting.NewMockStringsWatcher(s.applicationsChanges), @@ -152,6 +150,7 @@ func (s *firewallerSuite) TestPermission(c *gc.C) { s.st, commonCharmsAPI, appCharmInfoAPI, + s.appService, ) c.Assert(err, gc.ErrorMatches, "permission denied") } @@ -217,6 +216,8 @@ func (s *firewallerSuite) TestIsExposed(c *gc.C) { func (s *firewallerSuite) TestLife(c *gc.C) { defer s.setupMocks(c).Finish() + s.appService.EXPECT().GetApplicationLife(gomock.Any(), "gitlab").Return(life.Alive, nil) + results, err := s.facade.Life(context.Background(), params.Entities{ Entities: []params.Entity{ {Tag: "application-gitlab"}, @@ -271,6 +272,7 @@ func (s *firewallerSuite) setupMocks(c *gc.C) *gomock.Controller { s.st, commonCharmsAPI, appCharmInfoAPI, + s.appService, ) c.Assert(err, jc.ErrorIsNil) diff --git a/apiserver/facades/controller/caasfirewaller/mock_test.go b/apiserver/facades/controller/caasfirewaller/mock_test.go index 77b17c5be8e..22ac49b479f 100644 --- a/apiserver/facades/controller/caasfirewaller/mock_test.go +++ b/apiserver/facades/controller/caasfirewaller/mock_test.go @@ -67,18 +67,12 @@ func (st *mockState) Model() (*state.Model, error) { type mockApplication struct { testing.Stub state.Entity // Pull in Tag method (which tests don't use) - life state.Life exposed bool watcher state.NotifyWatcher appPortRanges network.GroupedPortRanges } -func (a *mockApplication) Life() state.Life { - a.MethodCall(a, "Life") - return a.life -} - func (a *mockApplication) IsExposed() bool { a.MethodCall(a, "IsExposed") return a.exposed diff --git a/apiserver/facades/controller/caasfirewaller/package_test.go b/apiserver/facades/controller/caasfirewaller/package_test.go index cfb925ce95e..18a04882d59 100644 --- a/apiserver/facades/controller/caasfirewaller/package_test.go +++ b/apiserver/facades/controller/caasfirewaller/package_test.go @@ -9,7 +9,8 @@ import ( gc "gopkg.in/check.v1" ) -//go:generate go run go.uber.org/mock/mockgen -typed -package caasfirewaller_test -destination service_mocks_test.go github.com/juju/juju/apiserver/common/charms CharmService,ApplicationService +//go:generate go run go.uber.org/mock/mockgen -typed -package caasfirewaller_test -destination common_service_mocks_test.go github.com/juju/juju/apiserver/common/charms CharmService +//go:generate go run go.uber.org/mock/mockgen -typed -package caasfirewaller_test -destination service_mocks_test.go github.com/juju/juju/apiserver/facades/controller/caasfirewaller ApplicationService func TestAll(t *testing.T) { gc.TestingT(t) diff --git a/apiserver/facades/controller/caasfirewaller/register.go b/apiserver/facades/controller/caasfirewaller/register.go index f1a7b68cfe5..b8d9298fee9 100644 --- a/apiserver/facades/controller/caasfirewaller/register.go +++ b/apiserver/facades/controller/caasfirewaller/register.go @@ -50,5 +50,6 @@ func newStateFacade(ctx facade.ModelContext) (*Facade, error) { &stateShim{State: ctx.State()}, commonCharmsAPI, appCharmInfoAPI, + applicationService, ) } diff --git a/apiserver/facades/controller/caasfirewaller/service_mocks_test.go b/apiserver/facades/controller/caasfirewaller/service_mocks_test.go index 0617cbcd594..7c0ef929267 100644 --- a/apiserver/facades/controller/caasfirewaller/service_mocks_test.go +++ b/apiserver/facades/controller/caasfirewaller/service_mocks_test.go @@ -1,9 +1,9 @@ // Code generated by MockGen. DO NOT EDIT. -// Source: github.com/juju/juju/apiserver/common/charms (interfaces: CharmService,ApplicationService) +// Source: github.com/juju/juju/apiserver/facades/controller/caasfirewaller (interfaces: ApplicationService) // // Generated by this command: // -// mockgen -typed -package caasfirewaller_test -destination service_mocks_test.go github.com/juju/juju/apiserver/common/charms CharmService,ApplicationService +// mockgen -typed -package caasfirewaller_test -destination service_mocks_test.go github.com/juju/juju/apiserver/facades/controller/caasfirewaller ApplicationService // // Package caasfirewaller_test is a generated GoMock package. @@ -13,174 +13,150 @@ import ( context "context" reflect "reflect" - charm "github.com/juju/juju/core/charm" - charm0 "github.com/juju/juju/domain/application/charm" - charm1 "github.com/juju/juju/internal/charm" + life "github.com/juju/juju/core/life" + charm "github.com/juju/juju/domain/application/charm" + charm0 "github.com/juju/juju/internal/charm" gomock "go.uber.org/mock/gomock" ) -// MockCharmService is a mock of CharmService interface. -type MockCharmService struct { +// MockApplicationService is a mock of ApplicationService interface. +type MockApplicationService struct { ctrl *gomock.Controller - recorder *MockCharmServiceMockRecorder + recorder *MockApplicationServiceMockRecorder } -// MockCharmServiceMockRecorder is the mock recorder for MockCharmService. -type MockCharmServiceMockRecorder struct { - mock *MockCharmService +// MockApplicationServiceMockRecorder is the mock recorder for MockApplicationService. +type MockApplicationServiceMockRecorder struct { + mock *MockApplicationService } -// NewMockCharmService creates a new mock instance. -func NewMockCharmService(ctrl *gomock.Controller) *MockCharmService { - mock := &MockCharmService{ctrl: ctrl} - mock.recorder = &MockCharmServiceMockRecorder{mock} +// NewMockApplicationService creates a new mock instance. +func NewMockApplicationService(ctrl *gomock.Controller) *MockApplicationService { + mock := &MockApplicationService{ctrl: ctrl} + mock.recorder = &MockApplicationServiceMockRecorder{mock} return mock } // EXPECT returns an object that allows the caller to indicate expected use. -func (m *MockCharmService) EXPECT() *MockCharmServiceMockRecorder { +func (m *MockApplicationService) EXPECT() *MockApplicationServiceMockRecorder { return m.recorder } -// GetCharm mocks base method. -func (m *MockCharmService) GetCharm(arg0 context.Context, arg1 charm.ID) (charm1.Charm, charm0.CharmOrigin, error) { +// GetApplicationLife mocks base method. +func (m *MockApplicationService) GetApplicationLife(arg0 context.Context, arg1 string) (life.Value, error) { m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "GetCharm", arg0, arg1) - ret0, _ := ret[0].(charm1.Charm) - ret1, _ := ret[1].(charm0.CharmOrigin) - ret2, _ := ret[2].(error) - return ret0, ret1, ret2 + ret := m.ctrl.Call(m, "GetApplicationLife", arg0, arg1) + ret0, _ := ret[0].(life.Value) + ret1, _ := ret[1].(error) + return ret0, ret1 } -// GetCharm indicates an expected call of GetCharm. -func (mr *MockCharmServiceMockRecorder) GetCharm(arg0, arg1 any) *MockCharmServiceGetCharmCall { +// GetApplicationLife indicates an expected call of GetApplicationLife. +func (mr *MockApplicationServiceMockRecorder) GetApplicationLife(arg0, arg1 any) *MockApplicationServiceGetApplicationLifeCall { mr.mock.ctrl.T.Helper() - call := mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetCharm", reflect.TypeOf((*MockCharmService)(nil).GetCharm), arg0, arg1) - return &MockCharmServiceGetCharmCall{Call: call} + call := mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetApplicationLife", reflect.TypeOf((*MockApplicationService)(nil).GetApplicationLife), arg0, arg1) + return &MockApplicationServiceGetApplicationLifeCall{Call: call} } -// MockCharmServiceGetCharmCall wrap *gomock.Call -type MockCharmServiceGetCharmCall struct { +// MockApplicationServiceGetApplicationLifeCall wrap *gomock.Call +type MockApplicationServiceGetApplicationLifeCall struct { *gomock.Call } // Return rewrite *gomock.Call.Return -func (c *MockCharmServiceGetCharmCall) Return(arg0 charm1.Charm, arg1 charm0.CharmOrigin, arg2 error) *MockCharmServiceGetCharmCall { - c.Call = c.Call.Return(arg0, arg1, arg2) +func (c *MockApplicationServiceGetApplicationLifeCall) Return(arg0 life.Value, arg1 error) *MockApplicationServiceGetApplicationLifeCall { + c.Call = c.Call.Return(arg0, arg1) return c } // Do rewrite *gomock.Call.Do -func (c *MockCharmServiceGetCharmCall) Do(f func(context.Context, charm.ID) (charm1.Charm, charm0.CharmOrigin, error)) *MockCharmServiceGetCharmCall { +func (c *MockApplicationServiceGetApplicationLifeCall) Do(f func(context.Context, string) (life.Value, error)) *MockApplicationServiceGetApplicationLifeCall { c.Call = c.Call.Do(f) return c } // DoAndReturn rewrite *gomock.Call.DoAndReturn -func (c *MockCharmServiceGetCharmCall) DoAndReturn(f func(context.Context, charm.ID) (charm1.Charm, charm0.CharmOrigin, error)) *MockCharmServiceGetCharmCall { +func (c *MockApplicationServiceGetApplicationLifeCall) DoAndReturn(f func(context.Context, string) (life.Value, error)) *MockApplicationServiceGetApplicationLifeCall { c.Call = c.Call.DoAndReturn(f) return c } -// GetCharmID mocks base method. -func (m *MockCharmService) GetCharmID(arg0 context.Context, arg1 charm0.GetCharmArgs) (charm.ID, error) { +// GetCharmByApplicationName mocks base method. +func (m *MockApplicationService) GetCharmByApplicationName(arg0 context.Context, arg1 string) (charm0.Charm, charm.CharmOrigin, charm.Platform, error) { m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "GetCharmID", arg0, arg1) - ret0, _ := ret[0].(charm.ID) - ret1, _ := ret[1].(error) - return ret0, ret1 + ret := m.ctrl.Call(m, "GetCharmByApplicationName", arg0, arg1) + ret0, _ := ret[0].(charm0.Charm) + ret1, _ := ret[1].(charm.CharmOrigin) + ret2, _ := ret[2].(charm.Platform) + ret3, _ := ret[3].(error) + return ret0, ret1, ret2, ret3 } -// GetCharmID indicates an expected call of GetCharmID. -func (mr *MockCharmServiceMockRecorder) GetCharmID(arg0, arg1 any) *MockCharmServiceGetCharmIDCall { +// GetCharmByApplicationName indicates an expected call of GetCharmByApplicationName. +func (mr *MockApplicationServiceMockRecorder) GetCharmByApplicationName(arg0, arg1 any) *MockApplicationServiceGetCharmByApplicationNameCall { mr.mock.ctrl.T.Helper() - call := mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetCharmID", reflect.TypeOf((*MockCharmService)(nil).GetCharmID), arg0, arg1) - return &MockCharmServiceGetCharmIDCall{Call: call} + call := mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetCharmByApplicationName", reflect.TypeOf((*MockApplicationService)(nil).GetCharmByApplicationName), arg0, arg1) + return &MockApplicationServiceGetCharmByApplicationNameCall{Call: call} } -// MockCharmServiceGetCharmIDCall wrap *gomock.Call -type MockCharmServiceGetCharmIDCall struct { +// MockApplicationServiceGetCharmByApplicationNameCall wrap *gomock.Call +type MockApplicationServiceGetCharmByApplicationNameCall struct { *gomock.Call } // Return rewrite *gomock.Call.Return -func (c *MockCharmServiceGetCharmIDCall) Return(arg0 charm.ID, arg1 error) *MockCharmServiceGetCharmIDCall { - c.Call = c.Call.Return(arg0, arg1) +func (c *MockApplicationServiceGetCharmByApplicationNameCall) Return(arg0 charm0.Charm, arg1 charm.CharmOrigin, arg2 charm.Platform, arg3 error) *MockApplicationServiceGetCharmByApplicationNameCall { + c.Call = c.Call.Return(arg0, arg1, arg2, arg3) return c } // Do rewrite *gomock.Call.Do -func (c *MockCharmServiceGetCharmIDCall) Do(f func(context.Context, charm0.GetCharmArgs) (charm.ID, error)) *MockCharmServiceGetCharmIDCall { +func (c *MockApplicationServiceGetCharmByApplicationNameCall) Do(f func(context.Context, string) (charm0.Charm, charm.CharmOrigin, charm.Platform, error)) *MockApplicationServiceGetCharmByApplicationNameCall { c.Call = c.Call.Do(f) return c } // DoAndReturn rewrite *gomock.Call.DoAndReturn -func (c *MockCharmServiceGetCharmIDCall) DoAndReturn(f func(context.Context, charm0.GetCharmArgs) (charm.ID, error)) *MockCharmServiceGetCharmIDCall { +func (c *MockApplicationServiceGetCharmByApplicationNameCall) DoAndReturn(f func(context.Context, string) (charm0.Charm, charm.CharmOrigin, charm.Platform, error)) *MockApplicationServiceGetCharmByApplicationNameCall { c.Call = c.Call.DoAndReturn(f) return c } -// MockApplicationService is a mock of ApplicationService interface. -type MockApplicationService struct { - ctrl *gomock.Controller - recorder *MockApplicationServiceMockRecorder -} - -// MockApplicationServiceMockRecorder is the mock recorder for MockApplicationService. -type MockApplicationServiceMockRecorder struct { - mock *MockApplicationService -} - -// NewMockApplicationService creates a new mock instance. -func NewMockApplicationService(ctrl *gomock.Controller) *MockApplicationService { - mock := &MockApplicationService{ctrl: ctrl} - mock.recorder = &MockApplicationServiceMockRecorder{mock} - return mock -} - -// EXPECT returns an object that allows the caller to indicate expected use. -func (m *MockApplicationService) EXPECT() *MockApplicationServiceMockRecorder { - return m.recorder -} - -// GetCharmByApplicationName mocks base method. -func (m *MockApplicationService) GetCharmByApplicationName(arg0 context.Context, arg1 string) (charm1.Charm, charm0.CharmOrigin, charm0.Platform, error) { +// GetUnitLife mocks base method. +func (m *MockApplicationService) GetUnitLife(arg0 context.Context, arg1 string) (life.Value, error) { m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "GetCharmByApplicationName", arg0, arg1) - ret0, _ := ret[0].(charm1.Charm) - ret1, _ := ret[1].(charm0.CharmOrigin) - ret2, _ := ret[2].(charm0.Platform) - ret3, _ := ret[3].(error) - return ret0, ret1, ret2, ret3 + ret := m.ctrl.Call(m, "GetUnitLife", arg0, arg1) + ret0, _ := ret[0].(life.Value) + ret1, _ := ret[1].(error) + return ret0, ret1 } -// GetCharmByApplicationName indicates an expected call of GetCharmByApplicationName. -func (mr *MockApplicationServiceMockRecorder) GetCharmByApplicationName(arg0, arg1 any) *MockApplicationServiceGetCharmByApplicationNameCall { +// GetUnitLife indicates an expected call of GetUnitLife. +func (mr *MockApplicationServiceMockRecorder) GetUnitLife(arg0, arg1 any) *MockApplicationServiceGetUnitLifeCall { mr.mock.ctrl.T.Helper() - call := mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetCharmByApplicationName", reflect.TypeOf((*MockApplicationService)(nil).GetCharmByApplicationName), arg0, arg1) - return &MockApplicationServiceGetCharmByApplicationNameCall{Call: call} + call := mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetUnitLife", reflect.TypeOf((*MockApplicationService)(nil).GetUnitLife), arg0, arg1) + return &MockApplicationServiceGetUnitLifeCall{Call: call} } -// MockApplicationServiceGetCharmByApplicationNameCall wrap *gomock.Call -type MockApplicationServiceGetCharmByApplicationNameCall struct { +// MockApplicationServiceGetUnitLifeCall wrap *gomock.Call +type MockApplicationServiceGetUnitLifeCall struct { *gomock.Call } // Return rewrite *gomock.Call.Return -func (c *MockApplicationServiceGetCharmByApplicationNameCall) Return(arg0 charm1.Charm, arg1 charm0.CharmOrigin, arg2 charm0.Platform, arg3 error) *MockApplicationServiceGetCharmByApplicationNameCall { - c.Call = c.Call.Return(arg0, arg1, arg2, arg3) +func (c *MockApplicationServiceGetUnitLifeCall) Return(arg0 life.Value, arg1 error) *MockApplicationServiceGetUnitLifeCall { + c.Call = c.Call.Return(arg0, arg1) return c } // Do rewrite *gomock.Call.Do -func (c *MockApplicationServiceGetCharmByApplicationNameCall) Do(f func(context.Context, string) (charm1.Charm, charm0.CharmOrigin, charm0.Platform, error)) *MockApplicationServiceGetCharmByApplicationNameCall { +func (c *MockApplicationServiceGetUnitLifeCall) Do(f func(context.Context, string) (life.Value, error)) *MockApplicationServiceGetUnitLifeCall { c.Call = c.Call.Do(f) return c } // DoAndReturn rewrite *gomock.Call.DoAndReturn -func (c *MockApplicationServiceGetCharmByApplicationNameCall) DoAndReturn(f func(context.Context, string) (charm1.Charm, charm0.CharmOrigin, charm0.Platform, error)) *MockApplicationServiceGetCharmByApplicationNameCall { +func (c *MockApplicationServiceGetUnitLifeCall) DoAndReturn(f func(context.Context, string) (life.Value, error)) *MockApplicationServiceGetUnitLifeCall { c.Call = c.Call.DoAndReturn(f) return c } diff --git a/apiserver/facades/controller/crossmodelrelations/crossmodelrelations.go b/apiserver/facades/controller/crossmodelrelations/crossmodelrelations.go index 77ce2758bc9..8928f3022d6 100644 --- a/apiserver/facades/controller/crossmodelrelations/crossmodelrelations.go +++ b/apiserver/facades/controller/crossmodelrelations/crossmodelrelations.go @@ -222,6 +222,7 @@ func (api *CrossModelRelationsAPIv3) registerRemoteRelation(ctx context.Context, if err != nil { return nil, errors.Annotatef(err, "cannot get application for offer %q", relation.OfferUUID) } + // TODO(units) - XXXX if localApp.Life() != state.Alive { // We don't want to leak the application name so just log it. api.logger.Warningf("local application for offer %v not found", localApplicationName) diff --git a/apiserver/facades/controller/firewaller/firewaller.go b/apiserver/facades/controller/firewaller/firewaller.go index b15621601ff..e14ff69ef63 100644 --- a/apiserver/facades/controller/firewaller/firewaller.go +++ b/apiserver/facades/controller/firewaller/firewaller.go @@ -19,6 +19,7 @@ import ( "github.com/juju/juju/apiserver/facade" "github.com/juju/juju/apiserver/internal" "github.com/juju/juju/controller" + "github.com/juju/juju/core/life" corelogger "github.com/juju/juju/core/logger" "github.com/juju/juju/core/network" "github.com/juju/juju/core/status" @@ -53,16 +54,18 @@ type FirewallerAPI struct { ControllerConfigAPI cloudspec.CloudSpecer - st State - networkService NetworkService - resources facade.Resources - watcherRegistry facade.WatcherRegistry - authorizer facade.Authorizer - accessUnit common.GetAuthFunc - accessApplication common.GetAuthFunc - accessMachine common.GetAuthFunc - accessModel common.GetAuthFunc - logger corelogger.Logger + st State + networkService NetworkService + applicationService ApplicationService + resources facade.Resources + watcherRegistry facade.WatcherRegistry + authorizer facade.Authorizer + accessUnit common.GetAuthFunc + accessApplication common.GetAuthFunc + accessMachine common.GetAuthFunc + accessModel common.GetAuthFunc + accessUnitApplicationOrMachineOrRelation common.GetAuthFunc + logger corelogger.Logger // Fetched on demand and memoized appEndpointBindings map[string]map[string]string @@ -82,6 +85,7 @@ func NewStateFirewallerAPI( controllerConfigAPI ControllerConfigAPI, controllerConfigService ControllerConfigService, modelConfigService ModelConfigService, + applicationService ApplicationService, logger corelogger.Logger, ) (*FirewallerAPI, error) { if !authorizer.AuthController() { @@ -131,29 +135,69 @@ func NewStateFirewallerAPI( ) return &FirewallerAPI{ - LifeGetter: lifeGetter, - ModelWatcher: modelWatcher, - AgentEntityWatcher: entityWatcher, - UnitsWatcher: unitsWatcher, - ModelMachinesWatcher: machinesWatcher, - InstanceIdGetter: instanceIdGetter, - CloudSpecer: cloudSpecAPI, - ControllerConfigAPI: controllerConfigAPI, - st: st, - resources: resources, - watcherRegistry: watcherRegistry, - authorizer: authorizer, - accessUnit: accessUnit, - accessApplication: accessApplication, - accessMachine: accessMachine, - accessModel: accessModel, - controllerConfigService: controllerConfigService, - modelConfigService: modelConfigService, - networkService: networkService, - logger: logger, + LifeGetter: lifeGetter, + ModelWatcher: modelWatcher, + AgentEntityWatcher: entityWatcher, + UnitsWatcher: unitsWatcher, + ModelMachinesWatcher: machinesWatcher, + InstanceIdGetter: instanceIdGetter, + CloudSpecer: cloudSpecAPI, + ControllerConfigAPI: controllerConfigAPI, + st: st, + resources: resources, + watcherRegistry: watcherRegistry, + authorizer: authorizer, + accessUnit: accessUnit, + accessApplication: accessApplication, + accessMachine: accessMachine, + accessUnitApplicationOrMachineOrRelation: accessUnitApplicationOrMachineOrRelation, + accessModel: accessModel, + controllerConfigService: controllerConfigService, + modelConfigService: modelConfigService, + networkService: networkService, + applicationService: applicationService, + logger: logger, }, nil } +// Life returns the life status of the specified entities. +func (f *FirewallerAPI) Life(ctx context.Context, args params.Entities) (params.LifeResults, error) { + result := params.LifeResults{ + Results: make([]params.LifeResult, len(args.Entities)), + } + if len(args.Entities) == 0 { + return result, nil + } + canRead, err := f.accessUnitApplicationOrMachineOrRelation() + if err != nil { + return params.LifeResults{}, errors.Trace(err) + } + // Entities will be machine, relation, or unit. + // For units, we use the domain application service. + // The other entity types are not ported across to dqlite yet. + for i, entity := range args.Entities { + tag, err := names.ParseTag(entity.Tag) + if err != nil { + result.Results[i].Error = apiservererrors.ServerError(apiservererrors.ErrPerm) + continue + } + if !canRead(tag) { + result.Results[i].Error = apiservererrors.ServerError(apiservererrors.ErrPerm) + continue + } + var lifeValue life.Value + switch tag.Kind() { + case names.UnitTagKind: + lifeValue, err = f.applicationService.GetUnitLife(ctx, tag.Id()) + default: + lifeValue, err = f.LifeGetter.OneLife(tag) + } + result.Results[i].Life = lifeValue + result.Results[i].Error = apiservererrors.ServerError(err) + } + return result, nil +} + // WatchOpenedPorts returns a new StringsWatcher for each given // model tag. func (f *FirewallerAPI) WatchOpenedPorts(ctx context.Context, args params.Entities) (params.StringsWatchResults, error) { diff --git a/apiserver/facades/controller/firewaller/firewaller_base_test.go b/apiserver/facades/controller/firewaller/firewaller_base_test.go index 718258d27a3..ec97c9b01f2 100644 --- a/apiserver/facades/controller/firewaller/firewaller_base_test.go +++ b/apiserver/facades/controller/firewaller/firewaller_base_test.go @@ -144,7 +144,7 @@ func (s *firewallerBaseSuite) testLife( {Life: "alive"}, {Life: "alive"}, {Error: apiservertesting.NotFoundError("machine 42")}, - {Error: apiservertesting.NotFoundError(`unit "foo/0"`)}, + {Error: ¶ms.Error{Code: "unit not found", Message: "unit not found"}}, {Error: apiservertesting.NotFoundError(`application "bar"`)}, {Error: apiservertesting.ErrUnauthorized}, {Error: apiservertesting.ErrUnauthorized}, diff --git a/apiserver/facades/controller/firewaller/firewaller_test.go b/apiserver/facades/controller/firewaller/firewaller_test.go index 0794122a101..b6cd0b8b34a 100644 --- a/apiserver/facades/controller/firewaller/firewaller_test.go +++ b/apiserver/facades/controller/firewaller/firewaller_test.go @@ -20,6 +20,7 @@ import ( "github.com/juju/juju/apiserver/facades/controller/firewaller" apiservertesting "github.com/juju/juju/apiserver/testing" "github.com/juju/juju/core/network" + applicationerrors "github.com/juju/juju/domain/application/errors" loggertesting "github.com/juju/juju/internal/logger/testing" "github.com/juju/juju/rpc/params" "github.com/juju/juju/state" @@ -37,6 +38,7 @@ type firewallerSuite struct { controllerConfigService *MockControllerConfigService modelConfigService *MockModelConfigService networkService *MockNetworkService + applicationService *MockApplicationService } var _ = gc.Suite(&firewallerSuite{}) @@ -51,6 +53,7 @@ func (s *firewallerSuite) setupMocks(c *gc.C) *gomock.Controller { s.controllerConfigService = NewMockControllerConfigService(ctrl) s.networkService = NewMockNetworkService(ctrl) s.modelConfigService = NewMockModelConfigService(ctrl) + s.applicationService = NewMockApplicationService(ctrl) return ctrl } @@ -80,6 +83,7 @@ func (s *firewallerSuite) setupAPI(c *gc.C) { s.controllerConfigAPI, s.controllerConfigService, s.modelConfigService, + s.applicationService, loggertesting.WrapCheckLog(c), ) c.Assert(err, jc.ErrorIsNil) @@ -103,6 +107,8 @@ func (s *firewallerSuite) TestLife(c *gc.C) { defer ctrl.Finish() s.setupAPI(c) + s.applicationService.EXPECT().GetUnitLife(gomock.Any(), "foo/0").Return("", applicationerrors.UnitNotFound) + s.testLife(c, s.firewaller) } diff --git a/apiserver/facades/controller/firewaller/firewaller_unit_test.go b/apiserver/facades/controller/firewaller/firewaller_unit_test.go index 24f8f0736d5..e7b2827ce5a 100644 --- a/apiserver/facades/controller/firewaller/firewaller_unit_test.go +++ b/apiserver/facades/controller/firewaller/firewaller_unit_test.go @@ -44,6 +44,7 @@ type RemoteFirewallerSuite struct { controllerConfigService *MockControllerConfigService modelConfigService *MockModelConfigService networkService *MockNetworkService + applicationService *MockApplicationService } func (s *RemoteFirewallerSuite) SetUpTest(c *gc.C) { @@ -69,6 +70,7 @@ func (s *RemoteFirewallerSuite) setupMocks(c *gc.C) *gomock.Controller { s.controllerConfigService = NewMockControllerConfigService(ctrl) s.modelConfigService = NewMockModelConfigService(ctrl) s.networkService = NewMockNetworkService(ctrl) + s.applicationService = NewMockApplicationService(ctrl) return ctrl } @@ -85,6 +87,7 @@ func (s *RemoteFirewallerSuite) setupAPI(c *gc.C) { s.controllerConfigAPI, s.controllerConfigService, s.modelConfigService, + s.applicationService, loggertesting.WrapCheckLog(c), ) c.Assert(err, jc.ErrorIsNil) @@ -179,6 +182,7 @@ type FirewallerSuite struct { controllerConfigService *MockControllerConfigService modelConfigService *MockModelConfigService networkService *MockNetworkService + applicationService *MockApplicationService } func (s *FirewallerSuite) SetUpTest(c *gc.C) { @@ -201,6 +205,7 @@ func (s *FirewallerSuite) setupMocks(c *gc.C) *gomock.Controller { s.controllerConfigService = NewMockControllerConfigService(ctrl) s.modelConfigService = NewMockModelConfigService(ctrl) s.networkService = NewMockNetworkService(ctrl) + s.applicationService = NewMockApplicationService(ctrl) return ctrl } @@ -217,6 +222,7 @@ func (s *FirewallerSuite) setupAPI(c *gc.C) { s.controllerConfigAPI, s.controllerConfigService, s.modelConfigService, + s.applicationService, loggertesting.WrapCheckLog(c), ) c.Assert(err, jc.ErrorIsNil) diff --git a/apiserver/facades/controller/firewaller/interface.go b/apiserver/facades/controller/firewaller/interface.go index 1af6166445f..118db12274d 100644 --- a/apiserver/facades/controller/firewaller/interface.go +++ b/apiserver/facades/controller/firewaller/interface.go @@ -12,6 +12,7 @@ import ( "gopkg.in/macaroon.v2" "github.com/juju/juju/apiserver/common/firewall" + "github.com/juju/juju/core/life" "github.com/juju/juju/core/network" "github.com/juju/juju/core/watcher" "github.com/juju/juju/rpc/params" @@ -42,6 +43,10 @@ type NetworkService interface { WatchSubnets(ctx context.Context, subnetUUIDsToWatch set.Strings) (watcher.StringsWatcher, error) } +type ApplicationService interface { + GetUnitLife(context.Context, string) (life.Value, error) +} + // ControllerConfigAPI provides the subset of common.ControllerConfigAPI // required by the remote firewaller facade type ControllerConfigAPI interface { diff --git a/apiserver/facades/controller/firewaller/package_test.go b/apiserver/facades/controller/firewaller/package_test.go index 8524a7969d0..d681715d58e 100644 --- a/apiserver/facades/controller/firewaller/package_test.go +++ b/apiserver/facades/controller/firewaller/package_test.go @@ -11,7 +11,7 @@ import ( //go:generate go run go.uber.org/mock/mockgen -typed -package firewaller_test -destination package_mock_test.go github.com/juju/juju/apiserver/facades/controller/firewaller State,ControllerConfigAPI //go:generate go run go.uber.org/mock/mockgen -typed -package firewaller_test -destination watcher_mock_test.go github.com/juju/juju/state NotifyWatcher -//go:generate go run go.uber.org/mock/mockgen -typed -package firewaller_test -destination service_mock_test.go github.com/juju/juju/apiserver/facades/controller/firewaller ControllerConfigService,ModelConfigService,NetworkService +//go:generate go run go.uber.org/mock/mockgen -typed -package firewaller_test -destination service_mock_test.go github.com/juju/juju/apiserver/facades/controller/firewaller ControllerConfigService,ModelConfigService,NetworkService,ApplicationService func TestAll(t *stdtesting.T) { testing.MgoTestPackage(t) diff --git a/apiserver/facades/controller/firewaller/register.go b/apiserver/facades/controller/firewaller/register.go index bbc57cc8458..255f2f8dc5b 100644 --- a/apiserver/facades/controller/firewaller/register.go +++ b/apiserver/facades/controller/firewaller/register.go @@ -13,6 +13,8 @@ import ( "github.com/juju/juju/apiserver/common/cloudspec" "github.com/juju/juju/apiserver/common/firewall" "github.com/juju/juju/apiserver/facade" + "github.com/juju/juju/domain/application/service" + "github.com/juju/juju/internal/storage" ) // Register is called to expose a package of facades onto a given registry. @@ -55,6 +57,10 @@ func newFirewallerAPIV7(ctx facade.ModelContext) (*FirewallerAPI, error) { controllerConfigAPI, serviceFactory.ControllerConfig(), serviceFactory.Config(), + serviceFactory.Application(service.ApplicationServiceParams{ + StorageRegistry: storage.NotImplementedProviderRegistry{}, + Secrets: service.NotImplementedSecretService{}, + }), ctx.Logger().Child("firewaller"), ) } diff --git a/apiserver/facades/controller/firewaller/service_mock_test.go b/apiserver/facades/controller/firewaller/service_mock_test.go index 2fda7f81c9b..412bb1ab915 100644 --- a/apiserver/facades/controller/firewaller/service_mock_test.go +++ b/apiserver/facades/controller/firewaller/service_mock_test.go @@ -1,9 +1,9 @@ // Code generated by MockGen. DO NOT EDIT. -// Source: github.com/juju/juju/apiserver/facades/controller/firewaller (interfaces: ControllerConfigService,ModelConfigService,NetworkService) +// Source: github.com/juju/juju/apiserver/facades/controller/firewaller (interfaces: ControllerConfigService,ModelConfigService,NetworkService,ApplicationService) // // Generated by this command: // -// mockgen -typed -package firewaller_test -destination service_mock_test.go github.com/juju/juju/apiserver/facades/controller/firewaller ControllerConfigService,ModelConfigService,NetworkService +// mockgen -typed -package firewaller_test -destination service_mock_test.go github.com/juju/juju/apiserver/facades/controller/firewaller ControllerConfigService,ModelConfigService,NetworkService,ApplicationService // // Package firewaller_test is a generated GoMock package. @@ -15,6 +15,7 @@ import ( set "github.com/juju/collections/set" controller "github.com/juju/juju/controller" + life "github.com/juju/juju/core/life" network "github.com/juju/juju/core/network" watcher "github.com/juju/juju/core/watcher" config "github.com/juju/juju/environs/config" @@ -284,3 +285,65 @@ func (c *MockNetworkServiceWatchSubnetsCall) DoAndReturn(f func(context.Context, c.Call = c.Call.DoAndReturn(f) return c } + +// MockApplicationService is a mock of ApplicationService interface. +type MockApplicationService struct { + ctrl *gomock.Controller + recorder *MockApplicationServiceMockRecorder +} + +// MockApplicationServiceMockRecorder is the mock recorder for MockApplicationService. +type MockApplicationServiceMockRecorder struct { + mock *MockApplicationService +} + +// NewMockApplicationService creates a new mock instance. +func NewMockApplicationService(ctrl *gomock.Controller) *MockApplicationService { + mock := &MockApplicationService{ctrl: ctrl} + mock.recorder = &MockApplicationServiceMockRecorder{mock} + return mock +} + +// EXPECT returns an object that allows the caller to indicate expected use. +func (m *MockApplicationService) EXPECT() *MockApplicationServiceMockRecorder { + return m.recorder +} + +// GetUnitLife mocks base method. +func (m *MockApplicationService) GetUnitLife(arg0 context.Context, arg1 string) (life.Value, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "GetUnitLife", arg0, arg1) + ret0, _ := ret[0].(life.Value) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// GetUnitLife indicates an expected call of GetUnitLife. +func (mr *MockApplicationServiceMockRecorder) GetUnitLife(arg0, arg1 any) *MockApplicationServiceGetUnitLifeCall { + mr.mock.ctrl.T.Helper() + call := mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetUnitLife", reflect.TypeOf((*MockApplicationService)(nil).GetUnitLife), arg0, arg1) + return &MockApplicationServiceGetUnitLifeCall{Call: call} +} + +// MockApplicationServiceGetUnitLifeCall wrap *gomock.Call +type MockApplicationServiceGetUnitLifeCall struct { + *gomock.Call +} + +// Return rewrite *gomock.Call.Return +func (c *MockApplicationServiceGetUnitLifeCall) Return(arg0 life.Value, arg1 error) *MockApplicationServiceGetUnitLifeCall { + c.Call = c.Call.Return(arg0, arg1) + return c +} + +// Do rewrite *gomock.Call.Do +func (c *MockApplicationServiceGetUnitLifeCall) Do(f func(context.Context, string) (life.Value, error)) *MockApplicationServiceGetUnitLifeCall { + c.Call = c.Call.Do(f) + return c +} + +// DoAndReturn rewrite *gomock.Call.DoAndReturn +func (c *MockApplicationServiceGetUnitLifeCall) DoAndReturn(f func(context.Context, string) (life.Value, error)) *MockApplicationServiceGetUnitLifeCall { + c.Call = c.Call.DoAndReturn(f) + return c +} diff --git a/apiserver/facades/controller/migrationmaster/facade.go b/apiserver/facades/controller/migrationmaster/facade.go index a394cc2d48e..2b3c54a60eb 100644 --- a/apiserver/facades/controller/migrationmaster/facade.go +++ b/apiserver/facades/controller/migrationmaster/facade.go @@ -55,6 +55,7 @@ type API struct { modelConfigService ModelConfigService modelInfoService ModelInfoService modelService ModelService + applicationService ApplicationService store objectstore.ObjectStore } @@ -77,6 +78,7 @@ func NewAPI( modelConfigService ModelConfigService, modelInfoService ModelInfoService, modelService ModelService, + applicationService ApplicationService, upgradeService UpgradeService, ) (*API, error) { if !authorizer.AuthController() { @@ -100,6 +102,7 @@ func NewAPI( modelConfigService: modelConfigService, modelInfoService: modelInfoService, modelService: modelService, + applicationService: applicationService, upgradeService: upgradeService, }, nil } @@ -274,6 +277,7 @@ func (api *API) Prechecks(ctx context.Context, arg params.PrechecksArgs) error { api.environscloudspecGetter, api.credentialService, api.upgradeService, + api.applicationService, ) } diff --git a/apiserver/facades/controller/migrationmaster/facade_test.go b/apiserver/facades/controller/migrationmaster/facade_test.go index b0de3aefd5e..fa60c651486 100644 --- a/apiserver/facades/controller/migrationmaster/facade_test.go +++ b/apiserver/facades/controller/migrationmaster/facade_test.go @@ -52,6 +52,7 @@ type Suite struct { modelConfigService *mocks.MockModelConfigService modelInfoService *mocks.MockModelInfoService modelService *mocks.MockModelService + applicationService *mocks.MockApplicationService precheckBackend *mocks.MockPrecheckBackend @@ -620,6 +621,7 @@ func (s *Suite) setupMocks(c *gc.C) *gomock.Controller { s.modelConfigService = mocks.NewMockModelConfigService(ctrl) s.modelInfoService = mocks.NewMockModelInfoService(ctrl) s.modelService = mocks.NewMockModelService(ctrl) + s.applicationService = mocks.NewMockApplicationService(ctrl) return ctrl } @@ -647,6 +649,7 @@ func (s *Suite) makeAPI() (*migrationmaster.API, error) { s.modelConfigService, s.modelInfoService, s.modelService, + s.applicationService, s.upgradeService, ) } diff --git a/apiserver/facades/controller/migrationmaster/mocks/backend.go b/apiserver/facades/controller/migrationmaster/mocks/backend.go index 66fcec4d369..bede3138455 100644 --- a/apiserver/facades/controller/migrationmaster/mocks/backend.go +++ b/apiserver/facades/controller/migrationmaster/mocks/backend.go @@ -1,9 +1,9 @@ // Code generated by MockGen. DO NOT EDIT. -// Source: github.com/juju/juju/apiserver/facades/controller/migrationmaster (interfaces: Backend,ControllerState,ModelExporter,UpgradeService,ControllerConfigService,ModelConfigService,ModelInfoService,ModelService) +// Source: github.com/juju/juju/apiserver/facades/controller/migrationmaster (interfaces: Backend,ControllerState,ModelExporter,UpgradeService,ControllerConfigService,ModelConfigService,ModelInfoService,ModelService,ApplicationService) // // Generated by this command: // -// mockgen -typed -package mocks -destination mocks/backend.go github.com/juju/juju/apiserver/facades/controller/migrationmaster Backend,ControllerState,ModelExporter,UpgradeService,ControllerConfigService,ModelConfigService,ModelInfoService,ModelService +// mockgen -typed -package mocks -destination mocks/backend.go github.com/juju/juju/apiserver/facades/controller/migrationmaster Backend,ControllerState,ModelExporter,UpgradeService,ControllerConfigService,ModelConfigService,ModelInfoService,ModelService,ApplicationService // // Package mocks is a generated GoMock package. @@ -15,6 +15,7 @@ import ( description "github.com/juju/description/v8" controller "github.com/juju/juju/controller" + life "github.com/juju/juju/core/life" model "github.com/juju/juju/core/model" network "github.com/juju/juju/core/network" objectstore "github.com/juju/juju/core/objectstore" @@ -711,3 +712,65 @@ func (c *MockModelServiceControllerModelCall) DoAndReturn(f func(context.Context c.Call = c.Call.DoAndReturn(f) return c } + +// MockApplicationService is a mock of ApplicationService interface. +type MockApplicationService struct { + ctrl *gomock.Controller + recorder *MockApplicationServiceMockRecorder +} + +// MockApplicationServiceMockRecorder is the mock recorder for MockApplicationService. +type MockApplicationServiceMockRecorder struct { + mock *MockApplicationService +} + +// NewMockApplicationService creates a new mock instance. +func NewMockApplicationService(ctrl *gomock.Controller) *MockApplicationService { + mock := &MockApplicationService{ctrl: ctrl} + mock.recorder = &MockApplicationServiceMockRecorder{mock} + return mock +} + +// EXPECT returns an object that allows the caller to indicate expected use. +func (m *MockApplicationService) EXPECT() *MockApplicationServiceMockRecorder { + return m.recorder +} + +// GetApplicationLife mocks base method. +func (m *MockApplicationService) GetApplicationLife(arg0 context.Context, arg1 string) (life.Value, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "GetApplicationLife", arg0, arg1) + ret0, _ := ret[0].(life.Value) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// GetApplicationLife indicates an expected call of GetApplicationLife. +func (mr *MockApplicationServiceMockRecorder) GetApplicationLife(arg0, arg1 any) *MockApplicationServiceGetApplicationLifeCall { + mr.mock.ctrl.T.Helper() + call := mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetApplicationLife", reflect.TypeOf((*MockApplicationService)(nil).GetApplicationLife), arg0, arg1) + return &MockApplicationServiceGetApplicationLifeCall{Call: call} +} + +// MockApplicationServiceGetApplicationLifeCall wrap *gomock.Call +type MockApplicationServiceGetApplicationLifeCall struct { + *gomock.Call +} + +// Return rewrite *gomock.Call.Return +func (c *MockApplicationServiceGetApplicationLifeCall) Return(arg0 life.Value, arg1 error) *MockApplicationServiceGetApplicationLifeCall { + c.Call = c.Call.Return(arg0, arg1) + return c +} + +// Do rewrite *gomock.Call.Do +func (c *MockApplicationServiceGetApplicationLifeCall) Do(f func(context.Context, string) (life.Value, error)) *MockApplicationServiceGetApplicationLifeCall { + c.Call = c.Call.Do(f) + return c +} + +// DoAndReturn rewrite *gomock.Call.DoAndReturn +func (c *MockApplicationServiceGetApplicationLifeCall) DoAndReturn(f func(context.Context, string) (life.Value, error)) *MockApplicationServiceGetApplicationLifeCall { + c.Call = c.Call.DoAndReturn(f) + return c +} diff --git a/apiserver/facades/controller/migrationmaster/package_test.go b/apiserver/facades/controller/migrationmaster/package_test.go index a7f9d610722..785e527f8d3 100644 --- a/apiserver/facades/controller/migrationmaster/package_test.go +++ b/apiserver/facades/controller/migrationmaster/package_test.go @@ -9,7 +9,7 @@ import ( gc "gopkg.in/check.v1" ) -//go:generate go run go.uber.org/mock/mockgen -typed -package mocks -destination mocks/backend.go github.com/juju/juju/apiserver/facades/controller/migrationmaster Backend,ControllerState,ModelExporter,UpgradeService,ControllerConfigService,ModelConfigService,ModelInfoService,ModelService +//go:generate go run go.uber.org/mock/mockgen -typed -package mocks -destination mocks/backend.go github.com/juju/juju/apiserver/facades/controller/migrationmaster Backend,ControllerState,ModelExporter,UpgradeService,ControllerConfigService,ModelConfigService,ModelInfoService,ModelService,ApplicationService //go:generate go run go.uber.org/mock/mockgen -typed -package mocks -destination mocks/precheckbackend.go github.com/juju/juju/internal/migration PrecheckBackend //go:generate go run go.uber.org/mock/mockgen -typed -package mocks -destination mocks/state.go github.com/juju/juju/state ModelMigration,NotifyWatcher //go:generate go run go.uber.org/mock/mockgen -typed -package mocks -destination mocks/objectstore.go github.com/juju/juju/core/objectstore ObjectStore diff --git a/apiserver/facades/controller/migrationmaster/register.go b/apiserver/facades/controller/migrationmaster/register.go index 0f128f0ffc9..98d3fa7dc14 100644 --- a/apiserver/facades/controller/migrationmaster/register.go +++ b/apiserver/facades/controller/migrationmaster/register.go @@ -11,7 +11,9 @@ import ( "github.com/juju/juju/apiserver/common/cloudspec" "github.com/juju/juju/apiserver/facade" + "github.com/juju/juju/domain/application/service" "github.com/juju/juju/internal/migration" + "github.com/juju/juju/internal/storage" ) // Register is called to expose a package of facades onto a given registry. @@ -63,6 +65,10 @@ func newMigrationMasterFacade(ctx facade.ModelContext) (*API, error) { serviceFactory.Config(), serviceFactory.ModelInfo(), serviceFactory.Model(), + serviceFactory.Application(service.ApplicationServiceParams{ + StorageRegistry: storage.NotImplementedProviderRegistry{}, + Secrets: service.NotImplementedSecretService{}, + }), serviceFactory.Upgrade(), ) } diff --git a/apiserver/facades/controller/migrationmaster/service.go b/apiserver/facades/controller/migrationmaster/service.go index aafd48f1058..b8ab1d67873 100644 --- a/apiserver/facades/controller/migrationmaster/service.go +++ b/apiserver/facades/controller/migrationmaster/service.go @@ -7,6 +7,7 @@ import ( "context" "github.com/juju/juju/controller" + "github.com/juju/juju/core/life" "github.com/juju/juju/core/model" "github.com/juju/juju/environs/config" ) @@ -41,3 +42,8 @@ type ModelService interface { // ControllerModel returns the model used for housing the Juju controller. ControllerModel(ctx context.Context) (model.Model, error) } + +// ApplicationService provides access to the application service. +type ApplicationService interface { + GetApplicationLife(context.Context, string) (life.Value, error) +} diff --git a/apiserver/facades/controller/migrationtarget/domain_mock_test.go b/apiserver/facades/controller/migrationtarget/domain_mock_test.go index 86d1b25cc35..c23c183fbea 100644 --- a/apiserver/facades/controller/migrationtarget/domain_mock_test.go +++ b/apiserver/facades/controller/migrationtarget/domain_mock_test.go @@ -1,9 +1,9 @@ // Code generated by MockGen. DO NOT EDIT. -// Source: github.com/juju/juju/apiserver/facades/controller/migrationtarget (interfaces: ControllerConfigService,ExternalControllerService,UpgradeService,ModelImporter,ModelMigrationService) +// Source: github.com/juju/juju/apiserver/facades/controller/migrationtarget (interfaces: ControllerConfigService,ExternalControllerService,UpgradeService,ModelImporter,ModelMigrationService,ApplicationService) // // Generated by this command: // -// mockgen -typed -package migrationtarget_test -destination domain_mock_test.go github.com/juju/juju/apiserver/facades/controller/migrationtarget ControllerConfigService,ExternalControllerService,UpgradeService,ModelImporter,ModelMigrationService +// mockgen -typed -package migrationtarget_test -destination domain_mock_test.go github.com/juju/juju/apiserver/facades/controller/migrationtarget ControllerConfigService,ExternalControllerService,UpgradeService,ModelImporter,ModelMigrationService,ApplicationService // // Package migrationtarget_test is a generated GoMock package. @@ -15,6 +15,7 @@ import ( controller "github.com/juju/juju/controller" crossmodel "github.com/juju/juju/core/crossmodel" + life "github.com/juju/juju/core/life" modelmigration "github.com/juju/juju/domain/modelmigration" state "github.com/juju/juju/state" version "github.com/juju/version/v2" @@ -407,3 +408,65 @@ func (c *MockModelMigrationServiceCheckMachinesCall) DoAndReturn(f func(context. c.Call = c.Call.DoAndReturn(f) return c } + +// MockApplicationService is a mock of ApplicationService interface. +type MockApplicationService struct { + ctrl *gomock.Controller + recorder *MockApplicationServiceMockRecorder +} + +// MockApplicationServiceMockRecorder is the mock recorder for MockApplicationService. +type MockApplicationServiceMockRecorder struct { + mock *MockApplicationService +} + +// NewMockApplicationService creates a new mock instance. +func NewMockApplicationService(ctrl *gomock.Controller) *MockApplicationService { + mock := &MockApplicationService{ctrl: ctrl} + mock.recorder = &MockApplicationServiceMockRecorder{mock} + return mock +} + +// EXPECT returns an object that allows the caller to indicate expected use. +func (m *MockApplicationService) EXPECT() *MockApplicationServiceMockRecorder { + return m.recorder +} + +// GetApplicationLife mocks base method. +func (m *MockApplicationService) GetApplicationLife(arg0 context.Context, arg1 string) (life.Value, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "GetApplicationLife", arg0, arg1) + ret0, _ := ret[0].(life.Value) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// GetApplicationLife indicates an expected call of GetApplicationLife. +func (mr *MockApplicationServiceMockRecorder) GetApplicationLife(arg0, arg1 any) *MockApplicationServiceGetApplicationLifeCall { + mr.mock.ctrl.T.Helper() + call := mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetApplicationLife", reflect.TypeOf((*MockApplicationService)(nil).GetApplicationLife), arg0, arg1) + return &MockApplicationServiceGetApplicationLifeCall{Call: call} +} + +// MockApplicationServiceGetApplicationLifeCall wrap *gomock.Call +type MockApplicationServiceGetApplicationLifeCall struct { + *gomock.Call +} + +// Return rewrite *gomock.Call.Return +func (c *MockApplicationServiceGetApplicationLifeCall) Return(arg0 life.Value, arg1 error) *MockApplicationServiceGetApplicationLifeCall { + c.Call = c.Call.Return(arg0, arg1) + return c +} + +// Do rewrite *gomock.Call.Do +func (c *MockApplicationServiceGetApplicationLifeCall) Do(f func(context.Context, string) (life.Value, error)) *MockApplicationServiceGetApplicationLifeCall { + c.Call = c.Call.Do(f) + return c +} + +// DoAndReturn rewrite *gomock.Call.DoAndReturn +func (c *MockApplicationServiceGetApplicationLifeCall) DoAndReturn(f func(context.Context, string) (life.Value, error)) *MockApplicationServiceGetApplicationLifeCall { + c.Call = c.Call.DoAndReturn(f) + return c +} diff --git a/apiserver/facades/controller/migrationtarget/migrationtarget.go b/apiserver/facades/controller/migrationtarget/migrationtarget.go index 0c2b5ab42e4..19e4add6239 100644 --- a/apiserver/facades/controller/migrationtarget/migrationtarget.go +++ b/apiserver/facades/controller/migrationtarget/migrationtarget.go @@ -21,12 +21,13 @@ import ( "github.com/juju/juju/core/crossmodel" coreerrors "github.com/juju/juju/core/errors" "github.com/juju/juju/core/facades" + "github.com/juju/juju/core/life" corelogger "github.com/juju/juju/core/logger" coremigration "github.com/juju/juju/core/migration" "github.com/juju/juju/core/model" "github.com/juju/juju/core/permission" "github.com/juju/juju/core/status" - modelmigration "github.com/juju/juju/domain/modelmigration" + "github.com/juju/juju/domain/modelmigration" "github.com/juju/juju/internal/errors" "github.com/juju/juju/internal/migration" "github.com/juju/juju/rpc/params" @@ -59,6 +60,11 @@ type ControllerConfigService interface { ControllerConfig(context.Context) (controller.Config, error) } +// ApplicationService provides access to the application service. +type ApplicationService interface { + GetApplicationLife(context.Context, string) (life.Value, error) +} + // ModelManagerService describes the method needed to update model metadata. type ModelManagerService interface { Create(context.Context, model.UUID) error @@ -96,6 +102,7 @@ type API struct { modelImporter ModelImporter upgradeService UpgradeService + applicationService ApplicationService controllerConfigService ControllerConfigService externalControllerService ExternalControllerService modelMigrationServiceGetter ModelMigrationServiceGetter @@ -116,6 +123,7 @@ func NewAPI( authorizer facade.Authorizer, controllerConfigService ControllerConfigService, externalControllerService ExternalControllerService, + applicationService ApplicationService, upgradeService UpgradeService, modelMigrationServiceGetter ModelMigrationServiceGetter, requiredMigrationFacadeVersions facades.FacadeVersions, @@ -127,6 +135,7 @@ func NewAPI( pool: ctx.StatePool(), controllerConfigService: controllerConfigService, externalControllerService: externalControllerService, + applicationService: applicationService, upgradeService: upgradeService, modelMigrationServiceGetter: modelMigrationServiceGetter, authorizer: authorizer, @@ -226,6 +235,7 @@ with an earlier version of the target controller and try again. }, api.presence.ModelPresence(controllerState.ModelUUID()), api.upgradeService, + api.applicationService, ); err != nil { return errors.Errorf("migration target prechecks failed: %w", err) } diff --git a/apiserver/facades/controller/migrationtarget/migrationtarget_test.go b/apiserver/facades/controller/migrationtarget/migrationtarget_test.go index b5557216b5e..63c62066f1d 100644 --- a/apiserver/facades/controller/migrationtarget/migrationtarget_test.go +++ b/apiserver/facades/controller/migrationtarget/migrationtarget_test.go @@ -52,6 +52,7 @@ type Suite struct { serviceFactory *MockServiceFactory serviceFactoryGetter *MockServiceFactoryGetter externalControllerService *MockExternalControllerService + applicationService *MockApplicationService upgradeService *MockUpgradeService modelImporter *MockModelImporter modelMigrationService *MockModelMigrationService @@ -543,6 +544,7 @@ func (s *Suite) setupMocks(c *gc.C) *gomock.Controller { s.serviceFactoryGetter = NewMockServiceFactoryGetter(ctrl) s.externalControllerService = NewMockExternalControllerService(ctrl) + s.applicationService = NewMockApplicationService(ctrl) s.upgradeService = NewMockUpgradeService(ctrl) s.modelImporter = NewMockModelImporter(ctrl) @@ -575,6 +577,7 @@ func (s *Suite) newAPI(versions facades.FacadeVersions, logDir string) (*migrati s.authorizer, s.controllerConfigService, s.externalControllerService, + s.applicationService, s.upgradeService, s.migrationServiceGetter, versions, diff --git a/apiserver/facades/controller/migrationtarget/package_test.go b/apiserver/facades/controller/migrationtarget/package_test.go index 378178d812e..500f2105253 100644 --- a/apiserver/facades/controller/migrationtarget/package_test.go +++ b/apiserver/facades/controller/migrationtarget/package_test.go @@ -9,7 +9,7 @@ import ( "github.com/juju/juju/internal/testing" ) -//go:generate go run go.uber.org/mock/mockgen -typed -package migrationtarget_test -destination domain_mock_test.go github.com/juju/juju/apiserver/facades/controller/migrationtarget ControllerConfigService,ExternalControllerService,UpgradeService,ModelImporter,ModelMigrationService +//go:generate go run go.uber.org/mock/mockgen -typed -package migrationtarget_test -destination domain_mock_test.go github.com/juju/juju/apiserver/facades/controller/migrationtarget ControllerConfigService,ExternalControllerService,UpgradeService,ModelImporter,ModelMigrationService,ApplicationService //go:generate go run go.uber.org/mock/mockgen -typed -package migrationtarget_test -destination servicefactory_mock_test.go github.com/juju/juju/internal/servicefactory ServiceFactoryGetter,ServiceFactory func TestAll(t *stdtesting.T) { diff --git a/apiserver/facades/controller/migrationtarget/register.go b/apiserver/facades/controller/migrationtarget/register.go index 020b432e3f2..06af80fdf1f 100644 --- a/apiserver/facades/controller/migrationtarget/register.go +++ b/apiserver/facades/controller/migrationtarget/register.go @@ -10,7 +10,9 @@ import ( "github.com/juju/juju/apiserver/facade" "github.com/juju/juju/core/facades" "github.com/juju/juju/core/model" + "github.com/juju/juju/domain/application/service" "github.com/juju/juju/internal/errors" + "github.com/juju/juju/internal/storage" ) // Register is called to expose a package of facades onto a given registry. @@ -50,6 +52,10 @@ func makeFacade( auth, serviceFactory.ControllerConfig(), serviceFactory.ExternalController(), + serviceFactory.Application(service.ApplicationServiceParams{ + StorageRegistry: storage.NotImplementedProviderRegistry{}, + Secrets: service.NotImplementedSecretService{}, + }), serviceFactory.Upgrade(), modelMigrationServiceGetter, facadeVersions, diff --git a/apiserver/facades/schema.json b/apiserver/facades/schema.json index 60c12feba04..4715183b2c4 100644 --- a/apiserver/facades/schema.json +++ b/apiserver/facades/schema.json @@ -6243,7 +6243,7 @@ "$ref": "#/definitions/LifeResults" } }, - "description": "Life returns the life status of every supplied entity, where available." + "description": "Life returns the life status of every supplied app or unit, where available." }, "ProvisionerConfig": { "type": "object", @@ -8204,7 +8204,7 @@ "$ref": "#/definitions/LifeResults" } }, - "description": "Life returns the life status of every supplied entity, where available." + "description": "Life returns the life status of the specified units." }, "Watch": { "type": "object", @@ -16261,7 +16261,7 @@ "$ref": "#/definitions/LifeResults" } }, - "description": "Life returns the life status of every supplied entity, where available." + "description": "Life returns the life of the specified units." }, "ModelUUID": { "type": "object", @@ -17692,7 +17692,7 @@ "$ref": "#/definitions/LifeResults" } }, - "description": "Life returns the life status of every supplied entity, where available." + "description": "Life returns the life status of the specified entities." }, "MacaroonForRelations": { "type": "object", @@ -39658,7 +39658,7 @@ "$ref": "#/definitions/LifeResults" } }, - "description": "Life returns the life status of every supplied entity, where available." + "description": "Life returns the life status of the specified units." }, "LogActionsMessages": { "type": "object", diff --git a/cmd/containeragent/initialize/command.go b/cmd/containeragent/initialize/command.go index 2b4f0ec18e9..3bd17688950 100644 --- a/cmd/containeragent/initialize/command.go +++ b/cmd/containeragent/initialize/command.go @@ -27,6 +27,7 @@ import ( jujucmd "github.com/juju/juju/cmd" "github.com/juju/juju/cmd/constants" "github.com/juju/juju/cmd/containeragent/utils" + applicationerrors "github.com/juju/juju/domain/application/errors" internallogger "github.com/juju/juju/internal/logger" pebbleidentity "github.com/juju/juju/internal/service/pebble/identity" pebbleplan "github.com/juju/juju/internal/service/pebble/plan" @@ -168,7 +169,7 @@ func (c *initCommand) Run(ctx *cmd.Context) (err error) { return errors.Trace(err) }, IsFatalError: func(err error) bool { - return !errors.Is(err, errors.NotAssigned) && !errors.Is(err, errors.AlreadyExists) + return !errors.Is(err, applicationerrors.UnitNotAssigned) && !errors.Is(err, applicationerrors.UnitAlreadyExists) }, Attempts: -1, Delay: 10 * time.Second, diff --git a/cmd/containeragent/initialize/command_test.go b/cmd/containeragent/initialize/command_test.go index 58919ae35bd..ef12d9e3cd2 100644 --- a/cmd/containeragent/initialize/command_test.go +++ b/cmd/containeragent/initialize/command_test.go @@ -11,7 +11,6 @@ import ( "github.com/juju/clock/testclock" "github.com/juju/cmd/v4" "github.com/juju/cmd/v4/cmdtesting" - "github.com/juju/errors" "github.com/juju/names/v5" jc "github.com/juju/testing/checkers" "go.uber.org/mock/gomock" @@ -22,6 +21,7 @@ import ( "github.com/juju/juju/cmd/containeragent/initialize" "github.com/juju/juju/cmd/containeragent/initialize/mocks" utilsmocks "github.com/juju/juju/cmd/containeragent/utils/mocks" + applicationerrors "github.com/juju/juju/domain/application/errors" coretesting "github.com/juju/juju/internal/testing" ) @@ -145,8 +145,8 @@ checks: gomock.InOrder( s.fileReaderWriter.EXPECT().Stat("/var/lib/juju/template-agent.conf").Return(nil, os.ErrNotExist), - s.applicationAPI.EXPECT().UnitIntroduction(gomock.Any(), `gitlab-0`, `gitlab-uuid`).Times(1).Return(nil, errors.NotAssignedf("yo we not needed yet")), - s.applicationAPI.EXPECT().UnitIntroduction(gomock.Any(), `gitlab-0`, `gitlab-uuid`).Times(1).Return(nil, errors.AlreadyExistsf("yo we dead atm")), + s.applicationAPI.EXPECT().UnitIntroduction(gomock.Any(), `gitlab-0`, `gitlab-uuid`).Times(1).Return(nil, applicationerrors.UnitNotAssigned), + s.applicationAPI.EXPECT().UnitIntroduction(gomock.Any(), `gitlab-0`, `gitlab-uuid`).Times(1).Return(nil, applicationerrors.UnitAlreadyExists), s.applicationAPI.EXPECT().UnitIntroduction(gomock.Any(), `gitlab-0`, `gitlab-uuid`).Times(1).Return(&caasapplication.UnitConfig{ UnitTag: names.NewUnitTag("gitlab/0"), AgentConf: data, diff --git a/domain/application/errors/errors.go b/domain/application/errors/errors.go index 7d7fbdc1927..189605f78ff 100644 --- a/domain/application/errors/errors.go +++ b/domain/application/errors/errors.go @@ -46,9 +46,9 @@ const ( // is not assigned. UnitNotAssigned = errors.ConstError("unit not assigned") - // ApplicationDyingOrDead describes an error where resource query fails because the - // application is dying or dead. - ApplicationDyingOrDead = errors.ConstError("application dying or dead") + // UnitAlreadyExists describes an error that occurs when the + // unit being created already exists. + UnitAlreadyExists = errors.ConstError("unit already exists") // UnitHasSubordinates describes an error that occurs when trying to set a unit's life // to Dead but it still has subordinates. diff --git a/domain/application/service/application.go b/domain/application/service/application.go index 68365c6834b..732327f9429 100644 --- a/domain/application/service/application.go +++ b/domain/application/service/application.go @@ -19,6 +19,7 @@ import ( corecharm "github.com/juju/juju/core/charm" coredatabase "github.com/juju/juju/core/database" "github.com/juju/juju/core/leadership" + corelife "github.com/juju/juju/core/life" "github.com/juju/juju/core/logger" coremodel "github.com/juju/juju/core/model" "github.com/juju/juju/core/network" @@ -325,6 +326,18 @@ func (s *ApplicationService) AddUnits(ctx context.Context, name string, units .. return errors.Annotatef(err, "adding units to application %q", name) } +// GetUnitLife looks up the life of the specified unit, returning an error +// satisfying [applicationerrors.UnitNotFoundError] if the unit is not found. +func (s *ApplicationService) GetUnitLife(ctx context.Context, unitName string) (corelife.Value, error) { + var result corelife.Value + err := s.st.RunAtomic(ctx, func(ctx domain.AtomicContext) error { + unitLife, err := s.st.GetUnitLife(ctx, unitName) + result = unitLife.Value() + return errors.Annotatef(err, "getting life for %q", unitName) + }) + return result, errors.Trace(err) +} + // DeleteUnit deletes the specified unit. // TODO(units) - rework when dual write is refactored // This method is called (mostly during cleanup) after a unit @@ -516,7 +529,7 @@ func (s *ApplicationService) RegisterCAASUnit(ctx context.Context, appName strin return s.insertCAASUnit(ctx, appID, args.OrderedId, unitArg) } if unitLife == life.Dead { - return fmt.Errorf("dead unit %q already exists%w", args.UnitName, errors.Hide(applicationerrors.ApplicationIsDead)) + return fmt.Errorf("dead unit %q already exists%w", args.UnitName, errors.Hide(applicationerrors.UnitAlreadyExists)) } return s.st.UpsertUnit(ctx, appID, unitArg) }) @@ -610,6 +623,23 @@ func (s *ApplicationService) DestroyApplication(ctx context.Context, appName str return errors.Annotatef(err, "destroying application %q", appName) } +// EnsureApplicationDead is called by the cleanup worker if a mongo +// destroy operation sets the application to dead. +// TODO(units): remove when everything is in dqlite. +func (s *ApplicationService) EnsureApplicationDead(ctx context.Context, appName string) error { + err := s.st.RunAtomic(ctx, func(ctx domain.AtomicContext) error { + appID, err := s.st.GetApplicationID(ctx, appName) + if errors.Is(err, applicationerrors.ApplicationIsDead) { + return nil + } + if err != nil { + return errors.Trace(err) + } + return s.st.SetApplicationLife(ctx, appID, life.Dead) + }) + return errors.Annotatef(err, "setting application %q life to Dead", appName) +} + // UpdateApplicationCharm sets a new charm for the application, validating that aspects such // as storage are still viable with the new charm. func (s *ApplicationService) UpdateApplicationCharm(ctx context.Context, name string, params UpdateCharmParams) error { @@ -758,6 +788,18 @@ func (s *ApplicationService) CAASUnitTerminating(ctx context.Context, appName st return restart, nil } +// GetApplicationLife looks up the life of the specified application, returning an error +// satisfying [applicationerrors.ApplicationNotFoundError] if the application is not found. +func (s *ApplicationService) GetApplicationLife(ctx context.Context, appName string) (corelife.Value, error) { + var result corelife.Value + err := s.st.RunAtomic(ctx, func(ctx domain.AtomicContext) error { + _, appLife, err := s.st.GetApplicationLife(ctx, appName) + result = appLife.Value() + return errors.Annotatef(err, "getting life for %q", appName) + }) + return result, errors.Trace(err) +} + // SetApplicationScale sets the application's desired scale value, returning an error // satisfying [applicationerrors.ApplicationNotFound] if the application is not found. // This is used on CAAS models. diff --git a/domain/application/service_test.go b/domain/application/service_test.go index 80e06b0149e..ec9e23c37ed 100644 --- a/domain/application/service_test.go +++ b/domain/application/service_test.go @@ -15,6 +15,7 @@ import ( coreapplication "github.com/juju/juju/core/application" corecharm "github.com/juju/juju/core/charm" "github.com/juju/juju/core/database" + "github.com/juju/juju/core/life" coresecrets "github.com/juju/juju/core/secrets" "github.com/juju/juju/domain/application" applicationerrors "github.com/juju/juju/domain/application/errors" @@ -83,6 +84,17 @@ func (s *serviceSuite) createApplication(c *gc.C, name string, units ...service. return appID } +func (s *serviceSuite) TestGetApplicationLife(c *gc.C) { + s.createApplication(c, "foo") + + lifeValue, err := s.svc.GetApplicationLife(context.Background(), "foo") + c.Assert(err, jc.ErrorIsNil) + c.Assert(lifeValue, gc.Equals, life.Alive) + + _, err = s.svc.GetApplicationLife(context.Background(), "bar") + c.Assert(err, jc.ErrorIs, applicationerrors.ApplicationNotFound) +} + func (s *serviceSuite) TestDestroyApplication(c *gc.C) { appID := s.createApplication(c, "foo") @@ -166,6 +178,50 @@ func (s *serviceSuite) TestDeleteApplicationNotFound(c *gc.C) { c.Assert(err, jc.ErrorIs, applicationerrors.ApplicationNotFound) } +func (s *serviceSuite) TestEnsureApplicationDead(c *gc.C) { + ctrl := gomock.NewController(c) + defer ctrl.Finish() + + s.createApplication(c, "foo") + + err := s.svc.EnsureApplicationDead(context.Background(), "foo") + c.Assert(err, jc.ErrorIsNil) + + var gotLife int + err = s.TxnRunner().StdTxn(context.Background(), func(ctx context.Context, tx *sql.Tx) error { + err := tx.QueryRowContext(ctx, "SELECT life_id FROM application WHERE name = ?", "foo"). + Scan(&gotLife) + if err != nil { + return err + } + return nil + }) + c.Assert(err, jc.ErrorIsNil) + c.Assert(gotLife, gc.Equals, 2) +} + +func (s *serviceSuite) TestEnsureApplicationDeadNotFound(c *gc.C) { + ctrl := gomock.NewController(c) + defer ctrl.Finish() + + err := s.svc.EnsureApplicationDead(context.Background(), "foo") + c.Assert(err, jc.ErrorIs, applicationerrors.ApplicationNotFound) +} + +func (s *serviceSuite) TestGetUnitLife(c *gc.C) { + u := service.AddUnitArg{ + UnitName: ptr("foo/666"), + } + s.createApplication(c, "foo", u) + + lifeValue, err := s.svc.GetUnitLife(context.Background(), "foo/666") + c.Assert(err, jc.ErrorIsNil) + c.Assert(lifeValue, gc.Equals, life.Alive) + + _, err = s.svc.GetUnitLife(context.Background(), "foo/667") + c.Assert(err, jc.ErrorIs, applicationerrors.UnitNotFound) +} + func (s *serviceSuite) TestDestroyUnit(c *gc.C) { u := service.AddUnitArg{ UnitName: ptr("foo/666"), @@ -394,7 +450,7 @@ func (s *serviceSuite) TestReplaceDeadCAASUnit(c *gc.C) { OrderedId: 1, } err = s.svc.RegisterCAASUnit(context.Background(), "foo", args) - c.Assert(err, jc.ErrorIs, applicationerrors.ApplicationIsDead) + c.Assert(err, jc.ErrorIs, applicationerrors.UnitAlreadyExists) } func (s *serviceSuite) TestNewCAASUnit(c *gc.C) { diff --git a/domain/life/life.go b/domain/life/life.go index 69776b4ca56..c22922e6eed 100644 --- a/domain/life/life.go +++ b/domain/life/life.go @@ -3,6 +3,8 @@ package life +import corelife "github.com/juju/juju/core/life" + // Life represents the life of an entity // as recorded in the life lookup table. type Life int @@ -12,3 +14,17 @@ const ( Dying Dead ) + +// Value returns the [github.com/juju/juju/core/life.Life] +// value corresponding to this life. +func (l Life) Value() corelife.Value { + switch l { + case Alive: + return corelife.Alive + case Dying: + return corelife.Dying + case Dead: + return corelife.Dead + } + return corelife.Alive +} diff --git a/domain/life/life_test.go b/domain/life/life_test.go index f845f74a983..9452f87b513 100644 --- a/domain/life/life_test.go +++ b/domain/life/life_test.go @@ -7,6 +7,7 @@ import ( jc "github.com/juju/testing/checkers" gc "gopkg.in/check.v1" + corelife "github.com/juju/juju/core/life" schematesting "github.com/juju/juju/domain/schema/testing" ) @@ -40,3 +41,9 @@ func (s *lifeSuite) TestLifeDBValues(c *gc.C) { Dead: "dead", }) } + +func (s *lifeSuite) TestValue(c *gc.C) { + c.Assert(Alive.Value(), gc.Equals, corelife.Alive) + c.Assert(Dying.Value(), gc.Equals, corelife.Dying) + c.Assert(Dead.Value(), gc.Equals, corelife.Dead) +} diff --git a/internal/migration/interface.go b/internal/migration/interface.go index b4a8554cba8..cfc8f8e131d 100644 --- a/internal/migration/interface.go +++ b/internal/migration/interface.go @@ -13,6 +13,7 @@ import ( "github.com/juju/juju/cloud" "github.com/juju/juju/controller" "github.com/juju/juju/core/credential" + "github.com/juju/juju/core/life" "github.com/juju/juju/core/presence" "github.com/juju/juju/core/status" environscloudspec "github.com/juju/juju/environs/cloudspec" @@ -47,6 +48,11 @@ type UpgradeService interface { IsUpgrading(context.Context) (bool, error) } +// ApplicationService provides access to the application service. +type ApplicationService interface { + GetApplicationLife(context.Context, string) (life.Value, error) +} + // ControllerConfigService describes the method needed to get the // controller config. type ControllerConfigService interface { @@ -88,7 +94,6 @@ type PrecheckMachine interface { // application needed by migration prechecks. type PrecheckApplication interface { Name() string - Life() state.Life CharmURL() (*string, bool) AllUnits() ([]PrecheckUnit, error) MinUnits() int diff --git a/internal/migration/migration_mock_test.go b/internal/migration/migration_mock_test.go index fb22ed46a8e..0f156835aaf 100644 --- a/internal/migration/migration_mock_test.go +++ b/internal/migration/migration_mock_test.go @@ -1,9 +1,9 @@ // Code generated by MockGen. DO NOT EDIT. -// Source: github.com/juju/juju/internal/migration (interfaces: ControllerConfigService,UpgradeService) +// Source: github.com/juju/juju/internal/migration (interfaces: ControllerConfigService,UpgradeService,ApplicationService) // // Generated by this command: // -// mockgen -typed -package migration_test -destination migration_mock_test.go github.com/juju/juju/internal/migration ControllerConfigService,UpgradeService +// mockgen -typed -package migration_test -destination migration_mock_test.go github.com/juju/juju/internal/migration ControllerConfigService,UpgradeService,ApplicationService // // Package migration_test is a generated GoMock package. @@ -14,6 +14,7 @@ import ( reflect "reflect" controller "github.com/juju/juju/controller" + life "github.com/juju/juju/core/life" gomock "go.uber.org/mock/gomock" ) @@ -140,3 +141,65 @@ func (c *MockUpgradeServiceIsUpgradingCall) DoAndReturn(f func(context.Context) c.Call = c.Call.DoAndReturn(f) return c } + +// MockApplicationService is a mock of ApplicationService interface. +type MockApplicationService struct { + ctrl *gomock.Controller + recorder *MockApplicationServiceMockRecorder +} + +// MockApplicationServiceMockRecorder is the mock recorder for MockApplicationService. +type MockApplicationServiceMockRecorder struct { + mock *MockApplicationService +} + +// NewMockApplicationService creates a new mock instance. +func NewMockApplicationService(ctrl *gomock.Controller) *MockApplicationService { + mock := &MockApplicationService{ctrl: ctrl} + mock.recorder = &MockApplicationServiceMockRecorder{mock} + return mock +} + +// EXPECT returns an object that allows the caller to indicate expected use. +func (m *MockApplicationService) EXPECT() *MockApplicationServiceMockRecorder { + return m.recorder +} + +// GetApplicationLife mocks base method. +func (m *MockApplicationService) GetApplicationLife(arg0 context.Context, arg1 string) (life.Value, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "GetApplicationLife", arg0, arg1) + ret0, _ := ret[0].(life.Value) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// GetApplicationLife indicates an expected call of GetApplicationLife. +func (mr *MockApplicationServiceMockRecorder) GetApplicationLife(arg0, arg1 any) *MockApplicationServiceGetApplicationLifeCall { + mr.mock.ctrl.T.Helper() + call := mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetApplicationLife", reflect.TypeOf((*MockApplicationService)(nil).GetApplicationLife), arg0, arg1) + return &MockApplicationServiceGetApplicationLifeCall{Call: call} +} + +// MockApplicationServiceGetApplicationLifeCall wrap *gomock.Call +type MockApplicationServiceGetApplicationLifeCall struct { + *gomock.Call +} + +// Return rewrite *gomock.Call.Return +func (c *MockApplicationServiceGetApplicationLifeCall) Return(arg0 life.Value, arg1 error) *MockApplicationServiceGetApplicationLifeCall { + c.Call = c.Call.Return(arg0, arg1) + return c +} + +// Do rewrite *gomock.Call.Do +func (c *MockApplicationServiceGetApplicationLifeCall) Do(f func(context.Context, string) (life.Value, error)) *MockApplicationServiceGetApplicationLifeCall { + c.Call = c.Call.Do(f) + return c +} + +// DoAndReturn rewrite *gomock.Call.DoAndReturn +func (c *MockApplicationServiceGetApplicationLifeCall) DoAndReturn(f func(context.Context, string) (life.Value, error)) *MockApplicationServiceGetApplicationLifeCall { + c.Call = c.Call.DoAndReturn(f) + return c +} diff --git a/internal/migration/package_test.go b/internal/migration/package_test.go index 779193c8689..9d2d3dfd65f 100644 --- a/internal/migration/package_test.go +++ b/internal/migration/package_test.go @@ -10,11 +10,12 @@ import ( "go.uber.org/mock/gomock" gc "gopkg.in/check.v1" + "github.com/juju/juju/core/life" "github.com/juju/juju/internal/testing" upgradevalidationmocks "github.com/juju/juju/internal/upgrades/upgradevalidation/mocks" ) -//go:generate go run go.uber.org/mock/mockgen -typed -package migration_test -destination migration_mock_test.go github.com/juju/juju/internal/migration ControllerConfigService,UpgradeService +//go:generate go run go.uber.org/mock/mockgen -typed -package migration_test -destination migration_mock_test.go github.com/juju/juju/internal/migration ControllerConfigService,UpgradeService,ApplicationService //go:generate go run go.uber.org/mock/mockgen -typed -package migration_test -destination servicefactory_mock_test.go github.com/juju/juju/internal/servicefactory ServiceFactoryGetter,ServiceFactory func TestPackage(t *stdtesting.T) { @@ -24,14 +25,15 @@ func TestPackage(t *stdtesting.T) { type precheckBaseSuite struct { testing.BaseSuite - upgradeService *MockUpgradeService + upgradeService *MockUpgradeService + applicationService *MockApplicationService server *upgradevalidationmocks.MockServer serverFactory *upgradevalidationmocks.MockServerFactory } func (s *precheckBaseSuite) checkRebootRequired(c *gc.C, runPrecheck precheckRunner) { - err := runPrecheck(newBackendWithRebootingMachine(), &fakeCredentialService{}, s.upgradeService) + err := runPrecheck(newBackendWithRebootingMachine(), &fakeCredentialService{}, s.upgradeService, s.applicationService) c.Assert(err, gc.ErrorMatches, "machine 0 is scheduled to reboot") } @@ -39,24 +41,29 @@ func (s *precheckBaseSuite) checkAgentVersionError(c *gc.C, runPrecheck precheck backend := &fakeBackend{ agentVersionErr: errors.New("boom"), } - err := runPrecheck(backend, &fakeCredentialService{}, s.upgradeService) + err := runPrecheck(backend, &fakeCredentialService{}, s.upgradeService, s.applicationService) c.Assert(err, gc.ErrorMatches, "retrieving model version: boom") } func (s *precheckBaseSuite) checkMachineVersionsDontMatch(c *gc.C, runPrecheck precheckRunner) { - err := runPrecheck(newBackendWithMismatchingTools(), &fakeCredentialService{}, s.upgradeService) + err := runPrecheck(newBackendWithMismatchingTools(), &fakeCredentialService{}, s.upgradeService, s.applicationService) c.Assert(err.Error(), gc.Equals, "machine 1 agent binaries don't match model (1.3.1 != 1.2.3)") } func (s *precheckBaseSuite) setupMocks(c *gc.C) *gomock.Controller { ctrl := gomock.NewController(c) s.upgradeService = NewMockUpgradeService(ctrl) + s.applicationService = NewMockApplicationService(ctrl) s.server = upgradevalidationmocks.NewMockServer(ctrl) s.serverFactory = upgradevalidationmocks.NewMockServerFactory(ctrl) return ctrl } +func (s *precheckBaseSuite) expectApplicationLife(appName string, l life.Value) { + s.applicationService.EXPECT().GetApplicationLife(gomock.Any(), appName).Return(l, nil) +} + func (s *precheckBaseSuite) expectIsUpgrade(upgrading bool) { s.upgradeService.EXPECT().IsUpgrading(gomock.Any()).Return(upgrading, nil) } diff --git a/internal/migration/precheck.go b/internal/migration/precheck.go index 2817eb16ba0..56c7bf7a346 100644 --- a/internal/migration/precheck.go +++ b/internal/migration/precheck.go @@ -13,6 +13,7 @@ import ( "github.com/juju/juju/apiserver/common" "github.com/juju/juju/core/credential" + "github.com/juju/juju/core/life" coremigration "github.com/juju/juju/core/migration" "github.com/juju/juju/core/status" "github.com/juju/juju/environs/config" @@ -31,8 +32,9 @@ func SourcePrecheck( environscloudspecGetter environsCloudSpecGetter, credentialService CredentialService, upgradeService UpgradeService, + applicationService ApplicationService, ) error { - c := newPrecheckSource(backend, modelPresence, environscloudspecGetter, credentialService, upgradeService) + c := newPrecheckSource(backend, modelPresence, environscloudspecGetter, credentialService, upgradeService, applicationService) if err := c.checkModel(ctx); err != nil { return errors.Trace(err) } @@ -61,7 +63,7 @@ func SourcePrecheck( if err != nil { return errors.Trace(err) } - controllerCtx := newPrecheckTarget(controllerBackend, controllerPresence, upgradeService) + controllerCtx := newPrecheckTarget(controllerBackend, controllerPresence, upgradeService, applicationService) if err := controllerCtx.checkController(ctx); err != nil { return errors.Annotate(err, "controller") } @@ -77,6 +79,7 @@ func TargetPrecheck(ctx context.Context, modelInfo coremigration.ModelInfo, presence ModelPresence, upgradeService UpgradeService, + applicationService ApplicationService, ) error { if err := modelInfo.Validate(); err != nil { return errors.Trace(err) @@ -110,7 +113,7 @@ func TargetPrecheck(ctx context.Context, modelInfo.ControllerAgentVersion, controllerVersion) } - controllerCtx := newPrecheckTarget(backend, presence, upgradeService) + controllerCtx := newPrecheckTarget(backend, presence, upgradeService, applicationService) if err := controllerCtx.checkController(ctx); err != nil { return errors.Trace(err) } @@ -155,20 +158,23 @@ func newPrecheckTarget( backend PrecheckBackend, presence ModelPresence, upgradeService UpgradeService, + applicationService ApplicationService, ) *precheckTarget { return &precheckTarget{ precheckContext: precheckContext{ - backend: backend, - presence: presence, - upgradeService: upgradeService, + backend: backend, + presence: presence, + upgradeService: upgradeService, + applicationService: applicationService, }, } } type precheckContext struct { - backend PrecheckBackend - presence ModelPresence - upgradeService UpgradeService + backend PrecheckBackend + presence ModelPresence + upgradeService UpgradeService + applicationService ApplicationService } func (c *precheckContext) checkController(ctx context.Context) error { @@ -248,8 +254,12 @@ func (c *precheckContext) checkApplications(ctx context.Context) (map[string][]P } appUnits := make(map[string][]PrecheckUnit, len(apps)) for _, app := range apps { - if app.Life() != state.Alive { - return nil, errors.Errorf("application %s is %s", app.Name(), app.Life()) + appLife, err := c.applicationService.GetApplicationLife(ctx, app.Name()) + if err != nil { + return nil, errors.Annotatef(err, "retrieving life for %q", app.Name()) + } + if appLife != life.Alive { + return nil, errors.Errorf("application %s is %s", app.Name(), appLife) } units, err := app.AllUnits() if err != nil { @@ -275,6 +285,7 @@ func (c *precheckContext) checkUnits(ctx context.Context, app PrecheckApplicatio } for _, unit := range units { + // if unit.Life() != state.Alive { return errors.Errorf("unit %s is %s", unit.Name(), unit.Life()) } @@ -381,12 +392,14 @@ func newPrecheckSource( backend PrecheckBackend, presence ModelPresence, environscloudspecGetter environsCloudSpecGetter, credentialService CredentialService, upgradeService UpgradeService, + applicationService ApplicationService, ) *precheckSource { return &precheckSource{ precheckContext: precheckContext{ - backend: backend, - presence: presence, - upgradeService: upgradeService, + backend: backend, + presence: presence, + upgradeService: upgradeService, + applicationService: applicationService, }, environscloudspecGetter: environscloudspecGetter, credentialService: credentialService, diff --git a/internal/migration/precheck_test.go b/internal/migration/precheck_test.go index 9f3970a3f56..45aceed5be0 100644 --- a/internal/migration/precheck_test.go +++ b/internal/migration/precheck_test.go @@ -17,6 +17,7 @@ import ( "github.com/juju/juju/cloud" "github.com/juju/juju/core/base" "github.com/juju/juju/core/credential" + "github.com/juju/juju/core/life" coremigration "github.com/juju/juju/core/migration" "github.com/juju/juju/core/presence" "github.com/juju/juju/core/status" @@ -43,7 +44,7 @@ type SourcePrecheckSuite struct { var _ = gc.Suite(&SourcePrecheckSuite{}) -func sourcePrecheck(backend migration.PrecheckBackend, credentialService migration.CredentialService, upgradeService migration.UpgradeService) error { +func sourcePrecheck(backend migration.PrecheckBackend, credentialService migration.CredentialService, upgradeService migration.UpgradeService, applicationService migration.ApplicationService) error { return migration.SourcePrecheck( context.Background(), backend, allAlivePresence(), allAlivePresence(), @@ -52,6 +53,7 @@ func sourcePrecheck(backend migration.PrecheckBackend, credentialService migrati }, credentialService, upgradeService, + applicationService, ) } @@ -59,6 +61,8 @@ func (s *SourcePrecheckSuite) TestSuccess(c *gc.C) { defer s.setupMocks(c).Finish() s.expectIsUpgrade(false) + s.expectApplicationLife("foo", life.Alive) + s.expectApplicationLife("bar", life.Alive) backend := newHappyBackend() backend.controllerBackend = newHappyBackend() @@ -70,6 +74,7 @@ func (s *SourcePrecheckSuite) TestSuccess(c *gc.C) { }, &fakeCredentialService{}, s.upgradeService, + s.applicationService, ) c.Assert(err, jc.ErrorIsNil) } @@ -79,13 +84,15 @@ func (s *SourcePrecheckSuite) TestDyingModel(c *gc.C) { backend := newFakeBackend() backend.model.life = state.Dying - err := sourcePrecheck(backend, &fakeCredentialService{}, s.upgradeService) + err := sourcePrecheck(backend, &fakeCredentialService{}, s.upgradeService, s.applicationService) c.Assert(err, gc.ErrorMatches, "model is dying") } func (s *SourcePrecheckSuite) TestCharmUpgrades(c *gc.C) { defer s.setupMocks(c).Finish() + s.expectApplicationLife("spanner", life.Alive) + backend := &fakeBackend{ apps: []migration.PrecheckApplication{ &fakeApp{ @@ -98,7 +105,7 @@ func (s *SourcePrecheckSuite) TestCharmUpgrades(c *gc.C) { }, }, } - err := sourcePrecheck(backend, &fakeCredentialService{}, s.upgradeService) + err := sourcePrecheck(backend, &fakeCredentialService{}, s.upgradeService, s.applicationService) c.Assert(err, gc.ErrorMatches, "unit spanner/1 is upgrading") } @@ -144,6 +151,7 @@ func (s *SourcePrecheckSuite) TestTargetController3Failed(c *gc.C) { }, &fakeCredentialService{}, s.upgradeService, + s.applicationService, ) c.Assert(err.Error(), gc.Equals, ` cannot migrate to controller due to issues: @@ -181,6 +189,7 @@ func (s *SourcePrecheckSuite) TestTargetController2Failed(c *gc.C) { }, &fakeCredentialService{}, s.upgradeService, + s.applicationService, ) c.Assert(err.Error(), gc.Equals, ` cannot migrate to controller due to issues: @@ -193,7 +202,7 @@ func (s *SourcePrecheckSuite) TestImportingModel(c *gc.C) { backend := newFakeBackend() backend.model.migrationMode = state.MigrationModeImporting - err := sourcePrecheck(backend, &fakeCredentialService{}, s.upgradeService) + err := sourcePrecheck(backend, &fakeCredentialService{}, s.upgradeService, s.applicationService) c.Assert(err, gc.ErrorMatches, "model is being imported as part of another migration") } @@ -202,7 +211,7 @@ func (s *SourcePrecheckSuite) TestCleanupsError(c *gc.C) { backend := newFakeBackend() backend.cleanupErr = errors.New("boom") - err := sourcePrecheck(backend, &fakeCredentialService{}, s.upgradeService) + err := sourcePrecheck(backend, &fakeCredentialService{}, s.upgradeService, s.applicationService) c.Assert(err, gc.ErrorMatches, "checking cleanups: boom") } @@ -211,7 +220,7 @@ func (s *SourcePrecheckSuite) TestCleanupsNeeded(c *gc.C) { backend := newFakeBackend() backend.cleanupNeeded = true - err := sourcePrecheck(backend, &fakeCredentialService{}, s.upgradeService) + err := sourcePrecheck(backend, &fakeCredentialService{}, s.upgradeService, s.applicationService) c.Assert(err, gc.ErrorMatches, "cleanup needed") } @@ -221,7 +230,7 @@ func (s *SourcePrecheckSuite) TestIsUpgradingError(c *gc.C) { s.expectIsUpgradeError(errors.New("boom")) backend := newFakeBackend() - err := sourcePrecheck(backend, &fakeCredentialService{}, s.upgradeService) + err := sourcePrecheck(backend, &fakeCredentialService{}, s.upgradeService, s.applicationService) c.Assert(err, gc.ErrorMatches, "controller: checking for upgrades: boom") } @@ -231,7 +240,7 @@ func (s *SourcePrecheckSuite) TestIsUpgrading(c *gc.C) { s.expectIsUpgrade(true) backend := newFakeBackend() - err := sourcePrecheck(backend, &fakeCredentialService{}, s.upgradeService) + err := sourcePrecheck(backend, &fakeCredentialService{}, s.upgradeService, s.applicationService) c.Assert(err, gc.ErrorMatches, "controller: upgrade in progress") } @@ -260,7 +269,7 @@ func (s *SourcePrecheckSuite) TestDyingMachine(c *gc.C) { defer s.setupMocks(c).Finish() backend := newBackendWithDyingMachine() - err := sourcePrecheck(backend, &fakeCredentialService{}, s.upgradeService) + err := sourcePrecheck(backend, &fakeCredentialService{}, s.upgradeService, s.applicationService) c.Assert(err, gc.ErrorMatches, "machine 0 is dying") } @@ -268,14 +277,14 @@ func (s *SourcePrecheckSuite) TestNonStartedMachine(c *gc.C) { defer s.setupMocks(c).Finish() backend := newBackendWithDownMachine() - err := sourcePrecheck(backend, &fakeCredentialService{}, s.upgradeService) + err := sourcePrecheck(backend, &fakeCredentialService{}, s.upgradeService, s.applicationService) c.Assert(err.Error(), gc.Equals, "machine 0 agent not functioning at this time (down)") } func (s *SourcePrecheckSuite) TestProvisioningMachine(c *gc.C) { defer s.setupMocks(c).Finish() - err := sourcePrecheck(newBackendWithProvisioningMachine(), &fakeCredentialService{}, s.upgradeService) + err := sourcePrecheck(newBackendWithProvisioningMachine(), &fakeCredentialService{}, s.upgradeService, s.applicationService) c.Assert(err.Error(), gc.Equals, "machine 0 not running (allocating)") } @@ -293,6 +302,7 @@ func (s *SourcePrecheckSuite) TestDownMachineAgent(c *gc.C) { }, &fakeCredentialService{}, s.upgradeService, + s.applicationService, ) c.Assert(err.Error(), gc.Equals, "machine 1 agent not functioning at this time (down)") } @@ -300,21 +310,24 @@ func (s *SourcePrecheckSuite) TestDownMachineAgent(c *gc.C) { func (s *SourcePrecheckSuite) TestDyingApplication(c *gc.C) { defer s.setupMocks(c).Finish() + s.expectApplicationLife("foo", life.Dying) + backend := &fakeBackend{ apps: []migration.PrecheckApplication{ &fakeApp{ name: "foo", - life: state.Dying, }, }, } - err := sourcePrecheck(backend, &fakeCredentialService{}, s.upgradeService) + err := sourcePrecheck(backend, &fakeCredentialService{}, s.upgradeService, s.applicationService) c.Assert(err.Error(), gc.Equals, "application foo is dying") } func (s *SourcePrecheckSuite) TestWithPendingMinUnits(c *gc.C) { defer s.setupMocks(c).Finish() + s.expectApplicationLife("foo", life.Alive) + backend := &fakeBackend{ apps: []migration.PrecheckApplication{ &fakeApp{ @@ -324,13 +337,16 @@ func (s *SourcePrecheckSuite) TestWithPendingMinUnits(c *gc.C) { }, }, } - err := sourcePrecheck(backend, &fakeCredentialService{}, s.upgradeService) + err := sourcePrecheck(backend, &fakeCredentialService{}, s.upgradeService, s.applicationService) c.Assert(err.Error(), gc.Equals, "application foo is below its minimum units threshold") } func (s *SourcePrecheckSuite) TestUnitVersionsDoNotMatch(c *gc.C) { defer s.setupMocks(c).Finish() + s.expectApplicationLife("foo", life.Alive) + s.expectApplicationLife("bar", life.Alive) + backend := &fakeBackend{ model: fakeModel{modelType: state.ModelTypeIAAS}, apps: []migration.PrecheckApplication{ @@ -347,7 +363,7 @@ func (s *SourcePrecheckSuite) TestUnitVersionsDoNotMatch(c *gc.C) { }, }, } - err := sourcePrecheck(backend, &fakeCredentialService{}, s.upgradeService) + err := sourcePrecheck(backend, &fakeCredentialService{}, s.upgradeService, s.applicationService) c.Assert(err.Error(), gc.Equals, "unit bar/1 agent binaries don't match model (1.2.4 != 1.2.3)") } @@ -355,6 +371,7 @@ func (s *SourcePrecheckSuite) TestCAASModelNoUnitVersionCheck(c *gc.C) { defer s.setupMocks(c).Finish() s.expectIsUpgrade(false) + s.expectApplicationLife("foo", life.Alive) backend := &fakeBackend{ model: fakeModel{modelType: state.ModelTypeCAAS}, @@ -365,13 +382,15 @@ func (s *SourcePrecheckSuite) TestCAASModelNoUnitVersionCheck(c *gc.C) { }, }, } - err := sourcePrecheck(backend, &fakeCredentialService{}, s.upgradeService) + err := sourcePrecheck(backend, &fakeCredentialService{}, s.upgradeService, s.applicationService) c.Assert(err, jc.ErrorIsNil) } func (s *SourcePrecheckSuite) TestDeadUnit(c *gc.C) { defer s.setupMocks(c).Finish() + s.expectApplicationLife("foo", life.Alive) + backend := &fakeBackend{ apps: []migration.PrecheckApplication{ &fakeApp{ @@ -382,7 +401,7 @@ func (s *SourcePrecheckSuite) TestDeadUnit(c *gc.C) { }, }, } - err := sourcePrecheck(backend, &fakeCredentialService{}, s.upgradeService) + err := sourcePrecheck(backend, &fakeCredentialService{}, s.upgradeService, s.applicationService) c.Assert(err.Error(), gc.Equals, "unit foo/0 is dead") } @@ -390,6 +409,7 @@ func (s *SourcePrecheckSuite) TestUnitExecuting(c *gc.C) { defer s.setupMocks(c).Finish() s.expectIsUpgrade(false) + s.expectApplicationLife("foo", life.Alive) backend := &fakeBackend{ apps: []migration.PrecheckApplication{ @@ -401,13 +421,15 @@ func (s *SourcePrecheckSuite) TestUnitExecuting(c *gc.C) { }, }, } - err := sourcePrecheck(backend, &fakeCredentialService{}, s.upgradeService) + err := sourcePrecheck(backend, &fakeCredentialService{}, s.upgradeService, s.applicationService) c.Assert(err, jc.ErrorIsNil) } func (s *SourcePrecheckSuite) TestUnitNotIdle(c *gc.C) { defer s.setupMocks(c).Finish() + s.expectApplicationLife("foo", life.Alive) + backend := &fakeBackend{ apps: []migration.PrecheckApplication{ &fakeApp{ @@ -418,13 +440,15 @@ func (s *SourcePrecheckSuite) TestUnitNotIdle(c *gc.C) { }, }, } - err := sourcePrecheck(backend, &fakeCredentialService{}, s.upgradeService) + err := sourcePrecheck(backend, &fakeCredentialService{}, s.upgradeService, s.applicationService) c.Assert(err.Error(), gc.Equals, "unit foo/0 not idle or executing (failed)") } func (s *SourcePrecheckSuite) TestUnitLost(c *gc.C) { defer s.setupMocks(c).Finish() + s.expectApplicationLife("foo", life.Alive) + backend := newHappyBackend() modelPresence := downAgentPresence("unit-foo-0") controllerPresence := allAlivePresence() @@ -436,6 +460,7 @@ func (s *SourcePrecheckSuite) TestUnitLost(c *gc.C) { }, &fakeCredentialService{}, s.upgradeService, + s.applicationService, ) c.Assert(err.Error(), gc.Equals, "unit foo/0 not idle or executing (lost)") } @@ -445,7 +470,7 @@ func (s *SourcePrecheckSuite) TestDyingControllerModel(c *gc.C) { backend := newFakeBackend() backend.controllerBackend.model.life = state.Dying - err := sourcePrecheck(backend, &fakeCredentialService{}, s.upgradeService) + err := sourcePrecheck(backend, &fakeCredentialService{}, s.upgradeService, s.applicationService) c.Assert(err, gc.ErrorMatches, "controller: model is dying") } @@ -456,7 +481,7 @@ func (s *SourcePrecheckSuite) TestControllerAgentVersionError(c *gc.C) { backend := newFakeBackend() backend.controllerBackend.agentVersionErr = errors.New("boom") - err := sourcePrecheck(backend, &fakeCredentialService{}, s.upgradeService) + err := sourcePrecheck(backend, &fakeCredentialService{}, s.upgradeService, s.applicationService) c.Assert(err, gc.ErrorMatches, "controller: retrieving model version: boom") } @@ -468,7 +493,7 @@ func (s *SourcePrecheckSuite) TestControllerMachineVersionsDoNotMatch(c *gc.C) { backend := newFakeBackend() backend.controllerBackend = newBackendWithMismatchingTools() - err := sourcePrecheck(backend, &fakeCredentialService{}, s.upgradeService) + err := sourcePrecheck(backend, &fakeCredentialService{}, s.upgradeService, s.applicationService) c.Assert(err, gc.ErrorMatches, "controller: machine . agent binaries don't match model.+") } @@ -482,7 +507,7 @@ func (s *SourcePrecheckSuite) TestControllerMachineRequiresReboot(c *gc.C) { backend := newFakeBackend() backend.controllerBackend = newBackendWithRebootingMachine() - err := sourcePrecheck(backend, &fakeCredentialService{}, s.upgradeService) + err := sourcePrecheck(backend, &fakeCredentialService{}, s.upgradeService, s.applicationService) c.Assert(err, gc.ErrorMatches, "controller: machine 0 is scheduled to reboot") } @@ -494,7 +519,7 @@ func (s *SourcePrecheckSuite) TestDyingControllerMachine(c *gc.C) { backend := &fakeBackend{ controllerBackend: newBackendWithDyingMachine(), } - err := sourcePrecheck(backend, &fakeCredentialService{}, s.upgradeService) + err := sourcePrecheck(backend, &fakeCredentialService{}, s.upgradeService, s.applicationService) c.Assert(err, gc.ErrorMatches, "controller: machine 0 is dying") } @@ -506,7 +531,7 @@ func (s *SourcePrecheckSuite) TestNonStartedControllerMachine(c *gc.C) { backend := &fakeBackend{ controllerBackend: newBackendWithDownMachine(), } - err := sourcePrecheck(backend, &fakeCredentialService{}, s.upgradeService) + err := sourcePrecheck(backend, &fakeCredentialService{}, s.upgradeService, s.applicationService) c.Assert(err.Error(), gc.Equals, "controller: machine 0 agent not functioning at this time (down)") } @@ -518,7 +543,7 @@ func (s *SourcePrecheckSuite) TestProvisioningControllerMachine(c *gc.C) { backend := &fakeBackend{ controllerBackend: newBackendWithProvisioningMachine(), } - err := sourcePrecheck(backend, &fakeCredentialService{}, s.upgradeService) + err := sourcePrecheck(backend, &fakeCredentialService{}, s.upgradeService, s.applicationService) c.Assert(err.Error(), gc.Equals, "controller: machine 0 not running (allocating)") } @@ -526,6 +551,8 @@ func (s *SourcePrecheckSuite) TestUnitsAllInScope(c *gc.C) { defer s.setupMocks(c).Finish() s.expectIsUpgrade(false) + s.expectApplicationLife("foo", life.Alive) + s.expectApplicationLife("bar", life.Alive) backend := newHappyBackend() backend.relations = []migration.PrecheckRelation{&fakeRelation{ @@ -539,13 +566,16 @@ func (s *SourcePrecheckSuite) TestUnitsAllInScope(c *gc.C) { "bar/1": {valid: true, inScope: true}, }, }} - err := sourcePrecheck(backend, &fakeCredentialService{}, s.upgradeService) + err := sourcePrecheck(backend, &fakeCredentialService{}, s.upgradeService, s.applicationService) c.Assert(err, jc.ErrorIsNil) } func (s *SourcePrecheckSuite) TestSubordinatesNotYetInScope(c *gc.C) { defer s.setupMocks(c).Finish() + s.expectApplicationLife("foo", life.Alive) + s.expectApplicationLife("bar", life.Alive) + backend := newHappyBackend() backend.relations = []migration.PrecheckRelation{&fakeRelation{ key: "foo:db bar:db", @@ -559,7 +589,7 @@ func (s *SourcePrecheckSuite) TestSubordinatesNotYetInScope(c *gc.C) { "bar/1": {unitName: "bar/1", valid: true, inScope: false}, }, }} - err := sourcePrecheck(backend, &fakeCredentialService{}, s.upgradeService) + err := sourcePrecheck(backend, &fakeCredentialService{}, s.upgradeService, s.applicationService) c.Assert(err, gc.ErrorMatches, `unit bar/1 hasn't joined relation "foo:db bar:db" yet`) } @@ -568,6 +598,9 @@ func (s *SourcePrecheckSuite) TestSubordinatesInvalidUnitsNotYetInScope(c *gc.C) s.expectIsUpgrade(false) + s.expectApplicationLife("foo", life.Alive) + s.expectApplicationLife("bar", life.Alive) + backend := newHappyBackend() backend.relations = []migration.PrecheckRelation{&fakeRelation{ key: "foo:db bar:db", @@ -581,13 +614,16 @@ func (s *SourcePrecheckSuite) TestSubordinatesInvalidUnitsNotYetInScope(c *gc.C) "bar/1": {valid: false, inScope: false}, }, }} - err := sourcePrecheck(backend, &fakeCredentialService{}, s.upgradeService) + err := sourcePrecheck(backend, &fakeCredentialService{}, s.upgradeService, s.applicationService) c.Assert(err, jc.ErrorIsNil) } func (s *SourcePrecheckSuite) TestCrossModelUnitsNotYetInScope(c *gc.C) { defer s.setupMocks(c).Finish() + s.expectApplicationLife("foo", life.Alive) + s.expectApplicationLife("bar", life.Alive) + backend := newHappyBackend() backend.relations = []migration.PrecheckRelation{&fakeRelation{ key: "foo:db remote-mysql:db", @@ -603,7 +639,7 @@ func (s *SourcePrecheckSuite) TestCrossModelUnitsNotYetInScope(c *gc.C) { "remote-mysql": {{unitName: "remote-mysql/0", valid: true, inScope: false}}, }, }} - err := sourcePrecheck(backend, &fakeCredentialService{}, s.upgradeService) + err := sourcePrecheck(backend, &fakeCredentialService{}, s.upgradeService, s.applicationService) c.Assert(err, gc.ErrorMatches, `unit remote-mysql/0 hasn't joined relation "foo:db remote-mysql:db" yet`) } @@ -623,8 +659,8 @@ func (s *TargetPrecheckSuite) SetUpTest(c *gc.C) { } } -func (s *TargetPrecheckSuite) runPrecheck(backend migration.PrecheckBackend, _ migration.CredentialService, upgradeService migration.UpgradeService) error { - return migration.TargetPrecheck(context.Background(), backend, nil, s.modelInfo, allAlivePresence(), upgradeService) +func (s *TargetPrecheckSuite) runPrecheck(backend migration.PrecheckBackend, _ migration.CredentialService, upgradeService migration.UpgradeService, applicationService migration.ApplicationService) error { + return migration.TargetPrecheck(context.Background(), backend, nil, s.modelInfo, allAlivePresence(), upgradeService, applicationService) } func (s *TargetPrecheckSuite) TestSuccess(c *gc.C) { @@ -632,7 +668,7 @@ func (s *TargetPrecheckSuite) TestSuccess(c *gc.C) { s.expectIsUpgrade(false) - err := s.runPrecheck(newHappyBackend(), nil, s.upgradeService) + err := s.runPrecheck(newHappyBackend(), nil, s.upgradeService, s.applicationService) c.Assert(err, jc.ErrorIsNil) } @@ -645,7 +681,7 @@ func (s *TargetPrecheckSuite) TestModelVersionAheadOfTarget(c *gc.C) { sourceVersion.Patch++ s.modelInfo.AgentVersion = sourceVersion - err := s.runPrecheck(backend, nil, s.upgradeService) + err := s.runPrecheck(backend, nil, s.upgradeService, s.applicationService) c.Assert(err.Error(), gc.Equals, `model has higher version than target controller (1.2.4 > 1.2.3)`) } @@ -661,7 +697,7 @@ func (s *TargetPrecheckSuite) TestSourceControllerMajorAhead(c *gc.C) { sourceVersion.Patch = 0 s.modelInfo.ControllerAgentVersion = sourceVersion - err := s.runPrecheck(backend, nil, s.upgradeService) + err := s.runPrecheck(backend, nil, s.upgradeService, s.applicationService) c.Assert(err.Error(), gc.Equals, `source controller has higher version than target controller (2.0.0 > 1.2.3)`) } @@ -676,7 +712,7 @@ func (s *TargetPrecheckSuite) TestSourceControllerMinorAhead(c *gc.C) { sourceVersion.Patch = 0 s.modelInfo.ControllerAgentVersion = sourceVersion - err := s.runPrecheck(backend, nil, s.upgradeService) + err := s.runPrecheck(backend, nil, s.upgradeService, s.applicationService) c.Assert(err.Error(), gc.Equals, `source controller has higher version than target controller (1.3.0 > 1.2.3)`) } @@ -692,7 +728,7 @@ func (s *TargetPrecheckSuite) TestSourceControllerPatchAhead(c *gc.C) { sourceVersion.Patch++ s.modelInfo.ControllerAgentVersion = sourceVersion - c.Assert(s.runPrecheck(backend, nil, s.upgradeService), jc.ErrorIsNil) + c.Assert(s.runPrecheck(backend, nil, s.upgradeService, s.applicationService), jc.ErrorIsNil) } func (s *TargetPrecheckSuite) TestSourceControllerBuildAhead(c *gc.C) { @@ -706,7 +742,7 @@ func (s *TargetPrecheckSuite) TestSourceControllerBuildAhead(c *gc.C) { sourceVersion.Build++ s.modelInfo.ControllerAgentVersion = sourceVersion - c.Assert(s.runPrecheck(backend, nil, s.upgradeService), jc.ErrorIsNil) + c.Assert(s.runPrecheck(backend, nil, s.upgradeService, s.applicationService), jc.ErrorIsNil) } func (s *TargetPrecheckSuite) TestSourceControllerTagMismatch(c *gc.C) { @@ -720,7 +756,7 @@ func (s *TargetPrecheckSuite) TestSourceControllerTagMismatch(c *gc.C) { sourceVersion.Tag = "alpha" s.modelInfo.ControllerAgentVersion = sourceVersion - c.Assert(s.runPrecheck(backend, nil, s.upgradeService), jc.ErrorIsNil) + c.Assert(s.runPrecheck(backend, nil, s.upgradeService, s.applicationService), jc.ErrorIsNil) } func (s *TargetPrecheckSuite) TestDying(c *gc.C) { @@ -728,7 +764,7 @@ func (s *TargetPrecheckSuite) TestDying(c *gc.C) { backend := newFakeBackend() backend.model.life = state.Dying - err := s.runPrecheck(backend, nil, s.upgradeService) + err := s.runPrecheck(backend, nil, s.upgradeService, s.applicationService) c.Assert(err, gc.ErrorMatches, "model is dying") } @@ -755,7 +791,7 @@ func (s *TargetPrecheckSuite) TestIsUpgradingError(c *gc.C) { s.expectIsUpgradeError(errors.New("boom")) backend := newFakeBackend() - err := s.runPrecheck(backend, nil, s.upgradeService) + err := s.runPrecheck(backend, nil, s.upgradeService, s.applicationService) c.Assert(err, gc.ErrorMatches, "checking for upgrades: boom") } @@ -765,7 +801,7 @@ func (s *TargetPrecheckSuite) TestIsUpgrading(c *gc.C) { s.expectIsUpgrade(true) backend := newFakeBackend() - err := s.runPrecheck(backend, nil, s.upgradeService) + err := s.runPrecheck(backend, nil, s.upgradeService, s.applicationService) c.Assert(err, gc.ErrorMatches, "upgrade in progress") } @@ -773,7 +809,7 @@ func (s *TargetPrecheckSuite) TestIsMigrationActiveError(c *gc.C) { defer s.setupMocks(c).Finish() backend := &fakeBackend{migrationActiveErr: errors.New("boom")} - err := s.runPrecheck(backend, nil, s.upgradeService) + err := s.runPrecheck(backend, nil, s.upgradeService, s.applicationService) c.Assert(err, gc.ErrorMatches, "checking for active migration: boom") } @@ -781,7 +817,7 @@ func (s *TargetPrecheckSuite) TestIsMigrationActive(c *gc.C) { defer s.setupMocks(c).Finish() backend := &fakeBackend{migrationActive: true} - err := s.runPrecheck(backend, nil, s.upgradeService) + err := s.runPrecheck(backend, nil, s.upgradeService, s.applicationService) c.Assert(err, gc.ErrorMatches, "model is being migrated out of target controller") } @@ -799,7 +835,7 @@ func (s *TargetPrecheckSuite) TestDyingMachine(c *gc.C) { s.expectIsUpgrade(false) backend := newBackendWithDyingMachine() - err := s.runPrecheck(backend, nil, s.upgradeService) + err := s.runPrecheck(backend, nil, s.upgradeService, s.applicationService) c.Assert(err, gc.ErrorMatches, "machine 0 is dying") } @@ -809,7 +845,7 @@ func (s *TargetPrecheckSuite) TestNonStartedMachine(c *gc.C) { s.expectIsUpgrade(false) backend := newBackendWithDownMachine() - err := s.runPrecheck(backend, nil, s.upgradeService) + err := s.runPrecheck(backend, nil, s.upgradeService, s.applicationService) c.Assert(err.Error(), gc.Equals, "machine 0 agent not functioning at this time (down)") } @@ -819,7 +855,7 @@ func (s *TargetPrecheckSuite) TestProvisioningMachine(c *gc.C) { s.expectIsUpgrade(false) backend := newBackendWithProvisioningMachine() - err := s.runPrecheck(backend, nil, s.upgradeService) + err := s.runPrecheck(backend, nil, s.upgradeService, s.applicationService) c.Assert(err.Error(), gc.Equals, "machine 0 not running (allocating)") } @@ -830,7 +866,7 @@ func (s *TargetPrecheckSuite) TestDownMachineAgent(c *gc.C) { backend := newHappyBackend() modelPresence := downAgentPresence("machine-1") - err := migration.TargetPrecheck(context.Background(), backend, nil, s.modelInfo, modelPresence, s.upgradeService) + err := migration.TargetPrecheck(context.Background(), backend, nil, s.modelInfo, modelPresence, s.upgradeService, s.applicationService) c.Assert(err.Error(), gc.Equals, "machine 1 agent not functioning at this time (down)") } @@ -851,7 +887,7 @@ func (s *TargetPrecheckSuite) TestModelNameAlreadyInUse(c *gc.C) { } backend := newFakeBackend() backend.models = pool.uuids() - err := migration.TargetPrecheck(context.Background(), backend, pool, s.modelInfo, allAlivePresence(), s.upgradeService) + err := migration.TargetPrecheck(context.Background(), backend, pool, s.modelInfo, allAlivePresence(), s.upgradeService, s.applicationService) c.Assert(err, gc.ErrorMatches, "model named \"model-name\" already exists") } @@ -871,7 +907,7 @@ func (s *TargetPrecheckSuite) TestModelNameOverlapOkForDifferentOwner(c *gc.C) { } backend := newFakeBackend() backend.models = pool.uuids() - err := migration.TargetPrecheck(context.Background(), backend, pool, s.modelInfo, allAlivePresence(), s.upgradeService) + err := migration.TargetPrecheck(context.Background(), backend, pool, s.modelInfo, allAlivePresence(), s.upgradeService, s.applicationService) c.Assert(err, jc.ErrorIsNil) } @@ -887,7 +923,7 @@ func (s *TargetPrecheckSuite) TestUUIDAlreadyExists(c *gc.C) { } backend := newFakeBackend() backend.models = pool.uuids() - err := migration.TargetPrecheck(context.Background(), backend, pool, s.modelInfo, allAlivePresence(), s.upgradeService) + err := migration.TargetPrecheck(context.Background(), backend, pool, s.modelInfo, allAlivePresence(), s.upgradeService, s.applicationService) c.Assert(err.Error(), gc.Equals, "model with same UUID already exists (model-uuid)") } @@ -907,7 +943,7 @@ func (s *TargetPrecheckSuite) TestUUIDAlreadyExistsButImporting(c *gc.C) { } backend := newFakeBackend() backend.models = pool.uuids() - err := migration.TargetPrecheck(context.Background(), backend, pool, s.modelInfo, allAlivePresence(), s.upgradeService) + err := migration.TargetPrecheck(context.Background(), backend, pool, s.modelInfo, allAlivePresence(), s.upgradeService, s.applicationService) c.Assert(err, jc.ErrorIsNil) } @@ -920,7 +956,7 @@ func (s *TargetPrecheckSuite) TestFanConfigInModelConfig(c *gc.C) { Config: testing.FakeConfig().Merge(testing.Attrs{"fan-config": "10.100.0.0/16=251.0.0.0/8 192.168.0.0/16=252.0.0.0/8"}), }) - err := s.runPrecheck(backend, nil, s.upgradeService) + err := s.runPrecheck(backend, nil, s.upgradeService, s.applicationService) c.Assert(err.Error(), gc.Equals, "fan networking not supported, remove fan-config \"10.100.0.0/16=251.0.0.0/8 192.168.0.0/16=252.0.0.0/8\" from migrating model config") } @@ -933,11 +969,11 @@ func (s *TargetPrecheckSuite) TestContainerNetworkingFan(c *gc.C) { Config: testing.FakeConfig().Merge(testing.Attrs{"container-networking-method": "fan"}), }) - err := s.runPrecheck(backend, nil, s.upgradeService) + err := s.runPrecheck(backend, nil, s.upgradeService, s.applicationService) c.Assert(err.Error(), gc.Equals, "fan networking not supported, remove container-networking-method \"fan\" from migrating model config") } -type precheckRunner func(migration.PrecheckBackend, migration.CredentialService, migration.UpgradeService) error +type precheckRunner func(migration.PrecheckBackend, migration.CredentialService, migration.UpgradeService, migration.ApplicationService) error func newHappyBackend() *fakeBackend { return &fakeBackend{ @@ -1243,7 +1279,6 @@ func (m *fakeMachine) AgentTools() (*tools.Tools, error) { type fakeApp struct { name string - life state.Life charmURL string units []migration.PrecheckUnit minunits int @@ -1253,10 +1288,6 @@ func (a *fakeApp) Name() string { return a.name } -func (a *fakeApp) Life() state.Life { - return a.life -} - func (a *fakeApp) CharmURL() (*string, bool) { url := a.charmURL if url == "" { diff --git a/internal/worker/caasapplicationprovisioner/application.go b/internal/worker/caasapplicationprovisioner/application.go index f54bf0d0aa4..7d905b419a1 100644 --- a/internal/worker/caasapplicationprovisioner/application.go +++ b/internal/worker/caasapplicationprovisioner/application.go @@ -116,7 +116,7 @@ func (a *appWorker) loop() error { // If the application no longer exists, return immediately. If it's in // Dead state, ensure it's deleted and terminated. appLife, err := a.facade.Life(ctx, a.name) - if errors.Is(err, errors.NotFound) { + if errors.Is(err, applicationerrors.ApplicationNotFound) { a.logger.Debugf("application %q no longer exists", a.name) return nil } else if err != nil { @@ -203,7 +203,7 @@ func (a *appWorker) loop() error { handleChange := func() error { appLife, err = a.facade.Life(ctx, a.name) - if errors.Is(err, errors.NotFound) { + if errors.Is(err, applicationerrors.ApplicationNotFound) { appLife = life.Dead } else if err != nil { return errors.Trace(err) @@ -371,10 +371,7 @@ func (a *appWorker) loop() error { break } err := a.ops.ReconcileDeadUnitScale(ctx, a.name, app, a.facade, a.logger) - // TODO(units): this probably needs to check UnitNotFound as well - if errors.Is(err, applicationerrors.ApplicationNotFound) || - // TODO(units) - remove this when Life() uses the service. - errors.Is(err, errors.NotFound) { + if errors.Is(err, applicationerrors.ApplicationNotFound) || errors.Is(err, applicationerrors.UnitNotFound) { reconcileDeadChan = a.clock.After(retryDelay) shouldRefresh = false } else if errors.Is(err, tryAgain) { diff --git a/internal/worker/caasapplicationprovisioner/application_test.go b/internal/worker/caasapplicationprovisioner/application_test.go index ac94e4c9013..c4a66f72b7b 100644 --- a/internal/worker/caasapplicationprovisioner/application_test.go +++ b/internal/worker/caasapplicationprovisioner/application_test.go @@ -98,7 +98,7 @@ func (s *ApplicationWorkerSuite) TestLifeNotFound(c *gc.C) { broker.EXPECT().Application("test", caas.DeploymentStateful).Return(brokerApp), facade.EXPECT().Life(gomock.Any(), "test").DoAndReturn(func(ctx context.Context, appName string) (life.Value, error) { close(done) - return "", errors.NotFoundf("test charm") + return "", applicationerrors.ApplicationNotFound }), ) appWorker := s.startAppWorker(c, nil, facade, broker, nil, ops, false) diff --git a/internal/worker/caasapplicationprovisioner/ops.go b/internal/worker/caasapplicationprovisioner/ops.go index 1f4b4e8370e..3abcb9004be 100644 --- a/internal/worker/caasapplicationprovisioner/ops.go +++ b/internal/worker/caasapplicationprovisioner/ops.go @@ -278,9 +278,18 @@ func appDying( appName string, app caas.Application, appLife life.Value, facade CAASProvisionerFacade, unitFacade CAASUnitProvisionerFacade, logger logger.Logger, -) error { +) (err error) { + // TODO(units) - remove this once life mgmt fully across to dqlite + // Since life is still handled in both mongo and dqlite, it's possible + // that a mongo cleanup job can mark the app as dead. + defer func() { + if errors.Is(err, applicationerrors.ApplicationIsDead) { + err = nil + } + }() + logger.Debugf("application %q dying", appName) - err := ensureScale(ctx, appName, app, appLife, facade, unitFacade, logger) + err = ensureScale(ctx, appName, app, appLife, facade, unitFacade, logger) if err != nil { return errors.Annotate(err, "cannot scale dying application to 0") } @@ -328,7 +337,8 @@ func checkCharmFormat( logger logger.Logger, ) (isOk bool, err error) { charmInfo, err := facade.ApplicationCharmInfo(ctx, appName) - if errors.Is(err, errors.NotFound) { + // TODO(units) - remove the dead check once life mgmt fully across to dqlite + if errors.Is(err, errors.NotFound) || errors.Is(err, applicationerrors.ApplicationIsDead) { logger.Debugf("application %q no longer exists", appName) return false, nil } else if err != nil { @@ -687,7 +697,7 @@ func ensureScale( return updateProvisioningState(ctx, appName, false, 0, facade) } - unitsToDestroy, err := app.UnitsToRemove(context.TODO(), ps.ScaleTarget) + unitsToDestroy, err := app.UnitsToRemove(ctx, ps.ScaleTarget) if err != nil && errors.Is(err, errors.NotFound) { return nil } else if err != nil { diff --git a/internal/worker/caasapplicationprovisioner/worker.go b/internal/worker/caasapplicationprovisioner/worker.go index a41bba57952..3fee37811c6 100644 --- a/internal/worker/caasapplicationprovisioner/worker.go +++ b/internal/worker/caasapplicationprovisioner/worker.go @@ -30,6 +30,7 @@ import ( "github.com/juju/juju/core/resources" "github.com/juju/juju/core/status" "github.com/juju/juju/core/watcher" + applicationerrors "github.com/juju/juju/domain/application/errors" "github.com/juju/juju/rpc/params" ) @@ -180,10 +181,10 @@ func (p *provisioner) loop() error { } for _, appName := range apps { _, err := p.facade.Life(ctx, appName) - if err != nil && !errors.Is(err, errors.NotFound) { + if err != nil && !errors.Is(err, applicationerrors.ApplicationNotFound) { return errors.Trace(err) } - if errors.Is(err, errors.NotFound) { + if errors.Is(err, applicationerrors.ApplicationNotFound) || errors.Is(err, applicationerrors.ApplicationIsDead) { p.logger.Debugf("application %q not found, ignoring", appName) continue } diff --git a/internal/worker/caasfirewaller/applicationworker.go b/internal/worker/caasfirewaller/applicationworker.go index b7a818537d3..7d679555e2f 100644 --- a/internal/worker/caasfirewaller/applicationworker.go +++ b/internal/worker/caasfirewaller/applicationworker.go @@ -15,6 +15,7 @@ import ( "github.com/juju/juju/core/logger" "github.com/juju/juju/core/network" "github.com/juju/juju/core/watcher" + applicationerrors "github.com/juju/juju/domain/application/errors" ) type applicationWorker struct { @@ -112,7 +113,8 @@ func (w *applicationWorker) setUp(ctx context.Context) (err error) { func (w *applicationWorker) loop() (err error) { defer func() { // If the application has been deleted, we can return nil. - if errors.Is(err, errors.NotFound) { + // TODO(units): remove generic not found error when everything is in dqlite + if errors.Is(err, errors.NotFound) || errors.Is(err, applicationerrors.ApplicationNotFound) { w.logger.Debugf("sidecar caas firewaller application %v has been removed", w.appName) err = nil } diff --git a/internal/worker/caasfirewaller/worker.go b/internal/worker/caasfirewaller/worker.go index 6460cad406e..601e309d95b 100644 --- a/internal/worker/caasfirewaller/worker.go +++ b/internal/worker/caasfirewaller/worker.go @@ -12,6 +12,7 @@ import ( "github.com/juju/juju/core/life" "github.com/juju/juju/core/logger" + applicationerrors "github.com/juju/juju/domain/application/errors" "github.com/juju/juju/internal/charm" ) @@ -121,7 +122,8 @@ func (p *firewaller) loop() error { for _, appName := range apps { // If charm is a v1 charm, skip processing. format, err := p.charmFormat(ctx, appName) - if errors.Is(err, errors.NotFound) { + // TODO(units) - remove the dead check once life mgmt fully across to dqlite + if errors.Is(err, errors.NotFound) || errors.Is(err, applicationerrors.ApplicationIsDead) { p.config.Logger.Debugf("application %q no longer exists", appName) continue } else if err != nil { @@ -133,7 +135,7 @@ func (p *firewaller) loop() error { } appLife, err := p.config.LifeGetter.Life(ctx, appName) - if errors.Is(err, errors.NotFound) || appLife == life.Dead { + if errors.Is(err, applicationerrors.ApplicationNotFound) || appLife == life.Dead { w, ok := p.appWorkers[appName] if ok { if err := worker.Stop(w); err != nil { diff --git a/internal/worker/caasfirewaller/worker_test.go b/internal/worker/caasfirewaller/worker_test.go index 2bae97cd710..8af61aaa25e 100644 --- a/internal/worker/caasfirewaller/worker_test.go +++ b/internal/worker/caasfirewaller/worker_test.go @@ -16,6 +16,7 @@ import ( "github.com/juju/juju/core/logger" "github.com/juju/juju/core/watcher" "github.com/juju/juju/core/watcher/watchertest" + applicationerrors "github.com/juju/juju/domain/application/errors" "github.com/juju/juju/internal/charm" loggertesting "github.com/juju/juju/internal/logger/testing" "github.com/juju/juju/internal/testing" @@ -157,7 +158,7 @@ func (s *workerSuite) TestStartStop(c *gc.C) { app2Worker.EXPECT().Wait().Return(nil) s.firewallerAPI.EXPECT().ApplicationCharmInfo(gomock.Any(), "app1").Return(charmInfo, nil) - s.lifeGetter.EXPECT().Life(gomock.Any(), "app1").Return(life.Value(""), errors.NotFoundf("%q", "app1")) + s.lifeGetter.EXPECT().Life(gomock.Any(), "app1").Return(life.Value(""), applicationerrors.ApplicationNotFound) // Stopped app1's worker because it's removed. app1Worker.EXPECT().Kill() app1Worker.EXPECT().Wait().Return(nil) diff --git a/internal/worker/deployer/deployer.go b/internal/worker/deployer/deployer.go index 3dbe1f8552a..b448d52914e 100644 --- a/internal/worker/deployer/deployer.go +++ b/internal/worker/deployer/deployer.go @@ -157,7 +157,7 @@ func (d *Deployer) changed(ctx context.Context, unitName string) error { d.logger.Infof("checking unit %q", unitName) var unitLife life.Value unit, err := d.client.Unit(ctx, unitTag) - if params.IsCodeNotFoundOrCodeUnauthorized(err) { + if params.IsCodeUnauthorized(err) || params.IsCodeUnitNotFound(err) { unitLife = life.Dead } else if err != nil { return err diff --git a/internal/worker/firewaller/firewaller.go b/internal/worker/firewaller/firewaller.go index 7fa3f90fba4..e8e5e0748fe 100644 --- a/internal/worker/firewaller/firewaller.go +++ b/internal/worker/firewaller/firewaller.go @@ -695,7 +695,7 @@ func (fw *Firewaller) unitsChanged(ctx context.Context, change *unitsChange) err for _, name := range change.units { unitTag := names.NewUnitTag(name) unit, err := fw.firewallerApi.Unit(ctx, unitTag) - if err != nil && !params.IsCodeNotFound(err) { + if err != nil && !params.IsCodeUnitNotFound(err) { return err } var machineTag names.MachineTag diff --git a/internal/worker/uniter/relation/statemanager.go b/internal/worker/uniter/relation/statemanager.go index 4aff4a2abe4..efae8627083 100644 --- a/internal/worker/uniter/relation/statemanager.go +++ b/internal/worker/uniter/relation/statemanager.go @@ -61,7 +61,10 @@ func (m *stateManager) RemoveRelation(ctx context.Context, id int, unitGetter Un unitExists, ok := knownUnits[unitName] if !ok { _, err := unitGetter.Unit(ctx, names.NewUnitTag(unitName)) - if err != nil && !params.IsCodeNotFoundOrCodeUnauthorized(err) { + ignoreError := params.IsCodeUnauthorized(err) || params.IsCodeUnitNotFound(err) + // TODO(units): when all apis are on dqlite, remove this. + ignoreError = ignoreError || params.IsCodeNotFound(err) + if err != nil && !ignoreError { return errors.Trace(err) } unitExists = err == nil diff --git a/internal/worker/uniter/relation/statemanager_test.go b/internal/worker/uniter/relation/statemanager_test.go index 0574705752d..b5bcc1623e9 100644 --- a/internal/worker/uniter/relation/statemanager_test.go +++ b/internal/worker/uniter/relation/statemanager_test.go @@ -191,7 +191,7 @@ func (s *stateManagerSuite) TestRemoveIgnoresMissingUnits(c *gc.C) { stateTwo.Members = map[string]int64{"foo/1": 0} s.expectState(c, stateTwo) s.expectSetStateEmpty(c) - s.mockUnitGetter.EXPECT().Unit(gomock.Any(), names.NewUnitTag("foo/1")).Return(nil, ¶ms.Error{Code: "not found"}) + s.mockUnitGetter.EXPECT().Unit(gomock.Any(), names.NewUnitTag("foo/1")).Return(nil, ¶ms.Error{Code: "unit not found"}) logger := logger.GetLogger("test") var tw loggo.TestWriter @@ -215,7 +215,7 @@ func (s *stateManagerSuite) TestRemoveCachesUnits(c *gc.C) { stateThree.Members = map[string]int64{"foo/1": 0} s.expectState(c, stateTwo, stateThree) s.expectSetState(c, stateThree) - s.mockUnitGetter.EXPECT().Unit(gomock.Any(), names.NewUnitTag("foo/1")).Return(nil, ¶ms.Error{Code: "not found"}) + s.mockUnitGetter.EXPECT().Unit(gomock.Any(), names.NewUnitTag("foo/1")).Return(nil, ¶ms.Error{Code: "unit not found"}) mgr, err := relation.NewStateManager(context.Background(), s.mockUnitRW, loggertesting.WrapCheckLog(c)) c.Assert(err, jc.ErrorIsNil) diff --git a/rpc/params/apierror.go b/rpc/params/apierror.go index 53795eba4c3..2f8a93b0f97 100644 --- a/rpc/params/apierror.go +++ b/rpc/params/apierror.go @@ -216,7 +216,11 @@ const ( CodeAccessRequired = "access required" CodeAppShouldNotHaveUnits = "application should not have units" CodeApplicationNotFound = "application not found" + CodeApplicationAlreadyExists = "application already exists" + CodeApplicationIsDead = "application is dead" CodeScalingStateInconsistent = "scaling state inconsistent" + CodeUnitNotFound = "unit not found" + CodeUnitAlreadyExists = "unit already exists" // // Tag based error @@ -305,8 +309,16 @@ func TranslateWellKnownError(err error) error { return fmt.Errorf("%s%w", err.Error(), errors.Hide(secretbackenderrors.NotFound)) case CodeApplicationNotFound: return fmt.Errorf("%s%w", err.Error(), errors.Hide(applicationerrors.ApplicationNotFound)) + case CodeApplicationAlreadyExists: + return fmt.Errorf("%s%w", err.Error(), errors.Hide(applicationerrors.ApplicationAlreadyExists)) + case CodeApplicationIsDead: + return fmt.Errorf("%s%w", err.Error(), errors.Hide(applicationerrors.ApplicationIsDead)) case CodeScalingStateInconsistent: return fmt.Errorf("%s%w", err.Error(), errors.Hide(applicationerrors.ScalingStateInconsistent)) + case CodeUnitAlreadyExists: + return fmt.Errorf("%s%w", err.Error(), errors.Hide(applicationerrors.UnitAlreadyExists)) + case CodeUnitNotFound: + return fmt.Errorf("%s%w", err.Error(), errors.Hide(applicationerrors.UnitNotFound)) case CodeUnauthorized: return errors.NewUnauthorized(err, "") case CodeNotImplemented: @@ -416,6 +428,14 @@ func IsCodeScalingStateInconsistent(err error) bool { return ErrCode(err) == CodeScalingStateInconsistent } +func IsCodeUnitAlreadyExists(err error) bool { + return ErrCode(err) == CodeUnitAlreadyExists +} + +func IsCodeUnitNotFound(err error) bool { + return ErrCode(err) == CodeUnitNotFound +} + func IsCodeUnauthorized(err error) bool { return ErrCode(err) == CodeUnauthorized } diff --git a/rpc/params/apierror_test.go b/rpc/params/apierror_test.go index 4b4bf8a0847..c6c8032c56f 100644 --- a/rpc/params/apierror_test.go +++ b/rpc/params/apierror_test.go @@ -60,6 +60,8 @@ func (*errorSuite) TestTranslateWellKnownError(c *gc.C) { {params.CodeSecretBackendNotSupported, params.Error{Code: params.CodeSecretBackendNotSupported, Message: "secret backend not found"}, secretbackenderrors.NotSupported}, {params.CodeSecretBackendNotValid, params.Error{Code: params.CodeSecretBackendNotValid, Message: "secret backend not found"}, secretbackenderrors.NotValid}, {params.CodeSecretBackendForbidden, params.Error{Code: params.CodeSecretBackendForbidden, Message: "secret backend not found"}, secretbackenderrors.Forbidden}, + {params.CodeUnitNotFound, params.Error{Code: params.CodeUnitNotFound, Message: "unit not found"}, applicationerrors.UnitNotFound}, + {params.CodeUnitAlreadyExists, params.Error{Code: params.CodeUnitAlreadyExists, Message: "unit already exists"}, applicationerrors.UnitAlreadyExists}, {params.CodeApplicationNotFound, params.Error{Code: params.CodeApplicationNotFound, Message: "application not found"}, applicationerrors.ApplicationNotFound}, {params.CodeScalingStateInconsistent, params.Error{Code: params.CodeScalingStateInconsistent, Message: "scaling state inconsistent"}, applicationerrors.ScalingStateInconsistent}, } diff --git a/state/cleanup.go b/state/cleanup.go index 2f37863b593..fb290cb8df3 100644 --- a/state/cleanup.go +++ b/state/cleanup.go @@ -175,6 +175,7 @@ type MachineRemover interface { type ApplicationService interface { DestroyApplication(context.Context, string) error DeleteApplication(context.Context, string) error + EnsureApplicationDead(ctx context.Context, appName string) error EnsureUnitDead(context.Context, string, leadership.Revoker) error DestroyUnit(context.Context, string) error DeleteUnit(context.Context, string) error @@ -624,8 +625,13 @@ func (st *State) cleanupApplication(ctx context.Context, store objectstore.Objec err = st.ApplyOperation(op) if len(op.Errors) != 0 { logger.Warningf("operational errors cleaning up application %v: %v", appName, op.Errors) - } else if err == nil && op.Removed { - err = appService.DeleteApplication(ctx, appName) + } else if err == nil { + if op.Removed { + err = appService.DeleteApplication(ctx, appName) + } + if op.PostDestroyAppLife == Dead { + err = appService.EnsureApplicationDead(ctx, appName) + } } return err } diff --git a/state/cleanup_test.go b/state/cleanup_test.go index f29d6f17096..09c74a63af8 100644 --- a/state/cleanup_test.go +++ b/state/cleanup_test.go @@ -1703,3 +1703,7 @@ func (r fakeAppRemover) DestroyUnit(context.Context, string) error { } func (fakeAppRemover) DeleteApplication(context.Context, string) error { return nil } + +func (fakeAppRemover) EnsureApplicationDead(ctx context.Context, appName string) error { + return nil +} From a922128fb52952b918d38f48e408347d283d577b Mon Sep 17 00:00:00 2001 From: wallyworld Date: Thu, 12 Sep 2024 23:25:47 +1000 Subject: [PATCH 2/3] feat(units): revert use of bespoke errors in workers Instead of propagating domain errors to the workers, wrap them in generic errors. --- api/agent/agent/facade.go | 2 +- api/agent/caasapplication/client.go | 8 ++-- api/agent/caasapplication/client_test.go | 7 ++- .../caasapplicationprovisioner/client.go | 2 +- api/package_test.go | 1 - apiserver/errors/errors.go | 25 +--------- apiserver/errors/errors_test.go | 11 ----- apiserver/facades/agent/agent/agent.go | 4 ++ .../agent/caasapplication/application.go | 16 ++++++- apiserver/facades/agent/deployer/deployer.go | 10 ++++ apiserver/facades/agent/uniter/uniter.go | 6 +++ .../facades/client/application/application.go | 6 +++ .../application/application_unit_test.go | 2 +- .../caasapplicationprovisioner/provisioner.go | 9 ++++ .../controller/caasfirewaller/firewaller.go | 7 +++ .../caasunitprovisioner/provisioner.go | 7 +++ .../crossmodelrelations.go | 1 - .../controller/firewaller/firewaller.go | 4 ++ .../firewaller/firewaller_base_test.go | 2 +- .../controller/firewaller/interface.go | 1 + cmd/containeragent/initialize/command.go | 3 +- cmd/containeragent/initialize/command_test.go | 6 +-- cmd/containeragent/initialize/package_test.go | 1 - domain/application/errors/errors.go | 3 -- domain/application/service/application.go | 10 +++- domain/application/service_test.go | 6 +-- domain/application/state/application.go | 17 +++---- domain/application/state/application_test.go | 10 ---- internal/migration/precheck.go | 5 +- .../caasapplicationprovisioner/application.go | 7 ++- .../application_test.go | 5 +- .../worker/caasapplicationprovisioner/ops.go | 19 ++------ .../caasapplicationprovisioner/worker.go | 5 +- .../caasfirewaller/applicationworker.go | 4 +- internal/worker/caasfirewaller/worker.go | 6 +-- internal/worker/caasfirewaller/worker_test.go | 3 +- internal/worker/deployer/deployer.go | 2 +- internal/worker/firewaller/firewaller.go | 2 +- .../worker/uniter/relation/statemanager.go | 5 +- .../uniter/relation/statemanager_test.go | 4 +- rpc/params/apierror.go | 47 ------------------- rpc/params/apierror_test.go | 5 -- 42 files changed, 127 insertions(+), 179 deletions(-) diff --git a/api/agent/agent/facade.go b/api/agent/agent/facade.go index 8d6fbb0913d..032e8941570 100644 --- a/api/agent/agent/facade.go +++ b/api/agent/agent/facade.go @@ -76,7 +76,7 @@ func (facade *connFacade) Life(ctx context.Context, entity names.Tag) (Life, err return "", errors.Errorf("expected 1 result, got %d", len(results.Entities)) } if err := results.Entities[0].Error; err != nil { - if params.IsCodeNotFoundOrCodeUnauthorized(err) || params.IsCodeUnitNotFound(err) { + if params.IsCodeNotFoundOrCodeUnauthorized(err) { return "", ErrDenied } return "", errors.Trace(err) diff --git a/api/agent/caasapplication/client.go b/api/agent/caasapplication/client.go index 9ffcb2a079f..580a2683250 100644 --- a/api/agent/caasapplication/client.go +++ b/api/agent/caasapplication/client.go @@ -6,10 +6,10 @@ package caasapplication import ( "context" + "github.com/juju/errors" "github.com/juju/names/v5" "github.com/juju/juju/api/base" - applicationerrors "github.com/juju/juju/domain/application/errors" "github.com/juju/juju/rpc/params" ) @@ -52,10 +52,10 @@ func (c *Client) UnitIntroduction(ctx context.Context, podName string, podUUID s return nil, err } if err := result.Error; err != nil { - if params.IsCodeUnitAlreadyExists(err) { - return nil, applicationerrors.UnitAlreadyExists + if params.IsCodeAlreadyExists(err) { + return nil, errors.AlreadyExists } else if params.IsCodeNotAssigned(err) { - return nil, applicationerrors.UnitNotAssigned + return nil, errors.NotAssigned } return nil, err } diff --git a/api/agent/caasapplication/client_test.go b/api/agent/caasapplication/client_test.go index 13cea1badb1..93be65c5df5 100644 --- a/api/agent/caasapplication/client_test.go +++ b/api/agent/caasapplication/client_test.go @@ -14,7 +14,6 @@ import ( "github.com/juju/juju/api/agent/caasapplication" basetesting "github.com/juju/juju/api/base/testing" - applicationerrors "github.com/juju/juju/domain/application/errors" "github.com/juju/juju/rpc/params" ) @@ -91,12 +90,12 @@ func (s *provisionerSuite) TestUnitIntroductionFailAlreadyExists(c *gc.C) { c.Assert(args.PodUUID, gc.Equals, "pod-uuid") c.Assert(result, gc.FitsTypeOf, ¶ms.CAASUnitIntroductionResult{}) *(result.(*params.CAASUnitIntroductionResult)) = params.CAASUnitIntroductionResult{ - Error: ¶ms.Error{Code: params.CodeUnitAlreadyExists}, + Error: ¶ms.Error{Code: params.CodeAlreadyExists}, } return nil }) _, err := client.UnitIntroduction(context.Background(), "pod-name", "pod-uuid") - c.Assert(err, jc.ErrorIs, applicationerrors.UnitAlreadyExists) + c.Assert(err, jc.ErrorIs, errors.AlreadyExists) c.Assert(called, jc.IsTrue) } @@ -118,7 +117,7 @@ func (s *provisionerSuite) TestUnitIntroductionFailNotAssigned(c *gc.C) { return nil }) _, err := client.UnitIntroduction(context.Background(), "pod-name", "pod-uuid") - c.Assert(err, jc.ErrorIs, applicationerrors.UnitNotAssigned) + c.Assert(err, jc.ErrorIs, errors.NotAssigned) c.Assert(called, jc.IsTrue) } diff --git a/api/controller/caasapplicationprovisioner/client.go b/api/controller/caasapplicationprovisioner/client.go index d80b91bacbe..b6f98d70328 100644 --- a/api/controller/caasapplicationprovisioner/client.go +++ b/api/controller/caasapplicationprovisioner/client.go @@ -409,7 +409,7 @@ func (c *Client) RemoveUnit(ctx context.Context, unitName string) error { return err } resultErr := result.OneError() - if params.IsCodeUnitNotFound(resultErr) { + if params.IsCodeNotFound(resultErr) { return nil } return resultErr diff --git a/api/package_test.go b/api/package_test.go index 81753da3b84..4221a310e63 100644 --- a/api/package_test.go +++ b/api/package_test.go @@ -51,7 +51,6 @@ func (*ImportSuite) TestImports(c *gc.C) { "core/user", "core/version", "core/watcher", - "domain/application/errors", "domain/model/errors", "domain/secret/errors", "domain/secretbackend/errors", diff --git a/apiserver/errors/errors.go b/apiserver/errors/errors.go index 781d3b8379a..d933b34b64b 100644 --- a/apiserver/errors/errors.go +++ b/apiserver/errors/errors.go @@ -15,7 +15,6 @@ import ( "github.com/juju/juju/core/lease" corelogger "github.com/juju/juju/core/logger" "github.com/juju/juju/core/upgrade" - applicationerrors "github.com/juju/juju/domain/application/errors" modelerrors "github.com/juju/juju/domain/model/errors" secreterrors "github.com/juju/juju/domain/secret/errors" secretbackenderrors "github.com/juju/juju/domain/secretbackend/errors" @@ -120,15 +119,9 @@ func ServerErrorAndStatus(err error) (*params.Error, int) { params.CodeSecretNotFound, params.CodeSecretRevisionNotFound, params.CodeSecretConsumerNotFound, - params.CodeSecretBackendNotFound, - params.CodeApplicationNotFound, - params.CodeUnitNotFound: + params.CodeSecretBackendNotFound: status = http.StatusNotFound - case params.CodeBadRequest, - params.CodeScalingStateInconsistent, - params.CodeApplicationAlreadyExists, - params.CodeApplicationIsDead, - params.CodeUnitAlreadyExists: + case params.CodeBadRequest: status = http.StatusBadRequest case params.CodeMethodNotAllowed: status = http.StatusMethodNotAllowed @@ -237,20 +230,6 @@ func ServerError(err error) *params.Error { code = params.CodeSecretBackendNotSupported case errors.Is(err, errors.BadRequest): code = params.CodeBadRequest - case errors.Is(err, applicationerrors.ApplicationNotFound): - code = params.CodeApplicationNotFound - case errors.Is(err, applicationerrors.ScalingStateInconsistent): - code = params.CodeScalingStateInconsistent - case errors.Is(err, applicationerrors.UnitNotAssigned): - code = params.CodeNotAssigned - case errors.Is(err, applicationerrors.ApplicationAlreadyExists): - code = params.CodeApplicationAlreadyExists - case errors.Is(err, applicationerrors.ApplicationIsDead): - code = params.CodeApplicationIsDead - case errors.Is(err, applicationerrors.UnitAlreadyExists): - code = params.CodeUnitAlreadyExists - case errors.Is(err, applicationerrors.UnitNotFound): - code = params.CodeUnitNotFound case errors.Is(err, errors.MethodNotAllowed): code = params.CodeMethodNotAllowed case errors.Is(err, errors.NotImplemented): diff --git a/apiserver/errors/errors_test.go b/apiserver/errors/errors_test.go index 6b25105177e..71216bde829 100644 --- a/apiserver/errors/errors_test.go +++ b/apiserver/errors/errors_test.go @@ -20,7 +20,6 @@ import ( "github.com/juju/juju/core/leadership" "github.com/juju/juju/core/lease" "github.com/juju/juju/core/network" - applicationerrors "github.com/juju/juju/domain/application/errors" secreterrors "github.com/juju/juju/domain/secret/errors" secretbackenderrors "github.com/juju/juju/domain/secretbackend/errors" "github.com/juju/juju/internal/testing" @@ -70,16 +69,6 @@ var errorTransformTests = []struct { code: params.CodeSecretBackendNotFound, status: http.StatusNotFound, helperFunc: params.IsCodeSecretBackendNotFound, -}, { - err: applicationerrors.ApplicationNotFound, - code: params.CodeApplicationNotFound, - status: http.StatusNotFound, - helperFunc: params.IsCodeApplicationNotFound, -}, { - err: applicationerrors.ScalingStateInconsistent, - code: params.CodeScalingStateInconsistent, - status: http.StatusBadRequest, - helperFunc: params.IsCodeScalingStateInconsistent, }, { err: secretbackenderrors.Forbidden, code: params.CodeSecretBackendForbidden, diff --git a/apiserver/facades/agent/agent/agent.go b/apiserver/facades/agent/agent/agent.go index 830412a0c27..7f0785c9ea1 100644 --- a/apiserver/facades/agent/agent/agent.go +++ b/apiserver/facades/agent/agent/agent.go @@ -20,6 +20,7 @@ import ( "github.com/juju/juju/core/credential" "github.com/juju/juju/core/life" "github.com/juju/juju/core/model" + applicationerrors "github.com/juju/juju/domain/application/errors" "github.com/juju/juju/internal/mongo" "github.com/juju/juju/rpc/params" "github.com/juju/juju/state" @@ -115,6 +116,9 @@ func (api *AgentAPI) GetEntities(ctx context.Context, args params.Entities) para // Eventually all entities will be supported via dqlite. if tag.Kind() == names.UnitTagKind { lifeValue, err := api.applicationService.GetUnitLife(ctx, tag.Id()) + if errors.Is(err, applicationerrors.UnitNotFound) { + err = errors.NotFoundf("unit %s", tag.Id()) + } results.Entities[i].Life = lifeValue results.Entities[i].Error = apiservererrors.ServerError(err) continue diff --git a/apiserver/facades/agent/caasapplication/application.go b/apiserver/facades/agent/caasapplication/application.go index e323dfb749c..8bc219cfbe8 100644 --- a/apiserver/facades/agent/caasapplication/application.go +++ b/apiserver/facades/agent/caasapplication/application.go @@ -23,6 +23,7 @@ import ( "github.com/juju/juju/core/logger" "github.com/juju/juju/core/network" "github.com/juju/juju/core/paths" + applicationerrors "github.com/juju/juju/domain/application/errors" applicationservice "github.com/juju/juju/domain/application/service" "github.com/juju/juju/internal/password" "github.com/juju/juju/rpc/params" @@ -96,8 +97,16 @@ func (f *Facade) UnitIntroduction(ctx context.Context, args params.CAASUnitIntro return params.CAASUnitIntroductionResult{}, apiservererrors.ErrPerm } + var unitName string errResp := func(err error) (params.CAASUnitIntroductionResult, error) { f.logger.Warningf("error introducing k8s pod %q: %v", args.PodName, err) + if errors.Is(err, applicationerrors.ApplicationNotFound) { + err = errors.NotFoundf("appliction %s", tag.Name) + } else if errors.Is(err, applicationerrors.UnitAlreadyExists) { + err = errors.AlreadyExistsf("unit %s", unitName) + } else if errors.Is(err, applicationerrors.UnitNotAssigned) { + err = errors.NotAssignedf("unit %s", unitName) + } return params.CAASUnitIntroductionResult{Error: apiservererrors.ServerError(err)}, nil } @@ -137,8 +146,8 @@ func (f *Facade) UnitIntroduction(ctx context.Context, args params.CAASUnitIntro if err != nil { return errResp(err) } - n := fmt.Sprintf("%s/%d", appName, ord) - upsert.UnitName = &n + unitName = fmt.Sprintf("%s/%d", appName, ord) + upsert.UnitName = &unitName upsert.OrderedId = ord upsert.OrderedScale = true default: @@ -266,6 +275,9 @@ func (f *Facade) UnitTerminating(ctx context.Context, args params.Entity) (param } errResp := func(err error) (params.CAASUnitTerminationResult, error) { + if errors.Is(err, applicationerrors.ApplicationNotFound) { + err = errors.NotFoundf("application %s", tag.Id()) + } return params.CAASUnitTerminationResult{Error: apiservererrors.ServerError(err)}, nil } diff --git a/apiserver/facades/agent/deployer/deployer.go b/apiserver/facades/agent/deployer/deployer.go index 9da469e0039..3eeacfa00ee 100644 --- a/apiserver/facades/agent/deployer/deployer.go +++ b/apiserver/facades/agent/deployer/deployer.go @@ -16,6 +16,7 @@ import ( "github.com/juju/juju/core/leadership" "github.com/juju/juju/core/life" "github.com/juju/juju/core/objectstore" + applicationerrors "github.com/juju/juju/domain/application/errors" "github.com/juju/juju/rpc/params" "github.com/juju/juju/state" ) @@ -175,6 +176,9 @@ func (d *DeployerAPI) Life(ctx context.Context, args params.Entities) (params.Li continue } lifeValue, err := d.applicationService.GetUnitLife(ctx, tag.Id()) + if errors.Is(err, applicationerrors.UnitNotFound) { + err = errors.NotFoundf("unit %s", tag.Id()) + } result.Results[i].Life = lifeValue result.Results[i].Error = apiservererrors.ServerError(err) } @@ -242,12 +246,18 @@ func (d *DeployerAPI) Remove(ctx context.Context, args params.Entities) (params. // Given the way dual write works, we need this for now. if err = d.applicationService.EnsureUnitDead(ctx, tag.Id(), d.leadershipRevoker); err != nil { + if errors.Is(err, applicationerrors.UnitNotFound) { + err = errors.NotFoundf("unit %s", tag.Id()) + } result.Results[i].Error = apiservererrors.ServerError(err) continue } // This is the call we will keep once mongo is removed. // We will need to remove the alive check. if err = d.applicationService.RemoveUnit(ctx, tag.Id(), d.leadershipRevoker); err != nil { + if errors.Is(err, applicationerrors.UnitNotFound) { + err = errors.NotFoundf("unit %s", tag.Id()) + } result.Results[i].Error = apiservererrors.ServerError(err) continue } diff --git a/apiserver/facades/agent/uniter/uniter.go b/apiserver/facades/agent/uniter/uniter.go index 4b0d12c1cf3..2faee21b8e1 100644 --- a/apiserver/facades/agent/uniter/uniter.go +++ b/apiserver/facades/agent/uniter/uniter.go @@ -1104,8 +1104,14 @@ func (u *UniterAPI) Life(ctx context.Context, args params.Entities) (params.Life switch tag.Kind() { case names.ApplicationTagKind: lifeValue, err = u.applicationService.GetApplicationLife(ctx, tag.Id()) + if errors.Is(err, applicationerrors.ApplicationNotFound) { + err = errors.NotFoundf("application %s", tag.Id()) + } case names.UnitTagKind: lifeValue, err = u.applicationService.GetUnitLife(ctx, tag.Id()) + if errors.Is(err, applicationerrors.UnitNotFound) { + err = errors.NotFoundf("unit %s", tag.Id()) + } default: result.Results[i].Error = apiservererrors.ServerError(apiservererrors.ErrPerm) continue diff --git a/apiserver/facades/client/application/application.go b/apiserver/facades/client/application/application.go index 6771dbafd2b..e03e247da5d 100644 --- a/apiserver/facades/client/application/application.go +++ b/apiserver/facades/client/application/application.go @@ -2322,6 +2322,9 @@ func (api *APIBase) ApplicationsInfo(ctx context.Context, in params.Entities) (p continue } appLife, err := api.applicationService.GetApplicationLife(ctx, tag.Name) + if errors.Is(err, applicationerrors.ApplicationNotFound) { + err = errors.NotFoundf("application %q", tag.Name) + } if err != nil { out[i].Error = apiservererrors.ServerError(err) continue @@ -2646,6 +2649,9 @@ func (api *APIBase) unitResultForUnit(ctx context.Context, unit Unit) (*params.U return nil, err } unitLife, err := api.applicationService.GetUnitLife(ctx, unit.Name()) + if errors.Is(err, applicationerrors.UnitNotFound) { + err = errors.NotFoundf("unit %s", unit.Name()) + } if err != nil { return nil, err } diff --git a/apiserver/facades/client/application/application_unit_test.go b/apiserver/facades/client/application/application_unit_test.go index bcb29a4accb..72e56b1ee3f 100644 --- a/apiserver/facades/client/application/application_unit_test.go +++ b/apiserver/facades/client/application/application_unit_test.go @@ -2802,7 +2802,7 @@ func (s *ApplicationSuite) TestApplicationsInfoMany(c *gc.C) { "juju-info": "myspace", }, }) - c.Assert(result.Results[1].Error, gc.ErrorMatches, `application not found`) + c.Assert(result.Results[1].Error, gc.ErrorMatches, `application "wordpress" not found`) c.Assert(result.Results[2].Error, gc.ErrorMatches, `"unit-postgresql-0" is not a valid application tag`) } diff --git a/apiserver/facades/controller/caasapplicationprovisioner/provisioner.go b/apiserver/facades/controller/caasapplicationprovisioner/provisioner.go index d5a443a89ed..fedd8925499 100644 --- a/apiserver/facades/controller/caasapplicationprovisioner/provisioner.go +++ b/apiserver/facades/controller/caasapplicationprovisioner/provisioner.go @@ -223,8 +223,14 @@ func (a *APIGroup) Life(ctx context.Context, args params.Entities) (params.LifeR switch tag.Kind() { case names.ApplicationTagKind: lifeValue, err = a.applicationService.GetApplicationLife(ctx, tag.Id()) + if errors.Is(err, applicationerrors.ApplicationNotFound) { + err = errors.NotFoundf("application %s", tag.Id()) + } case names.UnitTagKind: lifeValue, err = a.applicationService.GetUnitLife(ctx, tag.Id()) + if errors.Is(err, applicationerrors.UnitNotFound) { + err = errors.NotFoundf("unit %s", tag.Id()) + } default: result.Results[i].Error = apiservererrors.ServerError(apiservererrors.ErrPerm) continue @@ -1569,6 +1575,9 @@ func (a *API) SetProvisioningState(ctx context.Context, args params.CAASApplicat err = a.applicationService.SetApplicationScalingState(ctx, appTag.Id(), args.ProvisioningState.ScaleTarget, args.ProvisioningState.Scaling) if err != nil { + if errors.Is(err, applicationerrors.ScalingStateInconsistent) { + err = apiservererrors.ErrTryAgain + } result.Error = apiservererrors.ServerError(err) } diff --git a/apiserver/facades/controller/caasfirewaller/firewaller.go b/apiserver/facades/controller/caasfirewaller/firewaller.go index 5c35dda5f7d..7d60a71e5ec 100644 --- a/apiserver/facades/controller/caasfirewaller/firewaller.go +++ b/apiserver/facades/controller/caasfirewaller/firewaller.go @@ -18,6 +18,7 @@ import ( "github.com/juju/juju/core/network" "github.com/juju/juju/domain/application" applicationcharm "github.com/juju/juju/domain/application/charm" + applicationerrors "github.com/juju/juju/domain/application/errors" "github.com/juju/juju/internal/charm" "github.com/juju/juju/rpc/params" "github.com/juju/juju/state/watcher" @@ -177,8 +178,14 @@ func (f *Facade) Life(ctx context.Context, args params.Entities) (params.LifeRes switch tag.Kind() { case names.ApplicationTagKind: lifeValue, err = f.applicationService.GetApplicationLife(ctx, tag.Id()) + if errors.Is(err, applicationerrors.ApplicationNotFound) { + err = errors.NotFoundf("application %s", tag.Id()) + } case names.UnitTagKind: lifeValue, err = f.applicationService.GetUnitLife(ctx, tag.Id()) + if errors.Is(err, applicationerrors.UnitNotFound) { + err = errors.NotFoundf("unit %s", tag.Id()) + } default: result.Results[i].Error = apiservererrors.ServerError(apiservererrors.ErrPerm) continue diff --git a/apiserver/facades/controller/caasunitprovisioner/provisioner.go b/apiserver/facades/controller/caasunitprovisioner/provisioner.go index c333dd9ca69..b119143a417 100644 --- a/apiserver/facades/controller/caasunitprovisioner/provisioner.go +++ b/apiserver/facades/controller/caasunitprovisioner/provisioner.go @@ -17,6 +17,7 @@ import ( corelogger "github.com/juju/juju/core/logger" "github.com/juju/juju/core/network" "github.com/juju/juju/core/watcher" + applicationerrors "github.com/juju/juju/domain/application/errors" "github.com/juju/juju/rpc/params" statewatcher "github.com/juju/juju/state/watcher" ) @@ -228,10 +229,16 @@ func (f *Facade) UpdateApplicationsService(ctx context.Context, args params.Upda appName := appTag.Id() if err := f.applicationService.UpdateCloudService(ctx, appName, appUpdate.ProviderId, sAddrs); err != nil { + if errors.Is(err, applicationerrors.ApplicationNotFound) { + err = errors.NotFoundf("application %s not found", appName) + } result.Results[i].Error = apiservererrors.ServerError(err) } if appUpdate.Scale != nil { if err := f.applicationService.SetApplicationScale(ctx, appName, *appUpdate.Scale); err != nil { + if errors.Is(err, applicationerrors.ApplicationNotFound) { + err = errors.NotFoundf("application %s not found", appName) + } result.Results[i].Error = apiservererrors.ServerError(err) } } diff --git a/apiserver/facades/controller/crossmodelrelations/crossmodelrelations.go b/apiserver/facades/controller/crossmodelrelations/crossmodelrelations.go index 8928f3022d6..77ce2758bc9 100644 --- a/apiserver/facades/controller/crossmodelrelations/crossmodelrelations.go +++ b/apiserver/facades/controller/crossmodelrelations/crossmodelrelations.go @@ -222,7 +222,6 @@ func (api *CrossModelRelationsAPIv3) registerRemoteRelation(ctx context.Context, if err != nil { return nil, errors.Annotatef(err, "cannot get application for offer %q", relation.OfferUUID) } - // TODO(units) - XXXX if localApp.Life() != state.Alive { // We don't want to leak the application name so just log it. api.logger.Warningf("local application for offer %v not found", localApplicationName) diff --git a/apiserver/facades/controller/firewaller/firewaller.go b/apiserver/facades/controller/firewaller/firewaller.go index e14ff69ef63..8b28f0a7b60 100644 --- a/apiserver/facades/controller/firewaller/firewaller.go +++ b/apiserver/facades/controller/firewaller/firewaller.go @@ -24,6 +24,7 @@ import ( "github.com/juju/juju/core/network" "github.com/juju/juju/core/status" "github.com/juju/juju/core/watcher" + applicationerrors "github.com/juju/juju/domain/application/errors" "github.com/juju/juju/environs/config" "github.com/juju/juju/rpc/params" "github.com/juju/juju/state" @@ -189,6 +190,9 @@ func (f *FirewallerAPI) Life(ctx context.Context, args params.Entities) (params. switch tag.Kind() { case names.UnitTagKind: lifeValue, err = f.applicationService.GetUnitLife(ctx, tag.Id()) + if errors.Is(err, applicationerrors.UnitNotFound) { + err = errors.NotFoundf("unit %q", tag.Id()) + } default: lifeValue, err = f.LifeGetter.OneLife(tag) } diff --git a/apiserver/facades/controller/firewaller/firewaller_base_test.go b/apiserver/facades/controller/firewaller/firewaller_base_test.go index ec97c9b01f2..718258d27a3 100644 --- a/apiserver/facades/controller/firewaller/firewaller_base_test.go +++ b/apiserver/facades/controller/firewaller/firewaller_base_test.go @@ -144,7 +144,7 @@ func (s *firewallerBaseSuite) testLife( {Life: "alive"}, {Life: "alive"}, {Error: apiservertesting.NotFoundError("machine 42")}, - {Error: ¶ms.Error{Code: "unit not found", Message: "unit not found"}}, + {Error: apiservertesting.NotFoundError(`unit "foo/0"`)}, {Error: apiservertesting.NotFoundError(`application "bar"`)}, {Error: apiservertesting.ErrUnauthorized}, {Error: apiservertesting.ErrUnauthorized}, diff --git a/apiserver/facades/controller/firewaller/interface.go b/apiserver/facades/controller/firewaller/interface.go index 118db12274d..968ed9d99ee 100644 --- a/apiserver/facades/controller/firewaller/interface.go +++ b/apiserver/facades/controller/firewaller/interface.go @@ -43,6 +43,7 @@ type NetworkService interface { WatchSubnets(ctx context.Context, subnetUUIDsToWatch set.Strings) (watcher.StringsWatcher, error) } +// ApplicationService provides access to the application service. type ApplicationService interface { GetUnitLife(context.Context, string) (life.Value, error) } diff --git a/cmd/containeragent/initialize/command.go b/cmd/containeragent/initialize/command.go index 3bd17688950..2b4f0ec18e9 100644 --- a/cmd/containeragent/initialize/command.go +++ b/cmd/containeragent/initialize/command.go @@ -27,7 +27,6 @@ import ( jujucmd "github.com/juju/juju/cmd" "github.com/juju/juju/cmd/constants" "github.com/juju/juju/cmd/containeragent/utils" - applicationerrors "github.com/juju/juju/domain/application/errors" internallogger "github.com/juju/juju/internal/logger" pebbleidentity "github.com/juju/juju/internal/service/pebble/identity" pebbleplan "github.com/juju/juju/internal/service/pebble/plan" @@ -169,7 +168,7 @@ func (c *initCommand) Run(ctx *cmd.Context) (err error) { return errors.Trace(err) }, IsFatalError: func(err error) bool { - return !errors.Is(err, applicationerrors.UnitNotAssigned) && !errors.Is(err, applicationerrors.UnitAlreadyExists) + return !errors.Is(err, errors.NotAssigned) && !errors.Is(err, errors.AlreadyExists) }, Attempts: -1, Delay: 10 * time.Second, diff --git a/cmd/containeragent/initialize/command_test.go b/cmd/containeragent/initialize/command_test.go index ef12d9e3cd2..58919ae35bd 100644 --- a/cmd/containeragent/initialize/command_test.go +++ b/cmd/containeragent/initialize/command_test.go @@ -11,6 +11,7 @@ import ( "github.com/juju/clock/testclock" "github.com/juju/cmd/v4" "github.com/juju/cmd/v4/cmdtesting" + "github.com/juju/errors" "github.com/juju/names/v5" jc "github.com/juju/testing/checkers" "go.uber.org/mock/gomock" @@ -21,7 +22,6 @@ import ( "github.com/juju/juju/cmd/containeragent/initialize" "github.com/juju/juju/cmd/containeragent/initialize/mocks" utilsmocks "github.com/juju/juju/cmd/containeragent/utils/mocks" - applicationerrors "github.com/juju/juju/domain/application/errors" coretesting "github.com/juju/juju/internal/testing" ) @@ -145,8 +145,8 @@ checks: gomock.InOrder( s.fileReaderWriter.EXPECT().Stat("/var/lib/juju/template-agent.conf").Return(nil, os.ErrNotExist), - s.applicationAPI.EXPECT().UnitIntroduction(gomock.Any(), `gitlab-0`, `gitlab-uuid`).Times(1).Return(nil, applicationerrors.UnitNotAssigned), - s.applicationAPI.EXPECT().UnitIntroduction(gomock.Any(), `gitlab-0`, `gitlab-uuid`).Times(1).Return(nil, applicationerrors.UnitAlreadyExists), + s.applicationAPI.EXPECT().UnitIntroduction(gomock.Any(), `gitlab-0`, `gitlab-uuid`).Times(1).Return(nil, errors.NotAssignedf("yo we not needed yet")), + s.applicationAPI.EXPECT().UnitIntroduction(gomock.Any(), `gitlab-0`, `gitlab-uuid`).Times(1).Return(nil, errors.AlreadyExistsf("yo we dead atm")), s.applicationAPI.EXPECT().UnitIntroduction(gomock.Any(), `gitlab-0`, `gitlab-uuid`).Times(1).Return(&caasapplication.UnitConfig{ UnitTag: names.NewUnitTag("gitlab/0"), AgentConf: data, diff --git a/cmd/containeragent/initialize/package_test.go b/cmd/containeragent/initialize/package_test.go index fa896a483ea..047a7b6ce21 100644 --- a/cmd/containeragent/initialize/package_test.go +++ b/cmd/containeragent/initialize/package_test.go @@ -77,7 +77,6 @@ func (*importSuite) TestImports(c *gc.C) { "core/user", "core/version", "core/watcher", - "domain/application/errors", "domain/secret/errors", "domain/model/errors", "domain/secretbackend/errors", diff --git a/domain/application/errors/errors.go b/domain/application/errors/errors.go index 189605f78ff..75f67fd4cf1 100644 --- a/domain/application/errors/errors.go +++ b/domain/application/errors/errors.go @@ -16,9 +16,6 @@ const ( // application being created already exists. ApplicationAlreadyExists = errors.ConstError("application already exists") - // ApplicationIsDead describes an error that occurs when an application's life is Dead. - ApplicationIsDead = errors.ConstError("application is dead") - // ApplicationHasUnits describes an error that occurs when the application // being deleted still has associated units. ApplicationHasUnits = errors.ConstError("application has units") diff --git a/domain/application/service/application.go b/domain/application/service/application.go index 732327f9429..78a69132ed3 100644 --- a/domain/application/service/application.go +++ b/domain/application/service/application.go @@ -385,6 +385,9 @@ func (s *ApplicationService) deleteUnit(ctx domain.AtomicContext, unitName strin } err = s.ensureUnitDead(ctx, unitName) + if errors.Is(err, applicationerrors.UnitNotFound) { + return cleanups, nil + } if err != nil { return nil, errors.Trace(err) } @@ -423,6 +426,9 @@ func (s *ApplicationService) EnsureUnitDead(ctx context.Context, unitName string err := s.st.RunAtomic(ctx, func(ctx domain.AtomicContext) error { return s.ensureUnitDead(ctx, unitName) }) + if errors.Is(err, applicationerrors.UnitNotFound) { + return nil + } if err == nil { appName, _ := names.UnitApplication(unitName) if err := leadershipRevoker.RevokeLeadership(appName, unitName); err != nil && !errors.Is(err, leadership.ErrClaimNotHeld) { @@ -612,7 +618,7 @@ func (s *ApplicationService) DestroyApplication(ctx context.Context, appName str // For now, all we do is advance the application's life to Dying. err := s.st.RunAtomic(ctx, func(ctx domain.AtomicContext) error { appID, err := s.st.GetApplicationID(ctx, appName) - if errors.Is(err, applicationerrors.ApplicationIsDead) { + if errors.Is(err, applicationerrors.ApplicationNotFound) { return nil } if err != nil { @@ -629,7 +635,7 @@ func (s *ApplicationService) DestroyApplication(ctx context.Context, appName str func (s *ApplicationService) EnsureApplicationDead(ctx context.Context, appName string) error { err := s.st.RunAtomic(ctx, func(ctx domain.AtomicContext) error { appID, err := s.st.GetApplicationID(ctx, appName) - if errors.Is(err, applicationerrors.ApplicationIsDead) { + if errors.Is(err, applicationerrors.ApplicationNotFound) { return nil } if err != nil { diff --git a/domain/application/service_test.go b/domain/application/service_test.go index ec9e23c37ed..ab46c7adf1b 100644 --- a/domain/application/service_test.go +++ b/domain/application/service_test.go @@ -205,7 +205,7 @@ func (s *serviceSuite) TestEnsureApplicationDeadNotFound(c *gc.C) { defer ctrl.Finish() err := s.svc.EnsureApplicationDead(context.Background(), "foo") - c.Assert(err, jc.ErrorIs, applicationerrors.ApplicationNotFound) + c.Assert(err, jc.ErrorIsNil) } func (s *serviceSuite) TestGetUnitLife(c *gc.C) { @@ -281,7 +281,7 @@ func (s *serviceSuite) TestEnsureUnitDeadNotFound(c *gc.C) { revoker := application.NewMockRevoker(ctrl) err := s.svc.EnsureUnitDead(context.Background(), "foo/666", revoker) - c.Assert(err, jc.ErrorIs, applicationerrors.UnitNotFound) + c.Assert(err, jc.ErrorIsNil) } func (s *serviceSuite) TestDeleteUnit(c *gc.C) { @@ -333,7 +333,7 @@ func (s *serviceSuite) TestDeleteUnitNotFound(c *gc.C) { s.createApplication(c, "foo") err := s.svc.DeleteUnit(context.Background(), "foo/666") - c.Assert(err, jc.ErrorIs, applicationerrors.UnitNotFound) + c.Assert(err, jc.ErrorIsNil) } func (s *serviceSuite) TestRemoveUnit(c *gc.C) { diff --git a/domain/application/state/application.go b/domain/application/state/application.go index cc2c65e2367..a8263dd79aa 100644 --- a/domain/application/state/application.go +++ b/domain/application/state/application.go @@ -244,11 +244,11 @@ WHERE name = $applicationName.name return applicationerrors.ApplicationAlreadyExists } -func (st *ApplicationState) lookupApplication(ctx context.Context, tx *sqlair.TX, name string, deadOk bool) (coreapplication.ID, error) { +func (st *ApplicationState) lookupApplication(ctx context.Context, tx *sqlair.TX, name string) (coreapplication.ID, error) { var appID applicationID appName := applicationName{Name: name} queryApplication := ` -SELECT (uuid, life_id) AS (&applicationID.*) +SELECT (uuid) AS (&applicationID.*) FROM application WHERE name = $applicationName.name ` @@ -263,9 +263,6 @@ WHERE name = $applicationName.name } return "", fmt.Errorf("%w: %s", applicationerrors.ApplicationNotFound, name) } - if !deadOk && appID.LifeID == life.Dead { - return "", fmt.Errorf("%w: %s", applicationerrors.ApplicationIsDead, name) - } return coreapplication.ID(appID.ID), nil } @@ -295,7 +292,7 @@ func (st *ApplicationState) deleteApplication(ctx context.Context, tx *sqlair.TX return errors.Trace(err) } - appUUID, err := st.lookupApplication(ctx, tx, name, true) + appUUID, err := st.lookupApplication(ctx, tx, name) if err != nil { return errors.Trace(err) } @@ -364,7 +361,7 @@ func (st *ApplicationState) AddUnits(ctx context.Context, applicationName string return errors.Trace(err) } err = db.Txn(ctx, func(ctx context.Context, tx *sqlair.TX) error { - appID, err := st.lookupApplication(ctx, tx, applicationName, false) + appID, err := st.lookupApplication(ctx, tx, applicationName) if err != nil { return errors.Trace(err) } @@ -777,7 +774,7 @@ func (st *ApplicationState) GetApplicationID(ctx domain.AtomicContext, name stri var appID coreapplication.ID err := domain.Run(ctx, func(ctx context.Context, tx *sqlair.TX) error { var err error - appID, err = st.lookupApplication(ctx, tx, name, false) + appID, err = st.lookupApplication(ctx, tx, name) if err != nil { return fmt.Errorf("looking up application %q: %w", name, err) } @@ -1023,7 +1020,7 @@ ON CONFLICT(application_uuid) DO UPDATE } err = db.Txn(ctx, func(ctx context.Context, tx *sqlair.TX) error { - appID, err := st.lookupApplication(ctx, tx, name, false) + appID, err := st.lookupApplication(ctx, tx, name) if err != nil { return errors.Trace(err) } @@ -1142,7 +1139,7 @@ WHERE uuid = $applicationID.uuid appPlatform application.Platform ) if err := db.Txn(ctx, func(ctx context.Context, tx *sqlair.TX) error { - appID, err := st.lookupApplication(ctx, tx, name, false) + appID, err := st.lookupApplication(ctx, tx, name) if err != nil { return fmt.Errorf("looking up application %q: %w", name, err) } diff --git a/domain/application/state/application_test.go b/domain/application/state/application_test.go index 68454278840..433d5ab1928 100644 --- a/domain/application/state/application_test.go +++ b/domain/application/state/application_test.go @@ -781,16 +781,6 @@ func (s *applicationStateSuite) TestAddUnits(c *gc.C) { c.Check(unitID, gc.Equals, "foo/666") } -func (s *applicationStateSuite) TestAddUnitsDead(c *gc.C) { - s.createApplication(c, "foo", life.Dead) - - u := application.UpsertUnitArg{ - UnitName: ptr("foo/666"), - } - err := s.state.AddUnits(context.Background(), "foo", u) - c.Assert(err, jc.ErrorIs, applicationerrors.ApplicationIsDead) -} - func (s *applicationStateSuite) TestAddUnitsMissingApplication(c *gc.C) { u := application.UpsertUnitArg{ UnitName: ptr("foo/666"), diff --git a/internal/migration/precheck.go b/internal/migration/precheck.go index 56c7bf7a346..b4dd7881ad6 100644 --- a/internal/migration/precheck.go +++ b/internal/migration/precheck.go @@ -16,6 +16,7 @@ import ( "github.com/juju/juju/core/life" coremigration "github.com/juju/juju/core/migration" "github.com/juju/juju/core/status" + applicationerrors "github.com/juju/juju/domain/application/errors" "github.com/juju/juju/environs/config" "github.com/juju/juju/internal/tools" "github.com/juju/juju/internal/upgrades/upgradevalidation" @@ -256,6 +257,9 @@ func (c *precheckContext) checkApplications(ctx context.Context) (map[string][]P for _, app := range apps { appLife, err := c.applicationService.GetApplicationLife(ctx, app.Name()) if err != nil { + if errors.Is(err, applicationerrors.ApplicationNotFound) { + err = errors.NotFoundf("application %s", app.Name()) + } return nil, errors.Annotatef(err, "retrieving life for %q", app.Name()) } if appLife != life.Alive { @@ -285,7 +289,6 @@ func (c *precheckContext) checkUnits(ctx context.Context, app PrecheckApplicatio } for _, unit := range units { - // if unit.Life() != state.Alive { return errors.Errorf("unit %s is %s", unit.Name(), unit.Life()) } diff --git a/internal/worker/caasapplicationprovisioner/application.go b/internal/worker/caasapplicationprovisioner/application.go index 7d905b419a1..244f078f04d 100644 --- a/internal/worker/caasapplicationprovisioner/application.go +++ b/internal/worker/caasapplicationprovisioner/application.go @@ -19,7 +19,6 @@ import ( "github.com/juju/juju/core/logger" "github.com/juju/juju/core/status" "github.com/juju/juju/core/watcher" - applicationerrors "github.com/juju/juju/domain/application/errors" "github.com/juju/juju/internal/password" "github.com/juju/juju/rpc/params" ) @@ -116,7 +115,7 @@ func (a *appWorker) loop() error { // If the application no longer exists, return immediately. If it's in // Dead state, ensure it's deleted and terminated. appLife, err := a.facade.Life(ctx, a.name) - if errors.Is(err, applicationerrors.ApplicationNotFound) { + if errors.Is(err, errors.NotFound) { a.logger.Debugf("application %q no longer exists", a.name) return nil } else if err != nil { @@ -203,7 +202,7 @@ func (a *appWorker) loop() error { handleChange := func() error { appLife, err = a.facade.Life(ctx, a.name) - if errors.Is(err, applicationerrors.ApplicationNotFound) { + if errors.Is(err, errors.NotFound) { appLife = life.Dead } else if err != nil { return errors.Trace(err) @@ -371,7 +370,7 @@ func (a *appWorker) loop() error { break } err := a.ops.ReconcileDeadUnitScale(ctx, a.name, app, a.facade, a.logger) - if errors.Is(err, applicationerrors.ApplicationNotFound) || errors.Is(err, applicationerrors.UnitNotFound) { + if errors.Is(err, errors.NotFound) { reconcileDeadChan = a.clock.After(retryDelay) shouldRefresh = false } else if errors.Is(err, tryAgain) { diff --git a/internal/worker/caasapplicationprovisioner/application_test.go b/internal/worker/caasapplicationprovisioner/application_test.go index c4a66f72b7b..c46875f0ead 100644 --- a/internal/worker/caasapplicationprovisioner/application_test.go +++ b/internal/worker/caasapplicationprovisioner/application_test.go @@ -24,7 +24,6 @@ import ( "github.com/juju/juju/core/status" "github.com/juju/juju/core/watcher" "github.com/juju/juju/core/watcher/watchertest" - applicationerrors "github.com/juju/juju/domain/application/errors" loggertesting "github.com/juju/juju/internal/logger/testing" coretesting "github.com/juju/juju/internal/testing" "github.com/juju/juju/internal/worker/caasapplicationprovisioner" @@ -98,7 +97,7 @@ func (s *ApplicationWorkerSuite) TestLifeNotFound(c *gc.C) { broker.EXPECT().Application("test", caas.DeploymentStateful).Return(brokerApp), facade.EXPECT().Life(gomock.Any(), "test").DoAndReturn(func(ctx context.Context, appName string) (life.Value, error) { close(done) - return "", applicationerrors.ApplicationNotFound + return "", errors.NotFoundf("test charm") }), ) appWorker := s.startAppWorker(c, nil, facade, broker, nil, ops, false) @@ -199,7 +198,7 @@ func (s *ApplicationWorkerSuite) TestWorker(c *gc.C) { }), // appUnitsChan fired - ops.EXPECT().ReconcileDeadUnitScale(gomock.Any(), "test", app, facade, s.logger).Return(applicationerrors.ApplicationNotFound), + ops.EXPECT().ReconcileDeadUnitScale(gomock.Any(), "test", app, facade, s.logger).Return(errors.NotFound), ops.EXPECT().ReconcileDeadUnitScale(gomock.Any(), "test", app, facade, s.logger).Return(errors.ConstError("try again")), ops.EXPECT().ReconcileDeadUnitScale(gomock.Any(), "test", app, facade, s.logger).DoAndReturn(func(_ context.Context, _ string, _ caas.Application, _ caasapplicationprovisioner.CAASProvisionerFacade, _ logger.Logger) error { appChan <- struct{}{} diff --git a/internal/worker/caasapplicationprovisioner/ops.go b/internal/worker/caasapplicationprovisioner/ops.go index 3abcb9004be..609c7fdfbc5 100644 --- a/internal/worker/caasapplicationprovisioner/ops.go +++ b/internal/worker/caasapplicationprovisioner/ops.go @@ -19,7 +19,6 @@ import ( "github.com/juju/juju/core/life" "github.com/juju/juju/core/logger" "github.com/juju/juju/core/status" - applicationerrors "github.com/juju/juju/domain/application/errors" "github.com/juju/juju/internal/charm" "github.com/juju/juju/internal/cloudconfig/podcfg" "github.com/juju/juju/rpc/params" @@ -279,15 +278,6 @@ func appDying( facade CAASProvisionerFacade, unitFacade CAASUnitProvisionerFacade, logger logger.Logger, ) (err error) { - // TODO(units) - remove this once life mgmt fully across to dqlite - // Since life is still handled in both mongo and dqlite, it's possible - // that a mongo cleanup job can mark the app as dead. - defer func() { - if errors.Is(err, applicationerrors.ApplicationIsDead) { - err = nil - } - }() - logger.Debugf("application %q dying", appName) err = ensureScale(ctx, appName, app, appLife, facade, unitFacade, logger) if err != nil { @@ -337,8 +327,7 @@ func checkCharmFormat( logger logger.Logger, ) (isOk bool, err error) { charmInfo, err := facade.ApplicationCharmInfo(ctx, appName) - // TODO(units) - remove the dead check once life mgmt fully across to dqlite - if errors.Is(err, errors.NotFound) || errors.Is(err, applicationerrors.ApplicationIsDead) { + if errors.Is(err, errors.NotFound) { logger.Debugf("application %q no longer exists", appName) return false, nil } else if err != nil { @@ -401,7 +390,7 @@ func updateState( ProviderId: svc.Id, Addresses: params.FromProviderAddresses(svc.Addresses...), }) - if errors.Is(err, applicationerrors.ApplicationNotFound) { + if errors.Is(err, errors.NotFound) { // Do nothing } else if err != nil { return nil, errors.Trace(err) @@ -637,7 +626,7 @@ func reconcileDeadUnitScale( for _, deadUnit := range deadUnits { logger.Infof("removing dead unit %s", deadUnit.Tag.Id()) - if err := facade.RemoveUnit(ctx, deadUnit.Tag.Id()); err != nil && !errors.Is(err, applicationerrors.UnitNotFound) { + if err := facade.RemoveUnit(ctx, deadUnit.Tag.Id()); err != nil && !errors.Is(err, errors.NotFound) { return fmt.Errorf("removing dead unit %q: %w", deadUnit.Tag.Id(), err) } } @@ -741,7 +730,7 @@ func updateProvisioningState( ScaleTarget: scaleTarget, } err := facade.SetProvisioningState(ctx, appName, newPs) - if errors.Is(err, applicationerrors.ScalingStateInconsistent) { + if params.IsCodeTryAgain(err) { return tryAgain } else if err != nil { return errors.Annotatef(err, "setting provisiong state for application %q", appName) diff --git a/internal/worker/caasapplicationprovisioner/worker.go b/internal/worker/caasapplicationprovisioner/worker.go index 3fee37811c6..a41bba57952 100644 --- a/internal/worker/caasapplicationprovisioner/worker.go +++ b/internal/worker/caasapplicationprovisioner/worker.go @@ -30,7 +30,6 @@ import ( "github.com/juju/juju/core/resources" "github.com/juju/juju/core/status" "github.com/juju/juju/core/watcher" - applicationerrors "github.com/juju/juju/domain/application/errors" "github.com/juju/juju/rpc/params" ) @@ -181,10 +180,10 @@ func (p *provisioner) loop() error { } for _, appName := range apps { _, err := p.facade.Life(ctx, appName) - if err != nil && !errors.Is(err, applicationerrors.ApplicationNotFound) { + if err != nil && !errors.Is(err, errors.NotFound) { return errors.Trace(err) } - if errors.Is(err, applicationerrors.ApplicationNotFound) || errors.Is(err, applicationerrors.ApplicationIsDead) { + if errors.Is(err, errors.NotFound) { p.logger.Debugf("application %q not found, ignoring", appName) continue } diff --git a/internal/worker/caasfirewaller/applicationworker.go b/internal/worker/caasfirewaller/applicationworker.go index 7d679555e2f..b7a818537d3 100644 --- a/internal/worker/caasfirewaller/applicationworker.go +++ b/internal/worker/caasfirewaller/applicationworker.go @@ -15,7 +15,6 @@ import ( "github.com/juju/juju/core/logger" "github.com/juju/juju/core/network" "github.com/juju/juju/core/watcher" - applicationerrors "github.com/juju/juju/domain/application/errors" ) type applicationWorker struct { @@ -113,8 +112,7 @@ func (w *applicationWorker) setUp(ctx context.Context) (err error) { func (w *applicationWorker) loop() (err error) { defer func() { // If the application has been deleted, we can return nil. - // TODO(units): remove generic not found error when everything is in dqlite - if errors.Is(err, errors.NotFound) || errors.Is(err, applicationerrors.ApplicationNotFound) { + if errors.Is(err, errors.NotFound) { w.logger.Debugf("sidecar caas firewaller application %v has been removed", w.appName) err = nil } diff --git a/internal/worker/caasfirewaller/worker.go b/internal/worker/caasfirewaller/worker.go index 601e309d95b..6460cad406e 100644 --- a/internal/worker/caasfirewaller/worker.go +++ b/internal/worker/caasfirewaller/worker.go @@ -12,7 +12,6 @@ import ( "github.com/juju/juju/core/life" "github.com/juju/juju/core/logger" - applicationerrors "github.com/juju/juju/domain/application/errors" "github.com/juju/juju/internal/charm" ) @@ -122,8 +121,7 @@ func (p *firewaller) loop() error { for _, appName := range apps { // If charm is a v1 charm, skip processing. format, err := p.charmFormat(ctx, appName) - // TODO(units) - remove the dead check once life mgmt fully across to dqlite - if errors.Is(err, errors.NotFound) || errors.Is(err, applicationerrors.ApplicationIsDead) { + if errors.Is(err, errors.NotFound) { p.config.Logger.Debugf("application %q no longer exists", appName) continue } else if err != nil { @@ -135,7 +133,7 @@ func (p *firewaller) loop() error { } appLife, err := p.config.LifeGetter.Life(ctx, appName) - if errors.Is(err, applicationerrors.ApplicationNotFound) || appLife == life.Dead { + if errors.Is(err, errors.NotFound) || appLife == life.Dead { w, ok := p.appWorkers[appName] if ok { if err := worker.Stop(w); err != nil { diff --git a/internal/worker/caasfirewaller/worker_test.go b/internal/worker/caasfirewaller/worker_test.go index 8af61aaa25e..2bae97cd710 100644 --- a/internal/worker/caasfirewaller/worker_test.go +++ b/internal/worker/caasfirewaller/worker_test.go @@ -16,7 +16,6 @@ import ( "github.com/juju/juju/core/logger" "github.com/juju/juju/core/watcher" "github.com/juju/juju/core/watcher/watchertest" - applicationerrors "github.com/juju/juju/domain/application/errors" "github.com/juju/juju/internal/charm" loggertesting "github.com/juju/juju/internal/logger/testing" "github.com/juju/juju/internal/testing" @@ -158,7 +157,7 @@ func (s *workerSuite) TestStartStop(c *gc.C) { app2Worker.EXPECT().Wait().Return(nil) s.firewallerAPI.EXPECT().ApplicationCharmInfo(gomock.Any(), "app1").Return(charmInfo, nil) - s.lifeGetter.EXPECT().Life(gomock.Any(), "app1").Return(life.Value(""), applicationerrors.ApplicationNotFound) + s.lifeGetter.EXPECT().Life(gomock.Any(), "app1").Return(life.Value(""), errors.NotFoundf("%q", "app1")) // Stopped app1's worker because it's removed. app1Worker.EXPECT().Kill() app1Worker.EXPECT().Wait().Return(nil) diff --git a/internal/worker/deployer/deployer.go b/internal/worker/deployer/deployer.go index b448d52914e..3dbe1f8552a 100644 --- a/internal/worker/deployer/deployer.go +++ b/internal/worker/deployer/deployer.go @@ -157,7 +157,7 @@ func (d *Deployer) changed(ctx context.Context, unitName string) error { d.logger.Infof("checking unit %q", unitName) var unitLife life.Value unit, err := d.client.Unit(ctx, unitTag) - if params.IsCodeUnauthorized(err) || params.IsCodeUnitNotFound(err) { + if params.IsCodeNotFoundOrCodeUnauthorized(err) { unitLife = life.Dead } else if err != nil { return err diff --git a/internal/worker/firewaller/firewaller.go b/internal/worker/firewaller/firewaller.go index e8e5e0748fe..7fa3f90fba4 100644 --- a/internal/worker/firewaller/firewaller.go +++ b/internal/worker/firewaller/firewaller.go @@ -695,7 +695,7 @@ func (fw *Firewaller) unitsChanged(ctx context.Context, change *unitsChange) err for _, name := range change.units { unitTag := names.NewUnitTag(name) unit, err := fw.firewallerApi.Unit(ctx, unitTag) - if err != nil && !params.IsCodeUnitNotFound(err) { + if err != nil && !params.IsCodeNotFound(err) { return err } var machineTag names.MachineTag diff --git a/internal/worker/uniter/relation/statemanager.go b/internal/worker/uniter/relation/statemanager.go index efae8627083..4aff4a2abe4 100644 --- a/internal/worker/uniter/relation/statemanager.go +++ b/internal/worker/uniter/relation/statemanager.go @@ -61,10 +61,7 @@ func (m *stateManager) RemoveRelation(ctx context.Context, id int, unitGetter Un unitExists, ok := knownUnits[unitName] if !ok { _, err := unitGetter.Unit(ctx, names.NewUnitTag(unitName)) - ignoreError := params.IsCodeUnauthorized(err) || params.IsCodeUnitNotFound(err) - // TODO(units): when all apis are on dqlite, remove this. - ignoreError = ignoreError || params.IsCodeNotFound(err) - if err != nil && !ignoreError { + if err != nil && !params.IsCodeNotFoundOrCodeUnauthorized(err) { return errors.Trace(err) } unitExists = err == nil diff --git a/internal/worker/uniter/relation/statemanager_test.go b/internal/worker/uniter/relation/statemanager_test.go index b5bcc1623e9..0574705752d 100644 --- a/internal/worker/uniter/relation/statemanager_test.go +++ b/internal/worker/uniter/relation/statemanager_test.go @@ -191,7 +191,7 @@ func (s *stateManagerSuite) TestRemoveIgnoresMissingUnits(c *gc.C) { stateTwo.Members = map[string]int64{"foo/1": 0} s.expectState(c, stateTwo) s.expectSetStateEmpty(c) - s.mockUnitGetter.EXPECT().Unit(gomock.Any(), names.NewUnitTag("foo/1")).Return(nil, ¶ms.Error{Code: "unit not found"}) + s.mockUnitGetter.EXPECT().Unit(gomock.Any(), names.NewUnitTag("foo/1")).Return(nil, ¶ms.Error{Code: "not found"}) logger := logger.GetLogger("test") var tw loggo.TestWriter @@ -215,7 +215,7 @@ func (s *stateManagerSuite) TestRemoveCachesUnits(c *gc.C) { stateThree.Members = map[string]int64{"foo/1": 0} s.expectState(c, stateTwo, stateThree) s.expectSetState(c, stateThree) - s.mockUnitGetter.EXPECT().Unit(gomock.Any(), names.NewUnitTag("foo/1")).Return(nil, ¶ms.Error{Code: "unit not found"}) + s.mockUnitGetter.EXPECT().Unit(gomock.Any(), names.NewUnitTag("foo/1")).Return(nil, ¶ms.Error{Code: "not found"}) mgr, err := relation.NewStateManager(context.Background(), s.mockUnitRW, loggertesting.WrapCheckLog(c)) c.Assert(err, jc.ErrorIsNil) diff --git a/rpc/params/apierror.go b/rpc/params/apierror.go index 2f8a93b0f97..1ba9bf01c37 100644 --- a/rpc/params/apierror.go +++ b/rpc/params/apierror.go @@ -13,7 +13,6 @@ import ( "github.com/juju/errors" "gopkg.in/macaroon.v2" - applicationerrors "github.com/juju/juju/domain/application/errors" modelerrors "github.com/juju/juju/domain/model/errors" secreterrors "github.com/juju/juju/domain/secret/errors" secretbackenderrors "github.com/juju/juju/domain/secretbackend/errors" @@ -215,12 +214,6 @@ const ( CodeSecretBackendNotValid = "secret backend not valid" CodeAccessRequired = "access required" CodeAppShouldNotHaveUnits = "application should not have units" - CodeApplicationNotFound = "application not found" - CodeApplicationAlreadyExists = "application already exists" - CodeApplicationIsDead = "application is dead" - CodeScalingStateInconsistent = "scaling state inconsistent" - CodeUnitNotFound = "unit not found" - CodeUnitAlreadyExists = "unit already exists" // // Tag based error @@ -307,18 +300,6 @@ func TranslateWellKnownError(err error) error { return fmt.Errorf("%s%w", err.Error(), errors.Hide(secreterrors.SecretConsumerNotFound)) case CodeSecretBackendNotFound: return fmt.Errorf("%s%w", err.Error(), errors.Hide(secretbackenderrors.NotFound)) - case CodeApplicationNotFound: - return fmt.Errorf("%s%w", err.Error(), errors.Hide(applicationerrors.ApplicationNotFound)) - case CodeApplicationAlreadyExists: - return fmt.Errorf("%s%w", err.Error(), errors.Hide(applicationerrors.ApplicationAlreadyExists)) - case CodeApplicationIsDead: - return fmt.Errorf("%s%w", err.Error(), errors.Hide(applicationerrors.ApplicationIsDead)) - case CodeScalingStateInconsistent: - return fmt.Errorf("%s%w", err.Error(), errors.Hide(applicationerrors.ScalingStateInconsistent)) - case CodeUnitAlreadyExists: - return fmt.Errorf("%s%w", err.Error(), errors.Hide(applicationerrors.UnitAlreadyExists)) - case CodeUnitNotFound: - return fmt.Errorf("%s%w", err.Error(), errors.Hide(applicationerrors.UnitNotFound)) case CodeUnauthorized: return errors.NewUnauthorized(err, "") case CodeNotImplemented: @@ -404,38 +385,10 @@ func IsCodeSecretBackendNotFound(err error) bool { return ErrCode(err) == CodeSecretBackendNotFound } -func IsCodeSecretBackendAlreadyExists(err error) bool { - return ErrCode(err) == CodeSecretBackendAlreadyExists -} - -func IsCodeSecretBackendNotValid(err error) bool { - return ErrCode(err) == CodeSecretBackendAlreadyExists -} - func IsCodeSecretBackendForbidden(err error) bool { return ErrCode(err) == CodeSecretBackendForbidden } -func IsCodeSecretBackendNotSupported(err error) bool { - return ErrCode(err) == CodeSecretBackendNotSupported -} - -func IsCodeApplicationNotFound(err error) bool { - return ErrCode(err) == CodeApplicationNotFound -} - -func IsCodeScalingStateInconsistent(err error) bool { - return ErrCode(err) == CodeScalingStateInconsistent -} - -func IsCodeUnitAlreadyExists(err error) bool { - return ErrCode(err) == CodeUnitAlreadyExists -} - -func IsCodeUnitNotFound(err error) bool { - return ErrCode(err) == CodeUnitNotFound -} - func IsCodeUnauthorized(err error) bool { return ErrCode(err) == CodeUnauthorized } diff --git a/rpc/params/apierror_test.go b/rpc/params/apierror_test.go index c6c8032c56f..331b30e2ada 100644 --- a/rpc/params/apierror_test.go +++ b/rpc/params/apierror_test.go @@ -8,7 +8,6 @@ import ( jc "github.com/juju/testing/checkers" gc "gopkg.in/check.v1" - applicationerrors "github.com/juju/juju/domain/application/errors" modelerrors "github.com/juju/juju/domain/model/errors" secreterrors "github.com/juju/juju/domain/secret/errors" secretbackenderrors "github.com/juju/juju/domain/secretbackend/errors" @@ -60,10 +59,6 @@ func (*errorSuite) TestTranslateWellKnownError(c *gc.C) { {params.CodeSecretBackendNotSupported, params.Error{Code: params.CodeSecretBackendNotSupported, Message: "secret backend not found"}, secretbackenderrors.NotSupported}, {params.CodeSecretBackendNotValid, params.Error{Code: params.CodeSecretBackendNotValid, Message: "secret backend not found"}, secretbackenderrors.NotValid}, {params.CodeSecretBackendForbidden, params.Error{Code: params.CodeSecretBackendForbidden, Message: "secret backend not found"}, secretbackenderrors.Forbidden}, - {params.CodeUnitNotFound, params.Error{Code: params.CodeUnitNotFound, Message: "unit not found"}, applicationerrors.UnitNotFound}, - {params.CodeUnitAlreadyExists, params.Error{Code: params.CodeUnitAlreadyExists, Message: "unit already exists"}, applicationerrors.UnitAlreadyExists}, - {params.CodeApplicationNotFound, params.Error{Code: params.CodeApplicationNotFound, Message: "application not found"}, applicationerrors.ApplicationNotFound}, - {params.CodeScalingStateInconsistent, params.Error{Code: params.CodeScalingStateInconsistent, Message: "scaling state inconsistent"}, applicationerrors.ScalingStateInconsistent}, } for _, v := range tests { From 642d9e720b39828856ff5a14e5642d55fb5e7ff2 Mon Sep 17 00:00:00 2001 From: wallyworld Date: Fri, 13 Sep 2024 15:53:18 +1000 Subject: [PATCH 3/3] chore: return empty life for invalid db id --- apiserver/facades/agent/uniter/uniter.go | 2 +- .../controller/caasapplicationprovisioner/provisioner.go | 1 - apiserver/facades/controller/caasfirewaller/firewaller.go | 2 +- apiserver/facades/schema.json | 4 ++-- domain/life/life.go | 2 +- 5 files changed, 5 insertions(+), 6 deletions(-) diff --git a/apiserver/facades/agent/uniter/uniter.go b/apiserver/facades/agent/uniter/uniter.go index 2faee21b8e1..e99478e61e3 100644 --- a/apiserver/facades/agent/uniter/uniter.go +++ b/apiserver/facades/agent/uniter/uniter.go @@ -1078,7 +1078,7 @@ func (u *UniterAPI) RelationsStatus(ctx context.Context, args params.Entities) ( return result, nil } -// Life returns the life status of the specified units. +// Life returns the life status of the specified applications or units. func (u *UniterAPI) Life(ctx context.Context, args params.Entities) (params.LifeResults, error) { result := params.LifeResults{ Results: make([]params.LifeResult, len(args.Entities)), diff --git a/apiserver/facades/controller/caasapplicationprovisioner/provisioner.go b/apiserver/facades/controller/caasapplicationprovisioner/provisioner.go index fedd8925499..22e7b31525f 100644 --- a/apiserver/facades/controller/caasapplicationprovisioner/provisioner.go +++ b/apiserver/facades/controller/caasapplicationprovisioner/provisioner.go @@ -918,7 +918,6 @@ func (a *API) UpdateApplicationsUnits(ctx context.Context, args params.UpdateApp result.Results[i].Error = apiservererrors.ServerError(err) continue } - // app, err := a.state.Application(appTag.Id()) if err != nil { result.Results[i].Error = apiservererrors.ServerError(err) diff --git a/apiserver/facades/controller/caasfirewaller/firewaller.go b/apiserver/facades/controller/caasfirewaller/firewaller.go index 7d60a71e5ec..20c0b6224c4 100644 --- a/apiserver/facades/controller/caasfirewaller/firewaller.go +++ b/apiserver/facades/controller/caasfirewaller/firewaller.go @@ -152,7 +152,7 @@ func NewFacade( }, nil } -// Life returns the life status of the specified units. +// Life returns the life status of the specified applications or units. func (f *Facade) Life(ctx context.Context, args params.Entities) (params.LifeResults, error) { result := params.LifeResults{ Results: make([]params.LifeResult, len(args.Entities)), diff --git a/apiserver/facades/schema.json b/apiserver/facades/schema.json index 4715183b2c4..196b7c28d39 100644 --- a/apiserver/facades/schema.json +++ b/apiserver/facades/schema.json @@ -8204,7 +8204,7 @@ "$ref": "#/definitions/LifeResults" } }, - "description": "Life returns the life status of the specified units." + "description": "Life returns the life status of the specified applications or units." }, "Watch": { "type": "object", @@ -39658,7 +39658,7 @@ "$ref": "#/definitions/LifeResults" } }, - "description": "Life returns the life status of the specified units." + "description": "Life returns the life status of the specified applications or units." }, "LogActionsMessages": { "type": "object", diff --git a/domain/life/life.go b/domain/life/life.go index c22922e6eed..079b0f32d07 100644 --- a/domain/life/life.go +++ b/domain/life/life.go @@ -26,5 +26,5 @@ func (l Life) Value() corelife.Value { case Dead: return corelife.Dead } - return corelife.Alive + return "" }