Merge lp:~sberg-l/charms/trusty/nova-compute/tintri-interface into lp:~openstack-charmers-archive/charms/trusty/nova-compute/next

[Skyler Berg]

Add an interface for cinder-tintri charm.

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

charm_lint_check #8427 nova-compute-next for sberg-l mp268680
    LINT OK: passed

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

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

charm_unit_test #7821 nova-compute-next for sberg-l mp268680
    UNIT OK: passed

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

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

charm_amulet_test #5934 nova-compute-next for sberg-l mp268680
    AMULET OK: passed

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

[Ryan Beisner (1chb1n)]

Thank you for your work on this.

Since we had been talking in #juju, I wanted to chime in with a few thoughts to help make your project reviews go as smoothly as possible.

I think that my colleagues will like to see and review this in a batch along side any other proposed changes to other charms, so that we can understand and review the changes in full context.

Until a more official review, some things to double-check as potential blockers:

# Config
While I've not taken a deep dive into this, I know that (generally speaking) many config options have been relocated or deprecated, depending on which OpenStack version is in use. So I'd recommend double-checking all of the relevant config options and template placement against the upstream docs, such as:
http://docs.openstack.org/kilo/config-reference/content/list-of-compute-config-options.html
http://docs.openstack.org/juno/config-reference/content/list-of-compute-config-options.html
http://docs.openstack.org/icehouse/config-reference/content/list-of-compute-config-options.html

A couple of questions that jumped out at me after having a quick peek:
Will you also need to enable a volume_driver for nfs in nova.conf?
Will you also need to specify a nfs_mount_point_base in nova.conf?

# Supported Releases
Take note that with each OpenStack charm, we develop and test with the expectation that each charm will succeed on all currently-supported releases. Currently, that is Icehouse, Juno, Kilo, with Liberty up next in active development. As a handy reference, this page has a chart that shows the supported release timeline:
https://wiki.ubuntu.com/ServerTeam/CloudArchive

# Contacts
As you know, you can generally reach one of us in #juju. For a-sync questions, the juju mailing list is also effective in drawing input/feedback with regard to general design questions or issues. I highly recommend joining that, as it's fairly low-traffic, and definitely has the right audience.

https://lists.ubuntu.com/mailman/listinfo/juju

Thanks again!

[Skyler Berg (sberg-l)]

Thanks for the feedback, Ryan!

nfs_mount_options was added to the [libvirt] section of nova.conf in Icehouse and has continued to exist in the all subsequent releases. As such, I added to the templates for all Icehouse, Juno, and Kilo.

I do not need any additional options other than nfs_mount_options.

As for other changes, I do not anticipate any. This change should be isolated. Soon I will be uploading my cinder-tintri charm. In that charm's README I will instruct users to set this option appropriately. However, the code in my charm will not touch nova-compute at all.

[Billy Olsen (billy-olsen)]

Ah this is where you were going with the general config options? From our discussions on IRC, I understood you wanted general config settings set rather than ultimately seeing it this way.

Although, this is certainly one option of solving it, I think you are probably better going down the path of creating a subordinate charm and adding a tintri relation interface. As a simple subordinate charm, it could easily relate to both the nova-compute charm (which would react to tintri-relation-* hooks) and to cinder (also reacting to tintri-relation-* hooks).

For example, let's go with the idea of a tintri-storage charm which is a subordinate. The subordinate means it will be colocated with each unit it is paired with. In this charm, there would be the options for connecting to the tintri storage device (e.g. username, password, ip address, etc). It would provide the tintri interface, which shares access information to the related service (e.g. on the relation it would share the ip, password, etc). Then the nova-compute charm would be augmented to understand the tintri interface. On the tintri-changed-* hook it would read the relation values and configure the local storage access and any nova-compute parameters needed (ideally a new TintriContext or NFSContext etc would be created here), which would also include the NFS mount options etc.

I'm going to mark this as needs fixing as I think you really (ultimately) want what I've outlined here. That is to give the user one place to configure the tintri settings for the environment, and then let the charm relations handle the rest for you.

