Skip to content

Commit 4ea0f3d

Browse files
ZhyliaievDZhyliaievD
committed
fix: LB Creation avoid 404 API errors due to non-needed patches
Fix existing lbVirtualServer search by lbVirtualServerName
1 parent b744824 commit 4ea0f3d

File tree

2 files changed

+286
-16
lines changed

2 files changed

+286
-16
lines changed

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

Lines changed: 61 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -83,6 +83,7 @@
8383
import com.vmware.vapi.internal.protocol.client.rest.authn.BasicAuthenticationAppender;
8484
import com.vmware.vapi.protocol.HttpConfiguration;
8585
import com.vmware.vapi.std.errors.Error;
86+
import com.vmware.vapi.std.errors.NotFound;
8687
import org.apache.cloudstack.resource.NsxLoadBalancerMember;
8788
import org.apache.cloudstack.resource.NsxNetworkRule;
8889
import org.apache.cloudstack.utils.NsxControllerUtils;
@@ -96,9 +97,11 @@
9697
import java.util.Locale;
9798
import java.util.Objects;
9899
import java.util.Optional;
100+
import java.util.Set;
99101
import java.util.function.Function;
100102
import java.util.stream.Collectors;
101103

104+
import static java.util.stream.Collectors.toSet;
102105
import static org.apache.cloudstack.utils.NsxControllerUtils.getServerPoolMemberName;
103106
import static org.apache.cloudstack.utils.NsxControllerUtils.getServerPoolName;
104107
import static org.apache.cloudstack.utils.NsxControllerUtils.getServiceName;
@@ -656,9 +659,16 @@ List<LBPoolMember> getLbPoolMembers(List<NsxLoadBalancerMember> memberList, Stri
656659
public void createNsxLbServerPool(List<NsxLoadBalancerMember> memberList, String tier1GatewayName, String lbServerPoolName,
657660
String algorithm, String privatePort, String protocol) {
658661
try {
659-
String activeMonitorPath = getLbActiveMonitorPath(lbServerPoolName, privatePort, protocol);
660662
List<LBPoolMember> members = getLbPoolMembers(memberList, tier1GatewayName);
661663
LbPools lbPools = (LbPools) nsxService.apply(LbPools.class);
664+
List<LBPoolMember> existingMembers = getNsxLbServerPool(lbPools, lbServerPoolName)
665+
.map(LBPool::getMembers)
666+
.orElse(null);
667+
// Skip if pool exists and members unchanged
668+
if (existingMembers != null && hasSamePoolMembers(existingMembers, members)) {
669+
return;
670+
}
671+
String activeMonitorPath = getLbActiveMonitorPath(lbServerPoolName, privatePort, protocol);
662672
LBPool lbPool = new LBPool.Builder()
663673
.setId(lbServerPoolName)
664674
.setDisplayName(lbServerPoolName)
@@ -676,27 +686,62 @@ public void createNsxLbServerPool(List<NsxLoadBalancerMember> memberList, String
676686
}
677687
}
678688

689+
private Optional<LBPool> getNsxLbServerPool(LbPools lbPools, String lbServerPoolName) {
690+
try {
691+
return Optional.ofNullable(lbPools.get(lbServerPoolName));
692+
} catch (NotFound e) {
693+
logger.debug("Server Pool not found: {}", lbServerPoolName);
694+
return Optional.empty();
695+
}
696+
}
697+
698+
private boolean hasSamePoolMembers(List<LBPoolMember> poolMembersCurrent, List<LBPoolMember> poolMembersUpdate) {
699+
if (poolMembersCurrent.size() != poolMembersUpdate.size()) {
700+
return false;
701+
}
702+
Set<String> currentMembersSet = poolMembersCurrent.stream()
703+
.map(this::buildPoolMemberKey)
704+
.collect(toSet());
705+
return poolMembersUpdate.stream()
706+
.map(this::buildPoolMemberKey)
707+
.allMatch(currentMembersSet::contains);
708+
}
709+
710+
private String buildPoolMemberKey(LBPoolMember member) {
711+
return member.getIpAddress() + ':' + member.getPort() + ':' + member.getDisplayName();
712+
}
713+
679714
private String getLbActiveMonitorPath(String lbServerPoolName, String port, String protocol) {
680715
LbMonitorProfiles lbActiveMonitor = (LbMonitorProfiles) nsxService.apply(LbMonitorProfiles.class);
681716
String lbMonitorProfileId = getActiveMonitorProfileName(lbServerPoolName, port, protocol);
682-
if ("TCP".equals(protocol.toUpperCase(Locale.ROOT))) {
683-
LBTcpMonitorProfile lbTcpMonitorProfile = new LBTcpMonitorProfile.Builder(TCP_MONITOR_PROFILE)
684-
.setDisplayName(lbMonitorProfileId)
685-
.setMonitorPort(Long.parseLong(port))
686-
.build();
687-
lbActiveMonitor.patch(lbMonitorProfileId, lbTcpMonitorProfile);
688-
} else if ("UDP".equals(protocol.toUpperCase(Locale.ROOT))) {
689-
LBIcmpMonitorProfile icmpMonitorProfile = new LBIcmpMonitorProfile.Builder(ICMP_MONITOR_PROFILE)
690-
.setDisplayName(lbMonitorProfileId)
691-
.build();
692-
lbActiveMonitor.patch(lbMonitorProfileId, icmpMonitorProfile);
717+
Optional<Structure> monitorProfile = getMonitorProfile(lbActiveMonitor, lbMonitorProfileId);
718+
if (monitorProfile.isEmpty()) {
719+
if ("TCP".equals(protocol.toUpperCase(Locale.ROOT))) {
720+
LBTcpMonitorProfile lbTcpMonitorProfile = new LBTcpMonitorProfile.Builder(TCP_MONITOR_PROFILE)
721+
.setDisplayName(lbMonitorProfileId)
722+
.setMonitorPort(Long.parseLong(port))
723+
.build();
724+
lbActiveMonitor.patch(lbMonitorProfileId, lbTcpMonitorProfile);
725+
} else if ("UDP".equals(protocol.toUpperCase(Locale.ROOT))) {
726+
LBIcmpMonitorProfile icmpMonitorProfile = new LBIcmpMonitorProfile.Builder(ICMP_MONITOR_PROFILE)
727+
.setDisplayName(lbMonitorProfileId)
728+
.build();
729+
lbActiveMonitor.patch(lbMonitorProfileId, icmpMonitorProfile);
730+
}
731+
monitorProfile = getMonitorProfile(lbActiveMonitor, lbMonitorProfileId);
693732
}
694-
695-
LBMonitorProfileListResult listResult = listLBActiveMonitors(lbActiveMonitor);
696-
Optional<Structure> monitorProfile = listResult.getResults().stream().filter(profile -> profile._getDataValue().getField("id").toString().equals(lbMonitorProfileId)).findFirst();
697733
return monitorProfile.map(structure -> structure._getDataValue().getField("path").toString()).orElse(null);
698734
}
699735

736+
private Optional<Structure> getMonitorProfile(LbMonitorProfiles lbActiveMonitor, String lbMonitorProfileId) {
737+
try {
738+
return Optional.ofNullable(lbActiveMonitor.get(lbMonitorProfileId));
739+
} catch (NotFound e) {
740+
logger.debug("LB Monitor Profile not found: {}", lbMonitorProfileId);
741+
return Optional.empty();
742+
}
743+
}
744+
700745
LBMonitorProfileListResult listLBActiveMonitors(LbMonitorProfiles lbActiveMonitor) {
701746
return lbActiveMonitor.list(null, false, null, null, null, null);
702747
}
@@ -735,7 +780,7 @@ public void createAndAddNsxLbVirtualServer(String tier1GatewayName, long lbId, S
735780
String lbVirtualServerName = getVirtualServerName(tier1GatewayName, lbId);
736781
String lbServiceName = getLoadBalancerName(tier1GatewayName);
737782
LbVirtualServers lbVirtualServers = (LbVirtualServers) nsxService.apply(LbVirtualServers.class);
738-
if (Objects.nonNull(getLbVirtualServerService(lbVirtualServers, lbServiceName))) {
783+
if (Objects.nonNull(getLbVirtualServerService(lbVirtualServers, lbVirtualServerName))) {
739784
return;
740785
}
741786
LBVirtualServer lbVirtualServer = new LBVirtualServer.Builder()

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

Lines changed: 225 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,13 +18,29 @@
1818

1919
import com.cloud.network.Network;
2020
import com.cloud.network.SDNProviderNetworkRule;
21+
import com.cloud.utils.exception.CloudRuntimeException;
2122
import com.vmware.nsx.cluster.Status;
2223
import com.vmware.nsx.model.ClusterStatus;
2324
import com.vmware.nsx.model.ControllerClusterStatus;
25+
import com.vmware.nsx_policy.infra.LbMonitorProfiles;
26+
import com.vmware.nsx_policy.infra.LbPools;
27+
import com.vmware.nsx_policy.infra.LbServices;
28+
import com.vmware.nsx_policy.infra.LbVirtualServers;
2429
import com.vmware.nsx_policy.infra.domains.Groups;
30+
import com.vmware.nsx_policy.model.ApiError;
2531
import com.vmware.nsx_policy.model.Group;
32+
import com.vmware.nsx_policy.model.LBIcmpMonitorProfile;
33+
import com.vmware.nsx_policy.model.LBTcpMonitorProfile;
34+
import com.vmware.nsx_policy.model.LBPool;
35+
import com.vmware.nsx_policy.model.LBPoolMember;
36+
import com.vmware.nsx_policy.model.LBVirtualServer;
2637
import com.vmware.nsx_policy.model.PathExpression;
2738
import com.vmware.vapi.bindings.Service;
39+
import com.vmware.vapi.bindings.Structure;
40+
import com.vmware.vapi.std.errors.Error;
41+
import com.vmware.vapi.std.errors.NotFound;
42+
import org.apache.cloudstack.resource.NsxLoadBalancerMember;
43+
import org.apache.cloudstack.utils.NsxControllerUtils;
2844
import org.junit.Assert;
2945
import org.junit.Before;
3046
import org.junit.Test;
@@ -36,8 +52,20 @@
3652
import java.util.List;
3753
import java.util.function.Function;
3854

55+
import static org.junit.Assert.assertThrows;
56+
import static org.junit.Assert.assertTrue;
57+
import static org.mockito.ArgumentMatchers.any;
58+
import static org.mockito.ArgumentMatchers.anyString;
59+
import static org.mockito.ArgumentMatchers.eq;
60+
import static org.mockito.Mockito.doThrow;
61+
import static org.mockito.Mockito.never;
62+
import static org.mockito.Mockito.verify;
63+
import static org.mockito.Mockito.when;
64+
3965
public class NsxApiClientTest {
4066

67+
private static final String TIER_1_GATEWAY_NAME = "t1";
68+
4169
@Mock
4270
private Function<Class<? extends Service>, Service> nsxService;
4371
@Mock
@@ -108,4 +136,201 @@ public void testIsNsxControllerActive() {
108136
Mockito.when(clusterStatus.getControlClusterStatus()).thenReturn(status);
109137
Assert.assertTrue(client.isNsxControllerActive());
110138
}
139+
140+
@Test
141+
public void testCreateNsxLbServerPoolExistingMonitorProfileSkipsMonitorPatch() {
142+
String lbServerPoolName = NsxControllerUtils.getServerPoolName(TIER_1_GATEWAY_NAME, 1L);
143+
List<NsxLoadBalancerMember> memberList = List.of(new NsxLoadBalancerMember(1L, "10.0.0.1", 80));
144+
145+
LbPools lbPools = Mockito.mock(LbPools.class);
146+
LbMonitorProfiles lbMonitorProfiles = mockLbMonitorProfiles();
147+
148+
Mockito.when(nsxService.apply(LbPools.class)).thenReturn(lbPools);
149+
Mockito.when(lbPools.get(lbServerPoolName)).thenThrow(new NotFound(null, null));
150+
151+
client.createNsxLbServerPool(memberList, TIER_1_GATEWAY_NAME, lbServerPoolName, "roundrobin", "80", "TCP");
152+
153+
verify(lbMonitorProfiles, never()).patch(anyString(), any(LBTcpMonitorProfile.class));
154+
verify(lbPools).patch(eq(lbServerPoolName), any(LBPool.class));
155+
}
156+
157+
@Test
158+
public void testCreateNsxLbServerPoolMissingMonitorTCPProfilePerformsPatch() {
159+
String lbServerPoolName = NsxControllerUtils.getServerPoolName(TIER_1_GATEWAY_NAME, 1L);
160+
List<NsxLoadBalancerMember> memberList = List.of(new NsxLoadBalancerMember(1L, "10.0.0.1", 80));
161+
162+
LbPools lbPools = Mockito.mock(LbPools.class);
163+
LbMonitorProfiles lbMonitorProfiles = Mockito.mock(LbMonitorProfiles.class);
164+
Structure monitorStructure = Mockito.mock(Structure.class, Mockito.RETURNS_DEEP_STUBS);
165+
166+
Mockito.when(nsxService.apply(LbPools.class)).thenReturn(lbPools);
167+
Mockito.when(nsxService.apply(LbMonitorProfiles.class)).thenReturn(lbMonitorProfiles);
168+
Mockito.when(lbMonitorProfiles.get(anyString())).thenThrow(new NotFound(null, null)).thenReturn(monitorStructure);
169+
Mockito.when(monitorStructure._getDataValue().getField("path").toString()).thenReturn("/infra/lb-monitor-profiles/test");
170+
Mockito.when(lbPools.get(lbServerPoolName)).thenThrow(new NotFound(null, null));
171+
172+
client.createNsxLbServerPool(memberList, TIER_1_GATEWAY_NAME, lbServerPoolName, "roundrobin", "80", "TCP");
173+
174+
verify(lbMonitorProfiles).patch(anyString(), any(LBTcpMonitorProfile.class));
175+
verify(lbPools).patch(eq(lbServerPoolName), any(LBPool.class));
176+
}
177+
178+
@Test
179+
public void testCreateNsxLbServerPoolMissingMonitorUDPProfilePerformsPatch() {
180+
String lbServerPoolName = NsxControllerUtils.getServerPoolName(TIER_1_GATEWAY_NAME, 1L);
181+
List<NsxLoadBalancerMember> memberList = List.of(new NsxLoadBalancerMember(1L, "10.0.0.1", 80));
182+
183+
LbPools lbPools = Mockito.mock(LbPools.class);
184+
LbMonitorProfiles lbMonitorProfiles = Mockito.mock(LbMonitorProfiles.class);
185+
Structure monitorStructure = Mockito.mock(Structure.class, Mockito.RETURNS_DEEP_STUBS);
186+
187+
Mockito.when(nsxService.apply(LbPools.class)).thenReturn(lbPools);
188+
Mockito.when(nsxService.apply(LbMonitorProfiles.class)).thenReturn(lbMonitorProfiles);
189+
Mockito.when(lbMonitorProfiles.get(anyString())).thenThrow(new NotFound(null, null)).thenReturn(monitorStructure);
190+
Mockito.when(monitorStructure._getDataValue().getField("path").toString()).thenReturn("/infra/lb-monitor-profiles/test");
191+
Mockito.when(lbPools.get(lbServerPoolName)).thenThrow(new NotFound(null, null));
192+
193+
client.createNsxLbServerPool(memberList, TIER_1_GATEWAY_NAME, lbServerPoolName, "roundrobin", "80", "UDP");
194+
195+
verify(lbMonitorProfiles).patch(anyString(), any(LBIcmpMonitorProfile.class));
196+
verify(lbPools).patch(eq(lbServerPoolName), any(LBPool.class));
197+
}
198+
199+
@Test
200+
public void testCreateNsxLbServerPoolPoolExistsWithSameMembersSkipsPatch() {
201+
long lbId = 1L;
202+
String lbServerPoolName = NsxControllerUtils.getServerPoolName(TIER_1_GATEWAY_NAME, lbId);
203+
List<NsxLoadBalancerMember> memberList = List.of(
204+
new NsxLoadBalancerMember(1L, "10.0.0.1", 80),
205+
new NsxLoadBalancerMember(2L, "10.0.0.2", 80)
206+
);
207+
List<LBPoolMember> sameMembers = List.of(
208+
new LBPoolMember.Builder()
209+
.setDisplayName(NsxControllerUtils.getServerPoolMemberName(TIER_1_GATEWAY_NAME, 2L))
210+
.setIpAddress("10.0.0.2")
211+
.setPort("80")
212+
.build(),
213+
new LBPoolMember.Builder()
214+
.setDisplayName(NsxControllerUtils.getServerPoolMemberName(TIER_1_GATEWAY_NAME, 1L))
215+
.setIpAddress("10.0.0.1")
216+
.setPort("80")
217+
.build()
218+
);
219+
220+
LbPools lbPools = Mockito.mock(LbPools.class);
221+
LBPool existingPool = Mockito.mock(LBPool.class);
222+
223+
Mockito.when(nsxService.apply(LbPools.class)).thenReturn(lbPools);
224+
Mockito.when(lbPools.get(lbServerPoolName)).thenReturn(existingPool);
225+
Mockito.when(existingPool.getMembers()).thenReturn(sameMembers);
226+
227+
client.createNsxLbServerPool(memberList, TIER_1_GATEWAY_NAME, lbServerPoolName, "roundrobin", "80", "TCP");
228+
229+
verify(nsxService, never()).apply(LbMonitorProfiles.class);
230+
verify(lbPools, never()).patch(anyString(), any(LBPool.class));
231+
}
232+
233+
@Test
234+
public void testCreateNsxLbServerPoolPoolExistsWithDifferentMembersPerformsPatch() {
235+
long lbId = 1L;
236+
String lbServerPoolName = NsxControllerUtils.getServerPoolName(TIER_1_GATEWAY_NAME, lbId);
237+
List<NsxLoadBalancerMember> memberList = List.of(
238+
new NsxLoadBalancerMember(1L, "10.0.0.1", 80),
239+
new NsxLoadBalancerMember(2L, "10.0.0.2", 80)
240+
);
241+
242+
LbPools lbPools = Mockito.mock(LbPools.class);
243+
LBPool existingPool = Mockito.mock(LBPool.class);
244+
245+
mockLbMonitorProfiles();
246+
Mockito.when(nsxService.apply(LbPools.class)).thenReturn(lbPools);
247+
Mockito.when(lbPools.get(lbServerPoolName)).thenReturn(existingPool);
248+
Mockito.when(existingPool.getMembers()).thenReturn(List.of(
249+
new LBPoolMember.Builder()
250+
.setDisplayName(NsxControllerUtils.getServerPoolMemberName(TIER_1_GATEWAY_NAME, 1L))
251+
.setIpAddress("10.0.0.10")
252+
.setPort("80")
253+
.build()
254+
));
255+
256+
client.createNsxLbServerPool(memberList, TIER_1_GATEWAY_NAME, lbServerPoolName, "roundrobin", "80", "TCP");
257+
258+
verify(lbPools).patch(eq(lbServerPoolName), any(LBPool.class));
259+
}
260+
261+
@Test
262+
public void testCreateNsxLbServerPoolPoolDoesNotExistPerformsPatch() {
263+
String lbServerPoolName = NsxControllerUtils.getServerPoolName(TIER_1_GATEWAY_NAME, 1L);
264+
List<NsxLoadBalancerMember> memberList = List.of(new NsxLoadBalancerMember(1L, "10.0.0.1", 80));
265+
266+
LbPools lbPools = Mockito.mock(LbPools.class);
267+
268+
mockLbMonitorProfiles();
269+
Mockito.when(nsxService.apply(LbPools.class)).thenReturn(lbPools);
270+
Mockito.when(lbPools.get(lbServerPoolName)).thenThrow(new NotFound(null, null));
271+
272+
client.createNsxLbServerPool(memberList, TIER_1_GATEWAY_NAME, lbServerPoolName, "roundrobin", "80", "TCP");
273+
274+
verify(lbPools).patch(eq(lbServerPoolName), any(LBPool.class));
275+
}
276+
277+
@Test
278+
public void testCreateAndAddNsxLbVirtualServerVirtualServerAlreadyExistsSkipsPatch() {
279+
long lbId = 1L;
280+
String lbVirtualServerName = NsxControllerUtils.getVirtualServerName(TIER_1_GATEWAY_NAME, lbId);
281+
String lbServiceName = NsxControllerUtils.getLoadBalancerName(TIER_1_GATEWAY_NAME);
282+
List<NsxLoadBalancerMember> memberList = List.of(new NsxLoadBalancerMember(1L, "10.0.0.1", 80));
283+
284+
LbPools lbPools = Mockito.mock(LbPools.class);
285+
LbServices lbServices = Mockito.mock(LbServices.class);
286+
LbVirtualServers lbVirtualServers = Mockito.mock(LbVirtualServers.class);
287+
LBVirtualServer existingVs = Mockito.mock(LBVirtualServer.class);
288+
289+
mockLbMonitorProfiles();
290+
Mockito.when(nsxService.apply(LbPools.class)).thenReturn(lbPools);
291+
Mockito.when(nsxService.apply(LbServices.class)).thenReturn(lbServices);
292+
Mockito.when(nsxService.apply(LbVirtualServers.class)).thenReturn(lbVirtualServers);
293+
Mockito.when(lbPools.get(anyString())).thenThrow(new NotFound(null, null));
294+
Mockito.when(lbServices.get(anyString())).thenReturn(null);
295+
Mockito.when(lbVirtualServers.get(lbVirtualServerName)).thenReturn(existingVs);
296+
297+
client.createAndAddNsxLbVirtualServer(TIER_1_GATEWAY_NAME, lbId, "192.168.1.1", "443",
298+
memberList, "roundrobin", "TCP", "80");
299+
300+
verify(lbVirtualServers).get(lbVirtualServerName);
301+
verify(lbVirtualServers, never()).get(lbServiceName);
302+
verify(lbVirtualServers, never()).patch(anyString(), any(LBVirtualServer.class));
303+
}
304+
305+
@Test
306+
public void testCreateNsxLbServerPoolThrowsExceptionOnPatchError() {
307+
String lbServerPoolName = NsxControllerUtils.getServerPoolName(TIER_1_GATEWAY_NAME, 1L);
308+
List<NsxLoadBalancerMember> memberList = List.of(new NsxLoadBalancerMember(1L, "10.0.0.1", 80));
309+
310+
LbPools lbPools = Mockito.mock(LbPools.class);
311+
Structure errorData = Mockito.mock(Structure.class);
312+
ApiError apiError = new ApiError();
313+
apiError.setErrorData(errorData);
314+
315+
mockLbMonitorProfiles();
316+
Mockito.when(nsxService.apply(LbPools.class)).thenReturn(lbPools);
317+
Mockito.when(lbPools.get(lbServerPoolName)).thenThrow(new NotFound(null, null));
318+
when(errorData._convertTo(ApiError.class)).thenReturn(apiError);
319+
doThrow(new Error(List.of(), errorData)).when(lbPools).patch(eq(lbServerPoolName), any(LBPool.class));
320+
321+
CloudRuntimeException thrownException = assertThrows(CloudRuntimeException.class, () -> {
322+
client.createNsxLbServerPool(memberList, TIER_1_GATEWAY_NAME, lbServerPoolName, "roundrobin", "80", "TCP");
323+
});
324+
assertTrue(thrownException.getMessage().startsWith("Failed to create NSX LB server pool, due to"));
325+
}
326+
327+
private LbMonitorProfiles mockLbMonitorProfiles() {
328+
LbMonitorProfiles lbMonitorProfiles = Mockito.mock(LbMonitorProfiles.class);
329+
Structure monitorStructure = Mockito.mock(Structure.class, Mockito.RETURNS_DEEP_STUBS);
330+
331+
Mockito.when(nsxService.apply(LbMonitorProfiles.class)).thenReturn(lbMonitorProfiles);
332+
Mockito.when(lbMonitorProfiles.get(anyString())).thenReturn(monitorStructure);
333+
Mockito.when(monitorStructure._getDataValue().getField("path").toString()).thenReturn("/infra/lb-monitor-profiles/test");
334+
return lbMonitorProfiles;
335+
}
111336
}

0 commit comments

Comments
 (0)