Skip to content

Commit

Permalink
Merge pull request juju#18284 from SimonRichardson/wireup-lease-owner…
Browse files Browse the repository at this point in the history
…ship

juju#18284

This set of changes wires up the lease management so that it's possible to use that lease manager with the lease service. The lease manager was always available as a dependency via the dependency engine, so it was a matter of adding it to the domain services manifold and making it available to services that require a lease.

We luckily dodged a bullet here, because the lease system depends on the dqlite database, we didn't add it to the domain services, if we did, we would have had a chicken and egg issue. How do you get a domain service when you depend on it? Luckily the lease system is from the primordial soup before the domain service changes. We wouldn't want to offer it out as a service anyway. If we make leases per model, we could do the same trick that we've done before: lift the service out and implant it into its own worker tree. This was done for the provider tracker and the objectstore. 

This PR doesn't update any services, but I've included a small patch that can be applied to show it working.

## QA steps

Apply the following patch:

```diff
commit 0eb5a4aa42b4eaa5c8fe0ce8d6157363363de305
Author: Simon Richardson <[email protected]>
Date: Fri Oct 25 15:17:59 2024 +0100

 F

diff --git a/domain/application/modelmigration/export.go b/domain/application/modelmigration/export.go
index 40862b52f0..48b2fca0a2 100644
--- a/domain/application/modelmigration/export.go
+++ b/domain/application/modelmigration/export.go
@@ -65,6 +65,7 @@ func (e *exportOperation) Setup(scope modelmigration.Scope) error {
 // TODO (stickupkid): The storage.ProviderRegistry should be passed in
 // so that we can create the appropriate storage instances.
 e.service = service.NewService(
+ nil,
 state.NewApplicationState(scope.ModelDB(), e.logger),
 state.NewCharmState(scope.ModelDB()),
 corestorage.ConstModelStorageRegistry(func() storage.ProviderRegistry {
diff --git a/domain/application/modelmigration/import.go b/domain/application/modelmigration/import.go
index 96d1696819..6079dc3fa7 100644
--- a/domain/application/modelmigration/import.go
+++ b/domain/application/modelmigration/import.go
@@ -71,6 +71,7 @@ func (i *importOperation) Name() string {
 // Setup creates the service that is used to import applications.
 func (i *importOperation) Setup(scope modelmigration.Scope) error {
 i.service = service.NewService(
+ nil,
 state.NewApplicationState(scope.ModelDB(), i.logger),
 state.NewCharmState(scope.ModelDB()),
 i.registry,
diff --git a/domain/application/service/application.go b/domain/application/service/application.go
index 0e2c532c5c..6138900f9b 100644
--- a/domain/application/service/application.go
+++ b/domain/application/service/application.go
@@ -12,6 +12,7 @@ import (
 "github.com/juju/clock"
 "github.com/juju/collections/transform"
 "github.com/juju/errors"
+ "github.com/juju/loggo/v2"
 "github.com/juju/names/v5"
 "github.com/juju/version/v2"
 
@@ -22,6 +23,7 @@ import (
 corecharm "github.com/juju/juju/core/charm"
 coredatabase "github.com/juju/juju/core/database"
 "github.com/juju/juju/core/leadership"
+ "github.com/juju/juju/core/lease"
 corelife "github.com/juju/juju/core/life"
 "github.com/juju/juju/core/logger"
 coremodel "github.com/juju/juju/core/model"
@@ -223,6 +225,8 @@ func (NotImplementedSecretService) InternalDeleteSecret(domain.AtomicContext, *c
 
 // ApplicationService provides the API for working with applications.
 type ApplicationService struct {
+ lease *domain.LeaseService
+
 st ApplicationState
 logger logger.Logger
 clock clock.Clock
@@ -232,7 +236,7 @@ type ApplicationService struct {
 }
 
 // NewApplicationService returns a new service reference wrapping the input state.
-func NewApplicationService(st ApplicationState, storageRegistryGetter corestorage.ModelStorageRegistryGetter, secrets SecretService, logger logger.Logger) *ApplicationService {
+func NewApplicationService(leaseChecker lease.ModelLeaseManagerGetter, st ApplicationState, storageRegistryGetter corestorage.ModelStorageRegistryGetter, secrets SecretService, logger logger.Logger) *ApplicationService {
 // Some uses of application service don't need to supply a storage registry,
 // eg cleaner facade. In such cases it'd wasteful to create one as an
 // environ instance would be needed.
@@ -240,6 +244,7 @@ func NewApplicationService(st ApplicationState, storageRegistryGetter corestorag
 secrets = NotImplementedSecretService{}
 }
 return &ApplicationService{
+ lease: domain.NewLeaseService(leaseChecker),
 st: st,
 logger: logger,
 clock: clock.WallClock,
@@ -291,6 +296,12 @@ func (s *ApplicationService) CreateApplication(
 unitArgs[i] = arg
 }
 
+ e := s.lease.WithLease(ctx, "controller", "controller/0", func(ctx context.Context) error {
+ loggo.GetLogger("***").Criticalf("Got the lease for controller/0")
+ return nil
+ })
+ loggo.GetLogger("***").Criticalf("e: %v", e)
+
 var appID coreapplication.ID
 err = s.st.RunAtomic(ctx, func(ctx domain.AtomicContext) error {
 appID, err = s.st.CreateApplication(ctx, name, appArg)
@@ -1368,6 +1379,7 @@ type ProviderApplicationService struct {
 
 // NewProviderApplicationService returns a new Service for interacting with a models state.
 func NewProviderApplicationService(
+ leaseChecker lease.ModelLeaseManagerGetter,
 st ApplicationState,
 modelID coremodel.UUID,
 agentVersionGetter AgentVersionGetter,
@@ -1376,7 +1388,7 @@ func NewProviderApplicationService(
 secretService SecretService,
 logger logger.Logger,
 ) *ProviderApplicationService {
- service := NewApplicationService(st, storageRegistryGetter, secretService, logger)
+ service := NewApplicationService(leaseChecker, st, storageRegistryGetter, secretService, logger)
 
 return &ProviderApplicationService{
 ApplicationService: *service,
@@ -1429,6 +1441,7 @@ type WatchableApplicationService struct {
 
 // NewWatchableApplicationService returns a new service reference wrapping the input state.
 func NewWatchableApplicationService(
+ leaseChecker lease.ModelLeaseManagerGetter,
 st ApplicationState,
 watcherFactory WatcherFactory,
 modelID coremodel.UUID,
@@ -1438,7 +1451,7 @@ func NewWatchableApplicationService(
 secretService SecretService,
 logger logger.Logger,
 ) *WatchableApplicationService {
- service := NewProviderApplicationService(st, modelID, agentVersionGetter, provider, storageRegistryGetter, secretService, logger)
+ service := NewProviderApplicationService(leaseChecker, st, modelID, agentVersionGetter, provider, storageRegistryGetter, secretService, logger)
 
 return &WatchableApplicationService{
 ProviderApplicationService: *service,
diff --git a/domain/application/service/service.go b/domain/application/service/service.go
index c46db9a5d7..2933840b90 100644
--- a/domain/application/service/service.go
+++ b/domain/application/service/service.go
@@ -7,6 +7,7 @@ import (
 "context"
 
 "github.com/juju/juju/core/assumes"
+ "github.com/juju/juju/core/lease"
 "github.com/juju/juju/core/logger"
 coremodel "github.com/juju/juju/core/model"
 "github.com/juju/juju/core/providertracker"
@@ -29,6 +30,7 @@ type Service struct {
 // NewService returns a new Service for interacting with the underlying
 // application state.
 func NewService(
+ leaseChecker lease.ModelLeaseManagerGetter,
 appSt ApplicationState, charmSt CharmState,
 storageRegistryGetter corestorage.ModelStorageRegistryGetter,
 secretService SecretService,
@@ -36,7 +38,7 @@ func NewService(
 ) *Service {
 return &Service{
 CharmService: NewCharmService(charmSt, logger),
- ApplicationService: NewApplicationService(appSt, storageRegistryGetter, secretService, logger),
+ ApplicationService: NewApplicationService(leaseChecker, appSt, storageRegistryGetter, secretService, logger),
 }
 }
 
@@ -50,6 +52,7 @@ type WatchableService struct {
 
 // NewWatchableService returns a new service reference wrapping the input state.
 func NewWatchableService(
+ leaseChecker lease.ModelLeaseManagerGetter,
 appSt ApplicationState,
 charmSt CharmState,
 watcherFactory WatcherFactory,
@@ -61,7 +64,7 @@ func NewWatchableService(
 logger logger.Logger,
 ) *WatchableService {
 watchableCharmService := NewWatchableCharmService(charmSt, watcherFactory, logger)
- watchableApplicationService := NewWatchableApplicationService(appSt, watcherFactory, modelID, agentVersionGetter, provider, storageRegistryGetter, secretService, logger)
+ watchableApplicationService := NewWatchableApplicationService(leaseChecker, appSt, watcherFactory, modelID, agentVersionGetter, provider, storageRegistryGetter, secretService, logger)
 return &WatchableService{
 Service: Service{
 CharmService: &watchableCharmService.CharmService,
diff --git a/domain/port/modelmigration/import.go b/domain/port/modelmigration/import.go
index 38003f1f5c..26c0cfc1ad 100644
--- a/domain/port/modelmigration/import.go
+++ b/domain/port/modelmigration/import.go
@@ -67,6 +67,7 @@ func (i *importOperation) Setup(scope modelmigration.Scope) error {
 i.portService = service.NewService(
 state.NewState(scope.ModelDB()), i.logger)
 i.applicationService = applicationservice.NewService(
+ nil,
 applicationstate.NewApplicationState(scope.ModelDB(), i.logger),
 applicationstate.NewCharmState(scope.ModelDB()),
 corestorage.ConstModelStorageRegistry(func() storage.ProviderRegistry {
diff --git a/domain/services/model.go b/domain/services/model.go
index b5ec7ad262..c786437494 100644
--- a/domain/services/model.go
+++ b/domain/services/model.go
@@ -148,6 +148,7 @@ func (s *ModelFactory) BlockDevice() *blockdeviceservice.WatchableService {
 // Application returns the model's application service.
 func (s *ModelFactory) Application(secrets applicationservice.SecretService) *applicationservice.WatchableService {
 return applicationservice.NewWatchableService(
+ s.leaseManager,
 applicationstate.NewApplicationState(changestream.NewTxnRunnerFactory(s.modelDB),
 s.logger.Child("application"),
 ),
```

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

