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

pearl11594 pushed a commit to branch nsx-fix-code-smells
in repository https://gitbox.apache.org/repos/asf/cloudstack.git

commit 05be11a54f585e1f19e249223392baf650f2f68f
Author: Pearl Dsilva <pearl1...@gmail.com>
AuthorDate: Wed Dec 27 11:27:53 2023 -0500

    NSX: Fix code smells and reported bugs
---
 .../admin/network/CreateNetworkOfferingCmd.java    |   4 +-
 .../command/admin/vpc/CreateVPCOfferingCmd.java    |   4 +-
 .../cloud/configuration/ConfigurationManager.java  |  29 +++---
 .../engine/orchestration/NetworkOrchestrator.java  |   8 +-
 .../cloud/vm/VirtualMachineManagerImplTest.java    |   5 +-
 .../com/cloud/network/element/NsxProviderVO.java   |   1 +
 .../agent/api/CreateNsxDhcpRelayConfigCommand.java |  15 +++
 .../CreateNsxDistributedFirewallRulesCommand.java  |  15 +++
 .../api/CreateNsxLoadBalancerRuleCommand.java      |  15 +++
 .../agent/api/CreateNsxPortForwardRuleCommand.java |  16 ++++
 .../api/CreateOrUpdateNsxTier1NatRuleCommand.java  |  16 ++++
 .../api/DeleteNsxLoadBalancerRuleCommand.java      |  15 +++
 .../agent/api/DeleteNsxNatRuleCommand.java         |  16 ++++
 .../agent/api/DeleteNsxSegmentCommand.java         |  16 ++++
 .../agent/api/DeleteNsxTier1GatewayCommand.java    |  16 ++++
 .../apache/cloudstack/resource/NsxNetworkRule.java |   1 +
 .../apache/cloudstack/resource/NsxResource.java    |  14 +--
 .../apache/cloudstack/service/NsxApiClient.java    |  30 +++---
 .../org/apache/cloudstack/service/NsxElement.java  |  29 ++----
 .../cloudstack/service/NsxGuestNetworkGuru.java    | 103 +++++++++++----------
 .../cloudstack/service/NsxProviderServiceImpl.java |  17 ++--
 .../cloudstack/service/NsxPublicNetworkGuru.java   |  26 ++----
 .../org/apache/cloudstack/utils/NsxHelper.java     |   3 +
 .../cloudstack/service/NsxApiClientTest.java       |   4 +-
 .../service/NsxProviderServiceImplTest.java        |   8 +-
 .../configuration/ConfigurationManagerImpl.java    |   4 +-
 .../com/cloud/hypervisor/HypervisorGuruBase.java   |   2 +-
 27 files changed, 274 insertions(+), 158 deletions(-)

diff --git 
a/api/src/main/java/org/apache/cloudstack/api/command/admin/network/CreateNetworkOfferingCmd.java
 
b/api/src/main/java/org/apache/cloudstack/api/command/admin/network/CreateNetworkOfferingCmd.java
index 5d0a87ef0df..93de44f9291 100644
--- 
a/api/src/main/java/org/apache/cloudstack/api/command/admin/network/CreateNetworkOfferingCmd.java
+++ 
b/api/src/main/java/org/apache/cloudstack/api/command/admin/network/CreateNetworkOfferingCmd.java
@@ -286,7 +286,7 @@ public class CreateNetworkOfferingCmd extends BaseCmd {
         return forVpc;
     }
 
