From 748e4b5af048e0b23504339ac9510cf230cb46ed Mon Sep 17 00:00:00 2001
From: Michael Hanselmann <hansmi@google.com>
Date: Wed, 14 Jul 2010 20:15:59 +0200
Subject: [PATCH] KVM hypervisor: Use utils.ShellWriter for network script

This patch converts hv_kvm to use utils.ShellWriter for writing
the network script. It also adds a few unittests (the first
for any hypervisor modules).

Signed-off-by: Michael Hanselmann <hansmi@google.com>
Reviewed-by: Iustin Pop <iustin@google.com>
---
 Makefile.am                               |   1 +
 lib/hypervisor/hv_kvm.py                  | 162 +++++++++++++++-------
 test/ganeti.hypervisor.hv_kvm_unittest.py |  84 +++++++++++
 3 files changed, 194 insertions(+), 53 deletions(-)
 create mode 100755 test/ganeti.hypervisor.hv_kvm_unittest.py

diff --git a/Makefile.am b/Makefile.am
index 5cc72d8c9..f54d8e94a 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -376,6 +376,7 @@ python_tests = \
 	test/ganeti.errors_unittest.py \
 	test/ganeti.hooks_unittest.py \
 	test/ganeti.http_unittest.py \
+	test/ganeti.hypervisor.hv_kvm_unittest.py \
 	test/ganeti.impexpd_unittest.py \
 	test/ganeti.jqueue_unittest.py \
 	test/ganeti.locking_unittest.py \
diff --git a/lib/hypervisor/hv_kvm.py b/lib/hypervisor/hv_kvm.py
index f1381f845..35082a7cd 100644
--- a/lib/hypervisor/hv_kvm.py
+++ b/lib/hypervisor/hv_kvm.py
@@ -44,6 +44,109 @@ from ganeti.hypervisor import hv_base
 from ganeti import netutils
 
 
+_KVM_NETWORK_SCRIPT = constants.SYSCONFDIR + "/ganeti/kvm-vif-bridge"
+
+
+def _WriteNetScript(instance, nic, index):
+  """Write a script to connect a net interface to the proper bridge.
+
+  This can be used by any qemu-type hypervisor.
+
+  @type instance: L{objects.Instance}
+  @param instance: Instance object
+  @type nic: L{objects.NIC}
+  @param nic: NIC object
+  @type index: int
+  @param index: NIC index
+  @return: Script
+  @rtype: string
+
+  """
+  if instance.tags:
+    tags = " ".join(instance.tags)
+  else:
+    tags = ""
+
+  buf = StringIO()
+  sw = utils.ShellWriter(buf)
+  sw.Write("#!/bin/sh")
+  sw.Write("# this is autogenerated by Ganeti, please do not edit")
+  sw.Write("export PATH=$PATH:/sbin:/usr/sbin")
+  sw.Write("export INSTANCE=%s", utils.ShellQuote(instance.name))
+  sw.Write("export MAC=%s", utils.ShellQuote(nic.mac))
+  sw.Write("export MODE=%s",
+           utils.ShellQuote(nic.nicparams[constants.NIC_MODE]))
+  sw.Write("export INTERFACE=\"$1\"")
+  sw.Write("export TAGS=%s", utils.ShellQuote(tags))
+
+  if nic.ip:
+    sw.Write("export IP=%s", utils.ShellQuote(nic.ip))
+
+  if nic.nicparams[constants.NIC_LINK]:
+    sw.Write("export LINK=%s",
+             utils.ShellQuote(nic.nicparams[constants.NIC_LINK]))
+
+  if nic.nicparams[constants.NIC_MODE] == constants.NIC_MODE_BRIDGED:
+    sw.Write("export BRIDGE=%s",
+             utils.ShellQuote(nic.nicparams[constants.NIC_LINK]))
+
+  # TODO: make this configurable at ./configure time
+  sw.Write("if [ -x %s ]; then", utils.ShellQuote(_KVM_NETWORK_SCRIPT))
+  sw.IncIndent()
+  try:
+    sw.Write("# Execute the user-specific vif file")
+    sw.Write(_KVM_NETWORK_SCRIPT)
+  finally:
+    sw.DecIndent()
+  sw.Write("else")
+  sw.IncIndent()
+  try:
+    sw.Write("ifconfig $INTERFACE 0.0.0.0 up")
+
+    if nic.nicparams[constants.NIC_MODE] == constants.NIC_MODE_BRIDGED:
+      sw.Write("# Connect the interface to the bridge")
+      sw.Write("brctl addif $BRIDGE $INTERFACE")
+
+    elif nic.nicparams[constants.NIC_MODE] == constants.NIC_MODE_ROUTED:
+      if not nic.ip:
+        raise errors.HypervisorError("nic/%d is routed, but has no IP"
+                                     " address" % index)
+
+      sw.Write("# Route traffic targeted at the IP to the interface")
+      if nic.nicparams[constants.NIC_LINK]:
+        sw.Write("while ip rule del dev $INTERFACE; do :; done")
+        sw.Write("ip rule add dev $INTERFACE table $LINK")
+        sw.Write("ip route replace $IP table $LINK proto static"
+                 " dev $INTERFACE")
+      else:
+        sw.Write("ip route replace $IP proto static dev $INTERFACE")
+
+      interface_v4_conf = "/proc/sys/net/ipv4/conf/$INTERFACE"
+      sw.Write(" if [ -d %s ]; then", interface_v4_conf)
+      sw.IncIndent()
+      try:
+        sw.Write("echo 1 > %s/proxy_arp", interface_v4_conf)
+        sw.Write("echo 1 > %s/forwarding", interface_v4_conf)
+      finally:
+        sw.DecIndent()
+      sw.Write("fi")
+
+      interface_v6_conf = "/proc/sys/net/ipv6/conf/$INTERFACE"
+      sw.Write("if [ -d %s ]; then", interface_v6_conf)
+      sw.IncIndent()
+      try:
+        sw.Write("echo 1 > %s/proxy_ndp", interface_v6_conf)
+        sw.Write("echo 1 > %s/forwarding", interface_v6_conf)
+      finally:
+        sw.DecIndent()
+      sw.Write("fi")
+  finally:
+    sw.DecIndent()
+  sw.Write("fi")
+
+  return buf.getvalue()
+
+
 class KVMHypervisor(hv_base.BaseHypervisor):
   """KVM hypervisor interface"""
   CAN_MIGRATE = True
