Copilot commented on code in PR #88:
URL:
https://github.com/apache/cloudstack-kubernetes-provider/pull/88#discussion_r4044479142
##########
cloudstack_loadbalancer.go:
##########
@@ -794,6 +921,38 @@ func (lb *loadBalancer) createLoadBalancerRule(lbRuleName
string, port corev1.Se
return lbRule, nil
}
+// createConfiguredLoadBalancerRule creates a load balancer rule together with
+// its stickiness policy and host assignments. A failure after the rule exists
+// removes the rule again: a later sync would otherwise find it, take the
+// existing-rule path, and never assign its hosts.
+func (lb *loadBalancer) createConfiguredLoadBalancerRule(lbRuleName string,
port corev1.ServicePort, protocol LoadBalancerProtocol, service
*corev1.Service) (*cloudstack.LoadBalancerRule, error) {
+ lbRule, err := lb.createLoadBalancerRule(lbRuleName, port, protocol,
service)
+ if err != nil {
+ return nil, err
+ }
+ if _, err := lb.createStickinessPolicy(lbRuleName, lbRule.Id, service);
err != nil {
+ return nil, lb.rollBackLoadBalancerRule(lbRule, err)
+ }
+
+ klog.V(4).Infof("Assigning hosts (%v) to load balancer rule: %v",
lb.hostIDs, lbRuleName)
+ if err := lb.assignHostsToRule(lbRule, lb.hostIDs); err != nil {
+ return nil, lb.rollBackLoadBalancerRule(lbRule, err)
+ }
+
+ return lbRule, nil
+}
+
+// rollBackLoadBalancerRule deletes a rule whose configuration failed part way
+// through and returns the original failure, extended with the cleanup error
+// when the rule could not be removed either.
+func (lb *loadBalancer) rollBackLoadBalancerRule(lbRule
*cloudstack.LoadBalancerRule, cause error) error {
+ klog.V(4).Infof("Rolling back load balancer rule %v after a
configuration failure: %v", lbRule.Name, cause)
+ if err := lb.deleteLoadBalancerRule(lbRule); err != nil {
+ return fmt.Errorf("%v (rolling back the rule failed too: %v)",
cause, err)
Review Comment:
If `DeleteLoadBalancerRule` fails here, the partially configured rule
remains in CloudStack, but the next `EnsureLoadBalancer` retry adopts it
through the existing-rule path. That path can create/reconcile the stickiness
policy without calling `assignHostsToRule`, so a policy-creation or
host-assignment failure followed by a rollback failure can leave a permanent
hostless rule. Recover this state on retry (for example, assign the current
hosts for newly adopted/partial rules) instead of relying only on the failed
rollback.
--
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]
