Damans227 commented on code in PR #14131:
URL: https://github.com/apache/cloudstack/pull/14131#discussion_r4199313582
##########
server/src/main/java/com/cloud/network/lb/LoadBalancingRulesManagerImpl.java:
##########
@@ -2285,6 +2289,42 @@ public List<LbDestination> getExistingDestinations(long
lbId) {
return dstList;
}
+ /**
+ * Haproxy rejects a negative timeout, and a rejected file leaves every
rule on the router
+ * running its previous config. Refuse the value here rather than let it
reach the VR.
+ */
+ protected void validateConnectionTimeout(String name, Long value) {
+ if (value != null && value < 0) {
+ throw new InvalidParameterValueException(String.format("%s must be
0 or greater, got [%s]. 0 means no timeout.", name, value));
+ }
+ }
+
+ @Override
+ public boolean updateLoadBalancerConnectionSettings(long lbRuleId, Boolean
keepAlive, Long idleTimeout, Long keepAliveTimeout) {
+ validateConnectionTimeout(ApiConstants.IDLE_TIMEOUT, idleTimeout);
+ validateConnectionTimeout(ApiConstants.KEEPALIVE_TIMEOUT,
keepAliveTimeout);
+
+ boolean changed = storeDetail(lbRuleId, LoadBalancer.KEEPALIVE,
keepAlive == null ? null : keepAlive.toString());
+ changed |= storeDetail(lbRuleId, LoadBalancer.IDLE_TIMEOUT,
idleTimeout == null ? null : idleTimeout.toString());
+ changed |= storeDetail(lbRuleId, LoadBalancer.KEEPALIVE_TIMEOUT,
keepAliveTimeout == null ? null : keepAliveTimeout.toString());
+ return changed;
+ }
+
+ private boolean storeDetail(long lbRuleId, String key, String value) {
+ if (value == null) {
Review Comment:
once a rule has its own timeout, how does someone go back to the default?
clearing the field in the ui looks like it just keeps the old value
##########
server/src/main/java/com/cloud/network/lb/LoadBalancingRulesManagerImpl.java:
##########
@@ -2342,6 +2382,9 @@ public LoadBalancer
updateLoadBalancerRule(UpdateLoadBalancerRuleCmd cmd) {
lb.setCidrList(cidrListStr);
}
+ // lb.getId() rather than the id off the command, which is a Long and
unboxes badly
+ boolean settingsChanged =
updateLoadBalancerConnectionSettings(lb.getId(), cmd.getKeepAlive(),
cmd.getIdleTimeout(), cmd.getKeepAliveTimeout());
Review Comment:
if pushing the config to the router fails, should these settings be rolled
back too? looks like only the name, algorithm and cidrs get restored
--
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]
