Merge lp:~freyes/charms/trusty/nova-cloud-controller/single-nova-consoleauth into lp:~openstack-charmers-archive/charms/trusty/nova-cloud-controller/next

[Felipe Reyes]

Dear OpenStack Charmers,

This patch configures a new pacemaker resource that will allow to run a single instance of nova-consoleauth service when console-access-protocol is defined.

Specifically, nova-consoleauth is configured to run where the vip is allocated, with this patch this will be default behaviour. This is needed for environments where the memcached cannot (or doesn't want to) be related to nova-cloud-controller to share the authorization tokens across all the running instances.

Example output of 'sudo crm_mon -1' with this patch applied:

Last updated: Mon Mar 2 14:46:43 2015
Last change: Mon Mar 2 14:44:53 2015 via crm_resource on juju-freyes-machine-18
Stack: corosync
Current DC: juju-freyes-machine-18 (168103083) - partition with quorum
Version: 1.1.10-42f2063
3 Nodes configured
5 Resources configured

Online: [ juju-freyes-machine-16 juju-freyes-machine-17 juju-freyes-machine-18 ]

 Resource Group: grp_nova_vips
     res_nova_eth0_vip (ocf::heartbeat:IPaddr2): Started juju-freyes-machine-16
 Clone Set: cl_nova_haproxy [res_nova_haproxy]
     Started: [ juju-freyes-machine-16 juju-freyes-machine-17 juju-freyes-machine-18 ]
 res_nova_consoleauth (upstart:nova-consoleauth): Started juju-freyes-machine-16

$ python juju_rrun.py --service nova-cloud-controller 'sudo ps aux|grep nova-consoleauth'
---------- 16 nova-cloud-controller/0
---------- stdout ----------
nova 20773 1.2 7.0 312604 71312 ? Ss 14:44 0:02 /usr/bin/python /usr/bin/nova-consoleauth --config-file=/etc/nova/nova.conf
ubuntu 21188 0.0 0.6 134316 6836 ? Sl 14:47 0:00 juju-run nova-cloud-controller/0 sudo ps aux|grep nova-consoleauth
root 21195 0.0 0.0 8860 648 ? S 14:47 0:00 grep nova-consoleauth

---------- 17 nova-cloud-controller/1
---------- stdout ----------
ubuntu 20997 0.0 0.6 134316 6840 ? Sl 14:47 0:00 juju-run nova-cloud-controller/1 sudo ps aux|grep nova-consoleauth
root 21004 0.0 0.0 8860 648 ? S 14:47 0:00 grep nova-consoleauth

---------- 18 nova-cloud-controller/2
---------- stdout ----------
ubuntu 21325 0.0 0.8 134316 8888 ? Sl 14:47 0:00 juju-run nova-cloud-controller/2 sudo ps aux|grep nova-consoleauth
root 21332 0.0 0.0 8860 644 ? S 14:47 0:00 grep nova-consoleauth

Thanks,

[uosci-testing-bot (uosci-testing-bot)]

charm_lint_check #2119 nova-cloud-controller-next for freyes mp250339
    LINT OK: passed

Build: http://10.245.162.77:8080/job/charm_lint_check/2119/

[uosci-testing-bot (uosci-testing-bot)]

charm_unit_test #1909 nova-cloud-controller-next for freyes mp250339
    UNIT OK: passed

Build: http://10.245.162.77:8080/job/charm_unit_test/1909/

[uosci-testing-bot (uosci-testing-bot)]

charm_amulet_test #2028 nova-cloud-controller-next for freyes mp250339
    AMULET OK: passed

Build: http://10.245.162.77:8080/job/charm_amulet_test/2028/

[Billy Olsen (billy-olsen)]

Felipe,

Thanks for the submission. I haven't deployed this yet to try it out, but my concerns are around the upstart resources. Since we'll have a difference between systemd and upstart between vivid and previous releases I think we should be using an ocf file instead. There's already one that exists in https://github.com/madkiss/openstack-resource-agents/tree/master/ocf. Looks like its been awhile since its been touched so its either stable or not supported anymore. I think the openstack-resource-agents package also includes this, but I'm not sure as to its status (I believe its in universe).

An amulet test is always nice :-) We don't have anything in the way of the hacluster relation in our amulet tests at this point so I don't think its strictly a requirement (e.g. I wouldn't hold up the mp for it), but improvements are always welcomed.

review: Needs Fixing

[uosci-testing-bot (uosci-testing-bot)]

charm_lint_check #2392 nova-cloud-controller-next for freyes mp251456
    LINT FAIL: lint-test failed

LINT Results (max last 2 lines):
  unit_tests/test_nova_cc_hooks.py:727:51: E502 the backslash is redundant between brackets
  make: *** [lint] Error 1

Full lint test output: http://paste.ubuntu.com/10520210/
Build: http://10.245.162.77:8080/job/charm_lint_check/2392/

[uosci-testing-bot (uosci-testing-bot)]

charm_unit_test #2182 nova-cloud-controller-next for freyes mp251456
    UNIT OK: passed

Build: http://10.245.162.77:8080/job/charm_unit_test/2182/

[uosci-testing-bot (uosci-testing-bot)]

charm_amulet_test #2312 nova-cloud-controller-next for freyes mp251456
    AMULET OK: passed

Build: http://10.245.162.77:8080/job/charm_amulet_test/2312/

[Billy Olsen (billy-olsen)]

Felipe, thanks for the submission! Things look good. Approved.

review: Approve

[Billy Olsen (billy-olsen)]

Okay I spoke too soon. Everything looks good for enabling the single-nova-consoleauth and it works as expected while its running. However, if a user wishes to change from single-nova-consoleauth back to multi consoleauth then this patch doesn't actually:

1. remove the res_nova_consoleauth from the corosync/pacemaker configuration.
2. restart the services on nodes which the service is not running restoring the multi-consoleauth scenario.

Apologies for not chasing this scenario down sooner.

review: Needs Fixing

[uosci-testing-bot (uosci-testing-bot)]

charm_lint_check #2493 nova-cloud-controller-next for freyes mp252138
    LINT OK: passed

Build: http://10.245.162.77:8080/job/charm_lint_check/2493/

[uosci-testing-bot (uosci-testing-bot)]

charm_unit_test #2283 nova-cloud-controller-next for freyes mp252138
    UNIT OK: passed

Build: http://10.245.162.77:8080/job/charm_unit_test/2283/

[uosci-testing-bot (uosci-testing-bot)]

charm_amulet_test #2365 nova-cloud-controller-next for freyes mp252138
    AMULET FAIL: amulet-test failed

AMULET Results (max last 2 lines):
  ERROR subprocess encountered error code 1
  make: *** [test] Error 1

Full amulet test output: http://paste.ubuntu.com/10553839/
Build: http://10.245.162.77:8080/job/charm_amulet_test/2365/

[Felipe Reyes (freyes)]

The amulet failure is:

  juju-test.conductor.14-basic-precise-icehouse DEBUG : Running 14-basic-precise-icehouse (tests/14-basic-precise-icehouse)
  2015-03-07 01:22:16 Starting deployment of osci-sv00
  2015-03-07 01:22:49 Deploying services...
[...]
  2015-03-07 01:23:13 Deploying service rabbitmq-server using local:precise/rabbitmq-server
  2015-03-07 01:34:08 The following units had errors:
  unit: rabbitmq-server/0: machine: 7 agent-state: error details: hook failed: "config-changed"

This isn't related to my change

[Felipe Reyes (freyes)]

> 1. remove the res_nova_consoleauth from the corosync/pacemaker configuration.
> 2. restart the services on nodes which the service is not running restoring
> the multi-consoleauth scenario.

Makes sense, I'll update the patch.

Thanks,

[Felipe Reyes (freyes)]

@Billy, I pushed a new version of the patch to address your feedback, this can't be merged until bug 1433377 is fixed, but I'll appreciate if you could take a quick review to know if things look good.

[uosci-testing-bot (uosci-testing-bot)]

charm_lint_check #2747 nova-cloud-controller-next for freyes mp253309
    LINT OK: passed

Build: http://10.245.162.77:8080/job/charm_lint_check/2747/

[uosci-testing-bot (uosci-testing-bot)]

charm_unit_test #2538 nova-cloud-controller-next for freyes mp253309
    UNIT OK: passed

Build: http://10.245.162.77:8080/job/charm_unit_test/2538/

[uosci-testing-bot (uosci-testing-bot)]

charm_amulet_test #2586 nova-cloud-controller-next for freyes mp253309
    AMULET FAIL: amulet-test failed

AMULET Results (max last 2 lines):
  ERROR subprocess encountered error code 1
  make: *** [test] Error 1

Full amulet test output: http://paste.ubuntu.com/10630161/
Build: http://10.245.162.77:8080/job/charm_amulet_test/2586/

[Felipe Reyes (freyes)]

A couple of deployer configurations for testing this change were pushed to lp:~openstack-charm-testers/+junk/percona-mysql-agent

One of them is a complete openstack cloud (based in next.yaml from openstack-charm-testing repo) and the other one is a simple 3 node percona cluster + keystone.

[Edward Hope-Morley (hopem)]

Felipe, i'm deploying this as we speak. The code looks fine although I have a few comments about how it could be slightly neater, see inline. One approach might be so pull the relation_settings, travers the settings dict and apply any default/missing entries then process the results.

review: Needs Fixing

[Felipe Reyes (freyes)]

Ed, the bundle to test this MP is located at https://code.launchpad.net/~openstack-charm-testers/+junk/single-nova-consoleauth , sorry for the confusion.

[Felipe Reyes (freyes)]

Ed, I'll push another MP that addresses your feedback, except one of them, please see my reply.

[Edward Hope-Morley (hopem)]

So i should have noticed before but, your code is living in config_changed() but relies on the ha relation to exist so it will never get called unless the ha relation exists prior to config_changed() being run...which will not be the case unless you modify charm config after the hacluster has completed installation and relations have been added. So, i suggest you move that code into the ha_joined() or ha_changed() hooks and call it from config_changed() assuming that is safe to do.

review: Needs Fixing

[lp:~freyes/charms/trusty/nova-cloud-controller/single-nova-consoleauth]

154. By Felipe Reyes on 2015-05-18

Commit after merge

155. By Felipe Reyes on 2015-05-22

Move code into its own function and it's called from config-changed, ha-relation-changed and upgrade-charm

156. By Felipe Reyes on 2015-05-22

fix lint warning

157. By Felipe Reyes on 2015-05-22

Commit after merge

158. By Felipe Reyes on 2015-05-22

nova-consoleauth set to manual only if there is a ha relation

Unmerged revisions

=== modified file 'config.yaml'
--- config.yaml	2015-02-19 04:30:13 +0000
+++ config.yaml	2015-03-25 23:24:02 +0000
@@ -326,3 +326,14 @@
     description: |
       A comma-separated list of nagios servicegroups.
       If left empty, the nagios_context will be used as the servicegroup
+  single-nova-consoleauth:
+    type: boolean
+    default: true
+    description: |
+      When this configuration is set to True, a single instance of
+      nova-consoleauth service will be running, this allows users to always
+      authenticate against the same instance and avoid authentications issues
+      when the token used was stored in a different instance.
+      
+      If memcached is being used to store the tokens, then it's recommended to
+      change this configuration to False.

=== modified file 'hooks/nova_cc_hooks.py'
--- hooks/nova_cc_hooks.py	2015-02-19 04:29:22 +0000
+++ hooks/nova_cc_hooks.py	2015-03-25 23:24:02 +0000
@@ -120,6 +120,9 @@
 
 hooks = Hooks()
 CONFIGS = register_configs()
+COLO_CONSOLEAUTH = 'inf: res_nova_consoleauth grp_nova_vips'
+AGENT_CONSOLEAUTH = 'ocf:openstack:nova-consoleauth'
+AGENT_CA_PARAMS = 'op monitor interval="5s"'
 
 
 @hooks.hook()
@@ -171,6 +174,52 @@
     [cluster_joined(rid) for rid in relation_ids('cluster')]
     update_nrpe_config()
 
+    relids = relation_ids('ha')
+    data = {}
+    if len(relids) != 1:
+        log('Related to {} ha services'.format(len(relids)), level='DEBUG')
+        ha_relid = None
+    else:
+        ha_relid = relids[0]
+        data = relation_get(rid=ha_relid) or {}
+
+    for key in ['delete_resources', 'colocations', 'init_services',
+                'resources', 'resource_params']:
+        if key not in data:
+            if key != 'delete_resources':
+                data[key] = {}
+            else:
+                data[key] = []
+
+    if config('single-nova-consoleauth') and console_attributes('protocol'):
+        for item in ['vip_consoleauth', 'res_nova_consoleauth']:
+            if item in data['delete_resources']:
+                data['delete_resources'].remove(item)
+
+        # the new pcmkr resources have to be added to the existing ones
+        data['colocations']['vip_consoleauth'] = COLO_CONSOLEAUTH
+        data['init_services']['res_nova_consoleauth'] = 'nova-consoleauth'
+        data['resources']['res_nova_consoleauth'] = AGENT_CONSOLEAUTH
+        data['resource_params']['res_nova_consoleauth'] = AGENT_CA_PARAMS
+
+        for rid in relation_ids('ha'):
+            relation_set(rid, **data)
+
+    elif (not config('single-nova-consoleauth')
+          and console_attributes('protocol')):
+        for item in ['vip_consoleauth', 'res_nova_consoleauth']:
+            if item not in data['delete_resources']:
+                data['delete_resources'].append(item)
+
+        # remove them from the rel, so they aren't recreated
+        data['colocations'].pop('vip_consoleauth', None)
+        data['init_services'].pop('res_nova_consoleauth', None)
+        data['resources'].pop('res_nova_consoleauth', None)
+        data['resource_params'].pop('res_nova_consoleauth', None)
+
+        for rid in relation_ids('ha'):
+            relation_set(rid, **data)
+
 
 @hooks.hook('amqp-relation-joined')
 def amqp_joined(relation_id=None):
@@ -636,7 +685,7 @@
         'res_nova_haproxy': 'lsb:haproxy',
     }
     resource_params = {
-        'res_nova_haproxy': 'op monitor interval="5s"'
+        'res_nova_haproxy': 'op monitor interval="5s"',
     }
 
     vip_group = []
