Merge pull request #566 from cderici/bundles-with-overlays

#566

### Description

This PR adds the support for overlays in bundle deployments.

Fixes #510 

This PR relies on a change in Juju's api for getting changes for bundles with overlays (multi-part yaml support), juju/juju#13448.

Jira card [#142](https://warthogs.atlassian.net/browse/JUJU-142)



### QA Steps

`tests/integration/test_model.py` includes some new tests for the added support.

```
tox -e integration -- tests/integration/test_model.py
```

### Notes & Discussion

Please do not merge yet, as a couple of small things need to be done/added for this to be ready to land:

- [x] Add a PR on Juju for `GetChange` juju/juju#13448
- [x] Land that PR on Juju
- [x] Charmstore bundles with `--overlay` argument, along with its test
- [x] A test for a multi-part overlay as an `--overlay` argument to a local bundle being deployed
- [x] A test for a multi-part overlay as an `--overlay` argument to a charmstore bundle being deployed
- [x] Make sure that we resolve and inline `config: include-file://` and `config: include-base64://` here in `pylibjuju` side

juju/bundle.py

@@ -3,6 +3,7 @@
 import os
 import zipfile
 import requests
+import base64
 from contextlib import closing
 from pathlib import Path
 
@@ -29,6 +30,9 @@ def __init__(self, model, trusted=False, forced=False):
         self.model = model
         self.trusted = trusted
         self.forced = forced
+        self.bundle = None
+        self.overlays = []
+        self.overlay_removed_charms = set()
 
         self.charmstore = model.charmstore
         self.plan = []
@@ -77,7 +81,7 @@ async def _validate_bundle(self, bundle):
         """Validate the bundle for known issues, raises an error if it
         encounters a known problem
         """
-        apps_dict = bundle.get('applications', bundle.get('services', {}))
+        apps_dict = bundle.get('applications', {})
         for app_name in self.applications:
             app_dict = apps_dict[app_name]
             app_trusted = app_dict.get('trust')
@@ -103,7 +107,7 @@ async def _handle_local_charms(self, bundle, bundle_dir):
         apps, args = [], []
 
         default_series = bundle.get('series')
-        apps_dict = bundle.get('applications', bundle.get('services', {}))
+        apps_dict = bundle.get('applications', {})
         for app_name in self.applications:
             app_dict = apps_dict[app_name]
             charm_dir = app_dict['charm']
@@ -152,14 +156,70 @@ async def _handle_local_charms(self, bundle, bundle_dir):
                     app_name,
                     charm_url,
                     utils.get_local_charm_metadata(charm_dir),
-                    resources=bundle["applications"][app_name].get("resources", {}),
+                    resources=bundle.get('applications', {app_name: {}})[app_name].get("resources", {}),
                 )
                 apps_dict[app_name]['charm'] = charm_url
                 apps_dict[app_name]["resources"] = resources
 
         return bundle
 
-    async def fetch_plan(self, charm_url, origin):
+    def _resolve_include_file_config(self, bundle_dir):
+        """if any of the applications (including the ones in the overlays)
+        have "config: include-file:..." or "config:
+        include-base64:...", then we have to resolve and inline them
+        into the bundle here because they're all files with local
+        relative paths, so backend can't handle them.
+
+        """
+        bundle_apps = [self.bundle.get('applications', {})]
+        overlay_apps = [overlay.get('applications', {}) for overlay in self.overlays]
+
+        for apps in bundle_apps + overlay_apps:
+            for app_name, app in apps.items():
+
+                if app and 'options' in app:
+                    if 'config' in app['options'] and app['options']['config'].startswith('include-file'):
+                        # resolve the file
+                        if not bundle_dir:
+                            raise NotImplementedError('unable to resolve paths for config:include-file for non-local charms')
+                        try:
+                            config_path = (bundle_dir / Path(app['options']['config'].split('//')[1])).resolve()
+                        except IndexError:
+                            raise JujuError('the path for the included file should start with // and be relative to the bundle')
+                        if not config_path.exists():
+                            raise JujuError('unable to locate config file : %s for : %s' % (config_path, app_name))
+
+                        # get the contents of the file
+                        config_contents = yaml.safe_load(config_path.read_text())
+
+                        # inline the configurations for the current app into
+                        # the app['options']
+                        for key, val in config_contents[app_name].items():
+                            app['options'][key] = val
+
+                        # remove the 'include-file' config
+                        app['options'].pop('config')
+
+                    for option_key, option_val in app['options'].items():
+                        if isinstance(option_val, str) and option_val.startswith('include-base64'):
+                            # resolve the file
+                            if not bundle_dir:
+                                raise NotImplementedError('unable to resolve paths for config:include-base64 for non-local charms')
+                            try:
+                                base64_path = (bundle_dir / Path(option_val.split('//')[1])).resolve()
+                            except IndexError:
+                                raise JujuError('the path for the included base64 file should start with // and be relative to the bundle')
+
+                            if not base64_path.exists():
+                                raise JujuError('unable to locate the base64 file : %s for : %s' % (base64_path, app_name))
+
+                            # inline the base64 encoded config value
+                            base64_contents = base64.b64decode(base64_path.read_text())
+                            app['options'][option_key] = base64_contents
+
+        return self.bundle, self.overlays
+
+    async def fetch_plan(self, charm_url, origin, overlays=[]):
         entity_id = charm_url.path()
         is_local = Schema.LOCAL.matches(charm_url.schema)
         bundle_dir = None
