From 16c8c3b4e3c15ae5b52f429cc30e17fa11173565 Mon Sep 17 00:00:00 2001 From: Simon Richardson Date: Mon, 18 Nov 2024 17:34:09 +0000 Subject: [PATCH 1/7] feat: add application watcher Adding application watcher to service and state. This will return applications with pending charms. This will help migrate async charm downloader. --- domain/application/service/application.go | 65 ++++++--- .../application/service/package_mock_test.go | 78 +++++++++++ domain/application/service/service.go | 79 +++++++++++ domain/application/state/application.go | 125 +++++++++++++++--- domain/application/state/application_test.go | 100 ++++++++++++++ domain/application/watcher_test.go | 65 ++++++--- 6 files changed, 455 insertions(+), 57 deletions(-) diff --git a/domain/application/service/application.go b/domain/application/service/application.go index c13e4f1533b..668bfee1696 100644 --- a/domain/application/service/application.go +++ b/domain/application/service/application.go @@ -52,9 +52,9 @@ type AtomicApplicationState interface { GetUnitUUID(ctx domain.AtomicContext, unitName coreunit.Name) (coreunit.UUID, error) // CreateApplication creates an application, returning an error satisfying - // [applicationerrors.ApplicationAlreadyExists] if the application already exists. - // If returns as error satisfying [applicationerrors.CharmNotFound] if the charm - // for the application is not found. + // [applicationerrors.ApplicationAlreadyExists] if the application already + // exists. If returns as error satisfying [applicationerrors.CharmNotFound] + // if the charm for the application is not found. CreateApplication(domain.AtomicContext, string, application.AddApplicationArg) (coreapplication.ID, error) // AddUnits adds the specified units to the application. @@ -73,20 +73,25 @@ type AtomicApplicationState interface { // SetUnitPassword updates the password for the specified unit UUID. SetUnitPassword(domain.AtomicContext, coreunit.UUID, application.PasswordInfo) error - // SetCloudContainerStatus saves the given cloud container status, overwriting any current status data. - // If returns an error satisfying [applicationerrors.UnitNotFound] if the unit doesn't exist. + // SetCloudContainerStatus saves the given cloud container status, + // overwriting any current status data. If returns an error satisfying + // [applicationerrors.UnitNotFound] if the unit doesn't exist. SetCloudContainerStatus(domain.AtomicContext, coreunit.UUID, application.CloudContainerStatusStatusInfo) error - // SetUnitAgentStatus saves the given unit agent status, overwriting any current status data. - // If returns an error satisfying [applicationerrors.UnitNotFound] if the unit doesn't exist. + // SetUnitAgentStatus saves the given unit agent status, overwriting any + // current status data. If returns an error satisfying + // [applicationerrors.UnitNotFound] if the unit doesn't exist. SetUnitAgentStatus(domain.AtomicContext, coreunit.UUID, application.UnitAgentStatusInfo) error - // SetUnitWorkloadStatus saves the given unit workload status, overwriting any current status data. - // If returns an error satisfying [applicationerrors.UnitNotFound] if the unit doesn't exist. + // SetUnitWorkloadStatus saves the given unit workload status, overwriting + // any current status data. If returns an error satisfying + // [applicationerrors.UnitNotFound] if the unit doesn't exist. SetUnitWorkloadStatus(domain.AtomicContext, coreunit.UUID, application.UnitWorkloadStatusInfo) error - // GetApplicationLife looks up the life of the specified application, returning an error - // satisfying [applicationerrors.ApplicationNotFoundError] if the application is not found. + // GetApplicationLife looks up the life of the specified application, + // returning an error satisfying + // [applicationerrors.ApplicationNotFoundError] if the application is not + // found. GetApplicationLife(ctx domain.AtomicContext, appName string) (coreapplication.ID, life.Life, error) // SetApplicationLife sets the life of the specified application. @@ -97,11 +102,12 @@ type AtomicApplicationState interface { // [applicationerrors.ApplicationNotFound] if the application is not found. GetApplicationScaleState(domain.AtomicContext, coreapplication.ID) (application.ScaleState, error) - // SetApplicationScalingState sets the scaling details for the given caas application - // Scale is optional and is only set if not nil. + // SetApplicationScalingState sets the scaling details for the given caas + // application Scale is optional and is only set if not nil. SetApplicationScalingState(ctx domain.AtomicContext, appID coreapplication.ID, scale *int, targetScale int, scaling bool) error - // SetDesiredApplicationScale updates the desired scale of the specified application. + // SetDesiredApplicationScale updates the desired scale of the specified + // application. SetDesiredApplicationScale(domain.AtomicContext, coreapplication.ID, int) error // GetUnitLife looks up the life of the specified unit, returning an error @@ -111,9 +117,14 @@ type AtomicApplicationState interface { // SetUnitLife sets the life of the specified unit. SetUnitLife(domain.AtomicContext, coreunit.Name, life.Life) error - // InitialWatchStatementUnitLife returns the initial namespace query for the application unit life watcher. + // InitialWatchStatementUnitLife returns the initial namespace query for the + // application unit life watcher. InitialWatchStatementUnitLife(appName string) (string, eventsource.NamespaceQuery) + // InitialWatchStatementApplicationsWithPendingCharms returns the initial + // namespace query for the applications with pending charms watcher. + InitialWatchStatementApplicationsWithPendingCharms() (string, eventsource.NamespaceQuery) + // DeleteApplication deletes the specified application, returning an error // satisfying [applicationerrors.ApplicationNotFoundError] if the // application doesn't exist. If the application still has units, as error @@ -132,7 +143,8 @@ type AtomicApplicationState interface { ctx domain.AtomicContext, unitName coreunit.Name, ) ([]*coresecrets.URI, error) - // GetSecretsForApplication returns the secrets owned by the specified application. + // GetSecretsForApplication returns the secrets owned by the specified + // application. GetSecretsForApplication( ctx domain.AtomicContext, applicationName string, ) ([]*coresecrets.URI, error) @@ -200,6 +212,11 @@ type ApplicationState interface { // application. Returns [applicationerrors.ApplicationNotFound] if the // application is not found. GetCharmModifiedVersion(ctx context.Context, id coreapplication.ID) (int, error) + + // GetApplicationsWithPendingCharmsFromUUIDs returns the applications + // with pending charms for the specified UUIDs. If the application has a + // different status, it's ignored. + GetApplicationsWithPendingCharmsFromUUIDs(ctx context.Context, uuids []coreapplication.ID) ([]coreapplication.ID, error) } // DeleteSecretState describes methods used by the secret deleter plugin. @@ -1162,9 +1179,9 @@ func (s *Service) SetApplicationScalingState(ctx context.Context, appName string } -// GetApplicationScalingState returns the scale state of an application, returning an error -// satisfying [applicationerrors.ApplicationNotFoundError] if the application doesn't exist. -// This is used on CAAS models. +// GetApplicationScalingState returns the scale state of an application, +// returning an error satisfying [applicationerrors.ApplicationNotFoundError] if +// the application doesn't exist. This is used on CAAS models. func (s *Service) GetApplicationScalingState(ctx context.Context, appName string) (ScalingState, error) { var scaleState application.ScaleState err := s.st.RunAtomic(ctx, func(ctx domain.AtomicContext) error { @@ -1180,3 +1197,13 @@ func (s *Service) GetApplicationScalingState(ctx context.Context, appName string Scaling: scaleState.Scaling, }, errors.Trace(err) } + +// GetApplicationsWithPendingCharmsFromUUIDs returns the application UUIDs that +// have pending charms from the provided UUIDs. If there are no applications +// with pending status charms, then those applications are ignored. +func (s *Service) GetApplicationsWithPendingCharmsFromUUIDs(ctx context.Context, uuids []coreapplication.ID) ([]coreapplication.ID, error) { + if len(uuids) == 0 { + return nil, nil + } + return s.st.GetApplicationsWithPendingCharmsFromUUIDs(ctx, uuids) +} diff --git a/domain/application/service/package_mock_test.go b/domain/application/service/package_mock_test.go index b3c3f25d608..b815b160e1a 100644 --- a/domain/application/service/package_mock_test.go +++ b/domain/application/service/package_mock_test.go @@ -495,6 +495,45 @@ func (c *MockStateGetApplicationUnitLifeCall) DoAndReturn(f func(context.Context return c } +// GetApplicationsWithPendingCharmsFromUUIDs mocks base method. +func (m *MockState) GetApplicationsWithPendingCharmsFromUUIDs(arg0 context.Context, arg1 []application.ID) ([]application.ID, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "GetApplicationsWithPendingCharmsFromUUIDs", arg0, arg1) + ret0, _ := ret[0].([]application.ID) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// GetApplicationsWithPendingCharmsFromUUIDs indicates an expected call of GetApplicationsWithPendingCharmsFromUUIDs. +func (mr *MockStateMockRecorder) GetApplicationsWithPendingCharmsFromUUIDs(arg0, arg1 any) *MockStateGetApplicationsWithPendingCharmsFromUUIDsCall { + mr.mock.ctrl.T.Helper() + call := mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetApplicationsWithPendingCharmsFromUUIDs", reflect.TypeOf((*MockState)(nil).GetApplicationsWithPendingCharmsFromUUIDs), arg0, arg1) + return &MockStateGetApplicationsWithPendingCharmsFromUUIDsCall{Call: call} +} + +// MockStateGetApplicationsWithPendingCharmsFromUUIDsCall wrap *gomock.Call +type MockStateGetApplicationsWithPendingCharmsFromUUIDsCall struct { + *gomock.Call +} + +// Return rewrite *gomock.Call.Return +func (c *MockStateGetApplicationsWithPendingCharmsFromUUIDsCall) Return(arg0 []application.ID, arg1 error) *MockStateGetApplicationsWithPendingCharmsFromUUIDsCall { + c.Call = c.Call.Return(arg0, arg1) + return c +} + +// Do rewrite *gomock.Call.Do +func (c *MockStateGetApplicationsWithPendingCharmsFromUUIDsCall) Do(f func(context.Context, []application.ID) ([]application.ID, error)) *MockStateGetApplicationsWithPendingCharmsFromUUIDsCall { + c.Call = c.Call.Do(f) + return c +} + +// DoAndReturn rewrite *gomock.Call.DoAndReturn +func (c *MockStateGetApplicationsWithPendingCharmsFromUUIDsCall) DoAndReturn(f func(context.Context, []application.ID) ([]application.ID, error)) *MockStateGetApplicationsWithPendingCharmsFromUUIDsCall { + c.Call = c.Call.DoAndReturn(f) + return c +} + // GetCharm mocks base method. func (m *MockState) GetCharm(arg0 context.Context, arg1 charm.ID) (charm0.Charm, charm0.CharmOrigin, error) { m.ctrl.T.Helper() @@ -1357,6 +1396,45 @@ func (c *MockStateGetUnitUUIDsCall) DoAndReturn(f func(context.Context, []unit.N return c } +// InitialWatchStatementApplicationsWithPendingCharms mocks base method. +func (m *MockState) InitialWatchStatementApplicationsWithPendingCharms() (string, eventsource.NamespaceQuery) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "InitialWatchStatementApplicationsWithPendingCharms") + ret0, _ := ret[0].(string) + ret1, _ := ret[1].(eventsource.NamespaceQuery) + return ret0, ret1 +} + +// InitialWatchStatementApplicationsWithPendingCharms indicates an expected call of InitialWatchStatementApplicationsWithPendingCharms. +func (mr *MockStateMockRecorder) InitialWatchStatementApplicationsWithPendingCharms() *MockStateInitialWatchStatementApplicationsWithPendingCharmsCall { + mr.mock.ctrl.T.Helper() + call := mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "InitialWatchStatementApplicationsWithPendingCharms", reflect.TypeOf((*MockState)(nil).InitialWatchStatementApplicationsWithPendingCharms)) + return &MockStateInitialWatchStatementApplicationsWithPendingCharmsCall{Call: call} +} + +// MockStateInitialWatchStatementApplicationsWithPendingCharmsCall wrap *gomock.Call +type MockStateInitialWatchStatementApplicationsWithPendingCharmsCall struct { + *gomock.Call +} + +// Return rewrite *gomock.Call.Return +func (c *MockStateInitialWatchStatementApplicationsWithPendingCharmsCall) Return(arg0 string, arg1 eventsource.NamespaceQuery) *MockStateInitialWatchStatementApplicationsWithPendingCharmsCall { + c.Call = c.Call.Return(arg0, arg1) + return c +} + +// Do rewrite *gomock.Call.Do +func (c *MockStateInitialWatchStatementApplicationsWithPendingCharmsCall) Do(f func() (string, eventsource.NamespaceQuery)) *MockStateInitialWatchStatementApplicationsWithPendingCharmsCall { + c.Call = c.Call.Do(f) + return c +} + +// DoAndReturn rewrite *gomock.Call.DoAndReturn +func (c *MockStateInitialWatchStatementApplicationsWithPendingCharmsCall) DoAndReturn(f func() (string, eventsource.NamespaceQuery)) *MockStateInitialWatchStatementApplicationsWithPendingCharmsCall { + c.Call = c.Call.DoAndReturn(f) + return c +} + // InitialWatchStatementUnitLife mocks base method. func (m *MockState) InitialWatchStatementUnitLife(arg0 string) (string, eventsource.NamespaceQuery) { m.ctrl.T.Helper() diff --git a/domain/application/service/service.go b/domain/application/service/service.go index 27d66040f52..7a6527b1f5a 100644 --- a/domain/application/service/service.go +++ b/domain/application/service/service.go @@ -7,16 +7,19 @@ import ( "context" "fmt" "regexp" + "sort" "github.com/juju/clock" "github.com/juju/collections/transform" "github.com/juju/errors" "github.com/juju/version/v2" + coreapplication "github.com/juju/juju/core/application" "github.com/juju/juju/core/assumes" "github.com/juju/juju/core/changestream" corecharm "github.com/juju/juju/core/charm" "github.com/juju/juju/core/database" + coredatabase "github.com/juju/juju/core/database" "github.com/juju/juju/core/logger" coremodel "github.com/juju/juju/core/model" "github.com/juju/juju/core/providertracker" @@ -241,6 +244,82 @@ func (s *WatchableService) WatchApplicationScale(ctx context.Context, appName st return s.watcherFactory.NewValueMapperWatcher("application_scale", appID.String(), mask, mapper) } +// WatchApplicationsWithPendingCharms returns a watcher that observes changes to +// applications that have pending charms. +func (s *WatchableService) WatchApplicationsWithPendingCharms(ctx context.Context) (watcher.StringsWatcher, error) { + mapper := func(ctx context.Context, db coredatabase.TxnRunner, changes []changestream.ChangeEvent) ([]changestream.ChangeEvent, error) { + // Preserve the ordering of the changes, as this is a strings watcher + // and we want to return the changes in the order they were received. + + appChanges := make(map[coreapplication.ID][]indexedChanged) + uuids := make([]coreapplication.ID, 0) + for i, change := range changes { + appID, err := coreapplication.ParseID(change.Changed()) + if err != nil { + return nil, err + } + + if _, ok := appChanges[appID]; !ok { + uuids = append(uuids, appID) + } + + appChanges[appID] = append(appChanges[appID], indexedChanged{ + change: change, + index: i, + }) + } + + // Get all the applications with pending charms using the uuids. + apps, err := s.GetApplicationsWithPendingCharmsFromUUIDs(ctx, uuids) + if err != nil { + return nil, err + } + + // If any applications have no pending charms, then return no changes. + if len(apps) == 0 { + return nil, nil + } + + // Grab all the changes for the applications with pending charms, + // ensuring they're indexed so we can sort them later. + var indexed []indexedChanged + for _, appID := range apps { + events, ok := appChanges[appID] + if !ok { + s.logger.Errorf("application %q has pending charms but no change events", appID) + continue + } + + indexed = append(indexed, events...) + } + + // Sort the index so they're preserved + sort.Slice(indexed, func(i, j int) bool { + return indexed[i].index < indexed[j].index + }) + + // Grab the changes in the order they were received. + var results []changestream.ChangeEvent + for i, result := range indexed { + results[i] = result.change + } + + return results, nil + } + table, query := s.st.InitialWatchStatementApplicationsWithPendingCharms() + return s.watcherFactory.NewNamespaceMapperWatcher( + table, + changestream.Create|changestream.Update, + query, + mapper, + ) +} + +type indexedChanged struct { + change changestream.ChangeEvent + index int +} + // isValidApplicationName returns whether name is a valid application name. func isValidApplicationName(name string) bool { return validApplication.MatchString(name) diff --git a/domain/application/state/application.go b/domain/application/state/application.go index 729c16863fc..c1ccf55b0f6 100644 --- a/domain/application/state/application.go +++ b/domain/application/state/application.go @@ -1276,8 +1276,9 @@ WHERE application_uuid = $applicationScale.application_uuid return appScale.toScaleState(), errors.Annotatef(err, "querying application %q scale", appID) } -// GetApplicationLife looks up the life of the specified application, returning an error -// satisfying [applicationerrors.ApplicationNotFoundError] if the application is not found. +// GetApplicationLife looks up the life of the specified application, returning +// an error satisfying [applicationerrors.ApplicationNotFoundError] if the +// application is not found. func (st *State) GetApplicationLife(ctx domain.AtomicContext, appName string) (coreapplication.ID, life.Life, error) { app := applicationName{Name: appName} query := ` @@ -1325,7 +1326,8 @@ AND life_id <= $applicationID.life_id return errors.Annotatef(err, "updating application life for %q", appID) } -// SetDesiredApplicationScale updates the desired scale of the specified application. +// SetDesiredApplicationScale updates the desired scale of the specified +// application. func (st *State) SetDesiredApplicationScale(ctx domain.AtomicContext, appID coreapplication.ID, scale int) error { scaleDetails := applicationScale{ ApplicationID: appID, @@ -1346,8 +1348,8 @@ WHERE application_uuid = $applicationScale.application_uuid return errors.Trace(err) } -// SetApplicationScalingState sets the scaling details for the given caas application -// Scale is optional and is only set if not nil. +// SetApplicationScalingState sets the scaling details for the given caas +// application Scale is optional and is only set if not nil. func (st *State) SetApplicationScalingState(ctx domain.AtomicContext, appID coreapplication.ID, scale *int, targetScale int, scaling bool) error { scaleDetails := applicationScale{ ApplicationID: appID, @@ -1378,8 +1380,9 @@ WHERE application_uuid = $applicationScale.application_uuid return errors.Trace(err) } -// UpsertCloudService updates the cloud service for the specified application, returning an error -// satisfying [applicationerrors.ApplicationNotFoundError] if the application doesn't exist. +// UpsertCloudService updates the cloud service for the specified application, +// returning an error satisfying [applicationerrors.ApplicationNotFoundError] if +// the application doesn't exist. func (st *State) UpsertCloudService(ctx context.Context, name, providerID string, sAddrs network.SpaceAddresses) error { db, err := st.DB() if err != nil { @@ -1415,8 +1418,9 @@ ON CONFLICT(application_uuid) DO UPDATE type statusKeys []string -// saveStatusData saves the status key value data for the specified unit in the specified table. -// It's called from each different SaveStatus method which previously has confirmed the unit UUID exists. +// saveStatusData saves the status key value data for the specified unit in the +// specified table. It's called from each different SaveStatus method which +// previously has confirmed the unit UUID exists. func (st *State) saveStatusData(ctx context.Context, tx *sqlair.TX, table string, unitUUID coreunit.UUID, data map[string]string) error { unit := minimalUnit{UUID: unitUUID} var keys statusKeys @@ -1458,8 +1462,9 @@ ON CONFLICT(unit_uuid, key) DO UPDATE SET return nil } -// SetCloudContainerStatus saves the given cloud container status, overwriting any current status data. -// If returns an error satisfying [applicationerrors.UnitNotFound] if the unit doesn't exist. +// SetCloudContainerStatus saves the given cloud container status, overwriting +// any current status data. If returns an error satisfying +// [applicationerrors.UnitNotFound] if the unit doesn't exist. func (st *State) SetCloudContainerStatus(ctx domain.AtomicContext, unitUUID coreunit.UUID, status application.CloudContainerStatusStatusInfo) error { statusInfo := unitStatusInfo{ UnitUUID: unitUUID, @@ -1479,8 +1484,9 @@ ON CONFLICT(unit_uuid) DO UPDATE SET } err = domain.Run(ctx, func(ctx context.Context, tx *sqlair.TX) error { err = tx.Query(ctx, stmt, statusInfo).Run() - // This is purely defensive and is not expected in practice - the unitUUID - // is expected to be validated earlier in the atomic txn workflow. + // This is purely defensive and is not expected in practice - the + // unitUUID is expected to be validated earlier in the atomic txn + // workflow. if jujudb.IsErrConstraintForeignKey(err) { return fmt.Errorf("%w: %q", applicationerrors.UnitNotFound, unitUUID) } @@ -1490,8 +1496,9 @@ ON CONFLICT(unit_uuid) DO UPDATE SET return errors.Annotatef(err, "saving cloud container status for unit %q", unitUUID) } -// SetUnitAgentStatus saves the given unit agent status, overwriting any current status data. -// If returns an error satisfying [applicationerrors.UnitNotFound] if the unit doesn't exist. +// SetUnitAgentStatus saves the given unit agent status, overwriting any current +// status data. If returns an error satisfying [applicationerrors.UnitNotFound] +// if the unit doesn't exist. func (st *State) SetUnitAgentStatus(ctx domain.AtomicContext, unitUUID coreunit.UUID, status application.UnitAgentStatusInfo) error { statusInfo := unitStatusInfo{ UnitUUID: unitUUID, @@ -1522,8 +1529,9 @@ ON CONFLICT(unit_uuid) DO UPDATE SET return errors.Annotatef(err, "saving unit agent status for unit %q", unitUUID) } -// SetUnitWorkloadStatus saves the given unit workload status, overwriting any current status data. -// If returns an error satisfying [applicationerrors.UnitNotFound] if the unit doesn't exist. +// SetUnitWorkloadStatus saves the given unit workload status, overwriting any +// current status data. If returns an error satisfying +// [applicationerrors.UnitNotFound] if the unit doesn't exist. func (st *State) SetUnitWorkloadStatus(ctx domain.AtomicContext, unitUUID coreunit.UUID, status application.UnitWorkloadStatusInfo) error { statusInfo := unitStatusInfo{ UnitUUID: unitUUID, @@ -1543,8 +1551,9 @@ ON CONFLICT(unit_uuid) DO UPDATE SET } err = domain.Run(ctx, func(ctx context.Context, tx *sqlair.TX) error { err = tx.Query(ctx, stmt, statusInfo).Run() - // This is purely defensive and is not expected in practice - the unitUUID - // is expected to be validated earlier in the atomic txn workflow. + // This is purely defensive and is not expected in practice - the + // unitUUID is expected to be validated earlier in the atomic txn + // workflow. if jujudb.IsErrConstraintForeignKey(err) { return fmt.Errorf("%w: %q", applicationerrors.UnitNotFound, unitUUID) } @@ -1554,7 +1563,8 @@ ON CONFLICT(unit_uuid) DO UPDATE SET return errors.Annotatef(err, "saving unit workload status for unit %q", unitUUID) } -// InitialWatchStatementUnitLife returns the initial namespace query for the application unit life watcher. +// InitialWatchStatementUnitLife returns the initial namespace query for the +// application unit life watcher. func (st *State) InitialWatchStatementUnitLife(appName string) (string, eventsource.NamespaceQuery) { queryFunc := func(ctx context.Context, runner database.TxnRunner) ([]string, error) { app := applicationName{Name: appName} @@ -1587,8 +1597,79 @@ WHERE a.name = $applicationName.name return "unit", queryFunc } -// GetApplicationUnitLife returns the life values for the specified units of the given application. -// The supplied ids may belong to a different application; the application name is used to filter. +// InitialWatchStatementApplicationsWithPendingCharms returns the initial +// namespace query for the applications with pending charms watcher. +func (st *State) InitialWatchStatementApplicationsWithPendingCharms() (string, eventsource.NamespaceQuery) { + queryFunc := func(ctx context.Context, runner database.TxnRunner) ([]string, error) { + stmt, err := st.Prepare(` +SELECT a.uuid AS &applicationID.uuid +FROM application a +JOIN charm c ON a.charm_uuid = c.uuid +WHERE c.available = FALSE +`, applicationID{}) + if err != nil { + return nil, errors.Trace(err) + } + + var results []applicationID + err = runner.Txn(ctx, func(ctx context.Context, tx *sqlair.TX) error { + err := tx.Query(ctx, stmt).GetAll(&results) + if errors.Is(err, sqlair.ErrNoRows) { + return nil + } + return errors.Trace(err) + }) + if err != nil { + return nil, errors.Annotatef(err, "querying all applications with pending charms") + } + + return transform.Slice(results, func(r applicationID) string { + return r.ID.String() + }), nil + } + return "application", queryFunc +} + +// GetApplicationsWithPendingCharmsFromUUIDs returns the application IDs for the +// applications with pending charms from the specified UUIDs. +func (st *State) GetApplicationsWithPendingCharmsFromUUIDs(ctx context.Context, uuids []coreapplication.ID) ([]coreapplication.ID, error) { + db, err := st.DB() + if err != nil { + return nil, errors.Trace(err) + } + + type applicationIDs []coreapplication.ID + + stmt, err := st.Prepare(` +SELECT a.uuid AS &applicationID.uuid +FROM application AS a +JOIN charm AS c ON a.charm_uuid = c.uuid +WHERE a.uuid IN ($applicationIDs[:]) AND c.available = FALSE +`, applicationID{}, applicationIDs{}) + if err != nil { + return nil, errors.Trace(err) + } + + var results []applicationID + err = db.Txn(ctx, func(ctx context.Context, tx *sqlair.TX) error { + err := tx.Query(ctx, stmt, applicationIDs(uuids)).GetAll(&results) + if errors.Is(err, sqlair.ErrNoRows) { + return nil + } + return errors.Trace(err) + }) + if err != nil { + return nil, errors.Annotatef(err, "querying all applications with pending charms") + } + + return transform.Slice(results, func(r applicationID) coreapplication.ID { + return r.ID + }), nil +} + +// GetApplicationUnitLife returns the life values for the specified units of the +// given application. The supplied ids may belong to a different application; +// the application name is used to filter. func (st *State) GetApplicationUnitLife(ctx context.Context, appName string, ids ...coreunit.UUID) (map[coreunit.UUID]life.Life, error) { db, err := st.DB() if err != nil { diff --git a/domain/application/state/application_test.go b/domain/application/state/application_test.go index e3e3846d3d6..666f2ad8068 100644 --- a/domain/application/state/application_test.go +++ b/domain/application/state/application_test.go @@ -1857,6 +1857,106 @@ func (s *applicationStateSuite) TestCheckCharmExistsNotFound(c *gc.C) { c.Assert(err, jc.ErrorIs, applicationerrors.CharmNotFound) } +func (s *applicationStateSuite) TestInitialWatchStatementApplicationsWithPendingCharms(c *gc.C) { + name, query := s.state.InitialWatchStatementApplicationsWithPendingCharms() + c.Check(name, gc.Equals, "application") + + id := s.createApplication(c, "foo", life.Alive) + + result, err := query(context.Background(), s.TxnRunner()) + c.Assert(err, jc.ErrorIsNil) + c.Check(result, jc.SameContents, []string{id.String()}) +} + +func (s *applicationStateSuite) TestInitialWatchStatementApplicationsWithPendingCharmsIfAvailable(c *gc.C) { + // These use the same charm, so once you set one applications charm, you + // set both. + + name, query := s.state.InitialWatchStatementApplicationsWithPendingCharms() + c.Check(name, gc.Equals, "application") + + _ = s.createApplication(c, "foo", life.Alive) + id1 := s.createApplication(c, "bar", life.Alive) + + err := s.TxnRunner().StdTxn(context.Background(), func(ctx context.Context, tx *sql.Tx) error { + _, err := tx.ExecContext(ctx, ` +UPDATE charm SET available = TRUE +FROM application AS a +INNER JOIN charm AS c ON a.charm_uuid = c.uuid +WHERE a.uuid=?`, id1.String()) + return err + }) + c.Assert(err, jc.ErrorIsNil) + + result, err := query(context.Background(), s.TxnRunner()) + c.Assert(err, jc.ErrorIsNil) + c.Check(result, gc.HasLen, 0) +} + +func (s *applicationStateSuite) TestInitialWatchStatementApplicationsWithPendingCharmsNothing(c *gc.C) { + name, query := s.state.InitialWatchStatementApplicationsWithPendingCharms() + c.Check(name, gc.Equals, "application") + + result, err := query(context.Background(), s.TxnRunner()) + c.Assert(err, jc.ErrorIsNil) + c.Check(result, gc.HasLen, 0) +} + +func (s *applicationStateSuite) TestGetApplicationsWithPendingCharmsFromUUIDsIfPending(c *gc.C) { + id := s.createApplication(c, "foo", life.Alive) + + expected, err := s.state.GetApplicationsWithPendingCharmsFromUUIDs(context.Background(), []coreapplication.ID{id}) + c.Assert(err, jc.ErrorIsNil) + c.Check(expected, jc.DeepEquals, []coreapplication.ID{id}) +} + +func (s *applicationStateSuite) TestGetApplicationsWithPendingCharmsFromUUIDsIfAvailable(c *gc.C) { + id := s.createApplication(c, "foo", life.Alive) + + err := s.TxnRunner().StdTxn(context.Background(), func(ctx context.Context, tx *sql.Tx) error { + _, err := tx.ExecContext(ctx, ` +UPDATE charm SET available = TRUE +FROM application AS a +INNER JOIN charm AS c ON a.charm_uuid = c.uuid +WHERE a.uuid=?`, id.String()) + return err + }) + c.Assert(err, jc.ErrorIsNil) + + expected, err := s.state.GetApplicationsWithPendingCharmsFromUUIDs(context.Background(), []coreapplication.ID{id}) + c.Assert(err, jc.ErrorIsNil) + c.Check(expected, gc.HasLen, 0) +} + +func (s *applicationStateSuite) TestGetApplicationsWithPendingCharmsFromUUIDsNotFound(c *gc.C) { + expected, err := s.state.GetApplicationsWithPendingCharmsFromUUIDs(context.Background(), []coreapplication.ID{"foo"}) + c.Assert(err, jc.ErrorIsNil) + c.Check(expected, gc.HasLen, 0) +} + +func (s *applicationStateSuite) TestGetApplicationsWithPendingCharmsFromUUIDsForSameCharm(c *gc.C) { + // These use the same charm, so once you set one applications charm, you + // set both. + + id0 := s.createApplication(c, "foo", life.Alive) + id1 := s.createApplication(c, "bar", life.Alive) + + err := s.TxnRunner().StdTxn(context.Background(), func(ctx context.Context, tx *sql.Tx) error { + _, err := tx.ExecContext(ctx, ` +UPDATE charm SET available = TRUE +FROM application AS a +INNER JOIN charm AS c ON a.charm_uuid = c.uuid +WHERE a.uuid=?`, id1.String()) + return err + }) + c.Assert(err, jc.ErrorIsNil) + + expected, err := s.state.GetApplicationsWithPendingCharmsFromUUIDs(context.Background(), []coreapplication.ID{id0, id1}) + c.Assert(err, jc.ErrorIsNil) + + c.Check(expected, gc.HasLen, 0) +} + func (s *applicationStateSuite) assertApplication( c *gc.C, name string, diff --git a/domain/application/watcher_test.go b/domain/application/watcher_test.go index dee543834df..68824891da1 100644 --- a/domain/application/watcher_test.go +++ b/domain/application/watcher_test.go @@ -93,22 +93,6 @@ func (s *watcherSuite) TestWatchCharm(c *gc.C) { harness.Run(c, []string(nil)) } -func (s *watcherSuite) createApplication(c *gc.C, svc *service.WatchableService, name string, units ...service.AddUnitArg) coreapplication.ID { - ctx := context.Background() - appID, err := svc.CreateApplication(ctx, name, &stubCharm{}, corecharm.Origin{ - Source: corecharm.CharmHub, - Platform: corecharm.Platform{ - Channel: "24.04", - OS: "ubuntu", - Architecture: "amd64", - }, - }, service.AddApplicationArgs{ - ReferenceName: name, - }, units...) - c.Assert(err, jc.ErrorIsNil) - return appID -} - func (s *watcherSuite) TestWatchUnitLife(c *gc.C) { factory := changestream.NewWatchableDBFactoryForNamespace(s.GetWatchableDB, "unit") @@ -389,6 +373,35 @@ func (s *watcherSuite) TestWatchApplicationScale(c *gc.C) { harness.Run(c, struct{}{}) } +func (s *watcherSuite) TestWatchApplicationsWithPendingCharms(c *gc.C) { + factory := changestream.NewWatchableDBFactoryForNamespace(s.GetWatchableDB, "application") + + svc := s.setupService(c, factory) + + id0 := s.createApplication(c, svc, "foo") + id1 := s.createApplication(c, svc, "bar") + + ctx := context.Background() + watcher, err := svc.WatchApplicationsWithPendingCharms(ctx) + c.Assert(err, jc.ErrorIsNil) + + harness := watchertest.NewHarness[[]string](s, watchertest.NewWatcherC[[]string](c, watcher)) + + harness.AddTest(func(c *gc.C) { + }, func(w watchertest.WatcherC[[]string]) { + // No change should occur after the initial setup. + w.AssertNoChange() + }) + + harness.AddTest(func(c *gc.C) { + + }, func(w watchertest.WatcherC[[]string]) { + w.AssertNoChange() + }) + + harness.Run(c, []string{id0.String(), id1.String()}) +} + func (s *watcherSuite) setupService(c *gc.C, factory domain.WatchableDBFactory) *service.WatchableService { return service.NewWatchableService( state.NewState(func() (database.TxnRunner, error) { return s.ModelTxnRunner(), nil }, @@ -408,6 +421,26 @@ func (s *watcherSuite) setupService(c *gc.C, factory domain.WatchableDBFactory) ) } +func (s *watcherSuite) createApplication(c *gc.C, svc *service.WatchableService, name string, units ...service.AddUnitArg) coreapplication.ID { + return s.createApplicationWithCharm(c, svc, name, &stubCharm{}, units...) +} + +func (s *watcherSuite) createApplicationWithCharm(c *gc.C, svc *service.WatchableService, name string, charm internalcharm.Charm, units ...service.AddUnitArg) coreapplication.ID { + ctx := context.Background() + appID, err := svc.CreateApplication(ctx, name, charm, corecharm.Origin{ + Source: corecharm.CharmHub, + Platform: corecharm.Platform{ + Channel: "24.04", + OS: "ubuntu", + Architecture: "amd64", + }, + }, service.AddApplicationArgs{ + ReferenceName: name, + }, units...) + c.Assert(err, jc.ErrorIsNil) + return appID +} + type stubCharm struct{} func (s *stubCharm) Meta() *internalcharm.Meta { From c5a11db6e49ad23cfa8819fb0473f043d1bd62ea Mon Sep 17 00:00:00 2001 From: Simon Richardson Date: Tue, 19 Nov 2024 15:36:08 +0000 Subject: [PATCH 2/7] feat: wire up pending and available state for charm If a charm has a storage path (will be replaced with a object store UUID) then the charm will be marked as available. The charm is no longer a pending charm. --- domain/application/service/application.go | 9 +- domain/application/service/params.go | 2 + domain/application/state/application.go | 2 +- domain/application/state/application_test.go | 48 +++- domain/application/state/charm_test.go | 38 ++++ domain/application/state/state.go | 2 + domain/application/state/types.go | 1 + domain/application/types.go | 11 +- domain/application/watcher_test.go | 74 +++++- domain/schema/model.go | 7 +- .../model/triggers/application-scale.gen.go | 49 ---- .../triggers/application-triggers.gen.go | 213 ++++++++++++++++++ domain/schema/model/triggers/charm.gen.go | 48 ---- .../schema/model/triggers/port-range.gen.go | 51 ----- domain/schema/model/triggers/unit.gen.go | 54 ----- 15 files changed, 377 insertions(+), 232 deletions(-) delete mode 100644 domain/schema/model/triggers/application-scale.gen.go create mode 100644 domain/schema/model/triggers/application-triggers.gen.go delete mode 100644 domain/schema/model/triggers/charm.gen.go delete mode 100644 domain/schema/model/triggers/port-range.gen.go delete mode 100644 domain/schema/model/triggers/unit.gen.go diff --git a/domain/application/service/application.go b/domain/application/service/application.go index 668bfee1696..4945cd2c21a 100644 --- a/domain/application/service/application.go +++ b/domain/application/service/application.go @@ -349,10 +349,11 @@ func makeCreateApplicationArgs( } return application.AddApplicationArg{ - Charm: ch, - Platform: platformArg, - Origin: originArg, - Channel: channelArg, + Charm: ch, + Platform: platformArg, + Origin: originArg, + Channel: channelArg, + CharmStoragePath: args.CharmStoragePath, }, nil } diff --git a/domain/application/service/params.go b/domain/application/service/params.go index 1a5a1f0da7b..912ec9e2c79 100644 --- a/domain/application/service/params.go +++ b/domain/application/service/params.go @@ -27,6 +27,8 @@ type AddApplicationArgs struct { // that specific revision. The only way to then locate the charm directly // via the name is use the proxy name. ReferenceName string + // CharmStoragePath is the path to the charm in the storage. + CharmStoragePath string // Storage contains the application's storage directives. Storage map[string]storage.Directive } diff --git a/domain/application/state/application.go b/domain/application/state/application.go index c1ccf55b0f6..56402385142 100644 --- a/domain/application/state/application.go +++ b/domain/application/state/application.go @@ -191,7 +191,7 @@ func (st *State) CreateApplication(ctx domain.AtomicContext, name string, app ap // platforms for a charm, or just remove the charm_platform table // altogether. We can do a reverse lookup from the application // to the installed charm architecture. - if err := st.setCharm(ctx, tx, charmID, app.Charm, ""); err != nil { + if err := st.setCharm(ctx, tx, charmID, app.Charm, app.CharmStoragePath); err != nil { return errors.Annotate(err, "setting charm") } if err := tx.Query(ctx, createOriginStmt, originInfo).Run(); err != nil { diff --git a/domain/application/state/application_test.go b/domain/application/state/application_test.go index 666f2ad8068..214265306e6 100644 --- a/domain/application/state/application_test.go +++ b/domain/application/state/application_test.go @@ -102,7 +102,7 @@ func (s *applicationStateSuite) TestCreateApplication(c *gc.C) { }) c.Assert(err, jc.ErrorIsNil) scale := application.ScaleState{Scale: 1} - s.assertApplication(c, "666", platform, channel, scale, origin) + s.assertApplication(c, "666", platform, channel, scale, origin, false) } func (s *applicationStateSuite) TestCreateApplicationsWithSameCharm(c *gc.C) { @@ -150,8 +150,8 @@ func (s *applicationStateSuite) TestCreateApplicationsWithSameCharm(c *gc.C) { c.Assert(err, jc.ErrorIsNil) scale := application.ScaleState{} - s.assertApplication(c, "foo1", platform, channel, scale, origin) - s.assertApplication(c, "foo2", platform, channel, scale, origin) + s.assertApplication(c, "foo1", platform, channel, scale, origin, false) + s.assertApplication(c, "foo2", platform, channel, scale, origin, false) } func (s *applicationStateSuite) TestCreateApplicationWithoutChannel(c *gc.C) { @@ -180,7 +180,7 @@ func (s *applicationStateSuite) TestCreateApplicationWithoutChannel(c *gc.C) { }) c.Assert(err, jc.ErrorIsNil) scale := application.ScaleState{Scale: 1} - s.assertApplication(c, "666", platform, nil, scale, origin) + s.assertApplication(c, "666", platform, nil, scale, origin, false) } func (s *applicationStateSuite) TestCreateApplicationWithEmptyChannel(c *gc.C) { @@ -209,7 +209,37 @@ func (s *applicationStateSuite) TestCreateApplicationWithEmptyChannel(c *gc.C) { }) c.Assert(err, jc.ErrorIsNil) scale := application.ScaleState{Scale: 1} - s.assertApplication(c, "666", platform, channel, scale, origin) + s.assertApplication(c, "666", platform, channel, scale, origin, false) +} + +func (s *applicationStateSuite) TestCreateApplicationWithCharmStoragePath(c *gc.C) { + platform := application.Platform{ + Channel: "666", + OSType: charm.Ubuntu, + Architecture: charm.ARM64, + } + channel := &application.Channel{} + origin := charm.CharmOrigin{ + Source: charm.LocalSource, + Revision: 42, + } + err := s.state.RunAtomic(context.Background(), func(ctx domain.AtomicContext) error { + _, err := s.state.CreateApplication(ctx, "666", application.AddApplicationArg{ + Platform: platform, + Origin: origin, + Charm: charm.Charm{ + Metadata: charm.Metadata{ + Name: "666", + }, + }, + CharmStoragePath: "some-path", + Scale: 1, + }) + return err + }) + c.Assert(err, jc.ErrorIsNil) + scale := application.ScaleState{Scale: 1} + s.assertApplication(c, "666", platform, channel, scale, origin, true) } func (s *applicationStateSuite) TestGetApplicationLife(c *gc.C) { @@ -1964,6 +1994,7 @@ func (s *applicationStateSuite) assertApplication( channel *application.Channel, scale application.ScaleState, origin charm.CharmOrigin, + available bool, ) { var ( gotName string @@ -1973,6 +2004,7 @@ func (s *applicationStateSuite) assertApplication( gotChannel application.Channel gotScale application.ScaleState gotOrigin charm.CharmOrigin + gotAvailable bool ) err := s.TxnRunner().StdTxn(context.Background(), func(ctx context.Context, tx *sql.Tx) error { err := tx.QueryRowContext(ctx, "SELECT uuid, charm_uuid, name FROM application WHERE name=?", name).Scan(&gotUUID, &gotCharmUUID, &gotName) @@ -1999,7 +2031,10 @@ func (s *applicationStateSuite) assertApplication( if err != nil { return err } - + err = tx.QueryRowContext(ctx, "SELECT available FROM charm WHERE uuid=?", gotCharmUUID).Scan(&gotAvailable) + if err != nil { + return err + } return nil }) c.Assert(err, jc.ErrorIsNil) @@ -2007,6 +2042,7 @@ func (s *applicationStateSuite) assertApplication( c.Check(gotPlatform, jc.DeepEquals, platform) c.Check(gotScale, jc.DeepEquals, scale) c.Check(gotOrigin, gc.DeepEquals, origin) + c.Check(gotAvailable, gc.Equals, available) // Channel is optional, so we need to check it separately. if channel != nil { diff --git a/domain/application/state/charm_test.go b/domain/application/state/charm_test.go index f4a5f43de03..166ad1da48f 100644 --- a/domain/application/state/charm_test.go +++ b/domain/application/state/charm_test.go @@ -88,9 +88,47 @@ func (s *charmStateSuite) TestGetCharmIDLocalCharm(c *gc.C) { c.Check(charmID, gc.Equals, id) } +func (s *charmStateSuite) TestSetCharmNotAvailable(c *gc.C) { + st := NewState(s.TxnRunnerFactory(), loggertesting.WrapCheckLog(c)) + + expected := charm.Metadata{ + Name: "foo", + Summary: "summary", + Description: "description", + Subordinate: true, + RunAs: charm.RunAsRoot, + MinJujuVersion: version.MustParse("4.0.0"), + Assumes: []byte("null"), + } + + // The archive path is empty, so it's not immediately available. + + id, err := st.SetCharm(context.Background(), charm.Charm{ + Metadata: expected, + }, charm.SetStateArgs{ + Source: charm.LocalSource, + Revision: 42, + ReferenceName: "foo", + Hash: "hash", + Version: "deadbeef", + }) + c.Assert(err, jc.ErrorIsNil) + + charmID, err := st.GetCharmID(context.Background(), "foo", 42, charm.LocalSource) + c.Assert(err, jc.ErrorIsNil) + c.Check(charmID, gc.Equals, id) + + available, err := st.IsCharmAvailable(context.Background(), id) + c.Assert(err, jc.ErrorIsNil) + c.Check(available, jc.IsFalse) +} + func (s *charmStateSuite) TestSetCharmGetCharmID(c *gc.C) { st := NewState(s.TxnRunnerFactory(), loggertesting.WrapCheckLog(c)) + // The archive path is not empty because setStateArgs sets it to a + // value, which means that the charm is available. + expected := charm.Metadata{ Name: "foo", Summary: "summary", diff --git a/domain/application/state/state.go b/domain/application/state/state.go index db53e82967d..c97ddfc92be 100644 --- a/domain/application/state/state.go +++ b/domain/application/state/state.go @@ -152,6 +152,7 @@ func (s *State) setCharm(ctx context.Context, tx *sqlair.TX, id corecharm.ID, ch if err := s.setCharmManifest(ctx, tx, id, charm.Manifest); err != nil { return errors.Trace(err) } + return nil } @@ -164,6 +165,7 @@ func (s *State) setCharmState( data := setCharm{ UUID: id.String(), ArchivePath: archivePath, + Available: archivePath != "", } query := `INSERT INTO charm (*) VALUES ($setCharm.*);` diff --git a/domain/application/state/types.go b/domain/application/state/types.go index e9bc88d0c81..962f1d1d898 100644 --- a/domain/application/state/types.go +++ b/domain/application/state/types.go @@ -225,6 +225,7 @@ type setCharmHash struct { type setCharm struct { UUID string `db:"uuid"` ArchivePath string `db:"archive_path"` + Available bool `db:"available"` } // setInitialCharmOrigin is used to set the reference_name, source, revision diff --git a/domain/application/types.go b/domain/application/types.go index 8e04a7ec505..f061f9db923 100644 --- a/domain/application/types.go +++ b/domain/application/types.go @@ -14,11 +14,12 @@ import ( // AddApplicationArg contains parameters for saving an application to state. type AddApplicationArg struct { - Charm domaincharm.Charm - Origin domaincharm.CharmOrigin - Scale int - Platform Platform - Channel *Channel + Charm domaincharm.Charm + Origin domaincharm.CharmOrigin + Scale int + Platform Platform + Channel *Channel + CharmStoragePath string } // Channel represents the channel of a application charm. diff --git a/domain/application/watcher_test.go b/domain/application/watcher_test.go index 68824891da1..bf5d81c7ede 100644 --- a/domain/application/watcher_test.go +++ b/domain/application/watcher_test.go @@ -378,28 +378,75 @@ func (s *watcherSuite) TestWatchApplicationsWithPendingCharms(c *gc.C) { svc := s.setupService(c, factory) - id0 := s.createApplication(c, svc, "foo") - id1 := s.createApplication(c, svc, "bar") - ctx := context.Background() watcher, err := svc.WatchApplicationsWithPendingCharms(ctx) c.Assert(err, jc.ErrorIsNil) harness := watchertest.NewHarness[[]string](s, watchertest.NewWatcherC[[]string](c, watcher)) + var id0, id1 coreapplication.ID + harness.AddTest(func(c *gc.C) { + id0 = s.createApplication(c, svc, "foo") + id1 = s.createApplication(c, svc, "bar") + }, func(w watchertest.WatcherC[[]string]) { + w.Check( + watchertest.StringSliceAssert[string](id0.String(), id1.String()), + ) + }) + + // Updating the charm doesn't emit an event. + harness.AddTest(func(c *gc.C) { + db, err := factory() + c.Assert(err, jc.ErrorIsNil) + + err = db.StdTxn(context.Background(), func(ctx context.Context, tx *sql.Tx) error { + _, err := tx.ExecContext(ctx, ` +UPDATE charm SET available = TRUE +FROM application AS a +INNER JOIN charm AS c ON a.charm_uuid = c.uuid +WHERE a.uuid=?`, id0.String()) + return err + }) + c.Assert(err, jc.ErrorIsNil) + }, func(w watchertest.WatcherC[[]string]) { + w.AssertNoChange() + }) + + // Updating the application doesn't emit an event. harness.AddTest(func(c *gc.C) { + db, err := factory() + c.Assert(err, jc.ErrorIsNil) + + err = db.StdTxn(context.Background(), func(ctx context.Context, tx *sql.Tx) error { + _, err := tx.ExecContext(ctx, ` +UPDATE application SET charm_modified_version = 1 +WHERE uuid=?`, id0.String()) + return err + }) + c.Assert(err, jc.ErrorIsNil) }, func(w watchertest.WatcherC[[]string]) { - // No change should occur after the initial setup. w.AssertNoChange() }) + // Add another application with a pending charm. + var id2 coreapplication.ID harness.AddTest(func(c *gc.C) { + id2 = s.createApplication(c, svc, "baz") + }, func(w watchertest.WatcherC[[]string]) { + w.Check( + watchertest.StringSliceAssert[string](id2.String()), + ) + }) + // Add another application with a available charm. + // Available charms are not pending charms! + harness.AddTest(func(c *gc.C) { + id2 = s.createApplicationWithCharmAndStoragePath(c, svc, "jaz", &stubCharm{}, "deadbeef", service.AddUnitArg{}) }, func(w watchertest.WatcherC[[]string]) { w.AssertNoChange() }) - harness.Run(c, []string{id0.String(), id1.String()}) + harness.Run(c, []string{}) } func (s *watcherSuite) setupService(c *gc.C, factory domain.WatchableDBFactory) *service.WatchableService { @@ -422,10 +469,10 @@ func (s *watcherSuite) setupService(c *gc.C, factory domain.WatchableDBFactory) } func (s *watcherSuite) createApplication(c *gc.C, svc *service.WatchableService, name string, units ...service.AddUnitArg) coreapplication.ID { - return s.createApplicationWithCharm(c, svc, name, &stubCharm{}, units...) + return s.createApplicationWithCharmAndStoragePath(c, svc, name, &stubCharm{}, "", units...) } -func (s *watcherSuite) createApplicationWithCharm(c *gc.C, svc *service.WatchableService, name string, charm internalcharm.Charm, units ...service.AddUnitArg) coreapplication.ID { +func (s *watcherSuite) createApplicationWithCharmAndStoragePath(c *gc.C, svc *service.WatchableService, name string, charm internalcharm.Charm, storagePath string, units ...service.AddUnitArg) coreapplication.ID { ctx := context.Background() appID, err := svc.CreateApplication(ctx, name, charm, corecharm.Origin{ Source: corecharm.CharmHub, @@ -435,17 +482,24 @@ func (s *watcherSuite) createApplicationWithCharm(c *gc.C, svc *service.Watchabl Architecture: "amd64", }, }, service.AddApplicationArgs{ - ReferenceName: name, + ReferenceName: name, + CharmStoragePath: storagePath, }, units...) c.Assert(err, jc.ErrorIsNil) return appID } -type stubCharm struct{} +type stubCharm struct { + name string +} func (s *stubCharm) Meta() *internalcharm.Meta { + name := s.name + if name == "" { + name = "test" + } return &internalcharm.Meta{ - Name: "test", + Name: name, } } diff --git a/domain/schema/model.go b/domain/schema/model.go index 9258438f222..0726c560f0c 100644 --- a/domain/schema/model.go +++ b/domain/schema/model.go @@ -20,10 +20,7 @@ import ( //go:generate go run ./../../generate/triggergen -db=model -destination=./model/triggers/machine-triggers.gen.go -package=triggers -tables=machine,machine_lxd_profile //go:generate go run ./../../generate/triggergen -db=model -destination=./model/triggers/machine-cloud-instance-triggers.gen.go -package=triggers -tables=machine_cloud_instance //go:generate go run ./../../generate/triggergen -db=model -destination=./model/triggers/machine-requires-reboot-triggers.gen.go -package=triggers -tables=machine_requires_reboot -//go:generate go run ./../../generate/triggergen -db=model -destination=./model/triggers/charm.gen.go -package=triggers -tables=charm -//go:generate go run ./../../generate/triggergen -db=model -destination=./model/triggers/unit.gen.go -package=triggers -tables=unit -//go:generate go run ./../../generate/triggergen -db=model -destination=./model/triggers/application-scale.gen.go -package=triggers -tables=application_scale -//go:generate go run ./../../generate/triggergen -db=model -destination=./model/triggers/port-range.gen.go -package=triggers -tables=port_range +//go:generate go run ./../../generate/triggergen -db=model -destination=./model/triggers/application-triggers.gen.go -package=triggers -tables=application,charm,unit,application_scale,port_range //go:embed model/sql/*.sql var modelSchemaDir embed.FS @@ -54,6 +51,7 @@ const ( tableApplicationScale tablePortRange tableSecretDeletedValueRef + tableApplication ) // ModelDDL is used to create model databases. @@ -114,6 +112,7 @@ func ModelDDL() *schema.Schema { triggers.ChangeLogTriggersForApplicationScale("application_uuid", tableApplicationScale), triggers.ChangeLogTriggersForPortRange("unit_uuid", tablePortRange), triggers.ChangeLogTriggersForSecretDeletedValueRef("revision_uuid", tableSecretDeletedValueRef), + triggers.ChangeLogTriggersForApplication("uuid", tableApplication), ) // Generic triggers. diff --git a/domain/schema/model/triggers/application-scale.gen.go b/domain/schema/model/triggers/application-scale.gen.go deleted file mode 100644 index 9daee9d95a8..00000000000 --- a/domain/schema/model/triggers/application-scale.gen.go +++ /dev/null @@ -1,49 +0,0 @@ -// Code generated by triggergen. DO NOT EDIT. - -package triggers - -import ( - "fmt" - - "github.com/juju/juju/core/database/schema" -) - - -// ChangeLogTriggersForApplicationScale generates the triggers for the -// application_scale table. -func ChangeLogTriggersForApplicationScale(columnName string, namespaceID int) func() schema.Patch { - return func() schema.Patch { - return schema.MakePatch(fmt.Sprintf(` --- insert namespace for ApplicationScale -INSERT INTO change_log_namespace VALUES (%[2]d, 'application_scale', 'ApplicationScale changes based on %[1]s'); - --- insert trigger for ApplicationScale -CREATE TRIGGER trg_log_application_scale_insert -AFTER INSERT ON application_scale FOR EACH ROW -BEGIN - INSERT INTO change_log (edit_type_id, namespace_id, changed, created_at) - VALUES (1, %[2]d, NEW.%[1]s, DATETIME('now')); -END; - --- update trigger for ApplicationScale -CREATE TRIGGER trg_log_application_scale_update -AFTER UPDATE ON application_scale FOR EACH ROW -WHEN - NEW.application_uuid != OLD.application_uuid OR - (NEW.scale != OLD.scale OR (NEW.scale IS NOT NULL AND OLD.scale IS NULL) OR (NEW.scale IS NULL AND OLD.scale IS NOT NULL)) OR - (NEW.scale_target != OLD.scale_target OR (NEW.scale_target IS NOT NULL AND OLD.scale_target IS NULL) OR (NEW.scale_target IS NULL AND OLD.scale_target IS NOT NULL)) OR - (NEW.scaling != OLD.scaling OR (NEW.scaling IS NOT NULL AND OLD.scaling IS NULL) OR (NEW.scaling IS NULL AND OLD.scaling IS NOT NULL)) -BEGIN - INSERT INTO change_log (edit_type_id, namespace_id, changed, created_at) - VALUES (2, %[2]d, OLD.%[1]s, DATETIME('now')); -END; --- delete trigger for ApplicationScale -CREATE TRIGGER trg_log_application_scale_delete -AFTER DELETE ON application_scale FOR EACH ROW -BEGIN - INSERT INTO change_log (edit_type_id, namespace_id, changed, created_at) - VALUES (4, %[2]d, OLD.%[1]s, DATETIME('now')); -END;`, columnName, namespaceID)) - } -} - diff --git a/domain/schema/model/triggers/application-triggers.gen.go b/domain/schema/model/triggers/application-triggers.gen.go new file mode 100644 index 00000000000..d1a242597cf --- /dev/null +++ b/domain/schema/model/triggers/application-triggers.gen.go @@ -0,0 +1,213 @@ +// Code generated by triggergen. DO NOT EDIT. + +package triggers + +import ( + "fmt" + + "github.com/juju/juju/core/database/schema" +) + + +// ChangeLogTriggersForApplication generates the triggers for the +// application table. +func ChangeLogTriggersForApplication(columnName string, namespaceID int) func() schema.Patch { + return func() schema.Patch { + return schema.MakePatch(fmt.Sprintf(` +-- insert namespace for Application +INSERT INTO change_log_namespace VALUES (%[2]d, 'application', 'Application changes based on %[1]s'); + +-- insert trigger for Application +CREATE TRIGGER trg_log_application_insert +AFTER INSERT ON application FOR EACH ROW +BEGIN + INSERT INTO change_log (edit_type_id, namespace_id, changed, created_at) + VALUES (1, %[2]d, NEW.%[1]s, DATETIME('now')); +END; + +-- update trigger for Application +CREATE TRIGGER trg_log_application_update +AFTER UPDATE ON application FOR EACH ROW +WHEN + NEW.uuid != OLD.uuid OR + NEW.name != OLD.name OR + NEW.life_id != OLD.life_id OR + NEW.charm_uuid != OLD.charm_uuid OR + (NEW.charm_modified_version != OLD.charm_modified_version OR (NEW.charm_modified_version IS NOT NULL AND OLD.charm_modified_version IS NULL) OR (NEW.charm_modified_version IS NULL AND OLD.charm_modified_version IS NOT NULL)) OR + (NEW.charm_upgrade_on_error != OLD.charm_upgrade_on_error OR (NEW.charm_upgrade_on_error IS NOT NULL AND OLD.charm_upgrade_on_error IS NULL) OR (NEW.charm_upgrade_on_error IS NULL AND OLD.charm_upgrade_on_error IS NOT NULL)) OR + (NEW.exposed != OLD.exposed OR (NEW.exposed IS NOT NULL AND OLD.exposed IS NULL) OR (NEW.exposed IS NULL AND OLD.exposed IS NOT NULL)) OR + (NEW.placement != OLD.placement OR (NEW.placement IS NOT NULL AND OLD.placement IS NULL) OR (NEW.placement IS NULL AND OLD.placement IS NOT NULL)) OR + (NEW.password_hash_algorithm_id != OLD.password_hash_algorithm_id OR (NEW.password_hash_algorithm_id IS NOT NULL AND OLD.password_hash_algorithm_id IS NULL) OR (NEW.password_hash_algorithm_id IS NULL AND OLD.password_hash_algorithm_id IS NOT NULL)) OR + (NEW.password_hash != OLD.password_hash OR (NEW.password_hash IS NOT NULL AND OLD.password_hash IS NULL) OR (NEW.password_hash IS NULL AND OLD.password_hash IS NOT NULL)) +BEGIN + INSERT INTO change_log (edit_type_id, namespace_id, changed, created_at) + VALUES (2, %[2]d, OLD.%[1]s, DATETIME('now')); +END; +-- delete trigger for Application +CREATE TRIGGER trg_log_application_delete +AFTER DELETE ON application FOR EACH ROW +BEGIN + INSERT INTO change_log (edit_type_id, namespace_id, changed, created_at) + VALUES (4, %[2]d, OLD.%[1]s, DATETIME('now')); +END;`, columnName, namespaceID)) + } +} + +// ChangeLogTriggersForApplicationScale generates the triggers for the +// application_scale table. +func ChangeLogTriggersForApplicationScale(columnName string, namespaceID int) func() schema.Patch { + return func() schema.Patch { + return schema.MakePatch(fmt.Sprintf(` +-- insert namespace for ApplicationScale +INSERT INTO change_log_namespace VALUES (%[2]d, 'application_scale', 'ApplicationScale changes based on %[1]s'); + +-- insert trigger for ApplicationScale +CREATE TRIGGER trg_log_application_scale_insert +AFTER INSERT ON application_scale FOR EACH ROW +BEGIN + INSERT INTO change_log (edit_type_id, namespace_id, changed, created_at) + VALUES (1, %[2]d, NEW.%[1]s, DATETIME('now')); +END; + +-- update trigger for ApplicationScale +CREATE TRIGGER trg_log_application_scale_update +AFTER UPDATE ON application_scale FOR EACH ROW +WHEN + NEW.application_uuid != OLD.application_uuid OR + (NEW.scale != OLD.scale OR (NEW.scale IS NOT NULL AND OLD.scale IS NULL) OR (NEW.scale IS NULL AND OLD.scale IS NOT NULL)) OR + (NEW.scale_target != OLD.scale_target OR (NEW.scale_target IS NOT NULL AND OLD.scale_target IS NULL) OR (NEW.scale_target IS NULL AND OLD.scale_target IS NOT NULL)) OR + (NEW.scaling != OLD.scaling OR (NEW.scaling IS NOT NULL AND OLD.scaling IS NULL) OR (NEW.scaling IS NULL AND OLD.scaling IS NOT NULL)) +BEGIN + INSERT INTO change_log (edit_type_id, namespace_id, changed, created_at) + VALUES (2, %[2]d, OLD.%[1]s, DATETIME('now')); +END; +-- delete trigger for ApplicationScale +CREATE TRIGGER trg_log_application_scale_delete +AFTER DELETE ON application_scale FOR EACH ROW +BEGIN + INSERT INTO change_log (edit_type_id, namespace_id, changed, created_at) + VALUES (4, %[2]d, OLD.%[1]s, DATETIME('now')); +END;`, columnName, namespaceID)) + } +} + +// ChangeLogTriggersForCharm generates the triggers for the +// charm table. +func ChangeLogTriggersForCharm(columnName string, namespaceID int) func() schema.Patch { + return func() schema.Patch { + return schema.MakePatch(fmt.Sprintf(` +-- insert namespace for Charm +INSERT INTO change_log_namespace VALUES (%[2]d, 'charm', 'Charm changes based on %[1]s'); + +-- insert trigger for Charm +CREATE TRIGGER trg_log_charm_insert +AFTER INSERT ON charm FOR EACH ROW +BEGIN + INSERT INTO change_log (edit_type_id, namespace_id, changed, created_at) + VALUES (1, %[2]d, NEW.%[1]s, DATETIME('now')); +END; + +-- update trigger for Charm +CREATE TRIGGER trg_log_charm_update +AFTER UPDATE ON charm FOR EACH ROW +WHEN + NEW.uuid != OLD.uuid OR + (NEW.archive_path != OLD.archive_path OR (NEW.archive_path IS NOT NULL AND OLD.archive_path IS NULL) OR (NEW.archive_path IS NULL AND OLD.archive_path IS NOT NULL)) OR + (NEW.available != OLD.available OR (NEW.available IS NOT NULL AND OLD.available IS NULL) OR (NEW.available IS NULL AND OLD.available IS NOT NULL)) +BEGIN + INSERT INTO change_log (edit_type_id, namespace_id, changed, created_at) + VALUES (2, %[2]d, OLD.%[1]s, DATETIME('now')); +END; +-- delete trigger for Charm +CREATE TRIGGER trg_log_charm_delete +AFTER DELETE ON charm FOR EACH ROW +BEGIN + INSERT INTO change_log (edit_type_id, namespace_id, changed, created_at) + VALUES (4, %[2]d, OLD.%[1]s, DATETIME('now')); +END;`, columnName, namespaceID)) + } +} + +// ChangeLogTriggersForPortRange generates the triggers for the +// port_range table. +func ChangeLogTriggersForPortRange(columnName string, namespaceID int) func() schema.Patch { + return func() schema.Patch { + return schema.MakePatch(fmt.Sprintf(` +-- insert namespace for PortRange +INSERT INTO change_log_namespace VALUES (%[2]d, 'port_range', 'PortRange changes based on %[1]s'); + +-- insert trigger for PortRange +CREATE TRIGGER trg_log_port_range_insert +AFTER INSERT ON port_range FOR EACH ROW +BEGIN + INSERT INTO change_log (edit_type_id, namespace_id, changed, created_at) + VALUES (1, %[2]d, NEW.%[1]s, DATETIME('now')); +END; + +-- update trigger for PortRange +CREATE TRIGGER trg_log_port_range_update +AFTER UPDATE ON port_range FOR EACH ROW +WHEN + NEW.uuid != OLD.uuid OR + NEW.protocol_id != OLD.protocol_id OR + (NEW.from_port != OLD.from_port OR (NEW.from_port IS NOT NULL AND OLD.from_port IS NULL) OR (NEW.from_port IS NULL AND OLD.from_port IS NOT NULL)) OR + (NEW.to_port != OLD.to_port OR (NEW.to_port IS NOT NULL AND OLD.to_port IS NULL) OR (NEW.to_port IS NULL AND OLD.to_port IS NOT NULL)) OR + (NEW.relation_uuid != OLD.relation_uuid OR (NEW.relation_uuid IS NOT NULL AND OLD.relation_uuid IS NULL) OR (NEW.relation_uuid IS NULL AND OLD.relation_uuid IS NOT NULL)) OR + NEW.unit_uuid != OLD.unit_uuid +BEGIN + INSERT INTO change_log (edit_type_id, namespace_id, changed, created_at) + VALUES (2, %[2]d, OLD.%[1]s, DATETIME('now')); +END; +-- delete trigger for PortRange +CREATE TRIGGER trg_log_port_range_delete +AFTER DELETE ON port_range FOR EACH ROW +BEGIN + INSERT INTO change_log (edit_type_id, namespace_id, changed, created_at) + VALUES (4, %[2]d, OLD.%[1]s, DATETIME('now')); +END;`, columnName, namespaceID)) + } +} + +// ChangeLogTriggersForUnit generates the triggers for the +// unit table. +func ChangeLogTriggersForUnit(columnName string, namespaceID int) func() schema.Patch { + return func() schema.Patch { + return schema.MakePatch(fmt.Sprintf(` +-- insert namespace for Unit +INSERT INTO change_log_namespace VALUES (%[2]d, 'unit', 'Unit changes based on %[1]s'); + +-- insert trigger for Unit +CREATE TRIGGER trg_log_unit_insert +AFTER INSERT ON unit FOR EACH ROW +BEGIN + INSERT INTO change_log (edit_type_id, namespace_id, changed, created_at) + VALUES (1, %[2]d, NEW.%[1]s, DATETIME('now')); +END; + +-- update trigger for Unit +CREATE TRIGGER trg_log_unit_update +AFTER UPDATE ON unit FOR EACH ROW +WHEN + NEW.uuid != OLD.uuid OR + NEW.name != OLD.name OR + NEW.life_id != OLD.life_id OR + NEW.application_uuid != OLD.application_uuid OR + NEW.net_node_uuid != OLD.net_node_uuid OR + (NEW.charm_uuid != OLD.charm_uuid OR (NEW.charm_uuid IS NOT NULL AND OLD.charm_uuid IS NULL) OR (NEW.charm_uuid IS NULL AND OLD.charm_uuid IS NOT NULL)) OR + NEW.resolve_kind_id != OLD.resolve_kind_id OR + (NEW.password_hash_algorithm_id != OLD.password_hash_algorithm_id OR (NEW.password_hash_algorithm_id IS NOT NULL AND OLD.password_hash_algorithm_id IS NULL) OR (NEW.password_hash_algorithm_id IS NULL AND OLD.password_hash_algorithm_id IS NOT NULL)) OR + (NEW.password_hash != OLD.password_hash OR (NEW.password_hash IS NOT NULL AND OLD.password_hash IS NULL) OR (NEW.password_hash IS NULL AND OLD.password_hash IS NOT NULL)) +BEGIN + INSERT INTO change_log (edit_type_id, namespace_id, changed, created_at) + VALUES (2, %[2]d, OLD.%[1]s, DATETIME('now')); +END; +-- delete trigger for Unit +CREATE TRIGGER trg_log_unit_delete +AFTER DELETE ON unit FOR EACH ROW +BEGIN + INSERT INTO change_log (edit_type_id, namespace_id, changed, created_at) + VALUES (4, %[2]d, OLD.%[1]s, DATETIME('now')); +END;`, columnName, namespaceID)) + } +} + diff --git a/domain/schema/model/triggers/charm.gen.go b/domain/schema/model/triggers/charm.gen.go deleted file mode 100644 index d03042aafa0..00000000000 --- a/domain/schema/model/triggers/charm.gen.go +++ /dev/null @@ -1,48 +0,0 @@ -// Code generated by triggergen. DO NOT EDIT. - -package triggers - -import ( - "fmt" - - "github.com/juju/juju/core/database/schema" -) - - -// ChangeLogTriggersForCharm generates the triggers for the -// charm table. -func ChangeLogTriggersForCharm(columnName string, namespaceID int) func() schema.Patch { - return func() schema.Patch { - return schema.MakePatch(fmt.Sprintf(` --- insert namespace for Charm -INSERT INTO change_log_namespace VALUES (%[2]d, 'charm', 'Charm changes based on %[1]s'); - --- insert trigger for Charm -CREATE TRIGGER trg_log_charm_insert -AFTER INSERT ON charm FOR EACH ROW -BEGIN - INSERT INTO change_log (edit_type_id, namespace_id, changed, created_at) - VALUES (1, %[2]d, NEW.%[1]s, DATETIME('now')); -END; - --- update trigger for Charm -CREATE TRIGGER trg_log_charm_update -AFTER UPDATE ON charm FOR EACH ROW -WHEN - NEW.uuid != OLD.uuid OR - (NEW.archive_path != OLD.archive_path OR (NEW.archive_path IS NOT NULL AND OLD.archive_path IS NULL) OR (NEW.archive_path IS NULL AND OLD.archive_path IS NOT NULL)) OR - (NEW.available != OLD.available OR (NEW.available IS NOT NULL AND OLD.available IS NULL) OR (NEW.available IS NULL AND OLD.available IS NOT NULL)) -BEGIN - INSERT INTO change_log (edit_type_id, namespace_id, changed, created_at) - VALUES (2, %[2]d, OLD.%[1]s, DATETIME('now')); -END; --- delete trigger for Charm -CREATE TRIGGER trg_log_charm_delete -AFTER DELETE ON charm FOR EACH ROW -BEGIN - INSERT INTO change_log (edit_type_id, namespace_id, changed, created_at) - VALUES (4, %[2]d, OLD.%[1]s, DATETIME('now')); -END;`, columnName, namespaceID)) - } -} - diff --git a/domain/schema/model/triggers/port-range.gen.go b/domain/schema/model/triggers/port-range.gen.go deleted file mode 100644 index f6db9cfb3d7..00000000000 --- a/domain/schema/model/triggers/port-range.gen.go +++ /dev/null @@ -1,51 +0,0 @@ -// Code generated by triggergen. DO NOT EDIT. - -package triggers - -import ( - "fmt" - - "github.com/juju/juju/core/database/schema" -) - - -// ChangeLogTriggersForPortRange generates the triggers for the -// port_range table. -func ChangeLogTriggersForPortRange(columnName string, namespaceID int) func() schema.Patch { - return func() schema.Patch { - return schema.MakePatch(fmt.Sprintf(` --- insert namespace for PortRange -INSERT INTO change_log_namespace VALUES (%[2]d, 'port_range', 'PortRange changes based on %[1]s'); - --- insert trigger for PortRange -CREATE TRIGGER trg_log_port_range_insert -AFTER INSERT ON port_range FOR EACH ROW -BEGIN - INSERT INTO change_log (edit_type_id, namespace_id, changed, created_at) - VALUES (1, %[2]d, NEW.%[1]s, DATETIME('now')); -END; - --- update trigger for PortRange -CREATE TRIGGER trg_log_port_range_update -AFTER UPDATE ON port_range FOR EACH ROW -WHEN - NEW.uuid != OLD.uuid OR - NEW.protocol_id != OLD.protocol_id OR - (NEW.from_port != OLD.from_port OR (NEW.from_port IS NOT NULL AND OLD.from_port IS NULL) OR (NEW.from_port IS NULL AND OLD.from_port IS NOT NULL)) OR - (NEW.to_port != OLD.to_port OR (NEW.to_port IS NOT NULL AND OLD.to_port IS NULL) OR (NEW.to_port IS NULL AND OLD.to_port IS NOT NULL)) OR - (NEW.relation_uuid != OLD.relation_uuid OR (NEW.relation_uuid IS NOT NULL AND OLD.relation_uuid IS NULL) OR (NEW.relation_uuid IS NULL AND OLD.relation_uuid IS NOT NULL)) OR - NEW.unit_uuid != OLD.unit_uuid -BEGIN - INSERT INTO change_log (edit_type_id, namespace_id, changed, created_at) - VALUES (2, %[2]d, OLD.%[1]s, DATETIME('now')); -END; --- delete trigger for PortRange -CREATE TRIGGER trg_log_port_range_delete -AFTER DELETE ON port_range FOR EACH ROW -BEGIN - INSERT INTO change_log (edit_type_id, namespace_id, changed, created_at) - VALUES (4, %[2]d, OLD.%[1]s, DATETIME('now')); -END;`, columnName, namespaceID)) - } -} - diff --git a/domain/schema/model/triggers/unit.gen.go b/domain/schema/model/triggers/unit.gen.go deleted file mode 100644 index 0094270a7dd..00000000000 --- a/domain/schema/model/triggers/unit.gen.go +++ /dev/null @@ -1,54 +0,0 @@ -// Code generated by triggergen. DO NOT EDIT. - -package triggers - -import ( - "fmt" - - "github.com/juju/juju/core/database/schema" -) - - -// ChangeLogTriggersForUnit generates the triggers for the -// unit table. -func ChangeLogTriggersForUnit(columnName string, namespaceID int) func() schema.Patch { - return func() schema.Patch { - return schema.MakePatch(fmt.Sprintf(` --- insert namespace for Unit -INSERT INTO change_log_namespace VALUES (%[2]d, 'unit', 'Unit changes based on %[1]s'); - --- insert trigger for Unit -CREATE TRIGGER trg_log_unit_insert -AFTER INSERT ON unit FOR EACH ROW -BEGIN - INSERT INTO change_log (edit_type_id, namespace_id, changed, created_at) - VALUES (1, %[2]d, NEW.%[1]s, DATETIME('now')); -END; - --- update trigger for Unit -CREATE TRIGGER trg_log_unit_update -AFTER UPDATE ON unit FOR EACH ROW -WHEN - NEW.uuid != OLD.uuid OR - NEW.name != OLD.name OR - NEW.life_id != OLD.life_id OR - NEW.application_uuid != OLD.application_uuid OR - NEW.net_node_uuid != OLD.net_node_uuid OR - (NEW.charm_uuid != OLD.charm_uuid OR (NEW.charm_uuid IS NOT NULL AND OLD.charm_uuid IS NULL) OR (NEW.charm_uuid IS NULL AND OLD.charm_uuid IS NOT NULL)) OR - NEW.resolve_kind_id != OLD.resolve_kind_id OR - (NEW.password_hash_algorithm_id != OLD.password_hash_algorithm_id OR (NEW.password_hash_algorithm_id IS NOT NULL AND OLD.password_hash_algorithm_id IS NULL) OR (NEW.password_hash_algorithm_id IS NULL AND OLD.password_hash_algorithm_id IS NOT NULL)) OR - (NEW.password_hash != OLD.password_hash OR (NEW.password_hash IS NOT NULL AND OLD.password_hash IS NULL) OR (NEW.password_hash IS NULL AND OLD.password_hash IS NOT NULL)) -BEGIN - INSERT INTO change_log (edit_type_id, namespace_id, changed, created_at) - VALUES (2, %[2]d, OLD.%[1]s, DATETIME('now')); -END; --- delete trigger for Unit -CREATE TRIGGER trg_log_unit_delete -AFTER DELETE ON unit FOR EACH ROW -BEGIN - INSERT INTO change_log (edit_type_id, namespace_id, changed, created_at) - VALUES (4, %[2]d, OLD.%[1]s, DATETIME('now')); -END;`, columnName, namespaceID)) - } -} - From 6b1aea71a1936354498b5504502037418caa11cb Mon Sep 17 00:00:00 2001 From: Simon Richardson Date: Fri, 22 Nov 2024 12:12:48 +0000 Subject: [PATCH 3/7] fix: add model triggers --- domain/schema/schema_test.go | 62 +++++++++++++++++++----------------- 1 file changed, 33 insertions(+), 29 deletions(-) diff --git a/domain/schema/schema_test.go b/domain/schema/schema_test.go index bb7b611b2e9..4b8f5a6c9dd 100644 --- a/domain/schema/schema_test.go +++ b/domain/schema/schema_test.go @@ -613,6 +613,14 @@ func (s *schemaSuite) TestModelTriggers(c *gc.C) { // Expected changelog triggers. Additional triggers are not included and // can be added to the addition list. expected := set.NewStrings( + "trg_log_application_delete", + "trg_log_application_insert", + "trg_log_application_update", + + "trg_log_application_scale_delete", + "trg_log_application_scale_insert", + "trg_log_application_scale_update", + "trg_log_block_device_delete", "trg_log_block_device_insert", "trg_log_block_device_update", @@ -621,6 +629,22 @@ func (s *schemaSuite) TestModelTriggers(c *gc.C) { "trg_log_charm_insert", "trg_log_charm_update", + "trg_log_machine_cloud_instance_delete", + "trg_log_machine_cloud_instance_insert", + "trg_log_machine_cloud_instance_update", + + "trg_log_machine_delete", + "trg_log_machine_insert", + "trg_log_machine_update", + + "trg_log_machine_lxd_profile_delete", + "trg_log_machine_lxd_profile_insert", + "trg_log_machine_lxd_profile_update", + + "trg_log_machine_requires_reboot_delete", + "trg_log_machine_requires_reboot_insert", + "trg_log_machine_requires_reboot_update", + "trg_log_model_config_delete", "trg_log_model_config_insert", "trg_log_model_config_update", @@ -629,6 +653,14 @@ func (s *schemaSuite) TestModelTriggers(c *gc.C) { "trg_log_object_store_metadata_path_insert", "trg_log_object_store_metadata_path_update", + "trg_log_port_range_delete", + "trg_log_port_range_insert", + "trg_log_port_range_update", + + "trg_log_secret_deleted_value_ref_delete", + "trg_log_secret_deleted_value_ref_insert", + "trg_log_secret_deleted_value_ref_update", + "trg_log_secret_metadata_delete", "trg_log_secret_metadata_insert", "trg_log_secret_metadata_update", @@ -637,10 +669,6 @@ func (s *schemaSuite) TestModelTriggers(c *gc.C) { "trg_log_secret_reference_insert", "trg_log_secret_reference_update", - "trg_log_secret_deleted_value_ref_delete", - "trg_log_secret_deleted_value_ref_insert", - "trg_log_secret_deleted_value_ref_update", - "trg_log_secret_revision_delete", "trg_log_secret_revision_insert", "trg_log_secret_revision_update", @@ -685,33 +713,9 @@ func (s *schemaSuite) TestModelTriggers(c *gc.C) { "trg_log_subnet_insert", "trg_log_subnet_update", - "trg_log_machine_insert", - "trg_log_machine_update", - "trg_log_machine_delete", - - "trg_log_machine_lxd_profile_insert", - "trg_log_machine_lxd_profile_update", - "trg_log_machine_lxd_profile_delete", - - "trg_log_machine_cloud_instance_insert", - "trg_log_machine_cloud_instance_update", - "trg_log_machine_cloud_instance_delete", - - "trg_log_machine_requires_reboot_insert", - "trg_log_machine_requires_reboot_update", - "trg_log_machine_requires_reboot_delete", - + "trg_log_unit_delete", "trg_log_unit_insert", "trg_log_unit_update", - "trg_log_unit_delete", - - "trg_log_application_scale_insert", - "trg_log_application_scale_update", - "trg_log_application_scale_delete", - - "trg_log_port_range_insert", - "trg_log_port_range_update", - "trg_log_port_range_delete", ) // These are additional triggers that are not change log triggers, but From da60a253f553261b6087955336a8263835472741 Mon Sep 17 00:00:00 2001 From: Simon Richardson Date: Fri, 22 Nov 2024 12:45:43 +0000 Subject: [PATCH 4/7] test: watch applications with pending charms The trigger watches all applications, not just the ones with pending charms. We need to ensure that the order the changes went in are the same that come out (there is a dependency chain here!). The following just tests that with applications that are no longer pending charms and out of order sequence. --- .../application/service/application_test.go | 268 +++++++++++++++++- domain/application/service/package_test.go | 21 ++ domain/application/service/service.go | 108 +++---- 3 files changed, 342 insertions(+), 55 deletions(-) diff --git a/domain/application/service/application_test.go b/domain/application/service/application_test.go index 4405cd32445..6049926fd3f 100644 --- a/domain/application/service/application_test.go +++ b/domain/application/service/application_test.go @@ -5,20 +5,29 @@ package service import ( "context" + "math/rand/v2" "time" + "github.com/juju/clock/testclock" jujuerrors "github.com/juju/errors" + "github.com/juju/testing" jc "github.com/juju/testing/checkers" "github.com/juju/version/v2" "go.uber.org/mock/gomock" gc "gopkg.in/check.v1" + coreapplication "github.com/juju/juju/core/application" applicationtesting "github.com/juju/juju/core/application/testing" "github.com/juju/juju/core/assumes" + "github.com/juju/juju/core/changestream" corecharm "github.com/juju/juju/core/charm" charmtesting "github.com/juju/juju/core/charm/testing" + "github.com/juju/juju/core/model" + modeltesting "github.com/juju/juju/core/model/testing" + corestorage "github.com/juju/juju/core/storage" coreunit "github.com/juju/juju/core/unit" unittesting "github.com/juju/juju/core/unit/testing" + "github.com/juju/juju/domain" "github.com/juju/juju/domain/application" domaincharm "github.com/juju/juju/domain/application/charm" applicationerrors "github.com/juju/juju/domain/application/errors" @@ -28,7 +37,11 @@ import ( domaintesting "github.com/juju/juju/domain/testing" "github.com/juju/juju/internal/charm" "github.com/juju/juju/internal/errors" + loggertesting "github.com/juju/juju/internal/logger/testing" "github.com/juju/juju/internal/storage" + "github.com/juju/juju/internal/storage/provider" + dummystorage "github.com/juju/juju/internal/storage/provider/dummy" + internaltesting "github.com/juju/juju/internal/testing" ) type applicationServiceSuite struct { @@ -1014,13 +1027,243 @@ func (s *applicationServiceSuite) TestGetCharmModifiedVersion(c *gc.C) { c.Check(obtained, gc.DeepEquals, 42) } -type providerApplicationServiceSuite struct { +type applicationWatcherServiceSuite struct { + testing.IsolationSuite + + service *WatchableService + + state *MockState + charm *MockCharm + secret *MockDeleteSecretState + clock *testclock.Clock + watcherFactory *MockWatcherFactory +} + +var _ = gc.Suite(&applicationWatcherServiceSuite{}) + +func (s *applicationWatcherServiceSuite) TestWatchApplicationsWithPendingCharmMapper(c *gc.C) { + defer s.setupMocks(c).Finish() + + // There is an integration test to ensure correct wire up. This test ensures + // that the mapper correctly orders the results based on changes emitted by + // the watcher. + + appID := applicationtesting.GenApplicationUUID(c) + + s.state.EXPECT().GetApplicationsWithPendingCharmsFromUUIDs(gomock.Any(), []coreapplication.ID{appID}).Return([]coreapplication.ID{ + appID, + }, nil) + + changes := []changestream.ChangeEvent{&changeEvent{ + typ: changestream.All, + namespace: "application", + changed: appID.String(), + }} + + result, err := s.service.watchApplicationsWithPendingCharmsMapper(context.Background(), changes) + c.Assert(err, jc.ErrorIsNil) + c.Check(result, gc.DeepEquals, changes) +} + +func (s *applicationWatcherServiceSuite) TestWatchApplicationsWithPendingCharmMapperInvalidID(c *gc.C) { + defer s.setupMocks(c).Finish() + + // There is an integration test to ensure correct wire up. This test ensures + // that the mapper correctly orders the results based on changes emitted by + // the watcher. + + changes := []changestream.ChangeEvent{&changeEvent{ + typ: changestream.All, + namespace: "application", + changed: "foo", + }} + + _, err := s.service.watchApplicationsWithPendingCharmsMapper(context.Background(), changes) + c.Assert(err, jc.ErrorIs, jujuerrors.NotValid) +} + +func (s *applicationWatcherServiceSuite) TestWatchApplicationsWithPendingCharmMapperOrder(c *gc.C) { + defer s.setupMocks(c).Finish() + + // There is an integration test to ensure correct wire up. This test ensures + // that the mapper correctly orders the results based on changes emitted by + // the watcher. + + appIDs := make([]coreapplication.ID, 4) + for i := range appIDs { + appIDs[i] = applicationtesting.GenApplicationUUID(c) + } + + changes := make([]changestream.ChangeEvent, len(appIDs)) + for i, appID := range appIDs { + changes[i] = &changeEvent{ + typ: changestream.All, + namespace: "application", + changed: appID.String(), + } + } + + // Ensure order is persvered if the state returns the uuids in an unexpected + // order. This is because we can't guarantee the order if there are holes in + // the pending sequence. + + shuffle := make([]coreapplication.ID, len(appIDs)) + copy(shuffle, appIDs) + rand.Shuffle(len(shuffle), func(i, j int) { + shuffle[i], shuffle[j] = shuffle[j], shuffle[i] + }) + + s.state.EXPECT().GetApplicationsWithPendingCharmsFromUUIDs(gomock.Any(), appIDs).Return(shuffle, nil) + + result, err := s.service.watchApplicationsWithPendingCharmsMapper(context.Background(), changes) + c.Assert(err, jc.ErrorIsNil) + c.Check(result, gc.DeepEquals, changes) +} + +func (s *applicationWatcherServiceSuite) TestWatchApplicationsWithPendingCharmMapperDropped(c *gc.C) { + defer s.setupMocks(c).Finish() + + // There is an integration test to ensure correct wire up. This test ensures + // that the mapper correctly orders the results based on changes emitted by + // the watcher. + + appIDs := make([]coreapplication.ID, 10) + for i := range appIDs { + appIDs[i] = applicationtesting.GenApplicationUUID(c) + } + + changes := make([]changestream.ChangeEvent, len(appIDs)) + for i, appID := range appIDs { + changes[i] = &changeEvent{ + typ: changestream.All, + namespace: "application", + changed: appID.String(), + } + } + + // Ensure order is persvered if the state returns the uuids in an unexpected + // order. This is because we can't guarantee the order if there are holes in + // the pending sequence. + + var dropped []coreapplication.ID + var expected []changestream.ChangeEvent + for i, appID := range appIDs { + if rand.IntN(2) == 0 { + continue + } + dropped = append(dropped, appID) + expected = append(expected, changes[i]) + } + + s.state.EXPECT().GetApplicationsWithPendingCharmsFromUUIDs(gomock.Any(), appIDs).Return(dropped, nil) + + result, err := s.service.watchApplicationsWithPendingCharmsMapper(context.Background(), changes) + c.Assert(err, jc.ErrorIsNil) + c.Check(result, gc.DeepEquals, expected) +} + +func (s *applicationWatcherServiceSuite) TestWatchApplicationsWithPendingCharmMapperOrderDropped(c *gc.C) { + defer s.setupMocks(c).Finish() + + // There is an integration test to ensure correct wire up. This test ensures + // that the mapper correctly orders the results based on changes emitted by + // the watcher. + + appIDs := make([]coreapplication.ID, 10) + for i := range appIDs { + appIDs[i] = applicationtesting.GenApplicationUUID(c) + } + + changes := make([]changestream.ChangeEvent, len(appIDs)) + for i, appID := range appIDs { + changes[i] = &changeEvent{ + typ: changestream.All, + namespace: "application", + changed: appID.String(), + } + } + + // Ensure order is persvered if the state returns the uuids in an unexpected + // order. This is because we can't guarantee the order if there are holes in + // the pending sequence. + + var dropped []coreapplication.ID + var expected []changestream.ChangeEvent + for i, appID := range appIDs { + if rand.IntN(2) == 0 { + continue + } + dropped = append(dropped, appID) + expected = append(expected, changes[i]) + } + + // Shuffle them to replicate out of order return. + + shuffle := make([]coreapplication.ID, len(dropped)) + copy(shuffle, dropped) + rand.Shuffle(len(shuffle), func(i, j int) { + shuffle[i], shuffle[j] = shuffle[j], shuffle[i] + }) + + s.state.EXPECT().GetApplicationsWithPendingCharmsFromUUIDs(gomock.Any(), appIDs).Return(shuffle, nil) + + result, err := s.service.watchApplicationsWithPendingCharmsMapper(context.Background(), changes) + c.Assert(err, jc.ErrorIsNil) + c.Check(result, gc.DeepEquals, expected) +} + +func (s *applicationWatcherServiceSuite) setupMocks(c *gc.C) *gomock.Controller { + ctrl := gomock.NewController(c) + + s.state = NewMockState(ctrl) + s.charm = NewMockCharm(ctrl) + s.secret = NewMockDeleteSecretState(ctrl) + s.watcherFactory = NewMockWatcherFactory(ctrl) + + registry := corestorage.ConstModelStorageRegistry(func() storage.ProviderRegistry { + return storage.ChainedProviderRegistry{ + dummystorage.StorageProviders(), + provider.CommonStorageProviders(), + } + }) + + modelUUID := modeltesting.GenModelUUID(c) + + s.clock = testclock.NewClock(time.Time{}) + s.service = NewWatchableService( + s.state, + s.secret, + registry, + modelUUID, + s.watcherFactory, + nil, + nil, + s.clock, + loggertesting.WrapCheckLog(c), + ) + s.service.clock = s.clock + + s.state.EXPECT().RunAtomic(gomock.Any(), gomock.Any()).DoAndReturn(func(ctx context.Context, fn func(ctx domain.AtomicContext) error) error { + return fn(domaintesting.NewAtomicContext(ctx)) + }).AnyTimes() + + return ctrl +} + +type providerServiceSuite struct { baseSuite + + service *ProviderService + + modelID model.UUID + + agentVersionGetter *MockAgentVersionGetter + provider *MockProvider } -var _ = gc.Suite(&providerApplicationServiceSuite{}) +var _ = gc.Suite(&providerServiceSuite{}) -func (s *providerApplicationServiceSuite) TestGetSupportedFeatures(c *gc.C) { +func (s *providerServiceSuite) TestGetSupportedFeatures(c *gc.C) { defer s.setupMocks(c).Finish() agentVersion := version.MustParse("4.0.0") @@ -1040,7 +1283,7 @@ func (s *providerApplicationServiceSuite) TestGetSupportedFeatures(c *gc.C) { c.Check(features, jc.DeepEquals, fs) } -func (s *providerApplicationServiceSuite) TestGetSupportedFeaturesNotSupported(c *gc.C) { +func (s *providerServiceSuite) TestGetSupportedFeaturesNotSupported(c *gc.C) { ctrl := s.setupMocksWithProvider(c, func(ctx context.Context) (Provider, error) { return s.provider, jujuerrors.NotSupported }) @@ -1060,3 +1303,20 @@ func (s *providerApplicationServiceSuite) TestGetSupportedFeaturesNotSupported(c }) c.Check(features, jc.DeepEquals, fs) } + +func (s *providerServiceSuite) setupMocksWithProvider(c *gc.C, fn func(ctx context.Context) (Provider, error)) *gomock.Controller { + ctrl := gomock.NewController(c) + + s.modelID = model.UUID(internaltesting.ModelTag.Id()) + + s.agentVersionGetter = NewMockAgentVersionGetter(ctrl) + s.provider = NewMockProvider(ctrl) + + s.service = &ProviderService{ + modelID: s.modelID, + agentVersionGetter: s.agentVersionGetter, + provider: fn, + } + + return ctrl +} diff --git a/domain/application/service/package_test.go b/domain/application/service/package_test.go index 0dd5ae386b8..07af2781c72 100644 --- a/domain/application/service/package_test.go +++ b/domain/application/service/package_test.go @@ -14,6 +14,7 @@ import ( "go.uber.org/mock/gomock" gc "gopkg.in/check.v1" + "github.com/juju/juju/core/changestream" "github.com/juju/juju/core/model" modeltesting "github.com/juju/juju/core/model/testing" corestorage "github.com/juju/juju/core/storage" @@ -135,3 +136,23 @@ func (s *baseSuite) setupMocksWithAtomic(c *gc.C, fn func(domain.AtomicContext) func ptr[T any](v T) *T { return &v } + +type changeEvent struct { + typ changestream.ChangeType + namespace string + changed string +} + +var _ changestream.ChangeEvent = (*changeEvent)(nil) + +func (c *changeEvent) Type() changestream.ChangeType { + return c.typ +} + +func (c *changeEvent) Namespace() string { + return c.namespace +} + +func (c *changeEvent) Changed() string { + return c.changed +} diff --git a/domain/application/service/service.go b/domain/application/service/service.go index 7a6527b1f5a..1ac9b6ec505 100644 --- a/domain/application/service/service.go +++ b/domain/application/service/service.go @@ -247,72 +247,78 @@ func (s *WatchableService) WatchApplicationScale(ctx context.Context, appName st // WatchApplicationsWithPendingCharms returns a watcher that observes changes to // applications that have pending charms. func (s *WatchableService) WatchApplicationsWithPendingCharms(ctx context.Context) (watcher.StringsWatcher, error) { - mapper := func(ctx context.Context, db coredatabase.TxnRunner, changes []changestream.ChangeEvent) ([]changestream.ChangeEvent, error) { - // Preserve the ordering of the changes, as this is a strings watcher - // and we want to return the changes in the order they were received. - - appChanges := make(map[coreapplication.ID][]indexedChanged) - uuids := make([]coreapplication.ID, 0) - for i, change := range changes { - appID, err := coreapplication.ParseID(change.Changed()) - if err != nil { - return nil, err - } - if _, ok := appChanges[appID]; !ok { - uuids = append(uuids, appID) - } + table, query := s.st.InitialWatchStatementApplicationsWithPendingCharms() + return s.watcherFactory.NewNamespaceMapperWatcher( + table, + changestream.Create, + query, + func(ctx context.Context, _ coredatabase.TxnRunner, changes []changestream.ChangeEvent) ([]changestream.ChangeEvent, error) { + return s.watchApplicationsWithPendingCharmsMapper(ctx, changes) + }, + ) +} - appChanges[appID] = append(appChanges[appID], indexedChanged{ - change: change, - index: i, - }) - } +// watchApplicationsWithPendingCharmsMapper removes any applications that do not +// have pending charms. +func (s *WatchableService) watchApplicationsWithPendingCharmsMapper(ctx context.Context, changes []changestream.ChangeEvent) ([]changestream.ChangeEvent, error) { + // Preserve the ordering of the changes, as this is a strings watcher + // and we want to return the changes in the order they were received. - // Get all the applications with pending charms using the uuids. - apps, err := s.GetApplicationsWithPendingCharmsFromUUIDs(ctx, uuids) + appChanges := make(map[coreapplication.ID][]indexedChanged) + uuids := make([]coreapplication.ID, 0) + for i, change := range changes { + appID, err := coreapplication.ParseID(change.Changed()) if err != nil { return nil, err } - // If any applications have no pending charms, then return no changes. - if len(apps) == 0 { - return nil, nil + if _, ok := appChanges[appID]; !ok { + uuids = append(uuids, appID) } - // Grab all the changes for the applications with pending charms, - // ensuring they're indexed so we can sort them later. - var indexed []indexedChanged - for _, appID := range apps { - events, ok := appChanges[appID] - if !ok { - s.logger.Errorf("application %q has pending charms but no change events", appID) - continue - } + appChanges[appID] = append(appChanges[appID], indexedChanged{ + change: change, + index: i, + }) + } - indexed = append(indexed, events...) - } + // Get all the applications with pending charms using the uuids. + apps, err := s.GetApplicationsWithPendingCharmsFromUUIDs(ctx, uuids) + if err != nil { + return nil, err + } - // Sort the index so they're preserved - sort.Slice(indexed, func(i, j int) bool { - return indexed[i].index < indexed[j].index - }) + // If any applications have no pending charms, then return no changes. + if len(apps) == 0 { + return nil, nil + } - // Grab the changes in the order they were received. - var results []changestream.ChangeEvent - for i, result := range indexed { - results[i] = result.change + // Grab all the changes for the applications with pending charms, + // ensuring they're indexed so we can sort them later. + var indexed []indexedChanged + for _, appID := range apps { + events, ok := appChanges[appID] + if !ok { + s.logger.Errorf("application %q has pending charms but no change events", appID) + continue } - return results, nil + indexed = append(indexed, events...) } - table, query := s.st.InitialWatchStatementApplicationsWithPendingCharms() - return s.watcherFactory.NewNamespaceMapperWatcher( - table, - changestream.Create|changestream.Update, - query, - mapper, - ) + + // Sort the index so they're preserved + sort.Slice(indexed, func(i, j int) bool { + return indexed[i].index < indexed[j].index + }) + + // Grab the changes in the order they were received. + var results []changestream.ChangeEvent + for _, result := range indexed { + results = append(results, result.change) + } + + return results, nil } type indexedChanged struct { From feb456d1d91768e47dd9831967d035b9c028cbf7 Mon Sep 17 00:00:00 2001 From: Simon Richardson Date: Tue, 26 Nov 2024 14:50:58 +0000 Subject: [PATCH 5/7] test: clean up test suite hierarchy --- .../application/service/application_test.go | 26 ------------------- domain/application/watcher_test.go | 12 ++++----- 2 files changed, 6 insertions(+), 32 deletions(-) diff --git a/domain/application/service/application_test.go b/domain/application/service/application_test.go index 6049926fd3f..c9933b99255 100644 --- a/domain/application/service/application_test.go +++ b/domain/application/service/application_test.go @@ -22,7 +22,6 @@ import ( "github.com/juju/juju/core/changestream" corecharm "github.com/juju/juju/core/charm" charmtesting "github.com/juju/juju/core/charm/testing" - "github.com/juju/juju/core/model" modeltesting "github.com/juju/juju/core/model/testing" corestorage "github.com/juju/juju/core/storage" coreunit "github.com/juju/juju/core/unit" @@ -41,7 +40,6 @@ import ( "github.com/juju/juju/internal/storage" "github.com/juju/juju/internal/storage/provider" dummystorage "github.com/juju/juju/internal/storage/provider/dummy" - internaltesting "github.com/juju/juju/internal/testing" ) type applicationServiceSuite struct { @@ -1252,13 +1250,6 @@ func (s *applicationWatcherServiceSuite) setupMocks(c *gc.C) *gomock.Controller type providerServiceSuite struct { baseSuite - - service *ProviderService - - modelID model.UUID - - agentVersionGetter *MockAgentVersionGetter - provider *MockProvider } var _ = gc.Suite(&providerServiceSuite{}) @@ -1303,20 +1294,3 @@ func (s *providerServiceSuite) TestGetSupportedFeaturesNotSupported(c *gc.C) { }) c.Check(features, jc.DeepEquals, fs) } - -func (s *providerServiceSuite) setupMocksWithProvider(c *gc.C, fn func(ctx context.Context) (Provider, error)) *gomock.Controller { - ctrl := gomock.NewController(c) - - s.modelID = model.UUID(internaltesting.ModelTag.Id()) - - s.agentVersionGetter = NewMockAgentVersionGetter(ctrl) - s.provider = NewMockProvider(ctrl) - - s.service = &ProviderService{ - modelID: s.modelID, - agentVersionGetter: s.agentVersionGetter, - provider: fn, - } - - return ctrl -} diff --git a/domain/application/watcher_test.go b/domain/application/watcher_test.go index bf5d81c7ede..1083616d119 100644 --- a/domain/application/watcher_test.go +++ b/domain/application/watcher_test.go @@ -450,13 +450,13 @@ WHERE uuid=?`, id0.String()) } func (s *watcherSuite) setupService(c *gc.C, factory domain.WatchableDBFactory) *service.WatchableService { + modelDB := func() (database.TxnRunner, error) { + return s.ModelTxnRunner(), nil + } + return service.NewWatchableService( - state.NewState(func() (database.TxnRunner, error) { return s.ModelTxnRunner(), nil }, - loggertesting.WrapCheckLog(c), - ), - secretstate.NewState(func() (database.TxnRunner, error) { return s.ModelTxnRunner(), nil }, - loggertesting.WrapCheckLog(c), - ), + state.NewState(modelDB, loggertesting.WrapCheckLog(c)), + secretstate.NewState(modelDB, loggertesting.WrapCheckLog(c)), corestorage.ConstModelStorageRegistry(func() storage.ProviderRegistry { return provider.CommonStorageProviders() }), From 2728106c6576a633258ca996f10f57c1a594ee17 Mon Sep 17 00:00:00 2001 From: Simon Richardson Date: Tue, 26 Nov 2024 15:17:27 +0000 Subject: [PATCH 6/7] chore: rename error message to show intent --- domain/application/state/application.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/domain/application/state/application.go b/domain/application/state/application.go index 56402385142..39f2b070efe 100644 --- a/domain/application/state/application.go +++ b/domain/application/state/application.go @@ -1620,7 +1620,7 @@ WHERE c.available = FALSE return errors.Trace(err) }) if err != nil { - return nil, errors.Annotatef(err, "querying all applications with pending charms") + return nil, errors.Annotatef(err, "querying requested applications that have pending charms") } return transform.Slice(results, func(r applicationID) string { @@ -1659,7 +1659,7 @@ WHERE a.uuid IN ($applicationIDs[:]) AND c.available = FALSE return errors.Trace(err) }) if err != nil { - return nil, errors.Annotatef(err, "querying all applications with pending charms") + return nil, errors.Annotatef(err, "querying requested applications that have pending charms") } return transform.Slice(results, func(r applicationID) coreapplication.ID { From cf7d7e356140cc7cb3d49161ca4f2c44022ecdc7 Mon Sep 17 00:00:00 2001 From: Simon Richardson Date: Wed, 27 Nov 2024 10:24:11 +0000 Subject: [PATCH 7/7] fix: typo --- domain/application/service/application_test.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/domain/application/service/application_test.go b/domain/application/service/application_test.go index c9933b99255..d43f48f092b 100644 --- a/domain/application/service/application_test.go +++ b/domain/application/service/application_test.go @@ -1101,7 +1101,7 @@ func (s *applicationWatcherServiceSuite) TestWatchApplicationsWithPendingCharmMa } } - // Ensure order is persvered if the state returns the uuids in an unexpected + // Ensure order is preserved if the state returns the uuids in an unexpected // order. This is because we can't guarantee the order if there are holes in // the pending sequence. @@ -1139,7 +1139,7 @@ func (s *applicationWatcherServiceSuite) TestWatchApplicationsWithPendingCharmMa } } - // Ensure order is persvered if the state returns the uuids in an unexpected + // Ensure order is preserved if the state returns the uuids in an unexpected // order. This is because we can't guarantee the order if there are holes in // the pending sequence. @@ -1181,7 +1181,7 @@ func (s *applicationWatcherServiceSuite) TestWatchApplicationsWithPendingCharmMa } } - // Ensure order is persvered if the state returns the uuids in an unexpected + // Ensure order is preserved if the state returns the uuids in an unexpected // order. This is because we can't guarantee the order if there are holes in // the pending sequence.