Merge release branch 4.7 to master

* 4.7:
  CLOUDSTACK-9154 - Sets the pub interface down when all guest nets are gone
  CLOUDSTACK-9187 - Makes code ready for more something like ethXXXX, if we ever get that far
  CLOUDSTACK-9188 -  Reads network GC interval and wait from configDao
  CLOUDSTACK-9187 - Fixes interface allocation to VRRP instances
  CLOUDSTACK-9187 - Adds test to cover multiple nics and nic removal
  CLOUDSTACK-9154 - Adds test to cover nics state after GC
  CLOUDSTACK-9154 - Returns the guest iterface that is marked as added

 Conflicts:
	engine/orchestration/src/org/apache/cloudstack/engine/orchestration/NetworkOrchestrator.java

systemvm/patches/debian/config/opt/cloud/bin/cs/CsAddress.py

@@ -48,15 +48,19 @@ class CsAddress(CsDataBag):
 
     def get_guest_if(self):
         """
-        Return CsInterface object for the lowest guest interface
+        Return CsInterface object for the lowest in use guest interface
         """
-        ipr = []
+        guest_interface = None
+        lowest_device = 1000
         for ip in self.get_ips():
-            if ip.is_guest():
+            if ip.is_guest() and ip.is_added():
-                ipr.append(ip)
+                device = ip.get_device()
-        if len(ipr) > 0:
+                device_suffix = int(''.join([digit for digit in device if digit.isdigit()]))
-            return sorted(ipr)[-1]
+                if device_suffix < lowest_device:
-        return None
+                    lowest_device = device_suffix
+                    guest_interface = ip
+                    logging.debug("Guest interface will be set on device '%s' and IP '%s'" % (guest_interface.get_device(), guest_interface.get_ip()))
+        return guest_interface
 
     def get_guest_ip(self):
         """
@@ -190,6 +194,9 @@ class CsInterface:
             return True
         return False
     
+    def is_added(self):
+        return self.get_attr("add")
+
     def to_str(self):
         pprint(self.address)
 

systemvm/patches/debian/config/opt/cloud/bin/cs/CsRedundant.py

@@ -81,8 +81,10 @@ class CsRedundant(object):
 
     def _redundant_on(self):
         guest = self.address.get_guest_if()
+
         # No redundancy if there is no guest network
         if guest is None:
+            self.set_backup()
             self._redundant_off()
             return
 
@@ -108,7 +110,6 @@ class CsRedundant(object):
             CsHelper.service("keepalived", "stop")
             return
 
-
         CsHelper.mkdir(self.CS_RAMDISK_DIR, 0755, False)
         CsHelper.mount_tmpfs(self.CS_RAMDISK_DIR)
         CsHelper.mkdir(self.CS_ROUTER_DIR, 0755, False)
@@ -164,10 +165,14 @@ class CsRedundant(object):
         conntrackd_conf = CsFile(self.CONNTRACKD_CONF)
 
         is_equals = conntrackd_tmpl.compare(conntrackd_conf)
+
+        force_keepalived_restart = False
         proc = CsProcess(['/etc/conntrackd/conntrackd.conf'])
-        if not proc.find() or not is_equals:
+
+        if not proc.find() and not is_equals:
             CsHelper.copy(conntrackd_template_conf, self.CONNTRACKD_CONF)
             CsHelper.service("conntrackd", "restart")
+            force_keepalived_restart = True
 
         # Restore the template file and remove the backup.
         CsHelper.copy(conntrackd_temp_bkp, conntrackd_template_conf)
@@ -185,7 +190,7 @@ class CsRedundant(object):
         heartbeat_cron.commit()
 
         proc = CsProcess(['/usr/sbin/keepalived'])
-        if not proc.find() or keepalived_conf.is_changed():
+        if not proc.find() or keepalived_conf.is_changed() or force_keepalived_restart:
             keepalived_conf.commit()
             CsHelper.service("keepalived", "restart")
 
@@ -222,17 +227,21 @@ class CsRedundant(object):
 
         self.set_lock()
         logging.info("Router switched to fault mode")
-        ads = [o for o in self.address.get_ips() if o.is_public()]
+
-        for o in ads:
+        ips = [ip for ip in self.address.get_ips() if ip.is_public()]
-            CsHelper.execute("ifconfig %s down" % o.get_device())
+        for ip in ips:
+            CsHelper.execute("ifconfig %s down" % ip.get_device())
+
         cmd = "%s -C %s" % (self.CONNTRACKD_BIN, self.CONNTRACKD_CONF)
         CsHelper.execute("%s -s" % cmd)
         CsHelper.service("ipsec", "stop")
         CsHelper.service("xl2tpd", "stop")
         CsHelper.service("dnsmasq", "stop")
-        ads = [o for o in self.address.get_ips() if o.needs_vrrp()]
+
-        for o in ads:
+        ips = [ip for ip in self.address.get_ips() if ip.needs_vrrp()]
-            CsPasswdSvc(o.get_gateway()).stop()
+        for ip in ips:
+            CsPasswdSvc(ip.get_gateway()).stop()
+
         self.cl.set_fault_state()
         self.cl.save()
         self.release_lock()
@@ -246,22 +255,25 @@ class CsRedundant(object):
 
         self.set_lock()
         logging.debug("Setting router to backup")
-        ads = [o for o in self.address.get_ips() if o.is_public()]
+
         dev = ''
-        for o in ads:
+        ips = [ip for ip in self.address.get_ips() if ip.is_public()]
-            if dev == o.get_device():
+        for ip in ips:
+            if dev == ip.get_device():
                 continue
-            logging.info("Bringing public interface %s down" % o.get_device())
+            logging.info("Bringing public interface %s down" % ip.get_device())
-            cmd2 = "ip link set %s down" % o.get_device()
+            cmd2 = "ip link set %s down" % ip.get_device()
             CsHelper.execute(cmd2)
-            dev = o.get_device()
+            dev = ip.get_device()
+
         cmd = "%s -C %s" % (self.CONNTRACKD_BIN, self.CONNTRACKD_CONF)
         CsHelper.execute("%s -d" % cmd)
         CsHelper.service("ipsec", "stop")
         CsHelper.service("xl2tpd", "stop")
-        ads = [o for o in self.address.get_ips() if o.needs_vrrp()]
+
-        for o in ads:
+        ips = [ip for ip in self.address.get_ips() if ip.needs_vrrp()]
-            CsPasswdSvc(o.get_gateway()).stop()
+        for ip in ips:
+            CsPasswdSvc(ip.get_gateway()).stop()
         CsHelper.service("dnsmasq", "stop")
 
         self.cl.set_master_state(False)
@@ -278,13 +290,13 @@ class CsRedundant(object):
         self.set_lock()
         logging.debug("Setting router to master")
 
-        ads = [o for o in self.address.get_ips() if o.is_public()]
         dev = ''
+        ips = [ip for ip in self.address.get_ips() if ip.is_public()]
         route = CsRoute()
-        for o in ads:
+        for ip in ips:
-            if dev == o.get_device():
+            if dev == ip.get_device():
                 continue
-            dev = o.get_device()
+            dev = ip.get_device()
             logging.info("Will proceed configuring device ==> %s" % dev)
             cmd2 = "ip link set %s up" % dev
             if CsDevice(dev, self.config).waitfordevice():
@@ -292,7 +304,7 @@ class CsRedundant(object):
                 logging.info("Bringing public interface %s up" % dev)
 
                 try:
-                    gateway = o.get_gateway()
+                    gateway = ip.get_gateway()
                     logging.info("Adding gateway ==> %s to device ==> %s" % (gateway, dev))
                     route.add_defaultroute(gateway)
                 except:
@@ -341,13 +353,15 @@ class CsRedundant(object):
         that could function as a router and VPC router at the same time
         """
         lines = []
