Merge lp:~sinzui/juju-ci-tools/bind-single-space into lp:juju-ci-tools

[Curtis Hovey]

Test global charm bindings.

Bug 1671489 shows that this command fails because the juju is confused by the instruction to bind a space to *all* bindings:
    juju deploy mycharm --bind aspace

Juju is wrongly insisting that the user name each binding the charm has. This branch extends the bindings test to deploy the datastore charm to a single space without specifying the exact charm bindings. The test reads the bundle, then sets up the spaces and machines in the maas. We must use the bundle to test this case.

I made secondary changes to help me test
1. The bundle is now saved to artifacts so that we can see what was deployed
2. I extends ModelClient and the fake bootstrap manager to allow me to pass --bind to deploy.
   I don't actually use --bind because of how the test is setup :(
   IN a future branch, I will explicitly test the command line.

[Curtis Hovey (sinzui)]

PS I hacked
    http://juju-ci.vapour.ws/view/Juju%20Revisions/job/functional-endpoint-bindings-maas-2-1/
to use this branch. develop and 2.1 pass with the fix (and fail without the fix.) When this branch is merged, I will restore the job to use juju-ci-tools.

[Christopher Lee (veebers)]

In the future it would be safer to clone the job in question to test-run the updated test. That way there is no need to remember to update the actual job (and we limit the risk of tests running old branches).

A couple of comments inline.

review: Needs Fixing

[lp:~sinzui/juju-ci-tools/bind-single-space]

1938. By Curtis Hovey on 2017-03-10

Remove vestigial comment and move bind arg to end of list.

=== modified file 'assess_endpoint_bindings.py'
--- assess_endpoint_bindings.py	2017-03-08 23:33:39 +0000
+++ assess_endpoint_bindings.py	2017-03-10 15:45:36 +0000
@@ -7,6 +7,7 @@
 import contextlib
 import logging
 import os
+import shutil
 import sys
 import yaml
 
@@ -226,6 +227,10 @@
                 "constraints": "spaces={},{}".format(space_data, space_public),
                 "series": "xenial",
             },
+            "2": {
+                "constraints": "spaces={},{}".format(space_data, space_public),
+                "series": "xenial",
+            },
         },
         "services": {
             "datastore": {
@@ -247,6 +252,15 @@
                     "datastore": space_data,
                 },
             },
+            "monitor": {
+                "charm": "./xenial/datastore",
+                "series": "xenial",
+                "num_units": 1,
+                "to": ["2"],
+                "bindings": {
+                    "": space_data,
+                },
+            },
         },
         "relations": [
             ["datastore:datastore", "frontend:datastore"],
@@ -281,6 +295,7 @@
 
 
 def bootstrap_and_test(bootstrap_manager, bundle_path, machine):
+    shutil.copy(bundle_path, bootstrap_manager.log_dir)
     with bootstrap_manager.booted_context(False, no_gui=True):
         client = bootstrap_manager.client
         log.info("Deploying bundle.")

=== modified file 'jujupy/client.py'
--- jujupy/client.py	2017-03-02 20:03:35 +0000
+++ jujupy/client.py	2017-03-10 15:45:36 +0000
@@ -2022,7 +2022,7 @@
 
     def deploy(self, charm, repository=None, to=None, series=None,
                service=None, force=False, resource=None, num=None,
-               storage=None, constraints=None, alias=None):
+               storage=None, constraints=None, alias=None, bind=None):
         args = [charm]
         if service is not None:
             args.extend([service])
@@ -2040,6 +2040,8 @@
             args.extend(['--storage', storage])
         if constraints is not None:
             args.extend(['--constraints', constraints])
+        if bind is not None:
+            args.extend(['--bind', bind])
         if alias is not None:
             args.extend([alias])
         return self.juju('deploy', tuple(args))

