You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cloudstack.apache.org by da...@apache.org on 2023/01/05 20:36:39 UTC

[cloudstack] branch main updated: Allow creating atmost 1 physical network with null tag (#6781)

This is an automated email from the ASF dual-hosted git repository.

dahn pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/cloudstack.git


The following commit(s) were added to refs/heads/main by this push:
     new 20306d6129 Allow creating atmost 1 physical network with null tag (#6781)
20306d6129 is described below

commit 20306d612928712e5354bad57691b5fe4e1f59a9
Author: Sina Kashipazha <so...@users.noreply.github.com>
AuthorDate: Thu Jan 5 20:36:31 2023 +0000

    Allow creating atmost 1 physical network with null tag (#6781)
---
 .../java/com/cloud/network/dao/NetworkDaoImpl.java |   2 +-
 .../configuration/ConfigurationManagerImpl.java    |  43 ++-
 .../java/com/cloud/network/NetworkModelImpl.java   |  37 +-
 .../java/com/cloud/network/NetworkServiceImpl.java |  73 ++--
 .../test_multiple_physical_network_creation.py     | 405 +++++++++++++++++++++
 tools/marvin/marvin/lib/base.py                    |  10 +-
 6 files changed, 509 insertions(+), 61 deletions(-)

diff --git a/engine/schema/src/main/java/com/cloud/network/dao/NetworkDaoImpl.java b/engine/schema/src/main/java/com/cloud/network/dao/NetworkDaoImpl.java
index 5b759c5092..502ddfa7a7 100644
--- a/engine/schema/src/main/java/com/cloud/network/dao/NetworkDaoImpl.java
+++ b/engine/schema/src/main/java/com/cloud/network/dao/NetworkDaoImpl.java
@@ -129,7 +129,7 @@ public class NetworkDaoImpl extends GenericDaoBase<NetworkVO, Long>implements Ne
         AllFieldsSearch.and("account", AllFieldsSearch.entity().getAccountId(), Op.EQ);
         AllFieldsSearch.and("related", AllFieldsSearch.entity().getRelated(), Op.EQ);
         AllFieldsSearch.and("guestType", AllFieldsSearch.entity().getGuestType(), Op.EQ);
-        AllFieldsSearch.and("physicalNetwork", AllFieldsSearch.entity().getPhysicalNetworkId(), Op.EQ);
+        AllFieldsSearch.and("physicalNetworkId", AllFieldsSearch.entity().getPhysicalNetworkId(), Op.EQ);
         AllFieldsSearch.and("broadcastUri", AllFieldsSearch.entity().getBroadcastUri(), Op.EQ);
         AllFieldsSearch.and("vpcId", AllFieldsSearch.entity().getVpcId(), Op.EQ);
         AllFieldsSearch.and("aclId", AllFieldsSearch.entity().getNetworkACLId(), Op.EQ);
diff --git a/server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java b/server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java
index 7fa764d2f2..0edc96acf4 100644
--- a/server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java
+++ b/server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java
@@ -6720,19 +6720,10 @@ public class ConfigurationManagerImpl extends ManagerBase implements Configurati
         final Boolean sourceNatSupported = cmd.getSourceNatSupported();
         final List<String> pNtwkTags = new ArrayList<String>();
         boolean checkForTags = false;
+        boolean allowNullTag = false;
         if (zone != null) {
-            final List<PhysicalNetworkVO> pNtwks = _physicalNetworkDao.listByZoneAndTrafficType(zoneId, TrafficType.Guest);
-            if (pNtwks.size() > 1) {
-                checkForTags = true;
-                // go through tags
-                for (final PhysicalNetworkVO pNtwk : pNtwks) {
-                    final List<String> pNtwkTag = pNtwk.getTags();
-                    if (pNtwkTag == null || pNtwkTag.isEmpty()) {
-                        throw new CloudRuntimeException("Tags are not defined for physical network in the zone id=" + zoneId);
-                    }
-                    pNtwkTags.addAll(pNtwkTag);
-                }
-            }
+            allowNullTag = allowNetworkOfferingWithNullTag(zoneId, pNtwkTags);
+            checkForTags = !pNtwkTags.isEmpty() || allowNullTag;
         }
 
         // filter by supported services
@@ -6762,10 +6753,8 @@ public class ConfigurationManagerImpl extends ManagerBase implements Configurati
                 boolean addOffering = true;
                 List<Service> checkForProviders = new ArrayList<Service>();
 
-                if (checkForTags) {
-                    if (!pNtwkTags.contains(offering.getTags())) {
-                        continue;
-                    }
+                if (checkForTags && ! checkNetworkOfferingTags(pNtwkTags, allowNullTag, offering.getTags())) {
+                    continue;
                 }
 
                 if (listBySupportedServices) {
@@ -6815,6 +6804,28 @@ public class ConfigurationManagerImpl extends ManagerBase implements Configurati
         }
     }
 
+    private boolean allowNetworkOfferingWithNullTag(Long zoneId, List<String> allPhysicalNetworkTags) {
+        boolean allowNullTag = false;
+        final List<PhysicalNetworkVO> physicalNetworks = _physicalNetworkDao.listByZoneAndTrafficType(zoneId, TrafficType.Guest);
+        for (final PhysicalNetworkVO physicalNetwork : physicalNetworks) {
+            final List<String> physicalNetworkTags = physicalNetwork.getTags();
+            if (CollectionUtils.isEmpty(physicalNetworkTags)) {
+                if (!allowNullTag) {
+                    allowNullTag = true;
+                } else {
+                    throw new CloudRuntimeException("There are more than 1 physical network with empty tag in the zone id=" + zoneId);
+                }
+            } else {
+                allPhysicalNetworkTags.addAll(physicalNetworkTags);
+            }
+        }
+        return allowNullTag;
+    }
+
+    private boolean checkNetworkOfferingTags(List<String> physicalNetworkTags, boolean allowNullTag, String offeringTags) {
+      return (offeringTags != null || allowNullTag) && (offeringTags == null || physicalNetworkTags.contains(offeringTags));
+    }
+
     @Override
     public boolean isOfferingForVpc(final NetworkOffering offering) {
         return offering.isForVpc();
diff --git a/server/src/main/java/com/cloud/network/NetworkModelImpl.java b/server/src/main/java/com/cloud/network/NetworkModelImpl.java
index df6d8c61c3..8dabe5ce17 100644
--- a/server/src/main/java/com/cloud/network/NetworkModelImpl.java
+++ b/server/src/main/java/com/cloud/network/NetworkModelImpl.java
@@ -1189,26 +1189,31 @@ public class NetworkModelImpl extends ManagerBase implements NetworkModel, Confi
         }
 
         if (pNtwks.size() > 1) {
-            if (tag == null) {
-                throw new InvalidParameterValueException("More than one physical networks exist in zone id=" + zoneId +
-                    " and no tags are specified in order to make a choice");
-            }
+            return getPhysicalNetworkId(zoneId, pNtwks, tag);
+        } else {
+            return pNtwks.get(0).getId();
+        }
+    }
 
-            Long pNtwkId = null;
-            for (PhysicalNetwork pNtwk : pNtwks) {
-                if (pNtwk.getTags().contains(tag)) {
-                    s_logger.debug("Found physical network id=" + pNtwk.getId() + " based on requested tags " + tag);
-                    pNtwkId = pNtwk.getId();
-                    break;
+    private Long getPhysicalNetworkId(long zoneId, List<PhysicalNetworkVO> pNtwks, String tag) {
+        Long pNtwkId = null;
+        for (PhysicalNetwork pNtwk : pNtwks) {
+            if (tag == null && pNtwk.getTags().isEmpty()) {
+                s_logger.debug("Found physical network id=" + pNtwk.getId() + " with null tag");
+                if (pNtwkId != null) {
+                    throw new CloudRuntimeException("There is more than 1 physical network with empty tag in the zone id=" + zoneId);
                 }
+                pNtwkId = pNtwk.getId();
+            } else if (tag != null && pNtwk.getTags().contains(tag)) {
+                s_logger.debug("Found physical network id=" + pNtwk.getId() + " based on requested tags " + tag);
+                pNtwkId = pNtwk.getId();
+                break;
             }
-            if (pNtwkId == null) {
-                throw new InvalidParameterValueException("Unable to find physical network which match the tags " + tag);
-            }
-            return pNtwkId;
-        } else {
-            return pNtwks.get(0).getId();
         }
+        if (pNtwkId == null) {
+            throw new InvalidParameterValueException("Unable to find physical network which match the tags " + tag);
+        }
+        return pNtwkId;
     }
 
     @Override
diff --git a/server/src/main/java/com/cloud/network/NetworkServiceImpl.java b/server/src/main/java/com/cloud/network/NetworkServiceImpl.java
index 8e5001ce62..4b8dd8563f 100644
--- a/server/src/main/java/com/cloud/network/NetworkServiceImpl.java
+++ b/server/src/main/java/com/cloud/network/NetworkServiceImpl.java
@@ -3950,6 +3950,12 @@ public class NetworkServiceImpl extends ManagerBase implements NetworkService, C
             throw new InvalidParameterException("Unable to support more than one tag on network yet");
         }
 
+        // If tags are null, then check if there are any other networks with null tags
+        // of the same traffic type. If so then dont update the tags
+        if (tags != null && tags.size() == 0) {
+            checkForPhysicalNetworksWithoutTag(network);
+        }
+
         PhysicalNetwork.State networkState = null;
         if (state != null && !state.isEmpty()) {
             try {
@@ -3980,6 +3986,18 @@ public class NetworkServiceImpl extends ManagerBase implements NetworkService, C
 
     }
 
+    private void checkForPhysicalNetworksWithoutTag(PhysicalNetworkVO network) {
+        // Get all physical networks according to traffic type
+        Pair<List<PhysicalNetworkTrafficTypeVO>, Integer> result = _pNTrafficTypeDao
+                .listAndCountBy(network.getId());
+        if (result.second() > 0) {
+            for (PhysicalNetworkTrafficTypeVO physicalNetworkTrafficTypeVO : result.first()) {
+                TrafficType trafficType = physicalNetworkTrafficTypeVO.getTrafficType();
+                checkForPhysicalNetworksWithoutTag(network, trafficType);
+            }
+        }
+    }
+
     @DB
     public void addOrRemoveVnets(String[] listOfRanges, final PhysicalNetworkVO network) {
         List<String> addVnets = null;
@@ -4849,36 +4867,30 @@ public class NetworkServiceImpl extends ManagerBase implements NetworkService, C
 
     @Override
     public long findPhysicalNetworkId(long zoneId, String tag, TrafficType trafficType) {
-        List<PhysicalNetworkVO> pNtwks = new ArrayList<PhysicalNetworkVO>();
-        if (trafficType != null) {
-            pNtwks = _physicalNetworkDao.listByZoneAndTrafficType(zoneId, trafficType);
-        } else {
-            pNtwks = _physicalNetworkDao.listByZone(zoneId);
-        }
-
-        if (pNtwks.isEmpty()) {
-            throw new InvalidParameterValueException("Unable to find physical network in zone id=" + zoneId);
-        }
+        return _networkModel.findPhysicalNetworkId(zoneId, tag, trafficType);
+    }
 
-        if (pNtwks.size() > 1) {
-            if (tag == null) {
-                throw new InvalidParameterValueException("More than one physical networks exist in zone id=" + zoneId + " and no tags are specified in order to make a choice");
-            }
+    /**
+     * Function to check if there are any physical networks with traffic type of "trafficType"
+     * and check their tags. If there is more than one network with null tags then throw exception
+     * @param physicalNetwork
+     * @param trafficType
+     */
+    private void checkForPhysicalNetworksWithoutTag(PhysicalNetworkVO physicalNetwork, TrafficType trafficType) {
+        int networkWithoutTagCount = 0;
+        List<PhysicalNetworkVO> physicalNetworkVOList = _physicalNetworkDao
+                .listByZoneAndTrafficType(physicalNetwork.getDataCenterId(), trafficType);
 
-            Long pNtwkId = null;
-            for (PhysicalNetwork pNtwk : pNtwks) {
-                if (pNtwk.getTags().contains(tag)) {
-                    s_logger.debug("Found physical network id=" + pNtwk.getId() + " based on requested tags " + tag);
-                    pNtwkId = pNtwk.getId();
-                    break;
-                }
+        for (PhysicalNetworkVO physicalNetworkVO : physicalNetworkVOList) {
+            List<String> tags = physicalNetworkVO.getTags();
+            if (CollectionUtils.isEmpty(tags)) {
+                networkWithoutTagCount++;
             }
-            if (pNtwkId == null) {
-                throw new InvalidParameterValueException("Unable to find physical network which match the tags " + tag);
-            }
-            return pNtwkId;
-        } else {
-            return pNtwks.get(0).getId();
+        }
+        if (networkWithoutTagCount > 0) {
+            s_logger.error("Number of physical networks without tags are " + networkWithoutTagCount);
+            throw new CloudRuntimeException("There are more than 1 physical network without tags in the zone= " +
+                    physicalNetwork.getDataCenterId());
         }
     }
 
@@ -4929,6 +4941,13 @@ public class NetworkServiceImpl extends ManagerBase implements NetworkService, C
             }
         }
 
+        // Check if there are more than 1 physical network with null tags in same traffic type.
+        // If so then dont allow to add traffic type.
+        List<String> tags = network.getTags();
+        if (CollectionUtils.isEmpty(tags)) {
+            checkForPhysicalNetworksWithoutTag(network, trafficType);
+        }
+
         try {
             // Create the new traffic type in the database
             if (xenLabel == null) {
diff --git a/test/integration/component/test_multiple_physical_network_creation.py b/test/integration/component/test_multiple_physical_network_creation.py
new file mode 100644
index 0000000000..7f6117f3f2
--- /dev/null
+++ b/test/integration/component/test_multiple_physical_network_creation.py
@@ -0,0 +1,405 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+# Import Local Modules
+from nose.plugins.attrib import attr
+from marvin.cloudstackTestCase import cloudstackTestCase, unittest
+from marvin.lib.utils import (validateList,
+                              cleanup_resources,
+                              _execute_ssh_command,
+                              get_host_credentials,
+                              random_gen)
+from marvin.lib.base import (PhysicalNetwork,
+                             Host,
+                             TrafficType,
+                             Domain,
+                             Network,
+                             NetworkOffering,
+                             VirtualMachine,
+                             ServiceOffering,
+                             Zone)
+from marvin.lib.common import (get_domain,
+                               get_zone,
+                               get_template,
+                               list_virtual_machines)
+
+import logging
+
+class TestMulipleNetworkCreation(cloudstackTestCase):
+    @classmethod
+    def setUpClass(cls):
+        cls.testClient = super(
+            TestMulipleNetworkCreation,
+            cls).getClsTestClient()
+        cls.apiclient = cls.testClient.getApiClient()
+        cls.testdata = cls.testClient.getParsedTestDataConfig()
+        cls.services = cls.testClient.getParsedTestDataConfig()
+        cls.domain = get_domain(cls.apiclient)
+        zone = get_zone(cls.apiclient, cls.testClient.getZoneForTests())
+        cls.zone = Zone(zone.__dict__)
+        cls.template = get_template(cls.apiclient, cls.zone.id)
+        cls._cleanup = []
+
+        cls.logger = logging.getLogger("TestMulipleNetworkCreation")
+        cls.stream_handler = logging.StreamHandler()
+        cls.logger.setLevel(logging.DEBUG)
+        cls.logger.addHandler(cls.stream_handler)
+
+        # Disable the zone to create physical networks
+        cls.zone.update(
+            cls.apiclient,
+            allocationstate="Disabled"
+        )
+
+        try:
+            cls.physical_network = PhysicalNetwork.create(
+                cls.apiclient,
+                cls.services["l2-network"],
+                zoneid=cls.zone.id
+            )
+            cls._cleanup.append(cls.physical_network)
+
+            cls.physical_network_2 = PhysicalNetwork.create(
+                cls.apiclient,
+                cls.services["l2-network"],
+                zoneid=cls.zone.id
+            )
+            cls._cleanup.append(cls.physical_network_2)
+        except Exception as e:
+            cls.tearDownClass()
+            raise unittest.SkipTest(e)
+
+        cls.kvmnetworklabel=None
+        try:
+            hosts = Host.list(cls.apiclient, type='Routing')
+            if isinstance(hosts, list) and len(hosts) > 0:
+                host = hosts[0]
+            else:
+                return
+            if host.hypervisor.lower() not in "kvm":
+                return
+            host.user, host.passwd = get_host_credentials(cls.config, host.ipaddress)
+            bridges = _execute_ssh_command(host.ipaddress, 22, host.user, host.passwd, "brctl show |grep cloudbr |awk '{print $1}'")
+            existing_physical_networks = PhysicalNetwork.list(cls.apiclient)
+            for existing_physical_network in existing_physical_networks:
+                trafficTypes = TrafficType.list(
+                    cls.apiclient,
+                    physicalnetworkid=existing_physical_network.id)
+                if trafficTypes is None:
+                    continue
+                for traffic_type in trafficTypes:
+                    if traffic_type.traffictype == "Guest":
+                        try:
+                            for bridge in bridges:
+                                if bridge == str(traffic_type.kvmnetworklabel):
+                                    bridges.remove(bridge)
+                        except Exception as e:
+                            continue
+
+            if bridges is not None and len(bridges) > 0:
+                cls.kvmnetworklabel = bridges[0]
+            if cls.kvmnetworklabel is not None:
+                cls.logger.debug("Found an unused kvmnetworklabel %s" %cls.kvmnetworklabel)
+            else:
+                cls.logger.debug("Not find an unused kvmnetworklabel")
+        except Exception as e:
+            return
+
+    @classmethod
+    def tearDownClass(cls):
+        try:
+            cls.zone.update(
+                cls.apiclient,
+                allocationstate="Enabled"
+            )
+            # Cleanup resources used
+            super(TestMulipleNetworkCreation, cls).tearDownClass()
+        except Exception as e:
+            raise Exception("Warning: Exception during cleanup : %s" % e)
+        return
+
+    def setUp(self):
+        self.apiclient = self.testClient.getApiClient()
+        self.cleanup = []
+
+        return
+
+    def tearDown(self):
+        super(TestMulipleNetworkCreation, self).tearDown()
+
+    @attr(tags=["advanced"], required_hardware="false")
+    def test_01_add_traffictype_for_untagged_networks(self):
+        """
+        Try to add to traffic type Guest to each of the network
+        Two physical networks are already created without tags
+
+        1. Try to add traffic type Guest to physcial network
+        2. Ensure that it throws exception
+        3. Now add the tags to the physical network
+        4. Add traffic type Guest to the physical network and it should not throw exception
+        5. Do the same above steps for the other physical network
+
+        :return:
+        """
+        try:
+            # Add traffic type
+            self.physical_network.addTrafficType(
+                self.apiclient,
+                type="Guest"
+            )
+
+            # If the control comes then there is something wrong with the code.
+            # The code should throw exception as there are multiple networks with null tags
+            raise Exception("Exception should occur when adding traffic type since tags are null")
+        except Exception as e:
+            self.logger.info("Exception happened as expected")
+
+        # Now update the network with tags
+        self.physical_network.update(
+            self.apiclient,
+            tags="Guest"
+        )
+
+        # try adding traffic type again. it should not throw error
+        self.physical_network.addTrafficType(
+            self.apiclient,
+            type="Guest"
+        )
+
+        # Do the same thing for the second network
+        try:
+            self.physical_network_2.addTrafficType(
+                self.apiclient,
+                type="Guest"
+            )
+
+            # It should throw exception and should not come here
+            raise Exception("Exception should occur when adding traffic type since tags are null")
+        except Exception as e:
+            self.logger.info("Exception happened as expected")
+
+        # Now update the network with tags
+        self.physical_network_2.update(
+            self.apiclient,
+            tags="Guest"
+        )
+
+        # try adding traffic type again. it should not throw error
+        self.physical_network_2.addTrafficType(
+            self.apiclient,
+            type="Guest"
+        )
+
+    @attr(tags=["advanced"], required_hardware="false")
+    def test_02_created_shared_guest_network(self):
+        """
+        1. Create new physical network
+        2. Update the network with tags and traffic type "Guest"
+        3. Create a network offering and shared network based on the above physical network
+        4. Create a virtual machine using the above created network
+        4. Ensure that the traffic type is Guest and vlan is same as the shared network
+        :return:
+        """
+        #1. Create a physical network
+        self.physical_network_3 = PhysicalNetwork.create(
+            self.apiclient,
+            self.services["l2-network"],
+            isolationmethods="VLAN",
+            zoneid=self.zone.id
+        )
+        self.cleanup.append(self.physical_network_3)
+
+        # Enable the network
+        self.physical_network_3.update(
+            self.apiclient,
+            tags="guest",
+            state="Enabled"
+        )
+
+        #2. try adding traffic type Guest
+        TrafficType.add(
+            self.apiclient,
+            physicalnetworkid=self.physical_network_3.id,
+            kvmnetworklabel=self.kvmnetworklabel,
+            traffictype="Guest"
+        )
+
+        # Create network offering
+        self.services["network_offering_shared"]["supportedservices"] = ""
+        self.services["network_offering_shared"]["serviceProviderList"] = {}
+        self.services["network_offering_shared"]["tags"] = "guest"
+        self.network_offering = NetworkOffering.create(
+            self.apiclient,
+            self.services["network_offering_shared"]
+        )
+        self.cleanup.append(self.network_offering)
+
+        # Enable network offering
+        self.network_offering.update(
+            self.apiclient,
+            state='Enabled'
+        )
+
+        #3. Create a shared network
+        self.shared_network = Network.create(
+            self.apiclient,
+            self.services["network2"],
+            networkofferingid=self.network_offering.id,
+            zoneid=self.zone.id,
+            domainid=self.domain.id
+            #physicalnetworkid=self.physical_network_3.id
+        )
+        self.cleanup.append(self.shared_network)
+
+        # Create small service offering
+        self.service_offering = ServiceOffering.create(
+            self.apiclient,
+            self.testdata["service_offerings"]["small"]
+        )
+        self.cleanup.append(self.service_offering)
+
+        #4. Create virtual machine
+        self.testdata["virtual_machine"]["zoneid"] = self.zone.id
+        self.testdata["virtual_machine"]["template"] = self.template.id
+        self.virtual_machine = VirtualMachine.create(
+            self.apiclient,
+            self.testdata["virtual_machine"],
+            templateid=self.template.id,
+            serviceofferingid=self.service_offering.id,
+            networkids=self.shared_network.id
+        )
+        self.cleanup.append(self.virtual_machine)
+
+        list_vms = VirtualMachine.list(
+            self.apiclient,
+            id=self.virtual_machine.id
+        )
+
+        vm = list_vms[0]
+        # make sure that vm uses the same vlan of the network
+        self.assertEqual(vm.nic[0].traffictype,
+                         "Guest",
+                         "Vm traffic type is not same")
+
+        self.assertEqual(vm.nic[0].isolationuri,
+                         "vlan://" + str(self.services["network2"]["vlan"]),
+                         "Vm network vlan is not same")
+
+        self.network_offering.update(
+            self.apiclient,
+            state='Disabled'
+        )
+
+        hosts = Host.list(
+            self.apiclient,
+            id=vm.hostid
+        )
+        if isinstance(hosts, list) and len(hosts) > 0:
+            host = hosts[0]
+        else:
+            raise Exception("Cannot find the host where vm is running on")
+        if host.hypervisor.lower() not in "kvm":
+            return
+        if self.kvmnetworklabel is None:
+            return
+        try:
+            host.user, host.passwd = get_host_credentials(self.config, host.ipaddress)
+            physical_nic_kvmnetworklabel= _execute_ssh_command(host.ipaddress, 22, host.user, host.passwd, "brctl show %s | grep cloudbr |awk '{print $4}'" % self.kvmnetworklabel)
+            bridge_name = _execute_ssh_command(host.ipaddress, 22, host.user, host.passwd, "virsh domiflist %s |grep vnet |awk '{print $3}'" % vm.instancename)
+        except Exception as e:
+            return
+
+        if bridge_name is not None and physical_nic_kvmnetworklabel is not None:
+            if bridge_name[0].startswith("br%s-" %physical_nic_kvmnetworklabel[0]):
+                self.logger.debug("vm is running on physical nic %s and bridge %s" % (physical_nic_kvmnetworklabel[0], bridge_name[0]))
+            else:
+                raise Exception("vm should be running on physical nic %s but on bridge" % (physical_nic_kvmnetworklabel[0], bridge_name[0]))
+
+    @attr(tags=["advanced"], required_hardware="false")
+    def test_03_update_network_with_null_tags(self):
+        """
+        Update the tags to NULL for the existing physical network
+        Since the zone has already "Management" network of traffic type Guest without any tags,
+        we cant set the tags to null for the newly created networks
+        :return:
+        """
+        try:
+            # Set tags to null
+            self.physical_network.update(
+                self.apiclient,
+                tags=""
+            )
+
+            # it should throw exception and should not come here
+            raise Exception("Tags cannot be removed from network as there are more than 1 network without tags")
+        except Exception as e:
+            self.logger.info("Exception happened as expected while removing the tags")
+
+        try:
+            # Do the above steps for the second network
+            self.physical_network_2.update(
+                self.apiclient,
+                tags=""
+            )
+
+            # it should throw exception and should not come here
+            raise Exception("Tags cannot be removed from network as there are more than 1 network without tags")
+        except Exception as e:
+            self.logger.info("Exception happened as expected while removing the tags")
+
+    @attr(tags=["advanced"], required_hardware="false")
+    def test_04_add_invalid_network_tags(self):
+        """
+        Try to add other traffic types like "Storage", "Public" and "Management"
+        It should throw exception as there are physcial networks existing with this tag
+        :return:
+        """
+        try:
+            # Add traffic type storage
+            self.physical_network.addTrafficType(
+                self.apiclient,
+                type="Storage"
+            )
+
+            # control should not come here
+            raise Exception("Storage tag should be assigned as there is another network with the same tag")
+        except Exception as e:
+            self.logger.info("Exception happened as expected while adding traffic type")
+
+        try:
+            # Add traffic type Public
+            self.physical_network.addTrafficType(
+                self.apiclient,
+                type="Public"
+            )
+
+            # control should not come here
+            raise Exception("Public tag should be assigned as there is another network with the same tag")
+        except Exception as e:
+            self.logger.info("Exception happened as expected while adding traffic type")
+
+        try:
+            # Add traffic type Management
+            self.physical_network.addTrafficType(
+                self.apiclient,
+                type="Management"
+            )
+
+            # control should not come here
+            raise Exception("Management tag should be assigned as there is another network with the same tag")
+        except Exception as e:
+            self.logger.info("Exception happened as expected while adding traffic type")
diff --git a/tools/marvin/marvin/lib/base.py b/tools/marvin/marvin/lib/base.py
index 28dc33b928..e22bfafa4d 100755
--- a/tools/marvin/marvin/lib/base.py
+++ b/tools/marvin/marvin/lib/base.py
@@ -4175,7 +4175,7 @@ class PhysicalNetwork:
         self.__dict__.update(items)
 
     @classmethod
-    def create(cls, apiclient, services, zoneid, domainid=None):
+    def create(cls, apiclient, services, zoneid, domainid=None, isolationmethods=None):
         """Create physical network"""
         cmd = createPhysicalNetwork.createPhysicalNetworkCmd()
 
@@ -4183,6 +4183,8 @@ class PhysicalNetwork:
         cmd.zoneid = zoneid
         if domainid:
             cmd.domainid = domainid
+        if isolationmethods:
+            cmd.isolationmethods = isolationmethods
         return PhysicalNetwork(apiclient.createPhysicalNetwork(cmd).__dict__)
 
     def delete(self, apiclient):
@@ -5819,6 +5821,12 @@ class TrafficType:
     def __init__(self, items):
         self.__dict__.update(items)
 
+    @classmethod
+    def add(cls, apiclient, **kwargs):
+        cmd = addTrafficType.addTrafficTypeCmd()
+        [setattr(cmd, k, v) for k, v in kwargs.items()]
+        return apiclient.addTrafficType(cmd)
+
     @classmethod
     def list(cls, apiclient, **kwargs):
         """Lists traffic types"""