winterhazel commented on code in PR #12835:
URL: https://github.com/apache/cloudstack/pull/12835#discussion_r3041220623
##########
plugins/network-elements/nsx/src/main/java/org/apache/cloudstack/service/NsxApiClient.java:
##########
@@ -656,9 +659,19 @@ List<LBPoolMember>
getLbPoolMembers(List<NsxLoadBalancerMember> memberList, Stri
public void createNsxLbServerPool(List<NsxLoadBalancerMember> memberList,
String tier1GatewayName, String lbServerPoolName,
String algorithm, String privatePort,
String protocol) {
try {
- String activeMonitorPath =
getLbActiveMonitorPath(lbServerPoolName, privatePort, protocol);
List<LBPoolMember> members = getLbPoolMembers(memberList,
tier1GatewayName);
LbPools lbPools = (LbPools) nsxService.apply(LbPools.class);
+ Optional<LBPool> nsxLbServerPool = getNsxLbServerPool(lbPools,
lbServerPoolName);
+ // Skip if pool exists and members unchanged
+ if (nsxLbServerPool.isPresent()) {
+ List<LBPoolMember> existingMembers = nsxLbServerPool
+ .map(LBPool::getMembers)
+ .orElseGet(List::of);
+ if (hasSamePoolMembers(existingMembers, members)) {
+ return;
Review Comment:
A debug log here informing that the patch will be skipped because there
aren't changes may be nice to have.
##########
plugins/network-elements/nsx/src/main/java/org/apache/cloudstack/service/NsxApiClient.java:
##########
@@ -676,27 +689,62 @@ 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);
Review Comment:
Isn't this log better suited for `warn`?
##########
plugins/network-elements/nsx/src/main/java/org/apache/cloudstack/service/NsxApiClient.java:
##########
@@ -676,27 +689,62 @@ 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> existingMembers,
List<LBPoolMember> membersUpdate) {
+ Set<String> existingMembersSet = existingMembers.stream()
+ .map(this::buildPoolMemberKey)
+ .collect(toSet());
+ Set<String> updateMembersSet = membersUpdate.stream()
+ .map(this::buildPoolMemberKey)
+ .collect(toSet());
+
+ return existingMembersSet.size() == updateMembersSet.size()
+ && existingMembersSet.containsAll(updateMembersSet);
+ }
+
+ 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);
Review Comment:
This patch section could be extracted into a separate method to reduce the
indentation
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
