Merge lp:~sinzui/juju-ci-tools/controller-model into lp:juju-ci-tools

[Curtis Hovey]

Rename 'admin' model to 'controller', preserve old name in EnvJujuClient2B7.

The Juju team are ready to rename the 'admin' model to 'controller' model. These changes supports their branch. The get_admin_model_name() returns 'controller'. The original method is moved to the new EnvJujuClient2B7. The factory function will use the EnvJujuClient2B7 for 2.0-beta7.

Several ClientTest test methods were updated. Four were adapted to run in TestEnvJujuClient2B7. The FakeControllerState was updated to use controller. A few logging tests were also updated.

Thee are some lint spacing issues also fixed.

[Aaron Bentley (abentley)]

Looks good. Should we also be updating our API to reflect the name change, e.g. get_controller_model_name? That's "clean-as-you-go", I think.

Also, I think landing this will break testing of beta8 until the corresponding juju change is landed.

review: Approve

=== modified file 'jujupy.py'
--- jujupy.py	2016-05-24 23:36:01 +0000
+++ jujupy.py	2016-05-25 04:02:00 +0000
@@ -530,6 +530,8 @@
             client_class = EnvJujuClient2B2
         elif re.match('^2\.0-(beta[3-6])', version):
             client_class = EnvJujuClient2B3
+        elif re.match('^2\.0-(beta7)', version):
+            client_class = EnvJujuClient2B7
         else:
             client_class = EnvJujuClient
         return client_class(env, version, full_path, debug=debug)
@@ -924,7 +926,7 @@
         raise JujuResourceTimeout(
             'Timeout waiting for a resource to be downloaded. '
             'ResourceId: {} Service or Unit: {} Timeout: {}'.format(
-                    resource_id, service_or_unit, timeout))
+                resource_id, service_or_unit, timeout))
 
     def upgrade_charm(self, service, charm_path=None):
         args = (service,)
@@ -1104,7 +1106,7 @@
         Return the name of the environment when an 'admin' model does
         not exist.
         """
-        return 'admin'
+        return 'controller'
 
     def _acquire_model_client(self, name):
         """Get a client for a model with the supplied name.
@@ -1401,7 +1403,7 @@
         output = self.get_juju_output('add-user', *args, include_e=False)
         return self._get_register_command(output)
 
-    def revoke(self, username, models=None,  permissions='read'):
+    def revoke(self, username, models=None, permissions='read'):
         if models is None:
             models = self.env.environment
 
@@ -1410,7 +1412,18 @@
         self.controller_juju('revoke', args)
 
 
