From bcfebd73dc36de5a431779843adbe1c6f7b17eb2 Mon Sep 17 00:00:00 2001 From: Gaurav Ghildiyal Date: Tue, 31 Jan 2023 10:19:51 -0800 Subject: [PATCH] Remove Services.Get() from syncNegStatusAnnotation() --- pkg/neg/controller.go | 15 +++++++++++---- pkg/neg/controller_test.go | 10 +++++++++- 2 files changed, 20 insertions(+), 5 deletions(-) diff --git a/pkg/neg/controller.go b/pkg/neg/controller.go index 6de2c3cd8b..d8baa80134 100644 --- a/pkg/neg/controller.go +++ b/pkg/neg/controller.go @@ -683,11 +683,18 @@ func (c *Controller) syncNegStatusAnnotation(namespace, name string, portMap neg if err != nil { return err } - coreClient := c.client.CoreV1() - service, err := coreClient.Services(namespace).Get(context.TODO(), name, metav1.GetOptions{}) + obj, exists, err := c.serviceLister.GetByKey(getServiceKey(namespace, name).Key()) if err != nil { return err } + if !exists { + // Service no longer exists so doesn't require any update. + return nil + } + service, ok := obj.(*apiv1.Service) + if !ok { + return fmt.Errorf("cannot convert obj to Service; obj=%T", obj) + } // Remove NEG Status Annotation when no NEG is needed if len(portMap) == 0 { @@ -695,7 +702,7 @@ func (c *Controller) syncNegStatusAnnotation(namespace, name string, portMap neg newSvcObjectMeta := service.ObjectMeta.DeepCopy() delete(newSvcObjectMeta.Annotations, annotations.NEGStatusKey) klog.V(2).Infof("Removing NEG status annotation from service: %s/%s", namespace, name) - return patch.PatchServiceObjectMetadata(coreClient, service, *newSvcObjectMeta) + return patch.PatchServiceObjectMetadata(c.client.CoreV1(), service, *newSvcObjectMeta) } // service doesn't have the expose NEG annotation and doesn't need update return nil @@ -717,7 +724,7 @@ func (c *Controller) syncNegStatusAnnotation(namespace, name string, portMap neg } newSvcObjectMeta.Annotations[annotations.NEGStatusKey] = annotation klog.V(2).Infof("Updating NEG visibility annotation %q on service %s/%s.", annotation, namespace, name) - return patch.PatchServiceObjectMetadata(coreClient, service, *newSvcObjectMeta) + return patch.PatchServiceObjectMetadata(c.client.CoreV1(), service, *newSvcObjectMeta) } // syncDestinationRuleNegStatusAnnotation syncs the destinationrule related neg status annotation diff --git a/pkg/neg/controller_test.go b/pkg/neg/controller_test.go index c4dfd4e03e..eaf7a09284 100644 --- a/pkg/neg/controller_test.go +++ b/pkg/neg/controller_test.go @@ -683,7 +683,7 @@ func TestSyncNegAnnotation(t *testing.T) { controller := newTestController(fake.NewSimpleClientset()) defer controller.stop() svcClient := controller.client.CoreV1().Services(testServiceNamespace) - newTestService(controller, false, []int32{}) + svc := newTestService(controller, false, []int32{}) namespace := testServiceNamespace name := testServiceName namer := controller.namer @@ -732,6 +732,7 @@ func TestSyncNegAnnotation(t *testing.T) { for _, tc := range testCases { t.Run(tc.desc, func(t *testing.T) { + controller.serviceLister.Add(svc) controller.syncNegStatusAnnotation(namespace, name, tc.previousPortMap) svc, _ := svcClient.Get(context.TODO(), name, metav1.GetOptions{}) @@ -741,6 +742,10 @@ func TestSyncNegAnnotation(t *testing.T) { } validateServiceStateAnnotation(t, svc, oldSvcPorts, controller.namer) + // Mimic any Update calls to the API Server by manually updating the + // informer cache. + controller.serviceLister.Update(svc) + controller.syncNegStatusAnnotation(namespace, name, tc.portMap) svc, _ = svcClient.Get(context.TODO(), name, metav1.GetOptions{}) @@ -749,6 +754,9 @@ func TestSyncNegAnnotation(t *testing.T) { svcPorts = append(svcPorts, port.ServicePort) } validateServiceStateAnnotation(t, svc, svcPorts, controller.namer) + + // Reset state of controller for the next test run. + controller.serviceLister.Delete(svc) }) } }