Skip to content

Commit

Permalink
feat: sequence local charm revisions
Browse files Browse the repository at this point in the history 
Unfortunately we can't increment a sequence in one query because
we run into the following dqlite issue[1]. For now we can just do
a two step dance. Considering this won't be exercised that often
this isn't going to be that taxing.

 1. canonical/go-dqlite#192
  • Loading branch information
@SimonRichardson
SimonRichardson committed Dec 17, 2024
1 parent e067796 commit 2421016
Show file tree
Hide file tree
Showing 10 changed files with 289 additions and 95 deletions.
26 changes: 10 additions & 16 deletions domain/application/service/charm.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -608,21 +608,21 @@ 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:
if !args.Importing {
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)
}

Expand Down Expand Up @@ -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(),
Expand All @@ -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.
Expand Down Expand Up @@ -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
}

Expand Down
50 changes: 40 additions & 10 deletions domain/application/service/charm_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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{
Expand Down Expand Up @@ -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{
Expand Down
15 changes: 8 additions & 7 deletions domain/application/service/package_mock_test.go
View file

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

2 changes: 1 addition & 1 deletion domain/application/state/application_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
37 changes: 30 additions & 7 deletions domain/application/state/charm.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
Loading

0 comments on commit 2421016

Please sign in to comment.