weizhouapache commented on code in PR #10710:
URL: https://github.com/apache/cloudstack/pull/10710#discussion_r2309298866
##########
api/src/main/java/org/apache/cloudstack/api/response/RouterHealthCheckResultResponse.java:
##########
@@ -19,6 +19,7 @@
import java.util.Date;
+import com.cloud.network.VirtualNetworkApplianceService;
Review Comment:
may import
`com.cloud.network.VirtualNetworkApplianceService.RouterHealthStatus` instead
##########
server/src/main/java/com/cloud/network/router/VirtualNetworkApplianceManagerImpl.java:
##########
@@ -824,26 +811,23 @@ protected void updateSite2SiteVpnConnectionState(final
List<DomainRouterVO> rout
}
final String privateIP = router.getPrivateIpAddress();
final HostVO host = _hostDao.findById(router.getHostId());
- if (host == null || host.getState() != Status.Up) {
- continue;
- } else if (host.getManagementServerId() !=
ManagementServerNode.getManagementServerId()) {
- /* Only cover hosts managed by this management server */
- continue;
- } else if (privateIP != null) {
+ if ( !(host == null || host.getState() != Status.Up)
Review Comment:
maybe define a boolean variable ?
##########
server/src/main/java/com/cloud/network/router/VirtualNetworkApplianceManagerImpl.java:
##########
@@ -1176,35 +1129,34 @@ protected void runInContext() {
}
} catch (final Exception ex) {
logger.error("Fail to complete the
FetchRouterHealthChecksResultTask! ", ex);
- ex.printStackTrace();
}
}
}
private List<String> getFailingChecks(DomainRouterVO router,
GetRouterMonitorResultsAnswer answer) {
if (answer == null) {
- logger.warn("Unable to fetch monitor results for router " +
router);
- resetRouterHealthChecksAndConnectivity(router.getId(), false,
false, "Communication failed");
- return Arrays.asList(CONNECTIVITY_TEST);
+ logger.warn("Unable to fetch monitor results for router {}",
router);
+ resetRouterHealthChecksAndConnectivity(router.getId(),
RouterHealthStatus.UNKNOWN, RouterHealthStatus.UNKNOWN, "Communication failed");
+ return List.of(CONNECTIVITY_TEST);
} else if (!answer.getResult()) {
logger.warn("Failed to fetch monitor results from router " +
router + " with details: " + answer.getDetails());
if (StringUtils.isNotBlank(answer.getDetails()) &&
answer.getDetails().equalsIgnoreCase(READONLY_FILESYSTEM_ERROR)) {
- resetRouterHealthChecksAndConnectivity(router.getId(), true,
false, "Failed to write: " + answer.getDetails());
- return Arrays.asList(FILESYSTEM_WRITABLE_TEST);
+ resetRouterHealthChecksAndConnectivity(router.getId(),
RouterHealthStatus.SUCCESS, RouterHealthStatus.FAILED, "Failed to write: " +
answer.getDetails());
+ return List.of(FILESYSTEM_WRITABLE_TEST);
} else {
- resetRouterHealthChecksAndConnectivity(router.getId(), false,
false, "Failed to fetch results with details: " + answer.getDetails());
- return Arrays.asList(CONNECTIVITY_TEST);
+ resetRouterHealthChecksAndConnectivity(router.getId(),
RouterHealthStatus.FAILED, RouterHealthStatus.UNKNOWN, "Failed to fetch results
with details: " + answer.getDetails());
+ return List.of(CONNECTIVITY_TEST);
}
} else {
- resetRouterHealthChecksAndConnectivity(router.getId(), true, true,
"Successfully fetched data");
+ resetRouterHealthChecksAndConnectivity(router.getId(),
RouterHealthStatus.SUCCESS, RouterHealthStatus.SUCCESS, "Successfully fetched
data");
updateDbHealthChecksFromRouterResponse(router,
answer.getMonitoringResults());
return answer.getFailingChecks();
}
}
private void handleFailingChecks(DomainRouterVO router, List<String>
failingChecks) {
- if (failingChecks == null || failingChecks.size() == 0) {
+ if (failingChecks == null || failingChecks.isEmpty()) {
Review Comment:
CollectionUtils.isEmpty ?
##########
engine/components-api/src/main/java/com/cloud/configuration/ConfigurationManager.java:
##########
@@ -70,10 +70,19 @@ public interface ConfigurationManager {
"0.5",
"Weight for CPU (as a value between 0 and 1) applied to compute
capacity for Pods, Clusters and Hosts for COMBINED capacityType for ordering.
Weight for RAM will be (1 - weight of CPU)",
true, ConfigKey.Scope.Global);
+ ConfigKey<Integer> NETWORK_LB_HAPROXY_MAX_CONN = new ConfigKey<>(
Review Comment:
currently most network-related configurations are defined in
NetworkOrchestrator
##########
server/src/main/java/com/cloud/network/router/VirtualNetworkApplianceManagerImpl.java:
##########
@@ -1673,19 +1624,15 @@ private boolean
updateRouterHealthChecksConfig(DomainRouterVO router) {
}
private String getSystemThresholdsHealthChecksData(final DomainRouterVO
router) {
- return new StringBuilder()
- .append("minDiskNeeded=" +
RouterHealthChecksFreeDiskSpaceThreshold.valueIn(router.getDataCenterId()))
- .append(",maxCpuUsage=" +
RouterHealthChecksMaxCpuUsageThreshold.valueIn(router.getDataCenterId()))
- .append(",maxMemoryUsage=" +
RouterHealthChecksMaxMemoryUsageThreshold.valueIn(router.getDataCenterId()) +
";")
- .toString();
+ return "minDiskNeeded=" +
RouterHealthChecksFreeDiskSpaceThreshold.valueIn(router.getDataCenterId()) +
+ ",maxCpuUsage=" +
RouterHealthChecksMaxCpuUsageThreshold.valueIn(router.getDataCenterId()) +
+ ",maxMemoryUsage=" +
RouterHealthChecksMaxMemoryUsageThreshold.valueIn(router.getDataCenterId()) +
";";
}
private String getRouterVersionHealthChecksData(final DomainRouterVO
router) {
if (router.getTemplateVersion() != null && router.getScriptsVersion()
!= null) {
- StringBuilder routerVersion = new StringBuilder()
- .append("templateVersion=" + router.getTemplateVersion())
- .append(",scriptsVersion=" + router.getScriptsVersion());
- return routerVersion.toString();
+ return "templateVersion=" + router.getTemplateVersion() +
Review Comment:
String.format ?
##########
server/src/main/java/com/cloud/network/router/VirtualNetworkApplianceManagerImpl.java:
##########
@@ -1088,7 +1041,7 @@ protected void runInContext() {
continue;
}
- DomainRouterVO router = router0;
+ DomainRouterVO router;
Review Comment:
can this line and next 5 lines be simplified to 1 line ?
##########
server/src/main/java/com/cloud/network/router/CommandSetupHelper.java:
##########
@@ -542,11 +543,11 @@ public void createApplyIpv6FirewallRulesCommands(final
List<? extends FirewallRu
}
public void createFirewallRulesCommands(final List<? extends FirewallRule>
rules, final VirtualRouter router, final Commands cmds, final long
guestNetworkId) {
- final List<FirewallRuleTO> rulesTO = new ArrayList<FirewallRuleTO>();
+ final List<FirewallRuleTO> rulesTO = new ArrayList<>();
String systemRule = null;
Boolean defaultEgressPolicy = false;
if (rules != null) {
- if (rules.size() > 0) {
+ if (!rules.isEmpty()) {
Review Comment:
use CollectionUtils ?
it looks like `if (rules != null) {` and `if (!rules.isEmpty()) {` can be
merged into 1 check
--
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: commits-unsubscr...@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
