From: Cory Johns Date: Fri, 3 Mar 2017 19:39:45 +0000 (-0500) Subject: Fix single app deploy not using config_yaml X-Git-Tag: 0.4.0~16^2~2 X-Git-Url: https://osm.etsi.org/gitweb/?p=osm%2FN2VC.git;a=commitdiff_plain;h=8293ed139b3b3dc741d75184de46936e117d923e Fix single app deploy not using config_yaml Refactored to reduce duplicated code and ensure that both deploy paths (bundle and single app) use new config_yaml param instead of config. --- diff --git a/juju/client/connection.py b/juju/client/connection.py index 77edda5..c24f8c1 100644 --- a/juju/client/connection.py +++ b/juju/client/connection.py @@ -15,7 +15,7 @@ import asyncio import yaml from juju import tag -from juju.errors import JujuAPIError, JujuConnectionError +from juju.errors import JujuError, JujuAPIError, JujuConnectionError log = logging.getLogger("websocket") @@ -221,8 +221,7 @@ class Connection: """ jujudata = JujuData() controller_name = jujudata.current_controller() - models = jujudata.models()[controller_name] - model_name = models['current-model'] + model_name = jujudata.current_model() return await cls.connect_model( '{}:{}'.format(controller_name, model_name), loop) @@ -339,6 +338,12 @@ class JujuData: output = yaml.safe_load(output) return output.get('current-controller', '') + def current_model(self, controller_name=None): + models = self.models()[controller_name or self.current_controller()] + if 'current-model' not in models: + raise JujuError('No current model') + return models['current-model'] + def controllers(self): return self._load_yaml('controllers.yaml', 'controllers') diff --git a/juju/client/facade.py b/juju/client/facade.py index 5ed5320..817b37b 100644 --- a/juju/client/facade.py +++ b/juju/client/facade.py @@ -438,6 +438,8 @@ class Type: @classmethod def from_json(cls, data): + if isinstance(data, cls): + return data if isinstance(data, str): data = json.loads(data) d = {} diff --git a/juju/model.py b/juju/model.py index 812f35b..7a52ed5 100644 --- a/juju/model.py +++ b/juju/model.py @@ -974,11 +974,6 @@ class Model(object): - series is required; how do we pick a default? """ - if to: - placement = parse_placement(to) - else: - placement = [] - if storage: storage = { k: client.Constraints(**v) @@ -992,9 +987,7 @@ class Model(object): entity_id = await self.charmstore.entityId(entity_url) \ if not is_local else entity_url - app_facade = client.ApplicationFacade() client_facade = client.ClientFacade() - app_facade.connect(self.connection) client_facade.connect(self.connection) is_bundle = ((is_local and @@ -1019,9 +1012,6 @@ class Model(object): return [app for name, app in self.applications.items() if name in handler.applications] else: - log.debug( - 'Deploying %s', entity_id) - if not is_local: parts = entity_id[3:].split('/') if parts[0].startswith('~'): @@ -1047,26 +1037,52 @@ class Model(object): "Pass a 'series' kwarg to Model.deploy().".format( charm_dir)) entity_id = await self.add_local_charm_dir(charm_dir, series) + return await self._deploy( + charm_url=entity_id, + application=application_name, + series=series, + config=config or {}, + constraints=constraints, + endpoint_bindings=bind, + resources=resources, + storage=storage, + channel=channel, + num_units=num_units, + placement=parse_placement(to), + ) + + async def _deploy(self, charm_url, application, series, config, + constraints, endpoint_bindings, resources, storage, + channel=None, num_units=None, placement=None): + log.info('Deploying %s', charm_url) + + # stringify all config values for API, and convert to YAML + config = {k: str(v) for k, v in config.items()} + config = yaml.dump({application: config}, + default_flow_style=False) + + app_facade = client.ApplicationFacade() + app_facade.connect(self.connection) app = client.ApplicationDeploy( - application=application_name, + charm_url=charm_url, + application=application, + series=series, channel=channel, - charm_url=entity_id, - config=config, + config_yaml=config, constraints=parse_constraints(constraints), - endpoint_bindings=bind, + endpoint_bindings=endpoint_bindings, num_units=num_units, resources=resources, - series=series, storage=storage, + placement=placement, ) - app.placement = placement result = await app_facade.Deploy([app]) errors = [r.error.message for r in result.results if r.error] if errors: raise JujuError('\n'.join(errors)) - return await self._wait_for_new('application', application_name) + return await self._wait_for_new('application', application) def destroy(self): """Terminate all machines and resources for this model. @@ -1672,28 +1688,16 @@ class BundleHandler(object): """ # resolve indirect references charm = self.resolve(charm) - # stringify all config values for API, and convert to YAML - options = {k: str(v) for k, v in options.items()} - options = yaml.dump({application: options}, default_flow_style=False) - # build param object - app = client.ApplicationDeploy( + await self.model._deploy( charm_url=charm, - series=series, application=application, - # Pass options to config-yaml rather than config, as - # config-yaml invokes a newer codepath that better handles - # empty strings in the options values. - config_yaml=options, - constraints=parse_constraints(constraints), - storage=storage, + series=series, + config=options, + constraints=constraints, endpoint_bindings=endpoint_bindings, resources=resources, + storage=storage, ) - # do the do - log.info('Deploying %s', charm) - await self.app_facade.Deploy([app]) - # ensure the app is in the model for future operations - await self.model._wait_for_new('application', application) return application async def addUnit(self, application, to): diff --git a/tests/base.py b/tests/base.py index 382da43..af386ea 100644 --- a/tests/base.py +++ b/tests/base.py @@ -1,9 +1,11 @@ -import uuid +import mock import subprocess +import uuid import pytest from juju.controller import Controller +from juju.client.connection import JujuData def is_bootstrapped(): @@ -29,9 +31,16 @@ class CleanModel(): model_name = 'model-{}'.format(uuid.uuid4()) self.model = await self.controller.add_model(model_name) + # Ensure that we connect to the new model by default. This also + # prevents failures if test was started with no current model. + self._patch_cm = mock.patch.object(JujuData, 'current_model', + return_value=model_name) + self._patch_cm.start() + return self.model async def __aexit__(self, exc_type, exc, tb): + self._patch_cm.stop() await self.model.disconnect() await self.controller.destroy_model(self.model.info.uuid) await self.controller.disconnect()