Merge pull request #144 from aleofreddi/master

Allow the autoneg deletion finalizer to complete if the backend service is already deleted

Author: rosmo

Diff for: controllers/autoneg.go

@@ -203,7 +203,17 @@ func (b *ProdBackendController) ReconcileBackends(actual, intended AutonegStatus
 		for idx, remove := range _removes {
 			var oldSvc *compute.BackendService
 			oldSvc, err = b.getBackendService(remove.name, remove.region)
-			if err != nil {
+			var svcUpdated = false
+			var e *errNotFound
+			if errors.As(err, &e) {
+				// If the backend service is gone, we construct a BackendService with the same name
+				// and an empty list of backends.
+				err = nil
+				oldSvc = &compute.BackendService{
+					Name:     remove.name,
+					Backends: make([]*compute.Backend, 0),
+				}
+			} else if err != nil {
 				return
 			}
 
@@ -222,6 +232,7 @@ func (b *ProdBackendController) ReconcileBackends(actual, intended AutonegStatus
 			for _, d := range remove.backends {
 				for i, be := range oldSvc.Backends {
 					if d.Group == be.Group {
+						svcUpdated = true
 						copy(oldSvc.Backends[i:], oldSvc.Backends[i+1:])
 						oldSvc.Backends = oldSvc.Backends[:len(oldSvc.Backends)-1]
 						break
@@ -230,7 +241,7 @@ func (b *ProdBackendController) ReconcileBackends(actual, intended AutonegStatus
 			}
 
 			// If we are changing backend services, save the old service
-			if upsert.name != remove.name {
+			if upsert.name != remove.name && svcUpdated {
 				if err = b.updateBackends(remove.name, remove.region, oldSvc, forceCapacity); err != nil {
 					return
 				}
@@ -273,7 +284,9 @@ func (b *ProdBackendController) ReconcileBackends(actual, intended AutonegStatus
 					newSvc.Backends = append(newSvc.Backends, &newBackend)
 				}
 			}
-			err = b.updateBackends(upsert.name, upsert.region, newSvc, forceCapacity)
+			if len(upsert.backends) > 0 {
+				err = b.updateBackends(upsert.name, upsert.region, newSvc, forceCapacity)
+			}
 			if err != nil {
 				return err
 			}

Diff for: controllers/autoneg_test.go

@@ -17,7 +17,11 @@ limitations under the License.
 package controllers
 
 import (
+	"context"
+	"google.golang.org/api/option"
 	"math"
+	"net/http"
+	"net/http/httptest"
 	"reflect"
 	"testing"
 
@@ -979,3 +983,26 @@ func Test_checkOperation(t *testing.T) {
 		}
 	}
 }
+
+func TestReconcileBackendsDeletionWithMissingBackend(t *testing.T) {
+	s := httptest.NewServer(http.HandlerFunc(func(res http.ResponseWriter, req *http.Request) {
+		// Return not found on backend service get.
+		res.WriteHeader(http.StatusNotFound)
+	}))
+	cs, err := compute.NewService(context.Background(), option.WithEndpoint(s.URL), option.WithoutAuthentication())
+	if err != nil {
+		t.Fatalf("Failed to instantiate compute service: %v", err)
+	}
+	bc := ProdBackendController{
+		project: "test-project",
+		s:       cs,
+	}
+	err = bc.ReconcileBackends(statusBasicWithNEGs, AutonegStatus{
+		// On deletion, the intended state is set to empty.
+		AutonegConfig: AutonegConfig{},
+		NEGStatus:     negStatus,
+	})
+	if err != nil {
+		t.Errorf("ReconcileBackends() got err: %v, want none", err)
+	}
+}