Merge lp:~gnuoy/charms/trusty/nova-cloud-controller/add-console-access into lp:~openstack-charmers-archive/charms/trusty/nova-cloud-controller/next

[Liam Young]

Added support for console access to guest instances using vnc via browser (novnc), vnc via java client (xvpvnc) or spice via browser.

The bug reports suggests this should be baked into the nova-cloud-controller charm which is what I've done but I'm still sitting on the fence as to whether this would be better as a standalone principle charm.

[James Page (james-page)]

I think this is fine as a function of the nova-cc charm; I've made a couple of comments.

One other question - does this work OK when more that one nova-cc service unit is being used? I'm not familiar with how this part of openstack actually works. Right now, users would always be directed to the unit serving the VIP and would not be load balanced - but this might actually be the right behaviour!

review: Needs Fixing

[lp:~gnuoy/charms/trusty/nova-cloud-controller/add-console-access]

95. By Liam Young on 2014-07-29

Remove none as default option for console-access-protocol and tidyup code accordingly

96. By Liam Young on 2014-07-29

Rebased

97. By Liam Young on 2014-07-29

Updated README

98. By Liam Young on 2014-07-30

Set console_settings for compute node if leader or not as leadership is irrelevant and stale settings can persist otherwise

[Liam Young (gnuoy)]

> I think this is fine as a function of the nova-cc charm; I've made a couple of
> comments.
>

I've made the changes you suggested

> One other question - does this work OK when more that one nova-cc service unit
> is being used? I'm not familiar with how this part of openstack actually
> works. Right now, users would always be directed to the unit serving the VIP
> and would not be load balanced - but this might actually be the right
> behaviour!

I think this is behaviour is fine tbh. I've had a play with an ha setup and the client handshake fails. I haven't looked into to much further (maybe sticky haproxy sessions are needed). If you have no objection I'll create a wishlist bug and grab it to look at ha further but I'm not convinced it will work.

review: Needs Resubmitting

[lp:~gnuoy/charms/trusty/nova-cloud-controller/add-console-access]

99. By Liam Young on 2014-07-30

README update, enable ability to explictly set the use of the nova-cc address and use the PUBLIC address

=== modified file 'README.txt'
--- README.txt	2014-06-11 13:54:39 +0000
+++ README.txt	2014-07-30 10:50:10 +0000
@@ -9,6 +9,11 @@
 neutron-api endpoint in keystone. It will also use the quantum-security-groups setting which is passed to
 it by the api service rather than its own quantum-security-groups setting.
 
+If console access is required then console-proxy-ip should be set to a client accessible IP that resolves
+to the nova-cloud-controller. If running in HA mode then the public vip is used if console-proxy-ip is set
+to local. Note: The console access protocol is baked into a guest when it is created, if you change it then
+console access for existing guests will stop working
+
 ******************************************************
 Special considerations to be deployed using Postgresql
 ******************************************************

=== modified file 'config.yaml'
--- config.yaml	2014-07-29 11:50:07 +0000
+++ config.yaml	2014-07-30 10:50:10 +0000
@@ -203,3 +203,20 @@
         * shared-db or (pgsql-nova-db, pgsql-neutron-db)
         * amqp
         * identity-service
+  console-access-protocol:
+    type: string
+    description: |
+       Protocol to use when accessing virtual machine console. Supported types
+       are None, spice, xvpvnc, novnc and vnc (for both xvpvnc and novnc)
+  console-proxy-ip:
+    type: string
+    default: local
+    description: |
+       If console-access-protocol != None then this is the ip published to
+       clients for access to console proxy. Set to local for the ip address of
+       the nova-cloud-controller serving the request to be used
+  console-keymap:
+    type: string
+    default: 'en-us'
+    description: |
+       Console keymap

