You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cloudstack.apache.org by pe...@apache.org on 2024/04/12 14:11:00 UTC

(cloudstack) branch nsx-integration-fixes created (now bcc8ff27a6c)

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

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


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

This branch includes the following new commits:

     new 523620f6e14 Prevent addition of duplicate PF rules on scale up and no rules left behind on scale down (#32)
     new 357980650dc fix missing dependency injection
     new f228c7afe6c NSX: Fix concurrency issues on port forwarding rules deletion (#37)
     new a89964854e3 CKS: Externalize control and worker node setup wait time and installation attempts (#38)
     new 968235af183 NSX: Add shared network support (#41)
     new bcc8ff27a6c NSX: Fix number of physical networks for Guest traffic checks and leftover rules on CKS cluster deletion (#45)

The 6 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails.  The revisions
listed as "add" were already present in the repository and have only
been added to this reference.



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

Posted by pe...@apache.org.
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 <ni...@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)));
     }
 


(cloudstack) 04/06: CKS: Externalize control and worker node setup wait time and installation attempts (#38)

Posted by pe...@apache.org.
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 a89964854e30d8d703e72d956684e254a77d9d79
Author: Pearl Dsilva <pe...@gmail.com>
AuthorDate: Wed Apr 10 10:06:40 2024 -0400

    CKS: Externalize control and worker node setup wait time and installation attempts (#38)
---
 .../cluster/KubernetesClusterManagerImpl.java      |  6 +++++-
 .../cluster/KubernetesClusterService.java          | 24 ++++++++++++++++++++++
 ...ernetesClusterResourceModifierActionWorker.java |  9 +++++++-
 .../KubernetesClusterStartWorker.java              | 14 +++++++++++++
 .../main/resources/conf/k8s-control-node-add.yml   | 10 +++++++--
 .../src/main/resources/conf/k8s-control-node.yml   | 10 +++++++--
 .../src/main/resources/conf/k8s-node.yml           | 10 +++++++--
 7 files changed, 75 insertions(+), 8 deletions(-)

diff --git a/plugins/integrations/kubernetes-service/src/main/java/com/cloud/kubernetes/cluster/KubernetesClusterManagerImpl.java b/plugins/integrations/kubernetes-service/src/main/java/com/cloud/kubernetes/cluster/KubernetesClusterManagerImpl.java
index 834d6d33330..6fff17e3428 100644
--- a/plugins/integrations/kubernetes-service/src/main/java/com/cloud/kubernetes/cluster/KubernetesClusterManagerImpl.java
+++ b/plugins/integrations/kubernetes-service/src/main/java/com/cloud/kubernetes/cluster/KubernetesClusterManagerImpl.java
@@ -1984,7 +1984,11 @@ public class KubernetesClusterManagerImpl extends ManagerBase implements Kuberne
             KubernetesClusterUpgradeTimeout,
             KubernetesClusterUpgradeRetries,
             KubernetesClusterExperimentalFeaturesEnabled,
-            KubernetesMaxClusterSize
+            KubernetesMaxClusterSize,
+            KubernetesControlNodeInstallAttemptWait,
+            KubernetesControlNodeInstallReattempts,
+            KubernetesWorkerNodeInstallAttemptWait,
+            KubernetesWorkerNodeInstallReattempts
         };
     }
 }
diff --git a/plugins/integrations/kubernetes-service/src/main/java/com/cloud/kubernetes/cluster/KubernetesClusterService.java b/plugins/integrations/kubernetes-service/src/main/java/com/cloud/kubernetes/cluster/KubernetesClusterService.java
index 39b926537f5..5d5e188fa45 100644
--- a/plugins/integrations/kubernetes-service/src/main/java/com/cloud/kubernetes/cluster/KubernetesClusterService.java
+++ b/plugins/integrations/kubernetes-service/src/main/java/com/cloud/kubernetes/cluster/KubernetesClusterService.java
@@ -91,6 +91,30 @@ public interface KubernetesClusterService extends PluggableService, Configurable
             true,
             ConfigKey.Scope.Account,
             KubernetesServiceEnabled.key());
