Change in vdsm[ovirt-3.5]: openstacknet: Fix migration when using security groups

danken at redhat.com danken at redhat.com
Mon Dec 8 12:59:33 UTC 2014


Hello Antoni Segura Puimedon, Francesco Romani,

I'd like you to do a code review.  Please visit

    http://gerrit.ovirt.org/35961

to review the following change.

Change subject: openstacknet: Fix migration when using security groups
......................................................................

openstacknet: Fix migration when using security groups

When migrating, the destination libvirt receives an xml already
altered by before_device_create like this test one:

    <?xml version="1.0" encoding="utf-8"?>
    <interface type="bridge">
        <mac address="00:1a:4a:16:01:51"/>
        <model type="virtio"/>
        <source bridge="qbrtest_port_i"/>
        <target dev="taptest_port_i"/>
    </interface>

The issue is that before_device_create hooking point is not part
of the migration and it is the only thing that creates the security
groups bridge and the necessary veths. In order to have migration
working, then, it was necessary to add a hook that on the hooking
point 'before_device_migrate' does the security groups bridge and
veths creation.

Change-Id: Icd8a789c4565f32b32965af3966a4edd361949ea
Bug-Url: https://bugzilla.redhat.com/1048880
Signed-off-by: Antoni S. Puimedon <asegurap at redhat.com>
Reviewed-on: http://gerrit.ovirt.org/34406
Reviewed-by: Francesco Romani <fromani at redhat.com>
Reviewed-by: Dan Kenigsberg <danken at redhat.com>
---
M debian/vdsm-hook-openstacknet.install
M vdsm.spec.in
M vdsm_hooks/openstacknet/Makefile.am
M vdsm_hooks/openstacknet/before_device_create.py
A vdsm_hooks/openstacknet/before_device_migrate_destination.py
M vdsm_hooks/openstacknet/openstacknet_utils.py
6 files changed, 142 insertions(+), 47 deletions(-)


  git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/61/35961/1

diff --git a/debian/vdsm-hook-openstacknet.install b/debian/vdsm-hook-openstacknet.install
index 8de9b50..4e6a2ca 100644
--- a/debian/vdsm-hook-openstacknet.install
+++ b/debian/vdsm-hook-openstacknet.install
@@ -11,5 +11,7 @@
 usr/libexec/vdsm/hooks/after_nic_hotunplug/openstacknet_utils.py
 usr/libexec/vdsm/hooks/before_device_create/50_openstacknet
 usr/libexec/vdsm/hooks/before_device_create/openstacknet_utils.py
+usr/libexec/vdsm/hooks/before_device_migrate_destination/50_openstacknet
+usr/libexec/vdsm/hooks/before_device_migrate_destination/openstacknet_utils.py
 usr/libexec/vdsm/hooks/before_nic_hotplug/50_openstacknet
 usr/libexec/vdsm/hooks/before_nic_hotplug/openstacknet_utils.py
diff --git a/vdsm.spec.in b/vdsm.spec.in
index 57c8bc1..7b7c669 100644
--- a/vdsm.spec.in
+++ b/vdsm.spec.in
@@ -1307,6 +1307,8 @@
 %{_libexecdir}/%{vdsm_name}/hooks/after_nic_hotunplug/openstacknet_utils.py*
 %{_libexecdir}/%{vdsm_name}/hooks/before_device_create/50_openstacknet
 %{_libexecdir}/%{vdsm_name}/hooks/before_device_create/openstacknet_utils.py*
+%{_libexecdir}/%{vdsm_name}/hooks/before_device_migrate_destination/50_openstacknet
+%{_libexecdir}/%{vdsm_name}/hooks/before_device_migrate_destination/openstacknet_utils.py*
 %{_libexecdir}/%{vdsm_name}/hooks/before_nic_hotplug/50_openstacknet
 %{_libexecdir}/%{vdsm_name}/hooks/before_nic_hotplug/openstacknet_utils.py*
 
diff --git a/vdsm_hooks/openstacknet/Makefile.am b/vdsm_hooks/openstacknet/Makefile.am
index 2baab05..da3e470 100644
--- a/vdsm_hooks/openstacknet/Makefile.am
+++ b/vdsm_hooks/openstacknet/Makefile.am
@@ -32,6 +32,7 @@
 	after_device_create.py \
 	after_device_destroy.py \
 	before_device_create.py \
+	before_device_migrate_destination.py \
 	$(constsfile) \
 	sudoers.in
 
@@ -57,6 +58,9 @@
 	$(MKDIR_P) $(DESTDIR)$(vdsmhooksdir)/before_device_create
 	$(INSTALL_SCRIPT) $(srcdir)/before_device_create.py \
 		$(DESTDIR)$(vdsmhooksdir)/before_device_create/50_openstacknet