Ping me (wolsen) on irc if you want to discuss this any further or have questions. I'll be more than happy to help out!

review: Needs Fixing

[Billy Olsen (billy-olsen)]

btw, when it is ready for re-review/re-submission - change the status and add the openstack-charmers to make sure it goes into the general review queue and not just to me.

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

charm_lint_check #9245 nova-compute-next for sberg-l mp269972
    LINT OK: passed

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

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

charm_unit_test #8546 nova-compute-next for sberg-l mp269972
    UNIT OK: passed

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

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

charm_amulet_test #6206 nova-compute-next for sberg-l mp269972
    AMULET OK: passed

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

[Billy Olsen (billy-olsen)]

Skyler, this is definitely down the track of we discussed! Per Ryan's comment above, it'd be quite helpful to see the suite of charms to measure this in the broader context (and to be able to possibly test it), though I think I have a generally good sense of it from our discussions and openstack docs.

A couple of minor comments inline.

Before this gets merged, we need to make sure the tintri subordinate charm is promulgated. Does it exist out there to be able to look at?

review: Needs Fixing

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

charm_lint_check #9296 nova-compute-next for sberg-l mp269998
    LINT OK: passed

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

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

charm_unit_test #8594 nova-compute-next for sberg-l mp269998
    UNIT OK: passed

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

[Skyler Berg (sberg-l)]

I uploaded new commits that address your comments. I also just uploaded the cinder-tintri charm. Everything in it should be good to go, except it does not have a test for it's interface with nova because that would depend on this change.

https://code.launchpad.net/~sberg-l/charms/trusty/cinder-tintri/trunk

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

charm_amulet_test #6209 nova-compute-next for sberg-l mp269998
    AMULET FAIL: amulet-test failed

AMULET Results (max last 2 lines):
make: *** [test] Error 1
ERROR:root:Make target returned non-zero.

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

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

charm_amulet_test #6210 nova-compute-next for sberg-l mp269998
    AMULET OK: passed

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

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

charm_lint_check #9329 nova-compute-next for sberg-l mp269998
    LINT OK: passed

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

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

charm_unit_test #8627 nova-compute-next for sberg-l mp269998
    UNIT OK: passed

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

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

charm_amulet_test #6239 nova-compute-next for sberg-l mp269998
    AMULET FAIL: amulet-test failed

AMULET Results (max last 2 lines):
make: *** [test] Error 1
ERROR:root:Make target returned non-zero.

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

[Ryan Beisner (1chb1n)]

Safe to ignore amulet test fail $6239, undercloud woes. Will retrigger test.

juju-test.conductor.050-basic-trusty-icehouse-git DEBUG : Killed by timeout after 2700 seconds

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

charm_amulet_test #6254 nova-compute-next for sberg-l mp269998
    AMULET OK: passed

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

[James Page (james-page)]

Hi Skyler

Thanks for your merge proposal; I've been reflecting on the specificity of the new relation being added to this charm.

I think the objective here is to allow a cinder storage option to plug additional configuration into the nova compute service; so maybe the name and type should be generic, rather than specific.

I think that the name and type of the relation should be 'nova-cinder' - that way, any other storage option that comes along and needs todo something similar has a generic relation to plugin to.

Apart from that design change, I think you change is just fine - would you be OK to rework?

Thanks

James

review: Needs Fixing

Unmerged revisions

156. By Skyler Berg on 2015-09-03

Fix missing name change for "tintri-nova"

155. By Skyler Berg on 2015-09-03

Address review concerns

Rename "tintri-nova-configuration" to "tintri-nova". Only write nova.conf, not
all files. Clean up on relationship departed.

154. By Skyler Berg on 2015-09-02

Add tintri-nova-configuration interface

To allow the cinder-tintri charm to properly set nova's NFS options, we need a
new interface in this charm. This commit add such an interface. Additionally,
this commit updates nova.conf to allow arbitrary values to be set in the
libvirt section. This is necessary for the cinder-tintri charm.