+    static final ConfigKey<Long> KubernetesControlNodeInstallAttemptWait = new ConfigKey<Long>("Advanced", Long.class,
+            "cloud.kubernetes.control.node.install.attempt.wait.duration",
+            "15",
+            "Time in seconds for the installation process to wait before it re-attempts",
+            true,
+            KubernetesServiceEnabled.key());
+    static final ConfigKey<Long> KubernetesControlNodeInstallReattempts = new ConfigKey<Long>("Advanced", Long.class,
+            "cloud.kubernetes.control.node.install.reattempt.count",
+            "100",
+            "Number of times the offline installation of K8S will be re-attempted",
+            true,
+            KubernetesServiceEnabled.key());
+    final ConfigKey<Long> KubernetesWorkerNodeInstallAttemptWait = new ConfigKey<Long>("Advanced", Long.class,
+            "cloud.kubernetes.worker.node.install.attempt.wait.duration",
+            "30",
+            "Time in seconds for the installation process to wait before it re-attempts",
+            true,
+            KubernetesServiceEnabled.key());
+    static final ConfigKey<Long> KubernetesWorkerNodeInstallReattempts = new ConfigKey<Long>("Advanced", Long.class,
+            "cloud.kubernetes.worker.node.install.reattempt.count",
+            "40",
+            "Number of times the offline installation of K8S will be re-attempted",
+            true,
+            KubernetesServiceEnabled.key());
 
     KubernetesCluster findById(final Long id);
 
diff --git a/plugins/integrations/kubernetes-service/src/main/java/com/cloud/kubernetes/cluster/actionworkers/KubernetesClusterResourceModifierActionWorker.java b/plugins/integrations/kubernetes-service/src/main/java/com/cloud/kubernetes/cluster/actionworkers/KubernetesClusterResourceModifierActionWorker.java
index d0f9cf842c5..cf4bb07f58b 100644
--- a/plugins/integrations/kubernetes-service/src/main/java/com/cloud/kubernetes/cluster/actionworkers/KubernetesClusterResourceModifierActionWorker.java
+++ b/plugins/integrations/kubernetes-service/src/main/java/com/cloud/kubernetes/cluster/actionworkers/KubernetesClusterResourceModifierActionWorker.java
@@ -31,6 +31,7 @@ import java.util.stream.Collectors;
 
 import javax.inject.Inject;
 
+import com.cloud.kubernetes.cluster.KubernetesClusterService;
 import com.cloud.network.rules.FirewallManager;
 import com.cloud.offering.NetworkOffering;
 import com.cloud.offerings.dao.NetworkOfferingDao;