-        for o in self.address.get_ips():
+        for ip in self.address.get_ips():
-            if o.needs_vrrp():
+            if ip.needs_vrrp():
                 cmdline=self.config.get_cmdline_instance()
+                if not ip.is_added():
+                    continue
                 if(cmdline.get_type()=='router'):
-                    str = "        %s brd %s dev %s\n" % (cmdline.get_guest_gw(), o.get_broadcast(), o.get_device())
+                    str = "        %s brd %s dev %s\n" % (cmdline.get_guest_gw(), ip.get_broadcast(), ip.get_device())
                 else:
-                    str = "        %s brd %s dev %s\n" % (o.get_gateway_cidr(), o.get_broadcast(), o.get_device())
+                    str = "        %s brd %s dev %s\n" % (ip.get_gateway_cidr(), ip.get_broadcast(), ip.get_device())
                 lines.append(str)
         return lines
 

test/integration/smoke/test_vpc_redundant.py

@@ -34,7 +34,8 @@ from marvin.lib.base import (stopRouter,
                              NetworkOffering,
                              Network,
                              VirtualMachine,
-                             LoadBalancerRule)
+                             LoadBalancerRule,
+                             Configurations)
 from marvin.lib.common import (get_domain,
                                get_zone,
                                get_template,
@@ -134,7 +135,7 @@ class Services:
             "vpc": {
                 "name": "TestVPC",
                 "displaytext": "TestVPC",
-                "cidr": '10.0.0.1/24'
+                "cidr": '10.0.0.0/16'
             },
             "network": {
                 "name": "Test Network",
@@ -295,7 +296,7 @@ class TestVPCRedundancy(cloudstackTestCase):
             len(self.routers), count,
             "Check that %s routers were indeed created" % count)
 
-    def check_master_status(self,count=2, showall=False):
+    def check_routers_state(self,count=2, status_to_check="MASTER", expected_count=1, showall=False):
         vals = ["MASTER", "BACKUP", "UNKNOWN"]
         cnts = [0, 0, 0]
 
@@ -345,11 +346,11 @@ class TestVPCRedundancy(cloudstackTestCase):
                             "Marvin configuration has no host credentials to\
                                     check router services")
             
