diff --git a/pkg/neg/controller.go b/pkg/neg/controller.go index b04d5ddb85..446862b1b5 100644 --- a/pkg/neg/controller.go +++ b/pkg/neg/controller.go @@ -694,11 +694,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 { @@ -706,12 +713,11 @@ func (c *Controller) syncNegStatusAnnotation(namespace, name string, portMap neg newSvcObjectMeta := service.ObjectMeta.DeepCopy() delete(newSvcObjectMeta.Annotations, annotations.NEGStatusKey) c.logger.V(2).Info("Removing NEG status annotation from service", "service", klog.KRef(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 } - negStatus := annotations.NewNegStatus(zones, portMap.ToPortNegMap()) annotation, err := negStatus.Marshal() if err != nil { @@ -728,7 +734,7 @@ func (c *Controller) syncNegStatusAnnotation(namespace, name string, portMap neg } newSvcObjectMeta.Annotations[annotations.NEGStatusKey] = annotation c.logger.V(2).Info("Updating NEG visibility annotation on service", "annotation", annotation, "service", klog.KRef(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 5992cc453b..6729b423df 100644 --- a/pkg/neg/controller_test.go +++ b/pkg/neg/controller_test.go @@ -685,7 +685,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 @@ -734,6 +734,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{}) @@ -743,6 +744,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{}) @@ -751,6 +756,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) }) } }