@@ -174,6 +175,11 @@ 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 }}";
+        final String installWaitTime = "{{ k8s.install.wait.time }}";
+        final String installReattemptsCount = "{{ k8s.install.reattempts.count }}";
+
+        final Long waitTime = KubernetesClusterService.KubernetesWorkerNodeInstallAttemptWait.value();
+        final Long reattempts = KubernetesClusterService.KubernetesWorkerNodeInstallReattempts.value();
         String pubKey = "- \"" + configurationDao.getValue("ssh.publickey") + "\"";
         String sshKeyPair = kubernetesCluster.getKeyPair();
         if (StringUtils.isNotEmpty(sshKeyPair)) {
@@ -186,7 +192,8 @@ 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 = k8sNodeConfig.replace(installWaitTime, String.valueOf(waitTime));
+        k8sNodeConfig = k8sNodeConfig.replace(installReattemptsCount, String.valueOf(reattempts));
         k8sNodeConfig = updateKubeConfigWithRegistryDetails(k8sNodeConfig);
 
         return k8sNodeConfig;
diff --git a/plugins/integrations/kubernetes-service/src/main/java/com/cloud/kubernetes/cluster/actionworkers/KubernetesClusterStartWorker.java b/plugins/integrations/kubernetes-service/src/main/java/com/cloud/kubernetes/cluster/actionworkers/KubernetesClusterStartWorker.java
index a7cea8093c8..c1fdc6d2b45 100644
--- a/plugins/integrations/kubernetes-service/src/main/java/com/cloud/kubernetes/cluster/actionworkers/KubernetesClusterStartWorker.java
+++ b/plugins/integrations/kubernetes-service/src/main/java/com/cloud/kubernetes/cluster/actionworkers/KubernetesClusterStartWorker.java
@@ -139,6 +139,9 @@ 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 String installWaitTime = "{{ k8s.install.wait.time }}";
+        final String installReattemptsCount = "{{ k8s.install.reattempts.count }}";
+
         final List<String> addresses = new ArrayList<>();
         addresses.add(controlNodeIp);
         if (!serverIp.equals(controlNodeIp)) {
@@ -150,6 +153,8 @@ public class KubernetesClusterStartWorker extends KubernetesClusterResourceModif
         final String tlsClientCert = CertUtils.x509CertificateToPem(certificate.getClientCertificate());
         final String tlsPrivateKey = CertUtils.privateKeyToPem(certificate.getPrivateKey());
         final String tlsCaCert = CertUtils.x509CertificatesToPem(certificate.getCaCertificates());
+        final Long waitTime = KubernetesClusterService.KubernetesControlNodeInstallAttemptWait.value();
+        final Long reattempts = KubernetesClusterService.KubernetesControlNodeInstallReattempts.value();
         k8sControlNodeConfig = k8sControlNodeConfig.replace(apiServerCert, tlsClientCert.replace("\n", "\n      "));
         k8sControlNodeConfig = k8sControlNodeConfig.replace(apiServerKey, tlsPrivateKey.replace("\n", "\n      "));
         k8sControlNodeConfig = k8sControlNodeConfig.replace(caCert, tlsCaCert.replace("\n", "\n      "));
@@ -161,6 +166,8 @@ public class KubernetesClusterStartWorker extends KubernetesClusterResourceModif
                 pubKey += "\n      - \"" + sshkp.getPublicKey() + "\"";
             }
         }
+        k8sControlNodeConfig = k8sControlNodeConfig.replace(installWaitTime, String.valueOf(waitTime));
+        k8sControlNodeConfig = k8sControlNodeConfig.replace(installReattemptsCount, String.valueOf(reattempts));
         k8sControlNodeConfig = k8sControlNodeConfig.replace(sshPubKey, pubKey);
         k8sControlNodeConfig = k8sControlNodeConfig.replace(clusterToken, KubernetesClusterUtil.generateClusterToken(kubernetesCluster));
         String initArgs = "";
@@ -241,6 +248,11 @@ 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 }}";
+        final String installWaitTime = "{{ k8s.install.wait.time }}";
+        final String installReattemptsCount = "{{ k8s.install.reattempts.count }}";
+
+        final Long waitTime = KubernetesClusterService.KubernetesControlNodeInstallAttemptWait.value();
+        final Long reattempts = KubernetesClusterService.KubernetesControlNodeInstallReattempts.value();
         String pubKey = "- \"" + configurationDao.getValue("ssh.publickey") + "\"";
         String sshKeyPair = kubernetesCluster.getKeyPair();
         if (StringUtils.isNotEmpty(sshKeyPair)) {
@@ -249,6 +261,8 @@ public class KubernetesClusterStartWorker extends KubernetesClusterResourceModif
                 pubKey += "\n      - \"" + sshkp.getPublicKey() + "\"";
             }
         }
+        k8sControlNodeConfig = k8sControlNodeConfig.replace(installWaitTime, String.valueOf(waitTime));
+        k8sControlNodeConfig = k8sControlNodeConfig.replace(installReattemptsCount, String.valueOf(reattempts));
         k8sControlNodeConfig = k8sControlNodeConfig.replace(sshPubKey, pubKey);
         k8sControlNodeConfig = k8sControlNodeConfig.replace(joinIpKey, joinIp);
         k8sControlNodeConfig = k8sControlNodeConfig.replace(clusterTokenKey, KubernetesClusterUtil.generateClusterToken(kubernetesCluster));
