Merge lp:~gnuoy/charms/trusty/neutron-openvswitch/local-metadata into lp:~openstack-charmers-archive/charms/trusty/neutron-openvswitch/next

[Liam Young]

[James Page (james-page)]

Minor niggles - LGTM

review: Approve

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

charm_lint_check #10043 neutron-openvswitch-next for gnuoy mp270416
    LINT FAIL: lint-test failed
    LINT FAIL: charm-proof failed

LINT Results (max last 2 lines):
make: *** [lint] Error 100
ERROR:root:Make target returned non-zero.

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

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

charm_unit_test #9209 neutron-openvswitch-next for gnuoy mp270416
    UNIT OK: passed

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

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

charm_amulet_test #6429 neutron-openvswitch-next for gnuoy mp270416
    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/12415978/
Build: http://10.245.162.77:8080/job/charm_amulet_test/6429/

=== modified file 'config.yaml'
--- config.yaml	2015-08-18 21:18:45 +0000
+++ config.yaml	2015-09-12 09:04:08 +0000
@@ -98,4 +98,10 @@
       traffic to the external public network. Valid values are either MAC
       addresses (in which case only MAC addresses for interfaces without an IP
       address already assigned will be used), or interfaces (eth0)
-
+  enable-local-dhcp-and-metadata:
+    type: boolean
+    default: false
+    description: |
+      Enable a local dhcp server which also serves metadata. Useful for
+      deployments which do not include a neutron-gateway. This is a good option
+      for VLAN provider network management.

=== modified file 'hooks/neutron_ovs_context.py'
--- hooks/neutron_ovs_context.py	2015-04-15 14:08:36 +0000
+++ hooks/neutron_ovs_context.py	2015-09-12 09:04:08 +0000
@@ -108,10 +108,11 @@
     return secret
 
 
-class DVRSharedSecretContext(OSContextGenerator):
+class SharedSecretContext(OSContextGenerator):
 
     def __call__(self):
-        if NeutronAPIContext()()['enable_dvr']:
+        if NeutronAPIContext()()['enable_dvr'] or \
+                config('enable-local-dhcp-and-metadata'):
             ctxt = {
                 'shared_secret': get_shared_secret(),
                 'local_ip': resolve_address(),

=== modified file 'hooks/neutron_ovs_hooks.py'
--- hooks/neutron_ovs_hooks.py	2015-04-02 20:35:37 +0000
+++ hooks/neutron_ovs_hooks.py	2015-09-12 09:04:08 +0000
@@ -20,25 +20,24 @@
     restart_on_change
 )
 
-from charmhelpers.fetch import (
-    apt_install, apt_update, apt_purge
-)
-
 from charmhelpers.contrib.openstack.utils import (
     os_requires_version,
 )
 
 from neutron_ovs_utils import (
+    DHCP_PACKAGES,
     DVR_PACKAGES,
     configure_ovs,
-    determine_packages,
     git_install,
     get_topics,
-    determine_dvr_packages,
     get_shared_secret,
     register_configs,
     restart_map,
     use_dvr,
+    enable_nova_metadata,
+    enable_local_dhcp,
+    install_packages,
+    purge_packages,
 )
 
 hooks = Hooks()
@@ -47,11 +46,7 @@
 
 @hooks.hook()
 def install():
-    apt_update()
-    pkgs = determine_packages()
-    for pkg in pkgs:
-        apt_install(pkg, fatal=True)
-
+    install_packages()
     git_install(config('openstack-origin-git'))
 
 
@@ -59,10 +54,7 @@
 @hooks.hook('config-changed')
 @restart_on_change(restart_map())
 def config_changed():
-    if determine_dvr_packages():
-        apt_update()
-        apt_install(determine_dvr_packages(), fatal=True)
-
+    install_packages()
     if git_install_requested():
         if config_value_changed('openstack-origin-git'):
             git_install(config('openstack-origin-git'))
@@ -71,16 +63,17 @@
     CONFIGS.write_all()
     for rid in relation_ids('zeromq-configuration'):
         zeromq_configuration_relation_joined(rid)
+    for rid in relation_ids('neutron-plugin'):
+        neutron_plugin_joined(relation_id=rid)
 
 
 @hooks.hook('neutron-plugin-api-relation-changed')
 @restart_on_change(restart_map())
 def neutron_plugin_api_changed():
     if use_dvr():
-        apt_update()
-        apt_install(DVR_PACKAGES, fatal=True)
+        install_packages()
     else:
-        apt_purge(DVR_PACKAGES, fatal=True)
+        purge_packages(DVR_PACKAGES)
     configure_ovs()
     CONFIGS.write_all()
     # If dvr setting has changed, need to pass that on
