Skip to content

Commit

Permalink
Merge pull request juju#18433 from nvinuesa/juju-6846-2
Browse files Browse the repository at this point in the history
juju#18433

At juju#18413 we introduced the Source when retrieving a charm ID, which was in turn needed to deploy. 
This small patch fixes the deploy of a local charm because the Source was not being injected on that code path.

<!-- 
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

Deploy a local charm:

```
$ juju download ubuntu
$ juju deploy ./ubuntu_r25.charm
$ juju status
Model Controller Cloud/Region Version Timestamp
m c localhost/localhost 4.0-beta5.1 15:22:38+01:00

App Version Status Scale Charm Channel Rev Exposed Message
ubuntu 24.04 active 1 ubuntu 0 no

Unit Workload Agent Machine Public address Ports Message
ubuntu/0* active idle 0 10.165.241.21

Machine State Address Inst id Base AZ Message
0 started 10.165.241.21 juju-bc520e-0 [email protected] Running
```


## Links

**Jira card:** JUJU-6842
  • Loading branch information
jujubot authored Nov 28, 2024
2 parents eede4d8 + 8f3be92 commit e2523e8
Show file tree
Hide file tree
Showing 3 changed files with 107 additions and 7 deletions.
7 changes: 7 additions & 0 deletions apiserver/facades/client/application/application_get.go
Original file line number Diff line number Diff line change
Expand Up @@ -126,14 +126,21 @@ func (api *APIBase) getCharmID(ctx context.Context, charmURL string) (corecharm.
return "", errors.Annotate(err, "parsing charm URL")
}

