Remove Services.Get() from syncNegStatusAnnotation()

pkg/neg/controller.go

@@ -17,14 +17,12 @@ limitations under the License.
 package neg
 
 import (
-	"context"
 	"fmt"
 	"time"
 
 	apiv1 "k8s.io/api/core/v1"
 	discovery "k8s.io/api/discovery/v1"
 	v1 "k8s.io/api/networking/v1"
-	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/types"
 	"k8s.io/apimachinery/pkg/util/sets"
@@ -655,19 +653,26 @@ 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 {
 		if _, ok := service.Annotations[annotations.NEGStatusKey]; ok {
 			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
@@ -689,7 +694,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)
 }
 
 func (c *Controller) handleErr(err error, key interface{}) {

pkg/neg/controller_test.go

@@ -677,7 +677,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
@@ -726,6 +726,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{})
 
@@ -735,6 +736,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{})
 
@@ -743,6 +748,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)
 		})
 	}
 }