Skip to content

Commit

Permalink
Merge pull request juju#18439 from jack-w-shaw/JUJU-7227_add_charm_ar…
Browse files Browse the repository at this point in the history
…chive_methods

juju#18439

Add a method to the charm service to retrieve a charm archive from the object store. Return alongside the hash of the charm.

This required adding a new method to our object store indirection.

This method will be used my the migration worker to migrate charm blobs. At the moment, the worker retrieves charm archives through a costly (complexity-wise and resource-wise) API request to itself. Once this is done, the service will will be wired up to that worker to allow it to retrieve charm archives directly from the object store. 

## QA steps

Unit tests pass
  • Loading branch information
jujubot authored Nov 27, 2024
2 parents e6f8828 + b7b3741 commit eede4d8
Show file tree
Hide file tree
Showing 18 changed files with 353 additions and 12 deletions.
19 changes: 18 additions & 1 deletion domain/application/charm/store/store.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (
"context"
"encoding/base32"
"fmt"
"io"
"os"

"github.com/juju/juju/core/objectstore"
Expand All @@ -19,7 +20,7 @@ const (
ErrNotFound = errors.ConstError("file not found")
)

// CharmStore provides an API for storing charms.
// CharmStore provides an API for storing and retrieving charm blobs.
type CharmStore struct {
objectStoreGetter objectstore.ModelObjectStoreGetter
encoder *base32.Encoding
Expand Down Expand Up @@ -66,3 +67,19 @@ func (s *CharmStore) Store(ctx context.Context, name string, path string, size i
// Store the file in the object store.
return objectStore.PutAndCheckHash(ctx, uniqueName, file, size, hash)
}

// Get retrieves a ReadCloser for the charm archive at the give path from
// the underlying storage.
// NOTE: It is up to the caller to verify the integrity of the data from the charm
// hash stored in DQLite.
func (s *CharmStore) Get(ctx context.Context, archivePath string) (io.ReadCloser, error) {
store, err := s.objectStoreGetter.GetObjectStore(ctx)
if err != nil {
return nil, errors.Errorf("getting object store: %w", err)
}
reader, _, err := store.Get(ctx, archivePath)
if err != nil {
return nil, errors.Errorf("getting charm: %w", err)
}
return reader, nil
}
25 changes: 25 additions & 0 deletions domain/application/charm/store/store_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import (
"io"
"os"
"path/filepath"
"strings"

"github.com/juju/testing"
jc "github.com/juju/testing/checkers"
Expand Down Expand Up @@ -111,6 +112,30 @@ func (s *storeSuite) TestStoreFailed(c *gc.C) {
c.Assert(err, gc.ErrorMatches, ".*boom")
}

func (s *storeSuite) TestGet(c *gc.C) {
defer s.setupMocks(c).Finish()

archive := io.NopCloser(strings.NewReader("archive-content"))
s.objectStore.EXPECT().Get(gomock.Any(), "foo").Return(archive, 0, nil)

storage := NewCharmStore(s.objectStoreGetter)
reader, err := storage.Get(context.Background(), "foo")
c.Assert(err, jc.ErrorIsNil)
content, err := io.ReadAll(reader)
c.Assert(err, jc.ErrorIsNil)
c.Check(string(content), gc.Equals, "archive-content")
}

func (s *storeSuite) TestGetFailed(c *gc.C) {
defer s.setupMocks(c).Finish()

s.objectStore.EXPECT().Get(gomock.Any(), "foo").Return(nil, 0, errors.Errorf("boom"))

storage := NewCharmStore(s.objectStoreGetter)
_, err := storage.Get(context.Background(), "foo")
c.Assert(err, gc.ErrorMatches, ".*boom")
}

func (s *storeSuite) setupMocks(c *gc.C) *gomock.Controller {
ctrl := gomock.NewController(c)

Expand Down
5 changes: 5 additions & 0 deletions domain/application/errors/errors.go
Original file line number Diff line number Diff line change
Expand Up @@ -127,6 +127,11 @@ const (
// wrong value.
CharmRelationRoleNotValid = errors.ConstError("charm relation role not valid")

// MultipleCharmHashes describes and error that occurs when a charm has multiple
// hash values. At the moment, we only support sha256 hash format, so if another
// is found, an error is returned.
MultipleCharmHashes = errors.ConstError("multiple charm hashes found")

// ResourceNotFound describes an error that occurs when a resource is
// not found.
ResourceNotFound = errors.ConstError("resource not found")
Expand Down
1 change: 1 addition & 0 deletions domain/application/service/application_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1236,6 +1236,7 @@ func (s *applicationWatcherServiceSuite) setupMocks(c *gc.C) *gomock.Controller
s.watcherFactory,
nil,
nil,
nil,
s.clock,
loggertesting.WrapCheckLog(c),
)
Expand Down
45 changes: 42 additions & 3 deletions domain/application/service/charm.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ package service
import (
"context"
"fmt"
"io"
"regexp"

"github.com/juju/errors"
Expand All @@ -17,6 +18,7 @@ import (
"github.com/juju/juju/domain/application/charm"
applicationerrors "github.com/juju/juju/domain/application/errors"
internalcharm "github.com/juju/juju/internal/charm"
internalerrors "github.com/juju/juju/internal/errors"
)

var (
Expand Down Expand Up @@ -99,7 +101,12 @@ type CharmState interface {
// GetCharmArchivePath returns the archive storage path for the charm using
// the charm ID. If the charm does not exist, a
// [applicationerrors.CharmNotFound] error is returned.
GetCharmArchivePath(ctx context.Context, charmID corecharm.ID) (string, error)
GetCharmArchivePath(context.Context, corecharm.ID) (string, error)

// GetCharmArchiveMetadata returns the archive storage path and hash for the
// charm using the charm ID.
// If the charm does not exist, a [errors.CharmNotFound] error is returned.
GetCharmArchiveMetadata(context.Context, corecharm.ID) (archivePath string, hash string, err error)

// IsCharmAvailable returns whether the charm is available for use. If the
// charm does not exist, a [applicationerrors.CharmNotFound] error is
Expand Down Expand Up @@ -137,6 +144,14 @@ type CharmState interface {
ListCharmsWithOriginByNames(ctx context.Context, names []string) ([]charm.CharmWithOrigin, error)
}

// CharmStore defines the interface for storing and retrieving charms archive blobs
// from the underlying storage.
type CharmStore interface {
// GetCharm retrieves a ReadCloser for the charm archive at the give path from
// the underlying storage.
Get(ctx context.Context, archivePath string) (io.ReadCloser, error)
}

// GetCharmID returns a charm ID by name. It returns an error if the charm
// can not be found by the name.
// This can also be used as a cheap way to see if a charm exists without
Expand Down Expand Up @@ -411,16 +426,40 @@ func (s *Service) GetCharmLXDProfile(ctx context.Context, id corecharm.ID) (inte
// returned.
func (s *Service) GetCharmArchivePath(ctx context.Context, id corecharm.ID) (string, error) {
if err := id.Validate(); err != nil {
return "", fmt.Errorf("charm id: %w", err)
return "", internalerrors.Errorf("charm id: %w", err)
}

path, err := s.st.GetCharmArchivePath(ctx, id)
if err != nil {
return "", errors.Trace(err)
return "", internalerrors.Errorf("getting charm archive path: %w", err)
}
return path, nil
}

// GetCharmArchive returns a ReadCloser stream for the charm archive for a given
// charm id, along with the hash of the charm archive. Clients can use the hash
// to verify the integrity of the charm archive.
//
// If the charm does not exist, a [applicationerrors.CharmNotFound] error is
// returned.
func (s *Service) GetCharmArchive(ctx context.Context, id corecharm.ID) (io.ReadCloser, string, error) {
if err := id.Validate(); err != nil {
return nil, "", internalerrors.Errorf("charm id: %w", err)
}

archivePath, hash, err := s.st.GetCharmArchiveMetadata(ctx, id)
if err != nil {
return nil, "", internalerrors.Errorf("getting charm archive metadata: %w", err)
}

reader, err := s.charmStore.Get(ctx, archivePath)
if err != nil {
return nil, "", internalerrors.Errorf("getting charm archive: %w", err)
}

return reader, hash, nil
}

// IsCharmAvailable returns whether the charm is available for use. This
// indicates if the charm has been uploaded to the controller.
// This will return true if the charm is available, and false otherwise.
Expand Down
20 changes: 20 additions & 0 deletions domain/application/service/charm_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@ package service

import (
"context"
"io"
"strings"

"github.com/juju/errors"
jc "github.com/juju/testing/checkers"
Expand Down Expand Up @@ -398,6 +400,24 @@ func (s *charmServiceSuite) TestGetCharmArchivePathInvalidUUID(c *gc.C) {
c.Assert(err, jc.ErrorIs, errors.NotValid)
}

func (s *charmServiceSuite) TestGetCharmArchive(c *gc.C) {
defer s.setupMocks(c).Finish()

id := charmtesting.GenCharmID(c)
archive := io.NopCloser(strings.NewReader("archive-content"))

s.state.EXPECT().GetCharmArchiveMetadata(gomock.Any(), id).Return("archive-path", "hash", nil)
s.charmStore.EXPECT().Get(gomock.Any(), "archive-path").Return(archive, nil)

reader, hash, err := s.service.GetCharmArchive(context.Background(), id)
c.Assert(err, jc.ErrorIsNil)
c.Check(hash, gc.Equals, "hash")

content, err := io.ReadAll(reader)
c.Assert(err, jc.ErrorIsNil)
c.Check(string(content), gc.Equals, "archive-content")
}

func (s *charmServiceSuite) TestSetCharmAvailable(c *gc.C) {
defer s.setupMocks(c).Finish()

Expand Down
107 changes: 105 additions & 2 deletions domain/application/service/package_mock_test.go

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

7 changes: 6 additions & 1 deletion domain/application/service/package_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ import (
dummystorage "github.com/juju/juju/internal/storage/provider/dummy"
)

//go:generate go run go.uber.org/mock/mockgen -typed -package service -destination package_mock_test.go github.com/juju/juju/domain/application/service State,DeleteSecretState,ResourceStoreGetter,WatcherFactory,AgentVersionGetter,Provider
//go:generate go run go.uber.org/mock/mockgen -typed -package service -destination package_mock_test.go github.com/juju/juju/domain/application/service State,DeleteSecretState,ResourceStoreGetter,WatcherFactory,AgentVersionGetter,Provider,CharmStore
//go:generate go run go.uber.org/mock/mockgen -typed -package service -destination charm_mock_test.go github.com/juju/juju/internal/charm Charm

func TestPackage(t *testing.T) {
Expand All @@ -41,6 +41,7 @@ type baseSuite struct {
state *MockState
charm *MockCharm
secret *MockDeleteSecretState
charmStore *MockCharmStore
agentVersionGetter *MockAgentVersionGetter
provider *MockProvider

Expand All @@ -67,6 +68,7 @@ func (s *baseSuite) setupMocksWithProvider(c *gc.C, fn func(ctx context.Context)
s.state = NewMockState(ctrl)
s.charm = NewMockCharm(ctrl)
s.secret = NewMockDeleteSecretState(ctrl)
s.charmStore = NewMockCharmStore(ctrl)

s.storageRegistryGetter = corestorage.ConstModelStorageRegistry(func() storage.ProviderRegistry {
return storage.ChainedProviderRegistry{
Expand All @@ -83,6 +85,7 @@ func (s *baseSuite) setupMocksWithProvider(c *gc.C, fn func(ctx context.Context)
s.modelID,
s.agentVersionGetter,
fn,
s.charmStore,
s.clock,
loggertesting.WrapCheckLog(c),
)
Expand All @@ -103,6 +106,7 @@ func (s *baseSuite) setupMocksWithAtomic(c *gc.C, fn func(domain.AtomicContext)
s.state = NewMockState(ctrl)
s.charm = NewMockCharm(ctrl)
s.secret = NewMockDeleteSecretState(ctrl)
s.charmStore = NewMockCharmStore(ctrl)

s.storageRegistryGetter = corestorage.ConstModelStorageRegistry(func() storage.ProviderRegistry {
return storage.ChainedProviderRegistry{
Expand All @@ -121,6 +125,7 @@ func (s *baseSuite) setupMocksWithAtomic(c *gc.C, fn func(domain.AtomicContext)
func(ctx context.Context) (Provider, error) {
return s.provider, nil
},
s.charmStore,
s.clock,
loggertesting.WrapCheckLog(c),
)
Expand Down
Loading

0 comments on commit eede4d8

Please sign in to comment.