-    public Boolean isForNsx() {
+    public boolean isForNsx() {
         return BooleanUtils.isTrue(forNsx);
     }
 
@@ -314,7 +314,7 @@ public class CreateNetworkOfferingCmd extends BaseCmd {
     }
 
     public Map<String, List<String>> getServiceProviders() {
-        Map<String, List<String>> serviceProviderMap = new HashMap<String, 
List<String>>();
+        Map<String, List<String>> serviceProviderMap = new HashMap<>();
         if (serviceProviderList != null && !serviceProviderList.isEmpty() && 
!isForNsx()) {
             Collection servicesCollection = serviceProviderList.values();
             Iterator iter = servicesCollection.iterator();
diff --git 
a/api/src/main/java/org/apache/cloudstack/api/command/admin/vpc/CreateVPCOfferingCmd.java
 
b/api/src/main/java/org/apache/cloudstack/api/command/admin/vpc/CreateVPCOfferingCmd.java
index e514ddb508b..a6601626803 100644
--- 
a/api/src/main/java/org/apache/cloudstack/api/command/admin/vpc/CreateVPCOfferingCmd.java
+++ 
b/api/src/main/java/org/apache/cloudstack/api/command/admin/vpc/CreateVPCOfferingCmd.java
@@ -163,7 +163,7 @@ public class CreateVPCOfferingCmd extends 
BaseAsyncCreateCmd {
         return supportedServices;
     }
 
-    public Boolean isForNsx() {
+    public boolean isForNsx() {
         return BooleanUtils.isTrue(forNsx);
     }
 
@@ -172,7 +172,7 @@ public class CreateVPCOfferingCmd extends 
BaseAsyncCreateCmd {
     }
 
     public Map<String, List<String>> getServiceProviders() {
-        Map<String, List<String>> serviceProviderMap = new HashMap<String, 
List<String>>();;
+        Map<String, List<String>> serviceProviderMap = new HashMap<>();
         if (serviceProviderList != null && !serviceProviderList.isEmpty() && 
!isForNsx()) {
             Collection<? extends Map<String, String>> servicesCollection = 
serviceProviderList.values();
             Iterator<? extends Map<String, String>> iter = 
servicesCollection.iterator();
diff --git 
a/engine/components-api/src/main/java/com/cloud/configuration/ConfigurationManager.java
 
b/engine/components-api/src/main/java/com/cloud/configuration/ConfigurationManager.java
index c975e8f351f..bbddd8fd471 100644
--- 
a/engine/components-api/src/main/java/com/cloud/configuration/ConfigurationManager.java
+++ 
b/engine/components-api/src/main/java/com/cloud/configuration/ConfigurationManager.java
@@ -63,9 +63,9 @@ public interface ConfigurationManager {
     static final String VM_USERDATA_MAX_LENGTH_STRING = 
"vm.userdata.max.length";
     static final ConfigKey<Integer> VM_USERDATA_MAX_LENGTH = new 
ConfigKey<>("Advanced", Integer.class, VM_USERDATA_MAX_LENGTH_STRING, "32768",
             "Max length of vm userdata after base64 decoding. Default is 32768 
and maximum is 1048576", true);
-    public static final ConfigKey<Boolean> AllowNonRFC1918CompliantIPs = new 
ConfigKey<Boolean>(Boolean.class,
+    public static final ConfigKey<Boolean> AllowNonRFC1918CompliantIPs = new 
ConfigKey<>(Boolean.class,
             "allow.non.rfc1918.compliant.ips", "Advanced", "false",
-            "Allows non-compliant RFC 1918 IPs for Shared, Isolated networks 
and VPCs", true);
+            "Allows non-compliant RFC 1918 IPs for Shared, Isolated networks 
and VPCs", true, null);
 
     /**
      * @param offering
@@ -100,7 +100,6 @@ public interface ConfigurationManager {
 //     * @param volatileVm
 //     * @param hostTag
 //     * @param networkRate
-//     *            TODO
 //     * @param id
 //     * @param useVirtualNetwork
 //     * @param deploymentPlanner
@@ -170,11 +169,9 @@ public interface ConfigurationManager {
      * @param zoneType
      * @param allocationState
      * @param networkDomain
-     *            TODO
      * @param isSecurityGroupEnabled
-     *            TODO
-     * @param ip6Dns1 TODO
-     * @param ip6Dns2 TODO
+     * @param ip6Dns1
+     * @param ip6Dns2
      * @return
      * @throws
      * @throws
@@ -189,7 +186,7 @@ public interface ConfigurationManager {
      *
      * @param userId
      * @param vlanDbId
-     * @param caller TODO
+     * @param caller
      * @return success/failure
      */
     boolean deleteVlanAndPublicIpRange(long userId, long vlanDbId, Account 
caller);
@@ -206,16 +203,16 @@ public interface ConfigurationManager {
      * @param trafficType
      * @param tags
      * @param specifyVlan
-     * @param networkRate        TODO
-     * @param serviceProviderMap TODO
-     * @param isDefault          TODO
-     * @param type               TODO
-     * @param systemOnly         TODO
+     * @param networkRate
+     * @param serviceProviderMap
+     * @param isDefault
+     * @param type
+     * @param systemOnly
      * @param serviceOfferingId
      * @param conserveMode       ;
-     * @param specifyIpRanges    TODO
+     * @param specifyIpRanges
      * @param isPersistent       ;
-     * @param details            TODO
+     * @param details
      * @param forVpc
      * @param forTungsten
      * @param forNsx
@@ -228,7 +225,7 @@ public interface ConfigurationManager {
                                             Integer networkRate, Map<Service, 
Set<Provider>> serviceProviderMap, boolean isDefault, Network.GuestType type, 
boolean systemOnly, Long serviceOfferingId,
                                             boolean conserveMode, Map<Service, 
Map<Capability, String>> serviceCapabilityMap, boolean specifyIpRanges, boolean 
isPersistent,
                                             Map<NetworkOffering.Detail, 
String> details, boolean egressDefaultPolicy, Integer maxconn, boolean 
enableKeepAlive, Boolean forVpc,
-                                            Boolean forTungsten, Boolean 
forNsx, String mode, List<Long> domainIds, List<Long> zoneIds, boolean 
enableOffering, final NetUtils.InternetProtocol internetProtocol);
+                                            Boolean forTungsten, boolean 
forNsx, String mode, List<Long> domainIds, List<Long> zoneIds, boolean 
enableOffering, final NetUtils.InternetProtocol internetProtocol);
 
     Vlan createVlanAndPublicIpRange(long zoneId, long networkId, long 
physicalNetworkId, boolean forVirtualNetwork, boolean forSystemVms, Long podId, 
String startIP, String endIP,
         String vlanGateway, String vlanNetmask, String vlanId, boolean 
bypassVlanOverlapCheck, Domain domain, Account vlanOwner, String startIPv6, 
String endIPv6, String vlanIp6Gateway, String vlanIp6Cidr, boolean forNsx)
diff --git 
a/engine/orchestration/src/main/java/org/apache/cloudstack/engine/orchestration/NetworkOrchestrator.java
 
b/engine/orchestration/src/main/java/org/apache/cloudstack/engine/orchestration/NetworkOrchestrator.java
index 93c5a6020fc..b9873470a53 100644
--- 
a/engine/orchestration/src/main/java/org/apache/cloudstack/engine/orchestration/NetworkOrchestrator.java
+++ 
b/engine/orchestration/src/main/java/org/apache/cloudstack/engine/orchestration/NetworkOrchestrator.java
@@ -1036,7 +1036,7 @@ public class NetworkOrchestrator extends ManagerBase 
implements NetworkOrchestra
         if (profile == null) {
             return null;
         }
-        // TODO: Fix with the public network PR
+
         if (isNicAllocatedForNsxPublicNetworkOnVR(network, profile, vm)) {
             String guruName = "NsxPublicNetworkGuru";
             NetworkGuru nsxGuru = AdapterBase.getAdapterByName(networkGurus, 
guruName);
@@ -1090,7 +1090,6 @@ public class NetworkOrchestrator extends ManagerBase 
implements NetworkOrchestra
         }
 
         Long vpcId = 
_ipAddressDao.findByIp(requested.getIPv4Address()).getVpcId();
-        // TODO: Need to fix isolated network
         List<IPAddressVO> ips = _ipAddressDao.listByAssociatedVpc(vpcId, true);
 
         if (CollectionUtils.isEmpty(ips)) {
@@ -2860,10 +2859,9 @@ public class NetworkOrchestrator extends ManagerBase 
implements NetworkOrchestra
         }
 
         // Check if cidr is RFC1918 compliant if the network is Guest Isolated 
for IPv4
-        if (cidr != null && ntwkOff.getGuestType() == 
Network.GuestType.Isolated && ntwkOff.getTrafficType() == TrafficType.Guest) {
-            if (!NetUtils.validateGuestCidr(cidr, 
!ConfigurationManager.AllowNonRFC1918CompliantIPs.value())) {
+        if (cidr != null && (ntwkOff.getGuestType() == 
Network.GuestType.Isolated && ntwkOff.getTrafficType() == TrafficType.Guest) &&
+                !NetUtils.validateGuestCidr(cidr, 
!ConfigurationManager.AllowNonRFC1918CompliantIPs.value())) {
                 throw new InvalidParameterValueException("Virtual Guest Cidr " 
+ cidr + " is not RFC 1918 or 6598 compliant");
-            }
         }
 
         final String networkDomainFinal = networkDomain;
diff --git 
a/engine/orchestration/src/test/java/com/cloud/vm/VirtualMachineManagerImplTest.java
 
b/engine/orchestration/src/test/java/com/cloud/vm/VirtualMachineManagerImplTest.java
index 452f6cd1f4b..9e9a0df53b0 100644
--- 
a/engine/orchestration/src/test/java/com/cloud/vm/VirtualMachineManagerImplTest.java
+++ 
b/engine/orchestration/src/test/java/com/cloud/vm/VirtualMachineManagerImplTest.java
@@ -992,10 +992,11 @@ public class VirtualMachineManagerImplTest {
         Mockito.when(vpcVO.getId()).thenReturn(4L);
         Mockito.when(networkVO.getId()).thenReturn(5L);
         virtualMachineManagerImpl.setVmNetworkDetails(vm, vmTO);
-        assertEquals(vmTO.getNetworkIdToNetworkNameMap().size(), 1);
-        assertEquals(vmTO.getNetworkIdToNetworkNameMap().get(5L), 
"D3-A2-Z1-V4-S5");
+        assertEquals(1, vmTO.getNetworkIdToNetworkNameMap().size());
+        assertEquals("D3-A2-Z1-V4-S5", 
vmTO.getNetworkIdToNetworkNameMap().get(5L));
     }
 
+    @Test
     public void testOrchestrateStartNonNullPodId() throws Exception {
         VMInstanceVO vmInstance = new VMInstanceVO();
         ReflectionTestUtils.setField(vmInstance, "id", 1L);
diff --git 
a/engine/schema/src/main/java/com/cloud/network/element/NsxProviderVO.java 
b/engine/schema/src/main/java/com/cloud/network/element/NsxProviderVO.java
index 274038b74bb..f08e08b1ca0 100644
--- a/engine/schema/src/main/java/com/cloud/network/element/NsxProviderVO.java
+++ b/engine/schema/src/main/java/com/cloud/network/element/NsxProviderVO.java
@@ -213,6 +213,7 @@ public class NsxProviderVO implements NsxProvider {
 
 
         public Builder() {
+            // Default constructor
         }
 
         public Builder setZoneId(long zoneId) {
diff --git 
a/plugins/network-elements/nsx/src/main/java/org/apache/cloudstack/agent/api/CreateNsxDhcpRelayConfigCommand.java
 
b/plugins/network-elements/nsx/src/main/java/org/apache/cloudstack/agent/api/CreateNsxDhcpRelayConfigCommand.java
index 5b986628628..80f723380e1 100644
--- 
a/plugins/network-elements/nsx/src/main/java/org/apache/cloudstack/agent/api/CreateNsxDhcpRelayConfigCommand.java
+++ 
b/plugins/network-elements/nsx/src/main/java/org/apache/cloudstack/agent/api/CreateNsxDhcpRelayConfigCommand.java
@@ -17,6 +17,7 @@
 package org.apache.cloudstack.agent.api;
 
 import java.util.List;
+import java.util.Objects;
 
 public class CreateNsxDhcpRelayConfigCommand extends NsxCommand {
 
@@ -56,4 +57,18 @@ public class CreateNsxDhcpRelayConfigCommand extends 
NsxCommand {
     public List<String> getAddresses() {
         return addresses;
     }
+
+    @Override
+    public boolean equals(Object o) {
+        if (this == o) return true;
+        if (o == null || getClass() != o.getClass()) return false;
+        if (!super.equals(o)) return false;
+        CreateNsxDhcpRelayConfigCommand that = 
(CreateNsxDhcpRelayConfigCommand) o;
+        return networkId == that.networkId && Objects.equals(vpcId, 
that.vpcId) && Objects.equals(vpcName, that.vpcName) && 
Objects.equals(networkName, that.networkName) && Objects.equals(addresses, 
that.addresses);
+    }
+
+    @Override
+    public int hashCode() {
+        return Objects.hash(super.hashCode(), vpcId, vpcName, networkId, 
networkName, addresses);
+    }
 }
diff --git 
a/plugins/network-elements/nsx/src/main/java/org/apache/cloudstack/agent/api/CreateNsxDistributedFirewallRulesCommand.java
 
b/plugins/network-elements/nsx/src/main/java/org/apache/cloudstack/agent/api/CreateNsxDistributedFirewallRulesCommand.java
index 9283d1292cd..3b730909c9c 100644
--- 
a/plugins/network-elements/nsx/src/main/java/org/apache/cloudstack/agent/api/CreateNsxDistributedFirewallRulesCommand.java
+++ 
b/plugins/network-elements/nsx/src/main/java/org/apache/cloudstack/agent/api/CreateNsxDistributedFirewallRulesCommand.java
@@ -19,6 +19,7 @@ package org.apache.cloudstack.agent.api;
 import org.apache.cloudstack.resource.NsxNetworkRule;
 
 import java.util.List;
+import java.util.Objects;
 
 public class CreateNsxDistributedFirewallRulesCommand extends NsxCommand {
 
@@ -46,4 +47,18 @@ public class CreateNsxDistributedFirewallRulesCommand 
extends NsxCommand {
     public List<NsxNetworkRule> getRules() {
         return rules;
     }
+
+    @Override
+    public boolean equals(Object o) {
+        if (this == o) return true;
+        if (o == null || getClass() != o.getClass()) return false;
+        if (!super.equals(o)) return false;
+        CreateNsxDistributedFirewallRulesCommand that = 
(CreateNsxDistributedFirewallRulesCommand) o;
+        return networkId == that.networkId && Objects.equals(vpcId, 
that.vpcId) && Objects.equals(rules, that.rules);
+    }
+
+    @Override
+    public int hashCode() {
+        return Objects.hash(super.hashCode(), vpcId, networkId, rules);
+    }
 }
diff --git 
a/plugins/network-elements/nsx/src/main/java/org/apache/cloudstack/agent/api/CreateNsxLoadBalancerRuleCommand.java
 
b/plugins/network-elements/nsx/src/main/java/org/apache/cloudstack/agent/api/CreateNsxLoadBalancerRuleCommand.java
index b35bda26c19..8a51f96fd67 100644
--- 
a/plugins/network-elements/nsx/src/main/java/org/apache/cloudstack/agent/api/CreateNsxLoadBalancerRuleCommand.java
+++ 
b/plugins/network-elements/nsx/src/main/java/org/apache/cloudstack/agent/api/CreateNsxLoadBalancerRuleCommand.java
@@ -19,6 +19,7 @@ package org.apache.cloudstack.agent.api;
 import org.apache.cloudstack.resource.NsxLoadBalancerMember;
 
 import java.util.List;
+import java.util.Objects;
 
 public class CreateNsxLoadBalancerRuleCommand extends NsxNetworkCommand {
 
@@ -66,4 +67,18 @@ public class CreateNsxLoadBalancerRuleCommand extends 
NsxNetworkCommand {
     public String getProtocol() {
         return protocol;
     }
+
+    @Override
+    public boolean equals(Object o) {
+        if (this == o) return true;
+        if (o == null || getClass() != o.getClass()) return false;
+        if (!super.equals(o)) return false;
+        CreateNsxLoadBalancerRuleCommand command = 
(CreateNsxLoadBalancerRuleCommand) o;
+        return lbId == command.lbId && Objects.equals(publicPort, 
command.publicPort) && Objects.equals(privatePort, command.privatePort) && 
Objects.equals(algorithm, command.algorithm) && Objects.equals(protocol, 
command.protocol) && Objects.equals(memberList, command.memberList);
+    }
+
+    @Override
+    public int hashCode() {
+        return Objects.hash(super.hashCode(), publicPort, privatePort, 
algorithm, protocol, memberList, lbId);
+    }
 }
diff --git 
a/plugins/network-elements/nsx/src/main/java/org/apache/cloudstack/agent/api/CreateNsxPortForwardRuleCommand.java
 
b/plugins/network-elements/nsx/src/main/java/org/apache/cloudstack/agent/api/CreateNsxPortForwardRuleCommand.java
index 96d5213abd5..35ea1121738 100644
--- 
a/plugins/network-elements/nsx/src/main/java/org/apache/cloudstack/agent/api/CreateNsxPortForwardRuleCommand.java
+++ 
b/plugins/network-elements/nsx/src/main/java/org/apache/cloudstack/agent/api/CreateNsxPortForwardRuleCommand.java
@@ -16,6 +16,8 @@
 // under the License.
 package org.apache.cloudstack.agent.api;
 
+import java.util.Objects;
+
 public class CreateNsxPortForwardRuleCommand extends NsxNetworkCommand {
     private final String publicPort;
     private final String privatePort;
@@ -49,4 +51,18 @@ public class CreateNsxPortForwardRuleCommand extends 
NsxNetworkCommand {
     public String getProtocol() {
         return protocol;
     }
+
+    @Override
+    public boolean equals(Object o) {
+        if (this == o) return true;
+        if (o == null || getClass() != o.getClass()) return false;
+        if (!super.equals(o)) return false;
+        CreateNsxPortForwardRuleCommand that = 
(CreateNsxPortForwardRuleCommand) o;
+        return ruleId == that.ruleId && Objects.equals(publicPort, 
that.publicPort) && Objects.equals(privatePort, that.privatePort) && 
Objects.equals(protocol, that.protocol);
+    }
+
+    @Override
+    public int hashCode() {
+        return Objects.hash(super.hashCode(), publicPort, privatePort, 
protocol, ruleId);
+    }
 }
diff --git 
a/plugins/network-elements/nsx/src/main/java/org/apache/cloudstack/agent/api/CreateOrUpdateNsxTier1NatRuleCommand.java
 
b/plugins/network-elements/nsx/src/main/java/org/apache/cloudstack/agent/api/CreateOrUpdateNsxTier1NatRuleCommand.java
index 7e3c03fd28c..cf2c156e3ca 100644
--- 
a/plugins/network-elements/nsx/src/main/java/org/apache/cloudstack/agent/api/CreateOrUpdateNsxTier1NatRuleCommand.java
+++ 
b/plugins/network-elements/nsx/src/main/java/org/apache/cloudstack/agent/api/CreateOrUpdateNsxTier1NatRuleCommand.java
@@ -16,6 +16,8 @@
 // under the License.
 package org.apache.cloudstack.agent.api;
 
+import java.util.Objects;
+
 public class CreateOrUpdateNsxTier1NatRuleCommand extends NsxCommand {
 
     private String tier1GatewayName;
@@ -47,4 +49,18 @@ public class CreateOrUpdateNsxTier1NatRuleCommand extends 
NsxCommand {
     public String getNatRuleId() {
         return natRuleId;
     }
+
+    @Override
+    public boolean equals(Object o) {
+        if (this == o) return true;
+        if (o == null || getClass() != o.getClass()) return false;
+        if (!super.equals(o)) return false;
+        CreateOrUpdateNsxTier1NatRuleCommand that = 
(CreateOrUpdateNsxTier1NatRuleCommand) o;
+        return Objects.equals(tier1GatewayName, that.tier1GatewayName) && 
Objects.equals(action, that.action) && Objects.equals(translatedIpAddress, 
that.translatedIpAddress) && Objects.equals(natRuleId, that.natRuleId);
+    }
+
+    @Override
+    public int hashCode() {
+        return Objects.hash(super.hashCode(), tier1GatewayName, action, 
translatedIpAddress, natRuleId);
+    }
 }
diff --git 
a/plugins/network-elements/nsx/src/main/java/org/apache/cloudstack/agent/api/DeleteNsxLoadBalancerRuleCommand.java
 
b/plugins/network-elements/nsx/src/main/java/org/apache/cloudstack/agent/api/DeleteNsxLoadBalancerRuleCommand.java
index 21296aff991..a89842881ed 100644
--- 
a/plugins/network-elements/nsx/src/main/java/org/apache/cloudstack/agent/api/DeleteNsxLoadBalancerRuleCommand.java
+++ 
b/plugins/network-elements/nsx/src/main/java/org/apache/cloudstack/agent/api/DeleteNsxLoadBalancerRuleCommand.java
@@ -19,6 +19,7 @@ package org.apache.cloudstack.agent.api;
 import org.apache.cloudstack.resource.NsxLoadBalancerMember;
 
 import java.util.List;
+import java.util.Objects;
 
 public class DeleteNsxLoadBalancerRuleCommand extends NsxNetworkCommand {
     private long lbId;
@@ -37,4 +38,18 @@ public class DeleteNsxLoadBalancerRuleCommand extends 
NsxNetworkCommand {
     }
 
     public List<NsxLoadBalancerMember> getMemberList() { return memberList; }
+
+    @Override
+    public boolean equals(Object o) {
+        if (this == o) return true;
+        if (o == null || getClass() != o.getClass()) return false;
+        if (!super.equals(o)) return false;
+        DeleteNsxLoadBalancerRuleCommand that = 
(DeleteNsxLoadBalancerRuleCommand) o;
+        return lbId == that.lbId && Objects.equals(memberList, 
that.memberList);
+    }
+
+    @Override
+    public int hashCode() {
+        return Objects.hash(super.hashCode(), lbId, memberList);
+    }
 }
diff --git 
a/plugins/network-elements/nsx/src/main/java/org/apache/cloudstack/agent/api/DeleteNsxNatRuleCommand.java
 
b/plugins/network-elements/nsx/src/main/java/org/apache/cloudstack/agent/api/DeleteNsxNatRuleCommand.java
index 82ca54d5b0d..f054c24f6c0 100644
--- 
a/plugins/network-elements/nsx/src/main/java/org/apache/cloudstack/agent/api/DeleteNsxNatRuleCommand.java
+++ 
b/plugins/network-elements/nsx/src/main/java/org/apache/cloudstack/agent/api/DeleteNsxNatRuleCommand.java
@@ -18,6 +18,8 @@ package org.apache.cloudstack.agent.api;
 
 import com.cloud.network.Network;
 
+import java.util.Objects;
+
 public class DeleteNsxNatRuleCommand extends NsxNetworkCommand {
     private Long ruleId;
     private Network.Service service;
@@ -51,4 +53,18 @@ public class DeleteNsxNatRuleCommand extends 
NsxNetworkCommand {
     public String getProtocol() {
         return protocol;
     }
+
+    @Override
+    public boolean equals(Object o) {
+        if (this == o) return true;
+        if (o == null || getClass() != o.getClass()) return false;
+        if (!super.equals(o)) return false;
+        DeleteNsxNatRuleCommand that = (DeleteNsxNatRuleCommand) o;
+        return Objects.equals(ruleId, that.ruleId) && Objects.equals(service, 
that.service) && Objects.equals(privatePort, that.privatePort) && 
Objects.equals(protocol, that.protocol);
+    }
+
+    @Override
+    public int hashCode() {
+        return Objects.hash(super.hashCode(), ruleId, service, privatePort, 
protocol);
+    }
 }
diff --git 
a/plugins/network-elements/nsx/src/main/java/org/apache/cloudstack/agent/api/DeleteNsxSegmentCommand.java
 
b/plugins/network-elements/nsx/src/main/java/org/apache/cloudstack/agent/api/DeleteNsxSegmentCommand.java
index 06af4d8c31d..bcb00680f31 100644
--- 
a/plugins/network-elements/nsx/src/main/java/org/apache/cloudstack/agent/api/DeleteNsxSegmentCommand.java
+++ 
b/plugins/network-elements/nsx/src/main/java/org/apache/cloudstack/agent/api/DeleteNsxSegmentCommand.java
@@ -16,6 +16,8 @@
 // under the License.
 package org.apache.cloudstack.agent.api;
 
+import java.util.Objects;
+
 public class DeleteNsxSegmentCommand extends NsxCommand {
 
     private Long vpcId;
@@ -48,4 +50,18 @@ public class DeleteNsxSegmentCommand extends NsxCommand {
     public String getNetworkName() {
         return networkName;
     }
+
+    @Override
+    public boolean equals(Object o) {
+        if (this == o) return true;
+        if (o == null || getClass() != o.getClass()) return false;
+        if (!super.equals(o)) return false;
+        DeleteNsxSegmentCommand command = (DeleteNsxSegmentCommand) o;
+        return networkId == command.networkId && Objects.equals(vpcId, 
command.vpcId) && Objects.equals(vpcName, command.vpcName) && 
Objects.equals(networkName, command.networkName);
+    }
+
+    @Override
+    public int hashCode() {
+        return Objects.hash(super.hashCode(), vpcId, vpcName, networkId, 
networkName);
+    }
 }
diff --git 
a/plugins/network-elements/nsx/src/main/java/org/apache/cloudstack/agent/api/DeleteNsxTier1GatewayCommand.java
 
b/plugins/network-elements/nsx/src/main/java/org/apache/cloudstack/agent/api/DeleteNsxTier1GatewayCommand.java
index 2cfb3c3fe08..30af305954c 100644
--- 
a/plugins/network-elements/nsx/src/main/java/org/apache/cloudstack/agent/api/DeleteNsxTier1GatewayCommand.java
+++ 
b/plugins/network-elements/nsx/src/main/java/org/apache/cloudstack/agent/api/DeleteNsxTier1GatewayCommand.java
@@ -16,6 +16,8 @@
 // under the License.
 package org.apache.cloudstack.agent.api;
 
+import java.util.Objects;
+
 public class DeleteNsxTier1GatewayCommand extends NsxCommand {
 
     private Long networkResourceId;
@@ -41,4 +43,18 @@ public class DeleteNsxTier1GatewayCommand extends NsxCommand 
{
     public boolean isResourceVpc() {
         return isResourceVpc;
     }
+
+    @Override
+    public boolean equals(Object o) {
+        if (this == o) return true;
+        if (o == null || getClass() != o.getClass()) return false;
+        if (!super.equals(o)) return false;
+        DeleteNsxTier1GatewayCommand that = (DeleteNsxTier1GatewayCommand) o;
+        return isResourceVpc == that.isResourceVpc && 
Objects.equals(networkResourceId, that.networkResourceId) && 
Objects.equals(networkResourceName, that.networkResourceName);
+    }
+
+    @Override
+    public int hashCode() {
+        return Objects.hash(super.hashCode(), networkResourceId, 
networkResourceName, isResourceVpc);
+    }
 }
diff --git 
a/plugins/network-elements/nsx/src/main/java/org/apache/cloudstack/resource/NsxNetworkRule.java
 
b/plugins/network-elements/nsx/src/main/java/org/apache/cloudstack/resource/NsxNetworkRule.java
index bceee68389c..c11141db9d4 100644
--- 
a/plugins/network-elements/nsx/src/main/java/org/apache/cloudstack/resource/NsxNetworkRule.java
+++ 
b/plugins/network-elements/nsx/src/main/java/org/apache/cloudstack/resource/NsxNetworkRule.java
@@ -252,6 +252,7 @@ public class NsxNetworkRule {
         private Network.Service service;
 
         public Builder() {
+            // Default constructor
         }
 
         public Builder setDomainId(long domainId) {
diff --git 
a/plugins/network-elements/nsx/src/main/java/org/apache/cloudstack/resource/NsxResource.java
 
b/plugins/network-elements/nsx/src/main/java/org/apache/cloudstack/resource/NsxResource.java
index d36e9ee5809..a90266f7164 100644
--- 
a/plugins/network-elements/nsx/src/main/java/org/apache/cloudstack/resource/NsxResource.java
+++ 
b/plugins/network-elements/nsx/src/main/java/org/apache/cloudstack/resource/NsxResource.java
@@ -252,7 +252,7 @@ public class NsxResource implements ServerResource {
     }
 
     private Answer executeRequest(CreateNsxDhcpRelayConfigCommand cmd) {
-        long zoneId = cmd.getZoneId();
+        long datacenterId = cmd.getZoneId();
         long domainId = cmd.getDomainId();
         long accountId = cmd.getAccountId();
         Long vpcId = cmd.getVpcId();
@@ -261,7 +261,7 @@ public class NsxResource implements ServerResource {
         String networkName = cmd.getNetworkName();
         List<String> addresses = cmd.getAddresses();
 
-        String dhcpRelayConfigName = 
NsxControllerUtils.getNsxDhcpRelayConfigId(zoneId, domainId, accountId, vpcId, 
networkId);
+        String dhcpRelayConfigName = 
NsxControllerUtils.getNsxDhcpRelayConfigId(datacenterId, domainId, accountId, 
vpcId, networkId);
 
         String msg = String.format("Creating DHCP relay config with name %s on 
network %s of VPC %s",
                 dhcpRelayConfigName, networkName, vpcName);
@@ -275,7 +275,7 @@ public class NsxResource implements ServerResource {
             return new NsxAnswer(cmd, e);
         }
 
-        String segmentName = NsxControllerUtils.getNsxSegmentId(domainId, 
accountId, zoneId, vpcId, networkId);
+        String segmentName = NsxControllerUtils.getNsxSegmentId(domainId, 
accountId, datacenterId, vpcId, networkId);
         String dhcpConfigPath = String.format("%s/%s", 
DHCP_RELAY_CONFIGS_PATH_PREFIX, dhcpRelayConfigName);
         try {
             Segment segment = nsxApiClient.getSegmentById(segmentName);
@@ -295,13 +295,13 @@ public class NsxResource implements ServerResource {
     }
 
     private Answer executeRequest(CreateNsxTier1GatewayCommand cmd) {
-        String name = 
NsxControllerUtils.getTier1GatewayName(cmd.getDomainId(), cmd.getAccountId(), 
cmd.getZoneId(), cmd.getNetworkResourceId(), cmd.isResourceVpc());
+        String tier1GatewayName = 
NsxControllerUtils.getTier1GatewayName(cmd.getDomainId(), cmd.getAccountId(), 
cmd.getZoneId(), cmd.getNetworkResourceId(), cmd.isResourceVpc());
         boolean sourceNatEnabled = cmd.isSourceNatEnabled();
         try {
-            nsxApiClient.createTier1Gateway(name, tier0Gateway, edgeCluster, 
sourceNatEnabled);
+            nsxApiClient.createTier1Gateway(tier1GatewayName, tier0Gateway, 
edgeCluster, sourceNatEnabled);
             return new NsxAnswer(cmd, true, "");
         } catch (CloudRuntimeException e) {
-            String msg = String.format("Cannot create tier 1 gateway %s (%s: 
%s): %s", name,
+            String msg = String.format("Cannot create tier 1 gateway %s (%s: 
%s): %s", tier1GatewayName,
                     (cmd.isResourceVpc() ? "VPC" : "NETWORK"), 
cmd.getNetworkResourceName(), e.getMessage());
             LOGGER.error(msg);
             return new NsxAnswer(cmd, e);
@@ -457,7 +457,7 @@ public class NsxResource implements ServerResource {
                 cmd.getZoneId(), cmd.getNetworkResourceId(), 
cmd.isResourceVpc());
         String ruleName = 
NsxControllerUtils.getLoadBalancerRuleName(tier1GatewayName, cmd.getLbId());
         try {
-            nsxApiClient.deleteNsxLbResources(tier1GatewayName, cmd.getLbId(), 
cmd.getVmId());
+            nsxApiClient.deleteNsxLbResources(tier1GatewayName, cmd.getLbId());
         } catch (Exception e) {
             LOGGER.error(String.format("Failed to add NSX load balancer rule 
%s for network: %s", ruleName, cmd.getNetworkResourceName()));
             return new NsxAnswer(cmd, new 
CloudRuntimeException(e.getMessage()));
diff --git 
a/plugins/network-elements/nsx/src/main/java/org/apache/cloudstack/service/NsxApiClient.java
 
b/plugins/network-elements/nsx/src/main/java/org/apache/cloudstack/service/NsxApiClient.java
index 5a3cf48aa04..ef42c754c6a 100644
--- 
a/plugins/network-elements/nsx/src/main/java/org/apache/cloudstack/service/NsxApiClient.java
+++ 
b/plugins/network-elements/nsx/src/main/java/org/apache/cloudstack/service/NsxApiClient.java
@@ -105,7 +105,7 @@ public class NsxApiClient {
 
     // Constants
     private static final String TIER_1_RESOURCE_TYPE = "Tier1";
-    private static final String Tier_1_LOCALE_SERVICE_ID = "default";
+    private static final String TIER_1_LOCALE_SERVICE_ID = "default";
     private static final String SEGMENT_RESOURCE_TYPE = "Segment";
     private static final String TIER_0_GATEWAY_PATH_PREFIX = "/infra/tier-0s/";
     private static final String TIER_1_GATEWAY_PATH_PREFIX = "/infra/tier-1s/";
@@ -115,8 +115,6 @@ public class NsxApiClient {
 
     private enum PoolAllocation { ROUTING, LB_SMALL, LB_MEDIUM, LB_LARGE, 
LB_XLARGE }
 
-    private enum TYPE { ROUTED, NATTED }
-
     private enum HAMode { ACTIVE_STANDBY, ACTIVE_ACTIVE }
 
     private enum FailoverMode { PREEMPTIVE, NON_PREEMPTIVE }
@@ -155,10 +153,6 @@ public class NsxApiClient {
         JUMP_TO_APPLICATION
     }
 
-    private Map<String,String> actionMap = Map.of(
-            "Allow", FirewallActions.ALLOW.name(),
-            "Deny", FirewallActions.DROP.name());
-
     public enum  RouteAdvertisementType { TIER1_STATIC_ROUTES, 
TIER1_CONNECTED, TIER1_NAT,
         TIER1_LB_VIP, TIER1_LB_SNAT, TIER1_DNS_FORWARDER_IP, 
TIER1_IPSEC_LOCAL_ENDPOINT
     }
@@ -272,7 +266,7 @@ public class NsxApiClient {
             com.vmware.nsx_policy.infra.tier_1s.LocaleServices 
tier1LocalService = (com.vmware.nsx_policy.infra.tier_1s.LocaleServices) 
nsxService.apply(com.vmware.nsx_policy.infra.tier_1s.LocaleServices.class);
             com.vmware.nsx_policy.model.LocaleServices localeService = new 
com.vmware.nsx_policy.model.LocaleServices.Builder()
                     
.setEdgeClusterPath(localeServices.get(0).getEdgeClusterPath()).build();
-            tier1LocalService.patch(tier1Id, Tier_1_LOCALE_SERVICE_ID, 
localeService);
+            tier1LocalService.patch(tier1Id, TIER_1_LOCALE_SERVICE_ID, 
localeService);
         } catch (Error error) {
             throw new CloudRuntimeException(String.format("Failed to 
instantiate tier-1 gateway %s in edge cluster %s", tier1Id, edgeCluster));
         }
@@ -329,7 +323,7 @@ public class NsxApiClient {
             return;
         }
         removeTier1GatewayNatRules(tier1Id);
-        localeService.delete(tier1Id, Tier_1_LOCALE_SERVICE_ID);
+        localeService.delete(tier1Id, TIER_1_LOCALE_SERVICE_ID);
         Tier1s tier1service = (Tier1s) nsxService.apply(Tier1s.class);
         tier1service.delete(tier1Id);
     }
@@ -355,7 +349,7 @@ public class NsxApiClient {
             Sites sites = (Sites) nsxService.apply(Sites.class);
             return sites.list(null, false, null, null, null, null);
         } catch (Exception e) {
-            throw new CloudRuntimeException(String.format("Failed to fetch 
service segment list due to %s", e.getMessage()));
+            throw new CloudRuntimeException(String.format("Failed to fetch 
sites list due to %s", e.getMessage()));
         }
     }
 
@@ -364,7 +358,7 @@ public class NsxApiClient {
             EnforcementPoints enforcementPoints = (EnforcementPoints) 
nsxService.apply(EnforcementPoints.class);
             return enforcementPoints.list(siteId, null, false, null, null, 
null, null);
         } catch (Exception e) {
-            throw new CloudRuntimeException(String.format("Failed to fetch 
service segment list due to %s", e.getMessage()));
+            throw new CloudRuntimeException(String.format("Failed to fetch 
enforcement points due to %s", e.getMessage()));
         }
     }
 
@@ -373,7 +367,7 @@ public class NsxApiClient {
             com.vmware.nsx.TransportZones transportZones = 
(com.vmware.nsx.TransportZones) 
nsxService.apply(com.vmware.nsx.TransportZones.class);
             return transportZones.list(null, null, true, null, null, null, 
null, null, TransportType.OVERLAY.name(), null);
         } catch (Exception e) {
-            throw new CloudRuntimeException(String.format("Failed to fetch 
service segment list due to %s", e.getMessage()));
+            throw new CloudRuntimeException(String.format("Failed to fetch 
transport zones due to %s", e.getMessage()));
         }
     }
 
@@ -532,7 +526,7 @@ public class NsxApiClient {
         }
     }
 
-    public void createNsxLoadBalancer(String tier1GatewayName, long lbId) {
+    public void createNsxLoadBalancer(String tier1GatewayName) {
         try {
             String lbName = getLoadBalancerName(tier1GatewayName);
             LbServices lbServices = (LbServices) 
nsxService.apply(LbServices.class);
@@ -561,7 +555,7 @@ public class NsxApiClient {
         try {
             String lbServerPoolName = getServerPoolName(tier1GatewayName, 
lbId);
             createNsxLbServerPool(memberList, tier1GatewayName, 
lbServerPoolName, algorithm);
-            createNsxLoadBalancer(tier1GatewayName, lbId);
+            createNsxLoadBalancer(tier1GatewayName);
 
             String lbVirtualServerName = 
getVirtualServerName(tier1GatewayName, lbId);
             String lbServiceName = getLoadBalancerName(tier1GatewayName);
@@ -584,7 +578,7 @@ public class NsxApiClient {
         }
     }
 
-    public void deleteNsxLbResources(String tier1GatewayName, long lbId, long 
vmId) {
+    public void deleteNsxLbResources(String tier1GatewayName, long lbId) {
         try {
             // Delete associated Virtual servers
             LbVirtualServers lbVirtualServers = (LbVirtualServers) 
nsxService.apply(LbVirtualServers.class);
@@ -879,12 +873,14 @@ public class NsxApiClient {
         List<String> segmentGroup = List.of(String.format("%s/%s", 
GROUPS_PATH_PREFIX, segmentName));
         List<String> sourceCidrList = rule.getSourceCidrList();
         List<String> destCidrList = rule.getDestinationCidrList();
+        List<String> ingressSource = (rule.getService() == 
Network.Service.NetworkACL ? segmentGroup : destCidrList);
+        List<String> egressSource = (rule.getService() == 
Network.Service.NetworkACL ? sourceCidrList : destCidrList);
 
         String trafficType = rule.getTrafficType();
         if (trafficType.equalsIgnoreCase("ingress")) {
-            return source ? sourceCidrList : (rule.getService() == 
Network.Service.NetworkACL ? segmentGroup : destCidrList);
+            return source ? sourceCidrList : ingressSource;
         } else if (trafficType.equalsIgnoreCase("egress")) {
-            return source ? segmentGroup : (rule.getService() == 
Network.Service.NetworkACL ? sourceCidrList : destCidrList);
+            return source ? segmentGroup : egressSource;
        }
         String err = String.format("Unsupported traffic type %s", trafficType);
         LOGGER.error(err);
diff --git 
a/plugins/network-elements/nsx/src/main/java/org/apache/cloudstack/service/NsxElement.java
 
b/plugins/network-elements/nsx/src/main/java/org/apache/cloudstack/service/NsxElement.java
index c398312563d..bce62848e3e 100644
--- 
a/plugins/network-elements/nsx/src/main/java/org/apache/cloudstack/service/NsxElement.java
+++ 
b/plugins/network-elements/nsx/src/main/java/org/apache/cloudstack/service/NsxElement.java
@@ -234,9 +234,6 @@ public class NsxElement extends AdapterBase implements  
DhcpServiceProvider, Dns
 
     @Override
     public boolean implement(Network network, NetworkOffering offering, 
DeployDestination dest, ReservationContext context) throws 
ConcurrentOperationException, ResourceUnavailableException, 
InsufficientCapacityException {
-//        Account account = accountMgr.getAccount(network.getAccountId());
-//        DomainVO domain = domainDao.findById(network.getDomainId());
-//        return nsxService.createNetwork(network.getDataCenterId(), 
account.getId(), domain.getId(), network.getId(), network.getName());
         // TODO: Check if the network is NSX based (was already implemented as 
part of the guru.setup()
         return true;
     }
@@ -485,8 +482,6 @@ public class NsxElement extends AdapterBase implements  
DhcpServiceProvider, Dns
             if (vm == null || 
networkModel.getNicInNetworkIncludingRemoved(vm.getId(), config.getId()) == 
null) {
                 continue;
             }
-            Nic nic = networkModel.getNicInNetworkIncludingRemoved(vm.getId(), 
config.getId());
-            Network publicNetwork = 
networkModel.getSystemNetworkByZoneAndTrafficType(config.getDataCenterId(), 
Networks.TrafficType.Public);
             Pair<VpcVO, NetworkVO> vpcOrNetwork = 
getVpcOrNetwork(config.getVpcId(), config.getId());
             VpcVO vpc = vpcOrNetwork.first();
             NetworkVO network = vpcOrNetwork.second();
@@ -544,14 +539,10 @@ public class NsxElement extends AdapterBase implements  
DhcpServiceProvider, Dns
                     .setRuleId(rule.getId())
                     .setProtocol(rule.getProtocol().toUpperCase(Locale.ROOT))
                     .build();
-            if (rule.getState() == FirewallRule.State.Add) {
-                if (!nsxService.createPortForwardRule(networkRule)) {
-                    return false;
-                }
-            } else if (rule.getState() == FirewallRule.State.Revoke) {
-                if (!nsxService.deletePortForwardRule(networkRule)) {
-                    return false;
-                }
+            if (rule.getState() == FirewallRule.State.Add && 
!nsxService.createPortForwardRule(networkRule)) {
+                return false;
+            } else if (rule.getState() == FirewallRule.State.Revoke && 
!nsxService.deletePortForwardRule(networkRule)) {
+                return false;
             }
         }
         return true;
@@ -632,14 +623,10 @@ public class NsxElement extends AdapterBase implements  
DhcpServiceProvider, Dns
                     
.setProtocol(loadBalancingRule.getProtocol().toUpperCase(Locale.ROOT))
                     .setAlgorithm(loadBalancingRule.getAlgorithm())
                     .build();
-            if (loadBalancingRule.getState() == FirewallRule.State.Add) {
-                if (!nsxService.createLbRule(networkRule)) {
-                    return false;
-                }
-            } else if (loadBalancingRule.getState() == 
FirewallRule.State.Revoke) {
-                if (!nsxService.deleteLbRule(networkRule)) {
-                    return false;
-                }
+            if (loadBalancingRule.getState() == FirewallRule.State.Add && 
!nsxService.createLbRule(networkRule)) {
+                return false;
+            } else if (loadBalancingRule.getState() == 
FirewallRule.State.Revoke && !nsxService.deleteLbRule(networkRule)) {
+                return false;
             }
         }
         return true;
diff --git 
a/plugins/network-elements/nsx/src/main/java/org/apache/cloudstack/service/NsxGuestNetworkGuru.java
 
b/plugins/network-elements/nsx/src/main/java/org/apache/cloudstack/service/NsxGuestNetworkGuru.java
index 064bf404f47..8cf1d283fc7 100644
--- 
a/plugins/network-elements/nsx/src/main/java/org/apache/cloudstack/service/NsxGuestNetworkGuru.java
+++ 
b/plugins/network-elements/nsx/src/main/java/org/apache/cloudstack/service/NsxGuestNetworkGuru.java
@@ -197,64 +197,65 @@ public class NsxGuestNetworkGuru extends GuestNetworkGuru 
implements NetworkMigr
     @Override
     public NicProfile allocate(Network network, NicProfile nic, 
VirtualMachineProfile vm) throws InsufficientVirtualNetworkCapacityException, 
InsufficientAddressCapacityException {
         NicProfile nicProfile = super.allocate(network, nic, vm);
+        if (vm.getType() != VirtualMachine.Type.DomainRouter) {
+            return nicProfile;
+        }
 
-        if (vm.getType() == VirtualMachine.Type.DomainRouter) {
-            final DataCenter zone = _dcDao.findById(network.getDataCenterId());
-            long zoneId = network.getDataCenterId();
-            if (Objects.isNull(zone)) {
-                String msg = String.format("Unable to find zone with id: %s", 
zoneId);
-                LOGGER.error(msg);
-                throw new CloudRuntimeException(msg);
-            }
-            Account account = accountDao.findById(network.getAccountId());
-            if (Objects.isNull(account)) {
-                String msg = String.format("Unable to find account with id: 
%s", network.getAccountId());
-                LOGGER.error(msg);
-                throw new CloudRuntimeException(msg);
-            }
-            VpcVO vpc = _vpcDao.findById(network.getVpcId());
-            if (Objects.isNull(vpc)) {
-                String msg = String.format("Unable to find VPC with id: %s, 
allocating for network %s", network.getVpcId(), network.getName());
-                LOGGER.debug(msg);
-            }
-
-            DomainVO domain = domainDao.findById(account.getDomainId());
-            if (Objects.isNull(domain)) {
-                String msg = String.format("Unable to find domain with id: 
%s", account.getDomainId());
-                LOGGER.error(msg);
-                throw new CloudRuntimeException(msg);
-            }
+        final DataCenter zone = _dcDao.findById(network.getDataCenterId());
+        long zoneId = network.getDataCenterId();
+        if (Objects.isNull(zone)) {
+            String msg = String.format("Unable to find zone with id: %s", 
zoneId);
+            LOGGER.error(msg);
+            throw new CloudRuntimeException(msg);
+        }
+        Account account = accountDao.findById(network.getAccountId());
+        if (Objects.isNull(account)) {
+            String msg = String.format("Unable to find account with id: %s", 
network.getAccountId());
+            LOGGER.error(msg);
+            throw new CloudRuntimeException(msg);
+        }
+        VpcVO vpc = _vpcDao.findById(network.getVpcId());
+        if (Objects.isNull(vpc)) {
+            String msg = String.format("Unable to find VPC with id: %s, 
allocating for network %s", network.getVpcId(), network.getName());
+            LOGGER.debug(msg);
+        }
 
-            if (isNull(network.getVpcId())) {
-                long domainId = domain.getId();
-                long accountId = account.getId();
-                long dataCenterId = zone.getId();
-                long resourceId = network.getId();
-                PublicIpAddress ipAddress = 
networkModel.getSourceNatIpAddressForGuestNetwork(account, network);
-                String translatedIp = ipAddress.getAddress().addr();
-                String tier1GatewayName = 
NsxControllerUtils.getTier1GatewayName(domainId, accountId, dataCenterId, 
resourceId, false);
-                LOGGER.debug(String.format("Creating NSX NAT Rule for Tier1 GW 
%s for translated IP %s for Isolated network %s", tier1GatewayName, 
translatedIp, network.getName()));
-                String natRuleId = 
NsxControllerUtils.getNsxNatRuleId(domainId, accountId, dataCenterId, 
resourceId, false);
-                CreateOrUpdateNsxTier1NatRuleCommand cmd = 
NsxHelper.createOrUpdateNsxNatRuleCommand(domainId, accountId, dataCenterId, 
tier1GatewayName, "SNAT", translatedIp, natRuleId);
-                NsxAnswer nsxAnswer = nsxControllerUtils.sendNsxCommand(cmd, 
dataCenterId);
-                if (!nsxAnswer.getResult()) {
-                    String msg = String.format("Could not create NSX NAT Rule 
on Tier1 Gateway %s for IP %s  for Isolated network %s", tier1GatewayName, 
translatedIp, network.getName());
-                    LOGGER.error(msg);
-                    throw new CloudRuntimeException(msg);
-                }
-            }
+        DomainVO domain = domainDao.findById(account.getDomainId());
+        if (Objects.isNull(domain)) {
+            String msg = String.format("Unable to find domain with id: %s", 
account.getDomainId());
+            LOGGER.error(msg);
+            throw new CloudRuntimeException(msg);
+        }
 
-            // Create the DHCP relay config for the segment
-            String iPv4Address = nicProfile.getIPv4Address();
-            List<String> addresses = List.of(iPv4Address);
-            CreateNsxDhcpRelayConfigCommand command = 
NsxHelper.createNsxDhcpRelayConfigCommand(domain, account, zone, vpc, network, 
addresses);
-            NsxAnswer answer = nsxControllerUtils.sendNsxCommand(command, 
zone.getId());
-            if (!answer.getResult()) {
-                String msg = String.format("Error creating DHCP relay config 
for network %s and nic %s: %s", network.getName(), nic.getName(), 
answer.getDetails());
+        if (isNull(network.getVpcId())) {
+            long domainId = domain.getId();
+            long accountId = account.getId();
+            long dataCenterId = zone.getId();
+            long resourceId = network.getId();
+            PublicIpAddress ipAddress = 
networkModel.getSourceNatIpAddressForGuestNetwork(account, network);
+            String translatedIp = ipAddress.getAddress().addr();
+            String tier1GatewayName = 
NsxControllerUtils.getTier1GatewayName(domainId, accountId, dataCenterId, 
resourceId, false);
+            LOGGER.debug(String.format("Creating NSX NAT Rule for Tier1 GW %s 
for translated IP %s for Isolated network %s", tier1GatewayName, translatedIp, 
network.getName()));
+            String natRuleId = NsxControllerUtils.getNsxNatRuleId(domainId, 
accountId, dataCenterId, resourceId, false);
+            CreateOrUpdateNsxTier1NatRuleCommand cmd = 
NsxHelper.createOrUpdateNsxNatRuleCommand(domainId, accountId, dataCenterId, 
tier1GatewayName, "SNAT", translatedIp, natRuleId);
+            NsxAnswer nsxAnswer = nsxControllerUtils.sendNsxCommand(cmd, 
dataCenterId);
+            if (!nsxAnswer.getResult()) {
+                String msg = String.format("Could not create NSX NAT Rule on 
Tier1 Gateway %s for IP %s  for Isolated network %s", tier1GatewayName, 
translatedIp, network.getName());
                 LOGGER.error(msg);
                 throw new CloudRuntimeException(msg);
             }
         }
+
+        // Create the DHCP relay config for the segment
+        String iPv4Address = nicProfile.getIPv4Address();
+        List<String> addresses = List.of(iPv4Address);
+        CreateNsxDhcpRelayConfigCommand command = 
NsxHelper.createNsxDhcpRelayConfigCommand(domain, account, zone, vpc, network, 
addresses);
+        NsxAnswer answer = nsxControllerUtils.sendNsxCommand(command, 
zone.getId());
+        if (!answer.getResult()) {
+            String msg = String.format("Error creating DHCP relay config for 
network %s and nic %s: %s", network.getName(), nic.getName(), 
answer.getDetails());
+            LOGGER.error(msg);
+            throw new CloudRuntimeException(msg);
+        }
         return nicProfile;
     }
 
diff --git 
a/plugins/network-elements/nsx/src/main/java/org/apache/cloudstack/service/NsxProviderServiceImpl.java
 
b/plugins/network-elements/nsx/src/main/java/org/apache/cloudstack/service/NsxProviderServiceImpl.java
index b5c0c6f3ff4..773e44cf4c3 100644
--- 
a/plugins/network-elements/nsx/src/main/java/org/apache/cloudstack/service/NsxProviderServiceImpl.java
+++ 
b/plugins/network-elements/nsx/src/main/java/org/apache/cloudstack/service/NsxProviderServiceImpl.java
@@ -17,6 +17,7 @@
 package org.apache.cloudstack.service;
 
 import com.amazonaws.util.CollectionUtils;
+import com.cloud.configuration.ConfigurationManager;
 import com.cloud.dc.DataCenterVO;
 import com.cloud.dc.dao.DataCenterDao;
 import com.cloud.exception.InvalidParameterValueException;
@@ -42,6 +43,7 @@ import 
org.apache.cloudstack.api.command.ListNsxControllersCmd;
 import org.apache.cloudstack.api.BaseResponse;
 import org.apache.cloudstack.api.command.AddNsxControllerCmd;
 import org.apache.cloudstack.api.response.NsxControllerResponse;
+import 
org.apache.cloudstack.engine.orchestration.service.NetworkOrchestrationService;
 import org.apache.cloudstack.resource.NsxResource;
 import org.apache.commons.lang3.StringUtils;
 
@@ -191,20 +193,21 @@ public class NsxProviderServiceImpl implements 
NsxProviderService {
 
     @Override
     public List<Class<?>> getCommands() {
-        List<Class<?>> cmdList = new ArrayList<Class<?>>();
-        cmdList.add(AddNsxControllerCmd.class);
-        cmdList.add(ListNsxControllersCmd.class);
-        cmdList.add(DeleteNsxControllerCmd.class);
+        List<Class<?>> cmdList = new ArrayList<>();
+        if 
(Boolean.TRUE.equals(NetworkOrchestrationService.NSX_ENABLED.value())) {
+            cmdList.add(AddNsxControllerCmd.class);
+            cmdList.add(ListNsxControllersCmd.class);
+            cmdList.add(DeleteNsxControllerCmd.class);
+        }
         return cmdList;
     }
 
     @VisibleForTesting
     void validateNetworkState(List<NetworkVO> networkList) {
         for (NetworkVO network : networkList) {
-            if (network.getBroadcastDomainType() == 
Networks.BroadcastDomainType.NSX) {
-                if ((network.getState() != Network.State.Shutdown) && 
(network.getState() != Network.State.Destroy)) {
+            if (network.getBroadcastDomainType() == 
Networks.BroadcastDomainType.NSX &&
+                ((network.getState() != Network.State.Shutdown) && 
(network.getState() != Network.State.Destroy))) {
                     throw new CloudRuntimeException("This NSX Controller 
cannot be deleted as there are one or more logical networks provisioned by 
CloudStack on it.");
-                }
             }
         }
     }
diff --git 
a/plugins/network-elements/nsx/src/main/java/org/apache/cloudstack/service/NsxPublicNetworkGuru.java
 
b/plugins/network-elements/nsx/src/main/java/org/apache/cloudstack/service/NsxPublicNetworkGuru.java
index 74926ffd380..2e8127526b3 100644
--- 
a/plugins/network-elements/nsx/src/main/java/org/apache/cloudstack/service/NsxPublicNetworkGuru.java
+++ 
b/plugins/network-elements/nsx/src/main/java/org/apache/cloudstack/service/NsxPublicNetworkGuru.java
@@ -74,6 +74,7 @@ public class NsxPublicNetworkGuru extends PublicNetworkGuru {
         super();
     }
 
+    @Override
     protected boolean canHandle(NetworkOffering offering) {
         return isMyTrafficType(offering.getTrafficType()) && 
offering.isSystemOnly() && offering.isForNsx();
     }
@@ -85,19 +86,15 @@ public class NsxPublicNetworkGuru extends PublicNetworkGuru 
{
         }
 
         if (offering.getTrafficType() == Networks.TrafficType.Public) {
-            NetworkVO ntwk =
-                    new NetworkVO(offering.getTrafficType(), 
Networks.Mode.Static, network.getBroadcastDomainType(), offering.getId(), 
Network.State.Setup, plan.getDataCenterId(),
+            return new NetworkVO(offering.getTrafficType(), 
Networks.Mode.Static, network.getBroadcastDomainType(), offering.getId(), 
Network.State.Setup, plan.getDataCenterId(),
                             plan.getPhysicalNetworkId(), 
offering.isRedundantRouter());
-            return ntwk;
-        } else {
-            return null;
         }
+        return null;
     }
 
     @Override
     public NicProfile allocate(Network network, NicProfile nic, 
VirtualMachineProfile vm) throws InsufficientVirtualNetworkCapacityException, 
InsufficientAddressCapacityException, ConcurrentOperationException {
         s_logger.debug("NSX Public network guru: allocate");
-        // NicProfile createdProfile = super.allocate(network, nic, vm);
 
         IPAddressVO ipAddress = _ipAddressDao.findByIp(nic.getIPv4Address());
         if (ipAddress == null) {
@@ -107,10 +104,6 @@ public class NsxPublicNetworkGuru extends 
PublicNetworkGuru {
         }
         Long vpcId = ipAddress.getVpcId();
         boolean isForVpc = vpcId != null;
-        if (vpcId == null) {
-            // TODO: Pass network.getId() to support isolated networks
-            throw new CloudRuntimeException("TODO: Add support for isolated 
networks public network");
-        }
         VpcVO vpc = vpcDao.findById(vpcId);
         if (vpc == null) {
             String err = String.format("Cannot find a VPC with ID %s", vpcId);
@@ -119,7 +112,7 @@ public class NsxPublicNetworkGuru extends PublicNetworkGuru 
{
         }
 
         // For NSX, use VR Public IP != Source NAT
-        List<IPAddressVO> ips = _ipAddressDao.listByAssociatedVpc(vpcId, true);
+        List<IPAddressVO> ips = _ipAddressDao.listByAssociatedVpc(vpc.getId(), 
true);
         if (CollectionUtils.isEmpty(ips)) {
             String err = String.format("Cannot find a source NAT IP for the 
VPC %s", vpc.getName());
             s_logger.error(err);
@@ -134,7 +127,7 @@ public class NsxPublicNetworkGuru extends PublicNetworkGuru 
{
                 long accountId = vpc.getAccountId();
                 long domainId = vpc.getDomainId();
                 long dataCenterId = vpc.getZoneId();
-                long resourceId = isForVpc ? vpc.getId() : network.getId();
+                long resourceId = vpc.getId();
                 Network.Service[] services = { Network.Service.SourceNat };
                 boolean sourceNatEnabled = 
vpcOfferingServiceMapDao.areServicesSupportedByVpcOffering(vpc.getVpcOfferingId(),
 services);
 
@@ -147,13 +140,8 @@ public class NsxPublicNetworkGuru extends 
PublicNetworkGuru {
                 }
 
                 boolean hasNatSupport = false;
-                if (vpc == null) {
-                    NetworkOffering offering = 
offeringDao.findById(network.getNetworkOfferingId());
-                    hasNatSupport = 
NetworkOffering.NsxMode.NATTED.name().equals(offering.getNsxMode());
-                } else {
-                    VpcOffering vpcOffering = 
vpcOfferingDao.findById(vpc.getVpcOfferingId());
-                    hasNatSupport = 
NetworkOffering.NsxMode.NATTED.name().equals(vpcOffering.getNsxMode());
-                }
+                VpcOffering vpcOffering = 
vpcOfferingDao.findById(vpc.getVpcOfferingId());
+                hasNatSupport = 
NetworkOffering.NsxMode.NATTED.name().equals(vpcOffering.getNsxMode());
 
                 if (!hasNatSupport) {
                     return nic;
diff --git 
a/plugins/network-elements/nsx/src/main/java/org/apache/cloudstack/utils/NsxHelper.java
 
b/plugins/network-elements/nsx/src/main/java/org/apache/cloudstack/utils/NsxHelper.java
index 1d1dd033928..92e089e4849 100644
--- 
a/plugins/network-elements/nsx/src/main/java/org/apache/cloudstack/utils/NsxHelper.java
+++ 
b/plugins/network-elements/nsx/src/main/java/org/apache/cloudstack/utils/NsxHelper.java
@@ -31,6 +31,9 @@ import java.util.List;
 
 public class NsxHelper {
 
+    private NsxHelper() {
+    }
+
     public static CreateNsxDhcpRelayConfigCommand 
createNsxDhcpRelayConfigCommand(DomainVO domain, Account account, DataCenter 
zone, VpcVO vpc, Network network, List<String> addresses) {
         Long vpcId = vpc != null ? vpc.getId() : null;
         String vpcName = vpc != null ? vpc.getName() : null;
diff --git 
a/plugins/network-elements/nsx/src/test/java/org/apache/cloudstack/service/NsxApiClientTest.java
 
b/plugins/network-elements/nsx/src/test/java/org/apache/cloudstack/service/NsxApiClientTest.java
index 603f5fd628d..a0fde08ade8 100644
--- 
a/plugins/network-elements/nsx/src/test/java/org/apache/cloudstack/service/NsxApiClientTest.java
+++ 
b/plugins/network-elements/nsx/src/test/java/org/apache/cloudstack/service/NsxApiClientTest.java
@@ -61,9 +61,9 @@ public class NsxApiClientTest {
         ) {
             String segmentName = "segment1";
             client.createGroupForSegment(segmentName);
-            
Mockito.verify(groupService).patch(Mockito.eq(NsxApiClient.DEFAULT_DOMAIN), 
Mockito.eq(segmentName), Mockito.eq(groups[0]));
+            Mockito.verify(groupService).patch(NsxApiClient.DEFAULT_DOMAIN, 
segmentName, groups[0]);
             String segmentPath = String.format("%s/%s", 
NsxApiClient.SEGMENTS_PATH, segmentName);
-            
Mockito.verify(groups[0]).setExpression(Mockito.eq(List.of(pathExpressions[0])));
+            
Mockito.verify(groups[0]).setExpression(List.of(pathExpressions[0]));
             Mockito.verify(pathExpressions[0]).setPaths(List.of(segmentPath));
         }
     }
diff --git 
a/plugins/network-elements/nsx/src/test/java/org/apache/cloudstack/service/NsxProviderServiceImplTest.java
 
b/plugins/network-elements/nsx/src/test/java/org/apache/cloudstack/service/NsxProviderServiceImplTest.java
index 6a18592885d..cb6f6511d24 100644
--- 
a/plugins/network-elements/nsx/src/test/java/org/apache/cloudstack/service/NsxProviderServiceImplTest.java
+++ 
b/plugins/network-elements/nsx/src/test/java/org/apache/cloudstack/service/NsxProviderServiceImplTest.java
@@ -125,10 +125,10 @@ public class NsxProviderServiceImplTest {
 
         NsxControllerResponse response = 
nsxProviderService.createNsxControllerResponse(nsxProvider);
 
-        assertEquals(response.getEdgeCluster(), "EdgeCluster");
-        assertEquals(response.getTier0Gateway(), "Tier0Gw");
-        assertEquals(response.getTransportZone(), "Overlay");
-        assertEquals(response.getZoneName(), "ZoneNSX");
+        assertEquals("EdgeCluster", response.getEdgeCluster());
+        assertEquals("Tier0Gw", response.getTier0Gateway());
+        assertEquals("Overlay", response.getTransportZone());
+        assertEquals("ZoneNSX", response.getZoneName());
     }
 
     @Test
diff --git 
a/server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java 
b/server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java
index 37df5c2de1d..09e7e96bd88 100644
--- a/server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java
+++ b/server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java
@@ -4880,7 +4880,7 @@ public class ConfigurationManagerImpl extends ManagerBase 
implements Configurati
         if (isSharedNetworkWithoutSpecifyVlan) {
             bypassVlanOverlapCheck = true;
         }
-        if (!bypassVlanOverlapCheck && !forNsx && _zoneDao.findVnet(zoneId, 
physicalNetworkId, 
BroadcastDomainType.getValue(BroadcastDomainType.fromString(vlanId))).size() > 
0) {
+        if (!bypassVlanOverlapCheck && !forNsx && !_zoneDao.findVnet(zoneId, 
physicalNetworkId, 
BroadcastDomainType.getValue(BroadcastDomainType.fromString(vlanId))).isEmpty())
 {
             throw new InvalidParameterValueException("The VLAN tag " + vlanId 
+ " is already being used for dynamic vlan allocation for the guest network in 
zone "
                     + zone.getName());
         }
@@ -6418,7 +6418,7 @@ public class ConfigurationManagerImpl extends ManagerBase 
implements Configurati
                                                    final Long 
serviceOfferingId,
                                                    final boolean conserveMode, 
final Map<Service, Map<Capability, String>> serviceCapabilityMap, final boolean 
specifyIpRanges, final boolean isPersistent,
                                                    final Map<Detail, String> 
details, final boolean egressDefaultPolicy, final Integer maxconn, final 
boolean enableKeepAlive, Boolean forVpc,
-                                                   Boolean forTungsten, 
Boolean forNsx, String mode, final List<Long> domainIds, final List<Long> 
zoneIds, final boolean enableOffering, final NetUtils.InternetProtocol 
internetProtocol) {
+                                                   Boolean forTungsten, 
boolean forNsx, String mode, final List<Long> domainIds, final List<Long> 
zoneIds, final boolean enableOffering, final NetUtils.InternetProtocol 
internetProtocol) {
 
         String servicePackageUuid;
         String spDescription = null;
diff --git a/server/src/main/java/com/cloud/hypervisor/HypervisorGuruBase.java 
b/server/src/main/java/com/cloud/hypervisor/HypervisorGuruBase.java
index e0a7a4a18f4..55b98f82163 100644
--- a/server/src/main/java/com/cloud/hypervisor/HypervisorGuruBase.java
+++ b/server/src/main/java/com/cloud/hypervisor/HypervisorGuruBase.java
@@ -182,7 +182,7 @@ public abstract class HypervisorGuruBase extends 
AdapterBase implements Hypervis
             throw new CloudRuntimeException(String.format("Failed to find 
domain with ID: %s", network.getDomainId()));
         }
         VpcVO vpc = null;
-        if (Objects.nonNull(network) && Objects.nonNull(network.getVpcId())) {
+        if (Objects.nonNull(network.getVpcId())) {
             vpc = vpcDao.findById(network.getVpcId());
         }
         to.setNetworkSegmentName(getNetworkName(zone.getId(), domain.getId(), 
account.getId(), vpc, network.getId()));

Reply via email to