vishesh92 commented on code in PR #104:
URL:
https://github.com/apache/cloudstack-kubernetes-provider/pull/104#discussion_r4080999092
##########
cloudstack_loadbalancer.go:
##########
@@ -157,96 +174,33 @@ func (cs *CSCloud) EnsureLoadBalancer(ctx
context.Context, clusterName string, s
klog.V(4).Infof("Load balancer %v is associated with IP %v", lb.name,
lb.ipAddr)
- for _, port := range service.Spec.Ports {
- // Construct the protocol name first, we need it a few times
- protocol := ProtocolFromServicePort(port, service)
- if protocol == LoadBalancerProtocolInvalid {
- return nil, fmt.Errorf("unsupported load balancer
protocol: %v", port.Protocol)
- }
-
- // All ports have their own load balancer rule, so add the port
to lbName to keep the names unique.
- lbRuleName := fmt.Sprintf("%s-%s-%d", lb.name, protocol,
port.Port)
-
- // If the load balancer rule exists and is up-to-date, we move
on to the next rule.
- lbRule, needsUpdate, err :=
lb.checkLoadBalancerRule(lbRuleName, port, protocol, service, cs.version)
- if err != nil {
- return nil, err
- }
-
- if lbRule != nil {
- if needsUpdate {
- klog.V(4).Infof("Updating load balancer rule:
%v", lbRuleName)
- if err := lb.updateLoadBalancerRule(lbRuleName,
protocol, service, cs.version); err != nil {
- return nil, err
- }
- // Delete the rule from the map, to prevent it
being deleted.
- delete(lb.rules, lbRuleName)
- } else {
- klog.V(4).Infof("Load balancer rule %v is
up-to-date", lbRuleName)
- // Delete the rule from the map, to prevent it
being deleted.
- delete(lb.rules, lbRuleName)
- }
- } else {
- klog.V(4).Infof("Creating load balancer rule: %v",
lbRuleName)
- lbRule, err = lb.createLoadBalancerRule(lbRuleName,
port, protocol, service)
- if err != nil {
- return nil, 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, err
- }
- }
-
- network, count, err := lb.Network.GetNetworkByID(lb.networkID,
cloudstack.WithProject(lb.projectID))
- if err != nil {
- if count == 0 {
- return nil, err
- }
- return nil, err
- }
+ // Resolve every service port to the rule that should represent it.
+ desired, err := lb.resolveLoadBalancerRules(service, cs.version)
+ if err != nil {
+ return nil, err
+ }
- if lbRule != nil {
- if isFirewallSupported(network.Service) {
- klog.V(4).Infof("Creating firewall rules for
load balancer rule: %v (%v:%v:%v)", lbRuleName, protocol, lbRule.Publicip,
port.Port)
- if _, err :=
lb.updateFirewallRule(lbRule.Publicipid, int(port.Port), protocol,
service.Spec.LoadBalancerSourceRanges); err != nil {
- return nil, err
- }
- } else if isNetworkACLSupported(network.Service) {
- klog.V(4).Infof("Creating ACL rules for load
balancer rule: %v (%v:%v:%v)", lbRuleName, protocol, lbRule.Publicip, port.Port)
- if _, err :=
lb.updateNetworkACL(int(port.Port), protocol, network.Id); err != nil {
- return nil, err
- }
- }
- }
+ network, _, err := lb.Network.GetNetworkByID(lb.networkID,
cloudstack.WithProject(lb.projectID))
+ if err != nil {
+ return nil, err
}
Review Comment:
GetNetworkByID never returns a nil network without an error: when the count
is 0 it returns "No match found", and it only returns (network, 1, nil) on an
exact match (cloudstack-go NetworkService.go), so the err check here already
covers not-found. The count check this replaced on main returned the same error
on both branches.
--
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]