=== modified file 'hooks/nova_cc_hooks.py'
--- hooks/nova_cc_hooks.py	2014-07-29 12:25:27 +0000
+++ hooks/nova_cc_hooks.py	2014-07-30 10:50:10 +0000
@@ -71,6 +71,7 @@
     QUANTUM_CONF,
     NEUTRON_CONF,
     QUANTUM_API_PASTE,
+    console_attributes,
     service_guard,
     guard_map,
 )
@@ -124,6 +125,11 @@
     save_script_rc()
     configure_https()
     CONFIGS.write_all()
+    if console_attributes('protocol'):
+        apt_update()
+        apt_install(console_attributes('packages'), fatal=True)
+        [compute_joined(rid=rid)
+            for rid in relation_ids('cloud-compute')]
     for r_id in relation_ids('identity-service'):
         identity_joined(rid=r_id)
 
@@ -377,8 +383,38 @@
     return rel_settings
 
 
+def console_settings():
+    rel_settings = {}
+    proto = console_attributes('protocol')
+    if not proto:
+        return {}
+    rel_settings['console_keymap'] = config('console-keymap')
+    rel_settings['console_access_protocol'] = proto
+    if config('console-proxy-ip') == 'local':
+        proxy_base_addr = canonical_url(CONFIGS, PUBLIC)
+    else:
+        proxy_base_addr = "http://" + config('console-proxy-ip')
+    if proto == 'vnc':
+        protocols = ['novnc', 'xvpvnc']
+    else:
+        protocols = [proto]
+    for _proto in protocols:
+        rel_settings['console_proxy_%s_address' % (_proto)] = \
+            "%s:%s%s" % (proxy_base_addr,
+                         console_attributes('proxy-port', proto=_proto),
+                         console_attributes('proxy-page', proto=_proto))
+        rel_settings['console_proxy_%s_host' % (_proto)] = \
+            urlparse(proxy_base_addr).hostname
+        rel_settings['console_proxy_%s_port' % (_proto)] = \
+            console_attributes('proxy-port', proto=_proto)
+
+    return rel_settings
+
+
 @hooks.hook('cloud-compute-relation-joined')
 def compute_joined(rid=None, remote_restart=False):
