This is an automated email from the ASF dual-hosted git repository. vishesh pushed a commit to branch fix-loadbalancer-ip in repository https://gitbox.apache.org/repos/asf/cloudstack-kubernetes-provider.git
commit ecdc6bfc1150bc06e845ef9044fc52dad7fbee13 Author: vishesh92 <[email protected]> AuthorDate: Tue Nov 25 13:25:03 2025 +0100 Associate loadBalancerIP to the network when specified in the Spec --- cloudstack.go | 56 +++++++++++++++++++++++++++-- cloudstack_loadbalancer.go | 90 +++++++++++++++++++++++++++++++++++++++------- 2 files changed, 130 insertions(+), 16 deletions(-) diff --git a/cloudstack.go b/cloudstack.go index 8d78a861..9cf845a1 100644 --- a/cloudstack.go +++ b/cloudstack.go @@ -28,6 +28,9 @@ import ( "github.com/apache/cloudstack-go/v2/cloudstack" "gopkg.in/gcfg.v1" + corev1 "k8s.io/api/core/v1" + apierrors "k8s.io/apimachinery/pkg/api/errors" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" cloudprovider "k8s.io/cloud-provider" "k8s.io/klog/v2" @@ -50,9 +53,10 @@ type CSConfig struct { // CSCloud is an implementation of Interface for CloudStack. type CSCloud struct { - client *cloudstack.CloudStackClient - projectID string // If non-"", all resources will be created within this project - zone string + client *cloudstack.CloudStackClient + projectID string // If non-"", all resources will be created within this project + zone string + clientBuilder cloudprovider.ControllerClientBuilder } func init() { @@ -100,6 +104,7 @@ func newCSCloud(cfg *CSConfig) (*CSCloud, error) { // Initialize passes a Kubernetes clientBuilder interface to the cloud provider func (cs *CSCloud) Initialize(clientBuilder cloudprovider.ControllerClientBuilder, stop <-chan struct{}) { + cs.clientBuilder = clientBuilder } // LoadBalancer returns an implementation of LoadBalancer for CloudStack. @@ -238,3 +243,48 @@ func (cs *CSCloud) GetZoneByNodeName(ctx context.Context, nodeName types.NodeNam return zone, nil } + +// setServiceAnnotation updates a service annotation using the Kubernetes client. +func (cs *CSCloud) setServiceAnnotation(ctx context.Context, service *corev1.Service, key, value string) error { + if cs.clientBuilder == nil { + klog.V(4).Infof("Client builder not available, skipping annotation update for service %s/%s", service.Namespace, service.Name) + return nil + } + + client, err := cs.clientBuilder.Client("cloud-controller-manager") + if err != nil { + return fmt.Errorf("failed to get Kubernetes client: %v", err) + } + + // Get the current service + svc, err := client.CoreV1().Services(service.Namespace).Get(ctx, service.Name, metav1.GetOptions{}) + if err != nil { + if apierrors.IsNotFound(err) { + klog.V(4).Infof("Service %s/%s not found, skipping annotation update", service.Namespace, service.Name) + return nil + } + return fmt.Errorf("failed to get service: %v", err) + } + + // Check if annotation already has the correct value + if svc.Annotations != nil { + if existingValue, exists := svc.Annotations[key]; exists && existingValue == value { + klog.V(4).Infof("Annotation %s already set to %s for service %s/%s", key, value, service.Namespace, service.Name) + return nil + } + } + + // Update the annotation + if svc.Annotations == nil { + svc.Annotations = make(map[string]string) + } + svc.Annotations[key] = value + + _, err = client.CoreV1().Services(service.Namespace).Update(ctx, svc, metav1.UpdateOptions{}) + if err != nil { + return fmt.Errorf("failed to update service annotation: %v", err) + } + + klog.V(4).Infof("Successfully set annotation %s=%s on service %s/%s", key, value, service.Namespace, service.Name) + return nil +} diff --git a/cloudstack_loadbalancer.go b/cloudstack_loadbalancer.go index 7a3fd6b0..1db18474 100644 --- a/cloudstack_loadbalancer.go +++ b/cloudstack_loadbalancer.go @@ -44,19 +44,25 @@ const ( ServiceAnnotationLoadBalancerProxyProtocol = "service.beta.kubernetes.io/cloudstack-load-balancer-proxy-protocol" ServiceAnnotationLoadBalancerLoadbalancerHostname = "service.beta.kubernetes.io/cloudstack-load-balancer-hostname" + + // ServiceAnnotationLoadBalancerIPAssociatedByController indicates that the controller + // associated the IP address. This annotation is set by the controller when it associates + // an unallocated IP, and is used to determine if the IP should be disassociated on deletion. + ServiceAnnotationLoadBalancerIPAssociatedByController = "service.beta.kubernetes.io/cloudstack-load-balancer-ip-associated-by-controller" ) type loadBalancer struct { *cloudstack.CloudStackClient - name string - algorithm string - hostIDs []string - ipAddr string - ipAddrID string - networkID string - projectID string - rules map[string]*cloudstack.LoadBalancerRule + name string + algorithm string + hostIDs []string + ipAddr string + ipAddrID string + networkID string + projectID string + rules map[string]*cloudstack.LoadBalancerRule + ipAssociatedByController bool } // GetLoadBalancer returns whether the specified load balancer exists, and if so, what its status is. @@ -127,6 +133,14 @@ func (cs *CSCloud) EnsureLoadBalancer(ctx context.Context, clusterName string, s } }(lb) } + + // If the controller associated the IP, set the annotation to persist this information. + if lb.ipAssociatedByController && lb.ipAddr == service.Spec.LoadBalancerIP { + if err := cs.setServiceAnnotation(ctx, service, ServiceAnnotationLoadBalancerIPAssociatedByController, "true"); err != nil { + // Log the error but don't fail - the annotation is helpful but not critical + klog.Warningf("Failed to set annotation on service %s/%s: %v", service.Namespace, service.Name, err) + } + } } klog.V(4).Infof("Load balancer %v is associated with IP %v", lb.name, lb.ipAddr) @@ -353,10 +367,50 @@ func (cs *CSCloud) EnsureLoadBalancerDeleted(ctx context.Context, clusterName st } } - if lb.ipAddr != "" && lb.ipAddr != service.Spec.LoadBalancerIP { - klog.V(4).Infof("Releasing load balancer IP: %v", lb.ipAddr) - if err := lb.releaseLoadBalancerIP(); err != nil { - return err + if lb.ipAddr != "" { + // If the IP was allocated by the controller (not specified in service spec), release it. + if lb.ipAddr != service.Spec.LoadBalancerIP { + klog.V(4).Infof("Releasing load balancer IP: %v", lb.ipAddr) + if err := lb.releaseLoadBalancerIP(); err != nil { + return err + } + } else { + // If the IP was specified in service spec, check if it was associated by the controller. + // First, check if there's an annotation indicating the controller associated it. + // If not, check if there are any other load balancer rules using this IP. + shouldDisassociate := getBoolFromServiceAnnotation(service, ServiceAnnotationLoadBalancerIPAssociatedByController, false) + + if shouldDisassociate { + // Annotation is set, so check if there are any other load balancer rules using this IP. + // Since we've already deleted all rules for this service, any remaining rules must belong + // to other services. If no other rules exist, it's safe to disassociate the IP. + ip, count, err := lb.Address.GetPublicIpAddressByID(lb.ipAddrID) + if err != nil { + klog.Errorf("Error retrieving IP address %v for disassociation check: %v", lb.ipAddr, err) + } else if count > 0 && ip.Allocated != "" { + p := lb.LoadBalancer.NewListLoadBalancerRulesParams() + p.SetPublicipid(lb.ipAddrID) + p.SetListall(true) + if lb.projectID != "" { + p.SetProjectid(lb.projectID) + } + otherRules, err := lb.LoadBalancer.ListLoadBalancerRules(p) + if err != nil { + klog.Errorf("Error checking for other load balancer rules using IP %v: %v", lb.ipAddr, err) + } else if otherRules.Count > 0 { + // Other load balancer rules are using this IP (other services are using it), + // so don't disassociate. + shouldDisassociate = false + } + } + } + + if shouldDisassociate { + klog.V(4).Infof("Disassociating IP %v that was associated by the controller", lb.ipAddr) + if err := lb.releaseLoadBalancerIP(); err != nil { + return err + } + } } } @@ -491,6 +545,7 @@ func (lb *loadBalancer) getPublicIPAddress(loadBalancerIP string) error { p := lb.Address.NewListPublicIpAddressesParams() p.SetIpaddress(loadBalancerIP) + p.SetAllocatedonly(false) p.SetListall(true) if lb.projectID != "" { @@ -503,12 +558,16 @@ func (lb *loadBalancer) getPublicIPAddress(loadBalancerIP string) error { } if l.Count != 1 { - return fmt.Errorf("could not find IP address %v", loadBalancerIP) + return fmt.Errorf("could not find IP address %v. Found %d addresses", loadBalancerIP, l.Count) } lb.ipAddr = l.PublicIpAddresses[0].Ipaddress lb.ipAddrID = l.PublicIpAddresses[0].Id + // If the IP is not allocated, associate it. + if l.PublicIpAddresses[0].Allocated == "" { + return lb.associatePublicIPAddress() + } return nil } @@ -537,6 +596,10 @@ func (lb *loadBalancer) associatePublicIPAddress() error { p.SetProjectid(lb.projectID) } + if lb.ipAddr != "" { + p.SetIpaddress(lb.ipAddr) + } + // Associate a new IP address r, err := lb.Address.AssociateIpAddress(p) if err != nil { @@ -545,6 +608,7 @@ func (lb *loadBalancer) associatePublicIPAddress() error { lb.ipAddr = r.Ipaddress lb.ipAddrID = r.Id + lb.ipAssociatedByController = true return nil }
