Skip to content

Commit

Permalink
Merge pull request #1053 from luissimas/fix-storage-constraints
Browse files Browse the repository at this point in the history
#1053

#### Description

Fixes: #1052. Parses the storage constraints when deploying applications. Before this change the storage constraints where not parsing, resulting in an error when deploying bundles that contained applications with storage definitions.

#### QA Steps

The following python script can be used to verify both the bug in the current version as well as the fix implemented:

```python
import asyncio
from juju.model import Model

bundle_file = "./bundle.yaml"

bundle = """
name: sample-bundle

series: jammy

machines:
 "0":
 constraints: instance-type=type1

applications:
 swift-storage:
 charm: swift-storage
 channel: yoga/stable
 num_units: 1
 to:
 - "0"
 storage:
 block-devices: cinder,1,5G
"""

async def main():
 with open(bundle_file, "w") as f:
 f.write(bundle)

 model = Model()
 await model.connect()
 await model.deploy(bundle_file)

asyncio.run(main())
```

All CI tests need to pass.

#### Notes & Discussion

I wasn't able to add a regression test for this fix since the `juju-qa-test` charm does not support the addition of storage devices. With that said, I'm open to suggestions on how to test this behavior to make sure it's correct and that it stays that way on future changes.
  • Loading branch information
jujubot authored May 31, 2024
2 parents 77f02b2 + 49b1fdd commit 53cd33d
Show file tree
Hide file tree
Showing 3 changed files with 26 additions and 2 deletions.
5 changes: 3 additions & 2 deletions juju/model.py
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
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
Expand Down Expand Up @@ -2115,7 +2116,7 @@ async def _deploy(self, charm_url, application, series, config,
devices=devices,
dryrun=False,
placement=placement,
storage=storage,
storage={k: parse_storage_constraint(v) for k, v in (storage or dict()).items()},
trust=trust,
base=charm_origin.base,
channel=channel,
Expand Down Expand Up @@ -2150,7 +2151,7 @@ async def _deploy(self, charm_url, application, series, config,
endpoint_bindings=endpoint_bindings,
num_units=num_units,
resources=resources,
storage=storage,
storage={k: parse_storage_constraint(v) for k, v in (storage or dict()).items()},
placement=placement,
devices=devices,
attach_storage=attach_storage,
Expand Down
11 changes: 11 additions & 0 deletions tests/integration/bundle/bundle-with-storage-constraint.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
name: bundle-with-storage-constraint

series: jammy

applications:
postgresql:
charm: postgresql
num_units: 1
channel: 14/stable
storage:
pgdata: lxd,8G
12 changes: 12 additions & 0 deletions tests/integration/test_model.py
Original file line number Diff line number Diff line change
Expand Up @@ -211,6 +211,18 @@ async def test_deploy_invalid_bundle():
await model.deploy(str(bundle_path))


@base.bootstrapped
@pytest.mark.bundle
async def test_deploy_bundle_with_storage_constraint():
bundle_path = INTEGRATION_TEST_DIR / 'bundle' / 'bundle-with-storage-constraint.yaml'

async with base.CleanModel() as model:
await model.deploy(bundle_path)
await wait_for_bundle(model, bundle_path)
storage = await model.list_storage()
assert len(storage) == 1


@base.bootstrapped
async def test_deploy_local_charm():
charm_path = TESTS_DIR / 'charm'
Expand Down

0 comments on commit 53cd33d

Please sign in to comment.