NSX Integration fixes (#8906)

* Prevent addition of duplicate PF rules on scale up and no rules left behind on scale down (#32)

* fix missing dependency injection

* NSX: Fix concurrency issues on port forwarding rules deletion (#37)

* Fix concurrency issues on port forwarding rules deletion

* Refactor objectExists

* Fix unit test

* Fix test

* Small fixes

* CKS: Externalize control and worker node setup wait time and installation attempts (#38)

* NSX: Add shared network support (#41)

* NSX: Fix number of physical networks for Guest traffic checks and leftover rules on CKS cluster deletion (#45)

* Fix pf rules removal on CKS cluster deletion

* Fix check for number of physical networks for guest traffic

* Fix unit test

* fix logger

* NSX: Handle CheckHealthCommand to avoid host disconnection and errors on APIs

* NSX: Handle CheckHealthCommand to avoid host disconnection and errors on APIs

* Remove unused string

* fix logger

* Update UDP active monitor to ICMP

* Fix NPE on restarting VPC with additional public IPs

* NSX / VPC: Reuse Source NAT IP from systemVM range on restarts

* CKS: Public IP not found for VPC networks

* Externalize retries and inverval for NSX segment deletion (#67)

* remove unused import

* remove duplicate imports

* remove unused import

* revert externalizing cks settings

* fix test

* Refactor log messages

* Address comments

* Fix issue caused due to forward merge: 90fe1d

---------

Co-authored-by: Nicolas Vazquez <nicovazquez90@gmail.com>
Co-authored-by: Rohit Yadav <rohit.yadav@shapeblue.com>

api/src/main/java/com/cloud/network/nsx/NsxService.java

@@ -18,9 +18,19 @@ package com.cloud.network.nsx;
 
 import com.cloud.network.IpAddress;
 import com.cloud.network.vpc.Vpc;
+import org.apache.cloudstack.framework.config.ConfigKey;
 
 public interface NsxService {
 
+    ConfigKey<Integer> NSX_API_FAILURE_RETRIES = new ConfigKey<>("Advanced", Integer.class,
+            "nsx.api.failure.retries", "30",
+            "Number of retries for NSX API operations in case of failures",
+            true, ConfigKey.Scope.Zone);
+    ConfigKey<Integer> NSX_API_FAILURE_INTERVAL = new ConfigKey<>("Advanced", Integer.class,
+            "nsx.api.failure.interval", "60",
+            "Waiting time (in seconds) before retrying an NSX API operation in case of failure",
+            true, ConfigKey.Scope.Zone);
+
     boolean createVpcNetwork(Long zoneId, long accountId, long domainId, Long vpcId, String vpcName, boolean sourceNatEnabled);
     boolean updateVpcSourceNatIp(Vpc vpc, IpAddress address);
 }

engine/components-api/src/main/java/com/cloud/network/vpc/VpcManager.java

@@ -115,7 +115,8 @@ public interface VpcManager {
             throws ConcurrentOperationException, InsufficientCapacityException, ResourceAllocationException;
 
     /**
-     * Assigns source nat public IP address to VPC
+     * Assigns source nat public IP address to VPC.
+     * In case of NSX backed VPCs: CloudStack deploys VRs with Public NIC IP different to the VPC source NAT IP, the source NAT IP is on the NSX Public range
      *
      * @param owner
      * @param vpc
@@ -123,7 +124,7 @@ public interface VpcManager {
      * @throws InsufficientAddressCapacityException
      * @throws ConcurrentOperationException
      */
-    PublicIp assignSourceNatIpAddressToVpc(Account owner, Vpc vpc) throws InsufficientAddressCapacityException, ConcurrentOperationException;
+    PublicIp assignSourceNatIpAddressToVpc(Account owner, Vpc vpc, Long podId) throws InsufficientAddressCapacityException, ConcurrentOperationException;
 
     /**
      * Validates network offering to find if it can be used for network creation in VPC

engine/orchestration/src/main/java/org/apache/cloudstack/engine/orchestration/NetworkOrchestrator.java

@@ -1645,7 +1645,7 @@ public class NetworkOrchestrator extends ManagerBase implements NetworkOrchestra
                 if (ips.isEmpty()) {
                     final Vpc vpc = _vpcMgr.getActiveVpc(network.getVpcId());
                     logger.debug("Creating a source nat ip for vpc {}", vpc);
-                    _vpcMgr.assignSourceNatIpAddressToVpc(owner, vpc);
+                    _vpcMgr.assignSourceNatIpAddressToVpc(owner, vpc, null);
                 }
             } else {
                 ips = _ipAddressDao.listByAssociatedNetwork(network.getId(), true);

engine/schema/src/main/java/org/apache/cloudstack/resourcedetail/FirewallRuleDetailVO.java

@@ -79,4 +79,8 @@ public class FirewallRuleDetailVO implements ResourceDetail {
     public boolean isDisplay() {
         return display;
     }
+
+    public void setValue(String value) {
+        this.value = value;
+    }
 }

plugins/affinity-group-processors/host-affinity/src/main/java/org/apache/cloudstack/affinity/HostAffinityProcessor.java

@@ -63,7 +63,9 @@ public class HostAffinityProcessor extends AffinityProcessorBase implements Affi
             Transaction.execute(new TransactionCallbackNoReturn() {
                 @Override
                 public void doInTransactionWithoutResult(TransactionStatus status) {
-                    _affinityGroupDao.listByIds(affinityGroupIdList, true);
+                    if (!affinityGroupIdList.isEmpty()) {
+                        _affinityGroupDao.listByIds(affinityGroupIdList, true);
+                    }
                     for (AffinityGroupVMMapVO vmGroupMapping : vmGroupMappings) {
                         processAffinityGroup(vmGroupMapping, plan, vm, vmList);
                     }
@@ -149,7 +151,9 @@ public class HostAffinityProcessor extends AffinityProcessorBase implements Affi
         return Transaction.execute(new TransactionCallback<Boolean>() {
             @Override
             public Boolean doInTransaction(TransactionStatus status) {
-                _affinityGroupDao.listByIds(affinityGroupIds, true);
+                if (!affinityGroupIds.isEmpty()) {
+                    _affinityGroupDao.listByIds(affinityGroupIds, true);
+                }
                 for (AffinityGroupVMMapVO vmGroupMapping : vmGroupMappings) {
                     if (!checkAffinityGroup(vmGroupMapping, vm, plannedHostId)) {
                         return false;

plugins/affinity-group-processors/host-anti-affinity/src/main/java/org/apache/cloudstack/affinity/HostAntiAffinityProcessor.java

@@ -78,7 +78,9 @@ public class HostAntiAffinityProcessor extends AffinityProcessorBase implements
         Transaction.execute(new TransactionCallbackNoReturn() {
             @Override
             public void doInTransactionWithoutResult(TransactionStatus status) {
-                _affinityGroupDao.listByIds(affinityGroupIds, true);
+                if (!affinityGroupIds.isEmpty()) {
+                    _affinityGroupDao.listByIds(affinityGroupIds, true);
+                }
                 for (AffinityGroupVMMapVO vmGroupMapping : vmGroupMappings) {
                     processAffinityGroup(vmGroupMapping, avoid, vm);
                 }
@@ -165,7 +167,9 @@ public class HostAntiAffinityProcessor extends AffinityProcessorBase implements
         return Transaction.execute(new TransactionCallback<Boolean>() {
             @Override
             public Boolean doInTransaction(TransactionStatus status) {
-                _affinityGroupDao.listByIds(affinityGroupIds, true);
+                if (!affinityGroupIds.isEmpty()) {
+                    _affinityGroupDao.listByIds(affinityGroupIds, true);
+                }
                 for (AffinityGroupVMMapVO vmGroupMapping : vmGroupMappings) {
                     // if more than 1 VM's are present in the group then check for
                     // conflict due to parallel deployment

plugins/integrations/kubernetes-service/src/main/java/com/cloud/kubernetes/cluster/actionworkers/KubernetesClusterActionWorker.java

@@ -360,7 +360,7 @@ public class KubernetesClusterActionWorker {
             return null;
         }
         IpAddress address = ipAddressDao.findByUuid(detailsVO.getValue());
-        if (address == null || network.getVpcId() != address.getVpcId()) {
+        if (address == null || !Objects.equals(network.getVpcId(), address.getVpcId())) {
             logger.warn(String.format("Public IP with ID: %s linked to the Kubernetes cluster: %s is not usable", detailsVO.getValue(), kubernetesCluster.getName()));
             return null;
         }

plugins/integrations/kubernetes-service/src/main/java/com/cloud/kubernetes/cluster/actionworkers/KubernetesClusterResourceModifierActionWorker.java

@@ -17,6 +17,31 @@
 
 package com.cloud.kubernetes.cluster.actionworkers;
 
+import static com.cloud.utils.NumbersUtil.toHumanReadableSize;
+
+import java.io.File;
+import java.io.IOException;
+import java.util.ArrayList;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.concurrent.ConcurrentHashMap;
+import java.util.stream.Collectors;
+
+import javax.inject.Inject;
+
+import com.cloud.network.rules.FirewallManager;
+import com.cloud.offering.NetworkOffering;
+import com.cloud.offerings.dao.NetworkOfferingDao;
+import org.apache.cloudstack.api.ApiConstants;
+import org.apache.cloudstack.api.BaseCmd;
+import org.apache.cloudstack.api.command.user.firewall.CreateFirewallRuleCmd;
+import org.apache.cloudstack.api.command.user.network.CreateNetworkACLCmd;
+import org.apache.cloudstack.api.command.user.volume.ResizeVolumeCmd;
+import org.apache.commons.codec.binary.Base64;
+import org.apache.commons.collections.CollectionUtils;
+import org.apache.commons.lang3.StringUtils;
+
 import com.cloud.capacity.CapacityManager;
 import com.cloud.dc.ClusterDetailsDao;
 import com.cloud.dc.ClusterDetailsVO;
@@ -61,9 +86,7 @@ import com.cloud.network.vpc.NetworkACLItem;
 import com.cloud.network.vpc.NetworkACLItemDao;
 import com.cloud.network.vpc.NetworkACLItemVO;
 import com.cloud.network.vpc.NetworkACLService;
-import com.cloud.offering.NetworkOffering;
 import com.cloud.offering.ServiceOffering;
-import com.cloud.offerings.dao.NetworkOfferingDao;
 import com.cloud.resource.ResourceManager;
 import com.cloud.storage.Volume;
 import com.cloud.storage.VolumeApiService;
@@ -88,29 +111,9 @@ import com.cloud.vm.VirtualMachine;
 import com.cloud.vm.VmDetailConstants;
 import com.cloud.vm.dao.VMInstanceDao;
 import org.apache.cloudstack.api.ApiCommandResourceType;
-import org.apache.cloudstack.api.ApiConstants;
-import org.apache.cloudstack.api.BaseCmd;
-import org.apache.cloudstack.api.command.user.firewall.CreateFirewallRuleCmd;
-import org.apache.cloudstack.api.command.user.network.CreateNetworkACLCmd;
-import org.apache.cloudstack.api.command.user.volume.ResizeVolumeCmd;
 import org.apache.cloudstack.context.CallContext;
-import org.apache.commons.codec.binary.Base64;
-import org.apache.commons.collections.CollectionUtils;
-import org.apache.commons.lang3.StringUtils;
 import org.apache.logging.log4j.Level;
 
-import javax.inject.Inject;
-import java.io.File;
-import java.io.IOException;
-import java.util.ArrayList;
-import java.util.HashMap;
-import java.util.List;
-import java.util.Map;
-import java.util.concurrent.ConcurrentHashMap;
-import java.util.stream.Collectors;
-
-import static com.cloud.utils.NumbersUtil.toHumanReadableSize;
-
 public class KubernetesClusterResourceModifierActionWorker extends KubernetesClusterActionWorker {
 
     @Inject
@@ -134,6 +137,8 @@ public class KubernetesClusterResourceModifierActionWorker extends KubernetesClu
     @Inject
     protected RulesService rulesService;
     @Inject
+    protected FirewallManager firewallManager;
+    @Inject
     protected PortForwardingRulesDao portForwardingRulesDao;
     @Inject
     protected ResourceManager resourceManager;
@@ -169,6 +174,7 @@ public class KubernetesClusterResourceModifierActionWorker extends KubernetesClu
         final String joinIpKey = "{{ k8s_control_node.join_ip }}";
         final String clusterTokenKey = "{{ k8s_control_node.cluster.token }}";
         final String ejectIsoKey = "{{ k8s.eject.iso }}";
+
         String pubKey = "- \"" + configurationDao.getValue("ssh.publickey") + "\"";
         String sshKeyPair = kubernetesCluster.getKeyPair();
         if (StringUtils.isNotEmpty(sshKeyPair)) {
@@ -181,7 +187,6 @@ public class KubernetesClusterResourceModifierActionWorker extends KubernetesClu
         k8sNodeConfig = k8sNodeConfig.replace(joinIpKey, joinIp);
         k8sNodeConfig = k8sNodeConfig.replace(clusterTokenKey, KubernetesClusterUtil.generateClusterToken(kubernetesCluster));
         k8sNodeConfig = k8sNodeConfig.replace(ejectIsoKey, String.valueOf(ejectIso));
-
         k8sNodeConfig = updateKubeConfigWithRegistryDetails(k8sNodeConfig);
 
         return k8sNodeConfig;
@@ -522,17 +527,22 @@ public class KubernetesClusterResourceModifierActionWorker extends KubernetesClu
 
     protected void removePortForwardingRules(final IpAddress publicIp, final Network network, final Account account, final List<Long> removedVMIds) throws ResourceUnavailableException {
         if (!CollectionUtils.isEmpty(removedVMIds)) {
+            List<PortForwardingRuleVO> pfRules = new ArrayList<>();
+            List<PortForwardingRuleVO> revokedRules = new ArrayList<>();
             for (Long vmId : removedVMIds) {
-                List<PortForwardingRuleVO> pfRules = portForwardingRulesDao.listByNetwork(network.getId());
+                pfRules.addAll(portForwardingRulesDao.listByNetwork(network.getId()));
                 for (PortForwardingRuleVO pfRule : pfRules) {
                     if (pfRule.getVirtualMachineId() == vmId) {
                         portForwardingRulesDao.remove(pfRule.getId());
+                        logger.trace("Marking PF rule {} with Revoke state", pfRule);
+                        pfRule.setState(FirewallRule.State.Revoke);
+                        revokedRules.add(pfRule);
                         logger.debug("The Port forwarding rule [%s] with the id [%s] was removed.", pfRule.getName(), pfRule.getId());
                         break;
                     }
                 }
             }
-            rulesService.applyPortForwardingRules(publicIp.getId(), account);
+            firewallManager.applyRules(revokedRules, false, true);
         }
     }
 
@@ -542,10 +552,11 @@ public class KubernetesClusterResourceModifierActionWorker extends KubernetesClu
         for (PortForwardingRuleVO pfRule : pfRules) {
             if (startPort <= pfRule.getSourcePortStart() && pfRule.getSourcePortStart() <= endPort) {
                 portForwardingRulesDao.remove(pfRule.getId());
-                logger.debug("The Port forwarding rule [{}] with the id [{}] was removed.", pfRule.getName(), pfRule.getId());
+                logger.debug("The Port forwarding rule [{}] with the id [{}] was mark as revoked.", pfRule.getName(), pfRule.getId());
+                pfRule.setState(FirewallRule.State.Revoke);
             }
         }
-        rulesService.applyPortForwardingRules(publicIp.getId(), account);
+        firewallManager.applyRules(pfRules, false, true);
     }
 
     protected void removeLoadBalancingRule(final IpAddress publicIp, final Network network,

plugins/integrations/kubernetes-service/src/main/java/com/cloud/kubernetes/cluster/actionworkers/KubernetesClusterStartWorker.java

@@ -139,6 +139,7 @@ public class KubernetesClusterStartWorker extends KubernetesClusterResourceModif
         final String clusterToken = "{{ k8s_control_node.cluster.token }}";
         final String clusterInitArgsKey = "{{ k8s_control_node.cluster.initargs }}";
         final String ejectIsoKey = "{{ k8s.eject.iso }}";
+
         final List<String> addresses = new ArrayList<>();
         addresses.add(controlNodeIp);
         if (!serverIp.equals(controlNodeIp)) {
@@ -243,6 +244,7 @@ public class KubernetesClusterStartWorker extends KubernetesClusterResourceModif
         final String sshPubKey = "{{ k8s.ssh.pub.key }}";
         final String clusterHACertificateKey = "{{ k8s_control_node.cluster.ha.certificate.key }}";
         final String ejectIsoKey = "{{ k8s.eject.iso }}";
+
         String pubKey = "- \"" + configurationDao.getValue("ssh.publickey") + "\"";
         String sshKeyPair = kubernetesCluster.getKeyPair();
         if (StringUtils.isNotEmpty(sshKeyPair)) {

plugins/network-elements/nsx/src/main/java/org/apache/cloudstack/NsxAnswer.java

@@ -20,6 +20,9 @@ import com.cloud.agent.api.Answer;
 import com.cloud.agent.api.Command;
 
 public class NsxAnswer extends Answer {
+
+    private boolean objectExists;
+
     public NsxAnswer(final Command command, final boolean success, final String details) {
         super(command, success, details);
     }
@@ -28,4 +31,11 @@ public class NsxAnswer extends Answer {
         super(command, e);
     }
 
+    public boolean isObjectExistent() {
+        return objectExists;
+    }
+
+    public void setObjectExists(boolean objectExisted) {
+        this.objectExists = objectExisted;
+    }
 }

plugins/network-elements/nsx/src/main/java/org/apache/cloudstack/resource/NsxResource.java

@@ -18,6 +18,8 @@ package org.apache.cloudstack.resource;
 
 import com.cloud.agent.IAgentControl;
 import com.cloud.agent.api.Answer;
+import com.cloud.agent.api.CheckHealthAnswer;
+import com.cloud.agent.api.CheckHealthCommand;
 import com.cloud.agent.api.Command;
 import com.cloud.agent.api.PingCommand;
 import com.cloud.agent.api.ReadyAnswer;
@@ -102,6 +104,8 @@ public class NsxResource implements ServerResource {
     public Answer executeRequest(Command cmd) {
         if (cmd instanceof ReadyCommand) {
             return executeRequest((ReadyCommand) cmd);
+        } else if (cmd instanceof CheckHealthCommand) {
+            return executeRequest((CheckHealthCommand) cmd);
         } else if (cmd instanceof DeleteNsxTier1GatewayCommand) {
             return executeRequest((DeleteNsxTier1GatewayCommand) cmd);
         } else if (cmd instanceof DeleteNsxSegmentCommand) {
@@ -293,6 +297,10 @@ public class NsxResource implements ServerResource {
         return new ReadyAnswer(cmd);
     }
 
+    private Answer executeRequest(CheckHealthCommand cmd) {
+        return new CheckHealthAnswer(cmd, nsxApiClient.isNsxControllerActive());
+    }
+
     private Answer executeRequest(CreateNsxTier1GatewayCommand cmd) {
         String tier1GatewayName = NsxControllerUtils.getTier1GatewayName(cmd.getDomainId(), cmd.getAccountId(), cmd.getZoneId(), cmd.getNetworkResourceId(), cmd.isResourceVpc());
         boolean sourceNatEnabled = cmd.isSourceNatEnabled();
@@ -385,16 +393,21 @@ public class NsxResource implements ServerResource {
                 cmd.getNetworkResourceId(), cmd.isResourceVpc());
         try {
             String privatePort = cmd.getPrivatePort();
+            logger.debug("Checking if the rule {} exists on Tier 1 Gateway: {}", ruleName, tier1GatewayName);
+            if (nsxApiClient.doesPfRuleExist(ruleName, tier1GatewayName)) {
+                String msg = String.format("Port forward rule for port: %s (%s) exits on NSX, not adding it again", ruleName, privatePort);
+                logger.debug(msg);
+                NsxAnswer answer = new NsxAnswer(cmd, true, msg);
+                answer.setObjectExists(true);
+                return answer;
+            }
             String service = privatePort.contains("-") ? nsxApiClient.getServicePath(ruleName, privatePort, cmd.getProtocol(), null, null) :
                     nsxApiClient.getNsxInfraServices(ruleName, privatePort, cmd.getProtocol(), null, null);
-            if (nsxApiClient.doesPfRuleExist(ruleName, tier1GatewayName)) {
-                logger.debug(String.format("Port forward rule for port: %s exits on NSX, not adding it again", privatePort));
-                return new NsxAnswer(cmd, true, null);
-            }
             nsxApiClient.createPortForwardingRule(ruleName, tier1GatewayName, cmd.getNetworkResourceName(), cmd.getPublicIp(),
                     cmd.getVmIp(), cmd.getPublicPort(), service);
         } catch (Exception e) {
-            logger.error(String.format("Failed to add NSX port forward rule %s for network: %s", ruleName, cmd.getNetworkResourceName()));
+            String msg = String.format("Failed to add NSX port forward rule %s for network: %s", ruleName, cmd.getNetworkResourceName());
+            logger.error(msg, e);
             return new NsxAnswer(cmd, new CloudRuntimeException(e.getMessage()));
         }
         return new NsxAnswer(cmd, true, null);
@@ -415,8 +428,9 @@ public class NsxResource implements ServerResource {
             nsxApiClient.deleteNatRule(cmd.getService(), cmd.getPrivatePort(), cmd.getProtocol(),
                     cmd.getNetworkResourceName(), tier1GatewayName, ruleName);
         } catch (Exception e) {
-            logger.error(String.format("Failed to add NSX static NAT rule %s for network: %s", ruleName, cmd.getNetworkResourceName()));
+            String msg = String.format("Failed to delete NSX rule %s for network %s: due to %s", ruleName, cmd.getNetworkResourceName(), e.getMessage());
-            return new NsxAnswer(cmd, new CloudRuntimeException(e.getMessage()));
+            logger.error(msg, e);
+            return new NsxAnswer(cmd, new CloudRuntimeException(msg));
         }
         return new NsxAnswer(cmd, true, null);
     }

plugins/network-elements/nsx/src/main/java/org/apache/cloudstack/service/NsxApiClient.java

@@ -17,7 +17,11 @@
 package org.apache.cloudstack.service;
 
 import com.cloud.network.Network;
+import com.cloud.network.nsx.NsxService;
 import com.cloud.utils.exception.CloudRuntimeException;
+import com.vmware.nsx.cluster.Status;
+import com.vmware.nsx.model.ClusterStatus;
+import com.vmware.nsx.model.ControllerClusterStatus;
 import com.vmware.nsx.model.TransportZone;
 import com.vmware.nsx.model.TransportZoneListResult;
 import com.vmware.nsx_policy.infra.DhcpRelayConfigs;
@@ -45,13 +49,13 @@ import com.vmware.nsx_policy.model.GroupListResult;
 import com.vmware.nsx_policy.model.ICMPTypeServiceEntry;
 import com.vmware.nsx_policy.model.L4PortSetServiceEntry;
 import com.vmware.nsx_policy.model.LBAppProfileListResult;
+import com.vmware.nsx_policy.model.LBIcmpMonitorProfile;
 import com.vmware.nsx_policy.model.LBMonitorProfileListResult;
 import com.vmware.nsx_policy.model.LBPool;
 import com.vmware.nsx_policy.model.LBPoolListResult;
 import com.vmware.nsx_policy.model.LBPoolMember;
 import com.vmware.nsx_policy.model.LBService;
 import com.vmware.nsx_policy.model.LBTcpMonitorProfile;
-import com.vmware.nsx_policy.model.LBUdpMonitorProfile;
 import com.vmware.nsx_policy.model.LBVirtualServer;
 import com.vmware.nsx_policy.model.LBVirtualServerListResult;
 import com.vmware.nsx_policy.model.LocaleServicesListResult;
@@ -84,6 +88,7 @@ import org.apache.cloudstack.utils.NsxControllerUtils;
 import org.apache.commons.collections.CollectionUtils;
 import org.apache.logging.log4j.LogManager;
 import org.apache.logging.log4j.Logger;
+import org.apache.commons.lang3.BooleanUtils;
 
 import java.util.ArrayList;
 import java.util.List;
@@ -112,6 +117,7 @@ public class NsxApiClient {
     protected Logger logger = LogManager.getLogger(getClass());
 
     // Constants
+    private static final String CLUSTER_STATUS_STABLE = "STABLE";
     private static final String TIER_1_RESOURCE_TYPE = "Tier1";
     private static final String TIER_1_LOCALE_SERVICE_ID = "default";
     private static final String SEGMENT_RESOURCE_TYPE = "Segment";
@@ -123,7 +129,7 @@ public class NsxApiClient {
     // TODO: Pass as global / zone-level setting?
     protected static final String NSX_LB_PASSIVE_MONITOR = "/infra/lb-monitor-profiles/default-passive-lb-monitor";
     protected static final String TCP_MONITOR_PROFILE = "LBTcpMonitorProfile";
-    protected static final String UDP_MONITOR_PROFILE = "LBUdpMonitorProfile";
+    protected static final String ICMP_MONITOR_PROFILE = "LBIcmpMonitorProfile";
     protected static final String NAT_ID = "USER";
 
     private enum PoolAllocation { ROUTING, LB_SMALL, LB_MEDIUM, LB_LARGE, LB_XLARGE }
@@ -199,6 +205,26 @@ public class NsxApiClient {
         nsxService = apiClient::createStub;
     }
 
+    public boolean isNsxControllerActive() {
+        try {
+            Status statusService = (Status) nsxService.apply(Status.class);
+            ClusterStatus clusterStatus = statusService.get();
+            if (clusterStatus == null) {
+                logger.error("Cannot get NSX Cluster Status");
+                return false;
+            }
+            ControllerClusterStatus status = clusterStatus.getControlClusterStatus();
+            if (status == null) {
+                logger.error("Cannot get NSX Controller Cluster Status");
+                return false;
+            }
+            return CLUSTER_STATUS_STABLE.equalsIgnoreCase(status.getStatus());
+        } catch (Error error) {
+            logger.error("Error checking NSX Controller Health: {}", error.getMessage());
+            return false;
+        }
+    }
+
     public void createTier1NatRule(String tier1GatewayName, String natId, String natRuleId,
                                    String action, String translatedIp) {
         NatRules natRulesService = (NatRules) nsxService.apply(NatRules.class);
@@ -435,7 +461,7 @@ public class NsxApiClient {
                 String t1GatewayName = getTier1GatewayName(domainId, accountId, zoneId, networkId, false);
                 deleteLoadBalancer(getLoadBalancerName(t1GatewayName));
             }
-            removeSegment(segmentName);
+            removeSegment(segmentName, zoneId);
             DhcpRelayConfigs dhcpRelayConfig = (DhcpRelayConfigs) nsxService.apply(DhcpRelayConfigs.class);
             String dhcpRelayConfigId = NsxControllerUtils.getNsxDhcpRelayConfigId(zoneId, domainId, accountId, vpcId, networkId);
             logger.debug(String.format("Removing the DHCP relay config with ID %s", dhcpRelayConfigId));
@@ -448,7 +474,8 @@ public class NsxApiClient {
         }
     }
 
-    protected void removeSegment(String segmentName) {
+
+    protected void removeSegment(String segmentName, long zoneId) {
         logger.debug(String.format("Removing the segment with ID %s", segmentName));
         Segments segmentService = (Segments) nsxService.apply(Segments.class);
         String errMsg = String.format("The segment with ID %s is not found, skipping removal", segmentName);
@@ -467,15 +494,16 @@ public class NsxApiClient {
         SegmentPorts segmentPortsService = (SegmentPorts) nsxService.apply(SegmentPorts.class);
         PolicyGroupMembersListResult segmentPortsList = getSegmentPortList(segmentPortsService, segmentName, enforcementPointPath);
         Long portCount = segmentPortsList.getResultCount();
-        portCount = retrySegmentDeletion(segmentPortsService, portCount, segmentName, enforcementPointPath);
+        if (portCount > 0L) {
-        logger.info("Port count: " + portCount);
+            portCount = retrySegmentDeletion(segmentPortsService, segmentName, enforcementPointPath, zoneId);
+        }
         if (portCount == 0L) {
             logger.debug(String.format("Removing the segment with ID %s", segmentName));
             removeGroupForSegment(segmentName);
             segmentService.delete(segmentName);
         } else {
             String msg = String.format("Cannot remove the NSX segment %s because there are still %s port group(s) attached to it", segmentName, portCount);
-            logger.debug(msg);
+            logger.error(msg);
             throw new CloudRuntimeException(msg);
         }
     }
@@ -485,13 +513,16 @@ public class NsxApiClient {
                 false, null, 50L, false, null);
     }
 
-    private Long retrySegmentDeletion(SegmentPorts segmentPortsService, Long portCount, String segmentName, String enforcementPointPath) {
+    private Long retrySegmentDeletion(SegmentPorts segmentPortsService, String segmentName, String enforcementPointPath, long zoneId) {
-        int retries = 20;
+        int retries = NsxService.NSX_API_FAILURE_RETRIES.valueIn(zoneId);
+        int waitingSecs = NsxService.NSX_API_FAILURE_INTERVAL.valueIn(zoneId);
         int count = 1;
+        Long portCount;
         do {
             try {
-                logger.info("Waiting for all port groups to be unlinked from the segment - Attempt: " + count++ + " Waiting for 5 secs");
+                logger.info("Waiting for all port groups to be unlinked from the segment {} - " +
-                Thread.sleep(5000);
+                        "Attempt: {}. Waiting for {} secs", segmentName, count++, waitingSecs);
+                Thread.sleep(waitingSecs * 1000L);
                 portCount = getSegmentPortList(segmentPortsService, segmentName, enforcementPointPath).getResultCount();
                 retries--;
             } catch (InterruptedException e) {
@@ -526,24 +557,37 @@ public class NsxApiClient {
         }
     }
 
-    public void deleteNatRule(Network.Service service, String privatePort, String protocol, String networkName, String tier1GatewayName, String ruleName) {
+    protected void deletePortForwardingNatRuleService(String ruleName, String privatePort, String protocol) {
+        String svcName = getServiceName(ruleName, privatePort, protocol, null, null);
         try {
-            NatRules natService = (NatRules) nsxService.apply(NatRules.class);
+            Services services = (Services) nsxService.apply(Services.class);
-            logger.debug(String.format("Deleting NSX static NAT rule %s for tier-1 gateway %s (network: %s)", ruleName, tier1GatewayName, networkName));
+            com.vmware.nsx_policy.model.Service servicePFRule = services.get(svcName);
-            // delete NAT rule
+            if (servicePFRule != null && !servicePFRule.getMarkedForDelete() && !BooleanUtils.toBoolean(servicePFRule.getIsDefault())) {
-            natService.delete(tier1GatewayName, NatId.USER.name(), ruleName);
-            if (service == Network.Service.PortForwarding) {
-                String svcName = getServiceName(ruleName, privatePort, protocol, null, null);
-                // Delete service
-                Services services = (Services) nsxService.apply(Services.class);
                 services.delete(svcName);
             }
         } catch (Error error) {
-            ApiError ae = error.getData()._convertTo(ApiError.class);
+            String msg = String.format("Cannot find service %s associated to rule %s, skipping its deletion: %s",
-            String msg = String.format("Failed to delete NSX Static NAT rule %s for tier-1 gateway %s (VPC: %s), due to %s",
+                    svcName, ruleName, error.getMessage());
-                    ruleName, tier1GatewayName, networkName, ae.getErrorMessage());
+            logger.debug(msg);
-            logger.error(msg);
+        }
-            throw new CloudRuntimeException(msg);
+    }
+
+    public void deleteNatRule(Network.Service service, String privatePort, String protocol, String networkName, String tier1GatewayName, String ruleName) {
+        try {
+            NatRules natService = (NatRules) nsxService.apply(NatRules.class);
+            logger.debug("Deleting NSX NAT rule {} for tier-1 gateway {} (network: {})", ruleName, tier1GatewayName, networkName);
+            PolicyNatRule natRule = natService.get(tier1GatewayName, NatId.USER.name(), ruleName);
+            if (natRule != null && !natRule.getMarkedForDelete()) {
+                logger.debug("Deleting rule {} from Tier 1 Gateway {}", ruleName, tier1GatewayName);
+                natService.delete(tier1GatewayName, NatId.USER.name(), ruleName);
+            }
+        } catch (Error error) {
+            String msg = String.format("Cannot find NAT rule with name %s: %s, skipping deletion", ruleName, error.getMessage());
+            logger.debug(msg);
+        }
+
+        if (service == Network.Service.PortForwarding) {
+            deletePortForwardingNatRuleService(ruleName, privatePort, protocol);
         }
     }
 
@@ -577,9 +621,14 @@ public class NsxApiClient {
         try {
             NatRules natService = (NatRules) nsxService.apply(NatRules.class);
             PolicyNatRule rule = natService.get(tier1GatewayName, NAT_ID, ruleName);
+            logger.debug("Rule {} from Tier 1 GW {}: {}", ruleName, tier1GatewayName,
+                    rule == null ? "null" : rule.getId() + " " + rule.getPath());
             return !Objects.isNull(rule);
         } catch (Error error) {
-            logger.debug(String.format("Found a port forward rule named: %s on NSX", ruleName));
+            String msg = String.format("Error checking if port forwarding rule %s exists on Tier 1 Gateway %s: %s",
+                    ruleName, tier1GatewayName, error.getMessage());
+            Throwable throwable = error.getCause();
+            logger.error(msg, throwable);
             return false;
         }
     }
@@ -637,13 +686,10 @@ public class NsxApiClient {
                     .build();
             lbActiveMonitor.patch(lbMonitorProfileId, lbTcpMonitorProfile);
         } else if ("UDP".equals(protocol.toUpperCase(Locale.ROOT))) {
-            LBUdpMonitorProfile lbUdpMonitorProfile = new LBUdpMonitorProfile.Builder(UDP_MONITOR_PROFILE)
+            LBIcmpMonitorProfile icmpMonitorProfile = new LBIcmpMonitorProfile.Builder(ICMP_MONITOR_PROFILE)
                     .setDisplayName(lbMonitorProfileId)
-                    .setMonitorPort(Long.parseLong(port))
-                    .setSend("")
-                    .setReceive("")
                     .build();
-            lbActiveMonitor.patch(lbMonitorProfileId, lbUdpMonitorProfile);
+            lbActiveMonitor.patch(lbMonitorProfileId, icmpMonitorProfile);
         }
 
         LBMonitorProfileListResult listResult = listLBActiveMonitors(lbActiveMonitor);

plugins/network-elements/nsx/src/main/java/org/apache/cloudstack/service/NsxElement.java

@@ -91,6 +91,8 @@ import com.cloud.utils.Pair;
 import com.cloud.utils.component.AdapterBase;
 import com.cloud.utils.db.QueryBuilder;
 import com.cloud.utils.db.SearchCriteria;
+import com.cloud.utils.db.Transaction;
+import com.cloud.utils.db.TransactionCallback;
 import com.cloud.utils.exception.CloudRuntimeException;
 import com.cloud.vm.NicProfile;
 import com.cloud.vm.ReservationContext;
@@ -98,7 +100,9 @@ import com.cloud.vm.VMInstanceVO;
 import com.cloud.vm.VirtualMachineProfile;
 import com.cloud.vm.dao.VMInstanceDao;
 import net.sf.ehcache.config.InvalidConfigurationException;
+import org.apache.cloudstack.NsxAnswer;
 import org.apache.cloudstack.StartupNsxCommand;
+import org.apache.cloudstack.api.ApiConstants;
 import org.apache.cloudstack.api.command.admin.internallb.ConfigureInternalLoadBalancerElementCmd;
 import org.apache.cloudstack.api.command.admin.internallb.CreateInternalLoadBalancerElementCmd;
 import org.apache.cloudstack.api.command.admin.internallb.ListInternalLoadBalancerElementsCmd;
@@ -108,6 +112,8 @@ import org.apache.cloudstack.resource.NsxNetworkRule;
 import org.apache.cloudstack.resource.NsxOpObject;
 import org.apache.logging.log4j.LogManager;
 import org.apache.logging.log4j.Logger;
+import org.apache.cloudstack.resourcedetail.FirewallRuleDetailVO;
+import org.apache.cloudstack.resourcedetail.dao.FirewallRuleDetailsDao;
 import org.springframework.stereotype.Component;
 
 import javax.inject.Inject;
@@ -121,6 +127,7 @@ import java.util.Map;
 import java.util.Objects;
 import java.util.Set;
 import java.util.function.LongFunction;
+import java.util.stream.Collectors;
 
 @Component
 public class NsxElement extends AdapterBase implements  DhcpServiceProvider, DnsServiceProvider, VpcProvider,
@@ -160,6 +167,8 @@ public class NsxElement extends AdapterBase implements  DhcpServiceProvider, Dns
     VirtualRouterProviderDao vrProviderDao;
     @Inject
     PhysicalNetworkServiceProviderDao pNtwkSvcProviderDao;
+    @Inject
+    FirewallRuleDetailsDao firewallRuleDetailsDao;
 
     protected Logger logger = LogManager.getLogger(getClass());
 
@@ -395,10 +404,18 @@ public class NsxElement extends AdapterBase implements  DhcpServiceProvider, Dns
         Account account = null;
         boolean forNsx = false;
         List<PhysicalNetworkVO> physicalNetworks = physicalNetworkDao.listByZoneAndTrafficType(zone.getId(), Networks.TrafficType.Guest);
-        if (CollectionUtils.isNullOrEmpty(physicalNetworks) || physicalNetworks.size() > 1 ) {
+        if (CollectionUtils.isNullOrEmpty(physicalNetworks)) {
-            throw new InvalidConfigurationException(String.format("Desired number of physical networks is not present in the zone %s for traffic type %s. ", zone.getName(), Networks.TrafficType.Guest.name()));
+            String err = String.format("Desired physical network is not present in the zone %s for traffic type %s. ", zone.getName(), Networks.TrafficType.Guest.name());
+            logger.error(err);
+            throw new InvalidConfigurationException(err);
         }
-        if (physicalNetworks.get(0).getIsolationMethods().contains("NSX")) {
+        List<PhysicalNetworkVO> filteredPhysicalNetworks = physicalNetworks.stream().filter(x -> x.getIsolationMethods().contains("NSX")).collect(Collectors.toList());
+        if (CollectionUtils.isNullOrEmpty(filteredPhysicalNetworks)) {
+            String err = String.format("No physical network with NSX isolation type for traffic type %s is present in the zone %s.", Networks.TrafficType.Guest.name(), zone.getName());
+            logger.error(err);
+            throw new InvalidConfigurationException(err);
+        }
+        if (filteredPhysicalNetworks.get(0).getIsolationMethods().contains("NSX")) {
             account = accountMgr.getAccount(vpc.getAccountId());
             forNsx = true;
         }
@@ -527,45 +544,77 @@ public class NsxElement extends AdapterBase implements  DhcpServiceProvider, Dns
         return false;
     }
 
+    protected synchronized boolean applyPFRulesInternal(Network network, List<PortForwardingRule> rules) {
+        return Transaction.execute((TransactionCallback<Boolean>) status -> {
+            boolean result = true;
+            for (PortForwardingRule rule : rules) {
+                IPAddressVO publicIp = ApiDBUtils.findIpAddressById(rule.getSourceIpAddressId());
+                UserVm vm = ApiDBUtils.findUserVmById(rule.getVirtualMachineId());
+                if (vm == null && rule.getState() != FirewallRule.State.Revoke) {
+                    continue;
+                }
+                NsxOpObject nsxObject = getNsxOpObject(network);
+                String publicPort = getPublicPortRange(rule);
+
+                String privatePort = getPrivatePFPortRange(rule);
+
+                NsxNetworkRule networkRule = new NsxNetworkRule.Builder()
+                        .setDomainId(nsxObject.getDomainId())
+                        .setAccountId(nsxObject.getAccountId())
+                        .setZoneId(nsxObject.getZoneId())
+                        .setNetworkResourceId(nsxObject.getNetworkResourceId())
+                        .setNetworkResourceName(nsxObject.getNetworkResourceName())
+                        .setVpcResource(nsxObject.isVpcResource())
+                        .setVmId(Objects.nonNull(vm) ? vm.getId() : 0)
+                        .setVmIp(Objects.nonNull(vm) ? vm.getPrivateIpAddress() : null)
+                        .setPublicIp(publicIp.getAddress().addr())
+                        .setPrivatePort(privatePort)
+                        .setPublicPort(publicPort)
+                        .setRuleId(rule.getId())
+                        .setProtocol(rule.getProtocol().toUpperCase(Locale.ROOT))
+                        .build();
+                FirewallRuleDetailVO ruleDetail = firewallRuleDetailsDao.findDetail(rule.getId(), ApiConstants.FOR_NSX);
+                if (Arrays.asList(FirewallRule.State.Add, FirewallRule.State.Active).contains(rule.getState())) {
+                    if ((ruleDetail == null && FirewallRule.State.Add == rule.getState()) || (ruleDetail != null && !ruleDetail.getValue().equalsIgnoreCase("true"))) {
+                        logger.debug("Creating port forwarding rule on NSX for VM {} to ports {} - {}",
+                                vm.getUuid(), rule.getDestinationPortStart(), rule.getDestinationPortEnd());
+                        NsxAnswer answer = nsxService.createPortForwardRule(networkRule);
+                        boolean pfRuleResult = answer.getResult();
+                        if (pfRuleResult && !answer.isObjectExistent()) {
+                            logger.debug("Port forwarding rule {} created on NSX, adding detail on firewall rules details", rule.getId());
+                            if (ruleDetail == null && FirewallRule.State.Add == rule.getState()) {
+                                logger.debug("Adding new firewall detail for rule {}", rule.getId());
+                                firewallRuleDetailsDao.addDetail(rule.getId(), ApiConstants.FOR_NSX, "true", false);
+                            } else {
+                                logger.debug("Updating firewall detail for rule {}", rule.getId());
+                                ruleDetail.setValue("true");
+                                firewallRuleDetailsDao.update(ruleDetail.getId(), ruleDetail);
+                            }
+                        }
+                        result &= pfRuleResult;
+                    }
+                } else if (rule.getState() == FirewallRule.State.Revoke) {
+                    if (ruleDetail == null || (ruleDetail != null && ruleDetail.getValue().equalsIgnoreCase("true"))) {
+                        boolean pfRuleResult = nsxService.deletePortForwardRule(networkRule);
+                        if (pfRuleResult && ruleDetail != null) {
+                            logger.debug("Updating firewall rule detail {} for rule {}, set to false", ruleDetail.getId(), rule.getId());
+                            ruleDetail.setValue("false");
+                            firewallRuleDetailsDao.update(ruleDetail.getId(), ruleDetail);
+                        }
+                        result &= pfRuleResult;
+                    }
+                }
+            }
+            return result;
+        });
+    }
+
     @Override
     public boolean applyPFRules(Network network, List<PortForwardingRule> rules) throws ResourceUnavailableException {
         if (!canHandle(network, Network.Service.PortForwarding)) {
             return false;
         }
-        boolean result = true;
+        return applyPFRulesInternal(network, rules);
-        for (PortForwardingRule rule : rules) {
-            IPAddressVO publicIp = ApiDBUtils.findIpAddressById(rule.getSourceIpAddressId());
-            UserVm vm = ApiDBUtils.findUserVmById(rule.getVirtualMachineId());
-            if (vm == null && rule.getState() != FirewallRule.State.Revoke) {
-                continue;
-            }
-            NsxOpObject nsxObject = getNsxOpObject(network);
-            String publicPort = getPublicPortRange(rule);
-
-            String privatePort = getPrivatePFPortRange(rule);
-
-            NsxNetworkRule networkRule = new NsxNetworkRule.Builder()
-                    .setDomainId(nsxObject.getDomainId())
-                    .setAccountId(nsxObject.getAccountId())
-                    .setZoneId(nsxObject.getZoneId())
-                    .setNetworkResourceId(nsxObject.getNetworkResourceId())
-                    .setNetworkResourceName(nsxObject.getNetworkResourceName())
-                    .setVpcResource(nsxObject.isVpcResource())
-                    .setVmId(Objects.nonNull(vm) ? vm.getId() : 0)
-                    .setVmIp(Objects.nonNull(vm) ? vm.getPrivateIpAddress() : null)
-                    .setPublicIp(publicIp.getAddress().addr())
-                    .setPrivatePort(privatePort)
-                    .setPublicPort(publicPort)
-                    .setRuleId(rule.getId())
-                    .setProtocol(rule.getProtocol().toUpperCase(Locale.ROOT))
-                    .build();
-            if (Arrays.asList(FirewallRule.State.Add, FirewallRule.State.Active).contains(rule.getState())) {
-                result &= nsxService.createPortForwardRule(networkRule);
-            } else if (rule.getState() == FirewallRule.State.Revoke) {
-                result &= nsxService.deletePortForwardRule(networkRule);
-            }
-        }
-        return result;
     }
 
     public Pair<VpcVO, NetworkVO> getVpcOrNetwork(Long vpcId, long networkId) {

plugins/network-elements/nsx/src/main/java/org/apache/cloudstack/service/NsxServiceImpl.java

@@ -19,7 +19,6 @@ package org.apache.cloudstack.service;
 import com.cloud.network.IpAddress;
 import com.cloud.network.Network;
 import com.cloud.network.nsx.NsxService;
-import com.cloud.network.dao.NetworkDao;
 import com.cloud.network.dao.NetworkVO;
 import com.cloud.network.vpc.Vpc;
 import com.cloud.network.vpc.VpcVO;
@@ -37,6 +36,8 @@ import org.apache.cloudstack.agent.api.DeleteNsxLoadBalancerRuleCommand;
 import org.apache.cloudstack.agent.api.DeleteNsxSegmentCommand;
 import org.apache.cloudstack.agent.api.DeleteNsxNatRuleCommand;
 import org.apache.cloudstack.agent.api.DeleteNsxTier1GatewayCommand;
+import org.apache.cloudstack.framework.config.ConfigKey;
+import org.apache.cloudstack.framework.config.Configurable;
 import org.apache.cloudstack.resource.NsxNetworkRule;
 import org.apache.cloudstack.utils.NsxControllerUtils;
 import org.apache.cloudstack.utils.NsxHelper;
@@ -47,13 +48,11 @@ import javax.inject.Inject;
 import java.util.List;
 import java.util.Objects;
 
-public class NsxServiceImpl implements NsxService {
+public class NsxServiceImpl implements NsxService, Configurable {
     @Inject
     NsxControllerUtils nsxControllerUtils;
     @Inject
     VpcDao vpcDao;
-    @Inject
-    NetworkDao networkDao;
 
     protected Logger logger = LogManager.getLogger(getClass());
 
@@ -139,14 +138,13 @@ public class NsxServiceImpl implements NsxService {
         return result.getResult();
     }
 
-    public boolean createPortForwardRule(NsxNetworkRule netRule) {
+    public NsxAnswer createPortForwardRule(NsxNetworkRule netRule) {
         // TODO: if port doesn't exist in default list of services, create a service entry
         CreateNsxPortForwardRuleCommand createPortForwardCmd = new CreateNsxPortForwardRuleCommand(netRule.getDomainId(),
                 netRule.getAccountId(), netRule.getZoneId(), netRule.getNetworkResourceId(),
                 netRule.getNetworkResourceName(), netRule.isVpcResource(), netRule.getVmId(), netRule.getRuleId(),
                 netRule.getPublicIp(), netRule.getVmIp(), netRule.getPublicPort(), netRule.getPrivatePort(), netRule.getProtocol());
-        NsxAnswer result = nsxControllerUtils.sendNsxCommand(createPortForwardCmd, netRule.getZoneId());
+        return nsxControllerUtils.sendNsxCommand(createPortForwardCmd, netRule.getZoneId());
-        return result.getResult();
     }
 
     public boolean deletePortForwardRule(NsxNetworkRule netRule) {
@@ -190,4 +188,16 @@ public class NsxServiceImpl implements NsxService {
         NsxAnswer result = nsxControllerUtils.sendNsxCommand(command, network.getDataCenterId());
         return result.getResult();
     }
+
+    @Override
+    public String getConfigComponentName() {
+        return NsxApiClient.class.getSimpleName();
+    }
+
+    @Override
+    public ConfigKey<?>[] getConfigKeys() {
+        return new ConfigKey<?>[] {
+            NSX_API_FAILURE_RETRIES, NSX_API_FAILURE_INTERVAL
+        };
+    }
 }

plugins/network-elements/nsx/src/main/java/org/apache/cloudstack/utils/NsxControllerUtils.java

@@ -124,6 +124,9 @@ public class NsxControllerUtils {
     }
 
     public static String getActiveMonitorProfileName(String lbServerPoolName, String port, String protocol) {
+        if (protocol.equalsIgnoreCase("udp")) {
+            protocol =  "ICMP";
+        }
         return lbServerPoolName + "-" + protocol + "-" + port + "-AM";
     }
 

plugins/network-elements/nsx/src/test/java/org/apache/cloudstack/service/NsxApiClientTest.java

@@ -17,6 +17,9 @@
 package org.apache.cloudstack.service;
 
 import com.cloud.network.Network;
+import com.vmware.nsx.cluster.Status;
+import com.vmware.nsx.model.ClusterStatus;
+import com.vmware.nsx.model.ControllerClusterStatus;
 import com.vmware.nsx_policy.infra.domains.Groups;
 import com.vmware.nsx_policy.model.Group;
 import com.vmware.nsx_policy.model.PathExpression;
@@ -93,4 +96,16 @@ public class NsxApiClientTest {
         Assert.assertEquals(List.of(String.format("%s/%s", NsxApiClient.GROUPS_PATH_PREFIX, segmentName)), sourceGroups);
         Assert.assertEquals(List.of("ANY"), destinationGroups);
     }
+
+    @Test
+    public void testIsNsxControllerActive() {
+        Status statusService = Mockito.mock(Status.class);
+        Mockito.when(nsxService.apply(Status.class)).thenReturn(statusService);
+        ClusterStatus clusterStatus = Mockito.mock(ClusterStatus.class);
+        ControllerClusterStatus status = Mockito.mock(ControllerClusterStatus.class);
+        Mockito.when(status.getStatus()).thenReturn("stable");
+        Mockito.when(statusService.get()).thenReturn(clusterStatus);
+        Mockito.when(clusterStatus.getControlClusterStatus()).thenReturn(status);
+        Assert.assertTrue(client.isNsxControllerActive());
+    }
 }

plugins/network-elements/nsx/src/test/java/org/apache/cloudstack/service/NsxElementTest.java

@@ -61,6 +61,7 @@ import com.cloud.vm.dao.UserVmDao;
 import com.cloud.vm.dao.VMInstanceDao;
 import org.apache.cloudstack.acl.ControlledEntity;
 import org.apache.cloudstack.resource.NsxNetworkRule;
+import org.apache.cloudstack.resourcedetail.dao.FirewallRuleDetailsDao;
 import org.junit.Assert;
 import org.junit.Before;
 import org.junit.Test;
@@ -124,6 +125,8 @@ public class NsxElementTest {
     private VpcOfferingServiceMapDao vpcOfferingServiceMapDao;
     @Mock
     LoadBalancerVMMapDao lbVmMapDao;
+    @Mock
+    FirewallRuleDetailsDao firewallRuleDetailsDao;
 
     NsxElement nsxElement;
     ReservationContext reservationContext;
@@ -148,6 +151,7 @@ public class NsxElementTest {
         nsxElement.vmInstanceDao = vmInstanceDao;
         nsxElement.vpcDao = vpcDao;
         nsxElement.lbVmMapDao = lbVmMapDao;
+        nsxElement.firewallRuleDetailsDao = firewallRuleDetailsDao;
 
         Field field = ApiDBUtils.class.getDeclaredField("s_ipAddressDao");
         field.setAccessible(true);

server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java

@@ -8002,7 +8002,7 @@ public class ConfigurationManagerImpl extends ManagerBase implements Configurati
                 BYTES_MAX_READ_LENGTH, BYTES_MAX_WRITE_LENGTH, ADD_HOST_ON_SERVICE_RESTART_KVM, SET_HOST_DOWN_TO_MAINTENANCE,
                 VM_SERVICE_OFFERING_MAX_CPU_CORES, VM_SERVICE_OFFERING_MAX_RAM_SIZE, MIGRATE_VM_ACROSS_CLUSTERS,
                 ENABLE_ACCOUNT_SETTINGS_FOR_DOMAIN, ENABLE_DOMAIN_SETTINGS_FOR_CHILD_DOMAIN,
-                ALLOW_DOMAIN_ADMINS_TO_CREATE_TAGGED_OFFERINGS, DELETE_QUERY_BATCH_SIZE
+                ALLOW_DOMAIN_ADMINS_TO_CREATE_TAGGED_OFFERINGS, DELETE_QUERY_BATCH_SIZE, AllowNonRFC1918CompliantIPs
         };
     }
 

server/src/main/java/com/cloud/network/NetworkServiceImpl.java

@@ -1832,7 +1832,7 @@ public class NetworkServiceImpl extends ManagerBase implements NetworkService, C
 
     private void validateNetworkCreationSupported(long zoneId, String zoneName, GuestType guestType) {
         NsxProviderVO nsxProviderVO = nsxProviderDao.findByZoneId(zoneId);
-        if (Objects.nonNull(nsxProviderVO) && List.of(GuestType.L2, GuestType.Shared).contains(guestType)) {
+        if (Objects.nonNull(nsxProviderVO) && GuestType.L2.equals(guestType)) {
             throw new InvalidParameterValueException(
                     String.format("Creation of %s networks is not supported in NSX enabled zone %s", guestType.name(), zoneName)
             );

server/src/main/java/com/cloud/network/router/VpcNetworkHelperImpl.java

@@ -134,6 +134,7 @@ public class VpcNetworkHelperImpl extends NetworkHelperImpl {
                 final PublicIp publicIp = PublicIp.createFromAddrAndVlan(ip, _vlanDao.findById(ip.getVlanId()));
                 if ((ip.getState() == IpAddress.State.Allocated  || ip.getState() == IpAddress.State.Allocating)
                         && vpcMgr.isIpAllocatedToVpc(ip)
+                        && Objects.nonNull(publicIp.getVlanTag())
                         && !publicVlans.contains(publicIp.getVlanTag())) {
                     logger.debug("Allocating nic for router in vlan " + publicIp.getVlanTag());
                     final NicProfile publicNic = new NicProfile();

server/src/main/java/com/cloud/network/vpc/VpcManagerImpl.java

@@ -43,6 +43,9 @@ import javax.inject.Inject;
 import javax.naming.ConfigurationException;
 
 import com.cloud.configuration.ConfigurationManager;
+import com.cloud.dc.Vlan;
+import com.cloud.network.dao.NsxProviderDao;
+import com.cloud.network.element.NsxProviderVO;
 import com.cloud.configuration.ConfigurationManagerImpl;
 import com.cloud.dc.ASNumberVO;
 import com.cloud.bgp.BGPService;
@@ -282,6 +285,8 @@ public class VpcManagerImpl extends ManagerBase implements VpcManager, VpcProvis
     @Inject
     private VpcPrivateGatewayTransactionCallable vpcTxCallable;
     @Inject
+    private NsxProviderDao nsxProviderDao;
+    @Inject
     RoutedIpv4Manager routedIpv4Manager;
 
     private final ScheduledExecutorService _executor = Executors.newScheduledThreadPool(1, new NamedThreadFactory("VpcChecker"));
@@ -3169,7 +3174,7 @@ public class VpcManagerImpl extends ManagerBase implements VpcManager, VpcProvis
 
         logger.debug("Associating ip " + ipToAssoc + " to vpc " + vpc);
 
-        final boolean isSourceNatFinal = isSrcNatIpRequired(vpc.getVpcOfferingId()) && getExistingSourceNatInVpc(vpc.getAccountId(), vpcId) == null;
+        final boolean isSourceNatFinal = isSrcNatIpRequired(vpc.getVpcOfferingId()) && getExistingSourceNatInVpc(vpc.getAccountId(), vpcId, false) == null;
         Transaction.execute(new TransactionCallbackNoReturn() {
             @Override
             public void doInTransactionWithoutResult(final TransactionStatus status) {
@@ -3266,7 +3271,7 @@ public class VpcManagerImpl extends ManagerBase implements VpcManager, VpcProvis
         return guestNetwork;
     }
 
-    protected IPAddressVO getExistingSourceNatInVpc(final long ownerId, final long vpcId) {
+    protected IPAddressVO getExistingSourceNatInVpc(final long ownerId, final long vpcId, final boolean forNsx) {
 
         final List<IPAddressVO> addrs = listPublicIpsAssignedToVpc(ownerId, true, vpcId);
 
@@ -3277,8 +3282,16 @@ public class VpcManagerImpl extends ManagerBase implements VpcManager, VpcProvis
             // Account already has ip addresses
             for (final IPAddressVO addr : addrs) {
                 if (addr.isSourceNat()) {
-                    sourceNatIp = addr;
+                    if (!forNsx) {
-                    return sourceNatIp;
+                        sourceNatIp = addr;
+                    } else {
+                        if (addr.isForSystemVms()) {
+                            sourceNatIp = addr;
+                        }
+                    }
+                    if (Objects.nonNull(sourceNatIp)) {
+                        return sourceNatIp;
+                    }
                 }
             }
 
@@ -3302,17 +3315,23 @@ public class VpcManagerImpl extends ManagerBase implements VpcManager, VpcProvis
     }
 
     @Override
-    public PublicIp assignSourceNatIpAddressToVpc(final Account owner, final Vpc vpc) throws InsufficientAddressCapacityException, ConcurrentOperationException {
+    public PublicIp assignSourceNatIpAddressToVpc(final Account owner, final Vpc vpc, final Long podId) throws InsufficientAddressCapacityException, ConcurrentOperationException {
         final long dcId = vpc.getZoneId();
+        NsxProviderVO nsxProvider = nsxProviderDao.findByZoneId(dcId);
+        boolean forNsx = nsxProvider != null;
 
-        final IPAddressVO sourceNatIp = getExistingSourceNatInVpc(owner.getId(), vpc.getId());
+        final IPAddressVO sourceNatIp = getExistingSourceNatInVpc(owner.getId(), vpc.getId(), forNsx);
 
         PublicIp ipToReturn = null;
 
         if (sourceNatIp != null) {
             ipToReturn = PublicIp.createFromAddrAndVlan(sourceNatIp, _vlanDao.findById(sourceNatIp.getVlanId()));
         } else {
-            ipToReturn = _ipAddrMgr.assignDedicateIpAddress(owner, null, vpc.getId(), dcId, true);
+            if (forNsx) {
+                ipToReturn = _ipAddrMgr.assignPublicIpAddress(dcId, podId, owner, Vlan.VlanType.VirtualNetwork, null, null, false, true);
+            } else {
+                ipToReturn = _ipAddrMgr.assignDedicateIpAddress(owner, null, vpc.getId(), dcId, true);
+            }
         }
 
         return ipToReturn;

server/src/main/java/org/apache/cloudstack/network/router/deployment/VpcRouterDeploymentDefinition.java

@@ -22,7 +22,6 @@ import java.util.Map;
 import java.util.Objects;
 
 import com.cloud.dc.DataCenter;
-import com.cloud.dc.Vlan;
 import com.cloud.network.dao.IPAddressVO;
 import com.cloud.network.element.NsxProviderVO;
 
@@ -132,10 +131,9 @@ public class VpcRouterDeploymentDefinition extends RouterDeploymentDefinition {
 
         if (isPublicNetwork) {
             if (Objects.isNull(nsxProvider)) {
-                sourceNatIp = vpcMgr.assignSourceNatIpAddressToVpc(owner, vpc);
+                sourceNatIp = vpcMgr.assignSourceNatIpAddressToVpc(owner, vpc, null);
             } else {
-                // NSX deploys VRs with Public NIC != to the source NAT, the source NAT IP is on the NSX Public range
+                sourceNatIp = vpcMgr.assignSourceNatIpAddressToVpc(owner, vpc, getPodId());
-                sourceNatIp = ipAddrMgr.assignPublicIpAddress(zoneId, getPodId(), owner, Vlan.VlanType.VirtualNetwork, null, null, false, true);
                 if (vpc != null) {
                     IPAddressVO routerPublicIp = ipAddressDao.findByIp(sourceNatIp.getAddress().toString());
                     routerPublicIp.setVpcId(vpc.getId());

server/src/test/java/org/apache/cloudstack/network/router/deployment/VpcRouterDeploymentDefinitionTest.java

@@ -269,7 +269,7 @@ public class VpcRouterDeploymentDefinitionTest extends RouterDeploymentDefinitio
     public void testFindSourceNatIP() throws InsufficientAddressCapacityException, ConcurrentOperationException {
         // Prepare
         final PublicIp publicIp = mock(PublicIp.class);
-        when(vpcMgr.assignSourceNatIpAddressToVpc(mockOwner, mockVpc)).thenReturn(publicIp);
+        when(vpcMgr.assignSourceNatIpAddressToVpc(mockOwner, mockVpc, null)).thenReturn(publicIp);
         deployment.isPublicNetwork = true;
 
         // Execute

ui/src/views/network/CreateSharedNetworkForm.vue

@@ -18,14 +18,7 @@
 <template>
   <a-spin :spinning="loading">
     <div class="form-layout" v-ctrl-enter="handleSubmit">
-      <div v-if="isNsxEnabled">
+      <div class="form">
-        <a-alert type="warning">
-          <template #message>
-            <span v-html="$t('message.shared.network.unsupported.for.nsx')" />
-          </template>
-        </a-alert>
-      </div>
-      <div v-else class="form">
         <a-form
           :ref="formRef"
           :model="form"
@@ -725,7 +718,7 @@ export default {
           var trafficTypes = json.listtraffictypesresponse.traffictype
           if (this.arrayHasItems(trafficTypes)) {
             for (const type of trafficTypes) {
-              if (type.traffictype === 'Guest') {
+              if (type.traffictype === 'Guest' && physicalNetwork.isolationmethods !== 'NSX') {
                 this.formPhysicalNetworks.push(physicalNetwork)
                 break
               }