Overwrite service load-balancer ID on mismatch

This change causes the service load-balancer ID annotation to be
overwritten if it does not match the given load-balancer ID.

It is required to properly handle the case where the load-balancer is
deleted manually: in that case, CCM won't be able to look up the
load-balancer and create a new one. Without updating the ID, however,
subsequent reconciliations will continue to use the old ID, fail to find
one, try to create a new but fail because a load-balancer with the given
name (based on the service UUID) already exists. At this point, the
reconciliation attempt will loop endlessly.

We need to break the cycle by annotating the ID of the newly created
load-balancer.

Author: Timo Reimann

CHANGELOG.md

@@ -2,6 +2,10 @@
 
 ## unreleased
 
+### Changed
+
+* Overwrite service load-balancer ID on mismatch (@timoreimann)
+
 ## v0.1.18 (beta) - Aug 9th 2019
 
 ### Changed

cloud-controller-manager/do/loadbalancers.go

@@ -361,7 +361,7 @@ func (l *loadBalancers) retrieveLoadBalancer(ctx context.Context, service *v1.Se
 }
 
 func (l *loadBalancers) ensureLoadBalancerIDAnnot(service *v1.Service, lbID string) error {
-	if val := getLoadBalancerID(service); val != "" {
+	if val := getLoadBalancerID(service); val == lbID {
 		return nil
 	}
 

cloud-controller-manager/do/loadbalancers_test.go

@@ -3412,16 +3412,17 @@ func Test_GetLoadBalancer(t *testing.T) {
 
 func Test_EnsureLoadBalancer(t *testing.T) {
 	testcases := []struct {
-		name     string
-		droplets []godo.Droplet
-		getFn    func(context.Context, string) (*godo.LoadBalancer, *godo.Response, error)
-		listFn   func(context.Context, *godo.ListOptions) ([]godo.LoadBalancer, *godo.Response, error)
-		createFn func(context.Context, *godo.LoadBalancerRequest) (*godo.LoadBalancer, *godo.Response, error)
-		updateFn func(ctx context.Context, lbID string, lbr *godo.LoadBalancerRequest) (*godo.LoadBalancer, *godo.Response, error)
-		service  *v1.Service
-		nodes    []*v1.Node
-		lbStatus *v1.LoadBalancerStatus
-		err      error
+		name              string
+		droplets          []godo.Droplet
+		getFn             func(context.Context, string) (*godo.LoadBalancer, *godo.Response, error)
+		listFn            func(context.Context, *godo.ListOptions) ([]godo.LoadBalancer, *godo.Response, error)
+		createFn          func(context.Context, *godo.LoadBalancerRequest) (*godo.LoadBalancer, *godo.Response, error)
+		updateFn          func(ctx context.Context, lbID string, lbr *godo.LoadBalancerRequest) (*godo.LoadBalancer, *godo.Response, error)
+		service           *v1.Service
+		newLoadBalancerID string
+		nodes             []*v1.Node
+		lbStatus          *v1.LoadBalancerStatus
+		err               error
 	}{
 		{
 			name: "successfully ensured loadbalancer by name, already exists",
@@ -3666,7 +3667,9 @@ func Test_EnsureLoadBalancer(t *testing.T) {
 				return nil, newFakeNotOKResponse(), errors.New("list should not have been invoked")
 			},
 			createFn: func(context.Context, *godo.LoadBalancerRequest) (*godo.LoadBalancer, *godo.Response, error) {
-				return createLB(), newFakeOKResponse(), nil
+				lb := createLB()
+				lb.ID = "other-load-balancer-id"
+				return lb, newFakeOKResponse(), nil
 			},
 			updateFn: func(ctx context.Context, lbID string, lbr *godo.LoadBalancerRequest) (*godo.LoadBalancer, *godo.Response, error) {
 				return nil, newFakeNotOKResponse(), errors.New("update should not have been invoked")
@@ -3691,6 +3694,7 @@ func Test_EnsureLoadBalancer(t *testing.T) {
 					},
 				},
 			},
+			newLoadBalancerID: "other-load-balancer-id",
 			nodes: []*v1.Node{
 				{
 					ObjectMeta: metav1.ObjectMeta{
@@ -3821,6 +3825,9 @@ func Test_EnsureLoadBalancer(t *testing.T) {
 
 				gotLoadBalancerID := svc.Annotations[annoDOLoadBalancerID]
 				wantLoadBalancerID := "load-balancer-id"
+				if test.newLoadBalancerID != "" {
+					wantLoadBalancerID = test.newLoadBalancerID
+				}
 				if gotLoadBalancerID != wantLoadBalancerID {
 					t.Errorf("got load-balancer ID %q, want %q", gotLoadBalancerID, wantLoadBalancerID)
 				}