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

Proposed by Felipe Reyes
Status: Superseded
Proposed branch: lp://staging/~freyes/charms/trusty/nova-cloud-controller/single-nova-consoleauth
Merge into: lp://staging/~openstack-charmers-archive/charms/trusty/nova-cloud-controller/next
Diff against target: 276 lines (+195/-28)
3 files modified
config.yaml (+11/-0)
hooks/nova_cc_hooks.py (+60/-2)
unit_tests/test_nova_cc_hooks.py (+124/-26)
To merge this branch: bzr merge lp://staging/~freyes/charms/trusty/nova-cloud-controller/single-nova-consoleauth
Reviewer Review Type Date Requested Status
Edward Hope-Morley Needs Fixing
OpenStack Charmers Pending
Jorge Niedbalski Pending
Billy Olsen Pending
Review via email: mp+253309@code.staging.launchpad.net

This proposal supersedes a proposal from 2015-03-06.

This proposal has been superseded by a proposal from 2015-03-25.

Description of the change

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,

To post a comment you must log in.
Revision history for this message
uosci-testing-bot (uosci-testing-bot) wrote : Posted in a previous version of this proposal

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/

Revision history for this message
uosci-testing-bot (uosci-testing-bot) wrote : Posted in a previous version of this proposal

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/

Revision history for this message
uosci-testing-bot (uosci-testing-bot) wrote : Posted in a previous version of this proposal

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/

Revision history for this message
Billy Olsen (billy-olsen) wrote : Posted in a previous version of this proposal

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
Revision history for this message
uosci-testing-bot (uosci-testing-bot) wrote : Posted in a previous version of this proposal

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/

Revision history for this message
uosci-testing-bot (uosci-testing-bot) wrote : Posted in a previous version of this proposal

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/

Revision history for this message
uosci-testing-bot (uosci-testing-bot) wrote : Posted in a previous version of this proposal

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/

Revision history for this message
Billy Olsen (billy-olsen) wrote : Posted in a previous version of this proposal

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

review: Approve
Revision history for this message
Billy Olsen (billy-olsen) wrote : Posted in a previous version of this proposal

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
Revision history for this message
uosci-testing-bot (uosci-testing-bot) wrote : Posted in a previous version of this proposal

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/

Revision history for this message
uosci-testing-bot (uosci-testing-bot) wrote : Posted in a previous version of this proposal

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/

Revision history for this message
uosci-testing-bot (uosci-testing-bot) wrote : Posted in a previous version of this proposal

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/

Revision history for this message
Felipe Reyes (freyes) wrote : Posted in a previous version of this proposal

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

Revision history for this message
Felipe Reyes (freyes) wrote : Posted in a previous version of this proposal

> 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,

Revision history for this message
Felipe Reyes (freyes) wrote :

@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.

Revision history for this message
uosci-testing-bot (uosci-testing-bot) wrote :

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/

Revision history for this message
uosci-testing-bot (uosci-testing-bot) wrote :

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/

Revision history for this message
uosci-testing-bot (uosci-testing-bot) wrote :

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/

Revision history for this message
Felipe Reyes (freyes) wrote :

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.

Revision history for this message
Edward Hope-Morley (hopem) wrote :

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
Revision history for this message
Felipe Reyes (freyes) wrote :

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.

Revision history for this message
Felipe Reyes (freyes) wrote :

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

Revision history for this message
Edward Hope-Morley (hopem) wrote :

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
154. By Felipe Reyes

Commit after merge

155. By Felipe Reyes

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

156. By Felipe Reyes

fix lint warning

157. By Felipe Reyes

Commit after merge

158. By Felipe Reyes

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

Unmerged revisions

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
The diff is not available at this time. You can reload the page or download it.

Subscribers

People subscribed via source and target branches