Skip to content

Commit

Permalink
Merge pull request #1043 from Thanhphan1147/add_missing_revision_para…
Browse files Browse the repository at this point in the history
…ms_in_bundle_add_charms

#1043

#### Description

Fixes #1042.

When deploying a bundle, although the plan is properly generated with the correct revision for the charms in the bundle, and the change steps are generated with the correct parameter. I believe that the `addCharm` step does not take into account the specified "revision" of the charm to deploy, as a result the revision deployed is the latest revision instead of the specified revision. ref: https://github.com/juju/python-libjuju/blob/5ed5ae461514feb84dc0f96c2e46b6fab9f35861/juju/bundle.py#L709-L713

This PR simply adds the missing parameter to the `AddCharmChange` step and propagate the value to `client.CharmOrigin` to add the correct revision.

#### QA Steps
1. Create a bundle containing one or more charms with revision
2. Deploy the bundle with `model.deploy`
3. The deployed charms in the bundle should have the revision specified in the bundle.

- [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
- [x] [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 package~~ (I think this is too small of a change to warrant a doc update)
  • Loading branch information
jujubot authored Apr 23, 2024
2 parents 2c30901 + 04f0e53 commit d4f6677
Show file tree
Hide file tree
Showing 4 changed files with 32 additions and 0 deletions.
3 changes: 3 additions & 0 deletions juju/bundle.py
Original file line number Diff line number Diff line change
Expand Up @@ -659,6 +659,7 @@ class AddCharmChange(ChangeInfo):
_toPy = {'charm': 'charm',
'series': 'series',
'channel': 'channel',
'revision': 'revision',
'architecture': 'architecture'}

"""AddCharmChange holds a change for adding a charm to the environment.
Expand All @@ -675,6 +676,7 @@ class AddCharmChange(ChangeInfo):
:series: series of the charm to be added if the charm default is
not sufficient.
:channel: preferred channel for obtaining the charm.
:revision: specified revision of the charm to be added if specified.
"""
@staticmethod
def method():
Expand Down Expand Up @@ -710,6 +712,7 @@ async def run(self, context):
architecture=arch,
risk=ch.risk,
track=ch.track,
revision=self.revision,
base=base)
identifier, origin = await context.model._resolve_charm(url, origin)

Expand Down
6 changes: 6 additions & 0 deletions tests/integration/bundle/bundle-with-charm-revision.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
applications:
hello-juju:
charm: "hello-juju"
channel: latest/stable
revision: 7
num_units: 1
20 changes: 20 additions & 0 deletions tests/integration/test_model.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
import pytest
from juju import jasyncio, tag, url
from juju.client import client
from juju.client._definitions import FullStatus
from juju.errors import JujuError, JujuModelError, JujuUnitError, JujuConnectionError
from juju.model import Model, ModelObserver
from juju.utils import block_until, run_with_interrupt, wait_for_bundle, base_channel_to_series
Expand Down Expand Up @@ -181,6 +182,25 @@ async def test_deploy_bundle_local_charm_series_manifest():
assert model.units['test1/0'].workload_status == 'active'


@base.bootstrapped
@pytest.mark.bundle
async def test_deploy_bundle_with_pinned_charm_revision():
bundle_dir = INTEGRATION_TEST_DIR / 'bundle'
bundle_yaml_path = bundle_dir / 'bundle-with-charm-revision.yaml'
# Revision of the hello-juju charm defined in the bundle yaml
# We can also read the yaml to get the revision but we are hard-coding it for now for simplicity
pinned_revision = 7

async with base.CleanModel() as model:
await model.deploy(str(bundle_yaml_path))

application = model.applications.get('hello-juju', None)
status: FullStatus = await model.get_status([application.name])
# the 'charm' field of application status should be of this format:
# ch:amd64/{series}/{name}-{revision}
assert f"{application.name}-{pinned_revision}" in status.applications[application.name]["charm"]


@base.bootstrapped
@pytest.mark.bundle
async def test_deploy_invalid_bundle():
Expand Down
3 changes: 3 additions & 0 deletions tests/unit/test_bundle.py
Original file line number Diff line number Diff line change
Expand Up @@ -312,12 +312,14 @@ def test_dict_params(self):
change = AddCharmChange(1, [], params={"charm": "charm",
"series": "series",
"channel": "channel",
"revision": "revision",
"architecture": "architecture"})
self.assertEqual({"change_id": 1,
"requires": [],
"charm": "charm",
"series": "series",
"channel": "channel",
"revision": "revision",
"architecture": "architecture"}, change.__dict__)

def test_dict_params_missing_data(self):
Expand All @@ -328,6 +330,7 @@ def test_dict_params_missing_data(self):
"charm": "charm",
"series": "series",
"channel": None,
"revision": None,
"architecture": None}, change.__dict__)


Expand Down

0 comments on commit d4f6677

Please sign in to comment.