@@ -181,14 +241,40 @@ async def fetch_plan(self, charm_url, origin):
         if not bundle_yaml:
             raise JujuError('empty bundle, nothing to deploy')
 
-        self.bundle = yaml.safe_load(bundle_yaml)
+        _bundles = [b for b in yaml.safe_load_all(bundle_yaml)]
+        self.overlays = _bundles[1:]
+        self.bundle = _bundles[0]
+
+        if overlays != []:
+            for overlay_yaml_path in overlays:
+                try:
+                    overlay_contents = Path(overlay_yaml_path).read_text()
+                except (OSError, IOError) as e:
+                    raise JujuError('unable to open overlay %s \n %s' % (overlay_yaml_path, e))
+                self.overlays.extend(yaml.safe_load_all(overlay_contents))
+
+        # gather the names of the removed charms so model.deploy
+        # wouldn't wait for them to appear in the model
+        for overlay in self.overlays:
+            overlay_apps = overlay.get('applications', {})
+            for charm_name, val in overlay_apps.items():
+                if val is None:
+                    self.overlay_removed_charms.add(charm_name)
+
         self.bundle = await self._validate_bundle(self.bundle)
         if is_local:
             self.bundle = await self._handle_local_charms(self.bundle, bundle_dir)
 
+        self.bundle, self.overlays = self._resolve_include_file_config(bundle_dir)
+
+        _yaml_data = [yaml.dump(self.bundle)]
+        for overlay in self.overlays:
+            _yaml_data.append(yaml.dump(overlay).replace('null', ''))
+        yaml_data = "---\n".join(_yaml_data)
+
         self.plan = await self.bundle_facade.GetChanges(
             bundleurl=entity_id,
-            yaml=yaml.dump(self.bundle))
+            yaml=yaml_data)
 
         if self.plan.errors:
             raise JujuError(self.plan.errors)
@@ -298,14 +384,12 @@ async def execute_plan(self):
 
     @property
     def applications(self):
-        apps_dict = self.bundle.get('applications',
-                                    self.bundle.get('services', {}))
-        return list(apps_dict.keys())
+        apps_dict = self.bundle.get('applications', {})
+        return set(apps_dict.keys()) - self.overlay_removed_charms
 
     @property
     def applications_specs(self):
-        return self.bundle.get('applications',
-                               self.bundle.get('services', {}))
+        return self.bundle.get('applications', {})
 
     def resolve_relation(self, reference):
         parts = reference.split(":", maxsplit=1)

juju/model.py

