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

[Felipe Reyes]

This branch extends the charm to install python-memcache, and configure nova.conf adding the key memcached_servers when a relationship with memcached service is established.

If multiple units of memcached are available, all of them are used (memcached_servers = host1:port1,host2:port,host3:port3)

To secure memcached access this relies in the memcached charm ability to use iptables rules to only allow access to related machines[0]

This patch can be tested using the deployer file available at https://code.launchpad.net/~openstack-charm-testers/+junk/nova-memcached

[0] http://bazaar.launchpad.net/~charmers/charms/trusty/memcached/trunk/revision/61

[Ryan Beisner (1chb1n)]

UOSCI bot says:
charm_lint_check #771 nova-cloud-controller-next for freyes mp239671
    LINT OK: believed to pass, but you should confirm results

LINT Results (max last 4 lines) from
/var/lib/jenkins/workspace/charm_lint_check/make-lint.771:
I: config.yaml: option haproxy-client-timeout has no default value
I: config.yaml: option ssl_cert has no default value
I: config.yaml: option nvp-l3-uuid has no default value
I: config.yaml: option os-internal-network has no default value

Full lint output: http://paste.ubuntu.com/8694351/
Build: http://10.98.191.181:8080/job/charm_lint_check/771/

[Ryan Beisner (1chb1n)]

UOSCI bot says:
charm_unit_test #579 nova-cloud-controller-next for freyes mp239671
    UNIT FAIL: unit-test failed

UNIT Results (max last 4 lines) from
/var/lib/jenkins/workspace/charm_unit_test/unit-test.579:
Ran 4 tests in 0.150s

FAILED (errors=2)
make: *** [unit_test] Error 1

Full unit output: http://paste.ubuntu.com/8694352/
Build: http://10.98.191.181:8080/job/charm_unit_test/579/

[Ryan Beisner (1chb1n)]

UOSCI bot says:
charm_amulet_test #304 nova-cloud-controller-next for freyes mp239671
    AMULET OK: believed to pass, but you should confirm results

AMULET Results (max last 4 lines) from
/var/lib/jenkins/workspace/charm_amulet_test/make-test.304:
juju-test.conductor DEBUG : Tearing down osci-sv05 juju environment
juju-test.conductor DEBUG : Calling "juju destroy-environment -y osci-sv05"
WARNING cannot delete security group "juju-osci-sv05-0". Used by another environment?
juju-test INFO : Results: 3 passed, 0 failed, 0 errored

Full amulet output: http://paste.ubuntu.com/8694683/
Build: http://10.98.191.181:8080/job/charm_amulet_test/304/

[Ryan Beisner (1chb1n)]

UOSCI bot says:
charm_unit_test #582 nova-cloud-controller-next for freyes mp239671
    UNIT FAIL: unit-test failed

UNIT Results (max last 4 lines) from
/var/lib/jenkins/workspace/charm_unit_test/unit-test.582:
Ran 4 tests in 0.153s

FAILED (errors=2)
make: *** [unit_test] Error 1

Full unit output: http://paste.ubuntu.com/8705925/
Build: http://10.98.191.181:8080/job/charm_unit_test/582/

[Ryan Beisner (1chb1n)]

UOSCI bot says:
charm_lint_check #774 nova-cloud-controller-next for freyes mp239671
    LINT OK: believed to pass, but you should confirm results

LINT Results (max last 4 lines) from
/var/lib/jenkins/workspace/charm_lint_check/make-lint.774:
I: config.yaml: option haproxy-client-timeout has no default value
I: config.yaml: option ssl_cert has no default value
I: config.yaml: option nvp-l3-uuid has no default value
I: config.yaml: option os-internal-network has no default value

Full lint output: http://paste.ubuntu.com/8705924/
Build: http://10.98.191.181:8080/job/charm_lint_check/774/

[Ryan Beisner (1chb1n)]

UOSCI bot says:
charm_amulet_test #307 nova-cloud-controller-next for freyes mp239671
    AMULET FAIL: amulet-test failed

AMULET Results (max last 4 lines) from
/var/lib/jenkins/workspace/charm_amulet_test/make-test.307:
    return self.read(buflen)
  File "/usr/lib/python2.7/ssl.py", line 260, in read
    return self._sslobj.read(len)
socket.error: [Errno 110] Connection timed out

Full amulet output: http://paste.ubuntu.com/8706467/
Build: http://10.98.191.181:8080/job/charm_amulet_test/307/

[Edward Hope-Morley (hopem)]

I'm getting a couple of unit test failures here. Looks like a couple of extra mocks needed - see attached u/t output.

review: Needs Fixing

[Edward Hope-Morley (hopem)]

Hmm can't attach so pasting:

$ make unit_test
Starting unit tests...
nose.plugins.cover: ERROR: Coverage not available: unable to import coverage module
test_instance_console_context_with_memcache (unit_tests.test_nova_cc_contexts.NovaComputeContextTests) ... ok
test_instance_console_context_without_memcache (unit_tests.test_nova_cc_contexts.NovaComputeContextTests) ... ok
Failure: OSError ([Errno 2] No such file or directory) ... ERROR
Failure: OSError ([Errno 2] No such file or directory) ... ERROR

======================================================================
ERROR: Failure: OSError ([Errno 2] No such file or directory)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/usr/lib/python2.7/dist-packages/nose/loader.py", line 411, in loadTestsFromName
    addr.filename, addr.module)
  File "/usr/lib/python2.7/dist-packages/nose/importer.py", line 47, in importFromPath
    return self.importFromDir(dir_path, fqname)
  File "/usr/lib/python2.7/dist-packages/nose/importer.py", line 94, in importFromDir
    mod = load_module(part_fqname, fh, filename, desc)
  File "/home/gizmo/Documents/reviews/nova-cloud-controller/bug-989337/unit_tests/test_nova_cc_hooks.py", line 6, in <module>
    import nova_cc_utils as utils
  File "hooks/nova_cc_utils.py", line 142, in <module>
    nova_cc_context.NeutronPostgresqlDBContext(),
  File "hooks/nova_cc_context.py", line 268, in __init__
    self).__init__(config('neutron-database'))
  File "hooks/charmhelpers/core/hookenv.py", line 44, in wrapper
    res = func(*args, **kwargs)
  File "hooks/charmhelpers/core/hookenv.py", line 281, in config
    config_data = json.loads(subprocess.check_output(config_cmd_line))
  File "/usr/lib/python2.7/subprocess.py", line 566, in check_output
    process = Popen(stdout=PIPE, *popenargs, **kwargs)
  File "/usr/lib/python2.7/subprocess.py", line 710, in __init__
    errread, errwrite)
  File "/usr/lib/python2.7/subprocess.py", line 1327, in _execute_child
    raise child_exception
OSError: [Errno 2] No such file or directory