Check the `juju debug-log -m controller`, you should see:

```sh
machine-0: 15:03:37 CRITICAL prdesc e: checking lease token: lease not held
```

Then do the following:

```sh
$ juju switch controller
$ juju deploy ubuntu
```

Check the `juju debug-log -m controller`, you should now see:

```sh
machine-0: 15:04:31 CRITICAL prdesc Got the lease for controller/0
machine-0: 15:04:31 CRITICAL prdesc e: <nil>
```
  • Loading branch information
jujubot authored Nov 11, 2024
2 parents 90ddb0c + dc34beb commit 5e97299
Show file tree
Hide file tree
Showing 23 changed files with 874 additions and 176 deletions.
2 changes: 1 addition & 1 deletion apiserver/auditconfig_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -220,7 +220,7 @@ func (s *auditConfigSuite) TestAuditLoggingUsesExcludeMethods(c *gc.C) {
func (s *auditConfigSuite) TestNewServerValidatesConfig(c *gc.C) {
cfg := testing.DefaultServerConfig(c, nil)
cfg.GetAuditConfig = nil
cfg.DomainServicesGetter = s.DomainServicesGetter(c, s.NoopObjectStore(c))
cfg.DomainServicesGetter = s.DomainServicesGetter(c, s.NoopObjectStore(c), s.NoopLeaseManager(c))

srv, err := apiserver.NewServer(context.Background(), cfg)
c.Assert(err, gc.ErrorMatches, "missing GetAuditConfig not valid")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -533,7 +533,7 @@ func (s *withoutControllerSuite) TestSetModificationStatus(c *gc.C) {

func (s *withoutControllerSuite) TestMachinesWithTransientErrors(c *gc.C) {
st := s.ControllerModel(c).State()
domainServicesGetter := s.DomainServicesGetter(c, s.NoopObjectStore(c))
domainServicesGetter := s.DomainServicesGetter(c, s.NoopObjectStore(c), s.NoopLeaseManager(c))
machineService := domainServicesGetter.ServicesForModel(model.UUID(st.ModelUUID())).Machine()

now := time.Now()
Expand Down Expand Up @@ -1011,7 +1011,7 @@ func (s *withoutControllerSuite) TestDistributionGroup(c *gc.C) {
defer release()

st := s.ControllerModel(c).State()
domainServicesGetter := s.DomainServicesGetter(c, s.NoopObjectStore(c))
domainServicesGetter := s.DomainServicesGetter(c, s.NoopObjectStore(c), s.NoopLeaseManager(c))
machineService := domainServicesGetter.ServicesForModel(model.UUID(st.ModelUUID())).Machine()

addUnits := func(name string, machines ...*state.Machine) (units []*state.Unit) {
Expand Down Expand Up @@ -1324,7 +1324,7 @@ func (s *withoutControllerSuite) TestConstraints(c *gc.C) {
}

func (s *withoutControllerSuite) TestSetInstanceInfo(c *gc.C) {
domainServicesGetter := s.DomainServicesGetter(c, s.NoopObjectStore(c))
domainServicesGetter := s.DomainServicesGetter(c, s.NoopObjectStore(c), s.NoopLeaseManager(c))

st := s.ControllerModel(c).State()
storageService := domainServicesGetter.ServicesForModel(model.UUID(st.ModelUUID())).Storage()
Expand Down Expand Up @@ -1455,7 +1455,7 @@ func (s *withoutControllerSuite) TestSetInstanceInfo(c *gc.C) {
func (s *withoutControllerSuite) TestInstanceId(c *gc.C) {
st := s.ControllerModel(c).State()

domainServicesGetter := s.DomainServicesGetter(c, s.NoopObjectStore(c))
domainServicesGetter := s.DomainServicesGetter(c, s.NoopObjectStore(c), s.NoopLeaseManager(c))
machineService := domainServicesGetter.ServicesForModel(model.UUID(st.ModelUUID())).Machine()
// Provision 2 machines first.
machine0UUID, err := machineService.GetMachineUUID(context.Background(), coremachine.Name(s.machines[0].Id()))
Expand Down
4 changes: 2 additions & 2 deletions apiserver/facades/agent/provisioner/provisioninginfo_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ import (
)

func (s *withoutControllerSuite) TestProvisioningInfoWithStorage(c *gc.C) {
domainServicesGetter := s.DomainServicesGetter(c, s.NoopObjectStore(c))
domainServicesGetter := s.DomainServicesGetter(c, s.NoopObjectStore(c), s.NoopLeaseManager(c))

st := s.ControllerModel(c).State()
storageService := domainServicesGetter.ServicesForModel(model.UUID(st.ModelUUID())).Storage()
Expand Down Expand Up @@ -126,7 +126,7 @@ func (s *withoutControllerSuite) TestProvisioningInfoWithStorage(c *gc.C) {
}

func (s *withoutControllerSuite) TestProvisioningInfoRootDiskVolume(c *gc.C) {
domainServicesGetter := s.DomainServicesGetter(c, s.NoopObjectStore(c))
domainServicesGetter := s.DomainServicesGetter(c, s.NoopObjectStore(c), s.NoopLeaseManager(c))

st := s.ControllerModel(c).State()
storageService := domainServicesGetter.ServicesForModel(model.UUID(st.ModelUUID())).Storage()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ func (s *iaasProvisionerSuite) newApi(c *gc.C, blockDeviceService storageprovisi
c.Assert(err, jc.ErrorIsNil)
registry := storageprovider.NewStorageProviderRegistry(env)
s.st = s.ControllerModel(c).State()
domainServicesGetter := s.DomainServicesGetter(c, s.NoopObjectStore(c))
domainServicesGetter := s.DomainServicesGetter(c, s.NoopObjectStore(c), s.NoopLeaseManager(c))
storageService := domainServicesGetter.ServicesForModel(model.UUID(s.st.ModelUUID())).Storage()

s.authorizer = &apiservertesting.FakeAuthorizer{
Expand Down
2 changes: 1 addition & 1 deletion apiserver/facades/client/application/application_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,7 @@ func (s *applicationSuite) makeAPI(c *gc.C) {
Type: model.IAAS,
}
registry := provider.NewStorageProviderRegistry(env)
domainServicesGetter := s.DomainServicesGetter(c, s.NoopObjectStore(c))
domainServicesGetter := s.DomainServicesGetter(c, s.NoopObjectStore(c), s.NoopLeaseManager(c))
storageService := domainServicesGetter.ServicesForModel(model.UUID(st.ModelUUID())).Storage()
applicationService := domainServices.Application()

Expand Down
3 changes: 2 additions & 1 deletion apiserver/facades/client/controller/controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,8 @@ func (s *controllerSuite) SetUpTest(c *gc.C) {
s.DomainServicesSuite.ControllerConfig = controllerCfg
s.DomainServicesSuite.SetUpTest(c)

jujujujutesting.SeedDatabase(c, s.ControllerSuite.TxnRunner(), s.DomainServicesGetter(c, s.NoopObjectStore(c))(s.ControllerModelUUID), controllerCfg)
domainServiceGetter := s.DomainServicesGetter(c, s.NoopObjectStore(c), s.NoopLeaseManager(c))
jujujujutesting.SeedDatabase(c, s.ControllerSuite.TxnRunner(), domainServiceGetter(s.ControllerModelUUID), controllerCfg)

s.hub = pubsub.NewStructuredHub(nil)

Expand Down
1 change: 1 addition & 0 deletions cmd/jujud-controller/agent/machine/manifolds.go
Original file line number Diff line number Diff line change
Expand Up @@ -705,6 +705,7 @@ func commonManifolds(config ManifoldsConfig) dependency.Manifolds {
ObjectStoreName: objectStoreName,
StorageRegistryName: storageRegistryName,
HTTPClientName: httpClientName,
LeaseManagerName: leaseManagerName,
Logger: internallogger.GetLogger("juju.worker.services"),
Clock: config.Clock,
NewWorker: workerdomainservices.NewWorker,
Expand Down
28 changes: 28 additions & 0 deletions core/lease/lease.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
// Copyright 2024 Canonical Ltd.
// Licensed under the AGPLv3, see LICENCE file for details.

package lease

import "github.com/juju/juju/core/model"

// LeaseCheckerWaiter is an interface that checks and waits if a lease is held
// by a holder.
type LeaseCheckerWaiter interface {
Waiter
Checker
}

// LeaseManagerGetter is an interface that provides a method to get a lease
// manager for a given lease using its UUID. The lease namespace could be a
// model or an application.
type LeaseManagerGetter interface {
// GetLeaseManager returns a lease manager for the given model UUID.
GetLeaseManager(model.UUID) (LeaseCheckerWaiter, error)
}

// ModelLeaseManagerGetter is an interface that provides a method to
// get a lease manager in the scope of a model.
type ModelLeaseManagerGetter interface {
// GetLeaseManager returns a lease manager for the given model UUID.
GetLeaseManager() (LeaseCheckerWaiter, error)
}
167 changes: 165 additions & 2 deletions domain/lease_mock_test.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading

0 comments on commit 5e97299

Please sign in to comment.