-                if result.count(vals[0]) == 1:
+                if result.count(status_to_check) == 1:
-                    cnts[vals.index(vals[0])] += 1
+                    cnts[vals.index(status_to_check)] += 1
 
-        if cnts[vals.index('MASTER')] != 1:
+        if cnts[vals.index(status_to_check)] != expected_count:
-            self.fail("No Master or too many master routers found %s" % cnts[vals.index('MASTER')])
+            self.fail("Expected '%s' routers at state '%s', but found '%s'!" % (expected_count, status_to_check, cnts[vals.index(status_to_check)]))
 
     def stop_router(self, router):
         self.logger.debug('Stopping router %s' % router.id)
@@ -364,14 +365,14 @@ class TestVPCRedundancy(cloudstackTestCase):
         self.apiclient.rebootRouter(cmd)
 
     def stop_router_by_type(self, type):
-        self.check_master_status(2)
+        self.check_routers_state()
         self.logger.debug('Stopping %s router' % type)
         for router in self.routers:
             if router.redundantstate == type:
                 self.stop_router(router)
 
     def reboot_router_by_type(self, type):
-        self.check_master_status(2)
+        self.check_routers_state()
         self.logger.debug('Rebooting %s router' % type)
         for router in self.routers:
             if router.redundantstate == type:
@@ -387,7 +388,7 @@ class TestVPCRedundancy(cloudstackTestCase):
         self.routers = []
 
     def start_routers(self):
