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

[pearl11594]

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

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

commit bcc8ff27a6caa9dba679701c32f3c1b99a1e4aca
Author: Nicolas Vazquez <nicovazque...@gmail.com>
AuthorDate: Thu Apr 11 22:19:34 2024 -0300

    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
---
 .../org/apache/cloudstack/service/NsxElement.java   | 21 +++++++++++++++------
 .../apache/cloudstack/service/NsxElementTest.java   |  1 +
 2 files changed, 16 insertions(+), 6 deletions(-)

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 f3f4b1c135b..3937ba9aba1 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
@@ -127,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,
@@ -403,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 ) {
-            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 @@ public class NsxElement extends AdapterBase implements  
DhcpServiceProvider, Dns
                         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);
diff --git 
a/plugins/network-elements/nsx/src/test/java/org/apache/cloudstack/service/NsxElementTest.java
 
b/plugins/network-elements/nsx/src/test/java/org/apache/cloudstack/service/NsxElementTest.java
index 59930d68d10..7c44a7324fb 100644
--- 
a/plugins/network-elements/nsx/src/test/java/org/apache/cloudstack/service/NsxElementTest.java
+++ 
b/plugins/network-elements/nsx/src/test/java/org/apache/cloudstack/service/NsxElementTest.java
@@ -283,6 +283,7 @@ public class NsxElementTest {
         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)));
     }