diff --git a/plugins/integrations/kubernetes-service/src/main/resources/conf/k8s-control-node-add.yml b/plugins/integrations/kubernetes-service/src/main/resources/conf/k8s-control-node-add.yml
index 2c18efa0189..35134bd1e16 100644
--- a/plugins/integrations/kubernetes-service/src/main/resources/conf/k8s-control-node-add.yml
+++ b/plugins/integrations/kubernetes-service/src/main/resources/conf/k8s-control-node-add.yml
@@ -42,8 +42,14 @@ write_files:
       ATTEMPT_ONLINE_INSTALL=false
       setup_complete=false
 
-      OFFLINE_INSTALL_ATTEMPT_SLEEP=15
-      MAX_OFFLINE_INSTALL_ATTEMPTS=100
+      OFFLINE_INSTALL_ATTEMPT_SLEEP={{ k8s.install.wait.time }}
+      MAX_OFFLINE_INSTALL_ATTEMPTS={{ k8s.install.reattempts.count }}
+      if [[ -z $OFFLINE_INSTALL_ATTEMPT_SLEEP || $OFFLINE_INSTALL_ATTEMPT_SLEEP -eq 0 ]]; then
+        OFFLINE_INSTALL_ATTEMPT_SLEEP=15
+      fi
+      if [[ -z $MAX_OFFLINE_INSTALL_ATTEMPTS || $MAX_OFFLINE_INSTALL_ATTEMPTS -eq 0 ]]; then
+        MAX_OFFLINE_INSTALL_ATTEMPTS=100
+      fi
       offline_attempts=1
       MAX_SETUP_CRUCIAL_CMD_ATTEMPTS=3
       EJECT_ISO_FROM_OS={{ k8s.eject.iso }}
diff --git a/plugins/integrations/kubernetes-service/src/main/resources/conf/k8s-control-node.yml b/plugins/integrations/kubernetes-service/src/main/resources/conf/k8s-control-node.yml
index aa7eec97ac8..3154fb20251 100644
--- a/plugins/integrations/kubernetes-service/src/main/resources/conf/k8s-control-node.yml
+++ b/plugins/integrations/kubernetes-service/src/main/resources/conf/k8s-control-node.yml
@@ -62,8 +62,14 @@ write_files:
       ATTEMPT_ONLINE_INSTALL=false
       setup_complete=false
 
-      OFFLINE_INSTALL_ATTEMPT_SLEEP=15
-      MAX_OFFLINE_INSTALL_ATTEMPTS=100
+      OFFLINE_INSTALL_ATTEMPT_SLEEP={{ k8s.install.wait.time }}
+      MAX_OFFLINE_INSTALL_ATTEMPTS={{ k8s.install.reattempts.count }}
+      if [[ -z $OFFLINE_INSTALL_ATTEMPT_SLEEP || $OFFLINE_INSTALL_ATTEMPT_SLEEP -eq 0 ]]; then
+        OFFLINE_INSTALL_ATTEMPT_SLEEP=15
+      fi
+      if [[ -z $MAX_OFFLINE_INSTALL_ATTEMPTS || $MAX_OFFLINE_INSTALL_ATTEMPTS -eq 0 ]]; then
+        MAX_OFFLINE_INSTALL_ATTEMPTS=100
+      fi
       offline_attempts=1
       MAX_SETUP_CRUCIAL_CMD_ATTEMPTS=3
       EJECT_ISO_FROM_OS={{ k8s.eject.iso }}