======================================================================
ERROR: Failure: OSError ([Errno 2] No such file or directory)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/usr/lib/python2.7/dist-packages/nose/loader.py", line 411, in loadTestsFromName
    addr.filename, addr.module)
  File "/usr/lib/python2.7/dist-packages/nose/importer.py", line 47, in importFromPath
    return self.importFromDir(dir_path, fqname)
  File "/usr/lib/python2.7/dist-packages/nose/importer.py", line 94, in importFromDir
    mod = load_module(part_fqname, fh, filename, desc)
  File "/home/gizmo/Documents/reviews/nova-cloud-controller/bug-989337/unit_tests/test_nova_cc_utils.py", line 11, in <module>
    import nova_cc_utils as utils
  File "hooks/nova_cc_utils.py", line 142, in <module>
    nova_cc_context.NeutronPostgresqlDBContext(),
  File "hooks/nova_cc_context.py", line 268, in __init__
    self).__init__(config('neutron-database'))
  File "hooks/charmhelpers/core/hookenv.py", line 44, in wrapper
    res = func(*args, **kwa...

[Edward Hope-Morley (hopem)]

FWIW, other than the above this lgtm

[Felipe Reyes (freyes)]

Edward, I fixed the broken tests, the patch was a little bit larger than what I expected, but they are working OK now.

Name Stmts Miss Cover Missing
-----------------------------------------------------
hooks/nova_cc_context 166 112 33% 21-28, 39-41, 48-59, 65-79, 86-104, 116-169, 180-195, 203-204, 208, 212-214, 219, 222-236, 242-256, 273-276, 281-283, 297-299
hooks/nova_cc_hooks 447 146 67% 132-135, 148-149, 173, 184-185, 189, 224, 229-234, 245, 325-328, 336-339, 345-348, 358-374, 383-385, 395-409, 413-422, 508, 518, 522-523, 576, 582-592, 597-608, 618-628, 633-672, 682-697, 705-709, 734-743, 767, 772-780, 806, 861, 865-868
hooks/nova_cc_utils 442 111 75% 296-301, 312-315, 325-326, 382, 384, 428-430, 434, 448-456, 463-468, 472-486, 592-594, 599-602, 607, 611, 635-636, 650-652, 673-674, 680-703, 707-713, 717-723, 729, 735, 742, 753-757, 842, 902-908, 912-914, 918-921, 925-937
-----------------------------------------------------
TOTAL 1055 369 65%
----------------------------------------------------------------------
Ran 99 tests in 7.518s

[Ryan Beisner (1chb1n)]

UOSCI bot says:
charm_lint_check #922 nova-cloud-controller-next for freyes mp239671
    LINT OK: passed

LINT Results (max last 5 lines):
  I: config.yaml: option os-admin-network has no default value
  I: config.yaml: option haproxy-client-timeout has no default value
  I: config.yaml: option ssl_cert has no default value
  I: config.yaml: option nvp-l3-uuid has no default value
  I: config.yaml: option os-internal-network has no default value

Full lint test output: http://paste.ubuntu.com/8822840/
Build: http://10.98.191.181:8080/job/charm_lint_check/922/

[Ryan Beisner (1chb1n)]

UOSCI bot says:
charm_unit_test #757 nova-cloud-controller-next for freyes mp239671
    UNIT OK: passed

UNIT Results (max last 5 lines):
  hooks/nova_cc_hooks 447 146 67% 132-135, 148-149, 173, 184-185, 189, 224, 229-234, 245, 325-328, 336-339, 345-348, 358-374, 383-385, 395-409, 413-422, 508, 518, 522-523, 576, 582-592, 597-608, 618-628, 633-672, 682-697, 705-709, 734-743, 767, 772-780, 806, 861, 865-868
  hooks/nova_cc_utils 442 111 75% 296-301, 312-315, 325-326, 382, 384, 428-430, 434, 448-456, 463-468, 472-486, 592-594, 599-602, 607, 611, 635-636, 650-652, 673-674, 680-703, 707-713, 717-723, 729, 735, 742, 753-757, 842, 902-908, 912-914, 918-921, 925-937
  TOTAL 1055 369 65%
  Ran 99 tests in 8.707s
  OK

Full unit test output: http://paste.ubuntu.com/8822843/
Build: http://10.98.191.181:8080/job/charm_unit_test/757/

[Ryan Beisner (1chb1n)]

UOSCI bot says:
charm_amulet_test #340 nova-cloud-controller-next for freyes mp239671
    AMULET OK: passed

AMULET Results (max last 5 lines):
  juju-test.conductor.15-basic-trusty-icehouse RESULT :
  juju-test.conductor DEBUG : Tearing down osci-sv07 juju environment
  juju-test.conductor DEBUG : Calling "juju destroy-environment -y osci-sv07"
  WARNING cannot delete security group "juju-osci-sv07-0". Used by another environment?
  juju-test INFO : Results: 3 passed, 0 failed, 0 errored

Full amulet test output: http://paste.ubuntu.com/8823194/
Build: http://10.98.191.181:8080/job/charm_amulet_test/340/

[Ryan Beisner (1chb1n)]

UOSCI bot says:
charm_lint_check #979 nova-cloud-controller-next for freyes mp241318
    LINT OK: passed

LINT Results (max last 5 lines):
  I: config.yaml: option os-admin-network has no default value
  I: config.yaml: option haproxy-client-timeout has no default value
  I: config.yaml: option ssl_cert has no default value
  I: config.yaml: option nvp-l3-uuid has no default value
  I: config.yaml: option os-internal-network has no default value

Full lint test output: http://paste.ubuntu.com/8925102/
Build: http://10.98.191.181:8080/job/charm_lint_check/979/

[Ryan Beisner (1chb1n)]

UOSCI bot says:
charm_unit_test #814 nova-cloud-controller-next for freyes mp241318
    UNIT OK: passed

UNIT Results (max last 5 lines):
  hooks/nova_cc_hooks 447 146 67% 132-135, 148-149, 173, 184-185, 189, 224, 229-234, 245, 325-328, 336-339, 345-348, 358-374, 383-385, 395-409, 413-422, 508, 518, 522-523, 576, 582-592, 597-608, 618-628, 633-672, 682-697, 705-709, 734-743, 767, 772-780, 806, 861, 865-868
  hooks/nova_cc_utils 442 111 75% 296-301, 312-315, 325-326, 382, 384, 428-430, 434, 448-456, 463-468, 472-486, 592-594, 599-602, 607, 611, 635-636, 650-652, 673-674, 680-703, 707-713, 717-723, 729, 735, 742, 753-757, 842, 902-908, 912-914, 918-921, 925-937
  TOTAL 1055 369 65%
  Ran 99 tests in 8.712s
  OK

Full unit test output: http://paste.ubuntu.com/8925116/
Build: http://10.98.191.181:8080/job/charm_unit_test/814/

[Ryan Beisner (1chb1n)]

UOSCI bot says:
charm_amulet_test #359 nova-cloud-controller-next for freyes mp241318
    AMULET OK: passed

AMULET Results (max last 5 lines):
  juju-test.conductor.15-basic-trusty-icehouse RESULT :
  juju-test.conductor DEBUG : Tearing down osci-sv07 juju environment
  juju-test.conductor DEBUG : Calling "juju destroy-environment -y osci-sv07"
  WARNING cannot delete security group "juju-osci-sv07-0". Used by another environment?
  juju-test INFO : Results: 3 passed, 0 failed, 0 errored

Full amulet test output: http://paste.ubuntu.com/8925544/
Build: http://10.98.191.181:8080/job/charm_amulet_test/359/

[Edward Hope-Morley (hopem)]

Felipe, thanks for working on this it is much needed.

So some initial comments;

  * could you please rename the cache-relation-* to memcache-relation-*
  * is there are reason for replacing config() with hookenv.config()? I know it is not the nicest coding style but it is the same across all charms so to do it differently here would be breaking convention.
  * some more comments inline

review: Needs Fixing

[Felipe Reyes (freyes)]

Hi,

> * could you please rename the cache-relation-* to memcache-relation-*
I initially used memcache-relation-* hooks, but they weren't triggered, a quick look to mediawiki charm also uses cache-relation-* for memcache, I'll give it another try to double check this.

> * is there are reason for replacing config() with hookenv.config()? I know
> it is not the nicest coding style but it is the same across all charms so to
> do it differently here would be breaking convention.
I know, but importing config() function made fail the mocking due to the way the modules are imported, if this is a blocker I totally understand and I can revisit this to find a way to make the tests happy.

Best,

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

UOSCI bot says:
charm_lint_check #1151 nova-cloud-controller-next for freyes mp241318
    LINT OK: passed

LINT Results (max last 5 lines):
  I: config.yaml: option os-admin-network has no default value
  I: config.yaml: option haproxy-client-timeout has no default value
  I: config.yaml: option ssl_cert has no default value
  I: config.yaml: option nvp-l3-uuid has no default value
  I: config.yaml: option os-internal-network has no default value

Full lint test output: http://paste.ubuntu.com/9125387/
Build: http://10.98.191.181:8080/job/charm_lint_check/1151/

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

UOSCI bot says:
charm_unit_test #985 nova-cloud-controller-next for freyes mp241318
    UNIT OK: passed

UNIT Results (max last 5 lines):
  hooks/nova_cc_hooks 447 146 67% 132-135, 148-149, 173, 184-185, 189, 224, 229-234, 245, 325-328, 336-339, 345-348, 358-374, 383-385, 395-409, 413-422, 508, 518, 522-523, 576, 582-592, 597-608, 618-628, 633-672, 682-697, 705-709, 734-743, 767, 772-780, 806, 861, 865-868
  hooks/nova_cc_utils 442 111 75% 296-301, 312-315, 325-326, 382, 384, 428-430, 434, 448-456, 463-468, 472-486, 592-594, 599-602, 607, 611, 635-636, 650-652, 673-674, 680-703, 707-713, 717-723, 729, 735, 742, 753-757, 842, 902-908, 912-914, 918-921, 925-937
  TOTAL 1054 369 65%
  Ran 99 tests in 9.596s
  OK

Full unit test output: http://paste.ubuntu.com/9125402/
Build: http://10.98.191.181:8080/job/charm_unit_test/985/

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

UOSCI bot says:
charm_amulet_test #493 nova-cloud-controller-next for freyes mp241318
    AMULET FAIL: amulet-test failed

AMULET Results (max last 5 lines):
"
  WARNING cannot delete security group "juju-osci-sv07-0". Used by another environment?
  juju-test INFO : Results: 2 passed, 0 failed, 1 errored
  ERROR subprocess encountered error code 124
  make: *** [test] Error 124

Full amulet test output: http://paste.ubuntu.com/9125932/
Build: http://10.98.191.181:8080/job/charm_amulet_test/493/

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

UOSCI bot says:
charm_lint_check #1159 nova-cloud-controller-next for freyes mp242384
    LINT OK: passed

LINT Results (max last 5 lines):
  I: config.yaml: option os-admin-network has no default value
  I: config.yaml: option haproxy-client-timeout has no default value
  I: config.yaml: option ssl_cert has no default value
  I: config.yaml: option nvp-l3-uuid has no default value
  I: config.yaml: option os-internal-network has no default value

Full lint test output: http://paste.ubuntu.com/9127053/
Build: http://10.98.191.181:8080/job/charm_lint_check/1159/

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

UOSCI bot says:
charm_unit_test #993 nova-cloud-controller-next for freyes mp242384
    UNIT OK: passed

UNIT Results (max last 5 lines):
  hooks/nova_cc_hooks 445 146 67% 132-135, 148-149, 173, 184-185, 189, 221, 226-231, 242, 322-325, 333-336, 342-345, 355-371, 380-382, 392-406, 410-419, 505, 515, 519-520, 573, 579-589, 594-605, 615-625, 630-669, 679-694, 702-706, 731-740, 764, 769-777, 803, 858, 862-865
  hooks/nova_cc_utils 445 112 75% 298-303, 314-317, 327-328, 384, 386, 432-434, 438, 452-460, 467-472, 476-490, 546, 595-597, 602-605, 610, 614, 638-639, 653-655, 676-677, 683-706, 710-716, 720-726, 732, 738, 745, 756-760, 845, 905-911, 915-917, 921-924, 928-940
  TOTAL 1055 370 65%
  Ran 98 tests in 10.450s
  OK

Full unit test output: http://paste.ubuntu.com/9127055/
Build: http://10.98.191.181:8080/job/charm_unit_test/993/

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

UOSCI bot says:
charm_amulet_test #501 nova-cloud-controller-next for freyes mp242384
    AMULET FAIL: amulet-test failed

AMULET Results (max last 5 lines):
  subprocess.CalledProcessError: Command '['juju-deployer', '-W', '-L', '-c', '/tmp/amulet-juju-deployer-bskSEW.json', '-e', 'osci-sv05', '-t', '1000', 'osci-sv05']' returned non-zero exit status 1
  WARNING cannot delete security group "juju-osci-sv05-0". Used by another environment?
  juju-test INFO : Results: 0 passed, 2 failed, 1 errored
  ERROR subprocess encountered error code 2
  make: *** [test] Error 2

Full amulet test output: http://paste.ubuntu.com/9127779/
Build: http://10.98.191.181:8080/job/charm_amulet_test/501/

[Felipe Reyes (freyes)]

> juju-test.conductor.15-basic-trusty-icehouse RESULT : ✔
> juju-test.conductor DEBUG : Tearing down osci-sv07 juju environment
> juju-test.conductor DEBUG : Calling "juju destroy-environment -y osci-sv07ERROR:root:Make target returned non-zero.
> "
> WARNING cannot delete security group "juju-osci-sv07-0". Used by another environment?
> juju-test INFO : Results: 2 passed, 0 failed, 1 errored
> ERROR subprocess encountered error code 124

Should I do something with this error?

review: Needs Resubmitting

[Felipe Reyes (freyes)]

review: Abstain

[Felipe Reyes (freyes)]

By the way, this patch can be tested using the bundler file available at https://code.launchpad.net/~openstack-charm-testers/+junk/nova-memcached

Thanks,

[James Troup (elmo)]

memcache (by design) has no ACLs and is not safe to deploy without some sort of protection (e.g. local iptables rules or being run on a private network not accessible to anything but trusted machines). How is this MP dealing with that?

review: Needs Information

[Edward Hope-Morley (hopem)]

Could this work - https://code.google.com/p/memcached/wiki/SASLHowto

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

UOSCI bot says:
charm_lint_check #1236 nova-cloud-controller-next for freyes mp242980
    LINT OK: passed

LINT Results (max last 5 lines):
  I: config.yaml: option os-admin-network has no default value
  I: config.yaml: option haproxy-client-timeout has no default value
  I: config.yaml: option ssl_cert has no default value
  I: config.yaml: option nvp-l3-uuid has no default value
  I: config.yaml: option os-internal-network has no default value

Full lint test output: http://paste.ubuntu.com/9256250/
Build: http://10.98.191.181:8080/job/charm_lint_check/1236/

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

UOSCI bot says:
charm_unit_test #1070 nova-cloud-controller-next for freyes mp242980
    UNIT OK: passed

UNIT Results (max last 5 lines):
  hooks/nova_cc_hooks 445 146 67% 132-135, 148-149, 173, 184-185, 189, 221, 226-231, 242, 322-325, 333-336, 342-345, 355-371, 380-382, 392-406, 410-419, 505, 515, 519-520, 573, 579-589, 594-605, 615-625, 630-669, 679-694, 702-706, 731-740, 764, 769-777, 803, 858, 862-865
  hooks/nova_cc_utils 445 112 75% 298-303, 314-317, 327-328, 384, 386, 432-434, 438, 452-460, 467-472, 476-490, 546, 595-597, 602-605, 610, 614, 638-639, 653-655, 676-677, 683-706, 710-716, 720-726, 732, 738, 745, 756-760, 845, 905-911, 915-917, 921-924, 928-940
  TOTAL 1057 370 65%
  Ran 99 tests in 10.551s
  OK

Full unit test output: http://paste.ubuntu.com/9256252/
Build: http://10.98.191.181:8080/job/charm_unit_test/1070/

[Edward Hope-Morley (hopem)]

We should already be leveraging split network support here e.g. have memcached running on the os-admin-network if defined. That should suffice.

[Edward Hope-Morley (hopem)]

See inline comment

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

UOSCI bot says:
charm_amulet_test #539 nova-cloud-controller-next for freyes mp242980
    AMULET OK: passed

AMULET Results (max last 5 lines):
  juju-test.conductor.15-basic-trusty-icehouse RESULT :
  juju-test.conductor DEBUG : Tearing down osci-sv07 juju environment
  juju-test.conductor DEBUG : Calling "juju destroy-environment -y osci-sv07"
  WARNING cannot delete security group "juju-osci-sv07-0". Used by another environment?
  juju-test INFO : Results: 3 passed, 0 failed, 0 errored

Full amulet test output: http://paste.ubuntu.com/9256538/
Build: http://10.98.191.181:8080/job/charm_amulet_test/539/

[Jorge Niedbalski (niedbalski)]

Will not be so easy to enable SASL support on the proposal, mostly since the memcache client requires to support it, and the default python-memcache module seems to not support it. We could use python-binary-memcached or pylibmc-sasl and that will require changes on openstack upstream and packages .. I think the best we could do now is make some iptables assertions based on the related ncc hosts ..

[Felipe Reyes (freyes)]

@elmo, I'm addressing your concerns about memcached security adding iptables rules to allow access only to related machines, I would like to get your feedback on that MP[0]

[0] https://code.launchpad.net/~freyes/charms/trusty/memcached/python-rewrite/+merge/243407

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

charm_lint_check #202 nova-cloud-controller-next for freyes mp244432
    LINT OK: passed

Build: http://10.230.18.80:8080/job/charm_lint_check/202/

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

charm_unit_test #165 nova-cloud-controller-next for freyes mp244432
    UNIT OK: passed

Build: http://10.230.18.80:8080/job/charm_unit_test/165/

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

charm_amulet_test #120 nova-cloud-controller-next for freyes mp244432
    AMULET OK: passed

Build: http://10.230.18.80:8080/job/charm_amulet_test/120/

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

charm_lint_check #83 nova-cloud-controller-next for freyes mp244432
    LINT OK: passed

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

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

charm_lint_check #89 nova-cloud-controller-next for freyes mp244432
    LINT OK: passed

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

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

charm_unit_test #118 nova-cloud-controller-next for freyes mp244432
    UNIT OK: passed

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

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

charm_unit_test #121 nova-cloud-controller-next for freyes mp244432
    UNIT OK: passed

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

[Jorge Niedbalski (niedbalski)]

All LGTM. Unit and Amulet tests passing, the python written memcache charm has already landed. So just a very minor string refactor required.

Thanks Felipe

review: Approve

[Felipe Reyes (freyes)]

@niedbalski, updated MP to address your comment.

Thanks for reviewing it :)

