Merge lp:~mattrae/charm-helpers/lp1635067 into lp:charm-helpers

[Matt Rae]

Adding functions to create an ovs bridge and check if an interface is a linux bridge.

This is needed in charm-helpers in order to add support for using a linuxbridge bridge in data-port config of neutron-gateway and neutron-openvswitch charms, such as 'juju set neutron-openvswitch data-port="br-ex:br-eno1"' where br-eno1 is the bridge created on eno1 by juju by default.

https://review.openstack.org/#/c/392212/

[Jorge Niedbalski (niedbalski)]

LGTM, however I am missing unit tests for the change.

review: Needs Fixing

[lp:~mattrae/charm-helpers/lp1635067]

673. By Matt Rae on 2017-01-13

adding tests for add_ovsbridge_linuxbridge

[Matt Rae (mattrae)]

Thanks Jorge, I added a unit test for add_ovsbridge_linuxbridge. I'm not exactly sure how to add a test for is_linuxbridge_interface, but it might be trival enough that it doesn't need testing. let me know if i should test is_linuxbridge_interface as well.

[lp:~mattrae/charm-helpers/lp1635067]

674. By Matt Rae on 2017-01-27

changing is_linuxbridge_interface to use os.path.exists and adding tests

675. By Matt Rae on 2017-01-28

cleanup formatting

676. By Matt Rae on 2017-01-28

updating add_ovsbridge_linuxbridge to be idempotent

677. By Matt Rae on 2017-01-30

adding levels to log statements

678. By Matt Rae on 2017-02-05

making veth persistent by adding to interfaces.d

679. By Matt Rae on 2017-02-05

swap names of veth pair to make debugging easier

680. By Matt Rae on 2017-02-05

dropping log level to DEBUG

681. By Matt Rae on 2017-03-03

documenting functions and removing unneeded close()

Unmerged revisions

=== modified file 'charmhelpers/contrib/network/ovs/__init__.py'
--- charmhelpers/contrib/network/ovs/__init__.py	2016-07-06 14:41:05 +0000
+++ charmhelpers/contrib/network/ovs/__init__.py	2017-01-13 23:44:05 +0000
@@ -15,6 +15,7 @@
 ''' Helpers for interacting with OpenvSwitch '''
 import subprocess
 import os
+import glob
 from charmhelpers.core.hookenv import (
     log, WARNING
 )
@@ -60,6 +61,34 @@
     subprocess.check_call(["ip", "link", "set", port, "promisc", "off"])
 
 
+def add_ovsbridge_linuxbridge(name, bridge):
+    ''' Add linux bridge to the named openvswitch bridge '''
+    ovsbridge_port = "veth-" + name
+    linuxbridge_port = "veth-" + bridge
+    log('Adding linuxbridge {} to ovsbridge {}'.format(bridge, name))
+    try:
+        subprocess.check_call(["ip", "link", "add", "name", linuxbridge_port,
+                               "type", "veth", "peer", "name",
+                               ovsbridge_port])
+        subprocess.check_call(["ip", "link", "set", ovsbridge_port, "up"])
+        subprocess.check_call(["ip", "link", "set", linuxbridge_port, "up"])
+        subprocess.check_call(["ip", "link", "set", linuxbridge_port, "master",
+                               bridge])
+    except subprocess.CalledProcessError:
+        log('Interfaces {} and {} already exist'.format(ovsbridge_port,
+                                                        linuxbridge_port))
+        return
+    add_bridge_port(name, ovsbridge_port)
+
+
+def is_linuxbridge_interface(port):
+    ''' Check if the interface is a linuxbridge bridge '''
+    for sdir in glob.glob('/'.join(['/sys/class/net', port, '*'])):
+        if 'bridge' in sdir.split("/")[-1]:
+            return True
+    return False
+
+
 def set_manager(manager):
     ''' Set the controller for the local openvswitch '''
     log('Setting manager for local ovs to {}'.format(manager))

=== modified file 'tests/contrib/network/test_ovs.py'
--- tests/contrib/network/test_ovs.py	2016-03-23 10:55:08 +0000
+++ tests/contrib/network/test_ovs.py	2017-01-13 23:44:05 +0000
@@ -172,6 +172,23 @@
 
     @patch.object(ovs, 'log')
     @patch('subprocess.check_call')
+    def test_add_ovsbridge_linuxbridge(self, check_call, log):
+        ovs.add_ovsbridge_linuxbridge('br-ex', 'br-eno1')
+        check_call.assert_has_calls([
+            call(["ip", "link", "add", "name", "veth-br-eno1",
+                  "type", "veth", "peer", "name", "veth-br-ex"]),
+            call(["ip", "link", "set", "veth-br-ex", "up"]),
+            call(["ip", "link", "set", "veth-br-eno1", "up"]),
+            call(["ip", "link", "set", "veth-br-eno1", "master", "br-eno1"]),
+            call(["ovs-vsctl", "--", "--may-exist", "add-port",
+                  'br-ex', 'veth-br-ex']),
+            call(['ip', 'link', 'set', 'veth-br-ex', 'up']),
+            call(['ip', 'link', 'set', 'veth-br-ex', 'promisc', 'off'])
+        ])
+        self.assertTrue(log.call_count == 2)
+
+    @patch.object(ovs, 'log')
+    @patch('subprocess.check_call')
     def test_set_manager(self, check_call, log):
         ovs.set_manager('manager')
         check_call.assert_called_with(['ovs-vsctl', 'set-manager',