-        self.check_master_status(2, showall=True)
+        self.check_routers_state(showall=True)
         self.logger.debug('Starting stopped routers')
         for router in self.routers:
             self.logger.debug('Router %s has state %s' % (router.id, router.state))
@@ -397,7 +398,9 @@ class TestVPCRedundancy(cloudstackTestCase):
                 cmd.id = router.id
                 self.apiclient.startRouter(cmd)
 
-    def create_network(self, net_offerring, gateway='10.1.1.1', vpc=None):
+    def create_network(self, net_offerring, gateway='10.1.1.1', vpc=None, nr_vms=2, mark_net_cleanup=True):
+        if not nr_vms or nr_vms <= 0:
+            self.fail("At least 1 VM has to be created. You informed nr_vms < 1")
         try:
             self.logger.debug('Create NetworkOffering')
             net_offerring["name"] = "NET_OFF-" + str(gateway)
@@ -428,16 +431,17 @@ class TestVPCRedundancy(cloudstackTestCase):
             self.fail('Unable to create a Network with offering=%s because of %s ' % (net_offerring, e))
         o = networkO(obj_network)
 
-        vm1 = self.deployvm_in_network(obj_network)
+        self.cleanup.insert(0, nw_off)
-        vm2 = self.deployvm_in_network(obj_network)
+        if mark_net_cleanup:
-        self.cleanup.insert(2, obj_network)
+            self.cleanup.insert(0, obj_network)
-        self.cleanup.insert(3, nw_off)
 
+        for i in range(0, nr_vms):
+            vm1 = self.deployvm_in_network(obj_network, mark_vm_cleanup=mark_net_cleanup)
             o.add_vm(vm1)
-        o.add_vm(vm2)
+
         return o
 
-    def deployvm_in_network(self, network, host_id=None):
+    def deployvm_in_network(self, network, host_id=None, mark_vm_cleanup=True):
         try:
             self.logger.debug('Creating VM in network=%s' % network.name)
             vm = VirtualMachine.create(
@@ -451,6 +455,7 @@ class TestVPCRedundancy(cloudstackTestCase):
             )
 
             self.logger.debug('Created VM=%s in network=%s' % (vm.id, network.name))
+            if mark_vm_cleanup:
                 self.cleanup.insert(0, vm)
             return vm
         except:
@@ -524,22 +529,22 @@ class TestVPCRedundancy(cloudstackTestCase):
         self.query_routers()
         self.networks.append(self.create_network(self.services["network_offering"], "10.1.1.1"))
         self.networks.append(self.create_network(self.services["network_offering_no_lb"], "10.1.2.1"))
-        self.check_master_status(2)
+        self.check_routers_state()
         self.add_nat_rules()
         self.do_vpc_test(False)
         
         self.stop_router_by_type("MASTER")
-        self.check_master_status(1)
+        self.check_routers_state(1)
         self.do_vpc_test(False)
 
         self.delete_nat_rules()
-        self.check_master_status(1)
+        self.check_routers_state(count=1)
         self.do_vpc_test(True)
         self.delete_public_ip()
 
         self.start_routers()
         self.add_nat_rules()
-        self.check_master_status(2)
+        self.check_routers_state()
         self.do_vpc_test(False)
 
     @attr(tags=["advanced", "intervlan"], required_hardware="true")
@@ -549,7 +554,7 @@ class TestVPCRedundancy(cloudstackTestCase):
         self.query_routers()
         self.networks.append(self.create_network(self.services["network_offering"], "10.1.1.1"))
         self.networks.append(self.create_network(self.services["network_offering_no_lb"], "10.1.2.1"))
-        self.check_master_status(2)
+        self.check_routers_state()
         self.add_nat_rules()
         self.do_default_routes_test()
     
@@ -559,18 +564,89 @@ class TestVPCRedundancy(cloudstackTestCase):
         self.logger.debug("Starting test_01_create_redundant_VPC_2tiers_4VMs_4IPs_4PF_ACL")
         self.query_routers()
         self.networks.append(self.create_network(self.services["network_offering"], "10.1.1.1"))