[Edward Hope-Morley (hopem)]

Felipe, this mostly lgtm but I have a couple of nits. This is top of my list of reviews though!

review: Needs Fixing

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

charm_lint_check #183 nova-cloud-controller-next for freyes mp244763
    LINT OK: passed

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

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

charm_unit_test #216 nova-cloud-controller-next for freyes mp244763
    UNIT OK: passed

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

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

charm_amulet_test #236 nova-cloud-controller-next for freyes mp244763
    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/9535877/
Build: http://10.245.162.77:8080/job/charm_amulet_test/236/

[lp:~freyes/charms/trusty/nova-cloud-controller/bug-989337]

134. By Felipe Reyes on 2014-12-16

Refactor the way memcached_servers config is put in the templates

Instead of formatting the string with Jinja2, it's formatted with python
and passed to the template as a string.

Unmerged revisions

=== added symlink 'hooks/memcache-relation-broken'
=== target is u'nova_cc_hooks.py'
=== added symlink 'hooks/memcache-relation-changed'
=== target is u'nova_cc_hooks.py'
=== added symlink 'hooks/memcache-relation-departed'
=== target is u'nova_cc_hooks.py'
=== added symlink 'hooks/memcache-relation-joined'
=== target is u'nova_cc_hooks.py'
=== modified file 'hooks/nova_cc_context.py'
--- hooks/nova_cc_context.py	2014-12-03 23:49:34 +0000
+++ hooks/nova_cc_context.py	2014-12-16 14:10:29 +0000
@@ -6,6 +6,7 @@
     ERROR,
     unit_get,
     related_units,