+	$(MKDIR_P) $(DESTDIR)$(vdsmhooksdir)/before_device_migrate_destination
+	$(INSTALL_SCRIPT) $(srcdir)/before_device_migrate_destination.py \
+		$(DESTDIR)$(vdsmhooksdir)/before_device_migrate_destination/50_openstacknet
 	$(MKDIR_P) $(DESTDIR)$(vdsmhooksdir)/before_nic_hotplug
 	$(INSTALL_SCRIPT) $(srcdir)/before_device_create.py \
 		$(DESTDIR)$(vdsmhooksdir)/before_nic_hotplug/50_openstacknet
@@ -68,6 +72,7 @@
 	$(RM) $(DESTDIR)$(vdsmhooksdir)/after_nic_hotplug/50_openstacknet
 	$(RM) $(DESTDIR)$(vdsmhooksdir)/after_nic_hotunplug/50_openstacknet
 	$(RM) $(DESTDIR)$(vdsmhooksdir)/before_device_create/50_openstacknet
+	$(RM) $(DESTDIR)$(vdsmhooksdir)/before_device_migrate_destination/50_openstacknet
 	$(RM) $(DESTDIR)$(vdsmhooksdir)/before_nic_hotplug/50_openstacknet
 
 install-data-consts:
@@ -89,6 +94,9 @@
 	$(MKDIR_P) $(DESTDIR)$(vdsmhooksdir)/before_device_create
 	$(INSTALL_SCRIPT) $(srcdir)/$(constsfile) \
 		$(DESTDIR)$(vdsmhooksdir)/before_device_create/$(constsfile)
+	$(MKDIR_P) $(DESTDIR)$(vdsmhooksdir)/before_device_migrate_destination
+	$(INSTALL_SCRIPT) $(srcdir)/$(constsfile) \
+		$(DESTDIR)$(vdsmhooksdir)/before_device_migrate_destination/$(constsfile)
 	$(MKDIR_P) $(DESTDIR)$(vdsmhooksdir)/before_nic_hotplug
 	$(INSTALL_SCRIPT) $(srcdir)/$(constsfile) \
 		$(DESTDIR)$(vdsmhooksdir)/before_nic_hotplug/$(constsfile)
@@ -100,6 +108,7 @@
 	$(RM) $(DESTDIR)$(vdsmhooksdir)/after_nic_hotplug/$(constsfile)
 	$(RM) $(DESTDIR)$(vdsmhooksdir)/after_nic_hotunplug/$(constsfile)
 	$(RM) $(DESTDIR)$(vdsmhooksdir)/before_device_create/$(constsfile)
+	$(RM) $(DESTDIR)$(vdsmhooksdir)/before_device_migrate_destination/$(constsfile)
 	$(RM) $(DESTDIR)$(vdsmhooksdir)/before_nic_hotplug/$(constsfile)
 
 install-data-sudoers:
diff --git a/vdsm_hooks/openstacknet/before_device_create.py b/vdsm_hooks/openstacknet/before_device_create.py
index de7a896..d326cd5 100755
--- a/vdsm_hooks/openstacknet/before_device_create.py
+++ b/vdsm_hooks/openstacknet/before_device_create.py
@@ -31,15 +31,12 @@
 '''
 
 import os
-import subprocess
 import sys
 import traceback
 from xml.dom import minidom
 
 import hooking
 from openstacknet_utils import DUMMY_BRIDGE
-from openstacknet_utils import EXT_BRCTL
-from openstacknet_utils import EXT_IP
 from openstacknet_utils import INTEGRATION_BRIDGE
 from openstacknet_utils import OPENSTACK_NET_PROVIDER_TYPE
 from openstacknet_utils import PLUGIN_TYPE_KEY
@@ -48,10 +45,8 @@
 from openstacknet_utils import PT_OVS
 from openstacknet_utils import SECURITY_GROUPS_KEY
 from openstacknet_utils import VNIC_ID_KEY
-from openstacknet_utils import deviceExists
 from openstacknet_utils import devName
-from openstacknet_utils import executeOrExit
-from openstacknet_utils import ovs_vsctl
+from openstacknet_utils import setUpSecurityGroupVnic
 
 HELP_ARG = "-h"
 TEST_ARG = "-t"
@@ -94,36 +89,11 @@
 
 
 def addOvsHybridVnic(domxml, iface, portId):
+    setUpSecurityGroupVnic(
+        iface.getElementsByTagName('mac')[0].getAttribute('address'),
+        portId)
+
     brName = devName("qbr", portId)
-
-    # TODO: Remove this check after bz 1045626 is fixed
-    if not deviceExists(brName):
-        executeOrExit([EXT_BRCTL, 'addbr', brName])
-        executeOrExit([EXT_BRCTL, 'setfd', brName, '0'])
-        executeOrExit([EXT_BRCTL, 'stp', brName, 'off'])
-
-    vethBr = devName("qvb", portId)
-    vethOvs = devName("qvo", portId)
-
-    # TODO: Remove this check after bz 1045626 is fixed
-    if not deviceExists(vethOvs):
-        executeOrExit([EXT_IP, 'link', 'add', vethBr, 'type', 'veth', 'peer',
-                      'name', vethOvs])
-        for dev in [vethBr, vethOvs]:
-            executeOrExit([EXT_IP, 'link', 'set', dev, 'up'])
-            executeOrExit([EXT_IP, 'link', 'set', dev, 'promisc', 'on'])
-
-        executeOrExit([EXT_IP, 'link', 'set', brName, 'up'])
-        executeOrExit([EXT_BRCTL, 'addif', brName, vethBr])
-
-        mac = iface.getElementsByTagName('mac')[0].getAttribute('address')
-        executeOrExit([ovs_vsctl.cmd, '--', '--may-exist', 'add-port',
-                       INTEGRATION_BRIDGE, vethOvs,
-                       '--', 'set', 'Interface', vethOvs,
-                       'external-ids:iface-id=%s' % portId,
-                       'external-ids:iface-status=active',
-                       'external-ids:attached-mac=%s' % mac])
-
     defineLinuxBridge(domxml, iface, portId, brName)
 
 
@@ -165,16 +135,6 @@
         hooking.write_domxml(domxml)
 
 
-def mockExecuteOrExit(command):
-    print("Mocking successful execution of: %s"
-          % subprocess.list2cmdline(command))
-    return (0, '', '')
-
-
-def mockDeviceExists(dev):
-    return False
-
-
 def test(ovs, withSecurityGroups):
     domxml = minidom.parseString("""<?xml version="1.0" encoding="utf-8"?>
     <interface type="bridge">
@@ -188,8 +148,9 @@
     else:
         pluginType = PT_BRIDGE
 
-    globals()['executeOrExit'] = mockExecuteOrExit
-    globals()['deviceExists'] = mockDeviceExists
+    import openstacknet_utils
+    openstacknet_utils.executeOrExit = openstacknet_utils.mockExecuteOrExit
+    openstacknet_utils.deviceExists = openstacknet_utils.mockDeviceExists
     addOpenstackVnic(domxml,
                      pluginType,
                      'test_port_id',
diff --git a/vdsm_hooks/openstacknet/before_device_migrate_destination.py b/vdsm_hooks/openstacknet/before_device_migrate_destination.py
new file mode 100755
index 0000000..cc6fc76
--- /dev/null
+++ b/vdsm_hooks/openstacknet/before_device_migrate_destination.py
@@ -0,0 +1,78 @@
+#!/usr/bin/env python
+
+"""
+OpenStack Network Hook (pre device migration)
+=============================================
+The hook receives a port_id for a migrated virtual NIC that is to be handled by
+creating a security groups bridge if security groups are needed. If no security
+groups are needed, the xml of the device will already refer libvirt to use ovs,
+so no change is needed in that flow.
+
+For the security groups, then, the current implementation will connect the vNIC
+the tap will be connected to a dedicated Linux Bridge which will be connected
+by veth pair to the OVS integration bridge. The reason for this is that
+currently the Security Groups implementation (iptables) doesn't work on the OVS
+bridge, so a workaround had to be taken (same as OpenStack Compute does it).
+
+Syntax:
+    { 'provider_type': 'OPENSTACK_NETWORK', 'vnic_id': 'port_id',
+      'plugin_type': 'plugin_type_value', 'security_groups': .* }
+Where:
+    port_id should be replaced with the port id of the virtual NIC to be
+    connected to OpenStack Network.
+    plugin_type_value should be replaced with with OPEN_VSWITCH for OVS plugin
+    or anything else for other plugins.
+    security_groups will trigger the correct behavior for enabling security
+    groups support, mainly when using OVS. The value is unimportant.
+"""
+import hooking
+import os
+import sys
+import traceback
+
+from openstacknet_utils import OPENSTACK_NET_PROVIDER_TYPE
+from openstacknet_utils import PLUGIN_TYPE_KEY
+from openstacknet_utils import PROVIDER_TYPE_KEY
+from openstacknet_utils import PT_OVS
+from openstacknet_utils import SECURITY_GROUPS_KEY
+from openstacknet_utils import VNIC_ID_KEY
+from openstacknet_utils import setUpSecurityGroupVnic
+
+
+def main():
+    if PROVIDER_TYPE_KEY not in os.environ:
+        return
+
+    providerType = os.environ[PROVIDER_TYPE_KEY]
+    pluginType = os.environ[PLUGIN_TYPE_KEY]
+    if (providerType == OPENSTACK_NET_PROVIDER_TYPE and
+            pluginType == PT_OVS and SECURITY_GROUPS_KEY in os.environ):
+        domxml = hooking.read_domxml()
+        portId = os.environ[VNIC_ID_KEY]
+        iface = domxml.getElementsByTagName('interface')[0]
+        mac = iface.getElementsByTagName('mac')[0].getAttribute('address')
+        setUpSecurityGroupVnic(mac, portId)
+
+
+def test():
+    """Should print commands for:
+    - qbrtest_port_i linux bridge
+    - qvbtest_port_i veth attached to the bridge above
+    - qvbtest_port_i matching veth attached to the br-int with portId
+      'test_port_id' and mac '00:1a:4a:16:01:51'
+    """
+    import openstacknet_utils
+    openstacknet_utils.executeOrExit = openstacknet_utils.mockExecuteOrExit
+    openstacknet_utils.deviceExists = openstacknet_utils.mockDeviceExists
+    setUpSecurityGroupVnic("00:1a:4a:16:01:51", 'test_port_id')
+
+
+if __name__ == '__main__':
+    try:
+        if '-t' in sys.argv:
+            test()
+        else:
+            main()
+    except:
+        hooking.exit_hook('openstacknet hook: [unexpected error]: %s\n' %
+                          traceback.format_exc())
diff --git a/vdsm_hooks/openstacknet/openstacknet_utils.py b/vdsm_hooks/openstacknet/openstacknet_utils.py
index 5a75fb6..7593a6d 100644
--- a/vdsm_hooks/openstacknet/openstacknet_utils.py
+++ b/vdsm_hooks/openstacknet/openstacknet_utils.py
@@ -1,6 +1,7 @@
 #!/usr/bin/python
 
 import hooking
+import subprocess
 from vdsm.netinfo import DUMMY_BRIDGE
 from vdsm.utils import CommandPath
 
@@ -36,6 +37,12 @@
                            (command, err))
 
 
+def mockExecuteOrExit(command):
+    print("Mocking successful execution of: %s" %
+          subprocess.list2cmdline(command))
+    return (0, '', '')
+
+
 def devName(prefix, name):
     return (prefix + name)[:DEV_MAX_LENGTH]
 
@@ -44,3 +51,39 @@
     command = [EXT_IP, 'link', 'show', 'dev', dev]
     retcode, out, err = hooking.execCmd(command, raw=True)
     return retcode == 0
+
+
+def mockDeviceExists(dev):
+    return False
+
+
+def setUpSecurityGroupVnic(macAddr, portId):
+    hooking.log('Setting up vNIC (portId %s) security groups' % portId)
+    brName = devName("qbr", portId)
+
+    # TODO: Remove this check after bz 1045626 is fixed
+    if not deviceExists(brName):
+        executeOrExit([EXT_BRCTL, 'addbr', brName])
+        executeOrExit([EXT_BRCTL, 'setfd', brName, '0'])
+        executeOrExit([EXT_BRCTL, 'stp', brName, 'off'])
+
+    vethBr = devName("qvb", portId)
+    vethOvs = devName("qvo", portId)
+
+    # TODO: Remove this check after bz 1045626 is fixed
+    if not deviceExists(vethOvs):
+        executeOrExit([EXT_IP, 'link', 'add', vethBr, 'type', 'veth', 'peer',
+                      'name', vethOvs])
+        for dev in [vethBr, vethOvs]:
+            executeOrExit([EXT_IP, 'link', 'set', dev, 'up'])
+            executeOrExit([EXT_IP, 'link', 'set', dev, 'promisc', 'on'])
+
+        executeOrExit([EXT_IP, 'link', 'set', brName, 'up'])
+        executeOrExit([EXT_BRCTL, 'addif', brName, vethBr])
+
+        executeOrExit([ovs_vsctl.cmd, '--', '--may-exist', 'add-port',
+                       INTEGRATION_BRIDGE, vethOvs,
+                       '--', 'set', 'Interface', vethOvs,
+                       'external-ids:iface-id=%s' % portId,
+                       'external-ids:iface-status=active',
+                       'external-ids:attached-mac=%s' % macAddr])


-- 
To view, visit http://gerrit.ovirt.org/35961
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: Icd8a789c4565f32b32965af3966a4edd361949ea
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: ovirt-3.5
Gerrit-Owner: Dan Kenigsberg <danken at redhat.com>
Gerrit-Reviewer: Antoni Segura Puimedon <asegurap at redhat.com>
Gerrit-Reviewer: Francesco Romani <fromani at redhat.com>


More information about the vdsm-patches mailing list