diff --git a/plugins/integrations/kubernetes-service/src/main/resources/conf/k8s-node.yml b/plugins/integrations/kubernetes-service/src/main/resources/conf/k8s-node.yml
index de1f4c9ffc7..298236da0df 100644
--- a/plugins/integrations/kubernetes-service/src/main/resources/conf/k8s-node.yml
+++ b/plugins/integrations/kubernetes-service/src/main/resources/conf/k8s-node.yml
@@ -42,8 +42,14 @@ write_files:
       ATTEMPT_ONLINE_INSTALL=false
       setup_complete=false
 
-      OFFLINE_INSTALL_ATTEMPT_SLEEP=30
-      MAX_OFFLINE_INSTALL_ATTEMPTS=40
+      OFFLINE_INSTALL_ATTEMPT_SLEEP={{ k8s.install.wait.time }}
+      MAX_OFFLINE_INSTALL_ATTEMPTS={{ k8s.install.reattempts.count }}
+      if [[ -z $OFFLINE_INSTALL_ATTEMPT_SLEEP || $OFFLINE_INSTALL_ATTEMPT_SLEEP -eq 0 ]]; then
+        OFFLINE_INSTALL_ATTEMPT_SLEEP=30
+      fi
+      if [[ -z $MAX_OFFLINE_INSTALL_ATTEMPTS || $MAX_OFFLINE_INSTALL_ATTEMPTS -eq 0 ]]; then
+        MAX_OFFLINE_INSTALL_ATTEMPTS=40
+      fi
       offline_attempts=1
       MAX_SETUP_CRUCIAL_CMD_ATTEMPTS=3
       EJECT_ISO_FROM_OS={{ k8s.eject.iso }}