@@ -90,7 +83,11 @@
 
 @hooks.hook('neutron-plugin-relation-joined')
 def neutron_plugin_joined(relation_id=None):
-    secret = get_shared_secret() if use_dvr() else None
+    if enable_local_dhcp():
+        install_packages()
+    else:
+        purge_packages(DHCP_PACKAGES)
+    secret = get_shared_secret() if enable_nova_metadata() else None
     rel_data = {
         'metadata-shared-secret': secret,
     }

=== modified file 'hooks/neutron_ovs_utils.py'
--- hooks/neutron_ovs_utils.py	2015-07-14 19:52:15 +0000
+++ hooks/neutron_ovs_utils.py	2015-09-12 09:04:08 +0000
@@ -43,6 +43,13 @@
 
 from charmhelpers.core.templating import render
 
+from charmhelpers.fetch import (
+    apt_install,
+    apt_purge,
+    apt_update,
+    filter_installed_packages,
+)
+
 BASE_GIT_PACKAGES = [
     'libffi-dev',
     'libssl-dev',
@@ -66,6 +73,7 @@
 ]
 
 NOVA_CONF_DIR = "/etc/nova"
+NEUTRON_DHCP_AGENT_CONF = "/etc/neutron/dhcp_agent.ini"
 NEUTRON_CONF_DIR = "/etc/neutron"
 NEUTRON_CONF = '%s/neutron.conf' % NEUTRON_CONF_DIR
 NEUTRON_DEFAULT = '/etc/default/neutron-server'
@@ -75,6 +83,7 @@
 EXT_PORT_CONF = '/etc/init/ext-port.conf'
 NEUTRON_METADATA_AGENT_CONF = "/etc/neutron/metadata_agent.ini"
 DVR_PACKAGES = ['neutron-l3-agent']
+DHCP_PACKAGES = ['neutron-metadata-agent', 'neutron-dhcp-agent']
 PHY_NIC_MTU_CONF = '/etc/init/os-charm-phy-nic-mtu.conf'
 TEMPLATES = 'templates/'
 
@@ -95,6 +104,19 @@
         'contexts': [context.PhyNICMTUContext()],
     }),
 ])
+METADATA_RESOURCE_MAP = OrderedDict([
+    (NEUTRON_METADATA_AGENT_CONF, {
+        'services': ['neutron-metadata-agent'],
+        'contexts': [neutron_ovs_context.SharedSecretContext(),
+                     neutron_ovs_context.APIIdentityServiceContext()],
+    }),
+])
+DHCP_RESOURCE_MAP = OrderedDict([
+    (NEUTRON_DHCP_AGENT_CONF, {
+        'services': ['neutron-dhcp-agent'],
+        'contexts': [],
+    }),
+])
 DVR_RESOURCE_MAP = OrderedDict([
     (NEUTRON_L3_AGENT_CONF, {
         'services': ['neutron-l3-agent'],
@@ -108,11 +130,6 @@
         'services': ['neutron-l3-agent'],
         'contexts': [context.ExternalPortContext()],
     }),
-    (NEUTRON_METADATA_AGENT_CONF, {
-        'services': ['neutron-metadata-agent'],
-        'contexts': [neutron_ovs_context.DVRSharedSecretContext(),
-                     neutron_ovs_context.APIIdentityServiceContext()],
-    }),
 ])
 TEMPLATES = 'templates/'
 INT_BRIDGE = "br-int"
@@ -120,16 +137,30 @@
 DATA_BRIDGE = 'br-data'
 
 
-def determine_dvr_packages():
-    if not git_install_requested():
-        if use_dvr():
-            return DVR_PACKAGES
-    return []
+def install_packages():
+    apt_update()
+    apt_install(filter_installed_packages(determine_packages()))
+
+
+def purge_packages(pkg_list):
+    purge_pkgs = []
+    required_packages = determine_packages()
+    for pkg in pkg_list:
+        if pkg not in required_packages:
+            purge_pkgs.append(pkg)
+    if purge_pkgs:
+        apt_purge(purge_pkgs, fatal=True)
 
 
 def determine_packages():
-    pkgs = neutron_plugin_attribute('ovs', 'packages', 'neutron')
-    pkgs.extend(determine_dvr_packages())
+    pkgs = []
+    plugin_pkgs = neutron_plugin_attribute('ovs', 'packages', 'neutron')
+    for plugin_pkg in plugin_pkgs:
+        pkgs.extend(plugin_pkg)
+    if use_dvr():
+        pkgs.extend(DVR_PACKAGES)
+    if enable_local_dhcp():
+        pkgs.extend(DHCP_PACKAGES)
 
     if git_install_requested():
         pkgs.extend(BASE_GIT_PACKAGES)
