diff --git a/domain/application/service/charm.go b/domain/application/service/charm.go index d805c5b6eb0..7dbf01a2219 100644 --- a/domain/application/service/charm.go +++ b/domain/application/service/charm.go @@ -126,7 +126,7 @@ type CharmState interface { // SetCharm persists the charm metadata, actions, config and manifest to // state. - SetCharm(ctx context.Context, charm charm.Charm, downloadInfo *charm.DownloadInfo) (corecharm.ID, error) + SetCharm(ctx context.Context, charm charm.Charm, downloadInfo *charm.DownloadInfo) (corecharm.ID, charm.CharmLocator, error) // DeleteCharm removes the charm from the state. If the charm does not // exist, a [applicationerrors.CharmNotFound] error is returned. @@ -608,7 +608,7 @@ func (s *Service) GetAvailableCharmArchiveSHA256(ctx context.Context, id corecha // if the download info is not found. Returns [applicationerrors.CharmNotFound] // if the charm is not found. func (s *Service) ResolveUploadCharm(ctx context.Context, args charm.ResolveUploadCharm) (charm.CharmLocator, error) { - var uploadCharm bool + var localUploadCharm bool switch args.Source { case corecharm.CharmHub: @@ -616,13 +616,13 @@ func (s *Service) ResolveUploadCharm(ctx context.Context, args charm.ResolveUplo return charm.CharmLocator{}, applicationerrors.NonLocalCharmImporting } case corecharm.Local: - uploadCharm = !args.Importing + localUploadCharm = !args.Importing default: return charm.CharmLocator{}, applicationerrors.CharmSourceNotValid } // We're not importing a charm, this is a full blown upload. - if uploadCharm { + if localUploadCharm { return s.resolveLocalUploadedCharm(ctx, args) } @@ -653,15 +653,11 @@ func (s *Service) resolveLocalUploadedCharm(ctx context.Context, args charm.Reso return charm.CharmLocator{}, internalerrors.Errorf("reading charm archive %q: %w", result.ArchivePath, err) } - // TODO (stickupkid): Sequence a charm revision, so each charm has a unique - // revision. - // This is a full blown upload, we need to set everything up. resolved, warnings, err := s.setCharm(ctx, charm.SetCharmArgs{ Charm: ch, Source: args.Source, ReferenceName: args.Name, - Revision: args.Revision, Hash: digest.SHA256, ObjectStoreUUID: result.ObjectStoreUUID, Version: ch.Version(), @@ -670,6 +666,9 @@ func (s *Service) resolveLocalUploadedCharm(ctx context.Context, args charm.Reso Provenance: charm.ProvenanceUpload, }, + // The revision is not set, we need to sequence a revision. + Revision: -1, + // This is correct, we want to use the unique name of the stored charm // as the archive path. Once every blob is storing the UUID, we can // remove the archive path, until, just use the unique name. @@ -795,19 +794,14 @@ func (s *Service) setCharm(ctx context.Context, args charm.SetCharmArgs) (setCha ch.Available = args.ArchivePath != "" ch.Architecture = architecture - charmID, err := s.st.SetCharm(ctx, ch, args.DownloadInfo) + charmID, locator, err := s.st.SetCharm(ctx, ch, args.DownloadInfo) if err != nil { return setCharmResult{}, warnings, errors.Trace(err) } return setCharmResult{ - ID: charmID, - Locator: charm.CharmLocator{ - Name: args.ReferenceName, - Revision: ch.Revision, - Source: ch.Source, - Architecture: ch.Architecture, - }, + ID: charmID, + Locator: locator, }, warnings, nil } diff --git a/domain/application/service/charm_test.go b/domain/application/service/charm_test.go index d3ecb22be47..4b315d6ec9a 100644 --- a/domain/application/service/charm_test.go +++ b/domain/application/service/charm_test.go @@ -597,7 +597,12 @@ func (s *charmServiceSuite) TestSetCharm(c *gc.C) { Source: charm.LocalSource, Revision: 1, Architecture: architecture.AMD64, - }, downloadInfo).Return(id, nil) + }, downloadInfo).Return(id, charm.CharmLocator{ + Name: "foo", + Revision: 1, + Source: charm.LocalSource, + Architecture: architecture.AMD64, + }, nil) got, warnings, err := s.service.SetCharm(context.Background(), charm.SetCharmArgs{ Charm: s.charm, @@ -671,7 +676,12 @@ func (s *charmServiceSuite) TestSetCharmCharmhub(c *gc.C) { Source: charm.CharmHubSource, Revision: 1, Architecture: architecture.AMD64, - }, downloadInfo).Return(id, nil) + }, downloadInfo).Return(id, charm.CharmLocator{ + Name: "foo", + Revision: 1, + Source: charm.LocalSource, + Architecture: architecture.AMD64, + }, nil) got, warnings, err := s.service.SetCharm(context.Background(), charm.SetCharmArgs{ Charm: s.charm, @@ -961,7 +971,12 @@ func (s *charmServiceSuite) TestSetCharmRequireRelationToReservedNameSucceeds(c Architectures: []string{"arm64"}, }}}).MinTimes(1) - s.state.EXPECT().SetCharm(gomock.Any(), gomock.Any(), nil).Return(id, nil) + s.state.EXPECT().SetCharm(gomock.Any(), gomock.Any(), nil).Return(id, charm.CharmLocator{ + Name: "foo", + Revision: 1, + Source: charm.LocalSource, + Architecture: architecture.AMD64, + }, nil) got, warnings, err := s.service.SetCharm(context.Background(), charm.SetCharmArgs{ Charm: s.charm, @@ -1030,7 +1045,12 @@ func (s *charmServiceSuite) TestSetCharmRelationToReservedNameWithSpecialCharm(c Architectures: []string{"arm64"}, }}}).MinTimes(1) - s.state.EXPECT().SetCharm(gomock.Any(), gomock.Any(), nil).Return(id, nil) + s.state.EXPECT().SetCharm(gomock.Any(), gomock.Any(), nil).Return(id, charm.CharmLocator{ + Name: "foo", + Revision: 1, + Source: charm.LocalSource, + Architecture: architecture.AMD64, + }, nil) got, warnings, err := s.service.SetCharm(context.Background(), charm.SetCharmArgs{ Charm: s.charm, @@ -1070,7 +1090,12 @@ func (s *charmServiceSuite) TestSetCharmRelationToReservedNameOnRequiresValid(c Architectures: []string{"arm64"}, }}}).MinTimes(1) - s.state.EXPECT().SetCharm(gomock.Any(), gomock.Any(), nil).Return(id, nil) + s.state.EXPECT().SetCharm(gomock.Any(), gomock.Any(), nil).Return(id, charm.CharmLocator{ + Name: "foo", + Revision: 1, + Source: charm.LocalSource, + Architecture: architecture.AMD64, + }, nil) got, warnings, err := s.service.SetCharm(context.Background(), charm.SetCharmArgs{ Charm: s.charm, @@ -1259,9 +1284,14 @@ func (s *charmServiceSuite) TestResolveUploadCharmLocalCharmNotImporting(c *gc.C SHA384: "sha-384", Size: stat.Size(), }, nil) - s.state.EXPECT().SetCharm(gomock.Any(), gomock.Any(), downloadInfo).DoAndReturn(func(_ context.Context, charm charm.Charm, _ *charm.DownloadInfo) (corecharm.ID, error) { - c.Check(charm.Metadata.Name, gc.Equals, "dummy") - return charmID, nil + s.state.EXPECT().SetCharm(gomock.Any(), gomock.Any(), downloadInfo).DoAndReturn(func(_ context.Context, ch charm.Charm, _ *charm.DownloadInfo) (corecharm.ID, charm.CharmLocator, error) { + c.Check(ch.Metadata.Name, gc.Equals, "dummy") + return charmID, charm.CharmLocator{ + Name: "test", + Revision: 1, + Source: charm.LocalSource, + Architecture: architecture.AMD64, + }, nil }) locator, err := s.service.ResolveUploadCharm(context.Background(), charm.ResolveUploadCharm{ @@ -1335,8 +1365,8 @@ func (s *charmServiceSuite) TestResolveUploadCharmLocalCharmNotImportingFailedSe SHA384: "sha-384", Size: stat.Size(), }, nil) - s.state.EXPECT().SetCharm(gomock.Any(), gomock.Any(), downloadInfo).DoAndReturn(func(_ context.Context, charm charm.Charm, _ *charm.DownloadInfo) (corecharm.ID, error) { - return charmID, errors.NotValidf("failed to set charm") + s.state.EXPECT().SetCharm(gomock.Any(), gomock.Any(), downloadInfo).DoAndReturn(func(_ context.Context, _ charm.Charm, _ *charm.DownloadInfo) (corecharm.ID, charm.CharmLocator, error) { + return charmID, charm.CharmLocator{}, errors.NotValidf("failed to set charm") }) _, err = s.service.ResolveUploadCharm(context.Background(), charm.ResolveUploadCharm{ diff --git a/domain/application/service/package_mock_test.go b/domain/application/service/package_mock_test.go index 18a2e2aaba7..2818d0f7aa5 100644 --- a/domain/application/service/package_mock_test.go +++ b/domain/application/service/package_mock_test.go @@ -1975,12 +1975,13 @@ func (c *MockStateSetApplicationScalingStateCall) DoAndReturn(f func(domain.Atom } // SetCharm mocks base method. -func (m *MockState) SetCharm(arg0 context.Context, arg1 charm0.Charm, arg2 *charm0.DownloadInfo) (charm.ID, error) { +func (m *MockState) SetCharm(arg0 context.Context, arg1 charm0.Charm, arg2 *charm0.DownloadInfo) (charm.ID, charm0.CharmLocator, error) { m.ctrl.T.Helper() ret := m.ctrl.Call(m, "SetCharm", arg0, arg1, arg2) ret0, _ := ret[0].(charm.ID) - ret1, _ := ret[1].(error) - return ret0, ret1 + ret1, _ := ret[1].(charm0.CharmLocator) + ret2, _ := ret[2].(error) + return ret0, ret1, ret2 } // SetCharm indicates an expected call of SetCharm. @@ -1996,19 +1997,19 @@ type MockStateSetCharmCall struct { } // Return rewrite *gomock.Call.Return -func (c *MockStateSetCharmCall) Return(arg0 charm.ID, arg1 error) *MockStateSetCharmCall { - c.Call = c.Call.Return(arg0, arg1) +func (c *MockStateSetCharmCall) Return(arg0 charm.ID, arg1 charm0.CharmLocator, arg2 error) *MockStateSetCharmCall { + c.Call = c.Call.Return(arg0, arg1, arg2) return c } // Do rewrite *gomock.Call.Do -func (c *MockStateSetCharmCall) Do(f func(context.Context, charm0.Charm, *charm0.DownloadInfo) (charm.ID, error)) *MockStateSetCharmCall { +func (c *MockStateSetCharmCall) Do(f func(context.Context, charm0.Charm, *charm0.DownloadInfo) (charm.ID, charm0.CharmLocator, error)) *MockStateSetCharmCall { c.Call = c.Call.Do(f) return c } // DoAndReturn rewrite *gomock.Call.DoAndReturn -func (c *MockStateSetCharmCall) DoAndReturn(f func(context.Context, charm0.Charm, *charm0.DownloadInfo) (charm.ID, error)) *MockStateSetCharmCall { +func (c *MockStateSetCharmCall) DoAndReturn(f func(context.Context, charm0.Charm, *charm0.DownloadInfo) (charm.ID, charm0.CharmLocator, error)) *MockStateSetCharmCall { c.Call = c.Call.DoAndReturn(f) return c } diff --git a/domain/application/state/application_test.go b/domain/application/state/application_test.go index 0a7f3033a69..04f220b2720 100644 --- a/domain/application/state/application_test.go +++ b/domain/application/state/application_test.go @@ -1892,7 +1892,7 @@ func (s *applicationStateSuite) TestGetCharmIDByApplicationName(c *gc.C) { s.createApplication(c, "foo", life.Alive) - _, err := s.state.SetCharm(context.Background(), charm.Charm{ + _, _, err := s.state.SetCharm(context.Background(), charm.Charm{ Metadata: expectedMetadata, Manifest: expectedManifest, Actions: expectedActions, diff --git a/domain/application/state/charm.go b/domain/application/state/charm.go index 9c1da85f90d..654f5f19741 100644 --- a/domain/application/state/charm.go +++ b/domain/application/state/charm.go @@ -576,35 +576,58 @@ func (s *State) GetCharm(ctx context.Context, id corecharm.ID) (charm.Charm, *ch // SetCharm persists the charm metadata, actions, config and manifest to // state. -func (s *State) SetCharm(ctx context.Context, charm charm.Charm, downloadInfo *charm.DownloadInfo) (corecharm.ID, error) { +func (s *State) SetCharm(ctx context.Context, ch charm.Charm, downloadInfo *charm.DownloadInfo) (corecharm.ID, charm.CharmLocator, error) { db, err := s.DB() if err != nil { - return "", internalerrors.Capture(err) + return "", charm.CharmLocator{}, internalerrors.Capture(err) } id, err := corecharm.NewID() if err != nil { - return "", internalerrors.Errorf("setting charm: %w", err) + return "", charm.CharmLocator{}, internalerrors.Errorf("setting charm: %w", err) + } + + charmUUID := charmID{UUID: id.String()} + + var locator charmLocator + locatorQuery := ` +SELECT &charmLocator.* +FROM v_charm_locator +WHERE uuid = $charmID.uuid; + ` + locatorStmt, err := s.Prepare(locatorQuery, charmUUID, locator) + if err != nil { + return "", charm.CharmLocator{}, internalerrors.Errorf("preparing query: %w", err) } if err := db.Txn(ctx, func(ctx context.Context, tx *sqlair.TX) error { // Check the charm doesn't already exist, if it does, return an already // exists error. Also doing this early, prevents the moving straight // to a write transaction. - if _, err := s.checkCharmReferenceExists(ctx, tx, charm.ReferenceName, charm.Revision); err != nil { + if _, err := s.checkCharmReferenceExists(ctx, tx, ch.ReferenceName, ch.Revision); err != nil { return internalerrors.Capture(err) } - if err := s.setCharm(ctx, tx, id, charm, downloadInfo); err != nil { + if err := s.setCharm(ctx, tx, id, ch, downloadInfo); err != nil { return internalerrors.Capture(err) } + // The charm will be set, so we can use the new charmUUID to get the + // locator. + if err := tx.Query(ctx, locatorStmt, charmUUID).Get(&locator); err != nil { + return internalerrors.Errorf("getting charm locator: %w", err) + } return nil }); err != nil { - return "", internalerrors.Errorf("setting charm: %w", err) + return "", charm.CharmLocator{}, internalerrors.Errorf("setting charm: %w", err) } - return id, nil + chLocator, err := decodeCharmLocator(locator) + if err != nil { + return "", charm.CharmLocator{}, internalerrors.Errorf("decoding charm locator: %w", err) + } + + return id, chLocator, nil } // ListCharmLocatorsByNames returns a list of charm locators. diff --git a/domain/application/state/charm_test.go b/domain/application/state/charm_test.go index ac0c2ba6e58..e8e7deeabcc 100644 --- a/domain/application/state/charm_test.go +++ b/domain/application/state/charm_test.go @@ -116,7 +116,7 @@ INSERT INTO object_store_metadata (uuid, sha_256, sha_384, size) VALUES (?, 'foo }) c.Assert(err, jc.ErrorIsNil) - id, err := st.SetCharm(context.Background(), charm.Charm{ + id, _, err := st.SetCharm(context.Background(), charm.Charm{ Metadata: expected, Manifest: s.minimalManifest(c), Source: charm.LocalSource, @@ -157,7 +157,7 @@ func (s *charmStateSuite) TestSetCharmWithoutObjectStoreUUID(c *gc.C) { // The archive path is empty, so it's not immediately available. - id, err := st.SetCharm(context.Background(), charm.Charm{ + id, _, err := st.SetCharm(context.Background(), charm.Charm{ Metadata: expected, Manifest: s.minimalManifest(c), Source: charm.LocalSource, @@ -197,18 +197,25 @@ func (s *charmStateSuite) TestSetCharmNotAvailable(c *gc.C) { // The archive path is empty, so it's not immediately available. - id, err := st.SetCharm(context.Background(), charm.Charm{ + id, locator, err := st.SetCharm(context.Background(), charm.Charm{ Metadata: expected, Manifest: s.minimalManifest(c), Source: charm.LocalSource, - Revision: 42, + Revision: -1, ReferenceName: "foo", Hash: "hash", Version: "deadbeef", + Architecture: architecture.Unknown, }, nil) c.Assert(err, jc.ErrorIsNil) + c.Check(locator, gc.DeepEquals, charm.CharmLocator{ + Name: "foo", + Revision: 1, + Source: charm.LocalSource, + Architecture: architecture.Unknown, + }) - charmID, err := st.GetCharmID(context.Background(), "foo", 42, charm.LocalSource) + charmID, err := st.GetCharmID(context.Background(), "foo", locator.Revision, charm.LocalSource) c.Assert(err, jc.ErrorIsNil) c.Check(charmID, gc.Equals, id) @@ -233,11 +240,11 @@ func (s *charmStateSuite) TestSetCharmGetCharmID(c *gc.C) { Assumes: []byte("null"), } - id, err := st.SetCharm(context.Background(), charm.Charm{ + id, locator, err := st.SetCharm(context.Background(), charm.Charm{ Metadata: expected, Manifest: s.minimalManifest(c), Source: charm.LocalSource, - Revision: 42, + Revision: -1, ReferenceName: "foo", Hash: "hash", ArchivePath: "archive", @@ -245,7 +252,7 @@ func (s *charmStateSuite) TestSetCharmGetCharmID(c *gc.C) { }, nil) c.Assert(err, jc.ErrorIsNil) - charmID, err := st.GetCharmID(context.Background(), "foo", 42, charm.LocalSource) + charmID, err := st.GetCharmID(context.Background(), "foo", locator.Revision, charm.LocalSource) c.Assert(err, jc.ErrorIsNil) c.Check(charmID, gc.Equals, id) } @@ -1294,7 +1301,7 @@ func (s *charmStateSuite) TestSetCharmDownloadInfoForCharmhub(c *gc.C) { DownloadSize: 1234, } - id, err := st.SetCharm(context.Background(), charm.Charm{ + id, locator, err := st.SetCharm(context.Background(), charm.Charm{ Metadata: charm.Metadata{ Name: "ubuntu", Summary: "summary", @@ -1324,6 +1331,13 @@ func (s *charmStateSuite) TestSetCharmDownloadInfoForCharmhub(c *gc.C) { }, info) c.Assert(err, jc.ErrorIsNil) + c.Check(locator, gc.DeepEquals, charm.CharmLocator{ + Name: "ubuntu", + Revision: 42, + Source: charm.CharmHubSource, + Architecture: architecture.AMD64, + }) + _, downloadInfo, err := st.GetCharm(context.Background(), id) c.Assert(err, jc.ErrorIsNil) c.Check(downloadInfo, gc.DeepEquals, info) @@ -1332,7 +1346,7 @@ func (s *charmStateSuite) TestSetCharmDownloadInfoForCharmhub(c *gc.C) { func (s *charmStateSuite) TestSetCharmDownloadInfoForCharmhubWithoutDownloadInfo(c *gc.C) { st := NewState(s.TxnRunnerFactory(), clock.WallClock, loggertesting.WrapCheckLog(c)) - id, err := st.SetCharm(context.Background(), charm.Charm{ + id, _, err := st.SetCharm(context.Background(), charm.Charm{ Metadata: charm.Metadata{ Name: "ubuntu", Summary: "summary", @@ -1375,7 +1389,7 @@ func (s *charmStateSuite) TestSetCharmDownloadInfoForLocal(c *gc.C) { DownloadSize: 1234, } - id, err := st.SetCharm(context.Background(), charm.Charm{ + id, _, err := st.SetCharm(context.Background(), charm.Charm{ Metadata: charm.Metadata{ Name: "ubuntu", Summary: "summary", @@ -1413,7 +1427,7 @@ func (s *charmStateSuite) TestSetCharmDownloadInfoForLocal(c *gc.C) { func (s *charmStateSuite) TestSetCharmDownloadInfoForLocalWithoutInfo(c *gc.C) { st := NewState(s.TxnRunnerFactory(), clock.WallClock, loggertesting.WrapCheckLog(c)) - id, err := st.SetCharm(context.Background(), charm.Charm{ + id, _, err := st.SetCharm(context.Background(), charm.Charm{ Metadata: charm.Metadata{ Name: "ubuntu", Summary: "summary", @@ -1461,7 +1475,7 @@ func (s *charmStateSuite) TestSetCharmTwice(c *gc.C) { Assumes: []byte("null"), } - id, err := st.SetCharm(context.Background(), charm.Charm{ + id, _, err := st.SetCharm(context.Background(), charm.Charm{ Metadata: expected, Manifest: s.minimalManifest(c), Source: charm.LocalSource, @@ -1477,7 +1491,7 @@ func (s *charmStateSuite) TestSetCharmTwice(c *gc.C) { c.Assert(err, jc.ErrorIsNil) c.Check(got, gc.DeepEquals, expected) - _, err = st.SetCharm(context.Background(), charm.Charm{ + _, _, err = st.SetCharm(context.Background(), charm.Charm{ Metadata: expected, Manifest: s.minimalManifest(c), Source: charm.LocalSource, @@ -1535,7 +1549,7 @@ func (s *charmStateSuite) TestSetCharmThenGetCharm(c *gc.C) { } expectedLXDProfile := []byte("[{}]") - id, err := st.SetCharm(context.Background(), charm.Charm{ + id, _, err := st.SetCharm(context.Background(), charm.Charm{ Metadata: expectedMetadata, Manifest: expectedManifest, Actions: expectedActions, @@ -1615,7 +1629,7 @@ func (s *charmStateSuite) TestSetCharmThenGetCharmWithDifferentReferenceName(c * } expectedLXDProfile := []byte("[{}]") - _, err := st.SetCharm(context.Background(), charm.Charm{ + _, _, err := st.SetCharm(context.Background(), charm.Charm{ Metadata: expectedMetadata, Manifest: expectedManifest, Actions: expectedActions, @@ -1663,7 +1677,7 @@ func (s *charmStateSuite) TestSetCharmAllowsSameNameButDifferentRevision(c *gc.C Assumes: []byte("null"), } - id1, err := st.SetCharm(context.Background(), charm.Charm{ + id1, locator1, err := st.SetCharm(context.Background(), charm.Charm{ Metadata: expected, Manifest: s.minimalManifest(c), Source: charm.LocalSource, @@ -1672,14 +1686,22 @@ func (s *charmStateSuite) TestSetCharmAllowsSameNameButDifferentRevision(c *gc.C Hash: "hash", ArchivePath: "archive", Version: "deadbeef", + Architecture: architecture.AMD64, }, nil) c.Assert(err, jc.ErrorIsNil) + c.Check(locator1, gc.DeepEquals, charm.CharmLocator{ + Name: "ubuntu", + Revision: 1, + Source: charm.LocalSource, + Architecture: architecture.AMD64, + }) + got, err := st.GetCharmMetadata(context.Background(), id1) c.Assert(err, jc.ErrorIsNil) c.Check(got, gc.DeepEquals, expected) - id2, err := st.SetCharm(context.Background(), charm.Charm{ + id2, locator2, err := st.SetCharm(context.Background(), charm.Charm{ Metadata: expected, Manifest: s.minimalManifest(c), Source: charm.LocalSource, @@ -1694,6 +1716,13 @@ func (s *charmStateSuite) TestSetCharmAllowsSameNameButDifferentRevision(c *gc.C got, err = st.GetCharmMetadata(context.Background(), id2) c.Assert(err, jc.ErrorIsNil) c.Check(got, gc.DeepEquals, expected) + + c.Check(locator2, gc.DeepEquals, charm.CharmLocator{ + Name: "ubuntu", + Revision: 2, + Source: charm.LocalSource, + Architecture: architecture.AMD64, + }) } func (s *charmStateSuite) TestSetCharmThenGetCharmMetadata(c *gc.C) { @@ -1709,7 +1738,7 @@ func (s *charmStateSuite) TestSetCharmThenGetCharmMetadata(c *gc.C) { Assumes: []byte("null"), } - id, err := st.SetCharm(context.Background(), charm.Charm{ + id, _, err := st.SetCharm(context.Background(), charm.Charm{ Metadata: expected, Manifest: s.minimalManifest(c), Source: charm.LocalSource, @@ -1747,7 +1776,7 @@ func (s *charmStateSuite) TestSetCharmThenGetCharmMetadataWithTagsAndCategories( Categories: []string{"data", "kubernetes", "kubernetes"}, } - id, err := st.SetCharm(context.Background(), charm.Charm{ + id, _, err := st.SetCharm(context.Background(), charm.Charm{ Metadata: expected, Manifest: s.minimalManifest(c), Source: charm.LocalSource, @@ -1784,7 +1813,7 @@ func (s *charmStateSuite) TestSetCharmThenGetCharmMetadataWithTerms(c *gc.C) { Terms: []string{"foo", "foo", "bar"}, } - id, err := st.SetCharm(context.Background(), charm.Charm{ + id, _, err := st.SetCharm(context.Background(), charm.Charm{ Metadata: expected, Manifest: s.minimalManifest(c), Source: charm.LocalSource, @@ -1850,7 +1879,7 @@ func (s *charmStateSuite) TestSetCharmThenGetCharmMetadataWithRelations(c *gc.C) }, } - id, err := st.SetCharm(context.Background(), charm.Charm{ + id, _, err := st.SetCharm(context.Background(), charm.Charm{ Metadata: expected, Manifest: s.minimalManifest(c), Source: charm.LocalSource, @@ -1894,7 +1923,7 @@ func (s *charmStateSuite) TestSetCharmThenGetCharmMetadataWithExtraBindings(c *g }, } - id, err := st.SetCharm(context.Background(), charm.Charm{ + id, _, err := st.SetCharm(context.Background(), charm.Charm{ Metadata: expected, Manifest: s.minimalManifest(c), Source: charm.LocalSource, @@ -1954,7 +1983,7 @@ func (s *charmStateSuite) TestSetCharmThenGetCharmMetadataWithStorageWithNoPrope }, } - id, err := st.SetCharm(context.Background(), charm.Charm{ + id, _, err := st.SetCharm(context.Background(), charm.Charm{ Metadata: expected, Manifest: s.minimalManifest(c), Source: charm.LocalSource, @@ -2016,7 +2045,7 @@ func (s *charmStateSuite) TestSetCharmThenGetCharmMetadataWithStorageWithPropert }, } - id, err := st.SetCharm(context.Background(), charm.Charm{ + id, _, err := st.SetCharm(context.Background(), charm.Charm{ Metadata: expected, Manifest: s.minimalManifest(c), Source: charm.LocalSource, @@ -2069,7 +2098,7 @@ func (s *charmStateSuite) TestSetCharmThenGetCharmMetadataWithDevices(c *gc.C) { }, } - id, err := st.SetCharm(context.Background(), charm.Charm{ + id, _, err := st.SetCharm(context.Background(), charm.Charm{ Metadata: expected, Manifest: s.minimalManifest(c), Source: charm.LocalSource, @@ -2115,7 +2144,7 @@ func (s *charmStateSuite) TestSetCharmThenGetCharmMetadataWithPayloadClasses(c * }, } - id, err := st.SetCharm(context.Background(), charm.Charm{ + id, _, err := st.SetCharm(context.Background(), charm.Charm{ Metadata: expected, Manifest: s.minimalManifest(c), Source: charm.LocalSource, @@ -2165,7 +2194,7 @@ func (s *charmStateSuite) TestSetCharmThenGetCharmMetadataWithResources(c *gc.C) }, } - id, err := st.SetCharm(context.Background(), charm.Charm{ + id, _, err := st.SetCharm(context.Background(), charm.Charm{ Metadata: expected, Manifest: s.minimalManifest(c), Source: charm.LocalSource, @@ -2211,7 +2240,7 @@ func (s *charmStateSuite) TestSetCharmThenGetCharmMetadataWithContainersWithNoMo }, } - id, err := st.SetCharm(context.Background(), charm.Charm{ + id, _, err := st.SetCharm(context.Background(), charm.Charm{ Metadata: expected, Manifest: s.minimalManifest(c), Source: charm.LocalSource, @@ -2277,7 +2306,7 @@ func (s *charmStateSuite) TestSetCharmThenGetCharmMetadataWithContainersWithMoun }, } - id, err := st.SetCharm(context.Background(), charm.Charm{ + id, _, err := st.SetCharm(context.Background(), charm.Charm{ Metadata: expected, Manifest: s.minimalManifest(c), Source: charm.LocalSource, @@ -2409,7 +2438,7 @@ func (s *charmStateSuite) TestSetCharmThenGetCharmManifest(c *gc.C) { }, } - id, err := st.SetCharm(context.Background(), charm.Charm{ + id, _, err := st.SetCharm(context.Background(), charm.Charm{ Metadata: charm.Metadata{ Name: "ubuntu", }, @@ -2615,7 +2644,7 @@ func (s *charmStateSuite) TestSetCharmThenGetCharmConfig(c *gc.C) { }, } - id, err := st.SetCharm(context.Background(), charm.Charm{ + id, _, err := st.SetCharm(context.Background(), charm.Charm{ Metadata: charm.Metadata{ Name: "ubuntu", }, @@ -2740,7 +2769,7 @@ func (s *charmStateSuite) TestSetCharmThenGetCharmActions(c *gc.C) { }, } - id, err := st.SetCharm(context.Background(), charm.Charm{ + id, _, err := st.SetCharm(context.Background(), charm.Charm{ Metadata: charm.Metadata{ Name: "ubuntu", }, @@ -2799,7 +2828,7 @@ func (s *charmStateSuite) TestGetCharmActionsEmpty(c *gc.C) { func (s *charmStateSuite) TestSetCharmThenGetCharmArchivePath(c *gc.C) { st := NewState(s.TxnRunnerFactory(), clock.WallClock, loggertesting.WrapCheckLog(c)) - id, err := st.SetCharm(context.Background(), charm.Charm{ + id, _, err := st.SetCharm(context.Background(), charm.Charm{ Metadata: charm.Metadata{ Name: "ubuntu", }, @@ -2821,7 +2850,7 @@ func (s *charmStateSuite) TestSetCharmThenGetCharmArchivePath(c *gc.C) { func (s *charmStateSuite) TestSetCharmWithDuplicatedEndpointNames(c *gc.C) { st := NewState(s.TxnRunnerFactory(), clock.WallClock, loggertesting.WrapCheckLog(c)) - _, err := st.SetCharm(context.Background(), charm.Charm{ + _, _, err := st.SetCharm(context.Background(), charm.Charm{ Metadata: charm.Metadata{ Provides: map[string]charm.Relation{ "foo": { @@ -2864,7 +2893,7 @@ func (s *charmStateSuite) TestGetCharmArchivePathCharmNotFound(c *gc.C) { func (s *charmStateSuite) TestGetCharmArchiveMetadata(c *gc.C) { st := NewState(s.TxnRunnerFactory(), clock.WallClock, loggertesting.WrapCheckLog(c)) - id, err := st.SetCharm(context.Background(), charm.Charm{ + id, _, err := st.SetCharm(context.Background(), charm.Charm{ Metadata: charm.Metadata{ Name: "ubuntu", }, @@ -2888,7 +2917,7 @@ func (s *charmStateSuite) TestGetCharmArchiveMetadataInsertAdditionalHashKind(c st := NewState(s.TxnRunnerFactory(), clock.WallClock, loggertesting.WrapCheckLog(c)) - id, err := st.SetCharm(context.Background(), charm.Charm{ + id, _, err := st.SetCharm(context.Background(), charm.Charm{ Metadata: charm.Metadata{ Name: "ubuntu", }, @@ -2931,7 +2960,7 @@ func (s *charmStateSuite) TestListCharmLocatorsWithNoEntries(c *gc.C) { func (s *charmStateSuite) TestListCharmLocators(c *gc.C) { st := NewState(s.TxnRunnerFactory(), clock.WallClock, loggertesting.WrapCheckLog(c)) - _, err := st.SetCharm(context.Background(), charm.Charm{ + _, _, err := st.SetCharm(context.Background(), charm.Charm{ Metadata: charm.Metadata{ Name: "ubuntu", }, @@ -2962,7 +2991,7 @@ func (s *charmStateSuite) TestListCharmLocatorsMultipleEntries(c *gc.C) { for i := 0; i < 3; i++ { name := fmt.Sprintf("ubuntu-%d", i) - _, err := st.SetCharm(context.Background(), charm.Charm{ + _, _, err := st.SetCharm(context.Background(), charm.Charm{ Metadata: charm.Metadata{ Name: name, }, @@ -3004,7 +3033,7 @@ func (s *charmStateSuite) TestListCharmLocatorsByNamesMultipleEntries(c *gc.C) { for i := 0; i < 3; i++ { name := fmt.Sprintf("ubuntu-%d", i) - _, err := st.SetCharm(context.Background(), charm.Charm{ + _, _, err := st.SetCharm(context.Background(), charm.Charm{ Metadata: charm.Metadata{ Name: name, }, @@ -3042,7 +3071,7 @@ func (s *charmStateSuite) TestListCharmLocatorsByNamesInvalidEntries(c *gc.C) { for i := 0; i < 3; i++ { name := fmt.Sprintf("ubuntu-%d", i) - _, err := st.SetCharm(context.Background(), charm.Charm{ + _, _, err := st.SetCharm(context.Background(), charm.Charm{ Metadata: charm.Metadata{ Name: name, }, @@ -3065,7 +3094,7 @@ func (s *charmStateSuite) TestListCharmLocatorsByNamesInvalidEntries(c *gc.C) { func (s *charmStateSuite) TestGetCharmDownloadInfoWithNoInfo(c *gc.C) { st := NewState(s.TxnRunnerFactory(), clock.WallClock, loggertesting.WrapCheckLog(c)) - id, err := st.SetCharm(context.Background(), charm.Charm{ + id, _, err := st.SetCharm(context.Background(), charm.Charm{ Metadata: charm.Metadata{ Name: "foo", }, @@ -3094,7 +3123,7 @@ func (s *charmStateSuite) TestGetCharmDownloadInfoWithInfoForLocal(c *gc.C) { DownloadSize: 42, } - id, err := st.SetCharm(context.Background(), charm.Charm{ + id, _, err := st.SetCharm(context.Background(), charm.Charm{ Metadata: charm.Metadata{ Name: "foo", }, @@ -3123,7 +3152,7 @@ func (s *charmStateSuite) TestGetCharmDownloadInfoWithInfoForCharmhub(c *gc.C) { DownloadSize: 42, } - id, err := st.SetCharm(context.Background(), charm.Charm{ + id, _, err := st.SetCharm(context.Background(), charm.Charm{ Metadata: charm.Metadata{ Name: "foo", }, @@ -3152,7 +3181,7 @@ func (s *charmStateSuite) TestGetAvailableCharmArchiveSHA256(c *gc.C) { DownloadSize: 42, } - id, err := st.SetCharm(context.Background(), charm.Charm{ + id, _, err := st.SetCharm(context.Background(), charm.Charm{ Metadata: charm.Metadata{ Name: "foo", }, @@ -3182,7 +3211,7 @@ func (s *charmStateSuite) TestGetAvailableCharmArchiveSHA256NotAvailable(c *gc.C DownloadSize: 42, } - id, err := st.SetCharm(context.Background(), charm.Charm{ + id, _, err := st.SetCharm(context.Background(), charm.Charm{ Metadata: charm.Metadata{ Name: "foo", }, @@ -3228,7 +3257,7 @@ func (s *charmStateSuite) TestResolveMigratingUploadedCharmAlreadyAvailable(c *g Provenance: charm.ProvenanceMigration, } - id, err := st.SetCharm(context.Background(), charm.Charm{ + id, _, err := st.SetCharm(context.Background(), charm.Charm{ Metadata: charm.Metadata{ Name: "foo", }, @@ -3259,7 +3288,7 @@ func (s *charmStateSuite) TestResolveMigratingUploaded(c *gc.C) { Provenance: charm.ProvenanceMigration, } - id, err := st.SetCharm(context.Background(), charm.Charm{ + id, chLocator, err := st.SetCharm(context.Background(), charm.Charm{ Metadata: charm.Metadata{ Name: "foo", }, @@ -3285,6 +3314,7 @@ func (s *charmStateSuite) TestResolveMigratingUploaded(c *gc.C) { Revision: 42, Architecture: architecture.AMD64, }) + c.Check(chLocator, gc.DeepEquals, locator) available, err := st.IsCharmAvailable(context.Background(), id) c.Assert(err, jc.ErrorIsNil) diff --git a/domain/application/state/state.go b/domain/application/state/state.go index 5641e5677b5..dcc85169d03 100644 --- a/domain/application/state/state.go +++ b/domain/application/state/state.go @@ -201,10 +201,19 @@ func (s *State) setCharmState( } } + // We have a local charm that needs a new sequence revision. + revision := ch.Revision + if ch.Source == charm.LocalSource && revision == -1 { + revision, err = s.sequence(ctx, tx, ch.ReferenceName) + if err != nil { + return fmt.Errorf("getting next charm revision: %w", err) + } + } + chState := setCharmState{ UUID: id.String(), ReferenceName: ch.ReferenceName, - Revision: ch.Revision, + Revision: revision, ArchivePath: ch.ArchivePath, ObjectStoreUUID: nullableOjectStoreUUID, Available: ch.Available, @@ -243,6 +252,41 @@ func (s *State) setCharmState( return nil } +func (s *State) sequence(ctx context.Context, tx *sqlair.TX, referenceName string) (int, error) { + // This should be doable in a single query using the RETURNING result, but + // we run into https://github.com/canonical/go-dqlite/issues/192 + // So we have to do it in two steps. + + updateQuery := ` +INSERT INTO sequence_charm_local (reference_name, sequence) VALUES ($sequenceCharmsLocal.reference_name, 1) +ON CONFLICT (reference_name) DO UPDATE SET sequence = sequence_charm_local.sequence + 1 +` + updateStmt, err := s.Prepare(updateQuery, sequenceCharmsLocal{}) + if err != nil { + return -1, internalerrors.Errorf("preparing query: %w", err) + } + + selectQuery := ` +SELECT &sequence.* FROM sequence_charm_local +WHERE reference_name = $sequenceCharmsLocal.reference_name; + ` + selectStmt, err := s.Prepare(selectQuery, sequenceCharmsLocal{}, sequence{}) + if err != nil { + return -1, internalerrors.Errorf("preparing query: %w", err) + } + + if err := tx.Query(ctx, updateStmt, sequenceCharmsLocal{ReferenceName: referenceName}).Run(); err != nil { + return -1, internalerrors.Errorf("updating sequence: %w", err) + } + + var sequence sequence + if err := tx.Query(ctx, selectStmt, sequenceCharmsLocal{ReferenceName: referenceName}).Get(&sequence); err != nil { + return -1, internalerrors.Errorf("getting sequence: %w", err) + } + + return sequence.Sequence, nil +} + func (s *State) setCharmDownloadInfo(ctx context.Context, tx *sqlair.TX, id corecharm.ID, downloadInfo *charm.DownloadInfo) error { if downloadInfo == nil { return nil diff --git a/domain/application/state/state_test.go b/domain/application/state/state_test.go new file mode 100644 index 00000000000..093d8f95fb7 --- /dev/null +++ b/domain/application/state/state_test.go @@ -0,0 +1,66 @@ +// Copyright 2024 Canonical Ltd. +// Licensed under the AGPLv3, see LICENCE file for details. + +package state + +import ( + "context" + "fmt" + + "github.com/canonical/sqlair" + "github.com/juju/clock" + loggertesting "github.com/juju/juju/internal/logger/testing" + jc "github.com/juju/testing/checkers" + gc "gopkg.in/check.v1" +) + +type stateSuite struct { + baseSuite +} + +var _ = gc.Suite(&stateSuite{}) + +func (s *stateSuite) TestSequence(c *gc.C) { + st := NewState(s.TxnRunnerFactory(), clock.WallClock, loggertesting.WrapCheckLog(c)) + + var sequence int + err := s.TxnRunner().Txn(context.Background(), func(ctx context.Context, tx *sqlair.TX) error { + var err error + sequence, err = st.sequence(ctx, tx, "test") + return err + }) + c.Assert(err, jc.ErrorIsNil) + c.Assert(sequence, gc.Equals, 1) +} + +func (s *stateSuite) TestSequenceMultipleTimes(c *gc.C) { + st := NewState(s.TxnRunnerFactory(), clock.WallClock, loggertesting.WrapCheckLog(c)) + + for i := 0; i < 10; i++ { + var sequence int + err := s.TxnRunner().Txn(context.Background(), func(ctx context.Context, tx *sqlair.TX) error { + var err error + sequence, err = st.sequence(ctx, tx, "test") + return err + }) + c.Assert(err, jc.ErrorIsNil) + c.Assert(sequence, gc.Equals, i+1) + } +} + +func (s *stateSuite) TestSequenceMultipleTimesWithDifferentNames(c *gc.C) { + st := NewState(s.TxnRunnerFactory(), clock.WallClock, loggertesting.WrapCheckLog(c)) + + for i := 0; i < 10; i++ { + for j := 0; j < 5; j++ { + var sequence int + err := s.TxnRunner().Txn(context.Background(), func(ctx context.Context, tx *sqlair.TX) error { + var err error + sequence, err = st.sequence(ctx, tx, fmt.Sprintf("test-%d", j)) + return err + }) + c.Assert(err, jc.ErrorIsNil) + c.Assert(sequence, gc.Equals, i+1) + } + } +} diff --git a/domain/application/state/types.go b/domain/application/state/types.go index 89d7facca1a..14822abb64c 100644 --- a/domain/application/state/types.go +++ b/domain/application/state/types.go @@ -654,3 +654,13 @@ type linkResourceApplication struct { ResourceUUID string `db:"resource_uuid"` ApplicationUUID string `db:"application_uuid"` } + +// sequenceCharmsLocal is used to get the reference name of a charm. +type sequenceCharmsLocal struct { + ReferenceName string `db:"reference_name"` +} + +// sequence is used to get the sequence of a charm. +type sequence struct { + Sequence int `db:"sequence"` +} diff --git a/domain/schema/model/sql/0015-charm.sql b/domain/schema/model/sql/0015-charm.sql index 3506c72aadd..f42b06b20d3 100644 --- a/domain/schema/model/sql/0015-charm.sql +++ b/domain/schema/model/sql/0015-charm.sql @@ -83,17 +83,13 @@ ON charm (source_id, reference_name, revision); -- to group them. Instead, the group is the source_id and the reference_name. -- This is the unique identifier for the charm. CREATE TABLE sequence_charm_local ( - source_id INT NOT NULL, - reference_name TEXT NOT NULL, + reference_name TEXT NOT NULL PRIMARY KEY, -- The sequence number will start at 0 for each charm and will be -- incremented. sequence INT NOT NULL DEFAULT 0 ); -CREATE UNIQUE INDEX idx_sequence_charm_local_source_reference -ON sequence_charm_local (source_id, reference_name); - CREATE TABLE charm_provenance ( id INT PRIMARY KEY, name TEXT NOT NULL