Skip to content

Commit 2900708

Browse files
committed
Associate loadBalancerIP to the network when specified in the Spec
1 parent 90adbee commit 2900708

File tree

2 files changed

+130
-16
lines changed

2 files changed

+130
-16
lines changed

cloudstack.go

Lines changed: 53 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,9 @@ import (
2828

2929
"github.com/apache/cloudstack-go/v2/cloudstack"
3030
"gopkg.in/gcfg.v1"
31+
corev1 "k8s.io/api/core/v1"
32+
apierrors "k8s.io/apimachinery/pkg/api/errors"
33+
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
3134
"k8s.io/apimachinery/pkg/types"
3235
cloudprovider "k8s.io/cloud-provider"
3336
"k8s.io/klog/v2"
@@ -50,9 +53,10 @@ type CSConfig struct {
5053

5154
// CSCloud is an implementation of Interface for CloudStack.
5255
type CSCloud struct {
53-
client *cloudstack.CloudStackClient
54-
projectID string // If non-"", all resources will be created within this project
55-
zone string
56+
client *cloudstack.CloudStackClient
57+
projectID string // If non-"", all resources will be created within this project
58+
zone string
59+
clientBuilder cloudprovider.ControllerClientBuilder
5660
}
5761

5862
func init() {
@@ -100,6 +104,7 @@ func newCSCloud(cfg *CSConfig) (*CSCloud, error) {
100104

101105
// Initialize passes a Kubernetes clientBuilder interface to the cloud provider
102106
func (cs *CSCloud) Initialize(clientBuilder cloudprovider.ControllerClientBuilder, stop <-chan struct{}) {
107+
cs.clientBuilder = clientBuilder
103108
}
104109

105110
// LoadBalancer returns an implementation of LoadBalancer for CloudStack.
@@ -238,3 +243,48 @@ func (cs *CSCloud) GetZoneByNodeName(ctx context.Context, nodeName types.NodeNam
238243

239244
return zone, nil
240245
}
246+
247+
// setServiceAnnotation updates a service annotation using the Kubernetes client.
248+
func (cs *CSCloud) setServiceAnnotation(ctx context.Context, service *corev1.Service, key, value string) error {
249+
if cs.clientBuilder == nil {
250+
klog.V(4).Infof("Client builder not available, skipping annotation update for service %s/%s", service.Namespace, service.Name)
251+
return nil
252+
}
253+
254+
client, err := cs.clientBuilder.Client("cloud-controller-manager")
255+
if err != nil {
256+
return fmt.Errorf("failed to get Kubernetes client: %v", err)
257+
}
258+
259+
// Get the current service
260+
svc, err := client.CoreV1().Services(service.Namespace).Get(ctx, service.Name, metav1.GetOptions{})
261+
if err != nil {
262+
if apierrors.IsNotFound(err) {
263+
klog.V(4).Infof("Service %s/%s not found, skipping annotation update", service.Namespace, service.Name)
264+
return nil
265+
}
266+
return fmt.Errorf("failed to get service: %v", err)
267+
}
268+
269+
// Check if annotation already has the correct value
270+
if svc.Annotations != nil {
271+
if existingValue, exists := svc.Annotations[key]; exists && existingValue == value {
272+
klog.V(4).Infof("Annotation %s already set to %s for service %s/%s", key, value, service.Namespace, service.Name)
273+
return nil
274+
}
275+
}
276+
277+
// Update the annotation
278+
if svc.Annotations == nil {
279+
svc.Annotations = make(map[string]string)
280+
}
281+
svc.Annotations[key] = value
282+
283+
_, err = client.CoreV1().Services(service.Namespace).Update(ctx, svc, metav1.UpdateOptions{})
284+
if err != nil {
285+
return fmt.Errorf("failed to update service annotation: %v", err)
286+
}
287+
288+
klog.V(4).Infof("Successfully set annotation %s=%s on service %s/%s", key, value, service.Namespace, service.Name)
289+
return nil
290+
}

cloudstack_loadbalancer.go

Lines changed: 77 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -44,19 +44,25 @@ const (
4444
ServiceAnnotationLoadBalancerProxyProtocol = "service.beta.kubernetes.io/cloudstack-load-balancer-proxy-protocol"
4545

4646
ServiceAnnotationLoadBalancerLoadbalancerHostname = "service.beta.kubernetes.io/cloudstack-load-balancer-hostname"
47+
48+
// ServiceAnnotationLoadBalancerIPAssociatedByController indicates that the controller
49+
// associated the IP address. This annotation is set by the controller when it associates
50+
// an unallocated IP, and is used to determine if the IP should be disassociated on deletion.
51+
ServiceAnnotationLoadBalancerIPAssociatedByController = "service.beta.kubernetes.io/cloudstack-load-balancer-ip-associated-by-controller"
4752
)
4853

4954
type loadBalancer struct {
5055
*cloudstack.CloudStackClient
5156

52-
name string
53-
algorithm string
54-
hostIDs []string
55-
ipAddr string
56-
ipAddrID string
57-
networkID string
58-
projectID string
59-
rules map[string]*cloudstack.LoadBalancerRule
57+
name string
58+
algorithm string
59+
hostIDs []string
60+
ipAddr string
61+
ipAddrID string
62+
networkID string
63+
projectID string
64+
rules map[string]*cloudstack.LoadBalancerRule
65+
ipAssociatedByController bool
6066
}
6167

6268
// 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
127133
}
128134
}(lb)
129135
}
136+
137+
// If the controller associated the IP, set the annotation to persist this information.
138+
if lb.ipAssociatedByController && lb.ipAddr == service.Spec.LoadBalancerIP {
139+
if err := cs.setServiceAnnotation(ctx, service, ServiceAnnotationLoadBalancerIPAssociatedByController, "true"); err != nil {
140+
// Log the error but don't fail - the annotation is helpful but not critical
141+
klog.Warningf("Failed to set annotation on service %s/%s: %v", service.Namespace, service.Name, err)
142+
}
143+
}
130144
}
131145