=== modified file 'jujupy/tests/test_client.py'
--- jujupy/tests/test_client.py	2017-03-02 16:50:50 +0000
+++ jujupy/tests/test_client.py	2017-03-10 15:45:36 +0000
@@ -1233,6 +1233,12 @@
         mock_juju.assert_called_with(
             'deploy', ('mondogb', '--constraints', 'virt-type=kvm'))
 
+    def test_deploy_bind(self):
+        env = ModelClient(JujuData('foo', {'type': 'local'}), None, None)
+        with patch.object(env, 'juju') as mock_juju:
+            env.deploy('mydb', bind='backspace')
+        mock_juju.assert_called_with('deploy', ('mydb', '--bind', 'backspace'))
+
     def test_deploy_aliased(self):
         env = ModelClient(JujuData('foo', {'type': 'local'}), None, None)
         with patch.object(env, 'juju') as mock_juju:

=== modified file 'tests/test_assess_endpoint_bindings.py'
--- tests/test_assess_endpoint_bindings.py	2016-11-24 17:19:37 +0000
+++ tests/test_assess_endpoint_bindings.py	2017-03-10 15:45:36 +0000
@@ -1,19 +1,28 @@
 """Tests for assess_endpoint_bindings module."""
 
 import logging
-from mock import Mock, patch
+from mock import (
+    Mock,
+    patch,
+    )
+import os
 import StringIO
 
 from assess_endpoint_bindings import (
+    bootstrap_and_test,
+    create_test_charms,
     ensure_spaces,
     parse_args,
     machine_spaces_for_bundle,
     main,
 )
+from jujupy.client import JujuData
 from tests import (
     parse_error,
     TestCase,
 )
+from test_deploy_stack import FakeBootstrapManager
+from utility import temp_dir
 
 
 class TestParseArgs(TestCase):
@@ -152,6 +161,41 @@
         self.assertEqual(machines, [app_spaces, db_spaces, db_spaces])
 
 
+class AssessEndpointBindings(TestCase):
+
+    def test_create_test_charms(self):
+        bundle, charms = create_test_charms()
+        self.assertEqual(
+            ['0', '1', '2'],
+            sorted(bundle['machines'].keys()))
+        self.assertEqual(
+            ['datastore', 'frontend', 'monitor'],
+            sorted(bundle['services'].keys()))
+        self.assertEqual('datastore', charms[0].metadata['name'])
+        self.assertEqual('frontend', charms[1].metadata['name'])
+
+    def test_bootstrap_and_test(self):
+        juju_data = JujuData(
+            'foo', {'type': 'bar', 'region': 'region'}, juju_home='baz')
+        client = Mock(
+            spec=['bootstrap', 'kill_controller', 'deploy',
+                  'wait_for_started', 'wait_for_workloads'],
+            env=juju_data)
+        bootstrap_manager = FakeBootstrapManager(client)
+        with temp_dir() as bundle_dir:
+            bundle_path = os.path.join(bundle_dir, 'bundle.yaml')
+            with open(bundle_path, 'w') as bf:
+                bf.write('bundle')
+            with temp_dir() as log_dir:
+                bootstrap_manager.log_dir = log_dir
+                bootstrap_and_test(bootstrap_manager, bundle_path, None)
+                archived_bundle = os.path.join(log_dir, 'bundle.yaml')
+                self.assertIsTrue(os.path.exists(archived_bundle))
+        client.deploy.assert_called_once_with(bundle_path)
+        client.wait_for_started.assert_called_once_with()
+        client.wait_for_workloads.assert_called_once_with()
+
+
 class TestMain(TestCase):
 
     def test_main(self):

=== modified file 'tests/test_deploy_stack.py'
--- tests/test_deploy_stack.py	2017-03-01 19:02:24 +0000
+++ tests/test_deploy_stack.py	2017-03-10 15:45:36 +0000
@@ -1052,7 +1052,7 @@
         self.torn_down = True
 
     @contextmanager
-    def booted_context(self, upload_tools):
+    def booted_context(self, upload_tools, **kwargs):
         with self.top_context() as machines:
             with self.bootstrap_context(machines):
                 self.client.bootstrap(upload_tools)