Merge lp:~james-page/charms/precise/nova-compute/resize-fixes into lp:~charmers/charms/precise/nova-compute/trunk

[James Page]

Add feature to support resizing of instances; this required SSH access for the nova user between compute hosts.

[Marco Ceppi (marcoceppi)]

Deferring to openstack-charmers

review: Abstain

[Adam Gandelman (gandelman-a)]

review: Approve

[Jonathan Davies (jpds)]

+1 I need this for instance migration.

review: Approve

=== modified file 'config.yaml'
--- config.yaml	2013-10-17 19:25:15 +0000
+++ config.yaml	2013-12-17 11:22:05 +0000
@@ -50,6 +50,11 @@
     default: "yes"
     type: string
     description: Whether to run nova-api and nova-network on the compute nodes.
+  enable-resize:
+    default: False
+    type: boolean
+    description: Enable instance resizing, which requires that passwordless SSH
+      access be setup between compute hosts.
   enable-live-migration:
     default: False
     type: boolean

=== modified file 'hooks/nova_compute_hooks.py'
--- hooks/nova_compute_hooks.py	2013-11-08 05:48:58 +0000
+++ hooks/nova_compute_hooks.py	2013-12-17 11:22:05 +0000
@@ -48,7 +48,8 @@
     register_configs,
     NOVA_CONF,
     QUANTUM_CONF, NEUTRON_CONF,
-    ceph_config_file, CEPH_SECRET
+    ceph_config_file, CEPH_SECRET,
+    enable_shell, disable_shell
 )
 
 from nova_compute_context import CEPH_SECRET_UUID
@@ -75,7 +76,14 @@
         # Check-in with nova-c-c and register new ssh key, if it has just been
         # generated.
         initialize_ssh_keys()
-        [compute_joined(rid) for rid in relation_ids('cloud-compute')]
+
+    if config('enable-resize') is True:
+        enable_shell(user='nova')
+        initialize_ssh_keys(user='nova')
+    else:
+        disable_shell(user='nova')
+
+    [compute_joined(rid) for rid in relation_ids('cloud-compute')]
 
     CONFIGS.write_all()
 
@@ -140,15 +148,19 @@
 
 @hooks.hook('cloud-compute-relation-joined')
 def compute_joined(rid=None):
-    if not migration_enabled():
-        return
-    auth_type = config('migration-auth-type')
-    settings = {
-        'migration_auth_type': auth_type
-    }
-    if auth_type == 'ssh':
-        settings['ssh_public_key'] = public_ssh_key()
-    relation_set(relation_id=rid, **settings)
+    if migration_enabled():
+        auth_type = config('migration-auth-type')
+        settings = {
+            'migration_auth_type': auth_type
+        }
+        if auth_type == 'ssh':
+            settings['ssh_public_key'] = public_ssh_key()
+        relation_set(relation_id=rid, **settings)
+    if config('enable-resize'):
+        settings = {
+            'nova_ssh_public_key': public_ssh_key(user='nova')
+        }
+        relation_set(relation_id=rid, **settings)
 
 
 @hooks.hook('cloud-compute-relation-changed')
@@ -158,6 +170,7 @@
     # config advertised from controller.
     CONFIGS.write_all()
     import_authorized_keys()
+    import_authorized_keys(user='nova', prefix='nova')
     import_keystone_ca_cert()
     if (network_manager() in ['quantum', 'neutron']
             and neutron_plugin() == 'ovs'):

=== modified file 'hooks/nova_compute_utils.py'
--- hooks/nova_compute_utils.py	2013-11-01 16:11:43 +0000
+++ hooks/nova_compute_utils.py	2013-12-17 11:22:05 +0000
@@ -310,13 +310,18 @@
     check_output(['chown', '-R', user, ssh_dir])
 
 
-def import_authorized_keys(user='root'):
+def import_authorized_keys(user='root', prefix=None):
     """Import SSH authorized_keys + known_hosts from a cloud-compute relation
     and store in user's $HOME/.ssh.
     """
-    # XXX: Should this be managed via templates + contexts?
-    hosts = relation_get('known_hosts')
-    auth_keys = relation_get('authorized_keys')
+    if prefix:
+        hosts = relation_get('{}_known_hosts'.format(prefix))
+        auth_keys = relation_get('{}_authorized_keys'.format(prefix))
+    else:
+        # XXX: Should this be managed via templates + contexts?
+        hosts = relation_get('known_hosts')
+        auth_keys = relation_get('authorized_keys')
+
     # XXX: Need to fix charm-helpers to return None for empty settings,
     #      in all cases.
     if not hosts or not auth_keys:
@@ -394,3 +399,13 @@
     cmd = ['virsh', 'secret-set-value', '--secret', secret_uuid,
            '--base64', key]
     check_call(cmd)
+
+
+def enable_shell(user):
+    cmd = ['usermod', '-s', '/bin/bash', user]
+    check_call(cmd)
+
+
+def disable_shell(user):
+    cmd = ['usermod', '-s', '/bin/false', user]
+    check_call(cmd)

=== modified file 'unit_tests/test_nova_compute_hooks.py'
--- unit_tests/test_nova_compute_hooks.py	2013-11-17 21:47:29 +0000
+++ unit_tests/test_nova_compute_hooks.py	2013-12-17 11:22:05 +0000
@@ -48,6 +48,8 @@
     'neutron_plugin',
     'public_ssh_key',
     'register_configs',
+    'disable_shell',
+    'enable_shell',
     # misc_utils
     'ensure_ceph_keyring',
     'execd_preinstall'
@@ -95,6 +97,38 @@
             call('cloud-compute:1'),
         ]
         self.assertEquals(ex, compute_joined.call_args_list)