+    cons_settings = console_settings()
+    relation_set(relation_id=rid, **cons_settings)
     if not eligible_leader(CLUSTER_RES):
         return
     rel_settings = {

=== modified file 'hooks/nova_cc_utils.py'
--- hooks/nova_cc_utils.py	2014-07-29 13:32:55 +0000
+++ hooks/nova_cc_utils.py	2014-07-30 10:50:10 +0000
@@ -173,6 +173,27 @@
 
 NOVA_SSH_DIR = '/etc/nova/compute_ssh/'
 
+CONSOLE_CONFIG = {
+    'spice': {
+        'packages': ['nova-spiceproxy', 'nova-consoleauth'],
+        'services': ['nova-spiceproxy', 'nova-consoleauth'],
+        'proxy-page': '/spice_auto.html',
+        'proxy-port': 6082,
+    },
+    'novnc': {
+        'packages': ['nova-novncproxy', 'nova-consoleauth'],
+        'services': ['nova-novncproxy', 'nova-consoleauth'],
+        'proxy-page': '/vnc_auto.html',
+        'proxy-port': 6080,
+    },
+    'xvpvnc': {
+        'packages': ['nova-xvpvncproxy', 'nova-consoleauth'],
+        'services': ['nova-xvpvncproxy', 'nova-consoleauth'],
+        'proxy-page': '/console',
+        'proxy-port': 6081,
+    },
+}
+
 
 def resource_map():
     '''
@@ -235,6 +256,10 @@
     if os_release('nova-common') not in ['essex', 'folsom']:
         resource_map['/etc/nova/nova.conf']['services'] += ['nova-conductor']
 
+    if console_attributes('services'):
+        resource_map['/etc/nova/nova.conf']['services'] += \
+            console_attributes('services')
+
     # also manage any configs that are being updated by subordinates.
     vmware_ctxt = context.SubordinateConfigContext(interface='nova-vmware',
                                                    service='nova',
@@ -287,6 +312,27 @@
     return API_PORTS[service]
 
 
+def console_attributes(attr, proto=None):
+    '''Leave proto unset to query attributes of the protocal specified at
+    runtime'''
+    if proto:
+        console_proto = proto
+    else:
+        console_proto = config('console-access-protocol')
+    if attr == 'protocol':
+        return console_proto
+    # 'vnc' is a virtual type made up of novnc and xvpvnc
+    if console_proto == 'vnc':
+        if attr in ['packages', 'services']:
+            return list(set(CONSOLE_CONFIG['novnc'][attr] +
+                        CONSOLE_CONFIG['xvpvnc'][attr]))
+        else:
+            return None
+    if console_proto in CONSOLE_CONFIG:
+        return CONSOLE_CONFIG[console_proto][attr]
+    return None
+
+
 def determine_packages():
     # currently all packages match service names
     packages = [] + BASE_PACKAGES
@@ -296,6 +342,8 @@
         pkgs = neutron_plugin_attribute(neutron_plugin(), 'server_packages',
                                         network_manager())
         packages.extend(pkgs)
+    if console_attributes('packages'):
+        packages.extend(console_attributes('packages'))
     return list(set(packages))
 
 

=== modified file 'unit_tests/test_nova_cc_hooks.py'
--- unit_tests/test_nova_cc_hooks.py	2014-07-29 12:23:46 +0000
+++ unit_tests/test_nova_cc_hooks.py	2014-07-30 10:50:10 +0000
@@ -159,8 +159,10 @@
         self.assertEquals(sorted(self.relation_set.call_args_list),
                           sorted(expected_relations))
 
+    @patch.object(utils, 'config')
     @patch.object(hooks, '_auth_config')
-    def test_compute_joined_neutron(self, auth_config):
+    def test_compute_joined_neutron(self, auth_config, _util_config):
+        _util_config.return_value = None
         self.is_relation_made.return_value = False
         self.network_manager.return_value = 'neutron'
         self.eligible_leader = True
@@ -186,15 +188,18 @@
             quantum_plugin='nvp',
             network_manager='neutron', **FAKE_KS_AUTH_CFG)
 
+    @patch.object(utils, 'config')
     @patch.object(hooks, 'NeutronAPIContext')
     @patch.object(hooks, '_auth_config')
-    def test_compute_joined_neutron_api_rel(self, auth_config, napi):
+    def test_compute_joined_neutron_api_rel(self, auth_config, napi,
+                                            _util_config):
         def mock_NeutronAPIContext():
             return {
                 'neutron_plugin': 'bob',
                 'neutron_security_groups': 'yes',
                 'neutron_url': 'http://nova-cc-host1:9696',
             }
+        _util_config.return_value = None
         napi.return_value = mock_NeutronAPIContext
         self.is_relation_made.return_value = True
         self.network_manager.return_value = 'neutron'
@@ -365,3 +370,89 @@
         self.assertTrue(configs.write_all.called)
         self.assertTrue(_compute_joined.called)
         self.assertTrue(_quantum_joined.called)
+
+    @patch.object(utils, 'config')
+    def test_console_settings_vnc(self, _utils_config):
+        _utils_config.return_value = 'vnc'
+        _cc_host = "nova-cc-host1"
+        self.canonical_url.return_value = 'http://' + _cc_host
+        _con_sets = hooks.console_settings()
+        console_settings = {
+            'console_proxy_novnc_address': 'http://%s:6080/vnc_auto.html' %
+                                           (_cc_host),
+            'console_proxy_novnc_port': 6080,
+            'console_access_protocol': 'vnc',
+            'console_proxy_novnc_host': _cc_host,
+            'console_proxy_xvpvnc_port': 6081,
+            'console_proxy_xvpvnc_host': _cc_host,
+            'console_proxy_xvpvnc_address': 'http://%s:6081/console' %
+                                            (_cc_host),
+            'console_keymap': 'en-us'
+        }
+        self.assertEqual(_con_sets, console_settings)
+
+    @patch.object(utils, 'config')
+    def test_console_settings_xvpvnc(self, _utils_config):
+        _utils_config.return_value = 'xvpvnc'
+        _cc_host = "nova-cc-host1"
+        self.canonical_url.return_value = 'http://' + _cc_host
+        _con_sets = hooks.console_settings()
+        console_settings = {
+            'console_access_protocol': 'xvpvnc',
+            'console_keymap': 'en-us',
+            'console_proxy_xvpvnc_port': 6081,
+            'console_proxy_xvpvnc_host': _cc_host,
+            'console_proxy_xvpvnc_address': 'http://%s:6081/console' %
+                                            (_cc_host),
+        }
+        self.assertEqual(_con_sets, console_settings)
+
+    @patch.object(utils, 'config')
+    def test_console_settings_novnc(self, _utils_config):
+        _utils_config.return_value = 'novnc'
+        _cc_host = "nova-cc-host1"
+        self.canonical_url.return_value = 'http://' + _cc_host
+        _con_sets = hooks.console_settings()
+        console_settings = {
+            'console_proxy_novnc_address': 'http://%s:6080/vnc_auto.html' %
+                                           (_cc_host),
+            'console_proxy_novnc_port': 6080,
+            'console_access_protocol': 'novnc',
+            'console_proxy_novnc_host': _cc_host,
+            'console_keymap': 'en-us'
+        }
+        self.assertEqual(_con_sets, console_settings)
+
+    @patch.object(utils, 'config')
+    def test_console_settings_spice(self, _utils_config):
+        _utils_config.return_value = 'spice'
+        _cc_host = "nova-cc-host1"
+        self.canonical_url.return_value = 'http://' + _cc_host
+        _con_sets = hooks.console_settings()
+        console_settings = {
+            'console_proxy_spice_address': 'http://%s:6082/spice_auto.html' %
+                                           (_cc_host),
+            'console_proxy_spice_host': _cc_host,
+            'console_proxy_spice_port': 6082,
+            'console_access_protocol': 'spice',
+            'console_keymap': 'en-us'
+        }
+        self.assertEqual(_con_sets, console_settings)
+
+    @patch.object(utils, 'config')
+    def test_console_settings_explicit_ip(self, _utils_config):
+        _utils_config.return_value = 'spice'
+        _cc_public_host = "public-host"
+        _cc_private_host = "private-host"
+        self.test_config.set('console-proxy-ip', _cc_public_host)
+        _con_sets = hooks.console_settings()
+        self.canonical_url.return_value = 'http://' + _cc_private_host
+        console_settings = {
+            'console_proxy_spice_address': 'http://%s:6082/spice_auto.html' %
+                                           (_cc_public_host),
+            'console_proxy_spice_host': _cc_public_host,
+            'console_proxy_spice_port': 6082,
+            'console_access_protocol': 'spice',
+            'console_keymap': 'en-us'
+        }
+        self.assertEqual(_con_sets, console_settings)

=== modified file 'unit_tests/test_nova_cc_utils.py'
--- unit_tests/test_nova_cc_utils.py	2014-07-29 13:44:34 +0000
+++ unit_tests/test_nova_cc_utils.py	2014-07-30 10:50:10 +0000
@@ -214,6 +214,48 @@
         self.assertIn('nova-api-os-volume',
                       _map['/etc/nova/nova.conf']['services'])
 
+    @patch('charmhelpers.contrib.openstack.context.SubordinateConfigContext')
+    def test_resource_map_console_xvpvnc(self, subcontext):
+        self.test_config.set('console-access-protocol', 'xvpvnc')
+        self.relation_ids.return_value = []
+        _map = utils.resource_map()
+        console_services = ['nova-xvpvncproxy', 'nova-consoleauth']
+        for service in console_services:
+            self.assertIn(service, _map['/etc/nova/nova.conf']['services'])
+
+    @patch('charmhelpers.contrib.openstack.context.SubordinateConfigContext')
+    def test_resource_map_console_novnc(self, subcontext):
+        self.test_config.set('console-access-protocol', 'novnc')
+        self.relation_ids.return_value = []
+        _map = utils.resource_map()
+        console_services = ['nova-novncproxy', 'nova-consoleauth']
+        for service in console_services:
+            self.assertIn(service, _map['/etc/nova/nova.conf']['services'])
+
+    @patch('charmhelpers.contrib.openstack.context.SubordinateConfigContext')
+    def test_resource_map_console_vnc(self, subcontext):
+        self.test_config.set('console-access-protocol', 'vnc')
+        self.relation_ids.return_value = []
+        _map = utils.resource_map()
+        console_services = ['nova-novncproxy', 'nova-xvpvncproxy',
+                            'nova-consoleauth']
+        for service in console_services:
+            self.assertIn(service, _map['/etc/nova/nova.conf']['services'])
+
+    def test_console_attributes_none(self):
+        self.test_config.set('console-access-protocol', None)
+        _proto = utils.console_attributes('protocol')
+        self.assertEquals(_proto, None)
+
+    @patch('charmhelpers.contrib.openstack.context.SubordinateConfigContext')
+    def test_resource_map_console_spice(self, subcontext):
+        self.test_config.set('console-access-protocol', 'spice')
+        self.relation_ids.return_value = []
+        _map = utils.resource_map()
+        console_services = ['nova-spiceproxy', 'nova-consoleauth']
+        for service in console_services:
+            self.assertIn(service, _map['/etc/nova/nova.conf']['services'])
+
     @patch('os.path.exists')
     @patch('charmhelpers.contrib.openstack.context.SubordinateConfigContext')
     def test_restart_map_api_before_frontends(self, subcontext, _exists):
@@ -235,6 +277,23 @@
         self.assertTrue('/etc/apache2/sites-available/'
                         'openstack_https_frontend' not in _map)
 
+    def test_console_attributes_spice(self):
+        _proto = utils.console_attributes('protocol', proto='spice')
+        self.assertEquals(_proto, 'spice')
+
+    def test_console_attributes_vnc(self):
+        self.test_config.set('console-access-protocol', 'vnc')
+        _proto = utils.console_attributes('protocol')
+        _servs = utils.console_attributes('services')
+        _pkgs = utils.console_attributes('packages')
+        _proxy_page = utils.console_attributes('proxy-page')
+        vnc_pkgs = ['nova-novncproxy', 'nova-xvpvncproxy', 'nova-consoleauth']
+        vnc_servs = ['nova-novncproxy', 'nova-xvpvncproxy', 'nova-consoleauth']
+        self.assertEquals(_proto, 'vnc')
+        self.assertEquals(_servs, vnc_servs)
+        self.assertEquals(_pkgs, vnc_pkgs)
+        self.assertEquals(_proxy_page, None)
+
     @patch('charmhelpers.contrib.openstack.context.SubordinateConfigContext')
     def test_determine_packages_quantum(self, subcontext):
         self._resource_map(network_manager='quantum')
@@ -255,6 +314,15 @@
         self.assertIn('nova-api-os-volume', pkgs)
 
     @patch('charmhelpers.contrib.openstack.context.SubordinateConfigContext')
+    def test_determine_packages_console(self, subcontext):
+        self.test_config.set('console-access-protocol', 'spice')
+        self.relation_ids.return_value = []
+        pkgs = utils.determine_packages()
+        console_pkgs = ['nova-spiceproxy', 'nova-consoleauth']
+        for console_pkg in console_pkgs:
+            self.assertIn(console_pkg, pkgs)
+
+    @patch('charmhelpers.contrib.openstack.context.SubordinateConfigContext')
     def test_determine_packages_base(self, subcontext):
         self.relation_ids.return_value = []
         self.os_release.return_value = 'folsom'