charmSource, err := applicationcharm.ParseCharmSchema(charm.Schema(curl.Schema))
if err != nil {
return "", errors.Trace(err)
}
charmID, err := api.applicationService.GetCharmID(ctx, applicationcharm.GetCharmArgs{
Name: curl.Name,
Revision: ptr(curl.Revision),
Source: charmSource,
})
if errors.Is(err, applicationerrors.CharmNotFound) {
return "", errors.NotFoundf("charm %q", charmURL)
} else if errors.Is(err, applicationerrors.CharmNameNotValid) {
return "", errors.NotValidf("charm %q", charmURL)
} else if errors.Is(err, applicationerrors.CharmSourceNotValid) {
return "", errors.NotValidf("charm %q", charmURL)
} else if err != nil {
return "", errors.Annotate(err, "getting charm id")
}
Expand Down
106 changes: 99 additions & 7 deletions apiserver/facades/client/application/application_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import (
gomock "go.uber.org/mock/gomock"
gc "gopkg.in/check.v1"

"github.com/juju/juju/core/application"
coreassumes "github.com/juju/juju/core/assumes"
charmtesting "github.com/juju/juju/core/charm/testing"
"github.com/juju/juju/core/network"
Expand Down Expand Up @@ -44,7 +45,7 @@ func (s *applicationSuite) TestSetCharm(c *gc.C) {
s.setupAPI(c)
s.expectApplication(c, "foo")
s.expectCharm(c, "foo")
s.expectCharmConfig(c)
s.expectCharmConfig(c, 1)
s.expectCharmAssumes(c)
s.expectCharmFormatCheck(c, "foo")

Expand Down Expand Up @@ -138,7 +139,7 @@ func (s *applicationSuite) TestSetCharmEndpointBindings(c *gc.C) {
s.expectApplication(c, "foo")
s.expectSpaceName(c, "bar")
s.expectCharm(c, "foo")
s.expectCharmConfig(c)
s.expectCharmConfig(c, 1)
s.expectCharmAssumes(c)
s.expectCharmFormatCheck(c, "foo")

Expand Down Expand Up @@ -260,7 +261,7 @@ func (s *applicationSuite) TestSetCharmInvalidConfig(c *gc.C) {
s.setupAPI(c)
s.expectApplication(c, "foo")
s.expectCharm(c, "foo")
s.expectCharmConfig(c)
s.expectCharmConfig(c, 1)

err := s.api.SetCharm(context.Background(), params.ApplicationSetCharmV2{
ApplicationName: "foo",
Expand Down Expand Up @@ -292,7 +293,7 @@ func (s *applicationSuite) TestSetCharmWithoutTrust(c *gc.C) {
s.setupAPI(c)
s.expectApplication(c, "foo")
s.expectCharm(c, "foo")
s.expectCharmConfig(c)
s.expectCharmConfig(c, 1)
s.expectCharmAssumes(c)
s.expectCharmFormatCheck(c, "foo")

Expand Down Expand Up @@ -347,7 +348,7 @@ func (s *applicationSuite) TestSetCharmFormatDowngrade(c *gc.C) {
s.setupAPI(c)
s.expectApplication(c, "foo")
s.expectCharm(c, "foo")
s.expectCharmConfig(c)
s.expectCharmConfig(c, 1)
s.expectCharmAssumes(c)
s.expectCharmFormatCheckDowngrade(c, "foo")

Expand Down Expand Up @@ -375,6 +376,72 @@ func (s *applicationSuite) TestSetCharmFormatDowngrade(c *gc.C) {
c.Assert(err, gc.ErrorMatches, "cannot downgrade from v2 charm format to v1")
}

func (s *applicationSuite) TestDeploy(c *gc.C) {
defer s.setupMocks(c).Finish()

s.setupAPI(c)
s.expectCharm(c, "foo")
s.expectCharmConfig(c, 2)
s.expectCharmMeta("foo", 6)
s.expectReadSequence("foo", 1)
s.expectAddApplication()
s.expectCreateApplication("foo")

errorResults, err := s.api.Deploy(context.Background(), params.ApplicationsDeploy{
Applications: []params.ApplicationDeploy{
{
ApplicationName: "foo",
CharmURL: "local:foo-42",
CharmOrigin: &params.CharmOrigin{
Type: "charm",
Source: "local",
Base: params.Base{
Name: "ubuntu",
Channel: "24.04",
},
Architecture: "amd64",
Revision: ptr(42),
Track: ptr("1.0"),
Risk: "stable",
},
},
},
})
c.Assert(err, jc.ErrorIsNil)
c.Assert(errorResults.Results, gc.HasLen, 1)
c.Assert(errorResults.Results[0].Error, gc.IsNil)
}

func (s *applicationSuite) TestDeployInvalidSource(c *gc.C) {
defer s.setupMocks(c).Finish()

s.setupAPI(c)

errorResults, err := s.api.Deploy(context.Background(), params.ApplicationsDeploy{
Applications: []params.ApplicationDeploy{
{
ApplicationName: "foo",
CharmURL: "bad:foo-42",
CharmOrigin: &params.CharmOrigin{
Type: "charm",
Source: "bad",
Base: params.Base{
Name: "ubuntu",
Channel: "24.04",
},
Architecture: "amd64",
Revision: ptr(42),
Track: ptr("1.0"),
Risk: "stable",
},
},
},
})
c.Assert(err, jc.ErrorIsNil)
c.Assert(errorResults.Results, gc.HasLen, 1)
c.Assert(errorResults.Results[0].Error, gc.ErrorMatches, "\"bad\" not a valid charm origin source")
}

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

Expand All @@ -400,6 +467,23 @@ func (s *applicationSuite) expectApplicationNotFound(c *gc.C, name string) {
s.backend.EXPECT().Application(name).Return(nil, errors.NotFoundf("application %q", name))
}

func (s *applicationSuite) expectReadSequence(name string, seqResult int) {
s.backend.EXPECT().ReadSequence(name).Return(seqResult, nil)
}

func (s *applicationSuite) expectAddApplication() {
s.backend.EXPECT().AddApplication(gomock.Any(), s.objectStore).Return(s.application, nil)
}

func (s *applicationSuite) expectCreateApplication(name string) {
s.applicationService.EXPECT().CreateApplication(gomock.Any(),
name,
gomock.Any(),
gomock.Any(),
gomock.Any(),
).Return(application.ID("app-"+name), nil)
}

func (s *applicationSuite) expectSpaceName(c *gc.C, name string) {
s.networkService.EXPECT().SpaceByName(gomock.Any(), name).Return(&network.SpaceInfo{
ID: "space-1",
Expand All @@ -416,6 +500,7 @@ func (s *applicationSuite) expectCharm(c *gc.C, name string) {
s.applicationService.EXPECT().GetCharmID(gomock.Any(), applicationcharm.GetCharmArgs{
Name: name,
Revision: ptr(42),
Source: applicationcharm.LocalSource,
}).Return(id, nil)

s.applicationService.EXPECT().GetCharm(gomock.Any(), id).Return(s.charm, applicationcharm.CharmOrigin{
Expand All @@ -427,10 +512,11 @@ func (s *applicationSuite) expectCharmNotFound(c *gc.C, name string) {
s.applicationService.EXPECT().GetCharmID(gomock.Any(), applicationcharm.GetCharmArgs{
Name: name,
Revision: ptr(42),
Source: applicationcharm.LocalSource,
}).Return("", applicationerrors.CharmNotFound)
}

func (s *applicationSuite) expectCharmConfig(c *gc.C) {
func (s *applicationSuite) expectCharmConfig(c *gc.C, times int) {
cfg, err := internalcharm.ReadConfig(strings.NewReader(`
options:
stringOption:
Expand All @@ -440,7 +526,13 @@ options:
`))
c.Assert(err, jc.ErrorIsNil)

s.charm.EXPECT().Config().Return(cfg)
s.charm.EXPECT().Config().Return(cfg).Times(times)
}

func (s *applicationSuite) expectCharmMeta(name string, times int) {
s.charm.EXPECT().Meta().Return(&internalcharm.Meta{
Name: name,
}).Times(times)
}

func (s *applicationSuite) expectCharmAssumes(c *gc.C) {
Expand Down
1 change: 1 addition & 0 deletions apiserver/facades/client/application/package_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -137,6 +137,7 @@ func (s *baseSuite) newAPI(c *gc.C, modelType model.ModelType) {
UUID: s.modelUUID,
Type: modelType,
}
s.deployApplication = DeployApplication

var err error
s.api, err = NewAPIBase(
Expand Down

0 comments on commit e2523e8

Please sign in to comment.