(cloudstack) 01/06: Prevent addition of duplicate PF rules on scale up and no rules left behind on scale down (#32)

Posted by pe...@apache.org.
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 523620f6e14f40439a649afb66568a9c513bdf86
Author: Pearl Dsilva <pe...@gmail.com>
AuthorDate: Mon Mar 11 11:59:11 2024 -0400

    Prevent addition of duplicate PF rules on scale up and no rules left behind on scale down (#32)
---
 .../actionworkers/KubernetesClusterResourceModifierActionWorker.java | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/plugins/integrations/kubernetes-service/src/main/java/com/cloud/kubernetes/cluster/actionworkers/KubernetesClusterResourceModifierActionWorker.java b/plugins/integrations/kubernetes-service/src/main/java/com/cloud/kubernetes/cluster/actionworkers/KubernetesClusterResourceModifierActionWorker.java
index e8bc8e2851e..f500677754b 100644
--- a/plugins/integrations/kubernetes-service/src/main/java/com/cloud/kubernetes/cluster/actionworkers/KubernetesClusterResourceModifierActionWorker.java
+++ b/plugins/integrations/kubernetes-service/src/main/java/com/cloud/kubernetes/cluster/actionworkers/KubernetesClusterResourceModifierActionWorker.java
@@ -551,9 +551,12 @@ public class KubernetesClusterResourceModifierActionWorker extends KubernetesClu
         for (PortForwardingRuleVO pfRule : pfRules) {
             if (startPort <= pfRule.getSourcePortStart() && pfRule.getSourcePortStart() <= endPort) {
                 portForwardingRulesDao.remove(pfRule.getId());
+                LOGGER.trace("Marking PF rule " + pfRule + " with Revoke state");
+                pfRule.setState(FirewallRule.State.Revoke);
+
             }
         }
-        rulesService.applyPortForwardingRules(publicIp.getId(), account);
+        firewallManager.applyRules(pfRules, false, true);
     }
 
     protected void removeLoadBalancingRule(final IpAddress publicIp, final Network network,


(cloudstack) 02/06: fix missing dependency injection

Posted by pe...@apache.org.
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 357980650dcbcf8e09f1348367800de215036810
Author: Pearl Dsilva <pe...@gmail.com>
AuthorDate: Thu Mar 28 09:09:56 2024 -0400

    fix missing dependency injection
---
 .../actionworkers/KubernetesClusterResourceModifierActionWorker.java | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/plugins/integrations/kubernetes-service/src/main/java/com/cloud/kubernetes/cluster/actionworkers/KubernetesClusterResourceModifierActionWorker.java b/plugins/integrations/kubernetes-service/src/main/java/com/cloud/kubernetes/cluster/actionworkers/KubernetesClusterResourceModifierActionWorker.java
index f500677754b..d0f9cf842c5 100644
--- a/plugins/integrations/kubernetes-service/src/main/java/com/cloud/kubernetes/cluster/actionworkers/KubernetesClusterResourceModifierActionWorker.java
+++ b/plugins/integrations/kubernetes-service/src/main/java/com/cloud/kubernetes/cluster/actionworkers/KubernetesClusterResourceModifierActionWorker.java
@@ -31,6 +31,7 @@ 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;
@@ -136,6 +137,8 @@ public class KubernetesClusterResourceModifierActionWorker extends KubernetesClu
     @Inject
     protected RulesService rulesService;
     @Inject
+    protected FirewallManager firewallManager;
+    @Inject
     protected PortForwardingRulesDao portForwardingRulesDao;
     @Inject
     protected ResourceManager resourceManager;
@@ -551,7 +554,7 @@ public class KubernetesClusterResourceModifierActionWorker extends KubernetesClu
         for (PortForwardingRuleVO pfRule : pfRules) {
             if (startPort <= pfRule.getSourcePortStart() && pfRule.getSourcePortStart() <= endPort) {
                 portForwardingRulesDao.remove(pfRule.getId());
-                LOGGER.trace("Marking PF rule " + pfRule + " with Revoke state");
+                logger.trace("Marking PF rule " + pfRule + " with Revoke state");
                 pfRule.setState(FirewallRule.State.Revoke);
 
             }


(cloudstack) 03/06: NSX: Fix concurrency issues on port forwarding rules deletion (#37)

Posted by pe...@apache.org.
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 f228c7afe6c5ddde4a6a75b31cf3c0c3e0ee324f
Author: Nicolas Vazquez <ni...@gmail.com>
AuthorDate: Wed Apr 10 11:05:50 2024 -0300

    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
---
 .../resourcedetail/FirewallRuleDetailVO.java       |   4 +
 .../main/java/org/apache/cloudstack/NsxAnswer.java |  10 ++
 .../apache/cloudstack/resource/NsxResource.java    |  20 ++--
 .../apache/cloudstack/service/NsxApiClient.java    |  47 ++++++---
 .../org/apache/cloudstack/service/NsxElement.java  | 108 ++++++++++++++-------
 .../apache/cloudstack/service/NsxServiceImpl.java  |   5 +-
 .../apache/cloudstack/service/NsxElementTest.java  |   5 +-
 7 files changed, 140 insertions(+), 59 deletions(-)

diff --git a/engine/schema/src/main/java/org/apache/cloudstack/resourcedetail/FirewallRuleDetailVO.java b/engine/schema/src/main/java/org/apache/cloudstack/resourcedetail/FirewallRuleDetailVO.java
index 636d889fafe..1149d0b13e7 100644
--- a/engine/schema/src/main/java/org/apache/cloudstack/resourcedetail/FirewallRuleDetailVO.java
+++ b/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;
+    }
 }
diff --git a/plugins/network-elements/nsx/src/main/java/org/apache/cloudstack/NsxAnswer.java b/plugins/network-elements/nsx/src/main/java/org/apache/cloudstack/NsxAnswer.java
index 0820465a6b6..a667adda794 100644
--- a/plugins/network-elements/nsx/src/main/java/org/apache/cloudstack/NsxAnswer.java
+++ b/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;
+    }
 }
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 cd1d481b9f8..42ee24436ea 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
@@ -385,16 +385,21 @@ public class NsxResource implements ServerResource {
                 cmd.getNetworkResourceId(), cmd.isResourceVpc());
         try {
             String privatePort = cmd.getPrivatePort();
-            String service = privatePort.contains("-") ? nsxApiClient.getServicePath(ruleName, privatePort, cmd.getProtocol(), null, null) :
-                    nsxApiClient.getNsxInfraServices(ruleName, privatePort, cmd.getProtocol(), null, null);
+            LOGGER.debug(String.format("Checking if rule %s exists on Tier 1 Gateway: %s", ruleName, tier1GatewayName));
             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);
