diff --git a/juju/bundle.py b/juju/bundle.py index 9c1c8cca..99439744 100644 --- a/juju/bundle.py +++ b/juju/bundle.py @@ -9,6 +9,7 @@ import base64 from contextlib import closing from pathlib import Path +from typing import Dict, Optional import yaml @@ -16,6 +17,7 @@ from .client import client from .constraints import parse as parse_constraints +from .constraints import parse_storage_constraint from .errors import JujuError from . import utils, jasyncio from .origin import Channel, Source @@ -554,7 +556,10 @@ class AddApplicationChange(ChangeInfo): this will be used to scale the application. :options: holds application options. :constraints: optional application constraints. - :storage: optional storage constraints. + :storage: optional storage constraints, in the form of `{label: constraint}`. + The label is a string specified by the charm, while the constraint is a string following + `the juju storage constraint directive format `_, + specifying the storage pool, number of volumes, and size of each volume. :devices: optional devices constraints. :endpoint_bindings: optional endpoint bindings :resources: identifies the revision to use for each resource of the @@ -562,6 +567,9 @@ class AddApplicationChange(ChangeInfo): :local_resources: identifies the path to the local resource of the application's charm. """ + + storage: Optional[Dict[str, str]] + @staticmethod def method(): """method returns an associated ID for the Juju API call. @@ -630,7 +638,11 @@ async def run(self, context): constraints=self.constraints, endpoint_bindings=self.endpoint_bindings, resources=resources, - storage=self.storage, + storage={ + label: parse_storage_constraint(constraint) + for label, constraint + in (self.storage or {}).items() + }, channel=self.channel, devices=self.devices, num_units=self.num_units, diff --git a/juju/model.py b/juju/model.py index f3b274b1..2d2cd40b 100644 --- a/juju/model.py +++ b/juju/model.py @@ -29,7 +29,6 @@ from .client import client, connector from .client.overrides import Caveat, Macaroon from .constraints import parse as parse_constraints -from .constraints import parse_storage_constraint from .controller import Controller, ConnectedController from .delta import get_entity_class, get_entity_delta from .errors import JujuAPIError, JujuError, JujuModelConfigError, JujuBackupError @@ -2116,7 +2115,7 @@ async def _deploy(self, charm_url, application, series, config, devices=devices, dryrun=False, placement=placement, - storage={k: parse_storage_constraint(v) for k, v in (storage or dict()).items()}, + storage=storage, trust=trust, base=charm_origin.base, channel=channel, @@ -2151,7 +2150,7 @@ async def _deploy(self, charm_url, application, series, config, endpoint_bindings=endpoint_bindings, num_units=num_units, resources=resources, - storage={k: parse_storage_constraint(v) for k, v in (storage or dict()).items()}, + storage=storage, placement=placement, devices=devices, attach_storage=attach_storage, diff --git a/tests/unit/test_bundle.py b/tests/unit/test_bundle.py index 5140dfd3..a6f69a80 100644 --- a/tests/unit/test_bundle.py +++ b/tests/unit/test_bundle.py @@ -1,10 +1,11 @@ # Copyright 2023 Canonical Ltd. # Licensed under the Apache V2, see LICENCE file for details. -from pathlib import Path import unittest +from pathlib import Path from unittest import mock from mock import patch, Mock, ANY +from typing import Dict, List, Tuple import yaml @@ -23,6 +24,7 @@ SetAnnotationsChange, ) from juju import charmhub +from juju import constraints from juju.client import client from toposort import CircularDependencyError @@ -121,14 +123,15 @@ def test_dict_params_missing_data(self): class TestAddApplicationChangeRun: - async def test_run_with_charmhub_charm(self): + storage_label = "some-label" + storage_constraint = "ebs,100G,1" change = AddApplicationChange(1, [], params={"charm": "charm", "series": "series", "application": "application", "options": "options", "constraints": "constraints", - "storage": "storage", + "storage": {storage_label: storage_constraint}, "endpoint-bindings": "endpoint_bindings", "resources": "resources", "devices": "devices", @@ -160,22 +163,106 @@ async def test_run_with_charmhub_charm(self): constraints="constraints", endpoint_bindings="endpoint_bindings", resources=["resource1"], - storage="storage", + storage={storage_label: constraints.parse_storage_constraint(storage_constraint)}, devices="devices", channel="channel", charm_origin=ANY, num_units="num_units") + async def test_run_with_storage_variations(self): + """Test that various valid storage constraints are parsed as expected before model._deploy is called. + + Uses the mock call logic from test_run_with_charmhub_charm, which will run before this test. + """ + storage_arg_pairs: List[Tuple[Dict[str, str], Dict[str, constraints.StorageConstraintDict]]] = [ + # (storage_arg_for_change, storage_arg_for_deploy) + ({'some-label': 'ebs,100G,1'}, {'some-label': {'count': 1, 'pool': 'ebs', 'size': 102400}}), + ({'some-label': 'ebs,2.1G,3'}, {'some-label': {'count': 3, 'pool': 'ebs', 'size': 2150}}), + ({'some-label': 'ebs,100G'}, {'some-label': {'count': 1, 'pool': 'ebs', 'size': 102400}}), + ({'some-label': 'ebs,2'}, {'some-label': {'count': 2, 'pool': 'ebs'}}), + ({'some-label': '200G,7'}, {'some-label': {'count': 7, 'size': 204800}}), + ({'some-label': 'ebs'}, {'some-label': {'count': 1, 'pool': 'ebs'}}), + ({'some-label': '10YB'}, {'some-label': {'count': 1, 'size': 11529215046068469760}}), + ({'some-label': '1'}, {'some-label': {'count': 1}}), + ({'some-label': '-1'}, {'some-label': {'count': 1}}), + ({'some-label': ''}, {'some-label': {'count': 1}}), + ( + { + 'some-label': '2.1G,3', + 'data': '1MiB,70', + 'logs': 'ebs,-1', + }, + { + 'some-label': {'count': 3, 'size': 2150}, + 'data': {'count': 70, 'size': 1}, + 'logs': {'count': 1, 'pool': 'ebs'} + }, + ), + ] + for storage_arg_for_change, storage_arg_for_deploy in storage_arg_pairs: + change = AddApplicationChange( + 1, + [], + params={ + "charm": "charm", + "series": "series", + "application": "application", + "options": "options", + "constraints": "constraints", + "storage": storage_arg_for_change, + "endpoint-bindings": "endpoint_bindings", + "resources": "resources", + "devices": "devices", + "num-units": "num_units", + "channel": "channel", + }, + ) + # mock model + model = Mock() + model._deploy = mock.AsyncMock(return_value=None) + model._add_charmhub_resources = mock.AsyncMock(return_value=["resource1"]) + model.applications = {} + # mock context + context = Mock() + context.resolve.return_value = "ch:charm1" + context.origins = {"ch:charm1": Mock()} + context.trusted = False + context.model = model + # mock info_func + info_func = mock.AsyncMock(return_value=["12345", "name"]) + # patch and call + with patch.object(charmhub.CharmHub, 'get_charm_id', info_func): + result = await change.run(context) + assert result == "application" + # asserts + model._deploy.assert_called_once() + model._deploy.assert_called_with( + charm_url="ch:charm1", + application="application", + series="series", + config="options", + constraints="constraints", + endpoint_bindings="endpoint_bindings", + resources=["resource1"], + storage=storage_arg_for_deploy, # we're testing this + devices="devices", + channel="channel", + charm_origin=ANY, + num_units="num_units", + ) + async def test_run_with_charmhub_charm_no_channel(self): """Test to verify if when the given channel is None, the channel defaults to "local/stable", which is the default channel value for the Charm Hub """ + storage_label = "some-label" + storage_constraint = "ebs,100G,1" change = AddApplicationChange(1, [], params={"charm": "charm", "series": "series", "application": "application", "options": "options", "constraints": "constraints", - "storage": "storage", + "storage": {storage_label: storage_constraint}, "endpoint-bindings": "endpoint_bindings", "resources": "resources", "devices": "devices", @@ -208,19 +295,21 @@ async def test_run_with_charmhub_charm_no_channel(self): constraints="constraints", endpoint_bindings="endpoint_bindings", resources=["resource1"], - storage="storage", + storage={storage_label: constraints.parse_storage_constraint(storage_constraint)}, devices="devices", channel="latest/stable", charm_origin=ANY, num_units="num_units") async def test_run_local(self): + storage_label = "some-label" + storage_constraint = "ebs,100G,1" change = AddApplicationChange(1, [], params={"charm": "local:charm", "series": "series", "application": "application", "options": "options", "constraints": "constraints", - "storage": "storage", + "storage": {storage_label: storage_constraint}, "endpoint-bindings": "endpoint_bindings", "devices": "devices", "num-units": "num_units"}) @@ -246,19 +335,21 @@ async def test_run_local(self): constraints="constraints", endpoint_bindings="endpoint_bindings", resources={}, - storage="storage", + storage={storage_label: constraints.parse_storage_constraint(storage_constraint)}, devices="devices", num_units="num_units", channel="", charm_origin=ANY) async def test_run_no_series(self): + storage_label = "some-label" + storage_constraint = "ebs,100G,1" change = AddApplicationChange(1, [], params={"charm": "ch:charm1", "series": "", "application": "application", "options": "options", "constraints": "constraints", - "storage": "storage", + "storage": {storage_label: storage_constraint}, "endpoint-bindings": "endpoint_bindings", "resources": "resources", "devices": "devices", @@ -289,7 +380,7 @@ async def test_run_no_series(self): constraints="constraints", endpoint_bindings="endpoint_bindings", resources=["resource1"], - storage="storage", + storage={storage_label: constraints.parse_storage_constraint(storage_constraint)}, devices="devices", channel="channel", charm_origin=ANY,