@@ -108,8 +211,6 @@ class KVMHypervisor(hv_base.BaseHypervisor):
   _MIGRATION_INFO_MAX_BAD_ANSWERS = 5
   _MIGRATION_INFO_RETRY_DELAY = 2
 
-  _KVM_NETWORK_SCRIPT = constants.SYSCONFDIR + "/ganeti/kvm-vif-bridge"
-
   ANCILLARY_FILES = [
     _KVM_NETWORK_SCRIPT,
     ]
@@ -298,7 +399,8 @@ class KVMHypervisor(hv_base.BaseHypervisor):
       else:
         raise
 
-  def _WriteNetScript(self, instance, seq, nic):
+  @staticmethod
+  def _WriteNetScriptFile(instance, seq, nic):
     """Write a script to connect a net interface to the proper bridge.
 
     This can be used by any qemu-type hypervisor.
@@ -313,60 +415,14 @@ class KVMHypervisor(hv_base.BaseHypervisor):
     @rtype: string
 
     """
-    script = StringIO()
-    script.write("#!/bin/sh\n")
-    script.write("# this is autogenerated by Ganeti, please do not edit\n#\n")
-    script.write("PATH=$PATH:/sbin:/usr/sbin\n")
-    script.write("export INSTANCE=%s\n" % instance.name)
-    script.write("export MAC=%s\n" % nic.mac)
-    if nic.ip:
-      script.write("export IP=%s\n" % nic.ip)
-    script.write("export MODE=%s\n" % nic.nicparams[constants.NIC_MODE])
-    if nic.nicparams[constants.NIC_LINK]:
-      script.write("export LINK=%s\n" % nic.nicparams[constants.NIC_LINK])
-    if nic.nicparams[constants.NIC_MODE] == constants.NIC_MODE_BRIDGED:
-      script.write("export BRIDGE=%s\n" % nic.nicparams[constants.NIC_LINK])
-    script.write("export INTERFACE=$1\n")
-    if instance.tags:
-      script.write("export TAGS=\"%s\"\n" % " ".join(instance.tags))
-    # TODO: make this configurable at ./configure time
-    script.write("if [ -x '%s' ]; then\n" % self._KVM_NETWORK_SCRIPT)
-    script.write("  # Execute the user-specific vif file\n")
-    script.write("  %s\n" % self._KVM_NETWORK_SCRIPT)
-    script.write("else\n")
-    script.write("  ifconfig $INTERFACE 0.0.0.0 up\n")
-    if nic.nicparams[constants.NIC_MODE] == constants.NIC_MODE_BRIDGED:
-      script.write("  # Connect the interface to the bridge\n")
-      script.write("  brctl addif $BRIDGE $INTERFACE\n")
-    elif nic.nicparams[constants.NIC_MODE] == constants.NIC_MODE_ROUTED:
-      if not nic.ip:
-        raise errors.HypervisorError("nic/%d is routed, but has no ip." % seq)
-      script.write("  # Route traffic targeted at the IP to the interface\n")
-      if nic.nicparams[constants.NIC_LINK]:
-        script.write("  while ip rule del dev $INTERFACE; do :; done\n")
-        script.write("  ip rule add dev $INTERFACE table $LINK\n")
-        script.write("  ip route replace $IP table $LINK proto static"
-                     " dev $INTERFACE\n")
-      else:
-        script.write("  ip route replace $IP proto static"
-                     " dev $INTERFACE\n")
-      interface_v4_conf = "/proc/sys/net/ipv4/conf/$INTERFACE"
-      interface_v6_conf = "/proc/sys/net/ipv6/conf/$INTERFACE"
-      script.write("  if [ -d %s ]; then\n" % interface_v4_conf)
-      script.write("    echo 1 > %s/proxy_arp\n" % interface_v4_conf)
-      script.write("    echo 1 > %s/forwarding\n" % interface_v4_conf)
-      script.write("  fi\n")
-      script.write("  if [ -d %s ]; then\n" % interface_v6_conf)
-      script.write("    echo 1 > %s/proxy_ndp\n" % interface_v6_conf)
-      script.write("    echo 1 > %s/forwarding\n" % interface_v6_conf)
-      script.write("  fi\n")
-    script.write("fi\n\n")
+    script = _WriteNetScript(instance, nic, seq)
+
     # As much as we'd like to put this in our _ROOT_DIR, that will happen to be
     # mounted noexec sometimes, so we'll have to find another place.
     (tmpfd, tmpfile_name) = tempfile.mkstemp()
     tmpfile = os.fdopen(tmpfd, 'w')
     try:
-      tmpfile.write(script.getvalue())
+      tmpfile.write(script)
     finally:
       tmpfile.close()
     os.chmod(tmpfile_name, 0755)
@@ -681,7 +737,7 @@ class KVMHypervisor(hv_base.BaseHypervisor):
 
       for nic_seq, nic in enumerate(kvm_nics):
         nic_val = "nic,vlan=%s,macaddr=%s,%s" % (nic_seq, nic.mac, nic_model)
-        script = self._WriteNetScript(instance, nic_seq, nic)
+        script = self._WriteNetScriptFile(instance, nic_seq, nic)
         tap_val = "tap,vlan=%s,script=%s%s" % (nic_seq, script, tap_extra)
         kvm_cmd.extend(["-net", nic_val])
         kvm_cmd.extend(["-net", tap_val])
diff --git a/test/ganeti.hypervisor.hv_kvm_unittest.py b/test/ganeti.hypervisor.hv_kvm_unittest.py
new file mode 100755
index 000000000..28a936be1
--- /dev/null
+++ b/test/ganeti.hypervisor.hv_kvm_unittest.py
@@ -0,0 +1,84 @@
+#!/usr/bin/python
+#
+
+# Copyright (C) 2010 Google Inc.
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful, but
+# WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+# General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write to the Free Software
+# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA
+# 02110-1301, USA.
+
+
+"""Script for testing the hypervisor.hv_kvm module"""
+
+import unittest
+
+from ganeti import constants
+from ganeti import compat
+from ganeti import objects
+from ganeti import errors
+
+from ganeti.hypervisor import hv_kvm
+
+import testutils
+
+
+class TestWriteNetScript(unittest.TestCase):
+  def testBridged(self):
+    inst = objects.Instance(name="inst1.example.com", tags=[])
+    nic = objects.NIC(mac="01:23:45:67:89:0A",
+                      nicparams={
+                        constants.NIC_MODE: constants.NIC_MODE_BRIDGED,
+                        constants.NIC_LINK: "",
+                        })
+
+    script = hv_kvm._WriteNetScript(inst, nic, 0)
+    self.assert_(isinstance(script, basestring))
+
+  def testBridgedWithTags(self):
+    inst = objects.Instance(name="inst1.example.com", tags=["Hello", "World"])
+    nic = objects.NIC(mac="01:23:45:67:89:0A",
+                      nicparams={
+                        constants.NIC_MODE: constants.NIC_MODE_BRIDGED,
+                        constants.NIC_LINK: "",
+                        })
+
+    script = hv_kvm._WriteNetScript(inst, nic, 0)
+    self.assert_(isinstance(script, basestring))
+
+  def testRouted(self):
+    inst = objects.Instance(name="inst2.example.com", tags=[])
+    nic = objects.NIC(mac="A0:98:76:54:32:10",
+                      ip="192.0.2.4",
+                      nicparams={
+                        constants.NIC_MODE: constants.NIC_MODE_ROUTED,
+                        constants.NIC_LINK: "eth0",
+                        })
+
+    script = hv_kvm._WriteNetScript(inst, nic, 4)
+    self.assert_(isinstance(script, basestring))
+
+  def testRoutedNoIpAddress(self):
+    inst = objects.Instance(name="eiphei1e.example.com", tags=[])
+    nic = objects.NIC(mac="93:28:76:54:32:10",
+                      nicparams={
+                        constants.NIC_MODE: constants.NIC_MODE_ROUTED,
+                        constants.NIC_LINK: "",
+                        })
+
+    self.assertRaises(errors.HypervisorError, hv_kvm._WriteNetScript,
+                      inst, nic, 2)
+
+
+if __name__ == "__main__":
+  testutils.GanetiTestProgram()
-- 
GitLab