+    relations_for_id,
     relation_get,
 )
 from charmhelpers.fetch import (
@@ -299,3 +300,27 @@
         ctxt = super(NovaIPv6Context, self).__call__()
         ctxt['use_ipv6'] = config('prefer-ipv6')
         return ctxt
+
+
+class InstanceConsoleContext(context.OSContextGenerator):
+    interfaces = []
+
+    def __call__(self):
+        ctxt = {}
+        servers = []
+        try:
+            for rid in relation_ids('memcache'):
+                for rel in relations_for_id(rid):
+                    priv_addr = rel['private-address']
+                    # format it as IPv6 address if neeeded
+                    priv_addr = format_ipv6_addr(priv_addr) or priv_addr
+                    servers.append({'private-address': priv_addr,
+                                    'port': rel['port']})
+        except Exception as ex:
+            log("Couldn't get memcache servers: {}".format(str(ex)),
+                level='WARNING')
+            servers = []
+
+        ctxt['memcached_servers'] = ','.join(
+            ["%s:%s" % (s['private-address'], s['port']) for s in servers])
+        return ctxt

=== modified file 'hooks/nova_cc_hooks.py'
--- hooks/nova_cc_hooks.py	2014-10-23 07:32:45 +0000
+++ hooks/nova_cc_hooks.py	2014-12-16 14:10:29 +0000
@@ -849,6 +849,15 @@
         quantum_joined(rid=rid)
 
 
[e-mail]('memcache-relation-joined',
+            'memcache-relation-departed',
+            'memcache-relation-changed',
+            'memcache-relation-broken')
+@restart_on_change(restart_map())
+def memcached_joined():
+    CONFIGS.write(NOVA_CONF)
+
+
 def main():
     try:
         hooks.execute(sys.argv)

=== modified file 'hooks/nova_cc_utils.py'
--- hooks/nova_cc_utils.py	2014-11-14 09:55:10 +0000
+++ hooks/nova_cc_utils.py	2014-12-16 14:10:29 +0000
@@ -68,6 +68,7 @@
     'python-psycopg2',
     'python-psutil',
     'uuid',
+    'python-memcache',
 ]
 
 BASE_SERVICES = [
@@ -123,7 +124,8 @@
                      nova_cc_context.VolumeServiceContext(),
                      nova_cc_context.NovaIPv6Context(),
                      nova_cc_context.NeutronCCContext(),
-                     nova_cc_context.NovaConfigContext()],
+                     nova_cc_context.NovaConfigContext(),
+                     nova_cc_context.InstanceConsoleContext()],
     }),
     (NOVA_API_PASTE, {
         'services': [s for s in BASE_SERVICES if 'api' in s],

=== modified file 'metadata.yaml'
--- metadata.yaml	2014-07-11 09:14:57 +0000
+++ metadata.yaml	2014-12-16 14:10:29 +0000
@@ -40,6 +40,8 @@
   nova-vmware:
     interface: nova-vmware
     scope: container
+  memcache:
+    interface: memcache
 peers:
   cluster:
     interface: nova-ha

=== modified file 'templates/folsom/nova.conf'
--- templates/folsom/nova.conf	2014-04-16 08:25:14 +0000
+++ templates/folsom/nova.conf	2014-12-16 14:10:29 +0000
@@ -21,6 +21,11 @@
 enabled_apis=ec2,osapi_compute,metadata
 auth_strategy=keystone
 compute_driver=libvirt.LibvirtDriver
+
+{% if memcached_servers %}
+memcached_servers = {%for s in memcached_servers %}{% if loop.index0 != 0 %},{% endif %}{{s['private-address']}}:{{s['port']}}{% endfor %}
+{% endif %}
+
 {% if keystone_ec2_url -%}
 keystone_ec2_url = {{ keystone_ec2_url }}
 {% endif -%}

=== modified file 'templates/grizzly/nova.conf'
--- templates/grizzly/nova.conf	2014-03-31 11:56:09 +0000
+++ templates/grizzly/nova.conf	2014-12-16 14:10:29 +0000
@@ -20,6 +20,11 @@
 enabled_apis=ec2,osapi_compute,metadata
 auth_strategy=keystone
 compute_driver=libvirt.LibvirtDriver
+
+{% if memcached_servers %}
+memcached_servers = {{ memcached_servers }}
+{% endif %}
+
 {% if keystone_ec2_url -%}
 keystone_ec2_url = {{ keystone_ec2_url }}
 {% endif -%}

=== modified file 'templates/havana/nova.conf'
--- templates/havana/nova.conf	2014-12-03 23:31:19 +0000
+++ templates/havana/nova.conf	2014-12-16 14:10:29 +0000
@@ -27,6 +27,10 @@
 use_syslog={{ use_syslog }}
 my_ip = {{ host_ip }}
 
+{% if memcached_servers %}
+memcached_servers = {{ memcached_servers }}
+{% endif %}
+
 {% if keystone_ec2_url -%}
 keystone_ec2_url = {{ keystone_ec2_url }}
 {% endif -%}

=== modified file 'templates/icehouse/nova.conf'
--- templates/icehouse/nova.conf	2014-12-03 23:31:19 +0000
+++ templates/icehouse/nova.conf	2014-12-16 14:10:29 +0000
@@ -38,6 +38,10 @@
 use_syslog={{ use_syslog }}
 my_ip = {{ host_ip }}
 
+{% if memcached_servers %}
+memcached_servers = {{ memcached_servers }}
+{% endif %}
+
 {% if keystone_ec2_url -%}
 keystone_ec2_url = {{ keystone_ec2_url }}
 {% endif -%}

=== added file 'unit_tests/test_nova_cc_contexts.py'
--- unit_tests/test_nova_cc_contexts.py	1970-01-01 00:00:00 +0000
+++ unit_tests/test_nova_cc_contexts.py	2014-12-16 14:10:29 +0000
@@ -0,0 +1,87 @@
+from __future__ import print_function
+
+import mock
+
+#####
+# NOTE(freyes): this is a workaround to patch config() function imported by
+# nova_cc_utils before it gets a reference to the actual config() provided by
+# hookenv module.
+from charmhelpers.core import hookenv
+_conf = hookenv.config
+hookenv.config = mock.MagicMock()
+import nova_cc_utils as _utils
+# this assert is a double check + to avoid pep8 warning
+assert _utils.config == hookenv.config
+hookenv.config = _conf
+#####
+
+import nova_cc_context as context
+
+from charmhelpers.contrib.openstack import utils
+
+from test_utils import CharmTestCase
+
+
+TO_PATCH = [
+    'apt_install',
+    'filter_installed_packages',
+    'relation_ids',
+    'relation_get',
+    'related_units',
+    'config',
+    'log',
+    'unit_get',
+    'relations_for_id',
+]
+
+
+def fake_log(msg, level=None):
+    level = level or 'INFO'
+    print('[juju test log (%s)] %s' % (level, msg))
+
+
+class NovaComputeContextTests(CharmTestCase):
+    def setUp(self):
+        super(NovaComputeContextTests, self).setUp(context, TO_PATCH)
+        self.relation_get.side_effect = self.test_relation.get
+        self.config.side_effect = self.test_config.get
+        self.log.side_effect = fake_log
+
+    @mock.patch.object(utils, 'os_release')
+    @mock.patch('charmhelpers.contrib.network.ip.log')
+    def test_instance_console_context_without_memcache(self, os_release, log_):
+        self.unit_get.return_value = '127.0.0.1'
+        self.relation_ids.return_value = 'cache:0'
+        self.related_units.return_value = 'memcached/0'
+        instance_console = context.InstanceConsoleContext()
+        os_release.return_value = 'icehouse'
+        self.assertEqual({'memcached_servers': ''},
+                         instance_console())
+
+    @mock.patch.object(utils, 'os_release')
+    @mock.patch('charmhelpers.contrib.network.ip.log')
+    def test_instance_console_context_with_memcache(self, os_release, log_):
+        self.check_instance_console_context_with_memcache(os_release,
+                                                          '127.0.1.1',
+                                                          '127.0.1.1')
+
+    @mock.patch.object(utils, 'os_release')
+    @mock.patch('charmhelpers.contrib.network.ip.log')
+    def test_instance_console_context_with_memcache_ipv6(self, os_release,
+                                                         log_):
+        self.check_instance_console_context_with_memcache(os_release, '::1',
+                                                          '[::1]')
+
+    def check_instance_console_context_with_memcache(self, os_release, ip,
+                                                     formated_ip):
+        memcached_servers = [{'private-address': formated_ip,
+                              'port': '11211'}]
+        self.unit_get.return_value = ip
+        self.relation_ids.return_value = ['cache:0']
+        self.relations_for_id.return_value = memcached_servers
+        self.related_units.return_value = 'memcached/0'
+        instance_console = context.InstanceConsoleContext()
+        os_release.return_value = 'icehouse'
+        self.maxDiff = None
+        self.assertEqual({'memcached_servers': "%s:11211" % (formated_ip, )},
+                         instance_console())

=== modified file 'unit_tests/test_nova_cc_hooks.py'
--- unit_tests/test_nova_cc_hooks.py	2014-10-21 13:18:47 +0000
+++ unit_tests/test_nova_cc_hooks.py	2014-12-16 14:10:29 +0000
@@ -1,6 +1,7 @@
 from mock import MagicMock, patch, call
 from test_utils import CharmTestCase, patch_open
 import os
+
 with patch('charmhelpers.core.hookenv.config') as config:
     config.return_value = 'neutron'
     import nova_cc_utils as utils