@@ -674,12 +723,21 @@
     clones = {
         'cl_nova_haproxy': 'res_nova_haproxy'
     }
+    colocations = {}
+
+    if config('single-nova-consoleauth') and console_attributes('protocol'):
+        colocations['vip_consoleauth'] = COLO_CONSOLEAUTH
+        init_services['res_nova_consoleauth'] = 'nova-consoleauth'
+        resources['res_nova_consoleauth'] = AGENT_CONSOLEAUTH
+        resource_params['res_nova_consoleauth'] = AGENT_CA_PARAMS
+
     relation_set(init_services=init_services,
                  corosync_bindiface=cluster_config['ha-bindiface'],
                  corosync_mcastport=cluster_config['ha-mcastport'],
                  resources=resources,
                  resource_params=resource_params,
-                 clones=clones)
+                 clones=clones,
+                 colocations=colocations)
 
 
 @hooks.hook('ha-relation-changed')

=== modified file 'unit_tests/test_nova_cc_hooks.py'
--- unit_tests/test_nova_cc_hooks.py	2015-01-13 14:44:54 +0000
+++ unit_tests/test_nova_cc_hooks.py	2015-03-25 23:24:02 +0000
@@ -611,30 +611,128 @@
             'by the neutron-server process.'
         )
 
-    def test_ha_relation_joined_no_bound_ip(self):
-        self.get_hacluster_config.return_value = {
-            'ha-bindiface': 'em0',
-            'ha-mcastport': '8080',
-            'vip': '10.10.10.10',
-        }
-        self.test_config.set('vip_iface', 'eth120')
-        self.test_config.set('vip_cidr', '21')
-        self.get_iface_for_address.return_value = None
-        self.get_netmask_for_address.return_value = None
-        hooks.ha_joined()
-        args = {
-            'corosync_bindiface': 'em0',
-            'corosync_mcastport': '8080',
-            'init_services': {'res_nova_haproxy': 'haproxy'},
-            'resources': {'res_nova_eth120_vip': 'ocf:heartbeat:IPaddr2',
-                          'res_nova_haproxy': 'lsb:haproxy'},
-            'resource_params': {
-                'res_nova_eth120_vip': 'params ip="10.10.10.10"'
-                ' cidr_netmask="21" nic="eth120"',
-                'res_nova_haproxy': 'op monitor interval="5s"'},
-            'clones': {'cl_nova_haproxy': 'res_nova_haproxy'}
-        }
-        self.relation_set.assert_has_calls([
-            call(groups={'grp_nova_vips': 'res_nova_eth120_vip'}),
-            call(**args),
+    @patch('nova_cc_utils.config')
+    def test_ha_relation_joined_no_bound_ip(self, config):
+        self.get_hacluster_config.return_value = {
+            'ha-bindiface': 'em0',
+            'ha-mcastport': '8080',
+            'vip': '10.10.10.10',
+        }
+        self.test_config.set('vip_iface', 'eth120')
+        self.test_config.set('vip_cidr', '21')
+        config.return_value = None
+        self.get_iface_for_address.return_value = None
+        self.get_netmask_for_address.return_value = None
+        hooks.ha_joined()
+        args = {
+            'corosync_bindiface': 'em0',
+            'corosync_mcastport': '8080',
+            'init_services': {'res_nova_haproxy': 'haproxy'},
+            'resources': {'res_nova_eth120_vip': 'ocf:heartbeat:IPaddr2',
+                          'res_nova_haproxy': 'lsb:haproxy'},
+            'resource_params': {
+                'res_nova_eth120_vip': 'params ip="10.10.10.10"'
+                ' cidr_netmask="21" nic="eth120"',
+                'res_nova_haproxy': 'op monitor interval="5s"'},
+            'colocations': {},
+            'clones': {'cl_nova_haproxy': 'res_nova_haproxy'}
+        }
+        self.relation_set.assert_has_calls([
+            call(groups={'grp_nova_vips': 'res_nova_eth120_vip'}),
+            call(**args),
+        ])
+
+    @patch('nova_cc_utils.config')
+    def test_ha_relation_multi_consoleauth(self, config):
+        self.get_hacluster_config.return_value = {
+            'ha-bindiface': 'em0',
+            'ha-mcastport': '8080',
+            'vip': '10.10.10.10',
+        }
+        self.test_config.set('vip_iface', 'eth120')
+        self.test_config.set('vip_cidr', '21')
+        self.test_config.set('single-nova-consoleauth', False)
+        config.return_value = 'novnc'
+        self.get_iface_for_address.return_value = None
+        self.get_netmask_for_address.return_value = None
+        hooks.ha_joined()
+        args = {
+            'corosync_bindiface': 'em0',
+            'corosync_mcastport': '8080',
+            'init_services': {'res_nova_haproxy': 'haproxy'},
+            'resources': {'res_nova_eth120_vip': 'ocf:heartbeat:IPaddr2',
+                          'res_nova_haproxy': 'lsb:haproxy'},
+            'resource_params': {
+                'res_nova_eth120_vip': 'params ip="10.10.10.10"'
+                ' cidr_netmask="21" nic="eth120"',
+                'res_nova_haproxy': 'op monitor interval="5s"'},
+            'colocations': {},
+            'clones': {'cl_nova_haproxy': 'res_nova_haproxy'}
+        }
+        self.relation_set.assert_has_calls([
+            call(groups={'grp_nova_vips': 'res_nova_eth120_vip'}),
+            call(**args),
+        ])
+
+    @patch('nova_cc_utils.config')
+    def test_ha_relation_single_consoleauth(self, config):
+        self.get_hacluster_config.return_value = {
+            'ha-bindiface': 'em0',
+            'ha-mcastport': '8080',
+            'vip': '10.10.10.10',
+        }
+        self.test_config.set('vip_iface', 'eth120')
+        self.test_config.set('vip_cidr', '21')
+        config.return_value = 'novnc'
+        self.get_iface_for_address.return_value = None
+        self.get_netmask_for_address.return_value = None
+        hooks.ha_joined()
+        args = {
+            'corosync_bindiface': 'em0',
+            'corosync_mcastport': '8080',
+            'init_services': {'res_nova_haproxy': 'haproxy',
+                              'res_nova_consoleauth': 'nova-consoleauth'},
+            'resources': {'res_nova_eth120_vip': 'ocf:heartbeat:IPaddr2',
+                          'res_nova_haproxy': 'lsb:haproxy',
+                          'res_nova_consoleauth':
+                          'ocf:openstack:nova-consoleauth'},
+            'resource_params': {
+                'res_nova_eth120_vip': 'params ip="10.10.10.10"'
+                ' cidr_netmask="21" nic="eth120"',
+                'res_nova_haproxy': 'op monitor interval="5s"',
+                'res_nova_consoleauth': 'op monitor interval="5s"'},
+            'colocations': {
+                'vip_consoleauth': 'inf: res_nova_consoleauth grp_nova_vips'
+            },
+            'clones': {'cl_nova_haproxy': 'res_nova_haproxy'}
+        }
+        self.relation_set.assert_has_calls([
+            call(groups={'grp_nova_vips': 'res_nova_eth120_vip'}),
+            call(**args),
+        ])
+
+    @patch('nova_cc_hooks.configure_https')
+    @patch('nova_cc_utils.config')
+    def test_config_changed_single_consoleauth(self, config, *args):
+        config.return_value = 'novnc'
+        rids = {'ha': ['ha:1']}
+
+        def f(r):
+            return rids.get(r, [])
+
+        self.relation_ids.side_effect = f
+        hooks.config_changed()
+        args = {
+            'delete_resources': [],
+            'init_services': {'res_nova_consoleauth': 'nova-consoleauth'},
+            'resources': {'res_nova_consoleauth':
+                          'ocf:openstack:nova-consoleauth'},
+            'resource_params': {
+                'res_nova_consoleauth': 'op monitor interval="5s"'},
+            'colocations': {
+                'vip_consoleauth': 'inf: res_nova_consoleauth grp_nova_vips'
+            }
+        }
+        self.relation_set.assert_has_calls([
+            call(v, **args) for v in rids['ha']
         ])