-        self.check_master_status(2)
+        self.check_routers_state()
         self.add_nat_rules()
         self.do_vpc_test(False)
         
         self.reboot_router_by_type("MASTER")
-        self.check_master_status(2)
+        self.check_routers_state()
         self.do_vpc_test(False)
 
         self.reboot_router_by_type("MASTER")
-        self.check_master_status(2)
+        self.check_routers_state()
         self.do_vpc_test(False)
 
+    @attr(tags=["advanced", "intervlan"], required_hardware="true")
+    def test_04_rvpc_network_garbage_collector_nics(self):
+        """ Create a redundant VPC with 1 Tier, 1 VM, 1 ACL, 1 PF and test Network GC Nics"""
+        self.logger.debug("Starting test_04_rvpc_network_garbage_collector_nics")
+        self.query_routers()
+        self.networks.append(self.create_network(self.services["network_offering"], "10.1.1.1", nr_vms=1))
+        self.check_routers_state()
+        self.add_nat_rules()
+        self.do_vpc_test(False)
+
+        self.stop_vm()
+
+        gc_wait = Configurations.list(self.apiclient, name="network.gc.wait")
+        gc_interval = Configurations.list(self.apiclient, name="network.gc.interval")
+        
+        self.logger.debug("network.gc.wait is ==> %s" % gc_wait)
+        self.logger.debug("network.gc.interval is ==> %s" % gc_wait)
+
+        total_sleep = 120
+        if gc_wait and gc_interval:
+            total_sleep = int(gc_wait[0].value) + int(gc_interval[0].value)
+        else:
+            self.logger.debug("Could not retrieve the keys 'network.gc.interval' and 'network.gc.wait'. Sleeping for 2 minutes.")
+
+        time.sleep(total_sleep * 3)
+
+        self.check_routers_state(status_to_check="BACKUP", expected_count=2)
+        self.start_vm()
+        self.check_routers_state(status_to_check="MASTER")
+
+    @attr(tags=["advanced", "intervlan"], required_hardware="true")
+    def test_05_rvpc_multi_tiers(self):
+        """ Create a redundant VPC with 1 Tier, 1 VM, 1 ACL, 1 PF and test Network GC Nics"""
+        self.logger.debug("Starting test_04_rvpc_network_garbage_collector_nics")
+        self.query_routers()
+
+        network = self.create_network(self.services["network_offering"], "10.1.1.1", nr_vms=1, mark_net_cleanup=False)
+        self.networks.append(network)
+        self.networks.append(self.create_network(self.services["network_offering_no_lb"], "10.1.2.1", nr_vms=1))
+        self.networks.append(self.create_network(self.services["network_offering_no_lb"], "10.1.3.1", nr_vms=1))
+        
+        self.check_routers_state()
+        self.add_nat_rules()
+        self.do_vpc_test(False)
+
+        self.destroy_vm(network)
+        network.get_net().delete(self.apiclient)
+        self.networks.remove(network)
+        
+        self.check_routers_state(status_to_check="MASTER")
+        self.do_vpc_test(False)
+
+    def destroy_vm(self, network):
+        vms_to_delete = []
+        for vm in network.get_vms():
+            vm.get_vm().delete(self.apiclient, expunge=True)
+            vms_to_delete.append(vm)
+
+        all_vms = network.get_vms()
+        [all_vms.remove(vm) for vm in vms_to_delete]
+
+    def stop_vm(self):
+        for o in self.networks:
+            for vm in o.get_vms():
+                vm.get_vm().stop(self.apiclient)
+
+    def start_vm(self):
+        for o in self.networks:
+            for vm in o.get_vms():
+                vm.get_vm().start(self.apiclient)
+
     def delete_nat_rules(self):
         for o in self.networks:
             for vm in o.get_vms():