fix: LB Creation avoid 404 API errors due to non-needed patches

ZhyliaievD · ZhyliaievD · commit 9f079d8a78bf · 2026-03-17T11:31:53.000+02:00
Fix existing lbVirtualServer search by lbVirtualServerName
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
@@ -83,6 +83,7 @@
 import com.vmware.vapi.internal.protocol.client.rest.authn.BasicAuthenticationAppender;
 import com.vmware.vapi.protocol.HttpConfiguration;
 import com.vmware.vapi.std.errors.Error;
+import com.vmware.vapi.std.errors.NotFound;
 import org.apache.cloudstack.resource.NsxLoadBalancerMember;
 import org.apache.cloudstack.resource.NsxNetworkRule;
 import org.apache.cloudstack.utils.NsxControllerUtils;
@@ -96,9 +97,11 @@
 import java.util.Locale;
 import java.util.Objects;
 import java.util.Optional;
+import java.util.Set;
 import java.util.function.Function;
 import java.util.stream.Collectors;
 
+import static java.util.stream.Collectors.toSet;
 import static org.apache.cloudstack.utils.NsxControllerUtils.getServerPoolMemberName;
 import static org.apache.cloudstack.utils.NsxControllerUtils.getServerPoolName;
 import static org.apache.cloudstack.utils.NsxControllerUtils.getServiceName;
@@ -659,6 +662,13 @@ public void createNsxLbServerPool(List<NsxLoadBalancerMember> memberList, String
             String activeMonitorPath = getLbActiveMonitorPath(lbServerPoolName, privatePort, protocol);
             List<LBPoolMember> members = getLbPoolMembers(memberList, tier1GatewayName);
             LbPools lbPools = (LbPools) nsxService.apply(LbPools.class);
+            List<LBPoolMember> existingMembers = getNsxLbServerPool(lbPools, lbServerPoolName)
+                    .map(LBPool::getMembers)
+                    .orElse(null);
+            // Skip if pool exists and members unchanged
+            if (existingMembers != null && hasSamePoolMembers(existingMembers, members)) {
+                return;
+            }
             LBPool lbPool = new LBPool.Builder()
                     .setId(lbServerPoolName)
                     .setDisplayName(lbServerPoolName)
@@ -676,27 +686,65 @@ public void createNsxLbServerPool(List<NsxLoadBalancerMember> memberList, String
         }
     }
 
