From db6bbf2375233dba8bdb6d8a0b55ebd1dedbdc69 Mon Sep 17 00:00:00 2001 From: Quan Tian Date: Sat, 21 Sep 2019 09:32:18 -0700 Subject: [PATCH] Fix EndpointSliceController service deletion processing syncService shouldn't return error if the service doesn't exist which means it's triggered by service deletion, otherwise the service would be enqueued repeatedly even its cleanup has been executed successfully. This patch makes syncService return nil if the error is NotFound when getting the service, like the other controllers do. --- pkg/controller/endpointslice/endpointslice_controller.go | 2 ++ .../endpointslice/endpointslice_controller_test.go | 5 ++--- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/pkg/controller/endpointslice/endpointslice_controller.go b/pkg/controller/endpointslice/endpointslice_controller.go index e956530dcd3..e77a374f6c0 100644 --- a/pkg/controller/endpointslice/endpointslice_controller.go +++ b/pkg/controller/endpointslice/endpointslice_controller.go @@ -251,6 +251,8 @@ func (c *Controller) syncService(key string) error { if err != nil { if apierrors.IsNotFound(err) { c.triggerTimeTracker.DeleteService(namespace, name) + // The service has been deleted, return nil so that it won't be retried. + return nil } return err } diff --git a/pkg/controller/endpointslice/endpointslice_controller_test.go b/pkg/controller/endpointslice/endpointslice_controller_test.go index e8047b625a3..7db25d847bd 100644 --- a/pkg/controller/endpointslice/endpointslice_controller_test.go +++ b/pkg/controller/endpointslice/endpointslice_controller_test.go @@ -140,9 +140,8 @@ func TestSyncServiceMissing(t *testing.T) { err := esController.syncService(fmt.Sprintf("%s/%s", namespace, missingServiceName)) - // Since the service doesn't exist, we should get a not found error - assert.NotNil(t, err, "Expected no error syncing service") - assert.Equal(t, err.Error(), "service \"notthere\" not found") + // nil should be returned when the service doesn't exist + assert.Nil(t, err, "Expected no error syncing service") // That should mean no client actions were performed assert.Len(t, client.Actions(), 0)