router: Fixes #2789 fix proper mark based packet routing across interfaces (#2791)

Previously, the ethernet device index was used as rt_table index and
packet marking id/integer. With eth0 that is sometimes used as link-local
interface, the rt_table index `0` would fail as `0` is already defined
as a catchall (unspecified). The fwmarking on packets on eth0 with 0x0
would also fail. This fixes the routing issues, by adding 100 to the
ethernet device index so the value is a non-zero, for example then the
relationship between rt_table index and ethernet would be like:

100 -> Table_eth0 -> eth0 -> fwmark 100 or 0x64
101 -> Table_eth1 -> eth1 -> fwmark 101 or 0x65
102 -> Table_eth2 -> eth2 -> fwmark 102 or 0x66

This would maintain the legacy design of routing based on packet mark
and appropriate routing table rules per table/ids. This also fixes a
minor NPE issue around listing of snapshots.

This also backports fixes to smoketests from master.

Signed-off-by: Rohit Yadav <rohit.yadav@shapeblue.com>

server/src/com/cloud/api/ApiResponseHelper.java

@@ -501,7 +501,7 @@ public class ApiResponseHelper implements ResponseGenerator {
                 snapshotResponse.setZoneId(zone.getUuid());
             }
 
-            if (volume.getVolumeType() == Volume.Type.ROOT) {
+            if (volume.getVolumeType() == Volume.Type.ROOT && volume.getInstanceId() != null) {
                 //TODO combine lines and 489 into a join in the volume dao
                 VMInstanceVO instance = ApiDBUtils.findVMInstanceById(volume.getInstanceId());
                 if (instance != null) {

systemvm/debian/opt/cloud/bin/cloud-nic.sh

@@ -69,7 +69,7 @@ unplug_nic() {
 
 action=$1
 dev=$2
-tableNo=${dev:3}
+tableNo=$((100+${dev:3}))
 tableName="Table_$dev"
 
 if [ $action == 'add' ]

systemvm/debian/opt/cloud/bin/cs/CsNetfilter.py

@@ -189,7 +189,8 @@ class CsNetfilters(object):
     def add_chain(self, rule):
         """ Add the given chain if it is not already present """
         if not self.has_chain(rule.get_table(), rule.get_chain()):
-            CsHelper.execute("iptables -t %s -N %s" % (rule.get_table(), rule.get_chain()))
+            if rule.get_chain():
+                CsHelper.execute("iptables -t %s -N %s" % (rule.get_table(), rule.get_chain()))
             self.chain.add(rule.get_table(), rule.get_chain())
 
     def del_standard(self):

systemvm/debian/opt/cloud/bin/cs/CsRoute.py

@@ -30,7 +30,7 @@ class CsRoute:
         return self.table_prefix + name
 
     def add_table(self, devicename):
-        tablenumber = devicename[3:]
+        tablenumber = 100 + int(devicename[3:])
         tablename = self.get_tablename(devicename)
         str = "%s %s" % (tablenumber, tablename)
         filename = "/etc/iproute2/rt_tables"

systemvm/debian/opt/cloud/bin/cs/CsRule.py

@@ -27,7 +27,7 @@ class CsRule:
 
     def __init__(self, dev):
         self.dev = dev
-        self.tableNo = int(dev[3:])
+        self.tableNo = 100 + int(dev[3:])
         self.table = "Table_%s" % (dev)
 
     def addRule(self, rule):

systemvm/debian/opt/cloud/bin/ipassoc.sh

@@ -103,7 +103,7 @@ remove_routing() {
   logger -t cloud "$(basename $0):Remove routing $pubIp on interface $ethDev"
   local ipNoMask=$(echo $pubIp | awk -F'/' '{print $1}')
   local mask=$(echo $pubIp | awk -F'/' '{print $2}')
-  local tableNo=$(echo $ethDev | awk -F'eth' '{print $2}')
+  local tableNo=$((100+$(echo $ethDev | awk -F'eth' '{print $2}')))
 
   local tableName="Table_$ethDev"
   local remainip=`ip addr show $ethDev | grep "inet "`
@@ -149,7 +149,7 @@ add_routing() {
 
   local tableName="Table_$ethDev"
   local tablePresent=$(grep $tableName /etc/iproute2/rt_tables)
-  local tableNo=$(echo $ethDev | awk -F'eth' '{print $2}')
+  local tableNo=$((100+$(echo $ethDev | awk -F'eth' '{print $2}')))
   if [ "$tablePresent" == "" ]
   then
      if [ "$tableNo" == "" ]

systemvm/debian/opt/cloud/bin/update_config.py

@@ -30,7 +30,7 @@ logging.basicConfig(filename='/var/log/cloud.log', level=logging.INFO, format='%
 
 # first commandline argument should be the file to process
 if (len(sys.argv) != 2):
-    print "[ERROR]: Invalid usage"
+    logging.error("Invalid usage, args passed: %s" % sys.argv)
     sys.exit(1)
 
 # FIXME we should get this location from a configuration class
@@ -47,7 +47,7 @@ def finish_config():
 
 
 def process_file():
-    print "[INFO] Processing JSON file %s" % sys.argv[1]
+    logging.info("Processing JSON file %s" % sys.argv[1])
     qf = QueueFile()
     qf.setFile(sys.argv[1])
     qf.load(None)
@@ -70,7 +70,7 @@ def is_guestnet_configured(guestnet_dict, keys):
         '''
         It seems all the interfaces have been removed. Let's allow a new configuration to come in.
         '''
-        print "[WARN] update_config.py :: Reconfiguring guest network..."
+        logging.warn("update_config.py :: Reconfiguring guest network...")
         return False
 
     file = open(jsonConfigFile)
@@ -80,7 +80,7 @@ def is_guestnet_configured(guestnet_dict, keys):
         '''
         Guest network has to be removed.
         '''
-        print "[INFO] update_config.py :: Removing guest network..."
+        logging.info("update_config.py :: Removing guest network...")
         return False
 
     '''
@@ -121,7 +121,10 @@ if jsonFilename != "cmd_line.json" and os.path.isfile(jsonPath % "cmd_line.json"
     qf.load(None)
 
 if not (os.path.isfile(jsonConfigFile) and os.access(jsonConfigFile, os.R_OK)):
-    print "[ERROR] update_config.py :: Unable to read and access %s to process it" % jsonConfigFile
+    # Ignore if file is already processed
+    if os.path.isfile(jsonPath % ("processed/" + jsonFilename + ".gz")):
+        sys.exit(0)
+    logging.error("update_config.py :: Unable to read and access %s to process it" % jsonConfigFile)
     sys.exit(1)
 
 # If the guest network is already configured and have the same IP, do not try to configure it again otherwise it will break
@@ -131,14 +134,14 @@ if jsonFilename.startswith("guest_network.json"):
         guestnet_dict = json.load(file)
 
         if not is_guestnet_configured(guestnet_dict, ['eth1', 'eth2', 'eth3', 'eth4', 'eth5', 'eth6', 'eth7', 'eth8', 'eth9']):
-            print "[INFO] update_config.py :: Processing Guest Network."
+            logging.info("update_config.py :: Processing Guest Network.")
             process_file()
         else:
-            print "[INFO] update_config.py :: No need to process Guest Network."
+            logging.info("update_config.py :: No need to process Guest Network.")
             finish_config()
     else:
-        print "[INFO] update_config.py :: No GuestNetwork configured yet. Configuring first one now."
+        logging.info("update_config.py :: No GuestNetwork configured yet. Configuring first one now.")
         process_file()
 else:
-    print "[INFO] update_config.py :: Processing incoming file => %s" % sys.argv[1]
+    logging.info("update_config.py :: Processing incoming file => %s" % sys.argv[1])
     process_file()

test/integration/smoke/test_deploy_virtio_scsi_vm.py

@@ -25,35 +25,30 @@ from marvin.cloudstackTestCase import cloudstackTestCase
 # base - contains all resources as entities and defines create, delete,
 # list operations on them
 from marvin.lib.base import (Account,
-                            VirtualMachine,
+                             VirtualMachine,
-                            ServiceOffering,
+                             ServiceOffering,
-                            NetworkOffering,
+                             Template,
-                            Network,
+                             DiskOffering,
-                            Template,
+                             Volume,
-                            DiskOffering,
+                             Host,
-                            StoragePool,
+                             GuestOs)
-                            Volume,
-                            Host,
-                            GuestOs)
-
-
 
 # utils - utility classes for common cleanup, external library wrappers etc
 from marvin.lib.utils import cleanup_resources, get_hypervisor_type, validateList
 
 # common - commonly used methods for all tests are listed here
-from marvin.lib.common import get_zone, get_domain, get_template, list_hosts, get_pod
+from marvin.lib.common import get_zone, get_domain, get_pod
 
 from marvin.sshClient import SshClient
 
-from marvin.codes import FAILED, PASS
+from marvin.codes import FAILED
 
 from nose.plugins.attrib import attr
 
 import xml.etree.ElementTree as ET
-import code
 import logging
 
+
 class Templates:
     """Test data for templates
     """
@@ -75,11 +70,12 @@ class Templates:
             }
         }
 
-class TestDeployVirtioSCSIVM(cloudstackTestCase):
 
+class TestDeployVirtioSCSIVM(cloudstackTestCase):
     """
     Test deploy a kvm virtio scsi template
     """
+
     @classmethod
     def setUpClass(cls):
         cls.logger = logging.getLogger('TestDeployVirtioSCSIVM')
@@ -100,7 +96,6 @@ class TestDeployVirtioSCSIVM(cloudstackTestCase):
         cls.zone = get_zone(cls.apiclient, testClient.getZoneForTests())
         cls.pod = get_pod(cls.apiclient, cls.zone.id)
         cls.services['mode'] = cls.zone.networktype
-        cls._cleanup = []
         if cls.hypervisor.lower() not in ['kvm']:
             cls.hypervisorNotSupported = True
             return
@@ -153,41 +148,38 @@ class TestDeployVirtioSCSIVM(cloudstackTestCase):
 
         cls.vmhost = hosts[0]
 
-
+        # Stop VM to reset password
+        cls.virtual_machine.stop(cls.apiclient)
 
         password = cls.virtual_machine.resetPassword(cls.apiclient)
         cls.virtual_machine.username = "ubuntu"
         cls.virtual_machine.password = password
-        cls._cleanup = [
+
+        # Start VM after password reset
+        cls.virtual_machine.start(cls.apiclient)
+
+        cls.cleanup = [
             cls.template,
             cls.service_offering,
             cls.sparse_disk_offering,
             cls.account
         ]
 
-
     @classmethod
     def tearDownClass(cls):
         try:
+            cls.apiclient = super(
+                TestDeployVirtioSCSIVM,
+                cls
+            ).getClsTestClient().getApiClient()
             # Cleanup resources used
-            cleanup_resources(cls.apiclient, cls._cleanup)
+            cleanup_resources(cls.apiclient, cls.cleanup)
         except Exception as e:
             raise Exception("Warning: Exception during cleanup : %s" % e)
-        return
 
     def setUp(self):
         self.apiclient = self.testClient.getApiClient()
         self.dbclient = self.testClient.getDbConnection()
-        self.cleanup = []
-        return
-
-    def tearDown(self):
-        try:
-            # Clean up, terminate the created instance, volumes and snapshots
-            cleanup_resources(self.apiclient, self.cleanup)
-        except Exception as e:
-            raise Exception("Warning: Exception during cleanup : %s" % e)
-        return
 
     def verifyVirshState(self, diskcount):
         host = self.vmhost.ipaddress
@@ -212,14 +204,14 @@ class TestDeployVirtioSCSIVM(cloudstackTestCase):
             for child in disk:
                 if child.tag.lower() == "target":
                     dev = child.get("dev")
-                    self.assert_(dev != None and dev.startswith("sd"), "disk dev is invalid")
+                    self.assert_(dev is not None and dev.startswith("sd"), "disk dev is invalid")
                 elif child.tag.lower() == "address":
                     con = child.get("controller")
                     self.assertEqual(con, scsiindex, "disk controller not equal to SCSI " \
-                                     "controller index")
+                                                     "controller index")
                 elif child.tag.lower() == "driver":
                     discard = child.get("discard")
-                    if discard: # may not be defined by older qemu/libvirt
+                    if discard:  # may not be defined by older qemu/libvirt
                         self.assertEqual(discard, "unmap", "discard settings not unmap")
 
     def verifyGuestState(self, diskcount):
@@ -234,21 +226,21 @@ class TestDeployVirtioSCSIVM(cloudstackTestCase):
                          "Could not find appropriate number of scsi disks in guest")
 
     def getVirshXML(self, host, instancename):
-        if host == None:
+        if host is None:
             self.logger.debug("getVirshXML: host is none")
             return ""
         else:
             self.logger.debug("host is: " + host)
-        if instancename == None:
+        if instancename is None:
             self.logger.debug("getVirshXML: instancename is none")
             return ""
         else:
             self.logger.debug("instancename is: " + instancename)
         sshc = SshClient(
-                host=host,
+            host=host,
-                port=self.services['configurableData']['host']["publicport"],
+            port=self.services['configurableData']['host']["publicport"],
-                user=self.hostConfig['username'],
+            user=self.hostConfig['username'],
-                passwd=self.hostConfig['password'])
+            passwd=self.hostConfig['password'])
 
         ssh = sshc.ssh
 
@@ -354,9 +346,8 @@ class TestDeployVirtioSCSIVM(cloudstackTestCase):
         self.assertIsNotNone(ostypeid,
                              "Could not find ostypeid for Ubuntu 16.0.4 (64-bit) mapped to kvm")
 
-
         self.virtual_machine.update(self.apiclient, ostypeid=ostypeid,
-                                    details=[{"rootDiskController":"scsi"}])
+                                    details=[{"rootDiskController": "scsi"}])
 
         self.virtual_machine.start(self.apiclient)
 
@@ -371,6 +362,7 @@ class TestDeployVirtioSCSIVM(cloudstackTestCase):
 
         self.verifyGuestState(3)
 
+
 class CommandNonzeroException(Exception):
     def __init__(self, code, stderr):
         self.code = code

test/integration/smoke/test_vm_life_cycle.py

@@ -918,7 +918,7 @@ class TestSecuredVmMigration(cloudstackTestCase):
             cmd = provisionCertificate.provisionCertificateCmd()
             cmd.hostid = host.id
             cmd.reconnect = True
-            self.apiclient.updateConfiguration(cmd)
+            self.apiclient.provisionCertificate(cmd)
 
         for host in self.hosts:
             self.check_connection(secured='true', host=host)