From 4bc414d6bb65bde54b002a7d29c79c6287a8ca1b Mon Sep 17 00:00:00 2001 From: Simon Richardson Date: Tue, 2 Apr 2024 15:23:34 +0100 Subject: [PATCH] Remove caas broker from agent model manifolds We no longer require the caas broker for the agent model manifolds. It's still required via the model operator, but that's fine as it can use the api conn. --- cmd/jujud-controller/agent/model/manifolds.go | 28 ++++--- .../agent/model/manifolds_test.go | 81 ++++++++++++------- internal/worker/caasrbacmapper/manifold.go | 3 +- .../worker/providertracker/worker_test.go | 16 +--- 4 files changed, 74 insertions(+), 54 deletions(-) diff --git a/cmd/jujud-controller/agent/model/manifolds.go b/cmd/jujud-controller/agent/model/manifolds.go index 3771398a5c0..6bfdfb51937 100644 --- a/cmd/jujud-controller/agent/model/manifolds.go +++ b/cmd/jujud-controller/agent/model/manifolds.go @@ -30,7 +30,6 @@ import ( "github.com/juju/juju/internal/worker/apiconfigwatcher" "github.com/juju/juju/internal/worker/applicationscaler" "github.com/juju/juju/internal/worker/caasapplicationprovisioner" - "github.com/juju/juju/internal/worker/caasbroker" "github.com/juju/juju/internal/worker/caasenvironupgrader" "github.com/juju/juju/internal/worker/caasfirewaller" "github.com/juju/juju/internal/worker/caasmodelconfigmanager" @@ -491,6 +490,16 @@ func CAASManifolds(config ManifoldsConfig) dependency.Manifolds { agentConfig := config.Agent.CurrentConfig() modelTag := agentConfig.Model() manifolds := dependency.Manifolds{ + providerTrackerName: ifResponsible(providertracker.Manifold(providertracker.ManifoldConfig[caas.Broker]{ + ProviderServiceFactoryName: providerServiceFactoryName, + NewWorker: providertracker.NewWorker[caas.Broker], + GetProviderServiceFactory: providertracker.GetProviderServiceFactory, + Logger: config.LoggingContext.GetLogger("juju.worker.providertracker"), + GetProvider: providertracker.CAASGetProvider(func(ctx context.Context, args environs.OpenParams) (caas.Broker, error) { + return config.NewContainerBrokerFunc(ctx, args) + }), + })), + // The undertaker is currently the only ifNotAlive worker. undertakerName: ifNotAlive(undertaker.Manifold(undertaker.ManifoldConfig{ APICallerName: apiCallerName, @@ -504,16 +513,10 @@ func CAASManifolds(config ManifoldsConfig) dependency.Manifolds { }, })), - caasBrokerTrackerName: ifResponsible(caasbroker.Manifold(caasbroker.ManifoldConfig{ - APICallerName: apiCallerName, - NewContainerBrokerFunc: config.NewContainerBrokerFunc, - Logger: config.LoggingContext.GetLogger("juju.worker.caas"), - })), - caasFirewallerName: ifNotMigrating(caasfirewaller.Manifold( caasfirewaller.ManifoldConfig{ APICallerName: apiCallerName, - BrokerName: caasBrokerTrackerName, + BrokerName: providerTrackerName, ControllerUUID: agentConfig.Controller().Id(), ModelUUID: agentConfig.Model().Id(), NewClient: func(caller base.APICaller) caasfirewaller.Client { @@ -527,14 +530,14 @@ func CAASManifolds(config ManifoldsConfig) dependency.Manifolds { caasModelOperatorName: ifResponsible(caasmodeloperator.Manifold(caasmodeloperator.ManifoldConfig{ AgentName: agentName, APICallerName: apiCallerName, - BrokerName: caasBrokerTrackerName, + BrokerName: providerTrackerName, Logger: config.LoggingContext.GetLogger("juju.worker.caasmodeloperator"), ModelUUID: agentConfig.Model().Id(), })), caasmodelconfigmanagerName: ifResponsible(caasmodelconfigmanager.Manifold(caasmodelconfigmanager.ManifoldConfig{ APICallerName: apiCallerName, - BrokerName: caasBrokerTrackerName, + BrokerName: providerTrackerName, Logger: config.LoggingContext.GetLogger("juju.worker.caasmodelconfigmanager"), NewWorker: caasmodelconfigmanager.NewWorker, NewFacade: caasmodelconfigmanager.NewFacade, @@ -544,7 +547,7 @@ func CAASManifolds(config ManifoldsConfig) dependency.Manifolds { caasApplicationProvisionerName: ifNotMigrating(caasapplicationprovisioner.Manifold( caasapplicationprovisioner.ManifoldConfig{ APICallerName: apiCallerName, - BrokerName: caasBrokerTrackerName, + BrokerName: providerTrackerName, ClockName: clockName, NewWorker: caasapplicationprovisioner.NewProvisionerWorker, Logger: config.LoggingContext.GetLogger("juju.worker.caasapplicationprovisioner"), @@ -563,7 +566,7 @@ func CAASManifolds(config ManifoldsConfig) dependency.Manifolds { APICallerName: apiCallerName, Clock: config.Clock, Logger: config.LoggingContext.GetLogger("juju.worker.storageprovisioner"), - StorageRegistryName: caasBrokerTrackerName, + StorageRegistryName: providerTrackerName, Model: modelTag, NewCredentialValidatorFacade: common.NewCredentialInvalidatorFacade, NewWorker: storageprovisioner.NewCaasWorker, @@ -700,7 +703,6 @@ const ( caasmodelconfigmanagerName = "caas-model-config-manager" caasApplicationProvisionerName = "caas-application-provisioner" caasStorageProvisionerName = "caas-storage-provisioner" - caasBrokerTrackerName = "caas-broker-tracker" secretsPrunerName = "secrets-pruner" userSecretsDrainWorker = "user-secrets-drain-worker" diff --git a/cmd/jujud-controller/agent/model/manifolds_test.go b/cmd/jujud-controller/agent/model/manifolds_test.go index 92cd316f216..ab13346a022 100644 --- a/cmd/jujud-controller/agent/model/manifolds_test.go +++ b/cmd/jujud-controller/agent/model/manifolds_test.go @@ -90,7 +90,6 @@ func (s *ManifoldsSuite) TestCAASNames(c *gc.C) { "api-caller", "api-config-watcher", "caas-application-provisioner", - "caas-broker-tracker", "caas-firewaller", "caas-model-config-manager", "caas-model-operator", @@ -106,6 +105,7 @@ func (s *ManifoldsSuite) TestCAASNames(c *gc.C) { "not-alive-flag", "not-dead-flag", "provider-service-factory", + "provider-tracker", "provider-upgrade-gate", "provider-upgraded-flag", "provider-upgrader", @@ -214,30 +214,31 @@ var expectedCAASModelManifoldsWithDependencies = map[string][]string{ "is-responsible-flag", "migration-fortress", "migration-inactive-flag", + "not-dead-flag", "provider-upgrade-gate", "provider-upgraded-flag", - "not-dead-flag"}, + }, "secrets-pruner": { "agent", "api-caller", - "provider-upgrade-gate", - "provider-upgraded-flag", "is-responsible-flag", "migration-fortress", "migration-inactive-flag", "not-dead-flag", + "provider-upgrade-gate", + "provider-upgraded-flag", }, "user-secrets-drain-worker": { "agent", "api-caller", - "provider-upgrade-gate", - "provider-upgraded-flag", "is-responsible-flag", "migration-fortress", "migration-inactive-flag", "not-dead-flag", + "provider-upgrade-gate", + "provider-upgraded-flag", }, "agent": {}, @@ -246,51 +247,69 @@ var expectedCAASModelManifoldsWithDependencies = map[string][]string{ "api-config-watcher": {"agent"}, - "caas-broker-tracker": {"agent", "api-caller", "is-responsible-flag"}, + "provider-tracker": { + "agent", + "api-caller", + "is-responsible-flag", + "provider-service-factory", + }, - "caas-model-config-manager": {"agent", "api-caller", "caas-broker-tracker", "is-responsible-flag"}, + "caas-model-config-manager": { + "agent", + "api-caller", + "is-responsible-flag", + "provider-service-factory", + "provider-tracker", + }, "caas-firewaller": { "agent", "api-caller", - "caas-broker-tracker", "is-responsible-flag", "migration-fortress", "migration-inactive-flag", + "not-dead-flag", + "provider-service-factory", + "provider-tracker", "provider-upgrade-gate", "provider-upgraded-flag", - "not-dead-flag"}, + }, "caas-model-operator": { "agent", "api-caller", - "caas-broker-tracker", + "provider-service-factory", + "provider-tracker", "is-responsible-flag", }, "caas-application-provisioner": { "agent", "api-caller", - "caas-broker-tracker", "clock", "is-responsible-flag", "migration-fortress", "migration-inactive-flag", + "not-dead-flag", + "provider-service-factory", + "provider-tracker", "provider-upgrade-gate", "provider-upgraded-flag", - "not-dead-flag"}, + }, "caas-storage-provisioner": { "agent", "api-caller", - "caas-broker-tracker", "is-responsible-flag", "migration-fortress", "migration-inactive-flag", + "not-dead-flag", + "provider-service-factory", + "provider-tracker", "provider-upgrade-gate", "provider-upgraded-flag", - "not-dead-flag", - "valid-credential-flag"}, + "valid-credential-flag", + }, "charm-downloader": { "agent", @@ -298,10 +317,11 @@ var expectedCAASModelManifoldsWithDependencies = map[string][]string{ "is-responsible-flag", "migration-fortress", "migration-inactive-flag", + "not-dead-flag", "provider-upgrade-gate", "provider-upgraded-flag", - "not-dead-flag", - "valid-credential-flag"}, + "valid-credential-flag", + }, "charm-revision-updater": { "agent", @@ -309,9 +329,10 @@ var expectedCAASModelManifoldsWithDependencies = map[string][]string{ "is-responsible-flag", "migration-fortress", "migration-inactive-flag", + "not-dead-flag", "provider-upgrade-gate", "provider-upgraded-flag", - "not-dead-flag"}, + }, "clock": {}, @@ -323,35 +344,38 @@ var expectedCAASModelManifoldsWithDependencies = map[string][]string{ "is-responsible-flag", "migration-fortress", "migration-inactive-flag", + "not-dead-flag", "provider-upgrade-gate", "provider-upgraded-flag", - "not-dead-flag", }, "migration-fortress": { "agent", "api-caller", "is-responsible-flag", + "not-dead-flag", "provider-upgrade-gate", "provider-upgraded-flag", - "not-dead-flag"}, + }, "migration-inactive-flag": { "agent", "api-caller", "is-responsible-flag", + "not-dead-flag", "provider-upgrade-gate", "provider-upgraded-flag", - "not-dead-flag"}, + }, "migration-master": { "agent", "api-caller", "is-responsible-flag", "migration-fortress", + "not-dead-flag", "provider-upgrade-gate", "provider-upgraded-flag", - "not-dead-flag"}, + }, "provider-upgrade-gate": {}, @@ -360,9 +384,9 @@ var expectedCAASModelManifoldsWithDependencies = map[string][]string{ "provider-upgrader": { "agent", "api-caller", - "provider-upgrade-gate", "is-responsible-flag", "not-dead-flag", + "provider-upgrade-gate", "valid-credential-flag", }, @@ -378,9 +402,10 @@ var expectedCAASModelManifoldsWithDependencies = map[string][]string{ "is-responsible-flag", "migration-fortress", "migration-inactive-flag", + "not-dead-flag", "provider-upgrade-gate", "provider-upgraded-flag", - "not-dead-flag"}, + }, "state-cleaner": { "agent", @@ -388,9 +413,10 @@ var expectedCAASModelManifoldsWithDependencies = map[string][]string{ "is-responsible-flag", "migration-fortress", "migration-inactive-flag", + "not-dead-flag", "provider-upgrade-gate", "provider-upgraded-flag", - "not-dead-flag"}, + }, "status-history-pruner": { "agent", @@ -398,9 +424,10 @@ var expectedCAASModelManifoldsWithDependencies = map[string][]string{ "is-responsible-flag", "migration-fortress", "migration-inactive-flag", + "not-dead-flag", "provider-upgrade-gate", "provider-upgraded-flag", - "not-dead-flag"}, + }, "undertaker": { "agent", diff --git a/internal/worker/caasrbacmapper/manifold.go b/internal/worker/caasrbacmapper/manifold.go index 5b0099c9f9a..6c29570b5af 100644 --- a/internal/worker/caasrbacmapper/manifold.go +++ b/internal/worker/caasrbacmapper/manifold.go @@ -7,10 +7,11 @@ import ( "context" "github.com/juju/errors" - "github.com/juju/juju/caas" "github.com/juju/worker/v4" "github.com/juju/worker/v4/dependency" "k8s.io/client-go/informers" + + "github.com/juju/juju/caas" ) type K8sBroker interface { diff --git a/internal/worker/providertracker/worker_test.go b/internal/worker/providertracker/worker_test.go index 58d4da4480e..6a84cd13292 100644 --- a/internal/worker/providertracker/worker_test.go +++ b/internal/worker/providertracker/worker_test.go @@ -18,7 +18,6 @@ import ( modeltesting "github.com/juju/juju/core/model/testing" "github.com/juju/juju/core/watcher/watchertest" "github.com/juju/juju/environs" - cloudspec "github.com/juju/juju/environs/cloudspec" "github.com/juju/juju/environs/config" "github.com/juju/juju/testing" ) @@ -186,9 +185,9 @@ func (s *workerSuite) getConfig(environ environs.Environ) Config[environs.Enviro CloudService: s.cloudService, ConfigService: s.configService, CredentialService: s.credentialService, - GetProvider: func(ctx context.Context, pcg ProviderConfigGetter) (environs.Environ, cloudspec.CloudSpec, error) { - return environ, cloudspec.CloudSpec{}, nil - }, + GetProvider: IAASGetProvider(func(ctx context.Context, args environs.OpenParams) (environs.Environ, error) { + return environ, nil + }), Logger: s.logger, } } @@ -244,15 +243,6 @@ func (s *workerSuite) expectEnvironSetSpecUpdate(c *gc.C) { s.cloudSpecSetter.EXPECT().SetCloudSpec(gomock.Any(), gomock.Any()).Return(nil) } -func (s *workerSuite) expectEnvironSetSpecNoUpdate(c *gc.C) { - s.cloudService.EXPECT().Cloud(gomock.Any(), "cloud").Return(&cloud.Cloud{}, nil) - s.credentialService.EXPECT().CloudCredential(gomock.Any(), credential.Key{ - Cloud: "cloud", - Owner: "owner", - Name: "name", - }).Return(cloud.Credential{}, nil) -} - func (s *workerSuite) expectConfigWatcher(c *gc.C) chan []string { ch := make(chan []string) // Seed the initial event.