Skip to content

Commit

Permalink
Merge pull request juju#18474 from nvinuesa/juju-7256
Browse files Browse the repository at this point in the history
juju#18474

In order to start cleaning the legacy state out of Charm() calls, we need to first remove the one on the bootstrap worker so that Juju is able to deploy the controller charm without retrieving the charm from the legacy state.

This is possible because both for local and charmhub charms we have everything we need (namely URL and Meta), and therefore the only big change in this patch is to add the CharmURL to the attributes on AddApplication() and to move the URL() method out from CharmRef to CharmRefFull, this way we can pass everything during bootstrap.

<!-- 
The PR title should match: <type>(optional <scope>): <description>.

Please also ensure all commits in this PR comply with our conventional commits specification:
https://github.com/juju/juju/blob/main/doc/conventional-commits.md
-->

<!-- Why this change is needed and what it does. -->

## QA steps

Since the `AddApplication()` method has changed, with a new attribute, we must test bootstrapping and deploying (both local / ch charms):
```
$ juju bootstrap lxd c
$ juju download ubuntu
$ juju deploy ./ubuntu_r25.charm local
$ juju deploy ubuntu ch
```
Make sure there are no errors on the logs.

Also bootstrap with a local controller charm:
```
$ juju download juju-controller
Base "[email protected]" is not supported for charm "juju-controller", trying base "[email protected]"
Fetching charm "juju-controller" revision 91 using "stable" channel and base "amd64/ubuntu/22.04"
Install the "juju-controller" charm with:
 juju deploy ./juju-controller_r91.charm
$ juju bootstrap localhost c --build-agent --controller-charm-path ./juju-controller_r91.charm
```

Check if the controller is OK, simply with `juju status -m controller`.
## Links


**Jira card:** JUJU-7256
  • Loading branch information
