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

Author: nvazquez

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

@@ -127,6 +127,7 @@
 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,
@@ -403,10 +404,18 @@ private Pair<Boolean, Account> validateVpcConfigurationAndGetAccount(DataCenterV
         Account account = null;
         boolean forNsx = false;
         List<PhysicalNetworkVO> physicalNetworks = physicalNetworkDao.listByZoneAndTrafficType(zone.getId(), Networks.TrafficType.Guest);
-        if (CollectionUtils.isNullOrEmpty(physicalNetworks) || physicalNetworks.size() > 1 ) {
-            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()));
-        }
-        if (physicalNetworks.get(0).getIsolationMethods().contains("NSX")) {
+        if (CollectionUtils.isNullOrEmpty(physicalNetworks)) {
+            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);
+        }
+        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;
         }
@@ -585,9 +594,9 @@ protected synchronized boolean applyPFRulesInternal(Network network, List<PortFo
                         result &= pfRuleResult;
                     }
                 } else if (rule.getState() == FirewallRule.State.Revoke) {
-                    if (ruleDetail != null && ruleDetail.getValue().equalsIgnoreCase("true")) {
+                    if (ruleDetail == null || (ruleDetail != null && ruleDetail.getValue().equalsIgnoreCase("true"))) {
                         boolean pfRuleResult = nsxService.deletePortForwardRule(networkRule);
-                        if (pfRuleResult) {
+                        if (pfRuleResult && ruleDetail != null) {
                             LOGGER.debug(String.format("Updating firewall rule detail %s for rule %s, set to false", ruleDetail.getId(), rule.getId()));
                             ruleDetail.setValue("false");
                             firewallRuleDetailsDao.update(ruleDetail.getId(), ruleDetail);

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

@@ -283,6 +283,7 @@ public void testApplyPFRules_delete() throws ResourceUnavailableException {
         IPAddressVO ipAddress = new IPAddressVO(new Ip("10.1.13.10"), 1L, 1L, 1L,false);
         when(ApiDBUtils.findIpAddressById(anyLong())).thenReturn(ipAddress);
         when(nsxElement.canHandle(networkVO, service)).thenReturn(true);
+        when(nsxService.deletePortForwardRule(any(NsxNetworkRule.class))).thenReturn(true);
         assertTrue(nsxElement.applyPFRules(networkVO, List.of(rule)));
     }