132146
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
353367
}
354368
}
355369

356-
if lb.ipAddr != "" && lb.ipAddr != service.Spec.LoadBalancerIP {
357-
klog.V(4).Infof("Releasing load balancer IP: %v", lb.ipAddr)
358-
if err := lb.releaseLoadBalancerIP(); err != nil {
359-
return err
370+
if lb.ipAddr != "" {
371+
// If the IP was allocated by the controller (not specified in service spec), release it.
372+
if lb.ipAddr != service.Spec.LoadBalancerIP {
373+
klog.V(4).Infof("Releasing load balancer IP: %v", lb.ipAddr)
374+
if err := lb.releaseLoadBalancerIP(); err != nil {
375+
return err
376+
}
377+
} else {
378+
// If the IP was specified in service spec, check if it was associated by the controller.
379+
// First, check if there's an annotation indicating the controller associated it.
380+
// If not, check if there are any other load balancer rules using this IP.
381+
shouldDisassociate := getBoolFromServiceAnnotation(service, ServiceAnnotationLoadBalancerIPAssociatedByController, false)
382+
383+
if shouldDisassociate {
384+
// Annotation is set, so check if there are any other load balancer rules using this IP.
385+
// Since we've already deleted all rules for this service, any remaining rules must belong
386+
// to other services. If no other rules exist, it's safe to disassociate the IP.
387+
ip, count, err := lb.Address.GetPublicIpAddressByID(lb.ipAddrID)
388+
if err != nil {
389+
klog.Errorf("Error retrieving IP address %v for disassociation check: %v", lb.ipAddr, err)
390+
} else if count > 0 && ip.Allocated != "" {
391+
p := lb.LoadBalancer.NewListLoadBalancerRulesParams()
392+
p.SetPublicipid(lb.ipAddrID)
393+
p.SetListall(true)
394+
if lb.projectID != "" {
395+
p.SetProjectid(lb.projectID)
396+
}
397+
otherRules, err := lb.LoadBalancer.ListLoadBalancerRules(p)
398+
if err != nil {
399+
klog.Errorf("Error checking for other load balancer rules using IP %v: %v", lb.ipAddr, err)
400+
} else if otherRules.Count > 0 {
401+
// Other load balancer rules are using this IP (other services are using it),
402+
// so don't disassociate.
403+
shouldDisassociate = false
404+
}
405+
}
406+
}
407+
408+
if shouldDisassociate {
409+
klog.V(4).Infof("Disassociating IP %v that was associated by the controller", lb.ipAddr)
410+
if err := lb.releaseLoadBalancerIP(); err != nil {
411+
return err
412+
}
413+
}
360414
}
361415
}
362416

@@ -491,6 +545,7 @@ func (lb *loadBalancer) getPublicIPAddress(loadBalancerIP string) error {
491545

492546
p := lb.Address.NewListPublicIpAddressesParams()
493547
p.SetIpaddress(loadBalancerIP)
548+
p.SetAllocatedonly(false)
494549
p.SetListall(true)
495550

496551
if lb.projectID != "" {
@@ -503,12 +558,16 @@ func (lb *loadBalancer) getPublicIPAddress(loadBalancerIP string) error {
503558
}
504559

505560
if l.Count != 1 {
506-
return fmt.Errorf("could not find IP address %v", loadBalancerIP)
561+
return fmt.Errorf("could not find IP address %v. Found %d addresses", loadBalancerIP, l.Count)
507562
}
508563

509564
lb.ipAddr = l.PublicIpAddresses[0].Ipaddress
510565
lb.ipAddrID = l.PublicIpAddresses[0].Id
511566

567+
// If the IP is not allocated, associate it.
568+
if l.PublicIpAddresses[0].Allocated == "" {
569+
return lb.associatePublicIPAddress()
570+
}
512571
return nil
513572
}
514573

@@ -537,6 +596,10 @@ func (lb *loadBalancer) associatePublicIPAddress() error {
537596
p.SetProjectid(lb.projectID)
538597
}
539598

599+
if lb.ipAddr != "" {
600+
p.SetIpaddress(lb.ipAddr)
601+
}
602+
540603
// Associate a new IP address
541604
r, err := lb.Address.AssociateIpAddress(p)
542605
if err != nil {
@@ -545,6 +608,7 @@ func (lb *loadBalancer) associatePublicIPAddress() error {
545608

546609
lb.ipAddr = r.Ipaddress
547610
lb.ipAddrID = r.Id
611+
lb.ipAssociatedByController = true
548612

549613
return nil
550614
}

0 commit comments

Comments
 (0)