jujubot authored Dec 3, 2024
2 parents e07983e + a88275f commit 316c2cc
Show file tree
Hide file tree
Showing 20 changed files with 181 additions and 230 deletions.
2 changes: 2 additions & 0 deletions apiserver/facades/client/action/run_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,7 @@ func (s *runSuite) TestRunMachineAndApplication(c *gc.C) {

state.AddApplicationArgs{
Name: "magic", Charm: charm,
CharmURL: charm.URL(),
CharmOrigin: &state.CharmOrigin{Platform: &state.Platform{OS: "ubuntu", Channel: "20.04/stable"}},
},
jujutesting.NewObjectStore(c, s.ControllerModelUUID()),
Expand Down Expand Up @@ -159,6 +160,7 @@ func (s *runSuite) TestRunApplicationWorkload(c *gc.C) {

state.AddApplicationArgs{
Name: "magic", Charm: charm,
CharmURL: charm.URL(),
CharmOrigin: &state.CharmOrigin{Platform: &state.Platform{OS: "ubuntu", Channel: "20.04/stable"}},
},
jujutesting.NewObjectStore(c, s.ControllerModelUUID()),
Expand Down
2 changes: 1 addition & 1 deletion apiserver/facades/client/application/application_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -382,7 +382,7 @@ func (s *applicationSuite) TestDeploy(c *gc.C) {
s.setupAPI(c)
s.expectCharm(c, "foo")
s.expectCharmConfig(c, 2)
s.expectCharmMeta("foo", 6)
s.expectCharmMeta("foo", 7)
s.expectReadSequence("foo", 1)
s.expectAddApplication()
s.expectCreateApplication("foo")
Expand Down
1 change: 1 addition & 0 deletions apiserver/facades/client/application/deploy.go
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,7 @@ func DeployApplication(
asa := state.AddApplicationArgs{
Name: args.ApplicationName,
Charm: args.Charm,
CharmURL: args.Charm.URL(),
CharmOrigin: origin,
Storage: stateStorageDirectives(args.Storage),
Devices: stateDeviceConstraints(args.Devices),
Expand Down
1 change: 1 addition & 0 deletions apiserver/facades/client/application/deployrepository.go
Original file line number Diff line number Diff line change
Expand Up @@ -146,6 +146,7 @@ func (api *DeployFromRepositoryAPI) DeployFromRepository(ctx context.Context, ar
ApplicationConfig: dt.applicationConfig,
AttachStorage: dt.attachStorage,
Charm: ch,
CharmURL: ch.URL(),
CharmConfig: dt.charmSettings,
CharmOrigin: stOrigin,
Constraints: dt.constraints,
Expand Down
5 changes: 3 additions & 2 deletions apiserver/facades/client/client/status_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -950,8 +950,9 @@ func (s *statusUpgradeUnitSuite) AddApplication(c *gc.C, charmName, applicationN

st := s.ControllerModel(c).State()
_, err := st.AddApplication(state.AddApplicationArgs{
Name: applicationName,
Charm: ch,
Name: applicationName,
Charm: ch,
CharmURL: ch.URL(),
CharmOrigin: &state.CharmOrigin{
ID: "mycharmhubid",
Hash: "mycharmhash",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -141,7 +141,7 @@ func (a *CharmDownloaderAPI) downloadApplicationCharm(ctx context.Context, appTa
}

a.logger.Infof("downloading charm %q", pendingCharmURL)
downloadedOrigin, err := downloader.DownloadAndStore(ctx, pendingCharmURL, *resolvedOrigin, force)
downloadedOrigin, _, err := downloader.DownloadAndStore(ctx, pendingCharmURL, *resolvedOrigin, force)
if err != nil {
return errors.Annotatef(err, "cannot download and store charm %q", pendingCharmURL)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ func (s *charmDownloaderSuite) TestDownloadApplicationCharmsDeploy(c *gc.C) {

s.authChecker.EXPECT().AuthController().Return(true)
s.stateBackend.EXPECT().Application("ufo").Return(app, nil)
s.downloader.EXPECT().DownloadAndStore(gomock.Any(), charmURL, resolvedOrigin, false).Return(downloadedOrigin, nil)
s.downloader.EXPECT().DownloadAndStore(gomock.Any(), charmURL, resolvedOrigin, false).Return(downloadedOrigin, nil, nil)

got, err := s.api.DownloadApplicationCharms(context.Background(), params.Entities{
Entities: []params.Entity{
Expand Down Expand Up @@ -150,7 +150,7 @@ func (s *charmDownloaderSuite) TestDownloadApplicationCharmsDeployMultiAppOneCha
s.authChecker.EXPECT().AuthController().Return(true)
s.stateBackend.EXPECT().Application("ufo").Return(appOne, nil)
s.stateBackend.EXPECT().Application("another-ufo").Return(appTwo, nil)
s.downloader.EXPECT().DownloadAndStore(gomock.Any(), charmURL, resolvedOrigin, false).Return(downloadedOrigin, nil).AnyTimes()
s.downloader.EXPECT().DownloadAndStore(gomock.Any(), charmURL, resolvedOrigin, false).Return(downloadedOrigin, nil, nil).AnyTimes()

got, err := s.api.DownloadApplicationCharms(context.Background(), params.Entities{
Entities: []params.Entity{
Expand Down Expand Up @@ -193,7 +193,7 @@ func (s *charmDownloaderSuite) TestDownloadApplicationCharmsRefresh(c *gc.C) {

s.authChecker.EXPECT().AuthController().Return(true)
s.stateBackend.EXPECT().Application("ufo").Return(app, nil)
s.downloader.EXPECT().DownloadAndStore(gomock.Any(), charmURL, resolvedOrigin, false).Return(downloadedOrigin, nil)
s.downloader.EXPECT().DownloadAndStore(gomock.Any(), charmURL, resolvedOrigin, false).Return(downloadedOrigin, nil, nil)

got, err := s.api.DownloadApplicationCharms(context.Background(), params.Entities{
Entities: []params.Entity{
Expand Down Expand Up @@ -228,7 +228,7 @@ func (s *charmDownloaderSuite) TestDownloadApplicationCharmsSetStatusIfDownloadF

s.authChecker.EXPECT().AuthController().Return(true)
s.stateBackend.EXPECT().Application("ufo").Return(app, nil)
s.downloader.EXPECT().DownloadAndStore(gomock.Any(), charmURL, resolvedOrigin, false).Return(corecharm.Origin{}, errors.NotFoundf("charm"))
s.downloader.EXPECT().DownloadAndStore(gomock.Any(), charmURL, resolvedOrigin, false).Return(corecharm.Origin{}, nil, errors.NotFoundf("charm"))

got, err := s.api.DownloadApplicationCharms(context.Background(), params.Entities{
Entities: []params.Entity{
Expand Down
2 changes: 1 addition & 1 deletion apiserver/facades/controller/charmdownloader/interfaces.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ type Charm interface {

// Downloader defines an API for downloading and storing charms.
type Downloader interface {
DownloadAndStore(ctx context.Context, charmURL *charm.URL, requestedOrigin corecharm.Origin, force bool) (corecharm.Origin, error)
DownloadAndStore(ctx context.Context, charmURL *charm.URL, requestedOrigin corecharm.Origin, force bool) (corecharm.Origin, charm.Charm, error)
}

// AuthChecker provides an API for checking if the API client is a controller.
Expand Down
15 changes: 8 additions & 7 deletions apiserver/facades/controller/charmdownloader/mocks/mocks.go

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

Loading

0 comments on commit 316c2cc

Please sign in to comment.