Refactor network components method.

Author: Fernando Ojeda

SoftLayer/managers/vs.py

@@ -533,7 +533,11 @@ def _generate_create_dict(
         if datacenter:
             data["datacenter"] = {"name": datacenter}
 
-        self.get_network_components(data, kwargs, private_subnet, private_vlan, public_subnet, public_vlan)
+        network_components = self._create_network_components(public_vlan, private_vlan,
+                                                             private_subnet, public_subnet,
+                                                             kwargs.get('private_router'),
+                                                             kwargs.get('public_router'))
+        data.update(network_components)
 
         if public_security_groups:
             secgroups = [{'securityGroup': {'id': int(sg)}}
@@ -576,35 +580,19 @@ def _generate_create_dict(
 
         return data
 
-    def get_network_components(self, data, kwargs, private_subnet, private_vlan, public_subnet, public_vlan):
-        """Get the network components structure.
-
-        :param data: Array variable to add the network structure.
-        :param kwargs: Vs item list.
-        :param int private_subnet: Private subnet id.
-        :param int private_vlan: Private vlan id.
-        :param int public_subnet: Public subnet id.
-        :param int public_vlan: Public vlan id.
-        """
-        if kwargs.get('private_router') or kwargs.get('public_router'):
-            if private_vlan or public_vlan or private_subnet or public_subnet:
-                raise exceptions.SoftLayerError("You have to select network vlan or network vlan with a subnet or "
-                                                "only router, not all options")
-            network_components = self._create_network_components(public_vlan, private_vlan,
-                                                                 private_subnet, public_subnet,
-                                                                 kwargs.get('private_router'),
-                                                                 kwargs.get('public_router'))
-            data.update(network_components)
-        if private_vlan or public_vlan or private_subnet or public_subnet:
-            network_components = self._create_network_components(public_vlan, private_vlan,
-                                                                 private_subnet, public_subnet)
-            data.update(network_components)
-
     def _create_network_components(
             self, public_vlan=None, private_vlan=None,
             private_subnet=None, public_subnet=None,
             private_router=None, public_router=None):
         parameters = {}
+        if any([private_router, public_router]) and any([private_vlan, public_vlan, private_subnet, public_subnet]):
+            raise exceptions.SoftLayerError("You have to select network vlan or network vlan with a subnet or "
+                                            "only router, not all options")
+
+        if private_router:
+            parameters['primaryBackendNetworkComponent'] = {"router": {"id": int(private_router)}}
+        if public_router:
+            parameters['primaryNetworkComponent'] = {"router": {"id": int(public_router)}}
         if private_vlan:
             parameters['primaryBackendNetworkComponent'] = {"networkVlan": {"id": int(private_vlan)}}
         if public_vlan:
@@ -620,12 +608,6 @@ def _create_network_components(
 
             parameters['primaryBackendNetworkComponent']['networkVlan']['primarySubnet'] = {'id': int(private_subnet)}
 
-        if private_router:
-            parameters['primaryBackendNetworkComponent'] = {"router": {"id": int(private_router)}}
-
-        if public_router:
-            parameters['primaryNetworkComponent'] = {"router": {"id": int(public_router)}}
-
         return parameters
 
     @retry(logger=LOGGER)

tests/managers/vs/vs_tests.py

@@ -641,6 +641,18 @@ def test_create_network_components_by_routers(self):
 
         self.assertEqual(data, assert_data)
 
+    def test_create_network_components_by_routers_and_vlan(self):
+        actual = self.assertRaises(
+            exceptions.SoftLayerError,
+            self.vs._create_network_components,
+            private_router=1,
+            public_router=1,
+            private_vlan=1
+        )
+
+        self.assertEqual(str(actual), "You have to select network vlan or network vlan with a subnet or only router, "
+                                      "not all options")
+
     def test_create_network_components_vlan_subnet_private(self):
         data = self.vs._create_network_components(
             private_vlan=1,