+        self.assertTrue(self.initialize_ssh_keys.called)
+
+    @patch.object(hooks, 'compute_joined')
+    def test_config_changed_with_resize(self, compute_joined):
+        self.test_config.set('enable-resize', True)
+        self.relation_ids.return_value = [
+            'cloud-compute:0',
+            'cloud-compute:1'
+        ]
+        hooks.config_changed()
+        ex = [
+            call('cloud-compute:0'),
+            call('cloud-compute:1'),
+        ]
+        self.assertEquals(ex, compute_joined.call_args_list)
+        self.initialize_ssh_keys.assert_called_with(user='nova')
+        self.enable_shell.assert_called_with(user='nova')
+
+    @patch.object(hooks, 'compute_joined')
+    def test_config_changed_without_resize(self, compute_joined):
+        self.test_config.set('enable-resize', False)
+        self.relation_ids.return_value = [
+            'cloud-compute:0',
+            'cloud-compute:1'
+        ]
+        hooks.config_changed()
+        ex = [
+            call('cloud-compute:0'),
+            call('cloud-compute:1'),
+        ]
+        self.assertEquals(ex, compute_joined.call_args_list)
+        self.disable_shell.assert_called_with(user='nova')
 
     @patch.object(hooks, 'compute_joined')
     def test_config_changed_no_upgrade_no_migration(self, compute_joined):
@@ -239,7 +273,7 @@
         hooks.image_service_changed()
         configs.write.assert_called_with('/etc/nova/nova.conf')
 
-    def test_compute_joined_no_migration(self):
+    def test_compute_joined_no_migration_no_resize(self):
         self.migration_enabled.return_value = False
         hooks.compute_joined()
         self.assertFalse(self.relation_set.called)
@@ -261,14 +295,27 @@
             migration_auth_type='ssh'
         )
 
+    def test_compute_joined_with_resize(self):
+        self.test_config.set('enable-resize', True)
+        self.public_ssh_key.return_value = 'bar'
+        hooks.compute_joined()
+        self.relation_set.assert_called_with(
+            relation_id=None,
+            nova_ssh_public_key='bar'
+        )
+        hooks.compute_joined(rid='cloud-compute:2')
+        self.relation_set.assert_called_with(
+            relation_id='cloud-compute:2',
+            nova_ssh_public_key='bar'
+        )
+
     def test_compute_changed(self):
         hooks.compute_changed()
-        expected_funcs = [
-            self.import_authorized_keys,
-            self.import_keystone_ca_cert,
-        ]
-        for func in expected_funcs:
-            self.assertTrue(func.called)
+        self.assertTrue(self.import_keystone_ca_cert.called)
+        self.import_authorized_keys.assert_has_calls([
+            call(),
+            call(user='nova', prefix='nova'),
+        ])
 
     def test_ceph_joined(self):
         hooks.ceph_joined()

=== modified file 'unit_tests/test_nova_compute_utils.py'
--- unit_tests/test_nova_compute_utils.py	2013-11-17 21:47:29 +0000
+++ unit_tests/test_nova_compute_utils.py	2013-12-17 11:22:05 +0000
@@ -179,7 +179,7 @@
             self.assertFalse(_open.called)
 
     @patch('pwd.getpwnam')
-    def test_import_authorized_keys(self, getpwnam):
+    def _test_import_authorized_keys_base(self, getpwnam, prefix=None):
         getpwnam.return_value = self.fake_user('foo')
         self.relation_get.side_effect = [
             'Zm9vX2tleQo=',  # relation_get('known_hosts')
@@ -200,6 +200,38 @@
             self.assertEquals(ex_open, _open.call_args_list)
             self.assertEquals(ex_write, _file.write.call_args_list)
 
+        self.relation_get.assert_has_called([
+            call('known_hosts').
+            call('authorized_keys')
+        ])
+
+    @patch('pwd.getpwnam')
+    def test_import_authorized_keys_prefix(self, getpwnam):
+        getpwnam.return_value = self.fake_user('foo')
+        self.relation_get.side_effect = [
+            'Zm9vX2tleQo=',  # relation_get('known_hosts')
+            'Zm9vX2hvc3QK',  # relation_get('authorized_keys')
+        ]
+
+        ex_open = [
+            call('/home/foo/.ssh/authorized_keys', 'wb'),
+            call('/home/foo/.ssh/known_hosts', 'wb')
+        ]
+        ex_write = [
+            call('foo_host\n'),
+            call('foo_key\n'),
+        ]
+
+        with patch_open() as (_open, _file):
+            utils.import_authorized_keys(user='foo', prefix='bar')
+            self.assertEquals(ex_open, _open.call_args_list)
+            self.assertEquals(ex_write, _file.write.call_args_list)
+
+        self.relation_get.assert_has_called([
+            call('bar_known_hosts').
+            call('bar_authorized_keys')
+        ])
+
     @patch('subprocess.check_call')
     def test_import_keystone_cert_missing_data(self, check_call):
         self.relation_get.return_value = None
@@ -247,3 +279,14 @@
             call('/etc/nova/nova.conf', [ctxt1])
         ]
         self.assertEquals(fake_renderer.register.call_args_list, ex_reg)
+
+    @patch.object(utils, 'check_call')
+    def test_enable_shell(self, _check_call):
+        utils.enable_shell('dummy')
+        _check_call.assert_called_with(['usermod', '-s', '/bin/bash', 'dummy'])
+
+    @patch.object(utils, 'check_call')
+    def test_disable_shell(self, _check_call):
+        utils.disable_shell('dummy')
+        _check_call.assert_called_with(['usermod', '-s', '/bin/false',
+                                        'dummy'])