@@ -158,8 +189,14 @@
     resource_map = deepcopy(BASE_RESOURCE_MAP)
     if use_dvr():
         resource_map.update(DVR_RESOURCE_MAP)
+        resource_map.update(METADATA_RESOURCE_MAP)
         dvr_services = ['neutron-metadata-agent', 'neutron-l3-agent']
         resource_map[NEUTRON_CONF]['services'] += dvr_services
+    if enable_local_dhcp():
+        resource_map.update(METADATA_RESOURCE_MAP)
+        resource_map.update(DHCP_RESOURCE_MAP)
+        metadata_services = ['neutron-metadata-agent', 'neutron-dhcp-agent']
+        resource_map[NEUTRON_CONF]['services'] += metadata_services
     return resource_map
 
 
@@ -209,7 +246,7 @@
 
 
 def get_shared_secret():
-    ctxt = neutron_ovs_context.DVRSharedSecretContext()()
+    ctxt = neutron_ovs_context.SharedSecretContext()()
     if 'shared_secret' in ctxt:
         return ctxt['shared_secret']
 
@@ -218,6 +255,14 @@
     return context.NeutronAPIContext()()['enable_dvr']
 
 
+def enable_nova_metadata():
+    return use_dvr() or enable_local_dhcp()
+
+
+def enable_local_dhcp():
+    return config('enable-local-dhcp-and-metadata')
+
+
 def git_install(projects_yaml):
     """Perform setup, and install git repos specified in yaml parameter."""
     if git_install_requested():

=== added file 'templates/icehouse/dhcp_agent.ini'
--- templates/icehouse/dhcp_agent.ini	1970-01-01 00:00:00 +0000
+++ templates/icehouse/dhcp_agent.ini	2015-09-12 09:04:08 +0000
@@ -0,0 +1,14 @@
+###############################################################################
+# [ WARNING ]
+# Configuration file maintained by Juju. Local changes may be overwritten.
+###############################################################################
+[DEFAULT]
+state_path = /var/lib/neutron
+interface_driver = neutron.agent.linux.interface.OVSInterfaceDriver
+dhcp_driver = neutron.agent.linux.dhcp.Dnsmasq
+root_helper = sudo /usr/bin/neutron-rootwrap /etc/neutron/rootwrap.conf
+
+enable_metadata_network = True
+enable_isolated_metadata = True
+
+ovs_use_veth = True

=== modified file 'unit_tests/test_neutron_ovs_context.py'
--- unit_tests/test_neutron_ovs_context.py	2015-08-18 21:18:45 +0000
+++ unit_tests/test_neutron_ovs_context.py	2015-09-12 09:04:08 +0000
@@ -250,11 +250,11 @@
         self.assertEquals(context.L3AgentContext()(), {'agent_mode': 'legacy'})
 
 
-class DVRSharedSecretContext(CharmTestCase):
+class SharedSecretContext(CharmTestCase):
 
     def setUp(self):
-        super(DVRSharedSecretContext, self).setUp(context,
-                                                  TO_PATCH)
+        super(SharedSecretContext, self).setUp(context,
+                                               TO_PATCH)
         self.config.side_effect = self.test_config.get
 
     @patch('os.path')
@@ -286,7 +286,7 @@
         _NeutronAPIContext.side_effect = fake_context({'enable_dvr': True})
         _shared_secret.return_value = 'secret_thing'
         self.resolve_address.return_value = '10.0.0.10'
