Skip to content

Commit

Permalink
Merge pull request #144 from aleofreddi/master
Browse files Browse the repository at this point in the history
Allow the autoneg deletion finalizer to complete if the backend service is already deleted
  • Loading branch information
rosmo authored Jan 14, 2025
2 parents e8ddb7a + 50b21fe commit 638cad1
Show file tree
Hide file tree
Showing 2 changed files with 43 additions and 3 deletions.
19 changes: 16 additions & 3 deletions controllers/autoneg.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand All @@ -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
Expand All @@ -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
}
Expand Down Expand Up @@ -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
}
Expand Down
27 changes: 27 additions & 0 deletions controllers/autoneg_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"

Expand Down Expand Up @@ -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)
}
}

0 comments on commit 638cad1

Please sign in to comment.