Skip to content

Commit

Permalink
Merge pull request juju#16434 from jack-w-shaw/JUJU-4779_fix_charm_or…
Browse files Browse the repository at this point in the history 
…igin_switching

juju#16434

Local charm origins can not include hashes, ids or channels. However, when switching from a charmhub charm to a local one these attributes were accidentally copies over

Ensure these attributes are droped when constructing the new charm origin

Similarly with the revision, a new revision would not be set, meaning the revision of the original charm would be kept. Make sure the revision is updated as well

There is a slight nuance here, simply deploying a local charm does not fill in a revision, but switching to a local charm will fill in a revision. This is not a problem, as ideally we would have a revision for local charms. This is, in fact, something added in a later version anyway

This will require an equivalent change to python-libjuju

## Checklist

- [x] Code style: imports ordered, good names, simple structure, etc
- ~[ ] 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

Run the refresh integration test

```
$ juju deploy ubuntu ubuntu-switch
$ juju deploy ~/charms/ubuntu
$ juju refresh ubuntu --switch ~/charms/ubuntu
$ juju mongo
db.applications.find().pretty()
{
 "_id" : "cf0a16b4-b3f6-4d3f-8b78-77a4c4556841:ubuntu-switch",
 "name" : "ubuntu-switch",
 "model-uuid" : "cf0a16b4-b3f6-4d3f-8b78-77a4c4556841",
 "series" : "focal",
 "subordinate" : false,
 "charmurl" : "local:focal/ubuntu-12",
 "cs-channel" : "",
 "charm-origin" : {
 "source" : "local",
 "id" : "",
 "hash" : "",
 "revision" : 12,
 "platform" : {
 "architecture" : "amd64",
 "os" : "ubuntu",
 "series" : "focal"
 }
 },
 "charmmodifiedversion" : 1,
 "forcecharm" : false,
 "life" : 0,
 "unitcount" : 1,
 "relationcount" : 0,
 "minunits" : 0,
 "txn-revno" : NumberLong(5),
 "metric-credentials" : BinData(0,""),
 "exposed" : false,
 "scale" : 0,
 "passwordhash" : "",
 "provisioning-state" : null,
 "txn-queue" : [
 "6529679a9dc55c1aef137bfe_56f7652c"
 ]
}
{
 "_id" : "cf0a16b4-b3f6-4d3f-8b78-77a4c4556841:ubuntu",
 "name" : "ubuntu",
 "model-uuid" : "cf0a16b4-b3f6-4d3f-8b78-77a4c4556841",
 "series" : "bionic",
 "subordinate" : false,
 "charmurl" : "local:bionic/ubuntu-12",
 "cs-channel" : "",
 "charm-origin" : {
 "source" : "local",
 "id" : "",
 "hash" : "",
 "platform" : {
 "architecture" : "amd64",
 "os" : "ubuntu",
 "series" : "bionic"
 }
 },
 "charmmodifiedversion" : 0,
 "forcecharm" : false,
 "life" : 0,
 "unitcount" : 1,
 "relationcount" : 0,
 "minunits" : 0,
 "txn-revno" : NumberLong(6),
 "metric-credentials" : BinData(0,""),
 "exposed" : false,
 "scale" : 0,
 "passwordhash" : "",
 "provisioning-state" : null,
 "txn-queue" : [
 "652967dd9dc55c1aef137c51_5ab0fbc7"
 ]
}
```

Notice the id, hash, channel are absent in both charms
  • Loading branch information
@jujubot
jujubot authored Oct 24, 2023
2 parents 1a29d2a + 214a1b2 commit 11537fc
Show file tree
Hide file tree
Showing 2 changed files with 24 additions and 10 deletions.
30 changes: 20 additions & 10 deletions cmd/juju/application/refresh_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -547,14 +547,14 @@ func (s *RefreshSuccessStateSuite) SetUpSuite(c *gc.C) {
s.RepoSuite.SetUpSuite(c)
}

