Merge pull request juju#17060 from SimonRichardson/generalize-provide…

…r-tracker

juju#17060

This generalizes the provider trackers to handle both providers and caas brokers in the same 1 worker. Both providers and brokers require watching the same information; the cloud and credential changes, along with updating the cloud spec.

Integration in the dependency engine and removal of the workers can then be aligned to 1 worker servicing multiple users. Removal of the caas broker can't be fully done, as the jujud model operator requires access to the broker, which will do so over the API connection.

A follow-up to this PR will create a runner inside the worker that allows the caching of multiple environs per model uuid.

----

## Checklist

<!-- If an item is not applicable, use `~strikethrough~`. -->

- [x] Code style: imports ordered, good names, simple structure, etc
- [x] Comments saying why design decisions were made
- [x] Go unit tests, with comments saying what you're testing

## QA steps

Ensure that we can bootstrap both IAAS and CAAS.

```sh
$ juju bootstrap lxd test
$ juju add-model default
$ juju deploy ubuntu
```

```sh
$ juju bootstrap microk8s test
$ juju add-model default
$ juju deploy redis-k8s
```


## Links

**Jira card:** JUJU-5643

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"
@@ -351,12 +350,14 @@ func IAASManifolds(config ManifoldsConfig) dependency.Manifolds {
 	controllerTag := agentConfig.Controller()
 	modelTag := agentConfig.Model()
 	manifolds := dependency.Manifolds{
-		providerTrackerName: ifCredentialValid(ifResponsible(providertracker.Manifold(providertracker.ManifoldConfig{
+		providerTrackerName: ifCredentialValid(ifResponsible(providertracker.Manifold(providertracker.ManifoldConfig[environs.Environ]{
 			ProviderServiceFactoryName: providerServiceFactoryName,
-			NewEnviron:                 config.NewEnvironFunc,
-			NewWorker:                  providertracker.NewWorker,
+			NewWorker:                  providertracker.NewWorker[environs.Environ],
 			GetProviderServiceFactory:  providertracker.GetProviderServiceFactory,
 			Logger:                     config.LoggingContext.GetLogger("juju.worker.providertracker"),
+			GetProvider: providertracker.IAASGetProvider(func(ctx context.Context, args environs.OpenParams) (environs.Environ, error) {
+				return config.NewEnvironFunc(ctx, args)
+			}),
 		}))),
 
 		// Everything else should be wrapped in ifResponsible,
@@ -489,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,
@@ -502,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 {
@@ -525,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,
@@ -542,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"),
@@ -561,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,
@@ -698,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"

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,72 +247,92 @@ 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",
 		"api-caller",
 		"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",
 		"api-caller",
 		"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,29 +402,32 @@ 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",
 		"api-caller",
 		"is-responsible-flag",
 		"migration-fortress",
 		"migration-inactive-flag",
+		"not-dead-flag",
 		"provider-upgrade-gate",
 		"provider-upgraded-flag",
-		"not-dead-flag"},
+	},
 
 	"status-history-pruner": {
 		"agent",
 		"api-caller",
 		"is-responsible-flag",
 		"migration-fortress",
 		"migration-inactive-flag",
+		"not-dead-flag",
 		"provider-upgrade-gate",
 		"provider-upgraded-flag",
-		"not-dead-flag"},
+	},
 
 	"undertaker": {
 		"agent",

domain/model/modelmigration/import.go

@@ -15,9 +15,9 @@ import (
 	coremodel "github.com/juju/juju/core/model"
 	"github.com/juju/juju/core/modelmigration"
 	coreuser "github.com/juju/juju/core/user"
-	usererrors "github.com/juju/juju/domain/access/errors"
-	userservice "github.com/juju/juju/domain/access/service"
-	userstate "github.com/juju/juju/domain/access/state"
+	accesserrors "github.com/juju/juju/domain/access/errors"
+	accessservice "github.com/juju/juju/domain/access/service"
+	accessstate "github.com/juju/juju/domain/access/state"
 	controllerconfigservice "github.com/juju/juju/domain/controllerconfig/service"
 	controllerconfigstate "github.com/juju/juju/domain/controllerconfig/state"
 	domainmodel "github.com/juju/juju/domain/model"
@@ -103,7 +103,7 @@ func (i *importOperation) Setup(scope modelmigration.Scope) error {
 	i.readOnlyModelService = modelservice.NewModelService(
 		modelstate.NewModelState(scope.ModelDB()),
 	)
-	i.userService = userservice.NewService(userstate.NewState(scope.ControllerDB(), i.logger))
+	i.userService = accessservice.NewService(accessstate.NewState(scope.ControllerDB(), i.logger))
 	i.controllerConfigService = controllerconfigservice.NewService(
 		controllerconfigstate.NewState(scope.ControllerDB()),
 	)
@@ -116,17 +116,17 @@ func (i *importOperation) Setup(scope modelmigration.Scope) error {
 // If model name or uuid are undefined or are not strings in the model config an
 // error satisfying [errors.NotValid] will be returned.
 // If the user specified for the model cannot be found an error satisfying
-// [usererrors.NotFound] will be returned.
+// [accesserrors.NotFound] will be returned.
 func (i importOperation) Execute(ctx context.Context, model description.Model) error {
 	modelName, uuid, err := i.getModelNameAndUUID(model)
 	if err != nil {
 		return fmt.Errorf("importing model during migration %w", errors.NotValid)
 	}
 
 	user, err := i.userService.GetUserByName(ctx, model.Owner().Name())
-	if errors.Is(err, usererrors.UserNotFound) {
+	if errors.Is(err, accesserrors.UserNotFound) {
 		return fmt.Errorf("cannot import model %q with uuid %q, %w for name %q",
-			modelName, uuid, usererrors.UserNotFound, model.Owner().Name(),
+			modelName, uuid, accesserrors.UserNotFound, model.Owner().Name(),
 		)
 	} else if err != nil {
 		return fmt.Errorf(