From 86b1120fffea5ecac6a01589202b761f565706dd Mon Sep 17 00:00:00 2001 From: Jack Shaw Date: Mon, 30 Oct 2023 14:32:54 +0000 Subject: [PATCH 01/15] Use charm name string instead of charm URL core repository public methods DownloadCharm, GetDownloadMetadata, ResolveWithPreferredChannel, GetEssentialMetadata and ListResources all took a charm.URL param, but only needed the name of the charm. Simplify this to only need the charm name string This has the consequence that callers don't need to parse a URL to use these functions --- .../application/repository_mocks_test.go | 8 +- apiserver/facades/client/charms/client.go | 12 +- .../facades/client/charms/client_test.go | 59 ++++++---- .../facades/client/charms/mocks/repository.go | 8 +- cmd/jujud/agent/bootstrap_test.go | 2 +- cmd/jujud/agent/controllercharm.go | 2 +- core/charm/downloader/downloader.go | 34 +++--- core/charm/downloader/downloader_test.go | 28 ++--- core/charm/downloader/export_test.go | 6 +- core/charm/downloader/mocks/charm_mocks.go | 6 +- core/charm/repository.go | 12 +- core/charm/repository/charmhub.go | 106 +++++++++--------- core/charm/repository/charmhub_test.go | 47 ++++---- 13 files changed, 169 insertions(+), 161 deletions(-) diff --git a/apiserver/facades/client/application/repository_mocks_test.go b/apiserver/facades/client/application/repository_mocks_test.go index ecc4eacfa37..729ae7b2398 100644 --- a/apiserver/facades/client/application/repository_mocks_test.go +++ b/apiserver/facades/client/application/repository_mocks_test.go @@ -38,7 +38,7 @@ func (m *MockRepository) EXPECT() *MockRepositoryMockRecorder { } // DownloadCharm mocks base method. -func (m *MockRepository) DownloadCharm(arg0 *charm.URL, arg1 charm0.Origin, arg2 string) (charm0.CharmArchive, charm0.Origin, error) { +func (m *MockRepository) DownloadCharm(arg0 string, arg1 charm0.Origin, arg2 string) (charm0.CharmArchive, charm0.Origin, error) { m.ctrl.T.Helper() ret := m.ctrl.Call(m, "DownloadCharm", arg0, arg1, arg2) ret0, _ := ret[0].(charm0.CharmArchive) @@ -54,7 +54,7 @@ func (mr *MockRepositoryMockRecorder) DownloadCharm(arg0, arg1, arg2 interface{} } // GetDownloadURL mocks base method. -func (m *MockRepository) GetDownloadURL(arg0 *charm.URL, arg1 charm0.Origin) (*url.URL, charm0.Origin, error) { +func (m *MockRepository) GetDownloadURL(arg0 string, arg1 charm0.Origin) (*url.URL, charm0.Origin, error) { m.ctrl.T.Helper() ret := m.ctrl.Call(m, "GetDownloadURL", arg0, arg1) ret0, _ := ret[0].(*url.URL) @@ -89,7 +89,7 @@ func (mr *MockRepositoryMockRecorder) GetEssentialMetadata(arg0 ...interface{}) } // ListResources mocks base method. -func (m *MockRepository) ListResources(arg0 *charm.URL, arg1 charm0.Origin) ([]resource.Resource, error) { +func (m *MockRepository) ListResources(arg0 string, arg1 charm0.Origin) ([]resource.Resource, error) { m.ctrl.T.Helper() ret := m.ctrl.Call(m, "ListResources", arg0, arg1) ret0, _ := ret[0].([]resource.Resource) @@ -134,7 +134,7 @@ func (mr *MockRepositoryMockRecorder) ResolveResources(arg0, arg1 interface{}) * } // ResolveWithPreferredChannel mocks base method. -func (m *MockRepository) ResolveWithPreferredChannel(arg0 *charm.URL, arg1 charm0.Origin) (*charm.URL, charm0.Origin, []charm0.Platform, error) { +func (m *MockRepository) ResolveWithPreferredChannel(arg0 string, arg1 charm0.Origin) (*charm.URL, charm0.Origin, []charm0.Platform, error) { m.ctrl.T.Helper() ret := m.ctrl.Call(m, "ResolveWithPreferredChannel", arg0, arg1) ret0, _ := ret[0].(*charm.URL) diff --git a/apiserver/facades/client/charms/client.go b/apiserver/facades/client/charms/client.go index 33c767b5549..a3006ac5d1d 100644 --- a/apiserver/facades/client/charms/client.go +++ b/apiserver/facades/client/charms/client.go @@ -198,7 +198,7 @@ func (a *API) getDownloadInfo(arg params.CharmURLAndOrigin) (params.DownloadInfo if err != nil { return params.DownloadInfoResult{}, apiservererrors.ServerError(err) } - url, origin, err := repo.GetDownloadURL(curl, requestedOrigin) + url, origin, err := repo.GetDownloadURL(curl.Name, requestedOrigin) if err != nil { return params.DownloadInfoResult{}, apiservererrors.ServerError(err) } @@ -315,7 +315,7 @@ func (a *API) queueAsyncCharmDownload(args params.AddCharmWithAuth) (corecharm.O // to ensure that the resolved origin has the ID/Hash fields correctly // populated. if _, err := a.backendState.Charm(args.URL); err == nil { - _, resolvedOrigin, err := repo.GetDownloadURL(charmURL, requestedOrigin) + _, resolvedOrigin, err := repo.GetDownloadURL(charmURL.Name, requestedOrigin) return resolvedOrigin, errors.Trace(err) } @@ -323,8 +323,8 @@ func (a *API) queueAsyncCharmDownload(args params.AddCharmWithAuth) (corecharm.O // without downloading the full archive. The remaining metadata will // be populated once the charm gets downloaded. essentialMeta, err := repo.GetEssentialMetadata(corecharm.MetadataRequest{ - CharmURL: charmURL, - Origin: requestedOrigin, + CharmName: charmURL.Name, + Origin: requestedOrigin, }) if err != nil { return corecharm.Origin{}, errors.Annotatef(err, "retrieving essential metadata for charm %q", charmURL) @@ -389,7 +389,7 @@ func (a *API) resolveOneCharm(arg params.ResolveCharmWithChannel) params.Resolve return result } - resultURL, origin, resolvedBases, err := repo.ResolveWithPreferredChannel(curl, requestedOrigin) + resultURL, origin, resolvedBases, err := repo.ResolveWithPreferredChannel(curl.Name, requestedOrigin) if err != nil { result.Error = apiservererrors.ServerError(err) return result @@ -717,7 +717,7 @@ func (a *API) listOneCharmResources(arg params.CharmURLAndOrigin) ([]params.Char if err != nil { return nil, apiservererrors.ServerError(err) } - resources, err := repo.ListResources(curl, requestedOrigin) + resources, err := repo.ListResources(curl.Name, requestedOrigin) if err != nil { return nil, apiservererrors.ServerError(err) } diff --git a/apiserver/facades/client/charms/client_test.go b/apiserver/facades/client/charms/client_test.go index d3e76c21486..8ffe46420e6 100644 --- a/apiserver/facades/client/charms/client_test.go +++ b/apiserver/facades/client/charms/client_test.go @@ -201,7 +201,7 @@ func (s *charmsMockSuite) TestResolveCharms(c *gc.C) { expected := []params.ResolveCharmWithChannelResult{ { - URL: curl, + URL: seriesCurl, Origin: stableOrigin, SupportedBases: []params.Base{ {Name: "ubuntu", Channel: "18.04"}, @@ -209,7 +209,7 @@ func (s *charmsMockSuite) TestResolveCharms(c *gc.C) { {Name: "ubuntu", Channel: "16.04"}, }, }, { - URL: curl, + URL: seriesCurl, Origin: stableOrigin, SupportedBases: []params.Base{ {Name: "ubuntu", Channel: "18.04"}, @@ -254,7 +254,7 @@ func (s *charmsMockSuite) TestResolveCharmNoDefinedSeries(c *gc.C) { s.expectResolveWithPreferredChannelNoSeries() api := s.api(c) - seriesCurl := "ch:focal/testme" + seriesCurl := "ch:amd64/focal/testme" edgeOrigin := params.CharmOrigin{ Source: corecharm.CharmHub.String(), @@ -318,11 +318,11 @@ func (s *charmsMockSuite) TestResolveCharmV6(c *gc.C) { expected := []params.ResolveCharmWithChannelResultV6{ { - URL: curl, + URL: seriesCurl, Origin: stableOrigin, SupportedSeries: []string{"bionic", "focal", "xenial"}, }, { - URL: curl, + URL: seriesCurl, Origin: stableOrigin, SupportedSeries: []string{"bionic", "focal", "xenial"}, }, @@ -358,8 +358,7 @@ func (s *charmsMockSuite) TestAddCharmCharmhub(c *gc.C) { // Charmhub charms are downloaded asynchronously defer s.setupMocks(c).Finish() - curl, err := charm.ParseURL("chtest") - c.Assert(err, jc.ErrorIsNil) + curl := "chtest" requestedOrigin := corecharm.Origin{ Source: "charm-hub", @@ -382,15 +381,15 @@ func (s *charmsMockSuite) TestAddCharmCharmhub(c *gc.C) { }, } - s.state.EXPECT().Charm(curl.String()).Return(nil, errors.NotFoundf("%q", curl)) + s.state.EXPECT().Charm(curl).Return(nil, errors.NotFoundf("%q", curl)) s.repoFactory.EXPECT().GetCharmRepository(gomock.Any()).Return(s.repository, nil) expMeta := new(charm.Meta) expManifest := new(charm.Manifest) expConfig := new(charm.Config) s.repository.EXPECT().GetEssentialMetadata(corecharm.MetadataRequest{ - CharmURL: curl, - Origin: requestedOrigin, + CharmName: curl, + Origin: requestedOrigin, }).Return([]corecharm.EssentialMetadata{ { Meta: expMeta, @@ -402,7 +401,7 @@ func (s *charmsMockSuite) TestAddCharmCharmhub(c *gc.C) { s.state.EXPECT().AddCharmMetadata(gomock.Any()).DoAndReturn( func(ci state.CharmInfo) (*state.Charm, error) { - c.Assert(ci.ID, gc.DeepEquals, curl.String()) + c.Assert(ci.ID, gc.DeepEquals, curl) // Check that the essential metadata matches what // the repository returned. We use pointer checks here. c.Assert(ci.Charm.Meta(), gc.Equals, expMeta) @@ -415,7 +414,7 @@ func (s *charmsMockSuite) TestAddCharmCharmhub(c *gc.C) { api := s.api(c) args := params.AddCharmWithOrigin{ - URL: curl.String(), + URL: curl, Origin: params.CharmOrigin{ Source: "charm-hub", Base: params.Base{Name: "ubuntu", Channel: "20.04/stable"}, @@ -436,9 +435,8 @@ func (s *charmsMockSuite) TestAddCharmCharmhub(c *gc.C) { func (s *charmsMockSuite) TestQueueAsyncCharmDownloadResolvesAgainOriginForAlreadyDownloadedCharm(c *gc.C) { defer s.setupMocks(c).Finish() - curl, err := charm.ParseURL("chtest") - c.Assert(err, jc.ErrorIsNil) - resURL, err := url.Parse(curl.String()) + curl := "chtest" + resURL, err := url.Parse(curl) c.Assert(err, jc.ErrorIsNil) resolvedOrigin := corecharm.Origin{ @@ -452,14 +450,14 @@ func (s *charmsMockSuite) TestQueueAsyncCharmDownloadResolvesAgainOriginForAlrea }, } - s.state.EXPECT().Charm(curl.String()).Return(nil, nil) // a nil error indicates that the charm doc already exists + s.state.EXPECT().Charm(curl).Return(nil, nil) // a nil error indicates that the charm doc already exists s.repoFactory.EXPECT().GetCharmRepository(gomock.Any()).Return(s.repository, nil) s.repository.EXPECT().GetDownloadURL(curl, gomock.Any()).Return(resURL, resolvedOrigin, nil) api := s.api(c) args := params.AddCharmWithOrigin{ - URL: curl.String(), + URL: curl, Origin: params.CharmOrigin{ Source: "charm-hub", Risk: "edge", @@ -687,11 +685,10 @@ func (s *charmsMockSuite) setupMocks(c *gc.C) *gomock.Controller { func (s *charmsMockSuite) expectResolveWithPreferredChannel(times int, err error) { s.repoFactory.EXPECT().GetCharmRepository(gomock.Any()).Return(s.repository, nil).Times(times) s.repository.EXPECT().ResolveWithPreferredChannel( - gomock.AssignableToTypeOf(&charm.URL{}), + gomock.AssignableToTypeOf(""), gomock.AssignableToTypeOf(corecharm.Origin{}), ).DoAndReturn( - // Ensure the same curl that is provided, is returned. - func(curl *charm.URL, requestedOrigin corecharm.Origin) (*charm.URL, corecharm.Origin, []corecharm.Platform, error) { + func(name string, requestedOrigin corecharm.Origin) (*charm.URL, corecharm.Origin, []corecharm.Platform, error) { resolvedOrigin := requestedOrigin resolvedOrigin.Type = "charm" @@ -707,6 +704,14 @@ func (s *charmsMockSuite) expectResolveWithPreferredChannel(times int, err error {OS: "ubuntu", Channel: "20.04"}, {OS: "ubuntu", Channel: "16.04"}, } + // ensure the charm url returned is filled out + curl := &charm.URL{ + Schema: "ch", + Name: name, + Series: "focal", + Architecture: "amd64", + Revision: -1, + } return curl, resolvedOrigin, bases, err }).Times(times) } @@ -714,11 +719,10 @@ func (s *charmsMockSuite) expectResolveWithPreferredChannel(times int, err error func (s *charmsMockSuite) expectResolveWithPreferredChannelNoSeries() { s.repoFactory.EXPECT().GetCharmRepository(gomock.Any()).Return(s.repository, nil) s.repository.EXPECT().ResolveWithPreferredChannel( - gomock.AssignableToTypeOf(&charm.URL{}), + gomock.AssignableToTypeOf(""), gomock.AssignableToTypeOf(corecharm.Origin{}), ).DoAndReturn( - // Ensure the same curl that is provided, is returned. - func(curl *charm.URL, requestedOrigin corecharm.Origin) (*charm.URL, corecharm.Origin, []string, error) { + func(name string, requestedOrigin corecharm.Origin) (*charm.URL, corecharm.Origin, []string, error) { resolvedOrigin := requestedOrigin resolvedOrigin.Type = "charm" @@ -729,7 +733,14 @@ func (s *charmsMockSuite) expectResolveWithPreferredChannelNoSeries() { resolvedOrigin.Channel.Risk = "stable" } - + // ensure the charm url returned is filled out + curl := &charm.URL{ + Schema: "ch", + Name: name, + Series: "focal", + Architecture: "amd64", + Revision: -1, + } return curl, resolvedOrigin, []string{}, nil }) } diff --git a/apiserver/facades/client/charms/mocks/repository.go b/apiserver/facades/client/charms/mocks/repository.go index 665dbe63a6b..f6e2035f93d 100644 --- a/apiserver/facades/client/charms/mocks/repository.go +++ b/apiserver/facades/client/charms/mocks/repository.go @@ -38,7 +38,7 @@ func (m *MockRepository) EXPECT() *MockRepositoryMockRecorder { } // DownloadCharm mocks base method. -func (m *MockRepository) DownloadCharm(arg0 *charm.URL, arg1 charm0.Origin, arg2 string) (charm0.CharmArchive, charm0.Origin, error) { +func (m *MockRepository) DownloadCharm(arg0 string, arg1 charm0.Origin, arg2 string) (charm0.CharmArchive, charm0.Origin, error) { m.ctrl.T.Helper() ret := m.ctrl.Call(m, "DownloadCharm", arg0, arg1, arg2) ret0, _ := ret[0].(charm0.CharmArchive) @@ -54,7 +54,7 @@ func (mr *MockRepositoryMockRecorder) DownloadCharm(arg0, arg1, arg2 interface{} } // GetDownloadURL mocks base method. -func (m *MockRepository) GetDownloadURL(arg0 *charm.URL, arg1 charm0.Origin) (*url.URL, charm0.Origin, error) { +func (m *MockRepository) GetDownloadURL(arg0 string, arg1 charm0.Origin) (*url.URL, charm0.Origin, error) { m.ctrl.T.Helper() ret := m.ctrl.Call(m, "GetDownloadURL", arg0, arg1) ret0, _ := ret[0].(*url.URL) @@ -89,7 +89,7 @@ func (mr *MockRepositoryMockRecorder) GetEssentialMetadata(arg0 ...interface{}) } // ListResources mocks base method. -func (m *MockRepository) ListResources(arg0 *charm.URL, arg1 charm0.Origin) ([]resource.Resource, error) { +func (m *MockRepository) ListResources(arg0 string, arg1 charm0.Origin) ([]resource.Resource, error) { m.ctrl.T.Helper() ret := m.ctrl.Call(m, "ListResources", arg0, arg1) ret0, _ := ret[0].([]resource.Resource) @@ -134,7 +134,7 @@ func (mr *MockRepositoryMockRecorder) ResolveResources(arg0, arg1 interface{}) * } // ResolveWithPreferredChannel mocks base method. -func (m *MockRepository) ResolveWithPreferredChannel(arg0 *charm.URL, arg1 charm0.Origin) (*charm.URL, charm0.Origin, []charm0.Platform, error) { +func (m *MockRepository) ResolveWithPreferredChannel(arg0 string, arg1 charm0.Origin) (*charm.URL, charm0.Origin, []charm0.Platform, error) { m.ctrl.T.Helper() ret := m.ctrl.Call(m, "ResolveWithPreferredChannel", arg0, arg1) ret0, _ := ret[0].(*charm.URL) diff --git a/cmd/jujud/agent/bootstrap_test.go b/cmd/jujud/agent/bootstrap_test.go index 145a3e8cb81..f1fae11fc63 100644 --- a/cmd/jujud/agent/bootstrap_test.go +++ b/cmd/jujud/agent/bootstrap_test.go @@ -285,7 +285,7 @@ func (s *BootstrapSuite) TestStoreControllerCharm(c *gc.C) { storeCurl.Architecture = "amd64" storeOrigin := origin storeOrigin.Type = "charm" - repo.EXPECT().ResolveWithPreferredChannel(curl, origin).Return(&storeCurl, storeOrigin, nil, nil) + repo.EXPECT().ResolveWithPreferredChannel(curl.Name, origin).Return(&storeCurl, storeOrigin, nil, nil) downloader.EXPECT().DownloadAndStore(&storeCurl, storeOrigin, false). DoAndReturn(func(charmURL *charm.URL, requestedOrigin corecharm.Origin, force bool) (corecharm.Origin, error) { diff --git a/cmd/jujud/agent/controllercharm.go b/cmd/jujud/agent/controllercharm.go index 621617ab6e6..dc16652575b 100644 --- a/cmd/jujud/agent/controllercharm.go +++ b/cmd/jujud/agent/controllercharm.go @@ -186,7 +186,7 @@ func populateStoreControllerCharm(st *state.State, charmPath string, channel cha // error response. // // The controller charm doesn't have any series specific code. - curl, origin, _, err = charmRepo.ResolveWithPreferredChannel(curl, origin) + curl, origin, _, err = charmRepo.ResolveWithPreferredChannel(curl.Name, origin) if err != nil { return nil, nil, errors.Annotatef(err, "resolving %q", controllerCharmURL) } diff --git a/core/charm/downloader/downloader.go b/core/charm/downloader/downloader.go index a7fca417ef5..3ebca853a57 100644 --- a/core/charm/downloader/downloader.go +++ b/core/charm/downloader/downloader.go @@ -33,9 +33,9 @@ type CharmArchive interface { // CharmRepository provides an API for downloading charms/bundles. type CharmRepository interface { - GetDownloadURL(*charm.URL, corecharm.Origin) (*url.URL, corecharm.Origin, error) - ResolveWithPreferredChannel(charmURL *charm.URL, requestedOrigin corecharm.Origin) (*charm.URL, corecharm.Origin, []corecharm.Platform, error) - DownloadCharm(charmURL *charm.URL, requestedOrigin corecharm.Origin, archivePath string) (corecharm.CharmArchive, corecharm.Origin, error) + GetDownloadURL(string, corecharm.Origin) (*url.URL, corecharm.Origin, error) + ResolveWithPreferredChannel(charmName string, requestedOrigin corecharm.Origin) (*charm.URL, corecharm.Origin, []corecharm.Platform, error) + DownloadCharm(charmName string, requestedOrigin corecharm.Origin, archivePath string) (corecharm.CharmArchive, corecharm.Origin, error) } // RepositoryGetter returns a suitable CharmRepository for the specified Source. @@ -116,7 +116,7 @@ func (d *Downloader) DownloadAndStore(charmURL *charm.URL, requestedOrigin corec err error channelOrigin = requestedOrigin ) - channelOrigin.Platform, err = d.normalizePlatform(charmURL.String(), requestedOrigin.Platform) + channelOrigin.Platform, err = d.normalizePlatform(charmURL.Name, requestedOrigin.Platform) if err != nil { return corecharm.Origin{}, errors.Trace(err) } @@ -133,7 +133,7 @@ func (d *Downloader) DownloadAndStore(charmURL *charm.URL, requestedOrigin corec if err != nil { return corecharm.Origin{}, errors.Trace(err) } - _, resolvedOrigin, err := repo.GetDownloadURL(charmURL, requestedOrigin) + _, resolvedOrigin, err := repo.GetDownloadURL(charmURL.Name, requestedOrigin) return resolvedOrigin, errors.Trace(err) } @@ -157,31 +157,31 @@ func (d *Downloader) DownloadAndStore(charmURL *charm.URL, requestedOrigin corec return corecharm.Origin{}, errors.Trace(err) } - downloadedCharm, actualOrigin, err := d.downloadAndHash(charmURL, channelOrigin, repo, tmpFile.Name()) + downloadedCharm, actualOrigin, err := d.downloadAndHash(charmURL.Name, channelOrigin, repo, tmpFile.Name()) if err != nil { - return corecharm.Origin{}, errors.Annotatef(err, "downloading charm %q from origin %v", charmURL, requestedOrigin) + return corecharm.Origin{}, errors.Annotatef(err, "downloading charm %q from origin %v", charmURL.Name, requestedOrigin) } // Validate charm if err := downloadedCharm.verify(actualOrigin, force); err != nil { - return corecharm.Origin{}, errors.Annotatef(err, "verifying downloaded charm %q from origin %v", charmURL, requestedOrigin) + return corecharm.Origin{}, errors.Annotatef(err, "verifying downloaded charm %q from origin %v", charmURL.Name, requestedOrigin) } // Store Charm - if err := d.storeCharm(charmURL, downloadedCharm, tmpFile.Name()); err != nil { + if err := d.storeCharm(charmURL.String(), downloadedCharm, tmpFile.Name()); err != nil { return corecharm.Origin{}, errors.Annotatef(err, "storing charm %q from origin %v", charmURL, requestedOrigin) } return actualOrigin, nil } -func (d *Downloader) downloadAndHash(charmURL *charm.URL, requestedOrigin corecharm.Origin, repo CharmRepository, dstPath string) (DownloadedCharm, corecharm.Origin, error) { - d.logger.Debugf("downloading charm %q from requested origin %v", charmURL, requestedOrigin) - chArchive, actualOrigin, err := repo.DownloadCharm(charmURL, requestedOrigin, dstPath) +func (d *Downloader) downloadAndHash(charmName string, requestedOrigin corecharm.Origin, repo CharmRepository, dstPath string) (DownloadedCharm, corecharm.Origin, error) { + d.logger.Debugf("downloading charm %q from requested origin %v", charmName, requestedOrigin) + chArchive, actualOrigin, err := repo.DownloadCharm(charmName, requestedOrigin, dstPath) if err != nil { return DownloadedCharm{}, corecharm.Origin{}, errors.Trace(err) } - d.logger.Debugf("downloaded charm %q from actual origin %v", charmURL, actualOrigin) + d.logger.Debugf("downloaded charm %q from actual origin %v", charmName, actualOrigin) // Calculate SHA256 for the downloaded archive f, err := os.Open(dstPath) @@ -205,7 +205,7 @@ func (d *Downloader) downloadAndHash(charmURL *charm.URL, requestedOrigin corech }, actualOrigin, nil } -func (d *Downloader) storeCharm(charmURL *charm.URL, dc DownloadedCharm, archivePath string) error { +func (d *Downloader) storeCharm(charmURL string, dc DownloadedCharm, archivePath string) error { charmArchive, err := os.Open(archivePath) if err != nil { return errors.Annotatef(err, "unable to open downloaded charm archive at %q", archivePath) @@ -213,16 +213,16 @@ func (d *Downloader) storeCharm(charmURL *charm.URL, dc DownloadedCharm, archive defer func() { _ = charmArchive.Close() }() dc.CharmData = charmArchive - if err := d.storage.Store(charmURL.String(), dc); err != nil { + if err := d.storage.Store(charmURL, dc); err != nil { return errors.Trace(err) } return nil } -func (d *Downloader) normalizePlatform(charmURL string, platform corecharm.Platform) (corecharm.Platform, error) { +func (d *Downloader) normalizePlatform(charmName string, platform corecharm.Platform) (corecharm.Platform, error) { arc := platform.Architecture if platform.Architecture == "" || platform.Architecture == "all" { - d.logger.Warningf("received charm Architecture: %q, changing to %q, for charm %q", platform.Architecture, arch.DefaultArchitecture, charmURL) + d.logger.Warningf("received charm Architecture: %q, changing to %q, for charm %q", platform.Architecture, arch.DefaultArchitecture, charmName) arc = arch.DefaultArchitecture } diff --git a/core/charm/downloader/downloader_test.go b/core/charm/downloader/downloader_test.go index 271f9d5ded3..0cdedf65bdb 100644 --- a/core/charm/downloader/downloader_test.go +++ b/core/charm/downloader/downloader_test.go @@ -123,16 +123,16 @@ func (s *downloaderSuite) TestDownloadAndHash(c *gc.C) { tmpFile := filepath.Join(c.MkDir(), "ubuntu-lite.zip") c.Assert(os.WriteFile(tmpFile, []byte("meshuggah\n"), 0644), jc.ErrorIsNil) - curl := charm.MustParseURL("ch:ubuntu-lite") + name := "ch:ubuntu-lite" requestedOrigin := corecharm.Origin{Source: corecharm.CharmHub, Channel: mustParseChannel(c, "20.04/edge")} resolvedOrigin := corecharm.Origin{Source: corecharm.CharmHub, Channel: mustParseChannel(c, "20.04/candidate")} - s.repo.EXPECT().DownloadCharm(curl, requestedOrigin, tmpFile).Return(s.charmArchive, resolvedOrigin, nil) + s.repo.EXPECT().DownloadCharm(name, requestedOrigin, tmpFile).Return(s.charmArchive, resolvedOrigin, nil) s.charmArchive.EXPECT().Version().Return("the-version") s.charmArchive.EXPECT().LXDProfile().Return(nil) dl := s.newDownloader() - dc, gotOrigin, err := dl.DownloadAndHash(curl, requestedOrigin, repoAdapter{s.repo}, tmpFile) + dc, gotOrigin, err := dl.DownloadAndHash(name, requestedOrigin, repoAdapter{s.repo}, tmpFile) c.Assert(err, jc.ErrorIsNil) c.Assert(gotOrigin, gc.DeepEquals, resolvedOrigin, gc.Commentf("expected to get back the resolved origin")) c.Assert(dc.SHA256, gc.Equals, "4e97ed7423be2ea12939e8fdd592cfb3dcd4d0097d7d193ef998ab6b4db70461") @@ -151,7 +151,7 @@ func (s downloaderSuite) TestCharmAlreadyStored(c *gc.C) { ) s.repoGetter.EXPECT().GetCharmRepository(corecharm.CharmHub).Return(repoAdapter{s.repo}, nil) retURL, _ := url.Parse(curl.String()) - s.repo.EXPECT().GetDownloadURL(curl, requestedOrigin).Return(retURL, knownOrigin, nil) + s.repo.EXPECT().GetDownloadURL(curl.Name, requestedOrigin).Return(retURL, knownOrigin, nil) dl := s.newDownloader() gotOrigin, err := dl.DownloadAndStore(curl, requestedOrigin, false) @@ -176,13 +176,13 @@ func (s downloaderSuite) TestPrepareToStoreCharmError(c *gc.C) { } func (s downloaderSuite) TestNormalizePlatform(c *gc.C) { - curl := "ch:ubuntu-lite" + name := "ubuntu-lite" requestedPlatform := corecharm.Platform{ Channel: "20.04", OS: "Ubuntu", } - gotPlatform, err := s.newDownloader().NormalizePlatform(curl, requestedPlatform) + gotPlatform, err := s.newDownloader().NormalizePlatform(name, requestedPlatform) c.Assert(err, jc.ErrorIsNil) c.Assert(gotPlatform, gc.DeepEquals, corecharm.Platform{ Architecture: "amd64", @@ -228,8 +228,8 @@ func (s downloaderSuite) TestDownloadAndStore(c *gc.C) { }, ) s.repoGetter.EXPECT().GetCharmRepository(corecharm.CharmHub).Return(repoAdapter{s.repo}, nil) - s.repo.EXPECT().DownloadCharm(curl, requestedOriginWithPlatform, gomock.Any()).DoAndReturn( - func(_ *charm.URL, requestedOrigin corecharm.Origin, archivePath string) (downloader.CharmArchive, corecharm.Origin, error) { + s.repo.EXPECT().DownloadCharm(curl.Name, requestedOriginWithPlatform, gomock.Any()).DoAndReturn( + func(_ string, requestedOrigin corecharm.Origin, archivePath string) (downloader.CharmArchive, corecharm.Origin, error) { c.Assert(os.WriteFile(archivePath, []byte("meshuggah\n"), 0644), jc.ErrorIsNil) return s.charmArchive, resolvedOrigin, nil }, @@ -278,14 +278,14 @@ type repoAdapter struct { repo *mocks.MockCharmRepository } -func (r repoAdapter) DownloadCharm(charmURL *charm.URL, requestedOrigin corecharm.Origin, archivePath string) (corecharm.CharmArchive, corecharm.Origin, error) { - return r.repo.DownloadCharm(charmURL, requestedOrigin, archivePath) +func (r repoAdapter) DownloadCharm(charmName string, requestedOrigin corecharm.Origin, archivePath string) (corecharm.CharmArchive, corecharm.Origin, error) { + return r.repo.DownloadCharm(charmName, requestedOrigin, archivePath) } -func (r repoAdapter) ResolveWithPreferredChannel(charmURL *charm.URL, requestedOrigin corecharm.Origin) (*charm.URL, corecharm.Origin, []corecharm.Platform, error) { - return r.repo.ResolveWithPreferredChannel(charmURL, requestedOrigin) +func (r repoAdapter) ResolveWithPreferredChannel(charmName string, requestedOrigin corecharm.Origin) (*charm.URL, corecharm.Origin, []corecharm.Platform, error) { + return r.repo.ResolveWithPreferredChannel(charmName, requestedOrigin) } -func (r repoAdapter) GetDownloadURL(charmURL *charm.URL, requestedOrigin corecharm.Origin) (*url.URL, corecharm.Origin, error) { - return r.repo.GetDownloadURL(charmURL, requestedOrigin) +func (r repoAdapter) GetDownloadURL(charmName string, requestedOrigin corecharm.Origin) (*url.URL, corecharm.Origin, error) { + return r.repo.GetDownloadURL(charmName, requestedOrigin) } diff --git a/core/charm/downloader/export_test.go b/core/charm/downloader/export_test.go index 1fccef188f6..4d1c450d300 100644 --- a/core/charm/downloader/export_test.go +++ b/core/charm/downloader/export_test.go @@ -4,8 +4,6 @@ package downloader import ( - "github.com/juju/charm/v11" - corecharm "github.com/juju/juju/core/charm" ) @@ -17,6 +15,6 @@ func (d *Downloader) NormalizePlatform(charmURL string, platform corecharm.Platf return d.normalizePlatform(charmURL, platform) } -func (d *Downloader) DownloadAndHash(charmURL *charm.URL, requestedOrigin corecharm.Origin, repo CharmRepository, dstPath string) (DownloadedCharm, corecharm.Origin, error) { - return d.downloadAndHash(charmURL, requestedOrigin, repo, dstPath) +func (d *Downloader) DownloadAndHash(charmName string, requestedOrigin corecharm.Origin, repo CharmRepository, dstPath string) (DownloadedCharm, corecharm.Origin, error) { + return d.downloadAndHash(charmName, requestedOrigin, repo, dstPath) } diff --git a/core/charm/downloader/mocks/charm_mocks.go b/core/charm/downloader/mocks/charm_mocks.go index 406b0352aa4..2df5c6a39b4 100644 --- a/core/charm/downloader/mocks/charm_mocks.go +++ b/core/charm/downloader/mocks/charm_mocks.go @@ -172,7 +172,7 @@ func (m *MockCharmRepository) EXPECT() *MockCharmRepositoryMockRecorder { } // DownloadCharm mocks base method. -func (m *MockCharmRepository) DownloadCharm(arg0 *charm.URL, arg1 charm0.Origin, arg2 string) (charm0.CharmArchive, charm0.Origin, error) { +func (m *MockCharmRepository) DownloadCharm(arg0 string, arg1 charm0.Origin, arg2 string) (charm0.CharmArchive, charm0.Origin, error) { m.ctrl.T.Helper() ret := m.ctrl.Call(m, "DownloadCharm", arg0, arg1, arg2) ret0, _ := ret[0].(charm0.CharmArchive) @@ -188,7 +188,7 @@ func (mr *MockCharmRepositoryMockRecorder) DownloadCharm(arg0, arg1, arg2 interf } // GetDownloadURL mocks base method. -func (m *MockCharmRepository) GetDownloadURL(arg0 *charm.URL, arg1 charm0.Origin) (*url.URL, charm0.Origin, error) { +func (m *MockCharmRepository) GetDownloadURL(arg0 string, arg1 charm0.Origin) (*url.URL, charm0.Origin, error) { m.ctrl.T.Helper() ret := m.ctrl.Call(m, "GetDownloadURL", arg0, arg1) ret0, _ := ret[0].(*url.URL) @@ -204,7 +204,7 @@ func (mr *MockCharmRepositoryMockRecorder) GetDownloadURL(arg0, arg1 interface{} } // ResolveWithPreferredChannel mocks base method. -func (m *MockCharmRepository) ResolveWithPreferredChannel(arg0 *charm.URL, arg1 charm0.Origin) (*charm.URL, charm0.Origin, []charm0.Platform, error) { +func (m *MockCharmRepository) ResolveWithPreferredChannel(arg0 string, arg1 charm0.Origin) (*charm.URL, charm0.Origin, []charm0.Platform, error) { m.ctrl.T.Helper() ret := m.ctrl.Call(m, "ResolveWithPreferredChannel", arg0, arg1) ret0, _ := ret[0].(*charm.URL) diff --git a/core/charm/repository.go b/core/charm/repository.go index df54842f4fc..96546da2af7 100644 --- a/core/charm/repository.go +++ b/core/charm/repository.go @@ -16,18 +16,18 @@ type Repository interface { // GetDownloadURL returns a url from which a charm can be downloaded // based on the given charm url and charm origin. A charm origin // updated with the ID and hash for the download is also returned. - GetDownloadURL(*charm.URL, Origin) (*url.URL, Origin, error) + GetDownloadURL(string, Origin) (*url.URL, Origin, error) // DownloadCharm retrieves specified charm from the store and saves its // contents to the specified path. - DownloadCharm(charmURL *charm.URL, requestedOrigin Origin, archivePath string) (CharmArchive, Origin, error) + DownloadCharm(charmName string, requestedOrigin Origin, archivePath string) (CharmArchive, Origin, error) // ResolveWithPreferredChannel verified that the charm with the requested // channel exists. If no channel is specified, the latests, most stable is // used. It returns a charm URL which includes the most current revision, // if none was provided, a charm origin, and a slice of series supported by // this charm. - ResolveWithPreferredChannel(*charm.URL, Origin) (*charm.URL, Origin, []Platform, error) + ResolveWithPreferredChannel(string, Origin) (*charm.URL, Origin, []Platform, error) // GetEssentialMetadata resolves each provided MetadataRequest and // returns back a slice with the results. The results include the @@ -35,7 +35,7 @@ type Repository interface { GetEssentialMetadata(...MetadataRequest) ([]EssentialMetadata, error) // ListResources returns a list of resources associated with a given charm. - ListResources(*charm.URL, Origin) ([]charmresource.Resource, error) + ListResources(string, Origin) ([]charmresource.Resource, error) // ResolveResources looks at the provided repository and backend (already // downloaded) resources to determine which to use. Provided (uploaded) take @@ -64,8 +64,8 @@ type CharmArchive interface { // MetadataRequest encapsulates the arguments for a charm essential metadata // resolution request. type MetadataRequest struct { - CharmURL *charm.URL - Origin Origin + CharmName string + Origin Origin } // EssentialMetadata encapsulates the essential metadata required for deploying diff --git a/core/charm/repository/charmhub.go b/core/charm/repository/charmhub.go index 97f79499fa6..9c666f955a2 100644 --- a/core/charm/repository/charmhub.go +++ b/core/charm/repository/charmhub.go @@ -42,17 +42,17 @@ func NewCharmHubRepository(logger Logger, chClient CharmHubClient) *CharmHubRepo } } -// ResolveWithPreferredChannel defines a way using the given charm URL and +// ResolveWithPreferredChannel defines a way using the given charm name and // charm origin (platform and channel) to locate a matching charm against the // Charmhub API. -func (c *CharmHubRepository) ResolveWithPreferredChannel(charmURL *charm.URL, argOrigin corecharm.Origin) (*charm.URL, corecharm.Origin, []corecharm.Platform, error) { - c.logger.Tracef("Resolving CharmHub charm %q with origin %+v", charmURL, argOrigin) +func (c *CharmHubRepository) ResolveWithPreferredChannel(charmName string, argOrigin corecharm.Origin) (*charm.URL, corecharm.Origin, []corecharm.Platform, error) { + c.logger.Tracef("Resolving CharmHub charm %q with origin %+v", charmName, argOrigin) requestedOrigin, err := c.validateOrigin(argOrigin) if err != nil { return nil, corecharm.Origin{}, nil, err } - resCurl, outputOrigin, resolvedBases, _, err := c.resolveWithPreferredChannel(charmURL, requestedOrigin) + resCurl, outputOrigin, resolvedBases, _, err := c.resolveWithPreferredChannel(charmName, requestedOrigin) return resCurl, outputOrigin, resolvedBases, err } @@ -62,12 +62,12 @@ func (c *CharmHubRepository) ResolveWithPreferredChannel(charmURL *charm.URL, ar func (c *CharmHubRepository) ResolveForDeploy(arg corecharm.CharmID) (corecharm.ResolvedDataForDeploy, error) { c.logger.Tracef("Resolving CharmHub charm %q with origin %+v", arg.URL, arg.Origin) - resultURL, resolvedOrigin, _, resp, resolveErr := c.resolveWithPreferredChannel(arg.URL, arg.Origin) + resultURL, resolvedOrigin, _, resp, resolveErr := c.resolveWithPreferredChannel(arg.URL.Name, arg.Origin) if resolveErr != nil { return corecharm.ResolvedDataForDeploy{}, errors.Trace(resolveErr) } - essMeta, err := transformRefreshResult(resultURL, resp) + essMeta, err := transformRefreshResult(resultURL.Name, resp) if err != nil { return corecharm.ResolvedDataForDeploy{}, errors.Trace(err) } @@ -109,9 +109,9 @@ func (c *CharmHubRepository) ResolveForDeploy(arg corecharm.CharmID) (corecharm. // 3. In theory we could just return most of this information without the // re-request, but we end up with missing data and potential incorrect // charm downloads later. -func (c *CharmHubRepository) resolveWithPreferredChannel(charmURL *charm.URL, requestedOrigin corecharm.Origin) (*charm.URL, corecharm.Origin, []corecharm.Platform, transport.RefreshResponse, error) { +func (c *CharmHubRepository) resolveWithPreferredChannel(charmName string, requestedOrigin corecharm.Origin) (*charm.URL, corecharm.Origin, []corecharm.Platform, transport.RefreshResponse, error) { // First attempt to find the charm based on the only input provided. - response, err := c.refreshOne(charmURL, requestedOrigin) + response, err := c.refreshOne(charmName, requestedOrigin) if err != nil { return nil, corecharm.Origin{}, nil, transport.RefreshResponse{}, errors.Annotatef(err, "resolving with preferred channel") } @@ -129,7 +129,7 @@ func (c *CharmHubRepository) resolveWithPreferredChannel(charmURL *charm.URL, re ) switch { case response.Error != nil: - retryResult, err := c.retryResolveWithPreferredChannel(charmURL, requestedOrigin, response.Error) + retryResult, err := c.retryResolveWithPreferredChannel(charmName, requestedOrigin, response.Error) if err != nil { return nil, corecharm.Origin{}, nil, transport.RefreshResponse{}, errors.Trace(err) } @@ -174,21 +174,25 @@ func (c *CharmHubRepository) resolveWithPreferredChannel(charmURL *charm.URL, re // account for the closed tracks in a given channel. channel, err := charm.ParseChannelNormalize(effectiveChannel) if err != nil { - return nil, corecharm.Origin{}, nil, transport.RefreshResponse{}, errors.Annotatef(err, "invalid channel for %q", charmURL) + return nil, corecharm.Origin{}, nil, transport.RefreshResponse{}, errors.Annotatef(err, "invalid channel for %q", charmName) } // Ensure we send the updated charmURL back, with all the correct segments. revision := response.Entity.Revision - resCurl := charmURL. - WithArchitecture(chSuggestedOrigin.Platform.Architecture). - WithRevision(revision) // TODO(wallyworld) - does charm url still need a series? + var series string if chSuggestedOrigin.Platform.Channel != "" { - series, err := corebase.GetSeriesFromChannel(chSuggestedOrigin.Platform.OS, chSuggestedOrigin.Platform.Channel) + series, err = corebase.GetSeriesFromChannel(chSuggestedOrigin.Platform.OS, chSuggestedOrigin.Platform.Channel) if err != nil { return nil, corecharm.Origin{}, nil, transport.RefreshResponse{}, errors.Trace(err) } - resCurl = resCurl.WithSeries(series) + } + resCurl := &charm.URL{ + Schema: "ch", + Name: charmName, + Revision: revision, + Series: series, + Architecture: chSuggestedOrigin.Platform.Architecture, } // Create a resolved origin. Keep the original values for ID and Hash, if @@ -261,21 +265,21 @@ type retryResolveResult struct { // retryResolveWithPreferredChannel will attempt to inspect the transport // APIError and determine if a retry is possible with the information gathered // from the error. -func (c *CharmHubRepository) retryResolveWithPreferredChannel(charmURL *charm.URL, origin corecharm.Origin, resErr *transport.APIError) (*retryResolveResult, error) { +func (c *CharmHubRepository) retryResolveWithPreferredChannel(charmName string, origin corecharm.Origin, resErr *transport.APIError) (*retryResolveResult, error) { var ( err error bases []corecharm.Platform ) switch resErr.Code { case transport.ErrorCodeInvalidCharmPlatform, transport.ErrorCodeInvalidCharmBase: - c.logger.Tracef("Invalid charm platform %q %v - Default Base: %v", charmURL, origin, resErr.Extra.DefaultBases) + c.logger.Tracef("Invalid charm platform %q %v - Default Base: %v", charmName, origin, resErr.Extra.DefaultBases) if bases, err = c.selectNextBases(resErr.Extra.DefaultBases, origin); err != nil { return nil, errors.Annotatef(err, "selecting next bases") } case transport.ErrorCodeRevisionNotFound: - c.logger.Tracef("Revision not found %q %v - Releases: %v", charmURL, origin, resErr.Extra.Releases) + c.logger.Tracef("Revision not found %q %v - Releases: %v", charmName, origin, resErr.Extra.Releases) if bases, err = c.selectNextBasesFromReleases(resErr.Extra.Releases, origin); err != nil { return nil, errors.Annotatef(err, "selecting releases") @@ -298,11 +302,11 @@ func (c *CharmHubRepository) retryResolveWithPreferredChannel(charmURL *charm.UR origin.Platform.Channel = base.Channel if origin.Platform.Channel == "" { - return nil, errors.NotValidf("channel for %s", charmURL.Name) + return nil, errors.NotValidf("channel for %s", charmName) } - c.logger.Tracef("Refresh again with %q %v", charmURL, origin) - res, err := c.refreshOne(charmURL, origin) + c.logger.Tracef("Refresh again with %q %v", charmName, origin) + res, err := c.refreshOne(charmName, origin) if err != nil { return nil, errors.Annotatef(err, "retrying") } @@ -316,13 +320,13 @@ func (c *CharmHubRepository) retryResolveWithPreferredChannel(charmURL *charm.UR }, nil } -func transformRefreshResult(curl *charm.URL, refreshResult transport.RefreshResponse) (corecharm.EssentialMetadata, error) { +func transformRefreshResult(charmName string, refreshResult transport.RefreshResponse) (corecharm.EssentialMetadata, error) { if refreshResult.Entity.MetadataYAML == "" { - return corecharm.EssentialMetadata{}, errors.NotValidf("charmhub refresh response for %q does not include the contents of metadata.yaml", curl) + return corecharm.EssentialMetadata{}, errors.NotValidf("charmhub refresh response for %q does not include the contents of metadata.yaml", charmName) } chMeta, err := charm.ReadMeta(strings.NewReader(refreshResult.Entity.MetadataYAML)) if err != nil { - return corecharm.EssentialMetadata{}, errors.Annotatef(err, "parsing metadata.yaml for %q", curl) + return corecharm.EssentialMetadata{}, errors.Annotatef(err, "parsing metadata.yaml for %q", charmName) } configYAML := refreshResult.Entity.ConfigYAML @@ -336,7 +340,7 @@ func transformRefreshResult(curl *charm.URL, refreshResult transport.RefreshResp } else { chConfig, err = charm.ReadConfig(strings.NewReader(configYAML)) if err != nil { - return corecharm.EssentialMetadata{}, errors.Annotatef(err, "parsing config.yaml for %q", curl) + return corecharm.EssentialMetadata{}, errors.Annotatef(err, "parsing config.yaml for %q", charmName) } } @@ -344,7 +348,7 @@ func transformRefreshResult(curl *charm.URL, refreshResult transport.RefreshResp for _, base := range refreshResult.Entity.Bases { baseCh, err := charm.ParseChannelNormalize(base.Channel) if err != nil { - return corecharm.EssentialMetadata{}, errors.Annotatef(err, "parsing base channel for %q", curl) + return corecharm.EssentialMetadata{}, errors.Annotatef(err, "parsing base channel for %q", charmName) } chManifest.Bases = append(chManifest.Bases, charm.Base{ @@ -358,12 +362,12 @@ func transformRefreshResult(curl *charm.URL, refreshResult transport.RefreshResp // DownloadCharm retrieves specified charm from the store and saves its // contents to the specified path. -func (c *CharmHubRepository) DownloadCharm(charmURL *charm.URL, requestedOrigin corecharm.Origin, archivePath string) (corecharm.CharmArchive, corecharm.Origin, error) { - c.logger.Tracef("DownloadCharm %q, origin: %q", charmURL, requestedOrigin) +func (c *CharmHubRepository) DownloadCharm(charmName string, requestedOrigin corecharm.Origin, archivePath string) (corecharm.CharmArchive, corecharm.Origin, error) { + c.logger.Tracef("DownloadCharm %q, origin: %q", charmName, requestedOrigin) // Resolve charm URL to a link to the charm blob and keep track of the // actual resolved origin which may be different from the requested one. - resURL, actualOrigin, err := c.GetDownloadURL(charmURL, requestedOrigin) + resURL, actualOrigin, err := c.GetDownloadURL(charmName, requestedOrigin) if err != nil { return nil, corecharm.Origin{}, errors.Trace(err) } @@ -377,14 +381,14 @@ func (c *CharmHubRepository) DownloadCharm(charmURL *charm.URL, requestedOrigin } // GetDownloadURL returns the url from which to download the CharmHub charm/bundle -// defined by the provided curl and charm origin. An updated charm origin is +// defined by the provided charm name and origin. An updated charm origin is // also returned with the ID and hash for the charm to be downloaded. If the // provided charm origin has no ID, it is assumed that the charm is being // installed, not refreshed. -func (c *CharmHubRepository) GetDownloadURL(charmURL *charm.URL, requestedOrigin corecharm.Origin) (*url.URL, corecharm.Origin, error) { - c.logger.Tracef("GetDownloadURL %q, origin: %q", charmURL, requestedOrigin) +func (c *CharmHubRepository) GetDownloadURL(charmName string, requestedOrigin corecharm.Origin) (*url.URL, corecharm.Origin, error) { + c.logger.Tracef("GetDownloadURL %q, origin: %q", charmName, requestedOrigin) - refreshRes, err := c.refreshOne(charmURL, requestedOrigin) + refreshRes, err := c.refreshOne(charmName, requestedOrigin) if err != nil { return nil, corecharm.Origin{}, errors.Trace(err) } @@ -409,16 +413,16 @@ func (c *CharmHubRepository) GetDownloadURL(charmURL *charm.URL, requestedOrigin } // ListResources returns the resources for a given charm and origin. -func (c *CharmHubRepository) ListResources(charmURL *charm.URL, origin corecharm.Origin) ([]charmresource.Resource, error) { - c.logger.Tracef("ListResources %q", charmURL) +func (c *CharmHubRepository) ListResources(charmName string, origin corecharm.Origin) ([]charmresource.Resource, error) { + c.logger.Tracef("ListResources %q", charmName) - resCurl, resOrigin, _, err := c.ResolveWithPreferredChannel(charmURL, origin) + resCurl, resOrigin, _, err := c.ResolveWithPreferredChannel(charmName, origin) if isErrSelection(err) { var channel string if origin.Channel != nil { channel = origin.Channel.String() } - return nil, errors.Errorf("unable to locate charm %q with matching channel %q", charmURL.Name, channel) + return nil, errors.Errorf("unable to locate charm %q with matching channel %q", charmName, channel) } else if err != nil { return nil, errors.Trace(err) } @@ -429,7 +433,7 @@ func (c *CharmHubRepository) ListResources(charmURL *charm.URL, origin corecharm // specified. ListResources is used by the "charm-resources" cli cmd, // therefore specific charm revisions are less important. resOrigin.Revision = nil - resp, err := c.refreshOne(resCurl, resOrigin) + resp, err := c.refreshOne(resCurl.Name, resOrigin) if err != nil { return nil, errors.Trace(err) } @@ -453,7 +457,7 @@ func (c *CharmHubRepository) ListResources(charmURL *charm.URL, origin corecharm // downloaded) resources to determine which to use. Provided (uploaded) take // precedence. If charmhub has a newer resource than the back end, use that. func (c *CharmHubRepository) ResolveResources(resources []charmresource.Resource, id corecharm.CharmID) ([]charmresource.Resource, error) { - revisionResources, err := c.listResourcesIfRevisions(resources, id.URL) + revisionResources, err := c.listResourcesIfRevisions(resources, id.URL.Name) if err != nil { return nil, errors.Trace(err) } @@ -475,7 +479,7 @@ func (c *CharmHubRepository) ResolveResources(resources []charmresource.Resource return resolved, nil } -func (c *CharmHubRepository) listResourcesIfRevisions(resources []charmresource.Resource, curl *charm.URL) (map[string]charmresource.Resource, error) { +func (c *CharmHubRepository) listResourcesIfRevisions(resources []charmresource.Resource, charmName string) (map[string]charmresource.Resource, error) { results := make(map[string]charmresource.Resource, 0) for _, resource := range resources { // If not revision is specified, or the resource has already been @@ -483,9 +487,9 @@ func (c *CharmHubRepository) listResourcesIfRevisions(resources []charmresource. if resource.Revision == -1 || resource.Origin == charmresource.OriginUpload { continue } - refreshResp, err := c.client.ListResourceRevisions(context.TODO(), curl.Name, resource.Name) + refreshResp, err := c.client.ListResourceRevisions(context.TODO(), charmName, resource.Name) if err != nil { - return nil, errors.Annotatef(err, "refreshing charm %q", curl.String()) + return nil, errors.Annotatef(err, "refreshing charm %q", charmName) } if len(refreshResp) == 0 { return nil, errors.Errorf("no download refresh responses received") @@ -764,12 +768,12 @@ func (c *CharmHubRepository) GetEssentialMetadata(reqs ...corecharm.MetadataRequ for reqIdx, req := range reqs { // TODO(achilleasa): We should add support for resolving origin // batches and move this outside the loop. - _, resolvedOrigin, _, err := c.ResolveWithPreferredChannel(req.CharmURL, req.Origin) + _, resolvedOrigin, _, err := c.ResolveWithPreferredChannel(req.CharmName, req.Origin) if err != nil { - return nil, errors.Annotatef(err, "resolving origin for %q", req.CharmURL) + return nil, errors.Annotatef(err, "resolving origin for %q", req.CharmName) } - refreshCfg, err := refreshConfig(req.CharmURL, resolvedOrigin) + refreshCfg, err := refreshConfig(req.CharmName, resolvedOrigin) if err != nil { return nil, errors.Trace(err) } @@ -785,7 +789,7 @@ func (c *CharmHubRepository) GetEssentialMetadata(reqs ...corecharm.MetadataRequ var metaRes = make([]corecharm.EssentialMetadata, len(reqs)) for resIdx, refreshResult := range refreshResults { - essMeta, err := transformRefreshResult(reqs[resIdx].CharmURL, refreshResult) + essMeta, err := transformRefreshResult(reqs[resIdx].CharmName, refreshResult) if err != nil { return nil, err } @@ -796,8 +800,8 @@ func (c *CharmHubRepository) GetEssentialMetadata(reqs ...corecharm.MetadataRequ return metaRes, nil } -func (c *CharmHubRepository) refreshOne(charmURL *charm.URL, origin corecharm.Origin) (transport.RefreshResponse, error) { - cfg, err := refreshConfig(charmURL, origin) +func (c *CharmHubRepository) refreshOne(charmName string, origin corecharm.Origin) (transport.RefreshResponse, error) { + cfg, err := refreshConfig(charmName, origin) if err != nil { return transport.RefreshResponse{}, errors.Trace(err) } @@ -919,7 +923,7 @@ const ( // origin have a revision number in them when called by GetDownloadURL // to install a charm. Potentially causing an unexpected install by revision. // This is okay as all of the data is ready and correct in the origin. -func refreshConfig(charmURL *charm.URL, origin corecharm.Origin) (charmhub.RefreshConfig, error) { +func refreshConfig(charmName string, origin corecharm.Origin) (charmhub.RefreshConfig, error) { // Work out the correct install method. rev := -1 var method Method @@ -968,11 +972,11 @@ func refreshConfig(charmURL *charm.URL, origin corecharm.Origin) (charmhub.Refre // Install from just the name and the channel. If there is no origin ID, // we haven't downloaded this charm before. // Try channel first. - cfg, err = charmhub.InstallOneFromChannel(charmURL.Name, channel, base) + cfg, err = charmhub.InstallOneFromChannel(charmName, channel, base) case MethodRevision: // If there is a revision, install it using that. If there is no origin // ID, we haven't downloaded this charm before. - cfg, err = charmhub.InstallOneFromRevision(charmURL.Name, rev) + cfg, err = charmhub.InstallOneFromRevision(charmName, rev) case MethodID: // This must be a charm upgrade if we have an ID. Use the refresh // action for metric keeping on the CharmHub side. diff --git a/core/charm/repository/charmhub_test.go b/core/charm/repository/charmhub_test.go index daed16230a2..a3a1427bbf3 100644 --- a/core/charm/repository/charmhub_test.go +++ b/core/charm/repository/charmhub_test.go @@ -86,7 +86,7 @@ func (s *charmHubRepositorySuite) testResolve(c *gc.C, id string) { origin.InstanceKey = "instance-key" } - obtainedCurl, obtainedOrigin, obtainedBases, err := s.newClient().ResolveWithPreferredChannel(curl, origin) + obtainedCurl, obtainedOrigin, obtainedBases, err := s.newClient().ResolveWithPreferredChannel("wordpress", origin) c.Assert(err, jc.ErrorIsNil) curl.Revision = rev @@ -122,7 +122,7 @@ func (s *charmHubRepositorySuite) TestResolveWithChannel(c *gc.C) { }, } - obtainedCurl, obtainedOrigin, obtainedBases, err := s.newClient().ResolveWithPreferredChannel(curl, origin) + obtainedCurl, obtainedOrigin, obtainedBases, err := s.newClient().ResolveWithPreferredChannel("wordpress", origin) c.Assert(err, jc.ErrorIsNil) curl.Revision = 16 @@ -156,7 +156,7 @@ func (s *charmHubRepositorySuite) TestResolveWithoutBase(c *gc.C) { }, } - obtainedCurl, obtainedOrigin, obtainedBases, err := s.newClient().ResolveWithPreferredChannel(curl, origin) + obtainedCurl, obtainedOrigin, obtainedBases, err := s.newClient().ResolveWithPreferredChannel("wordpress", origin) c.Assert(err, jc.ErrorIsNil) curl.Revision = 16 @@ -263,7 +263,7 @@ func (s *charmHubRepositorySuite) TestResolveWithBundles(c *gc.C) { }, } - obtainedCurl, obtainedOrigin, obtainedBases, err := s.newClient().ResolveWithPreferredChannel(curl, origin) + obtainedCurl, obtainedOrigin, obtainedBases, err := s.newClient().ResolveWithPreferredChannel("core-kubernetes", origin) c.Assert(err, jc.ErrorIsNil) curl.Revision = 17 @@ -296,7 +296,7 @@ func (s *charmHubRepositorySuite) TestResolveInvalidPlatformError(c *gc.C) { }, } - obtainedCurl, obtainedOrigin, obtainedBases, err := s.newClient().ResolveWithPreferredChannel(curl, origin) + obtainedCurl, obtainedOrigin, obtainedBases, err := s.newClient().ResolveWithPreferredChannel("wordpress", origin) c.Assert(err, jc.ErrorIsNil) curl.Revision = 16 @@ -322,7 +322,6 @@ func (s *charmHubRepositorySuite) TestResolveRevisionNotFoundErrorWithNoSeries(c defer s.setupMocks(c).Finish() s.expectedRefreshRevisionNotFoundError(c) - curl := charm.MustParseURL("ch:wordpress") origin := corecharm.Origin{ Source: "charm-hub", Platform: corecharm.Platform{ @@ -330,7 +329,7 @@ func (s *charmHubRepositorySuite) TestResolveRevisionNotFoundErrorWithNoSeries(c }, } - _, _, _, err := s.newClient().ResolveWithPreferredChannel(curl, origin) + _, _, _, err := s.newClient().ResolveWithPreferredChannel("wordpress", origin) c.Assert(err, gc.ErrorMatches, `(?m)selecting releases: charm or bundle not found for channel "", platform "amd64" available releases are: @@ -352,7 +351,7 @@ func (s *charmHubRepositorySuite) TestResolveRevisionNotFoundError(c *gc.C) { }, } - obtainedCurl, obtainedOrigin, obtainedBases, err := s.newClient().ResolveWithPreferredChannel(curl, origin) + obtainedCurl, obtainedOrigin, obtainedBases, err := s.newClient().ResolveWithPreferredChannel("wordpress", origin) c.Assert(err, jc.ErrorIsNil) curl.Revision = 16 @@ -377,7 +376,6 @@ func (s *charmHubRepositorySuite) TestResolveRevisionNotFoundError(c *gc.C) { func (s *charmHubRepositorySuite) TestDownloadCharm(c *gc.C) { defer s.setupMocks(c).Finish() - curl := charm.MustParseURL("ch:wordpress") requestedOrigin := corecharm.Origin{ Source: "charm-hub", Platform: corecharm.Platform{ @@ -412,7 +410,7 @@ func (s *charmHubRepositorySuite) TestDownloadCharm(c *gc.C) { s.expectCharmRefreshInstallOneFromChannel(c) s.client.EXPECT().DownloadAndRead(context.TODO(), resolvedURL, "/tmp/foo").Return(resolvedArchive, nil) - gotArchive, gotOrigin, err := s.newClient().DownloadCharm(curl, requestedOrigin, "/tmp/foo") + gotArchive, gotOrigin, err := s.newClient().DownloadCharm("wordpress", requestedOrigin, "/tmp/foo") c.Assert(err, jc.ErrorIsNil) c.Assert(gotArchive, gc.Equals, resolvedArchive) // note: we are using gc.Equals to check the pointers here. c.Assert(gotOrigin, gc.DeepEquals, resolvedOrigin) @@ -421,7 +419,6 @@ func (s *charmHubRepositorySuite) TestDownloadCharm(c *gc.C) { func (s *charmHubRepositorySuite) TestGetDownloadURL(c *gc.C) { defer s.setupMocks(c).Finish() - curl := charm.MustParseURL("ch:wordpress") requestedOrigin := corecharm.Origin{ Source: "charm-hub", Platform: corecharm.Platform{ @@ -454,7 +451,7 @@ func (s *charmHubRepositorySuite) TestGetDownloadURL(c *gc.C) { s.expectCharmRefreshInstallOneFromChannel(c) - gotURL, gotOrigin, err := s.newClient().GetDownloadURL(curl, requestedOrigin) + gotURL, gotOrigin, err := s.newClient().GetDownloadURL("wordpress", requestedOrigin) c.Assert(err, jc.ErrorIsNil) c.Assert(gotURL, gc.DeepEquals, resolvedURL) c.Assert(gotOrigin, gc.DeepEquals, resolvedOrigin) @@ -463,7 +460,6 @@ func (s *charmHubRepositorySuite) TestGetDownloadURL(c *gc.C) { func (s *charmHubRepositorySuite) TestGetEssentialMetadata(c *gc.C) { defer s.setupMocks(c).Finish() - curl := charm.MustParseURL("ch:wordpress") requestedOrigin := corecharm.Origin{ Source: "charm-hub", Platform: corecharm.Platform{ @@ -481,8 +477,8 @@ func (s *charmHubRepositorySuite) TestGetEssentialMetadata(c *gc.C) { s.expectCharmRefreshInstallOneFromChannel(c) // refresh and get metadata got, err := s.newClient().GetEssentialMetadata(corecharm.MetadataRequest{ - CharmURL: curl, - Origin: requestedOrigin, + CharmName: "wordpress", + Origin: requestedOrigin, }) c.Assert(err, jc.ErrorIsNil) @@ -974,7 +970,7 @@ type refreshConfigSuite struct { var _ = gc.Suite(&refreshConfigSuite{}) func (refreshConfigSuite) TestRefreshByChannel(c *gc.C) { - curl := charm.MustParseURL("ch:wordpress") + name := "wordpress" platform := corecharm.MustParsePlatform("amd64/ubuntu/focal") channel := corecharm.MustParseChannel("latest/stable").Normalize() origin := corecharm.Origin{ @@ -982,7 +978,7 @@ func (refreshConfigSuite) TestRefreshByChannel(c *gc.C) { Channel: &channel, } - cfg, err := refreshConfig(curl, origin) + cfg, err := refreshConfig(name, origin) c.Assert(err, jc.ErrorIsNil) ch := channel.String() @@ -994,7 +990,7 @@ func (refreshConfigSuite) TestRefreshByChannel(c *gc.C) { Actions: []transport.RefreshRequestAction{{ Action: "install", InstanceKey: instanceKey, - Name: &curl.Name, + Name: &name, Channel: &ch, Base: &transport.Base{ Name: "ubuntu", @@ -1008,7 +1004,7 @@ func (refreshConfigSuite) TestRefreshByChannel(c *gc.C) { } func (refreshConfigSuite) TestRefreshByChannelVersion(c *gc.C) { - curl := charm.MustParseURL("ch:wordpress") + name := "wordpress" platform := corecharm.MustParsePlatform("amd64/ubuntu/20.10/latest") channel := corecharm.MustParseChannel("latest/stable").Normalize() origin := corecharm.Origin{ @@ -1016,7 +1012,7 @@ func (refreshConfigSuite) TestRefreshByChannelVersion(c *gc.C) { Channel: &channel, } - cfg, err := refreshConfig(curl, origin) + cfg, err := refreshConfig(name, origin) c.Assert(err, jc.ErrorIsNil) ch := channel.String() @@ -1028,7 +1024,7 @@ func (refreshConfigSuite) TestRefreshByChannelVersion(c *gc.C) { Actions: []transport.RefreshRequestAction{{ Action: "install", InstanceKey: instanceKey, - Name: &curl.Name, + Name: &name, Channel: &ch, Base: &transport.Base{ Name: "ubuntu", @@ -1043,14 +1039,14 @@ func (refreshConfigSuite) TestRefreshByChannelVersion(c *gc.C) { func (refreshConfigSuite) TestRefreshByRevision(c *gc.C) { revision := 1 - curl := charm.MustParseURL("ch:wordpress") + name := "wordpress" platform := corecharm.MustParsePlatform("amd64/ubuntu/focal") origin := corecharm.Origin{ Platform: platform, Revision: &revision, } - cfg, err := refreshConfig(curl, origin) + cfg, err := refreshConfig(name, origin) c.Assert(err, jc.ErrorIsNil) instanceKey := charmhub.ExtractConfigInstanceKey(cfg) @@ -1061,7 +1057,7 @@ func (refreshConfigSuite) TestRefreshByRevision(c *gc.C) { Actions: []transport.RefreshRequestAction{{ Action: "install", InstanceKey: instanceKey, - Name: &curl.Name, + Name: &name, Revision: &revision, }}, Context: []transport.RefreshRequestContext{}, @@ -1072,7 +1068,6 @@ func (refreshConfigSuite) TestRefreshByRevision(c *gc.C) { func (refreshConfigSuite) TestRefreshByID(c *gc.C) { id := "aaabbbccc" revision := 1 - curl := charm.MustParseURL("ch:wordpress") platform := corecharm.MustParsePlatform("amd64/ubuntu/focal") channel := corecharm.MustParseChannel("stable") origin := corecharm.Origin{ @@ -1084,7 +1079,7 @@ func (refreshConfigSuite) TestRefreshByID(c *gc.C) { InstanceKey: "instance-key", } - cfg, err := refreshConfig(curl, origin) + cfg, err := refreshConfig("wordpress", origin) c.Assert(err, jc.ErrorIsNil) instanceKey := charmhub.ExtractConfigInstanceKey(cfg) From 67cb267546872a00c82c1a6357854363be93aef9 Mon Sep 17 00:00:00 2001 From: Jordan Barrett Date: Tue, 7 Nov 2023 16:23:06 +0200 Subject: [PATCH 02/15] Bump juju/cmd to v3.0.14 --- go.mod | 2 +- go.sum | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/go.mod b/go.mod index 61ded302ed3..834d7ab840e 100644 --- a/go.mod +++ b/go.mod @@ -46,7 +46,7 @@ require ( github.com/juju/blobstore/v3 v3.0.2 github.com/juju/charm/v11 v11.0.2 github.com/juju/clock v1.0.3 - github.com/juju/cmd/v3 v3.0.13 + github.com/juju/cmd/v3 v3.0.14 github.com/juju/collections v1.0.4 github.com/juju/description/v4 v4.0.11 github.com/juju/errors v1.0.0 diff --git a/go.sum b/go.sum index 390d4a4cbaf..773a8b8208f 100644 --- a/go.sum +++ b/go.sum @@ -790,8 +790,8 @@ github.com/juju/clock v1.0.3 h1:yJHIsWXeU8j3QcBdiess09SzfiXRRrsjKPn2whnMeds= github.com/juju/clock v1.0.3/go.mod h1:HIBvJ8kiV/n7UHwKuCkdYL4l/MDECztHR2sAvWDxxf0= github.com/juju/cmd v0.0.0-20171107070456-e74f39857ca0/go.mod h1:yWJQHl73rdSX4DHVKGqkAip+huBslxRwS8m9CrOLq18= github.com/juju/cmd/v3 v3.0.0-20220202061353-b1cc80b193b0/go.mod h1:EoGJiEG+vbMwO9l+Es0SDTlaQPjH6nLcnnc4NfZB3cY= -github.com/juju/cmd/v3 v3.0.13 h1:lNK/dPmlZsh/9DZBGTjLrEv1AD75K3Nsnyo8WxJpNYE= -github.com/juju/cmd/v3 v3.0.13/go.mod h1:lGtDvm2BG+FKnIS8yY/vrhxQNX9imnL6bPIYGSTchuI= +github.com/juju/cmd/v3 v3.0.14 h1:KuuamArSH7vQ6SdQKEHYK2scEMkJTEZKLs8abrlW3XE= +github.com/juju/cmd/v3 v3.0.14/go.mod h1:lGtDvm2BG+FKnIS8yY/vrhxQNX9imnL6bPIYGSTchuI= github.com/juju/collections v0.0.0-20200605021417-0d0ec82b7271/go.mod h1:5XgO71dV1JClcOJE+4dzdn4HrI5LiyKd7PlVG6eZYhY= github.com/juju/collections v0.0.0-20220203020748-febd7cad8a7a/go.mod h1:JWeZdyttIEbkR51z2S13+J+aCuHVe0F6meRy+P0YGDo= github.com/juju/collections v1.0.0/go.mod h1:JWeZdyttIEbkR51z2S13+J+aCuHVe0F6meRy+P0YGDo= From b847b6a613a2dcce53dc57bad7a51d5b92774912 Mon Sep 17 00:00:00 2001 From: Jordan Barrett Date: Tue, 7 Nov 2023 17:08:28 +0200 Subject: [PATCH 03/15] Add empty file to provider/all package This allows you to compile Juju with just BUILD_TAGS='minimal' (no providers). --- provider/all/none.go | 8 ++++++++ 1 file changed, 8 insertions(+) create mode 100644 provider/all/none.go diff --git a/provider/all/none.go b/provider/all/none.go new file mode 100644 index 00000000000..cf3e3fe713c --- /dev/null +++ b/provider/all/none.go @@ -0,0 +1,8 @@ +// Copyright 2023 Canonical Ltd. +// Licensed under the AGPLv3, see LICENCE file for details. + +package all + +// This file is here so that you can compile Juju with no providers +// (i.e. BUILD_TAGS='minimal'). Otherwise, Go will complain that the build +// constraints exclude all Go files in this package. From 74a6e904c81e292f5de7c91365440b08e98e109a Mon Sep 17 00:00:00 2001 From: Jordan Barrett Date: Thu, 9 Nov 2023 08:40:21 +0200 Subject: [PATCH 04/15] minor changes to bootstrap help text Improve the formatting of the generated Markdown --- cmd/juju/commands/bootstrap.go | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/cmd/juju/commands/bootstrap.go b/cmd/juju/commands/bootstrap.go index 0f94e982e56..2a1c37bf5bc 100644 --- a/cmd/juju/commands/bootstrap.go +++ b/cmd/juju/commands/bootstrap.go @@ -73,7 +73,7 @@ Used without arguments, bootstrap will step you through the process of initializing a Juju cloud environment. Initialization consists of creating a 'controller' model and provisioning a machine to act as controller. -We recommend you call your controller ‘username-region’ e.g. ‘fred-us-east-1’ +We recommend you call your controller ‘username-region’ e.g. ‘fred-us-east-1’. See --clouds for a list of clouds and credentials. See --regions for a list of available regions for a given cloud. @@ -101,13 +101,13 @@ used with the MAAS provider ('--to .maas'). You can change the default timeout and retry delays used during the bootstrap by changing the following settings in your configuration (all values represent number of seconds): + # How long to wait for a connection to the controller - bootstrap-timeout: 1200 # default: 20 minutes - # How long to wait between connection attempts to a controller -address. - bootstrap-retry-delay: 5 # default: 5 seconds + bootstrap-timeout: 1200 # default: 20 minutes + # How long to wait between connection attempts to a controller address. + bootstrap-retry-delay: 5 # default: 5 seconds # How often to refresh controller addresses from the API server. - bootstrap-addresses-delay: 10 # default: 10 seconds + bootstrap-addresses-delay: 10 # default: 10 seconds It is possible to override the base e.g. ubuntu@22.04, Juju attempts to bootstrap on to, by supplying a base argument to '--bootstrap-base'. @@ -115,9 +115,9 @@ to bootstrap on to, by supplying a base argument to '--bootstrap-base'. An error is emitted if the determined base is not supported. Using the '--force' option to override this check: - juju bootstrap --bootstrap-base=ubuntu@22.04 --force + juju bootstrap --bootstrap-base=ubuntu@22.04 --force -The '--bootstrap-series' can be still used, but is deprecated in favour +The '--bootstrap-series' flag can be still used, but is deprecated in favour of '--bootstrap-base'. Private clouds may need to specify their own custom image metadata and @@ -135,9 +135,9 @@ The agent version can be specified a simple numeric version, e.g. 2.2.4. For example, at the time when 2.3.0, 2.3.1 and 2.3.2 are released and your agent stream is 'released' (default), then a 2.3.1 client can bootstrap: - * 2.3.0 controller by running '... bootstrap --agent-version=2.3.0 ...'; - * 2.3.1 controller by running '... bootstrap ...'; - * 2.3.2 controller by running 'bootstrap --auto-upgrade'. + * 2.3.0 controller by running '... bootstrap --agent-version=2.3.0 ...'; + * 2.3.1 controller by running '... bootstrap ...'; + * 2.3.2 controller by running 'bootstrap --auto-upgrade'. However, if this client has a copy of codebase, then a local copy of Juju will be built and bootstrapped - 2.3.1.1. From aea2a13944af772c5d8a04d81a42273f4ccc314b Mon Sep 17 00:00:00 2001 From: Kelvin Liu Date: Thu, 9 Nov 2023 17:59:48 +1100 Subject: [PATCH 05/15] Wait for vault ready; --- tests/suites/secrets_k8s/k8s.sh | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/tests/suites/secrets_k8s/k8s.sh b/tests/suites/secrets_k8s/k8s.sh index cfa591f7e9c..2b7e4182a7b 100644 --- a/tests/suites/secrets_k8s/k8s.sh +++ b/tests/suites/secrets_k8s/k8s.sh @@ -301,7 +301,16 @@ prepare_vault() { export VAULT_ADDR="http://${ip}:8200" export VAULT_TOKEN="$root_token" - vault status + # wait for vault server to be ready. + attempt=0 + until [[ $(vault status -format yaml 2>/dev/null | yq .initialized | grep -i 'true') ]]; do + if [[ ${attempt} -ge 30 ]]; then + echo "Failed: vault server was not able to be ready." + exit 1 + fi + sleep 2 + attempt=$((attempt + 1)) + done } test_secrets() { From 02479f4a1bfd01797900def1d5fb0a601a42f082 Mon Sep 17 00:00:00 2001 From: Ian Booth Date: Thu, 9 Nov 2023 23:21:48 +1000 Subject: [PATCH 06/15] Bump version to 3.3.0 --- scripts/win-installer/setup.iss | 2 +- snap/snapcraft.yaml | 2 +- version/version.go | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/scripts/win-installer/setup.iss b/scripts/win-installer/setup.iss index e796a94e735..0695c8ec01c 100644 --- a/scripts/win-installer/setup.iss +++ b/scripts/win-installer/setup.iss @@ -4,7 +4,7 @@ #if GetEnv('JUJU_VERSION') != "" #define MyAppVersion=GetEnv('JUJU_VERSION') #else -#define MyAppVersion="3.3-rc3" +#define MyAppVersion="3.3.0" #endif #define MyAppName "Juju" diff --git a/snap/snapcraft.yaml b/snap/snapcraft.yaml index cde024ebcca..ce42ec585d1 100644 --- a/snap/snapcraft.yaml +++ b/snap/snapcraft.yaml @@ -1,5 +1,5 @@ name: juju -version: 3.3-rc3 +version: 3.3.0 summary: Juju - a model-driven operator lifecycle manager for K8s and machines license: AGPL-3.0 description: | diff --git a/version/version.go b/version/version.go index c96e2dc3ad3..249338c2d78 100644 --- a/version/version.go +++ b/version/version.go @@ -18,7 +18,7 @@ import ( // The presence and format of this constant is very important. // The debian/rules build recipe uses this value for the version // number of the release package. -const version = "3.3-rc3" +const version = "3.3.0" // UserAgentVersion defines a user agent version used for communication for // outside resources. From 1f177afa8eba12d7f01a0d5ba0321ab53c9af4d4 Mon Sep 17 00:00:00 2001 From: jujubot Date: Thu, 9 Nov 2023 19:14:51 +0000 Subject: [PATCH 07/15] Increment juju to 3.3.1 --- scripts/win-installer/setup.iss | 2 +- snap/snapcraft.yaml | 2 +- version/version.go | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/scripts/win-installer/setup.iss b/scripts/win-installer/setup.iss index 0695c8ec01c..d4a37fbbb27 100644 --- a/scripts/win-installer/setup.iss +++ b/scripts/win-installer/setup.iss @@ -4,7 +4,7 @@ #if GetEnv('JUJU_VERSION') != "" #define MyAppVersion=GetEnv('JUJU_VERSION') #else -#define MyAppVersion="3.3.0" +#define MyAppVersion="3.3.1" #endif #define MyAppName "Juju" diff --git a/snap/snapcraft.yaml b/snap/snapcraft.yaml index ce42ec585d1..a45870bb248 100644 --- a/snap/snapcraft.yaml +++ b/snap/snapcraft.yaml @@ -1,5 +1,5 @@ name: juju -version: 3.3.0 +version: 3.3.1 summary: Juju - a model-driven operator lifecycle manager for K8s and machines license: AGPL-3.0 description: | diff --git a/version/version.go b/version/version.go index 249338c2d78..c17e95ae238 100644 --- a/version/version.go +++ b/version/version.go @@ -18,7 +18,7 @@ import ( // The presence and format of this constant is very important. // The debian/rules build recipe uses this value for the version // number of the release package. -const version = "3.3.0" +const version = "3.3.1" // UserAgentVersion defines a user agent version used for communication for // outside resources. From adf9b6d9fc22439d5c120a95ec07ab1bb08cd481 Mon Sep 17 00:00:00 2001 From: Simon Richardson Date: Fri, 10 Nov 2023 07:07:59 +0000 Subject: [PATCH 08/15] Removal of lxd retry logic The following was added in [1] to help with locating the vm if it didn't start up in time. This isn't required any more as the vm correctly starts up cleanly. In addition the retry was at level that causes retries for requests that don't expect retries to happen. This was identified in bug[2] as a failure. One thing we should follow up on is the number of cores to run a vm for bootstrap should be a minimum of 2. 1. https://github.com/juju/juju/pull/15004 2. https://bugs.launchpad.net/juju/+bug/2029524 --- container/lxd/container.go | 36 ------------------------------------ 1 file changed, 36 deletions(-) diff --git a/container/lxd/container.go b/container/lxd/container.go index 53e04dbf81f..3cf8fb5e28a 100644 --- a/container/lxd/container.go +++ b/container/lxd/container.go @@ -199,42 +199,6 @@ func (s *Server) AliveContainers(prefix string) ([]Container, error) { // FilterContainers retrieves the list of containers from the server and filters // them based on the input namespace prefix and any supplied statuses. func (s *Server) FilterContainers(prefix string, statuses ...string) ([]Container, error) { - // The retry here is required here because when creating a virtual machine - // container type, it does not always appear in the list of containers. - // There doesn't seem to be a status that's available to us that indicates - // that the machine is being created, but not yet quite alive. - // - // As we're trying to not have any logic that differentiates between - // containers and virtual machines, at a higher level, we retry here to - // prevent leaking the difference. - var containers []Container - err := retry.Call(retry.CallArgs{ - Func: func() error { - var err error - containers, err = s.filterContainers(prefix, statuses) - return err - }, - IsFatalError: func(err error) bool { - return !errors.Is(err, errors.NotFound) - }, - NotifyFunc: func(err error, attempt int) { - logger.Debugf("failed to retrieve containers from LXD, attempt %d: %v", attempt, err) - }, - Attempts: 10, - Delay: 1 * time.Second, - MaxDelay: 5 * time.Second, - MaxDuration: 30 * time.Second, - Clock: s.clock, - BackoffFunc: retry.ExpBackoff(1*time.Second, 10*time.Second, 2.0, true), - }) - if err != nil && !errors.Is(err, errors.NotFound) { - // No containers found is not an error. - return nil, errors.Trace(err) - } - return containers, nil -} - -func (s *Server) filterContainers(prefix string, statuses []string) ([]Container, error) { instances, err := s.GetInstances(api.InstanceTypeAny) if err != nil { return nil, errors.Trace(err) From d3db2684f14c21863a91e27363beccd3abed9dd6 Mon Sep 17 00:00:00 2001 From: Jordan Barrett Date: Fri, 10 Nov 2023 10:28:34 +0200 Subject: [PATCH 09/15] add doc.go for charmrevision[updater] worker --- worker/charmrevision/doc.go | 13 +++++++++++++ 1 file changed, 13 insertions(+) create mode 100644 worker/charmrevision/doc.go diff --git a/worker/charmrevision/doc.go b/worker/charmrevision/doc.go new file mode 100644 index 00000000000..7d53d9ca5f8 --- /dev/null +++ b/worker/charmrevision/doc.go @@ -0,0 +1,13 @@ +// Copyright 2023 Canonical Ltd. +// Licensed under the AGPLv3, see LICENCE file for details. + +// Package charmrevision defines the charm revision updater worker. This worker +// is responsible for polling Charmhub every 24 hours to check if there are new +// revisions available of any charm deployed in the model. If so, it will put a +// document in the Juju database, so that the next time the user runs +// `juju status`, they can see that there is an update available. This worker +// also sends anonymised usage metrics to Charmhub when it polls. +// +// This worker doesn't contain much business logic - most of the work is +// delegated to the facade call. +package charmrevision From aad4e40ab5553186f0458e746406ebd69f6673b6 Mon Sep 17 00:00:00 2001 From: Simon Richardson Date: Fri, 10 Nov 2023 09:05:03 +0000 Subject: [PATCH 10/15] Bootstrapping a lxd virtual machine requires better defaults Bootstrapping a lxd virtual machine requires at least some CPU and memory constraints, otherwise it doesn't come up without specifying them. The solution is to allow bootstrap constraints on the lxd provider if the virt-type is a virtual-machine. --- environs/bootstrap/bootstrap.go | 10 ++++++++-- environs/interface.go | 2 +- provider/lxd/environ_policy.go | 5 ++++- provider/lxd/environ_policy_test.go | 27 +++++++++++++++++++++++++++ 4 files changed, 40 insertions(+), 4 deletions(-) diff --git a/environs/bootstrap/bootstrap.go b/environs/bootstrap/bootstrap.go index b0a794bea23..87b6e84589e 100644 --- a/environs/bootstrap/bootstrap.go +++ b/environs/bootstrap/bootstrap.go @@ -214,10 +214,16 @@ func withDefaultControllerConstraints(cons constraints.Value) constraints.Value // A default of 3.5GiB will result in machines with up to 4GiB of memory, eg // - 3.75GiB on AWS, Google // - 3.5GiB on Azure - // - 4GiB on Rackspace etc var mem uint64 = 3.5 * 1024 cons.Mem = &mem } + // If we're bootstrapping a controller on a lxd virtual machine, we want to + // ensure that it has at least 2 cores. Less than 2 cores can cause the + // controller to become unresponsive when installing. + if !cons.HasCpuCores() && cons.HasVirtType() && *cons.VirtType == "virtual-machine" { + var cores = uint64(2) + cons.CpuCores = &cores + } return cons } @@ -445,7 +451,7 @@ func bootstrapIAAS( // The follow is used to determine if we should apply the default // constraints when we bootstrap. Generally speaking this should always be // applied, but there are exceptions to the rule e.g. local LXD - if checker, ok := environ.(environs.DefaultConstraintsChecker); !ok || checker.ShouldApplyControllerConstraints() { + if checker, ok := environ.(environs.DefaultConstraintsChecker); !ok || checker.ShouldApplyControllerConstraints(bootstrapConstraints) { bootstrapConstraints = withDefaultControllerConstraints(bootstrapConstraints) } bootstrapParams.BootstrapConstraints = bootstrapConstraints diff --git a/environs/interface.go b/environs/interface.go index 56bee888048..f37bb0d1238 100644 --- a/environs/interface.go +++ b/environs/interface.go @@ -627,7 +627,7 @@ type PrecheckJujuUpgradeStep interface { type DefaultConstraintsChecker interface { // ShouldApplyControllerConstraints returns if bootstrapping logic should // use default constraints - ShouldApplyControllerConstraints() bool + ShouldApplyControllerConstraints(constraints.Value) bool } // HardwareCharacteristicsDetector is implemented by environments that can diff --git a/provider/lxd/environ_policy.go b/provider/lxd/environ_policy.go index de4ab83ec5b..7bb68c4a8f2 100644 --- a/provider/lxd/environ_policy.go +++ b/provider/lxd/environ_policy.go @@ -39,7 +39,10 @@ func (env *environ) ConstraintsValidator(ctx context.ProviderCallContext) (const // ShouldApplyControllerConstraints returns if bootstrapping logic should use // default constraints -func (env *environ) ShouldApplyControllerConstraints() bool { +func (env *environ) ShouldApplyControllerConstraints(cons constraints.Value) bool { + if cons.HasVirtType() && *cons.VirtType == "virtual-machine" { + return true + } return false } diff --git a/provider/lxd/environ_policy_test.go b/provider/lxd/environ_policy_test.go index c4c6e86e650..82a96975957 100644 --- a/provider/lxd/environ_policy_test.go +++ b/provider/lxd/environ_policy_test.go @@ -238,6 +238,33 @@ func (s *environPolicySuite) TestSupportNetworks(c *gc.C) { c.Check(isSupported, jc.IsFalse) } +func (s *environPolicySuite) TestShouldApplyControllerConstraints(c *gc.C) { + defer s.setupMocks(c).Finish() + + cons := constraints.MustParse("") + + ok := s.env.(environs.DefaultConstraintsChecker).ShouldApplyControllerConstraints(cons) + c.Assert(ok, jc.IsFalse) +} + +func (s *environPolicySuite) TestShouldApplyControllerConstraintsInvalid(c *gc.C) { + defer s.setupMocks(c).Finish() + + cons := constraints.MustParse("virt-type=invalid") + + ok := s.env.(environs.DefaultConstraintsChecker).ShouldApplyControllerConstraints(cons) + c.Assert(ok, jc.IsFalse) +} + +func (s *environPolicySuite) TestShouldApplyControllerConstraintsForVirtualMachine(c *gc.C) { + defer s.setupMocks(c).Finish() + + cons := constraints.MustParse("virt-type=virtual-machine") + + ok := s.env.(environs.DefaultConstraintsChecker).ShouldApplyControllerConstraints(cons) + c.Assert(ok, jc.IsTrue) +} + func (s *environPolicySuite) setupMocks(c *gc.C) *gomock.Controller { ctrl := gomock.NewController(c) From 721555f02ae4cd1e56b45a83f2025191e927245e Mon Sep 17 00:00:00 2001 From: Max Asnaashari Date: Wed, 8 Nov 2023 11:26:06 +0000 Subject: [PATCH 11/15] container/lxd: Check error before checking image field If an image is not found, we should check the error first before attempting to check a field on the returned nil object to avoid a panic. Signed-off-by: Max Asnaashari --- container/lxd/image.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/container/lxd/image.go b/container/lxd/image.go index 55829250eb3..bd6190c067f 100644 --- a/container/lxd/image.go +++ b/container/lxd/image.go @@ -60,7 +60,7 @@ func (s *Server) FindImage( // We already have an image with the given alias, so just use that. target = entry.Target image, _, err := s.GetImage(target) - if isCompatibleVirtType(virtType, image.Type) && err == nil { + if err == nil && isCompatibleVirtType(virtType, image.Type) { logger.Debugf("Found image locally - %q %q", image.Filename, target) return SourcedImage{ Image: image, From d7c393ab4a4b7fb773fe6c8769619c89eeb69901 Mon Sep 17 00:00:00 2001 From: Simon Richardson Date: Fri, 10 Nov 2023 09:37:03 +0000 Subject: [PATCH 12/15] Update the mocks for environs testing The mocks were out of date, so a new generate fixes this. --- container/lxd/container.go | 3 -- container/lxd/container_test.go | 70 -------------------------------- container/lxd/manager_test.go | 5 --- environs/testing/package_mock.go | 8 ++-- 4 files changed, 4 insertions(+), 82 deletions(-) diff --git a/container/lxd/container.go b/container/lxd/container.go index 3cf8fb5e28a..da9541c0901 100644 --- a/container/lxd/container.go +++ b/container/lxd/container.go @@ -214,9 +214,6 @@ func (s *Server) FilterContainers(prefix string, statuses ...string) ([]Containe } results = append(results, Container{c}) } - if len(results) == 0 { - return nil, errors.NotFoundf("containers with prefix %q", prefix) - } return results, nil } diff --git a/container/lxd/container_test.go b/container/lxd/container_test.go index 7c7763ce253..5b0fa764ed9 100644 --- a/container/lxd/container_test.go +++ b/container/lxd/container_test.go @@ -131,76 +131,6 @@ func (s *containerSuite) TestFilterContainers(c *gc.C) { c.Check(filtered, gc.DeepEquals, expected) } -func (s *containerSuite) TestFilterContainersRetry(c *gc.C) { - ctrl := gomock.NewController(c) - defer ctrl.Finish() - cSvr := s.NewMockServer(ctrl) - - for i := 0; i < 3; i++ { - ret := []api.Instance{ - { - Name: "prefix-c1", - StatusCode: api.Pending, - }, - { - Name: "prefix-c2", - StatusCode: api.Pending, - }, - { - Name: "prefix-c3", - StatusCode: api.Pending, - }, - { - Name: "not-prefix-c4", - StatusCode: api.Pending, - }, - } - cSvr.EXPECT().GetInstances(api.InstanceTypeAny).Return(ret, nil) - } - - matching := []api.Instance{ - { - Name: "prefix-c1", - StatusCode: api.Starting, - }, - { - Name: "prefix-c2", - StatusCode: api.Stopped, - }, - } - ret := append(matching, []api.Instance{ - { - Name: "prefix-c3", - StatusCode: api.Started, - }, - { - Name: "not-prefix-c4", - StatusCode: api.Stopped, - }, - }...) - cSvr.EXPECT().GetInstances(api.InstanceTypeAny).Return(ret, nil) - - clock := mocks.NewMockClock(ctrl) - clock.EXPECT().Now().Return(time.Now()).AnyTimes() - clock.EXPECT().After(gomock.Any()).DoAndReturn(func(d time.Duration) <-chan time.Time { - c := make(chan time.Time, 1) - defer close(c) - return c - }).AnyTimes() - - jujuSvr, err := lxd.NewTestingServer(cSvr, clock) - c.Assert(err, jc.ErrorIsNil) - - filtered, err := jujuSvr.FilterContainers("prefix", "Starting", "Stopped") - c.Assert(err, jc.ErrorIsNil) - - expected := make([]lxd.Container, len(matching)) - for i, v := range matching { - expected[i] = lxd.Container{v} - } - c.Check(filtered, gc.DeepEquals, expected) -} - func (s *containerSuite) TestAliveContainers(c *gc.C) { ctrl := gomock.NewController(c) defer ctrl.Finish() diff --git a/container/lxd/manager_test.go b/container/lxd/manager_test.go index 3dcec490b35..edea8e4bff0 100644 --- a/container/lxd/manager_test.go +++ b/container/lxd/manager_test.go @@ -5,7 +5,6 @@ package lxd_test import ( "errors" - stdtesting "testing" lxdclient "github.com/canonical/lxd/client" "github.com/canonical/lxd/shared" @@ -33,10 +32,6 @@ import ( coretools "github.com/juju/juju/tools" ) -func Test(t *stdtesting.T) { - gc.TestingT(t) -} - type managerSuite struct { lxdtesting.BaseSuite diff --git a/environs/testing/package_mock.go b/environs/testing/package_mock.go index 67fb077491d..39595b4fc00 100644 --- a/environs/testing/package_mock.go +++ b/environs/testing/package_mock.go @@ -1365,17 +1365,17 @@ func (m *MockDefaultConstraintsChecker) EXPECT() *MockDefaultConstraintsCheckerM } // ShouldApplyControllerConstraints mocks base method. -func (m *MockDefaultConstraintsChecker) ShouldApplyControllerConstraints() bool { +func (m *MockDefaultConstraintsChecker) ShouldApplyControllerConstraints(arg0 constraints.Value) bool { m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "ShouldApplyControllerConstraints") + ret := m.ctrl.Call(m, "ShouldApplyControllerConstraints", arg0) ret0, _ := ret[0].(bool) return ret0 } // ShouldApplyControllerConstraints indicates an expected call of ShouldApplyControllerConstraints. -func (mr *MockDefaultConstraintsCheckerMockRecorder) ShouldApplyControllerConstraints() *gomock.Call { +func (mr *MockDefaultConstraintsCheckerMockRecorder) ShouldApplyControllerConstraints(arg0 interface{}) *gomock.Call { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "ShouldApplyControllerConstraints", reflect.TypeOf((*MockDefaultConstraintsChecker)(nil).ShouldApplyControllerConstraints)) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "ShouldApplyControllerConstraints", reflect.TypeOf((*MockDefaultConstraintsChecker)(nil).ShouldApplyControllerConstraints), arg0) } // MockProviderCredentialsRegister is a mock of ProviderCredentialsRegister interface. From 4cfd27529a74404aa812db1fb15af76a9cb428e5 Mon Sep 17 00:00:00 2001 From: hmlanigan Date: Tue, 14 Nov 2023 11:26:49 -0500 Subject: [PATCH 13/15] Only repository charms are queried for updates. --- worker/charmrevision/doc.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/worker/charmrevision/doc.go b/worker/charmrevision/doc.go index 7d53d9ca5f8..2db711d7d01 100644 --- a/worker/charmrevision/doc.go +++ b/worker/charmrevision/doc.go @@ -3,8 +3,8 @@ // Package charmrevision defines the charm revision updater worker. This worker // is responsible for polling Charmhub every 24 hours to check if there are new -// revisions available of any charm deployed in the model. If so, it will put a -// document in the Juju database, so that the next time the user runs +// revisions available of any repository charm deployed in the model. If so, it +// will put a document in the Juju database, so that the next time the user runs // `juju status`, they can see that there is an update available. This worker // also sends anonymised usage metrics to Charmhub when it polls. // From b136048ccd2e77bb167e42252cdfe958444eacf3 Mon Sep 17 00:00:00 2001 From: Jack Shaw Date: Fri, 10 Nov 2023 10:24:42 +0200 Subject: [PATCH 14/15] Write caas application provisioner worker doc.go --- worker/caasapplicationprovisioner/doc.go | 12 ++++++++++++ 1 file changed, 12 insertions(+) create mode 100644 worker/caasapplicationprovisioner/doc.go diff --git a/worker/caasapplicationprovisioner/doc.go b/worker/caasapplicationprovisioner/doc.go new file mode 100644 index 00000000000..0cdc3930a97 --- /dev/null +++ b/worker/caasapplicationprovisioner/doc.go @@ -0,0 +1,12 @@ +// Copyright 2023 Canonical Ltd. +// Licensed under the AGPLv3, see LICENCE file for details. + +// Package caasapplicationprovisioner defines two types of worker: +// - provisioner: Watches a Kubernetes model and starts a new worker +// of the appWorker type whenever an application is created. +// - appWorker: Drives the Kubernetes provider to create, manage, +// and destroy Kubernetes resources to match a requested state. Also +// writes the state of created resources (application/unit status, +// application/unit IP addresses & ports, filesystem info, etc.) +// back into the database. +package caasapplicationprovisioner From 4117d19a80f3fffd69c15eee634c09af0eb7ec19 Mon Sep 17 00:00:00 2001 From: Nicolas Vinuesa Date: Tue, 10 Oct 2023 16:21:12 +0200 Subject: [PATCH 15/15] Add wait_for after removing offers, relations and saas CMR tests continue to fail on CI because of the controller not being cleaned up. This happens because we try to destroy it *before* removing the offers on the model. This patch fixes this. --- tests/suites/cmr/offer_consume.sh | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/tests/suites/cmr/offer_consume.sh b/tests/suites/cmr/offer_consume.sh index a423dc36a09..e2026d9a2ba 100644 --- a/tests/suites/cmr/offer_consume.sh +++ b/tests/suites/cmr/offer_consume.sh @@ -56,6 +56,8 @@ run_offer_consume() { echo "Remove offer" juju remove-relation dummy-sink dummy-offer + # wait for the relation to be removed. + wait_for null '.applications["dummy-sink"] | .relations' juju remove-saas dummy-offer # wait for saas to be removed. wait_for null '.["application-endpoints"]' @@ -64,6 +66,7 @@ run_offer_consume() { juju switch "model-offer" wait_for null '.offers."dummy-offer"."total-connected-count"' juju remove-offer "admin/model-offer.dummy-offer" -y + wait_for null '.offers' echo "Clean up" destroy_model "model-offer" @@ -117,6 +120,8 @@ run_offer_consume_cross_controller() { echo "Remove offer" juju remove-relation dummy-sink dummy-source + # wait for the relation to be removed. + wait_for null '.applications["dummy-sink"] | .relations' juju remove-saas dummy-source # wait for saas to be removed. wait_for null '.["application-endpoints"]' @@ -125,6 +130,7 @@ run_offer_consume_cross_controller() { juju switch "${offer_controller}:model-offer" wait_for null '.offers."dummy-offer"."total-connected-count"' juju remove-offer "${offer_controller}:admin/model-offer.dummy-source" -y + wait_for null '.offers' echo "Clean up" destroy_controller "controller-consume"