func (s *RefreshSuccessStateSuite) assertUpgraded(c *gc.C, riak *state.Application, revision int, forced bool) *charm.URL {
func (s *RefreshSuccessStateSuite) assertUpgraded(c *gc.C, riak *state.Application, revision int, forced bool) (*charm.URL, *state.CharmOrigin) {
err := riak.Refresh()
c.Assert(err, jc.ErrorIsNil)
ch, force, err := riak.Charm()
c.Assert(err, jc.ErrorIsNil)
c.Assert(ch.Revision(), gc.Equals, revision)
c.Assert(force, gc.Equals, forced)
return ch.URL()
return ch.URL(), riak.CharmOrigin()
}

func (s *RefreshSuccessStateSuite) runRefresh(c *gc.C, cmd cmd.Command, args ...string) (*cmd.Context, error) {
Expand Down Expand Up @@ -614,7 +614,7 @@ func (s *RefreshSuccessStateSuite) assertLocalRevision(c *gc.C, revision int, pa
func (s *RefreshSuccessStateSuite) TestLocalRevisionUnchanged(c *gc.C) {
_, err := s.runRefresh(c, s.cmd, "riak", "--path", s.path)
c.Assert(err, jc.ErrorIsNil)
curl := s.assertUpgraded(c, s.riak, 8, false)
curl, _ := s.assertUpgraded(c, s.riak, 8, false)
s.AssertCharmUploaded(c, curl)
// Even though the remote revision is bumped, the local one should
// be unchanged.
Expand Down Expand Up @@ -767,7 +767,7 @@ func (s *RefreshSuccessStateSuite) TestRespectsLocalRevisionWhenPossible(c *gc.C
}
_, err = s.runRefresh(c, s.cmd, "riak", "--path", s.path)
c.Assert(err, jc.ErrorIsNil)
curl := s.assertUpgraded(c, s.riak, 42, false)
curl, _ := s.assertUpgraded(c, s.riak, 42, false)
s.AssertCharmUploaded(c, curl)
s.assertLocalRevision(c, 42, s.path)
}
Expand Down Expand Up @@ -1089,7 +1089,7 @@ func (s *RefreshCharmHubSuite) TestUpgradeResourceNoChange(c *gc.C) {
func (s *RefreshSuccessStateSuite) TestForcedUnitsUpgrade(c *gc.C) {
_, err := s.runRefresh(c, s.cmd, "riak", "--force-units", "--path", s.path)
c.Assert(err, jc.ErrorIsNil)
curl := s.assertUpgraded(c, s.riak, 8, true)
curl, _ := s.assertUpgraded(c, s.riak, 8, true)
s.AssertCharmUploaded(c, curl)
// Local revision is not changed.
s.assertLocalRevision(c, 7, s.path)
Expand All @@ -1100,7 +1100,7 @@ func (s *RefreshSuccessStateSuite) TestBlockForcedUnitsUpgrade(c *gc.C) {
s.BlockAllChanges(c, "TestBlockForcedUpgrade")
_, err := s.runRefresh(c, s.cmd, "riak", "--force-units", "--path", s.path)
c.Assert(err, jc.ErrorIsNil)
curl := s.assertUpgraded(c, s.riak, 8, true)
curl, _ := s.assertUpgraded(c, s.riak, 8, true)
s.AssertCharmUploaded(c, curl)
// Local revision is not changed.
s.assertLocalRevision(c, 7, s.path)
Expand All @@ -1114,7 +1114,7 @@ func (s *RefreshSuccessStateSuite) TestCharmPath(c *gc.C) {
c.Assert(err, jc.ErrorIsNil)
_, err = s.runRefresh(c, s.cmd, "riak", "--path", myriakPath)
c.Assert(err, jc.ErrorIsNil)
curl := s.assertUpgraded(c, s.riak, 42, false)
curl, _ := s.assertUpgraded(c, s.riak, 42, false)
c.Assert(curl.String(), gc.Equals, "local:bionic/riak-42")
s.assertLocalRevision(c, 42, myriakPath)
}
Expand All @@ -1135,9 +1135,19 @@ func (s *RefreshSuccessStateSuite) TestSwitchToLocal(c *gc.C) {
c.Assert(err, jc.ErrorIsNil)
_, err = s.runRefresh(c, s.cmd, "riak", "--switch", myriakPath)
c.Assert(err, jc.ErrorIsNil)
curl := s.assertUpgraded(c, s.riak, 42, false)
rev := 42
curl, origin := s.assertUpgraded(c, s.riak, rev, false)
c.Assert(curl.String(), gc.Equals, "local:bionic/riak-42")
s.assertLocalRevision(c, 42, myriakPath)
c.Assert(origin, gc.DeepEquals, &state.CharmOrigin{
Source: "local",
Revision: &rev,
Platform: &state.Platform{
Architecture: "amd64",
OS: "ubuntu",
Series: "bionic",
},
})
s.assertLocalRevision(c, rev, myriakPath)
}

func (s *RefreshSuccessStateSuite) TestSwitchToLocalNotFound(c *gc.C) {
Expand All @@ -1154,7 +1164,7 @@ func (s *RefreshSuccessStateSuite) TestCharmPathNoRevUpgrade(c *gc.C) {
s.assertLocalRevision(c, 7, myriakPath)
_, err := s.runRefresh(c, s.cmd, "riak", "--path", myriakPath)
c.Assert(err, jc.ErrorIsNil)
curl := s.assertUpgraded(c, s.riak, 8, false)
curl, _ := s.assertUpgraded(c, s.riak, 8, false)
c.Assert(curl.String(), gc.Equals, "local:bionic/riak-8")
}

Expand Down
4 changes: 4 additions & 0 deletions cmd/juju/application/refresher/refresher.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -214,6 +214,10 @@ func (d *localCharmRefresher) Refresh() (*CharmID, error) {

newOrigin := d.charmOrigin
newOrigin.Source = corecharm.Local
newOrigin.Channel = nil
newOrigin.Hash = ""
newOrigin.ID = ""
newOrigin.Revision = &addedURL.Revision
return &CharmID{
URL: addedURL,
Origin: newOrigin,
Expand Down

0 comments on commit 11537fc

Please sign in to comment.