diff --git a/cloud-controller-manager/civo/loadbalancer.go b/cloud-controller-manager/civo/loadbalancer.go index 782f3ae..6d5f5ce 100644 --- a/cloud-controller-manager/civo/loadbalancer.go +++ b/cloud-controller-manager/civo/loadbalancer.go @@ -91,7 +91,7 @@ func (l *loadbalancer) EnsureLoadBalancer(ctx context.Context, clusterName strin return nil, err } - // CivLB has been found + // CivoLB has been found if err == nil { ul, err := l.updateLBConfig(civolb, service, nodes) if err != nil { @@ -146,7 +146,7 @@ func (l *loadbalancer) updateLBConfig(civolb *civogo.LoadBalancer, service *v1.S } lbuc.Backends = backends - if ip := getReservedIP(service); ip != "" { + if ip := getReservedIPFromAnnotation(service); ip != "" { rip, err := l.client.civoClient.FindIP(ip) if err != nil { klog.Errorf("Unable to find reserved IP, error: %v", err) @@ -162,12 +162,12 @@ func (l *loadbalancer) updateLBConfig(civolb *civogo.LoadBalancer, service *v1.S } } } else { - ips, err := l.client.civoClient.ListIPs() + ip, err := findIPWithLBID(l.client.civoClient, civolb.ID) if err != nil { - klog.Errorf("Unable to list IPs, error: %v", err) + klog.Errorf("Unable to find IP with loadbalancer ID, error: %v", err) return nil, err } - ip := findIPWithLBID(ips.Items, civolb.ID) + if ip != nil { _, err = l.client.civoClient.UnassignIP(ip.ID) if err != nil { @@ -187,13 +187,21 @@ func (l *loadbalancer) updateLBConfig(civolb *civogo.LoadBalancer, service *v1.S } -func findIPWithLBID(ips []civogo.IP, lbID string) *civogo.IP { - for _, ip := range ips { +// there's no direct way to find if the LB is using a reserved IP. This method lists all the reserved IPs in the account +// and checks if the loadbalancer is using one of them. +func findIPWithLBID(civo civogo.Clienter, lbID string) (*civogo.IP, error) { + ips, err := civo.ListIPs() + if err != nil { + klog.Errorf("Unable to list IPs, error: %v", err) + return nil, err + } + + for _, ip := range ips.Items { if ip.AssignedTo.ID == lbID { - return &ip + return &ip, nil } } - return nil + return nil, nil } func lbStatusFor(civolb *civogo.LoadBalancer) (*v1.LoadBalancerStatus, error) { @@ -389,30 +397,19 @@ func getEnableProxyProtocol(service *v1.Service) string { // getAlgorithm returns the algorithm value from the service annotation. func getAlgorithm(service *v1.Service) string { - algorithm, ok := service.Annotations[annotationCivoLoadBalancerAlgorithm] - if !ok { - return "" - } + algorithm, _ := service.Annotations[annotationCivoLoadBalancerAlgorithm] return algorithm } -// getReservedIP returns the reservedIP value from the service annotation. -func getReservedIP(service *v1.Service) string { - ip, ok := service.Annotations[annotationCivoIPv4] - if !ok { - return "" - } - +// getReservedIPFromAnnotation returns the reservedIP value from the service annotation. +func getReservedIPFromAnnotation(service *v1.Service) string { + ip, _ := service.Annotations[annotationCivoIPv4] return ip } // getFirewallID returns the firewallID value from the service annotation. func getFirewallID(service *v1.Service) string { - firewallID, ok := service.Annotations[annotationCivoFirewallID] - if !ok { - return "" - } - + firewallID, _ := service.Annotations[annotationCivoFirewallID] return firewallID } diff --git a/e2e/loadbalancer_test.go b/e2e/loadbalancer_test.go index 846e68f..c48df7c 100644 --- a/e2e/loadbalancer_test.go +++ b/e2e/loadbalancer_test.go @@ -109,12 +109,13 @@ func TestLoadbalancerProxy(t *testing.T) { } -func TestLoadbalancerPublicIP(t *testing.T) { +func TestLoadbalancerReservedIP(t *testing.T) { g := NewGomegaWithT(t) _, err := deployMirrorPods(e2eTest.tenantClient) g.Expect(err).ShouldNot(HaveOccurred()) + fmt.Println("Create a reserved IP for e2e test (if it doesn't exist)") ip, err := getOrCreateIP(e2eTest.civo) g.Expect(err).ShouldNot(HaveOccurred()) @@ -124,7 +125,21 @@ func TestLoadbalancerPublicIP(t *testing.T) { }, "2m", "5s").ShouldNot(BeEmpty()) fmt.Println("Creating Service") - svc, err := getOrCreateSvc(e2eTest.tenantClient, ip) + svc, err := getOrCreateSvc(e2eTest.tenantClient) + g.Expect(err).ShouldNot(HaveOccurred()) + + patchSvc := &corev1.Service{} + err = e2eTest.tenantClient.Get(context.TODO(), client.ObjectKeyFromObject(svc), patchSvc) + originalSvc := svc.DeepCopy() + if patchSvc.Annotations == nil { + patchSvc.Annotations = make(map[string]string, 0) + } + patchSvc.Annotations = map[string]string{ + "kubernetes.civo.com/ipv4-address": ip.IP, + } + + fmt.Println("Updating service with reserved IP annotation") + err = e2eTest.tenantClient.Patch(context.TODO(), patchSvc, client.MergeFrom(originalSvc)) g.Expect(err).ShouldNot(HaveOccurred()) g.Eventually(func() string { @@ -141,6 +156,7 @@ func TestLoadbalancerPublicIP(t *testing.T) { err = e2eTest.tenantClient.Update(context.TODO(), svc) g.Expect(err).ShouldNot(HaveOccurred()) + fmt.Println("Waiting for auto-assigned IP to be attached to LB") g.Eventually(func() string { err = e2eTest.tenantClient.Get(context.TODO(), client.ObjectKeyFromObject(svc), svc) if len(svc.Status.LoadBalancer.Ingress) == 0 { @@ -184,20 +200,17 @@ func getOrCreateIP(c *civogo.Client) (*civogo.IP, error) { return ip, err } -func getOrCreateSvc(c client.Client, ip *civogo.IP) (*corev1.Service, error) { +func getOrCreateSvc(c client.Client) (*corev1.Service, error) { svc := &corev1.Service{} err := c.Get(context.TODO(), client.ObjectKey{Name: "echo-pods", Namespace: "default"}, svc) if err != nil && errors.IsNotFound(err) { lbls := map[string]string{"app": "mirror-pod"} // Create a service of type: LoadBalancer - fmt.Println("Creating Service with IP: ", ip.IP) svc = &corev1.Service{ ObjectMeta: metav1.ObjectMeta{ - Name: "echo-pods", - Namespace: "default", - Annotations: map[string]string{ - "kubernetes.civo.com/ipv4-address": ip.IP, - }, + Name: "echo-pods", + Namespace: "default", + Annotations: map[string]string{}, }, Spec: corev1.ServiceSpec{ Ports: []corev1.ServicePort{