Skip to content

Commit

Permalink
Merge pull request juju#16990 from jack-w-shaw/JUJU-5348_remove_url_s…
Browse files Browse the repository at this point in the history
…eries_from_resolve_tests

juju#16990

Facade v7 has been suppported since at least 3.3

Since this is an unreleased major version, we can drop support for the legacy versions

This is important since facade v6 returns charm supported series, instead of bases. Dropping this facade version is a step towards eliminating series from our codebase

Also, remove charm url series from some tests in this package

## Checklist

- [x] Code style: imports ordered, good names, simple structure, etc
- [x] Comments saying why design decisions were made
- [x] Go unit tests, with comments saying what you're testing
- [ ] [Integration tests](https://github.com/juju/juju/tree/main/tests), with comments saying what you're testing
- [ ] [doc.go](https://discourse.charmhub.io/t/readme-in-packages/451) added or updated in changed packages

## QA steps

### 3.3 client with controller from this PR 

```
$ juju-main bootstrap lxd lxd
$ juju-main add-model m
$ juju-3.3 deploy mysql
Deployed "mysql" from charm-hub charm "mysql", revision 196 in channel 8.0/stable on [email protected]/stable
$ juju-3.3 deploy mysql --base [email protected]
ERROR selecting releases: charm or bundle not found for channel "", base "amd64/ubuntu/20.04"
available releases are:
 channel "8.0/beta": available bases are: [email protected]
 channel "8.0/edge": available bases are: [email protected]
 channel "8.0/stable": available bases are: [email protected]
 channel "8.0/candidate": available bases are: [email protected]

$ juju-3.3 status
Model Controller Cloud/Region Version Timestamp
m lxd localhost/localhost 4.0-beta3.1 13:20:55Z

App Version Status Scale Charm Channel Rev Exposed Message
mysql 8.0.34-0ubun... active 1 mysql 8.0/stable 196 no 

Unit Workload Agent Machine Public address Ports Message
mysql/0* active idle 0 10.219.211.104 3306,33060/tcp Primary

Machine State Address Inst id Base AZ Message
0 started 10.219.211.104 juju-8d59f0-0 [email protected] Running
```

### client from this PR with 3.3 controller

```
$ juju-3.3 bootstrap lxd lxd-3.3
$ juju-main add-model m
$ juju deploy mysql
Deployed "mysql" from charm-hub charm "mysql", revision 196 in channel 8.0/stable on [email protected]/stable
$ juju deploy mysql --base [email protected]
ERROR selecting releases: charm or bundle not found for channel "", base "amd64/ubuntu/20.04"
available releases are:
 channel "8.0/stable": available bases are: [email protected]
 channel "8.0/candidate": available bases are: [email protected]
 channel "8.0/beta": available bases are: [email protected]
 channel "8.0/edge": available bases are: [email protected]

$ juju-main status
Model Controller Cloud/Region Version SLA Timestamp
m lxd-3.3 localhost/localhost 3.3.4.1 unsupported 13:28:03Z

App Version Status Scale Charm Channel Rev Exposed Message
mysql 8.0.34-0ubun... active 1 mysql 8.0/stable 196 no 

Unit Workload Agent Machine Public address Ports Message
mysql/1* active idle 1 10.219.211.146 3306,33060/tcp Primary

Machine State Address Inst id Base AZ Message
1 started 10.219.211.146 juju-fc2acc-1 [email protected] Running
```

### Migrate 3.5 -> this PR

```
$ juju-3.5 bootstrap lxd lxd-3.5
$ juju-3.5 add-model m
$ juju-3.5 deploy ubuntu
(wait)
$ juju status
$ juju status
Model Controller Cloud/Region Version SLA Timestamp
m lxd-3.5 localhost/localhost 3.5-beta1.1 unsupported 13:20:41Z

App Version Status Scale Charm Channel Rev Exposed Message
ubuntu 22.04 active 1 ubuntu stable 24 no 

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

Machine State Address Inst id Base AZ Message
0 started 10.219.211.147 juju-6aa3df-0 [email protected] Running

$ juju-3.5 migrate m lxd
Migration started with ID "96ec3473-f1fd-4ce7-8204-e581e56aa3df:0"
(wait)
$ juju-3.5 status
ERROR Model "admin/m" has been migrated to controller "lxd".
To access it run 'juju switch lxd:admin/m'.

$ juju-3.5 switch lxd:m
lxd-3.5:admin/m -> lxd:admin/m

$ juju-3.5 status
Model Controller Cloud/Region Version Timestamp
m lxd localhost/localhost 3.5-beta1.1 13:21:45Z

App Version Status Scale Charm Channel Rev Exposed Message
ubuntu 22.04 active 1 ubuntu latest/stable 24 no 

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

Machine State Address Inst id Base AZ Message
0 started 10.219.211.147 juju-6aa3df-0 [email protected] Running
```
  • Loading branch information
jujubot authored Feb 29, 2024
2 parents df3b600 + d697450 commit b10fa7c
Show file tree
Hide file tree
Showing 4 changed files with 6 additions and 143 deletions.
56 changes: 0 additions & 56 deletions apiserver/facades/client/charms/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@ import (
"github.com/juju/juju/apiserver/facade"
charmsinterfaces "github.com/juju/juju/apiserver/facades/client/charms/interfaces"
"github.com/juju/juju/core/arch"
"github.com/juju/juju/core/base"
corecharm "github.com/juju/juju/core/charm"
"github.com/juju/juju/core/constraints"
"github.com/juju/juju/core/permission"
Expand All @@ -35,23 +34,10 @@ import (
)

// APIv7 provides the Charms API facade for version 7.
// v7 guarantees SupportedBases will be provided in ResolveCharms
type APIv7 struct {
*API
}

// APIv6 provides the Charms API facade for version 6.
// It removes the AddCharmWithAuthorization function, as
// we no longer support macaroons.
type APIv6 struct {
*APIv7
}

// APIv5 provides the Charms API facade for version 5.
type APIv5 struct {
*APIv6
}

// API implements the charms interface and is the concrete
// implementation of the API end point.
type API struct {
Expand Down Expand Up @@ -249,17 +235,6 @@ func (a *API) AddCharm(ctx context.Context, args params.AddCharmWithOrigin) (par
})
}

// AddCharmWithAuthorization adds the given charm URL (which must include
// revision) to the environment, if it does not exist yet. Local charms are
// not supported, only charm hub URLs. See also AddLocalCharm().
//
// Since the charm macaroons are no longer supported, this is the same as
// AddCharm. We keep it for backwards compatibility in APIv5.
func (a *APIv5) AddCharmWithAuthorization(ctx context.Context, args params.AddCharmWithAuth) (params.CharmOriginResult, error) {
a.logger.Tracef("AddCharmWithAuthorization %+v", args)
return a.addCharmWithAuthorization(ctx, args)
}

func (a *API) addCharmWithAuthorization(ctx context.Context, args params.AddCharmWithAuth) (params.CharmOriginResult, error) {
if commoncharm.OriginSource(args.Origin.Source) != commoncharm.OriginCharmHub {
return params.CharmOriginResult{}, errors.Errorf("unknown schema for charm URL %q", args.URL)
Expand Down Expand Up @@ -419,37 +394,6 @@ func (a *API) resolveOneCharm(ctx context.Context, arg params.ResolveCharmWithCh
return result
}

// ResolveCharms resolves the given charm URLs with an optionally specified
// preferred channel. Channel provided via CharmOrigin.
// We need to include SupportedSeries in facade version 6
func (a *APIv6) ResolveCharms(ctx context.Context, args params.ResolveCharmsWithChannel) (params.ResolveCharmWithChannelResultsV6, error) {
res, err := a.API.ResolveCharms(ctx, args)
if err != nil {
return params.ResolveCharmWithChannelResultsV6{}, errors.Trace(err)
}
results, err := transform.SliceOrErr(res.Results, func(result params.ResolveCharmWithChannelResult) (params.ResolveCharmWithChannelResultV6, error) {
supportedSeries, err := transform.SliceOrErr(result.SupportedBases, func(pBase params.Base) (string, error) {
b, err := base.ParseBase(pBase.Name, pBase.Channel)
if err != nil {
return "", err
}
return base.GetSeriesFromBase(b)
})
if err != nil {
return params.ResolveCharmWithChannelResultV6{}, err
}
return params.ResolveCharmWithChannelResultV6{
URL: result.URL,
Origin: result.Origin,
Error: result.Error,
SupportedSeries: supportedSeries,
}, nil
})
return params.ResolveCharmWithChannelResultsV6{
Results: results,
}, err
}

func convertCharmBase(in corecharm.Platform) params.Base {
return params.Base{
Name: in.OS,
Expand Down
69 changes: 5 additions & 64 deletions apiserver/facades/client/charms/client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@ func (s *charmsMockSuite) TestResolveCharms(c *gc.C) {
api := s.api(c)

curl := "ch:testme"
seriesCurl := "ch:amd64/focal/testme"
fullCurl := "ch:amd64/testme"

edgeOrigin := params.CharmOrigin{
Source: corecharm.CharmHub.String(),
Expand All @@ -128,21 +128,21 @@ func (s *charmsMockSuite) TestResolveCharms(c *gc.C) {
Architecture: "amd64",
}},
{Reference: curl, Origin: stableOrigin},
{Reference: seriesCurl, Origin: edgeOrigin},
{Reference: fullCurl, Origin: edgeOrigin},
},
}

expected := []params.ResolveCharmWithChannelResult{
{
URL: seriesCurl,
URL: fullCurl,
Origin: stableOrigin,
SupportedBases: []params.Base{
{Name: "ubuntu", Channel: "18.04"},
{Name: "ubuntu", Channel: "20.04"},
{Name: "ubuntu", Channel: "16.04"},
},
}, {
URL: seriesCurl,
URL: fullCurl,
Origin: stableOrigin,
SupportedBases: []params.Base{
{Name: "ubuntu", Channel: "18.04"},
Expand All @@ -151,7 +151,7 @@ func (s *charmsMockSuite) TestResolveCharms(c *gc.C) {
},
},
{
URL: seriesCurl,
URL: fullCurl,
Origin: edgeOrigin,
SupportedBases: []params.Base{
{Name: "ubuntu", Channel: "18.04"},
Expand Down Expand Up @@ -182,64 +182,6 @@ func (s *charmsMockSuite) TestResolveCharmsUnknownSchema(c *gc.C) {
c.Assert(result.Results[0].Error, gc.ErrorMatches, `unknown schema for charm URL "local:testme"`)
}

func (s *charmsMockSuite) TestResolveCharmV6(c *gc.C) {
defer s.setupMocks(c).Finish()
s.expectResolveWithPreferredChannel(3, nil)
apiv6 := charms.APIv6{
&charms.APIv7{
API: s.api(c),
},
}

curl := "ch:testme"
seriesCurl := "ch:amd64/focal/testme"

edgeOrigin := params.CharmOrigin{
Source: corecharm.CharmHub.String(),
Type: "charm",
Risk: "edge",
Architecture: "amd64",
}
stableOrigin := params.CharmOrigin{
Source: corecharm.CharmHub.String(),
Type: "charm",
Risk: "stable",
Architecture: "amd64",
}

args := params.ResolveCharmsWithChannel{
Resolve: []params.ResolveCharmWithChannel{
{Reference: curl, Origin: params.CharmOrigin{
Source: corecharm.CharmHub.String(),
Architecture: "amd64",
}},
{Reference: curl, Origin: stableOrigin},
{Reference: seriesCurl, Origin: edgeOrigin},
},
}

expected := []params.ResolveCharmWithChannelResultV6{
{
URL: seriesCurl,
Origin: stableOrigin,
SupportedSeries: []string{"bionic", "focal", "xenial"},
}, {
URL: seriesCurl,
Origin: stableOrigin,
SupportedSeries: []string{"bionic", "focal", "xenial"},
},
{
URL: seriesCurl,
Origin: edgeOrigin,
SupportedSeries: []string{"bionic", "focal", "xenial"},
},
}
result, err := apiv6.ResolveCharms(context.Background(), args)
c.Assert(err, jc.ErrorIsNil)
c.Assert(result.Results, gc.HasLen, 3)
c.Assert(result.Results, jc.DeepEquals, expected)
}

func (s *charmsMockSuite) TestAddCharmWithLocalSource(c *gc.C) {
defer s.setupMocks(c).Finish()
api := s.api(c)
Expand Down Expand Up @@ -607,7 +549,6 @@ func (s *charmsMockSuite) expectResolveWithPreferredChannel(times int, err error
curl := &charm.URL{
Schema: "ch",
Name: name,
Series: "focal",
Architecture: "amd64",
Revision: -1,
}
Expand Down
22 changes: 0 additions & 22 deletions apiserver/facades/client/charms/register.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,12 +18,6 @@ import (

// Register is called to expose a package of facades onto a given registry.
func Register(registry facade.FacadeRegistry) {
registry.MustRegister("Charms", 5, func(stdCtx context.Context, ctx facade.ModelContext) (facade.Facade, error) {
return newFacadeV5(ctx)
}, reflect.TypeOf((*APIv5)(nil)))
registry.MustRegister("Charms", 6, func(stdCtx context.Context, ctx facade.ModelContext) (facade.Facade, error) {
return newFacadeV6(ctx)
}, reflect.TypeOf((*APIv6)(nil)))
registry.MustRegister("Charms", 7, func(stdCtx context.Context, ctx facade.ModelContext) (facade.Facade, error) {
return newFacadeV7(ctx)
}, reflect.TypeOf((*APIv7)(nil)))
Expand All @@ -37,22 +31,6 @@ func newFacadeV7(ctx facade.ModelContext) (*APIv7, error) {
return &APIv7{api}, nil
}

func newFacadeV6(ctx facade.ModelContext) (*APIv6, error) {
api, err := newFacadeV7(ctx)
if err != nil {
return nil, errors.Trace(err)
}
return &APIv6{api}, nil
}

func newFacadeV5(ctx facade.ModelContext) (*APIv5, error) {
api, err := newFacadeV6(ctx)
if err != nil {
return nil, errors.Trace(err)
}
return &APIv5{api}, nil
}

// newFacadeBase provides the signature required for facade registration.
func newFacadeBase(ctx facade.ModelContext) (*API, error) {
authorizer := ctx.Auth()
Expand Down
2 changes: 1 addition & 1 deletion apiserver/facades/schema.json
Original file line number Diff line number Diff line change
Expand Up @@ -10557,7 +10557,7 @@
},
{
"Name": "Charms",
"Description": "APIv7 provides the Charms API facade for version 7.\nv7 guarantees SupportedBases will be provided in ResolveCharms",
"Description": "APIv7 provides the Charms API facade for version 7.",
"Version": 7,
"AvailableTo": [
"model-user"
Expand Down

0 comments on commit b10fa7c

Please sign in to comment.