+                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);
             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 +420,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()));
-            return new NsxAnswer(cmd, new CloudRuntimeException(e.getMessage()));
+            String msg = String.format("Failed to delete NSX rule %s for network %s: due to %s", ruleName, cmd.getNetworkResourceName(), e.getMessage());
+            LOGGER.error(msg, e);
+            return new NsxAnswer(cmd, new CloudRuntimeException(msg));
         }
         return new NsxAnswer(cmd, true, null);
     }
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 d443b0e14e7..b814af686ac 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
@@ -84,6 +84,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;
@@ -526,24 +527,37 @@ public class NsxApiClient {
         }
     }
 
+    protected void deletePortForwardingNatRuleService(String ruleName, String privatePort, String protocol) {
+        String svcName = getServiceName(ruleName, privatePort, protocol, null, null);
+        try {
+            Services services = (Services) nsxService.apply(Services.class);
+            com.vmware.nsx_policy.model.Service servicePFRule = services.get(svcName);
+            if (servicePFRule != null && !servicePFRule.getMarkedForDelete() && !BooleanUtils.toBoolean(servicePFRule.getIsDefault())) {
+                services.delete(svcName);
+            }
+        } catch (Error error) {
+            String msg = String.format("Cannot find service %s associated to rule %s, skipping its deletion: %s",
+                    svcName, ruleName, error.getMessage());
+            logger.debug(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(String.format("Deleting NSX static NAT rule %s for tier-1 gateway %s (network: %s)", ruleName, tier1GatewayName, networkName));
-            // delete NAT rule
-            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);
+            logger.debug(String.format("Deleting NSX NAT rule %s for tier-1 gateway %s (network: %s)", ruleName, tier1GatewayName, networkName));
+            PolicyNatRule natRule = natService.get(tier1GatewayName, NatId.USER.name(), ruleName);
+            if (natRule != null && !natRule.getMarkedForDelete()) {
+                logger.debug(String.format("Deleting rule %s from Tier 1 Gateway %s", ruleName, tier1GatewayName));
+                natService.delete(tier1GatewayName, NatId.USER.name(), ruleName);
             }
         } catch (Error error) {
-            ApiError ae = error.getData()._convertTo(ApiError.class);
-            String msg = String.format("Failed to delete NSX Static NAT rule %s for tier-1 gateway %s (VPC: %s), due to %s",
-                    ruleName, tier1GatewayName, networkName, ae.getErrorMessage());
-            logger.error(msg);
-            throw new CloudRuntimeException(msg);
+            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 +591,14 @@ public class NsxApiClient {
         try {
             NatRules natService = (NatRules) nsxService.apply(NatRules.class);
             PolicyNatRule rule = natService.get(tier1GatewayName, NAT_ID, ruleName);
+            logger.debug(String.format("Rule %s from Tier 1 GW %s: %s", 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;
         }
     }
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 d09049700e5..f3f4b1c135b 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
@@ -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;
@@ -160,6 +166,8 @@ public class NsxElement extends AdapterBase implements  DhcpServiceProvider, Dns
     VirtualRouterProviderDao vrProviderDao;
     @Inject
     PhysicalNetworkServiceProviderDao pNtwkSvcProviderDao;
+    @Inject
+    FirewallRuleDetailsDao firewallRuleDetailsDao;
 
     protected Logger logger = LogManager.getLogger(getClass());
 
@@ -527,45 +535,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(String.format("Creating port forwarding rule on NSX for VM %s to ports %s - %s",
+                                vm.getUuid(), rule.getDestinationPortStart(), rule.getDestinationPortEnd()));
+                        NsxAnswer answer = nsxService.createPortForwardRule(networkRule);
+                        boolean pfRuleResult = answer.getResult();
+                        if (pfRuleResult && !answer.isObjectExistent()) {
+                            LOGGER.debug(String.format("Port forwarding rule %s created on NSX, adding detail on firewall rules details", rule.getId()));
+                            if (ruleDetail == null && FirewallRule.State.Add == rule.getState()) {
+                                LOGGER.debug(String.format("Adding new firewall detail for rule %s", rule.getId()));
+                                firewallRuleDetailsDao.addDetail(rule.getId(), ApiConstants.FOR_NSX, "true", false);
+                            } else {
+                                LOGGER.debug(String.format("Updating firewall detail for rule %s", rule.getId()));
+                                ruleDetail.setValue("true");
+                                firewallRuleDetailsDao.update(ruleDetail.getId(), ruleDetail);
+                            }
+                        }
+                        result &= pfRuleResult;
+                    }
+                } else if (rule.getState() == FirewallRule.State.Revoke) {
+                    if (ruleDetail != null && ruleDetail.getValue().equalsIgnoreCase("true")) {
+                        boolean pfRuleResult = nsxService.deletePortForwardRule(networkRule);
+                        if (pfRuleResult) {
+                            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);
+                        }
+                        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;
-        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;
+        return applyPFRulesInternal(network, rules);
     }
 
     public Pair<VpcVO, NetworkVO> getVpcOrNetwork(Long vpcId, long networkId) {
diff --git a/plugins/network-elements/nsx/src/main/java/org/apache/cloudstack/service/NsxServiceImpl.java b/plugins/network-elements/nsx/src/main/java/org/apache/cloudstack/service/NsxServiceImpl.java
index f8880826a3f..19f29ce62ea 100644
--- a/plugins/network-elements/nsx/src/main/java/org/apache/cloudstack/service/NsxServiceImpl.java
+++ b/plugins/network-elements/nsx/src/main/java/org/apache/cloudstack/service/NsxServiceImpl.java
@@ -139,14 +139,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 result.getResult();
+        return nsxControllerUtils.sendNsxCommand(createPortForwardCmd, netRule.getZoneId());
     }
 
     public boolean deletePortForwardRule(NsxNetworkRule netRule) {
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 ff7fa5427ee..59930d68d10 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
@@ -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);
@@ -279,7 +283,6 @@ 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)));
     }
 