+    private Optional<LBPool> getNsxLbServerPool(LbPools lbPools, String lbServerPoolName) {
+        try {
+            return Optional.ofNullable(lbPools.get(lbServerPoolName));
+        } catch (NotFound e) {
+            logger.debug("Server Pool not found: {}", lbServerPoolName);
+            return Optional.empty();
+        }
+    }
+
+    private boolean hasSamePoolMembers(List<LBPoolMember> poolMembersCurrent, List<LBPoolMember> poolMembersUpdate) {
+        if (poolMembersCurrent == null || poolMembersUpdate == null) {
+            return poolMembersCurrent == poolMembersUpdate;
+        }
+        if (poolMembersCurrent.size() != poolMembersUpdate.size()) {
+            return false;
+        }
+        Set<String> currentMembersSet = poolMembersCurrent.stream()
+                .map(this::buildPoolMemberKey)
+                .collect(toSet());
+        return poolMembersUpdate.stream()
+                .map(this::buildPoolMemberKey)
+                .allMatch(currentMembersSet::contains);
+    }
+
+    private String buildPoolMemberKey(LBPoolMember member) {
+        return member.getIpAddress() + ':' + member.getPort() + ':' + member.getDisplayName();
+    }
+
     private String getLbActiveMonitorPath(String lbServerPoolName, String port, String protocol) {
         LbMonitorProfiles lbActiveMonitor = (LbMonitorProfiles) nsxService.apply(LbMonitorProfiles.class);
         String lbMonitorProfileId = getActiveMonitorProfileName(lbServerPoolName, port, protocol);
-        if ("TCP".equals(protocol.toUpperCase(Locale.ROOT))) {
-            LBTcpMonitorProfile lbTcpMonitorProfile = new LBTcpMonitorProfile.Builder(TCP_MONITOR_PROFILE)
-                    .setDisplayName(lbMonitorProfileId)
-                    .setMonitorPort(Long.parseLong(port))
-                    .build();
-            lbActiveMonitor.patch(lbMonitorProfileId, lbTcpMonitorProfile);
-        } else if ("UDP".equals(protocol.toUpperCase(Locale.ROOT))) {
-            LBIcmpMonitorProfile icmpMonitorProfile = new LBIcmpMonitorProfile.Builder(ICMP_MONITOR_PROFILE)
-                    .setDisplayName(lbMonitorProfileId)
-                    .build();
-            lbActiveMonitor.patch(lbMonitorProfileId, icmpMonitorProfile);
+        Optional<Structure> monitorProfile = getMonitorProfile(lbActiveMonitor, lbMonitorProfileId);
+        if (monitorProfile.isEmpty()) {
+            if ("TCP".equals(protocol.toUpperCase(Locale.ROOT))) {
+                LBTcpMonitorProfile lbTcpMonitorProfile = new LBTcpMonitorProfile.Builder(TCP_MONITOR_PROFILE)
+                        .setDisplayName(lbMonitorProfileId)
+                        .setMonitorPort(Long.parseLong(port))
+                        .build();
+                lbActiveMonitor.patch(lbMonitorProfileId, lbTcpMonitorProfile);
+            } else if ("UDP".equals(protocol.toUpperCase(Locale.ROOT))) {
+                LBIcmpMonitorProfile icmpMonitorProfile = new LBIcmpMonitorProfile.Builder(ICMP_MONITOR_PROFILE)
+                        .setDisplayName(lbMonitorProfileId)
+                        .build();
+                lbActiveMonitor.patch(lbMonitorProfileId, icmpMonitorProfile);
+            }
+            monitorProfile = getMonitorProfile(lbActiveMonitor, lbMonitorProfileId);
         }
-
-        LBMonitorProfileListResult listResult = listLBActiveMonitors(lbActiveMonitor);
-        Optional<Structure> monitorProfile = listResult.getResults().stream().filter(profile -> profile._getDataValue().getField("id").toString().equals(lbMonitorProfileId)).findFirst();
         return monitorProfile.map(structure -> structure._getDataValue().getField("path").toString()).orElse(null);
     }
 
+    private Optional<Structure> getMonitorProfile(LbMonitorProfiles lbActiveMonitor, String lbMonitorProfileId) {
+        try {
+            return Optional.ofNullable(lbActiveMonitor.get(lbMonitorProfileId));
+        } catch (NotFound e) {
+            logger.debug("LB Monitor Profile not found: {}", lbMonitorProfileId);
+            return Optional.empty();
+        }
+    }
+
     LBMonitorProfileListResult listLBActiveMonitors(LbMonitorProfiles lbActiveMonitor) {
         return lbActiveMonitor.list(null, false, null, null, null, null);
     }
@@ -735,7 +783,7 @@ public void createAndAddNsxLbVirtualServer(String tier1GatewayName, long lbId, S
             String lbVirtualServerName = getVirtualServerName(tier1GatewayName, lbId);
             String lbServiceName = getLoadBalancerName(tier1GatewayName);
             LbVirtualServers lbVirtualServers = (LbVirtualServers) nsxService.apply(LbVirtualServers.class);
-            if (Objects.nonNull(getLbVirtualServerService(lbVirtualServers, lbServiceName))) {
+            if (Objects.nonNull(getLbVirtualServerService(lbVirtualServers, lbVirtualServerName))) {
                 return;
             }
             LBVirtualServer lbVirtualServer = new LBVirtualServer.Builder()
diff --git a/plugins/network-elements/nsx/src/test/java/org/apache/cloudstack/service/NsxApiClientTest.java b/plugins/network-elements/nsx/src/test/java/org/apache/cloudstack/service/NsxApiClientTest.java
@@ -21,10 +21,22 @@
 import com.vmware.nsx.cluster.Status;
 import com.vmware.nsx.model.ClusterStatus;
 import com.vmware.nsx.model.ControllerClusterStatus;
+import com.vmware.nsx_policy.infra.LbMonitorProfiles;
+import com.vmware.nsx_policy.infra.LbPools;
+import com.vmware.nsx_policy.infra.LbServices;
+import com.vmware.nsx_policy.infra.LbVirtualServers;
 import com.vmware.nsx_policy.infra.domains.Groups;
 import com.vmware.nsx_policy.model.Group;
+import com.vmware.nsx_policy.model.LBTcpMonitorProfile;
+import com.vmware.nsx_policy.model.LBPool;
+import com.vmware.nsx_policy.model.LBPoolMember;
+import com.vmware.nsx_policy.model.LBVirtualServer;
 import com.vmware.nsx_policy.model.PathExpression;
 import com.vmware.vapi.bindings.Service;
+import com.vmware.vapi.bindings.Structure;
+import com.vmware.vapi.std.errors.NotFound;
+import org.apache.cloudstack.resource.NsxLoadBalancerMember;
+import org.apache.cloudstack.utils.NsxControllerUtils;
 import org.junit.Assert;
 import org.junit.Before;
 import org.junit.Test;
@@ -36,8 +48,16 @@
 import java.util.List;
 import java.util.function.Function;
 
+import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.ArgumentMatchers.anyString;
+import static org.mockito.ArgumentMatchers.eq;
+import static org.mockito.Mockito.never;
+import static org.mockito.Mockito.verify;
+
 public class NsxApiClientTest {
 
+    private static final String TIER_1_GATEWAY_NAME = "t1";
+
     @Mock
     private Function<Class<? extends Service>, Service> nsxService;
     @Mock
@@ -108,4 +128,159 @@ public void testIsNsxControllerActive() {
         Mockito.when(clusterStatus.getControlClusterStatus()).thenReturn(status);
         Assert.assertTrue(client.isNsxControllerActive());
     }
+
+    @Test
+    public void testCreateNsxLbServerPoolExistingMonitorProfileSkipsMonitorPatch() {
+        String lbServerPoolName = NsxControllerUtils.getServerPoolName(TIER_1_GATEWAY_NAME, 1L);
+        List<NsxLoadBalancerMember> memberList = List.of(new NsxLoadBalancerMember(1L, "10.0.0.1", 80));
+
+        LbPools lbPools = Mockito.mock(LbPools.class);
+        LbMonitorProfiles lbMonitorProfiles = mockLbMonitorProfiles();
+
+        Mockito.when(nsxService.apply(LbPools.class)).thenReturn(lbPools);
+        Mockito.when(lbPools.get(lbServerPoolName)).thenThrow(new NotFound(null, null));
+
+        client.createNsxLbServerPool(memberList, TIER_1_GATEWAY_NAME, lbServerPoolName, "roundrobin", "80", "TCP");
+
+        verify(lbMonitorProfiles, never()).patch(anyString(), any(LBTcpMonitorProfile.class));
+        verify(lbPools).patch(eq(lbServerPoolName), any(LBPool.class));
+    }
+
+    @Test
+    public void testCreateNsxLbServerPoolMissingMonitorProfilePerformsPatch() {
+        String lbServerPoolName = NsxControllerUtils.getServerPoolName(TIER_1_GATEWAY_NAME, 1L);
+        List<NsxLoadBalancerMember> memberList = List.of(new NsxLoadBalancerMember(1L, "10.0.0.1", 80));
+
+        LbPools lbPools = Mockito.mock(LbPools.class);
+        LbMonitorProfiles lbMonitorProfiles = Mockito.mock(LbMonitorProfiles.class);
+        Structure monitorStructure = Mockito.mock(Structure.class, Mockito.RETURNS_DEEP_STUBS);
+
+        Mockito.when(nsxService.apply(LbPools.class)).thenReturn(lbPools);
+        Mockito.when(nsxService.apply(LbMonitorProfiles.class)).thenReturn(lbMonitorProfiles);
+        Mockito.when(lbMonitorProfiles.get(anyString())).thenThrow(new NotFound(null, null)).thenReturn(monitorStructure);
+        Mockito.when(monitorStructure._getDataValue().getField("path").toString()).thenReturn("/infra/lb-monitor-profiles/test");
+        Mockito.when(lbPools.get(lbServerPoolName)).thenThrow(new NotFound(null, null));
+
+        client.createNsxLbServerPool(memberList, TIER_1_GATEWAY_NAME, lbServerPoolName, "roundrobin", "80", "TCP");
+
+        verify(lbMonitorProfiles).patch(anyString(), any(LBTcpMonitorProfile.class));
+        verify(lbPools).patch(eq(lbServerPoolName), any(LBPool.class));
+    }
+
+    @Test
+    public void testCreateNsxLbServerPoolPoolExistsWithSameMembersSkipsPatch() {
+        long lbId = 1L;
+        String lbServerPoolName = NsxControllerUtils.getServerPoolName(TIER_1_GATEWAY_NAME, lbId);
+        List<NsxLoadBalancerMember> memberList = List.of(
+                new NsxLoadBalancerMember(1L, "10.0.0.1", 80),
+                new NsxLoadBalancerMember(2L, "10.0.0.2", 80)
+        );
+        List<LBPoolMember> sameMembers = List.of(
+                new LBPoolMember.Builder()
+                        .setDisplayName(NsxControllerUtils.getServerPoolMemberName(TIER_1_GATEWAY_NAME, 2L))
+                        .setIpAddress("10.0.0.2")
+                        .setPort("80")
+                        .build(),
+                new LBPoolMember.Builder()
+                        .setDisplayName(NsxControllerUtils.getServerPoolMemberName(TIER_1_GATEWAY_NAME, 1L))
+                        .setIpAddress("10.0.0.1")
+                        .setPort("80")
+                        .build()
+        );
+
+        LbPools lbPools = Mockito.mock(LbPools.class);
+        LBPool existingPool = Mockito.mock(LBPool.class);
+        LbMonitorProfiles lbMonitorProfiles = mockLbMonitorProfiles();
+
+        Mockito.when(nsxService.apply(LbPools.class)).thenReturn(lbPools);
+        Mockito.when(lbPools.get(lbServerPoolName)).thenReturn(existingPool);
+        Mockito.when(existingPool.getMembers()).thenReturn(sameMembers);
+
+        client.createNsxLbServerPool(memberList, TIER_1_GATEWAY_NAME, lbServerPoolName, "roundrobin", "80", "TCP");
+
+        verify(lbMonitorProfiles, never()).patch(anyString(), any(LBTcpMonitorProfile.class));
+        verify(lbPools, never()).patch(anyString(), any(LBPool.class));
+    }
+
+    @Test
+    public void testCreateNsxLbServerPoolPoolExistsWithDifferentMembersPerformsPatch() {
+        long lbId = 1L;
+        String lbServerPoolName = NsxControllerUtils.getServerPoolName(TIER_1_GATEWAY_NAME, lbId);
+        List<NsxLoadBalancerMember> memberList = List.of(
+                new NsxLoadBalancerMember(1L, "10.0.0.1", 80),
+                new NsxLoadBalancerMember(2L, "10.0.0.2", 80)
+        );
+
+        LbPools lbPools = Mockito.mock(LbPools.class);
+        LBPool existingPool = Mockito.mock(LBPool.class);
+
+        mockLbMonitorProfiles();
+        Mockito.when(nsxService.apply(LbPools.class)).thenReturn(lbPools);
+        Mockito.when(lbPools.get(lbServerPoolName)).thenReturn(existingPool);
+        Mockito.when(existingPool.getMembers()).thenReturn(List.of(
+                new LBPoolMember.Builder()
+                        .setDisplayName(NsxControllerUtils.getServerPoolMemberName(TIER_1_GATEWAY_NAME, 1L))
+                        .setIpAddress("10.0.0.10")
+                        .setPort("80")
+                        .build()
+        ));
+
+        client.createNsxLbServerPool(memberList, TIER_1_GATEWAY_NAME, lbServerPoolName, "roundrobin", "80", "TCP");
+
+        verify(lbPools).patch(eq(lbServerPoolName), any(LBPool.class));
+    }
+
+    @Test
+    public void testCreateNsxLbServerPoolPoolDoesNotExistPerformsPatch() {
+        String lbServerPoolName = NsxControllerUtils.getServerPoolName(TIER_1_GATEWAY_NAME, 1L);
+        List<NsxLoadBalancerMember> memberList = List.of(new NsxLoadBalancerMember(1L, "10.0.0.1", 80));
+
+        LbPools lbPools = Mockito.mock(LbPools.class);
+
+        mockLbMonitorProfiles();
+        Mockito.when(nsxService.apply(LbPools.class)).thenReturn(lbPools);
+        Mockito.when(lbPools.get(lbServerPoolName)).thenThrow(new NotFound(null, null));
+
+        client.createNsxLbServerPool(memberList, TIER_1_GATEWAY_NAME, lbServerPoolName, "roundrobin", "80", "TCP");
+
+        verify(lbPools).patch(eq(lbServerPoolName), any(LBPool.class));
+    }
+
+    @Test
+    public void testCreateAndAddNsxLbVirtualServerVirtualServerAlreadyExistsSkipsPatch() {
+        long lbId = 1L;
+        String lbVirtualServerName = NsxControllerUtils.getVirtualServerName(TIER_1_GATEWAY_NAME, lbId);
+        String lbServiceName = NsxControllerUtils.getLoadBalancerName(TIER_1_GATEWAY_NAME);
+        List<NsxLoadBalancerMember> memberList = List.of(new NsxLoadBalancerMember(1L, "10.0.0.1", 80));
+
+        LbPools lbPools = Mockito.mock(LbPools.class);
+        LbServices lbServices = Mockito.mock(LbServices.class);
+        LbVirtualServers lbVirtualServers = Mockito.mock(LbVirtualServers.class);
+        LBVirtualServer existingVs = Mockito.mock(LBVirtualServer.class);
+
+        mockLbMonitorProfiles();
+        Mockito.when(nsxService.apply(LbPools.class)).thenReturn(lbPools);
+        Mockito.when(nsxService.apply(LbServices.class)).thenReturn(lbServices);
+        Mockito.when(nsxService.apply(LbVirtualServers.class)).thenReturn(lbVirtualServers);
+        Mockito.when(lbPools.get(anyString())).thenThrow(new NotFound(null, null));
+        Mockito.when(lbServices.get(anyString())).thenReturn(null);
+        Mockito.when(lbVirtualServers.get(lbVirtualServerName)).thenReturn(existingVs);
+
+        client.createAndAddNsxLbVirtualServer(TIER_1_GATEWAY_NAME, lbId, "192.168.1.1", "443",
+                memberList, "roundrobin", "TCP", "80");
+
+        verify(lbVirtualServers).get(lbVirtualServerName);
+        verify(lbVirtualServers, never()).get(lbServiceName);
+        verify(lbVirtualServers, never()).patch(anyString(), any(LBVirtualServer.class));
+    }
+
+    private LbMonitorProfiles mockLbMonitorProfiles() {
+        LbMonitorProfiles lbMonitorProfiles = Mockito.mock(LbMonitorProfiles.class);
+        Structure monitorStructure = Mockito.mock(Structure.class, Mockito.RETURNS_DEEP_STUBS);
+
+        Mockito.when(nsxService.apply(LbMonitorProfiles.class)).thenReturn(lbMonitorProfiles);
+        Mockito.when(lbMonitorProfiles.get(anyString())).thenReturn(monitorStructure);
+        Mockito.when(monitorStructure._getDataValue().getField("path").toString()).thenReturn("/infra/lb-monitor-profiles/test");
+        return lbMonitorProfiles;
+    }
 }