-        self.assertEquals(context.DVRSharedSecretContext()(),
+        self.assertEquals(context.SharedSecretContext()(),
                           {'shared_secret': 'secret_thing',
                            'local_ip': '10.0.0.10'})
 
@@ -297,4 +297,4 @@
         _NeutronAPIContext.side_effect = fake_context({'enable_dvr': False})
         _shared_secret.return_value = 'secret_thing'
         self.resolve_address.return_value = '10.0.0.10'
-        self.assertEquals(context.DVRSharedSecretContext()(), {})
+        self.assertEquals(context.SharedSecretContext()(), {})

=== modified file 'unit_tests/test_neutron_ovs_hooks.py'
--- unit_tests/test_neutron_ovs_hooks.py	2015-04-02 20:35:37 +0000
+++ unit_tests/test_neutron_ovs_hooks.py	2015-09-12 09:04:08 +0000
@@ -1,4 +1,4 @@
-from mock import MagicMock, patch, call
+from mock import MagicMock, patch
 import yaml
 
 from test_utils import CharmTestCase
@@ -19,13 +19,8 @@
 utils.restart_map = _map
 
 TO_PATCH = [
-    'apt_update',
-    'apt_install',
-    'apt_purge',
     'config',
     'CONFIGS',
-    'determine_packages',
-    'determine_dvr_packages',
     'get_shared_secret',
     'git_install',
     'log',
@@ -33,6 +28,10 @@
     'relation_set',
     'configure_ovs',
     'use_dvr',
+    'install_packages',
+    'purge_packages',
+    'enable_nova_metadata',
+    'enable_local_dhcp',
 ]
 NEUTRON_CONF_DIR = "/etc/neutron"
 
@@ -54,19 +53,12 @@
     @patch.object(hooks, 'git_install_requested')
     def test_install_hook(self, git_requested):
         git_requested.return_value = False
-        _pkgs = ['foo', 'bar']
-        self.determine_packages.return_value = [_pkgs]
         self._call_hook('install')
-        self.apt_update.assert_called_with()
-        self.apt_install.assert_has_calls([
-            call(_pkgs, fatal=True),
-        ])
+        self.install_packages.assert_called_with()
 
     @patch.object(hooks, 'git_install_requested')
     def test_install_hook_git(self, git_requested):
         git_requested.return_value = True
-        _pkgs = ['foo', 'bar']
-        self.determine_packages.return_value = _pkgs
         openstack_origin_git = {
             'repositories': [
                 {'name': 'requirements',
@@ -81,8 +73,7 @@
         projects_yaml = yaml.dump(openstack_origin_git)
         self.test_config.set('openstack-origin-git', projects_yaml)
         self._call_hook('install')
-        self.apt_update.assert_called_with()
-        self.assertTrue(self.determine_packages)
+        self.install_packages.assert_called_with()
         self.git_install.assert_called_with(projects_yaml)
 
     @patch.object(hooks, 'git_install_requested')
@@ -124,13 +115,9 @@
     @patch.object(hooks, 'git_install_requested')
     def test_config_changed_dvr(self, git_requested):
         git_requested.return_value = False
-        self.determine_dvr_packages.return_value = ['dvr']
         self._call_hook('config-changed')
-        self.apt_update.assert_called_with()
+        self.install_packages.assert_called_with()
         self.assertTrue(self.CONFIGS.write_all.called)
-        self.apt_install.assert_has_calls([
-            call(['dvr'], fatal=True),
-        ])
         self.configure_ovs.assert_called_with()
 
     @patch.object(hooks, 'neutron_plugin_joined')
@@ -140,9 +127,22 @@
         self.configure_ovs.assert_called_with()
         self.assertTrue(self.CONFIGS.write_all.called)
         _plugin_joined.assert_called_with(relation_id='rid')
+        self.install_packages.assert_called_with()
+
+    @patch.object(hooks, 'neutron_plugin_joined')
+    def test_neutron_plugin_api_nodvr(self, _plugin_joined):
+        self.use_dvr.return_value = False
+        self.relation_ids.return_value = ['rid']
+        self._call_hook('neutron-plugin-api-relation-changed')
+        self.configure_ovs.assert_called_with()
+        self.assertTrue(self.CONFIGS.write_all.called)
+        _plugin_joined.assert_called_with(relation_id='rid')
+        self.purge_packages.assert_called_with(['neutron-l3-agent'])
 
     @patch.object(hooks, 'git_install_requested')
     def test_neutron_plugin_joined(self, git_requested):
+        self.enable_nova_metadata.return_value = True
+        self.enable_local_dhcp.return_value = True
         git_requested.return_value = False
         self.get_shared_secret.return_value = 'secret'
         self._call_hook('neutron-plugin-relation-joined')

=== modified file 'unit_tests/test_neutron_ovs_utils.py'
--- unit_tests/test_neutron_ovs_utils.py	2015-05-07 18:48:58 +0000
+++ unit_tests/test_neutron_ovs_utils.py	2015-09-12 09:04:08 +0000
@@ -18,8 +18,11 @@
 TO_PATCH = [
     'add_bridge',
     'add_bridge_port',
+    'apt_install',
+    'apt_update',
     'config',
     'os_release',
+    'filter_installed_packages',
     'neutron_plugin_attribute',
     'full_restart',
     'service_restart',
@@ -75,19 +78,57 @@
         # Reset cached cache
         hookenv.cache = {}
 
+    @patch.object(nutils, 'determine_packages')
+    def test_install_packages(self, _determine_packages):
+        _determine_packages.return_value = 'randompkg'
+        nutils.install_packages()
+        self.apt_update.assert_called_with()
+        self.apt_install.assert_called_with(self.filter_installed_packages())
+
     @patch.object(nutils, 'use_dvr')
     @patch.object(nutils, 'git_install_requested')
     @patch.object(charmhelpers.contrib.openstack.neutron, 'os_release')
     @patch.object(charmhelpers.contrib.openstack.neutron, 'headers_package')
     def test_determine_packages(self, _head_pkgs, _os_rel, _git_requested,
                                 _use_dvr):
-        _git_requested.return_value = False
-        _use_dvr.return_value = False
-        _os_rel.return_value = 'trusty'
-        _head_pkgs.return_value = head_pkg
-        pkg_list = nutils.determine_packages()
-        expect = [['neutron-plugin-openvswitch-agent'], [head_pkg]]
-        self.assertItemsEqual(pkg_list, expect)
+        self.test_config.set('enable-local-dhcp-and-metadata', False)
+        _git_requested.return_value = False
+        _use_dvr.return_value = False
+        _os_rel.return_value = 'trusty'
+        _head_pkgs.return_value = head_pkg
+        pkg_list = nutils.determine_packages()
+        expect = ['neutron-plugin-openvswitch-agent', head_pkg]
+        self.assertItemsEqual(pkg_list, expect)
+
+    @patch.object(nutils, 'use_dvr')
+    @patch.object(nutils, 'git_install_requested')
+    @patch.object(charmhelpers.contrib.openstack.neutron, 'os_release')
+    @patch.object(charmhelpers.contrib.openstack.neutron, 'headers_package')
+    def test_determine_packages_metadata(self, _head_pkgs, _os_rel,
+                                         _git_requested, _use_dvr):
+        self.test_config.set('enable-local-dhcp-and-metadata', True)
+        _git_requested.return_value = False
+        _use_dvr.return_value = False
+        _os_rel.return_value = 'trusty'
+        _head_pkgs.return_value = head_pkg
+        pkg_list = nutils.determine_packages()
+        expect = ['neutron-plugin-openvswitch-agent', head_pkg,
+                  'neutron-metadata-agent', 'neutron-dhcp-agent']
+        self.assertItemsEqual(pkg_list, expect)
+
+    @patch.object(nutils, 'use_dvr')
+    @patch.object(nutils, 'git_install_requested')
+    @patch.object(charmhelpers.contrib.openstack.neutron, 'os_release')
+    @patch.object(charmhelpers.contrib.openstack.neutron, 'headers_package')
+    def test_determine_packages_git(self, _head_pkgs, _os_rel,
+                                    _git_requested, _use_dvr):
+        self.test_config.set('enable-local-dhcp-and-metadata', False)
+        _git_requested.return_value = True
+        _use_dvr.return_value = True
+        _os_rel.return_value = 'trusty'
+        _head_pkgs.return_value = head_pkg
+        pkg_list = nutils.determine_packages()
+        self.assertFalse('neutron-l3-agent' in pkg_list)
 
     @patch.object(nutils, 'use_dvr')
     def test_register_configs(self, _use_dvr):
@@ -128,6 +169,19 @@
         [self.assertIn(q_conf, _map.keys()) for q_conf in confs]
         self.assertEqual(_map[nutils.NEUTRON_CONF]['services'], svcs)
 
+    @patch.object(nutils, 'enable_local_dhcp')
+    @patch.object(nutils, 'use_dvr')
+    def test_resource_map_dhcp(self, _use_dvr, _enable_local_dhcp):
+        _enable_local_dhcp.return_value = True
+        _use_dvr.return_value = False
+        _map = nutils.resource_map()
+        svcs = ['neutron-plugin-openvswitch-agent', 'neutron-metadata-agent',
+                'neutron-dhcp-agent']
+        confs = [nutils.NEUTRON_CONF, nutils.NEUTRON_METADATA_AGENT_CONF,
+                 nutils.NEUTRON_DHCP_AGENT_CONF]
+        [self.assertIn(q_conf, _map.keys()) for q_conf in confs]
+        self.assertEqual(_map[nutils.NEUTRON_CONF]['services'], svcs)
+
     @patch.object(nutils, 'use_dvr')
     def test_restart_map(self, _use_dvr):
         _use_dvr.return_value = False
@@ -213,7 +267,7 @@
         ])
         self.add_bridge_port.assert_called_with('br-ex', 'eth0')
 
-    @patch.object(neutron_ovs_context, 'DVRSharedSecretContext')
+    @patch.object(neutron_ovs_context, 'SharedSecretContext')
     def test_get_shared_secret(self, _dvr_secret_ctxt):
         _dvr_secret_ctxt.return_value = \
             DummyContext(return_value={'shared_secret': 'supersecret'})