@@ -1533,16 +1533,15 @@ def _get_series(self, entity_url, entity):
         return ss['SupportedSeries'][0]
 
     async def deploy(
-            self, entity_url, application_name=None, bind=None, budget=None,
+            self, entity_url, application_name=None, bind=None,
             channel=None, config=None, constraints=None, force=False,
-            num_units=1, plan=None, resources=None, series=None, storage=None,
-            to=None, devices=None, trust=False):
+            num_units=1, overlays=[], plan=None, resources=None, series=None,
+            storage=None, to=None, devices=None, trust=False):
         """Deploy a new service or bundle.
 
         :param str entity_url: Charm or bundle url
         :param str application_name: Name to give the service
         :param dict bind: <charm endpoint>:<network space> pairs
-        :param dict budget: <budget name>:<limit> pairs
         :param str channel: Charm store channel from which to retrieve
             the charm or bundle, e.g. 'edge'
         :param dict config: Charm configuration dictionary
@@ -1551,6 +1550,7 @@ async def deploy(
         :param bool force: Allow charm to be deployed to a machine running
             an unsupported series
         :param int num_units: Number of units to deploy
+        :param [] overlays: Bundles to overlay on the primary bundle, applied in order
         :param str plan: Plan under which to deploy charm
         :param dict resources: <resource name>:<file path> pairs
         :param str series: Series on which to deploy
@@ -1597,10 +1597,10 @@ async def deploy(
         series = res.origin.series or series
         if res.is_bundle:
             handler = BundleHandler(self, trusted=trust, forced=force)
-            await handler.fetch_plan(url, res.origin)
+            await handler.fetch_plan(url, res.origin, overlays=overlays)
             await handler.execute_plan()
             extant_apps = {app for app in self.applications}
-            pending_apps = set(handler.applications) - extant_apps
+            pending_apps = handler.applications - extant_apps
             if pending_apps:
                 # new apps will usually be in the model by now, but if some
                 # haven't made it yet we'll need to wait on them to be added
@@ -1612,6 +1612,8 @@ async def deploy(
             return [app for name, app in self.applications.items()
                     if name in handler.applications]
         else:
+            if overlays:
+                raise JujuError("options provided but not supported when deploying a charm: overlays=%s" % overlays)
             # XXX: we're dropping local resources here, but we don't
             # actually support them yet anyway
             if not res.is_local:

tests/integration/bundle/bundle-include-base64.yaml

@@ -0,0 +1,15 @@
+series: xenial
+applications:
+  ghost:
+    charm: "cs:ghost-19"
+    num_units: 1
+  mysql:
+    charm: "cs:trusty/mysql-57"
+    num_units: 1
+    options:
+      max-connections: 2
+      tuning-level: include-base64://config-base64.yaml
+  test:
+    charm: "../charm"
+relations:
+  - ["ghost", "mysql"]

tests/integration/bundle/bundle-include-file.yaml

@@ -0,0 +1,14 @@
+series: xenial
+applications:
+  ghost:
+    charm: "cs:ghost-19"
+    num_units: 1
+    options:
+      config: include-file://config1.yaml
+  mysql:
+    charm: "cs:trusty/mysql-57"
+    num_units: 1
+  test:
+    charm: "../charm"
+relations:
+  - ["ghost", "mysql"]

tests/integration/bundle/bundle-resource-rev.yaml

@@ -1,5 +1,5 @@
 series: xenial
-services:
+applications:
   ghost:
     charm: "cs:ghost-19"
     num_units: 1

tests/integration/bundle/bundle.yaml

@@ -1,5 +1,5 @@
 series: xenial
-services:
+applications:
   ghost:
     charm: "cs:ghost-19"
     num_units: 1

tests/integration/bundle/config-base64.yaml

@@ -0,0 +1 @@
+ZmFzdA==

tests/integration/bundle/config1.yaml

@@ -0,0 +1,3 @@
+ghost:
+  url: "http://my-ghost.blg"
+  port: 2369

tests/integration/bundle/test-overlays/bundle-with-overlay-multi.yaml

@@ -0,0 +1,14 @@
+series: xenial
+applications:
+  ghost:
+    charm: "cs:ghost-19"
+    num_units: 1
+  mysql:
+    charm: "cs:trusty/mysql-57"
+    num_units: 1
+relations:
+  - ["ghost", "mysql"]    
+--- # overlay.yaml
+description: Overlay to remove the ghost app and the relation
+applications:
+  ghost:

tests/integration/bundle/test-overlays/wiki-multi-overlay.yaml

@@ -0,0 +1,17 @@
+description: An overlay for the wiki-simple bundle to remove mysql and add memcached
+applications:
+  mysql:
+  memcached:
+    charm: "cs:memcached-34"
+    num_units: 1
+relations:
+  - ["wiki", "memcached"]
+---
+description: Another overlay to remove memcached and add back the mysql and relate
+applications:
+  memcached:
+  mysql:
+    charm: "cs:trusty/mysql-57"
+    num_units: 1
+relations:
+  - ["wiki:db", "mysql:db"]

tests/integration/bundle/test-overlays/wiki-overlay1.yaml

@@ -0,0 +1,7 @@
+description: An overlay for the wiki-simple bundle to remove mysql and add memcached
+applications:
+  test:
+  mysql:
+  memcached:
+    charm: "cs:memcached-34"
+    num_units: 1

tests/integration/bundle/test-overlays/wiki-overlay2.yaml

@@ -0,0 +1,6 @@
+description: Another overlay to remove memcached and add back the mysql and relate
+applications:
+  memcached:
+  ghost:
+    options:
+      config: include-file://config1.yaml

tests/integration/bundle/test-overlays/wiki-simple-overlay.yaml

@@ -0,0 +1,8 @@
+description: An overlay for the wiki-simple bundle to remove mysql and add memcached
+applications:
+  mysql:
+  memcached:
+    charm: "cs:memcached-34"
+    num_units: 1
+relations:
+  - ["wiki", "memcached"]