(cloudstack) 05/06: NSX: Add shared network support (#41)

Posted by pe...@apache.org.
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 968235af18344fd12332b3743176ce22351bfaf7
Author: Pearl Dsilva <pe...@gmail.com>
AuthorDate: Wed Apr 10 10:10:00 2024 -0400

    NSX: Add shared network support (#41)
---
 .../src/main/java/com/cloud/network/NetworkServiceImpl.java   |  2 +-
 ui/src/views/network/CreateSharedNetworkForm.vue              | 11 ++---------
 2 files changed, 3 insertions(+), 10 deletions(-)

diff --git a/server/src/main/java/com/cloud/network/NetworkServiceImpl.java b/server/src/main/java/com/cloud/network/NetworkServiceImpl.java
index 6168de1ef63..2afa16a06d3 100644
--- a/server/src/main/java/com/cloud/network/NetworkServiceImpl.java
+++ b/server/src/main/java/com/cloud/network/NetworkServiceImpl.java
@@ -1749,7 +1749,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) && List.of(GuestType.L2).contains(guestType)) {
             throw new InvalidParameterValueException(
                     String.format("Creation of %s networks is not supported in NSX enabled zone %s", guestType.name(), zoneName)
             );
diff --git a/ui/src/views/network/CreateSharedNetworkForm.vue b/ui/src/views/network/CreateSharedNetworkForm.vue
index 4fd42370946..b0c7203f833 100644
--- a/ui/src/views/network/CreateSharedNetworkForm.vue
+++ b/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">
-        <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">
+      <div 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
               }