=== modified file 'hooks/nova_compute_hooks.py'
--- hooks/nova_compute_hooks.py	2015-08-19 15:07:10 +0000
+++ hooks/nova_compute_hooks.py	2015-09-03 00:30:30 +0000
@@ -385,6 +385,13 @@
     CONFIGS.write(NOVA_CONF)
 
 
[e-mail]('tintri-nova-relation-changed')
[e-mail]('tintri-nova-relation-departed')
+@restart_on_change(restart_map())
+def tintri_nova_relation_changed():
+    CONFIGS.write(NOVA_CONF)
+
+
 def main():
     try:
         hooks.execute(sys.argv)

=== modified file 'hooks/nova_compute_utils.py'
--- hooks/nova_compute_utils.py	2015-08-13 14:39:13 +0000
+++ hooks/nova_compute_utils.py	2015-09-03 00:30:30 +0000
@@ -163,7 +163,9 @@
                      NovaComputeCephContext(),
                      context.SyslogContext(),
                      context.SubordinateConfigContext(
-                         interface=['neutron-plugin', 'nova-ceilometer'],
+                         interface=['neutron-plugin',
+                                    'nova-ceilometer',
+                                    'tintri-nova'],
                          service=['nova-compute', 'nova'],
                          config_file=NOVA_CONF),
                      InstanceConsoleContext(),

=== added symlink 'hooks/tintri-nova-relation-changed'
=== target is u'nova_compute_hooks.py'
=== added symlink 'hooks/tintri-nova-relation-departed'
=== target is u'nova_compute_hooks.py'
=== added symlink 'hooks/tintri-nova-relation-joined'
=== target is u'nova_compute_hooks.py'
=== modified file 'metadata.yaml'
--- metadata.yaml	2015-04-13 12:46:16 +0000
+++ metadata.yaml	2015-09-03 00:30:30 +0000
@@ -33,6 +33,9 @@
   zeromq-configuration:
     interface: zeromq-configuration
     scope: container
+  tintri-nova:
+    interface: tintri-nova
+    scope: container
 peers:
   compute-peer:
     interface: nova

=== modified file 'revision'
--- revision	2014-06-02 18:37:32 +0000
+++ revision	2015-09-03 00:30:30 +0000
@@ -1,1 +1,1 @@
-133
+134

=== modified file 'templates/icehouse/nova.conf'
--- templates/icehouse/nova.conf	2015-08-25 17:40:00 +0000
+++ templates/icehouse/nova.conf	2015-09-03 00:30:30 +0000
@@ -157,4 +157,8 @@
 {% if disk_cachemodes -%}
 disk_cachemodes = {{ disk_cachemodes }}
 {% endif -%}
-
+{% if sections and 'libvirt' in sections -%}
+{% for key, value in sections['libvirt'] -%}
+{{ key }} = {{ value }}
+{% endfor -%}
+{% endif -%}

=== modified file 'templates/juno/nova.conf'
--- templates/juno/nova.conf	2015-08-25 17:40:00 +0000
+++ templates/juno/nova.conf	2015-09-03 00:30:30 +0000
@@ -161,3 +161,9 @@
 {% if disk_cachemodes -%}
 disk_cachemodes = {{ disk_cachemodes }}
 {% endif -%}
+{% if sections and 'libvirt' in sections -%}
+{% for key, value in sections['libvirt'] -%}
+{{ key }} = {{ value }}
+{% endfor -%}
+{% endif -%}
+

=== modified file 'templates/kilo/nova.conf'
--- templates/kilo/nova.conf	2015-08-25 17:40:00 +0000
+++ templates/kilo/nova.conf	2015-09-03 00:30:30 +0000
@@ -166,6 +166,11 @@
 {% if disk_cachemodes -%}
 disk_cachemodes = {{ disk_cachemodes }}
 {% endif -%}
+{% if sections and 'libvirt' in sections -%}
+{% for key, value in sections['libvirt'] -%}
+{{ key }} = {{ value }}
+{% endfor -%}
+{% endif -%}
 
 {% include "parts/section-database" %}