Merge pull request #168 from luthermonson/check-move

always return lbnotfound from getNodeBalancerForService

Author: tchinmai7

cloud/linode/loadbalancers.go

@@ -70,17 +70,7 @@ func (l *loadbalancers) getNodeBalancerForService(ctx context.Context, service *
 
 	if hasIDAnn {
 		sentry.SetTag(ctx, "load_balancer_id", rawID)
-		nb, err := l.getNodeBalancerByID(ctx, service, id)
-		switch err.(type) {
-		case nil:
-			return nb, nil
-
-		case lbNotFoundError:
-			return nil, fmt.Errorf("%s annotation points to a NodeBalancer that does not exist: %s", annLinodeNodeBalancerID, err)
-
-		default:
-			return nil, err
-		}
+		return l.getNodeBalancerByID(ctx, service, id)
 	}
 	return l.getNodeBalancerByStatus(ctx, service)
 }
@@ -199,6 +189,13 @@ func (l *loadbalancers) EnsureLoadBalancer(ctx context.Context, clusterName stri
 	nb, err = l.getNodeBalancerForService(ctx, service)
 	switch err.(type) {
 	case lbNotFoundError:
+		if service.GetAnnotations()[annLinodeNodeBalancerID] != "" {
+			// a load balancer annotation has been created so a NodeBalancer is coming, error out and retry later
+			klog.Infof("NodeBalancer created but not available yet, waiting...")
+			sentry.CaptureError(ctx, err)
+			return nil, err
+		}
+
 		if nb, err = l.buildLoadBalancerRequest(ctx, clusterName, service, nodes); err != nil {
 			sentry.CaptureError(ctx, err)
 			return nil, err

cloud/linode/loadbalancers_test.go

@@ -2123,10 +2123,12 @@ func testGetNodeBalancerForServiceIDDoesNotExist(t *testing.T, client *linodego.
 		t.Fatal("expected getNodeBalancerForService to return an error")
 	}
 
-	expectedErr := fmt.Sprintf("%s annotation points to a NodeBalancer that does not exist: LoadBalancer (%s) not found for service (%s)",
-		annLinodeNodeBalancerID, bogusNodeBalancerID, getServiceNn(svc))
-
-	if err.Error() != expectedErr {
+	nbid, _ := strconv.Atoi(bogusNodeBalancerID)
+	expectedErr := lbNotFoundError{
+		serviceNn:      getServiceNn(svc),
+		nodeBalancerID: nbid,
+	}
+	if err.Error() != expectedErr.Error() {
 		t.Errorf("expected error to be '%s' but got '%s'", expectedErr, err)
 	}
 }

e2e/test/ccm_e2e_test.go

@@ -6,6 +6,9 @@ import (
 	"fmt"
 	"os/exec"
 	"strconv"
+	"time"
+
+	"k8s.io/apimachinery/pkg/api/errors"
 
 	"github.com/linode/linodego"
 	. "github.com/onsi/ginkgo/v2"
@@ -90,6 +93,14 @@ var _ = Describe("e2e tests", func() {
 		Eventually(watcher.ResultChan()).Should(Receive(EnsuredService()))
 	}
 
+	ensureServiceWasDeleted := func() {
+		err := func() error {
+			_, err := f.LoadBalancer.GetService()
+			return err
+		}
+		Eventually(err).WithTimeout(10 * time.Second).Should(MatchError(errors.IsNotFound, "IsNotFound"))
+	}
+
 	createServiceWithSelector := func(selector map[string]string, ports []core.ServicePort, isSessionAffinityClientIP bool) {
 		Expect(f.LoadBalancer.CreateService(selector, nil, ports, isSessionAffinityClientIP)).NotTo(HaveOccurred())
 		Eventually(f.LoadBalancer.GetServiceEndpoints).Should(Not(BeEmpty()))
@@ -942,7 +953,7 @@ var _ = Describe("e2e tests", func() {
 					By("Creating new NodeBalancer")
 					nbID := createNodeBalancer()
 
-					By("Waiting for currenct NodeBalancer to be ready")
+					By("Waiting for current NodeBalancer to be ready")
 					checkNodeBalancerID(framework.TestServerResourceName, nodeBalancerID)
 
 					By("Annotating service with new NodeBalancer ID")
@@ -957,6 +968,97 @@ var _ = Describe("e2e tests", func() {
 				})
 			})
 
+			Context("Deleted Service when NodeBalancer not present", func() {
+				var (
+					pods         []string
+					labels       map[string]string
+					annotations  map[string]string
+					servicePorts []core.ServicePort
+
+					nodeBalancerID int
+				)
+
+				BeforeEach(func() {
+					pods = []string{"test-pod-1"}
+					ports := []core.ContainerPort{
+						{
+							Name:          "http-1",
+							ContainerPort: 8080,
+						},
+					}
+					servicePorts = []core.ServicePort{
+						{
+							Name:       "http-1",
+							Port:       80,
+							TargetPort: intstr.FromInt(8080),
+							Protocol:   "TCP",
+						},
+					}
+
+					labels = map[string]string{
+						"app": "test-loadbalancer-with-nodebalancer-id",
+					}
+
+					By("Creating NodeBalancer")
+					nodeBalancerID = createNodeBalancer()
+
+					annotations = map[string]string{
+						annLinodeNodeBalancerID: strconv.Itoa(nodeBalancerID),
+					}
+
+					By("Creating Pod")
+					createPodWithLabel(pods, ports, framework.TestServerImage, labels, false)
+
+					By("Creating Service")
+					createServiceWithAnnotations(labels, annotations, servicePorts, false)
+				})
+
+				AfterEach(func() {
+					By("Deleting the Pods")
+					deletePods(pods)
+
+					err := root.Recycle()
+					Expect(err).NotTo(HaveOccurred())
+				})
+
+				It("should use the specified NodeBalancer", func() {
+					By("Checking the NodeBalancerID")
+					checkNodeBalancerID(framework.TestServerResourceName, nodeBalancerID)
+				})
+
+				It("should use the newly specified NodeBalancer ID", func() {
+					By("Creating new NodeBalancer")
+					nbID := createNodeBalancer()
+
+					By("Waiting for current NodeBalancer to be ready")
+					checkNodeBalancerID(framework.TestServerResourceName, nodeBalancerID)
+
+					By("Annotating service with new NodeBalancer ID")
+					annotations[annLinodeNodeBalancerID] = strconv.Itoa(nbID)
+					updateServiceWithAnnotations(labels, annotations, servicePorts, false)
+
+					By("Checking the NodeBalancer ID")
+					checkNodeBalancerID(framework.TestServerResourceName, nbID)
+
+					By("Checking old NodeBalancer was deleted")
+					checkNodeBalancerNotExists(nodeBalancerID)
+				})
+
+				It("should delete the service with no NodeBalancer present", func() {
+					By("Deleting the NodeBalancer")
+					deleteNodeBalancer(nodeBalancerID)
+
+					By("Checking old NodeBalancer was deleted")
+					checkNodeBalancerNotExists(nodeBalancerID)
+
+					By("Deleting the Service")
+					deleteService()
+
+					By("Checking if the service was deleted")
+					ensureServiceWasDeleted()
+				})
+			})
+
 			Context("With Preserve Annotation", func() {
 				var (
 					pods           []string

e2e/test/framework/service.go

@@ -69,6 +69,10 @@ func (i *lbInvocation) GetServiceWatcher() (watch.Interface, error) {
 	return watcher, nil
 }
 
+func (i *lbInvocation) GetService() (*core.Service, error) {
+	return i.kubeClient.CoreV1().Services(i.Namespace()).Get(context.TODO(), TestServerResourceName, metav1.GetOptions{})
+}
+
 func (i *lbInvocation) CreateService(selector, annotations map[string]string, ports []core.ServicePort, isSessionAffinityClientIP bool) error {
 	return i.createOrUpdateService(selector, annotations, ports, isSessionAffinityClientIP, true)
 }