-class EnvJujuClient2B3(EnvJujuClient):
+class EnvJujuClient2B7(EnvJujuClient):
+
+    def get_admin_model_name(self):
+        """Return the name of the 'admin' model.
+
+        Return the name of the environment when an 'admin' model does
+        not exist.
+        """
+        return 'admin'
+
+
+class EnvJujuClient2B3(EnvJujuClient2B7):
 
     def _add_model(self, model_name, config_file):
         self.controller_juju('create-model', (

=== modified file 'tests/test_deploy_stack.py'
--- tests/test_deploy_stack.py	2016-05-23 03:02:18 +0000
+++ tests/test_deploy_stack.py	2016-05-25 04:02:00 +0000
@@ -1238,7 +1238,7 @@
 
         self.assertItemsEqual(
             [call(client, os.path.join(log_dir, 'name'), None, {}),
-             call(clients['admin'], os.path.join(log_dir, 'admin'),
+             call(clients['controller'], os.path.join(log_dir, 'controller'),
                   'foo/models/cache.yaml', {})],
             del_mock.mock_calls)
 
@@ -1260,7 +1260,7 @@
 
         self.assertItemsEqual(
             [call(client, os.path.join(log_dir, 'name'), None, {}),
-             call(clients['admin'], os.path.join(log_dir, 'admin'),
+             call(clients['controller'], os.path.join(log_dir, 'controller'),
                   'foo/models/cache.yaml', {})],
             del_mock.mock_calls)
 

=== modified file 'tests/test_jujupy.py'
--- tests/test_jujupy.py	2016-05-24 23:36:01 +0000
+++ tests/test_jujupy.py	2016-05-25 04:02:00 +0000
@@ -48,6 +48,7 @@
     EnvJujuClient2A2,
     EnvJujuClient2B2,
     EnvJujuClient2B3,
+    EnvJujuClient2B7,
     ErroredUnit,
     GroupReporter,
     get_cache_path,
@@ -129,7 +130,7 @@
         default_model = self.add_model(model_name)
         default_model.name = model_name
         if separate_admin:
-            admin_model = default_model.controller.add_model('admin')
+            admin_model = default_model.controller.add_model('controller')
         else:
             admin_model = default_model
         self.admin_model = admin_model
@@ -297,7 +298,7 @@
         self.debug = debug
         self.juju_timings = {}
 
-    def clone(self, full_path=None,  version=None, debug=None,
+    def clone(self, full_path=None, version=None, debug=None,
               feature_flags=None):
         if version is None:
             version = self.version
@@ -1007,6 +1008,7 @@
             yield '2.0-beta5'
             yield '2.0-beta6'
             yield '2.0-beta7'
+            yield '2.0-beta8'
             yield '2.0-delta1'
 
         context = patch.object(
@@ -1068,10 +1070,13 @@
             self.assertIs(type(client), EnvJujuClient2B3)
             self.assertEqual(client.version, '2.0-beta6')
             client = EnvJujuClient.by_version(None)
-            self.assertIs(type(client), EnvJujuClient)
+            self.assertIs(type(client), EnvJujuClient2B7)
             self.assertEqual(client.version, '2.0-beta7')
             client = EnvJujuClient.by_version(None)
             self.assertIs(type(client), EnvJujuClient)
+            self.assertEqual(client.version, '2.0-beta8')
+            client = EnvJujuClient.by_version(None)
+            self.assertIs(type(client), EnvJujuClient)
             self.assertEqual(client.version, '2.0-delta1')
             with self.assertRaises(StopIteration):
                 EnvJujuClient.by_version(None)
@@ -1154,10 +1159,11 @@
         env = JujuData('foo')
         client = EnvJujuClient(env, None, 'my/juju/bin')
         with patch.object(client, 'get_admin_model_name',
-                          return_value='admin') as gamn_mock:
+                          return_value='controller') as gamn_mock:
             full = client._full_args('bar', False, ('baz', 'qux'), admin=True)
         self.assertEqual((
-            'juju', '--show-log', 'bar', '-m', 'admin', 'baz', 'qux'), full)
+            'juju', '--show-log', 'bar', '-m', 'controller', 'baz', 'qux'),
+            full)
         gamn_mock.assert_called_once_with()
 
     def test__bootstrap_config(self):
@@ -2062,7 +2068,7 @@
     def test_get_admin_model_name(self):
         models = {
             'models': [
-                {'name': 'admin', 'model-uuid': 'aaaa'},
+                {'name': 'controller', 'model-uuid': 'aaaa'},
                 {'name': 'bar', 'model-uuid': 'bbbb'}],
             'current-model': 'bar'
         }
@@ -2071,7 +2077,7 @@
                           return_value=models) as gm_mock:
             admin_name = client.get_admin_model_name()
         self.assertEqual(0, gm_mock.call_count)
-        self.assertEqual('admin', admin_name)
+        self.assertEqual('controller', admin_name)
 
     def test_get_admin_model_name_without_admin(self):
         models = {
@@ -2083,21 +2089,22 @@
         client = EnvJujuClient(JujuData('foo'), None, None)
         with patch.object(client, 'get_models', return_value=models):
             admin_name = client.get_admin_model_name()
-        self.assertEqual('admin', admin_name)
+        self.assertEqual('controller', admin_name)
 
     def test_get_admin_model_name_no_models(self):
         client = EnvJujuClient(JujuData('foo'), None, None)
         with patch.object(client, 'get_models', return_value={}):
             admin_name = client.get_admin_model_name()
-        self.assertEqual('admin', admin_name)
+        self.assertEqual('controller', admin_name)
 
     def test_get_admin_client(self):
         client = EnvJujuClient(
             JujuData('foo', {'bar': 'baz'}, 'myhome'), None, None)
         admin_client = client.get_admin_client()
         admin_env = admin_client.env
-        self.assertEqual('admin', admin_env.environment)
-        self.assertEqual({'bar': 'baz', 'name': 'admin'}, admin_env.config)
+        self.assertEqual('controller', admin_env.environment)
+        self.assertEqual(
+            {'bar': 'baz', 'name': 'controller'}, admin_env.config)
 
     def test_list_controllers(self):
         client = EnvJujuClient(JujuData('foo'), None, None)
@@ -3006,6 +3013,49 @@
         self.assertTrue(output.startswith('juju register'))
 
 
+class TestEnvJujuClient2B7(ClientTest):
+
+    def test_get_admin_model_name(self):
+        models = {
+            'models': [
+                {'name': 'admin', 'model-uuid': 'aaaa'},
+                {'name': 'bar', 'model-uuid': 'bbbb'}],
+            'current-model': 'bar'
+        }
+        client = EnvJujuClient2B7(JujuData('foo'), None, None)
+        with patch.object(client, 'get_models',
+                          return_value=models) as gm_mock:
+            admin_name = client.get_admin_model_name()
+        self.assertEqual(0, gm_mock.call_count)
+        self.assertEqual('admin', admin_name)
+
+    def test_get_admin_model_name_without_admin(self):
+        models = {
+            'models': [
+                {'name': 'bar', 'model-uuid': 'aaaa'},
+                {'name': 'baz', 'model-uuid': 'bbbb'}],
+            'current-model': 'bar'
+        }
+        client = EnvJujuClient2B7(JujuData('foo'), None, None)
+        with patch.object(client, 'get_models', return_value=models):
+            admin_name = client.get_admin_model_name()
+        self.assertEqual('admin', admin_name)
+
+    def test_get_admin_model_name_no_models(self):
+        client = EnvJujuClient2B7(JujuData('foo'), None, None)
+        with patch.object(client, 'get_models', return_value={}):
+            admin_name = client.get_admin_model_name()
+        self.assertEqual('admin', admin_name)
+
+    def test_get_admin_client(self):
+        client = EnvJujuClient2B7(
+            JujuData('foo', {'bar': 'baz'}, 'myhome'), None, None)
+        admin_client = client.get_admin_client()
+        admin_env = admin_client.env
+        self.assertEqual('admin', admin_env.environment)
+        self.assertEqual({'bar': 'baz', 'name': 'admin'}, admin_env.config)
+
+
 class TestEnvJujuClient2B3(ClientTest):
 
     def test_add_model_hypenated_controller(self):
@@ -3272,6 +3322,7 @@
             yield '2.0-beta5'
             yield '2.0-beta6'
             yield '2.0-beta7'
+            yield '2.0-beta8'
             yield '2.0-delta1'
 
         context = patch.object(
@@ -3333,10 +3384,13 @@
             self.assertIs(type(client), EnvJujuClient2B3)
             self.assertEqual(client.version, '2.0-beta6')
             client = EnvJujuClient1X.by_version(None)
-            self.assertIs(type(client), EnvJujuClient)
+            self.assertIs(type(client), EnvJujuClient2B7)
             self.assertEqual(client.version, '2.0-beta7')
             client = EnvJujuClient1X.by_version(None)
             self.assertIs(type(client), EnvJujuClient)
+            self.assertEqual(client.version, '2.0-beta8')
+            client = EnvJujuClient1X.by_version(None)
+            self.assertIs(type(client), EnvJujuClient)
             self.assertEqual(client.version, '2.0-delta1')
             with self.assertRaises(StopIteration):
                 EnvJujuClient1X.by_version(None)