Merge pull request #99345 from robscott/endpointslice-wait-for-cache

Updating EndpointSlice controller to wait for cache to be updated

pkg/controller/endpointslice/endpointslice_controller.go

@@ -346,6 +346,10 @@ func (c *Controller) syncService(key string) error {
 		return err
 	}
 
+	if c.endpointSliceTracker.StaleSlices(service, endpointSlices) {
+		return &StaleInformerCache{"EndpointSlice informer cache is out of date"}
+	}
+
 	// We call ComputeEndpointLastChangeTriggerTime here to make sure that the
 	// state of the trigger time tracker gets updated even if the sync turns out
 	// to be no-op and we don't update the EndpointSlice objects.
@@ -395,7 +399,7 @@ func (c *Controller) onEndpointSliceAdd(obj interface{}) {
 		utilruntime.HandleError(fmt.Errorf("Invalid EndpointSlice provided to onEndpointSliceAdd()"))
 		return
 	}
-	if managedByController(endpointSlice) && c.endpointSliceTracker.Stale(endpointSlice) {
+	if managedByController(endpointSlice) && c.endpointSliceTracker.ShouldSync(endpointSlice) {
 		c.queueServiceForEndpointSlice(endpointSlice)
 	}
 }
@@ -411,7 +415,18 @@ func (c *Controller) onEndpointSliceUpdate(prevObj, obj interface{}) {
 		utilruntime.HandleError(fmt.Errorf("Invalid EndpointSlice provided to onEndpointSliceUpdate()"))
 		return
 	}
-	if managedByChanged(prevEndpointSlice, endpointSlice) || (managedByController(endpointSlice) && c.endpointSliceTracker.Stale(endpointSlice)) {
+	// EndpointSlice generation does not change when labels change. Although the
+	// controller will never change LabelServiceName, users might. This check
+	// ensures that we handle changes to this label.
+	svcName := endpointSlice.Labels[discovery.LabelServiceName]
+	prevSvcName := prevEndpointSlice.Labels[discovery.LabelServiceName]
+	if svcName != prevSvcName {
+		klog.Warningf("%s label changed from %s  to %s for %s", discovery.LabelServiceName, prevSvcName, svcName, endpointSlice.Name)
+		c.queueServiceForEndpointSlice(endpointSlice)
+		c.queueServiceForEndpointSlice(prevEndpointSlice)
+		return
+	}
+	if managedByChanged(prevEndpointSlice, endpointSlice) || (managedByController(endpointSlice) && c.endpointSliceTracker.ShouldSync(endpointSlice)) {
 		c.queueServiceForEndpointSlice(endpointSlice)
 	}
 }
@@ -422,7 +437,11 @@ func (c *Controller) onEndpointSliceUpdate(prevObj, obj interface{}) {
 func (c *Controller) onEndpointSliceDelete(obj interface{}) {
 	endpointSlice := getEndpointSliceFromDeleteAction(obj)
 	if endpointSlice != nil && managedByController(endpointSlice) && c.endpointSliceTracker.Has(endpointSlice) {
-		c.queueServiceForEndpointSlice(endpointSlice)
+		// This returns false if we didn't expect the EndpointSlice to be
+		// deleted. If that is the case, we queue the Service for another sync.
+		if !c.endpointSliceTracker.HandleDeletion(endpointSlice) {
+			c.queueServiceForEndpointSlice(endpointSlice)
+		}
 	}
 }
 

pkg/controller/endpointslice/endpointslice_controller_test.go

@@ -1426,6 +1426,81 @@ func TestPodDeleteBatching(t *testing.T) {
 	}
 }
 
+func TestSyncServiceStaleInformer(t *testing.T) {
+	testcases := []struct {
+		name                     string
+		informerGenerationNumber int64
+		trackerGenerationNumber  int64
+		expectError              bool
+	}{
+		{
+			name:                     "informer cache outdated",
+			informerGenerationNumber: 10,
+			trackerGenerationNumber:  12,
+			expectError:              true,
+		},
+		{
+			name:                     "cache and tracker synced",
+			informerGenerationNumber: 10,
+			trackerGenerationNumber:  10,
+			expectError:              false,
+		},
+		{
+			name:                     "tracker outdated",
+			informerGenerationNumber: 10,
+			trackerGenerationNumber:  1,
+			expectError:              false,
+		},
+	}
+
+	for _, testcase := range testcases {
+		t.Run(testcase.name, func(t *testing.T) {
+			_, esController := newController([]string{"node-1"}, time.Duration(0))
+			ns := metav1.NamespaceDefault
+			serviceName := "testing-1"
+
+			// Store Service in the cache
+			esController.serviceStore.Add(&v1.Service{
+				ObjectMeta: metav1.ObjectMeta{Name: serviceName, Namespace: ns},
+				Spec: v1.ServiceSpec{
+					Selector: map[string]string{"foo": "bar"},
+					Ports:    []v1.ServicePort{{TargetPort: intstr.FromInt(80)}},
+				},
+			})
+
+			// Create EndpointSlice in the informer cache with informerGenerationNumber
+			epSlice1 := &discovery.EndpointSlice{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:       "matching-1",
+					Namespace:  ns,
+					Generation: testcase.informerGenerationNumber,
+					Labels: map[string]string{
+						discovery.LabelServiceName: serviceName,
+						discovery.LabelManagedBy:   controllerName,
+					},
+				},
+				AddressType: discovery.AddressTypeIPv4,
+			}
+			err := esController.endpointSliceStore.Add(epSlice1)
+			if err != nil {
+				t.Fatalf("Expected no error adding EndpointSlice: %v", err)
+			}
+
+			// Create EndpointSlice in the tracker with trackerGenerationNumber
+			epSlice2 := epSlice1.DeepCopy()
+			epSlice2.Generation = testcase.trackerGenerationNumber
+			esController.endpointSliceTracker.Update(epSlice2)
+
+			err = esController.syncService(fmt.Sprintf("%s/%s", ns, serviceName))
+			// Check if we got a StaleInformerCache error
+			if isStaleInformerCacheErr(err) != testcase.expectError {
+				t.Fatalf("Expected error because informer cache is outdated")
+			}
+
+		})
+	}
+}
+
 // Test helpers
 func addPods(t *testing.T, esController *endpointSliceController, namespace string, podsCount int) {
 	t.Helper()

pkg/controller/endpointslice/endpointslice_tracker.go

@@ -19,102 +19,154 @@ package endpointslice
 import (
 	"sync"
 
+	"k8s.io/api/core/v1"
 	discovery "k8s.io/api/discovery/v1beta1"
 	"k8s.io/apimachinery/pkg/types"
 )
 
-// endpointSliceResourceVersions tracks expected EndpointSlice resource versions
+const (
-// by EndpointSlice name.
+	deletionExpected = -1
-type endpointSliceResourceVersions map[string]string
+)
 
-// endpointSliceTracker tracks EndpointSlices and their associated resource
+// generationsBySlice tracks expected EndpointSlice generations by EndpointSlice
-// versions to help determine if a change to an EndpointSlice has been processed
+// uid. A value of deletionExpected (-1) may be used here to indicate that we
-// by the EndpointSlice controller.
+// expect this EndpointSlice to be deleted.
+type generationsBySlice map[types.UID]int64
+
+// endpointSliceTracker tracks EndpointSlices and their associated generation to
+// help determine if a change to an EndpointSlice has been processed by the
+// EndpointSlice controller.
 type endpointSliceTracker struct {
-	// lock protects resourceVersionsByService.
+	// lock protects generationsByService.
 	lock sync.Mutex
-	// resourceVersionsByService tracks the list of EndpointSlices and
+	// generationsByService tracks the generations of EndpointSlices for each
-	// associated resource versions expected for a given Service.
+	// Service.
-	resourceVersionsByService map[types.NamespacedName]endpointSliceResourceVersions
+	generationsByService map[types.NamespacedName]generationsBySlice
 }
 
 // newEndpointSliceTracker creates and initializes a new endpointSliceTracker.
 func newEndpointSliceTracker() *endpointSliceTracker {
 	return &endpointSliceTracker{
-		resourceVersionsByService: map[types.NamespacedName]endpointSliceResourceVersions{},
+		generationsByService: map[types.NamespacedName]generationsBySlice{},
 	}
 }
 
-// Has returns true if the endpointSliceTracker has a resource version for the
+// Has returns true if the endpointSliceTracker has a generation for the
 // provided EndpointSlice.
 func (est *endpointSliceTracker) Has(endpointSlice *discovery.EndpointSlice) bool {
 	est.lock.Lock()
 	defer est.lock.Unlock()
 
-	rrv, ok := est.relatedResourceVersions(endpointSlice)
+	gfs, ok := est.generationsForSliceUnsafe(endpointSlice)
 	if !ok {
 		return false
 	}
-	_, ok = rrv[endpointSlice.Name]
+	_, ok = gfs[endpointSlice.UID]
 	return ok
 }
 
-// Stale returns true if this endpointSliceTracker does not have a resource
+// ShouldSync returns true if this endpointSliceTracker does not have a
-// version for the provided EndpointSlice or it does not match the resource
+// generation for the provided EndpointSlice or it is greater than the
-// version of the provided EndpointSlice.
+// generation of the tracked EndpointSlice.
-func (est *endpointSliceTracker) Stale(endpointSlice *discovery.EndpointSlice) bool {
+func (est *endpointSliceTracker) ShouldSync(endpointSlice *discovery.EndpointSlice) bool {
 	est.lock.Lock()
 	defer est.lock.Unlock()
 
-	rrv, ok := est.relatedResourceVersions(endpointSlice)
+	gfs, ok := est.generationsForSliceUnsafe(endpointSlice)
 	if !ok {
 		return true
 	}
-	return rrv[endpointSlice.Name] != endpointSlice.ResourceVersion
+	g, ok := gfs[endpointSlice.UID]
+	return !ok || endpointSlice.Generation > g
 }
 
-// Update adds or updates the resource version in this endpointSliceTracker for
+// StaleSlices returns true if one or more of the provided EndpointSlices
-// the provided EndpointSlice.
+// have older generations than the corresponding tracked ones or if the tracker
+// is expecting one or more of the provided EndpointSlices to be deleted.
+func (est *endpointSliceTracker) StaleSlices(service *v1.Service, endpointSlices []*discovery.EndpointSlice) bool {
+	est.lock.Lock()
+	defer est.lock.Unlock()
+
+	nn := types.NamespacedName{Name: service.Name, Namespace: service.Namespace}
+	gfs, ok := est.generationsByService[nn]
+	if !ok {
+		return false
+	}
+	for _, endpointSlice := range endpointSlices {
+		g, ok := gfs[endpointSlice.UID]
+		if ok && (g == deletionExpected || g > endpointSlice.Generation) {
+			return true
+		}
+	}
+	return false
+}
+
+// Update adds or updates the generation in this endpointSliceTracker for the
+// provided EndpointSlice.
 func (est *endpointSliceTracker) Update(endpointSlice *discovery.EndpointSlice) {
 	est.lock.Lock()
 	defer est.lock.Unlock()
 
-	rrv, ok := est.relatedResourceVersions(endpointSlice)
+	gfs, ok := est.generationsForSliceUnsafe(endpointSlice)
+
 	if !ok {
-		rrv = endpointSliceResourceVersions{}
+		gfs = generationsBySlice{}
-		est.resourceVersionsByService[getServiceNN(endpointSlice)] = rrv
+		est.generationsByService[getServiceNN(endpointSlice)] = gfs
 	}
-	rrv[endpointSlice.Name] = endpointSlice.ResourceVersion
+	gfs[endpointSlice.UID] = endpointSlice.Generation
 }
 
-// DeleteService removes the set of resource versions tracked for the Service.
+// DeleteService removes the set of generations tracked for the Service.
 func (est *endpointSliceTracker) DeleteService(namespace, name string) {
 	est.lock.Lock()
 	defer est.lock.Unlock()
 
 	serviceNN := types.NamespacedName{Name: name, Namespace: namespace}
-	delete(est.resourceVersionsByService, serviceNN)
+	delete(est.generationsByService, serviceNN)
 }
 
-// Delete removes the resource version in this endpointSliceTracker for the
+// ExpectDeletion sets the generation to deletionExpected in this
-// provided EndpointSlice.
+// endpointSliceTracker for the provided EndpointSlice.
-func (est *endpointSliceTracker) Delete(endpointSlice *discovery.EndpointSlice) {
+func (est *endpointSliceTracker) ExpectDeletion(endpointSlice *discovery.EndpointSlice) {
 	est.lock.Lock()
 	defer est.lock.Unlock()
 
-	rrv, ok := est.relatedResourceVersions(endpointSlice)
+	gfs, ok := est.generationsForSliceUnsafe(endpointSlice)
-	if ok {
+
-		delete(rrv, endpointSlice.Name)
+	if !ok {
+		gfs = generationsBySlice{}
+		est.generationsByService[getServiceNN(endpointSlice)] = gfs
 	}
+	gfs[endpointSlice.UID] = deletionExpected
 }
 
-// relatedResourceVersions returns the set of resource versions tracked for the
+// HandleDeletion removes the generation in this endpointSliceTracker for the
-// Service corresponding to the provided EndpointSlice, and a bool to indicate
+// provided EndpointSlice. This returns true if the tracker expected this
-// if it exists.
+// EndpointSlice to be deleted and false if not.
-func (est *endpointSliceTracker) relatedResourceVersions(endpointSlice *discovery.EndpointSlice) (endpointSliceResourceVersions, bool) {
+func (est *endpointSliceTracker) HandleDeletion(endpointSlice *discovery.EndpointSlice) bool {
+	est.lock.Lock()
+	defer est.lock.Unlock()
+
+	gfs, ok := est.generationsForSliceUnsafe(endpointSlice)
+
+	if ok {
+		g, ok := gfs[endpointSlice.UID]
+		delete(gfs, endpointSlice.UID)
+		if ok && g != deletionExpected {
+			return false
+		}
+	}
+
+	return true
+}
+
+// generationsForSliceUnsafe returns the generations for the Service
+// corresponding to the provided EndpointSlice, and a bool to indicate if it
+// exists. A lock must be applied before calling this function.
+func (est *endpointSliceTracker) generationsForSliceUnsafe(endpointSlice *discovery.EndpointSlice) (generationsBySlice, bool) {
 	serviceNN := getServiceNN(endpointSlice)
-	vers, ok := est.resourceVersionsByService[serviceNN]
+	generations, ok := est.generationsByService[serviceNN]
-	return vers, ok
+	return generations, ok
 }
 
 // getServiceNN returns a namespaced name for the Service corresponding to the

pkg/controller/endpointslice/endpointslice_tracker_test.go

@@ -19,8 +19,7 @@ package endpointslice
 import (
 	"testing"
 
-	"github.com/stretchr/testify/assert"
+	"k8s.io/api/core/v1"
-
 	discovery "k8s.io/api/discovery/v1beta1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/types"
@@ -29,72 +28,59 @@ import (
 func TestEndpointSliceTrackerUpdate(t *testing.T) {
 	epSlice1 := &discovery.EndpointSlice{
 		ObjectMeta: metav1.ObjectMeta{
-			Name:            "example-1",
+			Name:       "example-1",
-			Namespace:       "ns1",
+			Namespace:  "ns1",
-			ResourceVersion: "rv1",
+			UID:        "original",
-			Labels:          map[string]string{discovery.LabelServiceName: "svc1"},
+			Generation: 1,
+			Labels:     map[string]string{discovery.LabelServiceName: "svc1"},
 		},
 	}
 
 	epSlice1DifferentNS := epSlice1.DeepCopy()
 	epSlice1DifferentNS.Namespace = "ns2"
+	epSlice1DifferentNS.UID = "diff-ns"
 
 	epSlice1DifferentService := epSlice1.DeepCopy()
 	epSlice1DifferentService.Labels[discovery.LabelServiceName] = "svc2"
+	epSlice1DifferentService.UID = "diff-svc"
 
-	epSlice1DifferentRV := epSlice1.DeepCopy()
+	epSlice1NewerGen := epSlice1.DeepCopy()
-	epSlice1DifferentRV.ResourceVersion = "rv2"
+	epSlice1NewerGen.Generation = 2
 
 	testCases := map[string]struct {
-		updateParam                     *discovery.EndpointSlice
+		updateParam      *discovery.EndpointSlice
-		checksParam                     *discovery.EndpointSlice
+		checksParam      *discovery.EndpointSlice
-		expectHas                       bool
+		expectHas        bool
-		expectStale                     bool
+		expectShouldSync bool
-		expectResourceVersionsByService map[types.NamespacedName]endpointSliceResourceVersions
+		expectGeneration int64
 	}{
 		"same slice": {
-			updateParam: epSlice1,
+			updateParam:      epSlice1,
-			checksParam: epSlice1,
+			checksParam:      epSlice1,
-			expectHas:   true,
+			expectHas:        true,
-			expectStale: false,
+			expectShouldSync: false,
-			expectResourceVersionsByService: map[types.NamespacedName]endpointSliceResourceVersions{
+			expectGeneration: epSlice1.Generation,
-				{Namespace: epSlice1.Namespace, Name: "svc1"}: {
-					epSlice1.Name: epSlice1.ResourceVersion,
-				},
-			},
 		},
 		"different namespace": {
-			updateParam: epSlice1,
+			updateParam:      epSlice1,
-			checksParam: epSlice1DifferentNS,
+			checksParam:      epSlice1DifferentNS,
-			expectHas:   false,
+			expectHas:        false,
-			expectStale: true,
+			expectShouldSync: true,
-			expectResourceVersionsByService: map[types.NamespacedName]endpointSliceResourceVersions{
+			expectGeneration: epSlice1.Generation,
-				{Namespace: epSlice1.Namespace, Name: "svc1"}: {
-					epSlice1.Name: epSlice1.ResourceVersion,
-				},
-			},
 		},
 		"different service": {
-			updateParam: epSlice1,
+			updateParam:      epSlice1,
-			checksParam: epSlice1DifferentService,
+			checksParam:      epSlice1DifferentService,
-			expectHas:   false,
+			expectHas:        false,
-			expectStale: true,
+			expectShouldSync: true,
-			expectResourceVersionsByService: map[types.NamespacedName]endpointSliceResourceVersions{
+			expectGeneration: epSlice1.Generation,
-				{Namespace: epSlice1.Namespace, Name: "svc1"}: {
-					epSlice1.Name: epSlice1.ResourceVersion,
-				},
-			},
 		},
-		"different resource version": {
+		"newer generation": {
-			updateParam: epSlice1,
+			updateParam:      epSlice1,
-			checksParam: epSlice1DifferentRV,
+			checksParam:      epSlice1NewerGen,
-			expectHas:   true,
+			expectHas:        true,
-			expectStale: true,
+			expectShouldSync: true,
-			expectResourceVersionsByService: map[types.NamespacedName]endpointSliceResourceVersions{
+			expectGeneration: epSlice1.Generation,
-				{Namespace: epSlice1.Namespace, Name: "svc1"}: {
-					epSlice1.Name: epSlice1.ResourceVersion,
-				},
-			},
 		},
 	}
 
@@ -105,80 +91,195 @@ func TestEndpointSliceTrackerUpdate(t *testing.T) {
 			if esTracker.Has(tc.checksParam) != tc.expectHas {
 				t.Errorf("tc.tracker.Has(%+v) == %t, expected %t", tc.checksParam, esTracker.Has(tc.checksParam), tc.expectHas)
 			}
-			if esTracker.Stale(tc.checksParam) != tc.expectStale {
+			if esTracker.ShouldSync(tc.checksParam) != tc.expectShouldSync {
-				t.Errorf("tc.tracker.Stale(%+v) == %t, expected %t", tc.checksParam, esTracker.Stale(tc.checksParam), tc.expectStale)
+				t.Errorf("tc.tracker.ShouldSync(%+v) == %t, expected %t", tc.checksParam, esTracker.ShouldSync(tc.checksParam), tc.expectShouldSync)
+			}
+			serviceNN := types.NamespacedName{Namespace: epSlice1.Namespace, Name: "svc1"}
+			gfs, ok := esTracker.generationsByService[serviceNN]
+			if !ok {
+				t.Fatalf("expected tracker to have generations for %s Service", serviceNN.Name)
+			}
+			generation, ok := gfs[epSlice1.UID]
+			if !ok {
+				t.Fatalf("expected tracker to have generation for %s EndpointSlice", epSlice1.Name)
+			}
+			if tc.expectGeneration != generation {
+				t.Fatalf("expected generation to be %d, got %d", tc.expectGeneration, generation)
 			}
-			assert.Equal(t, tc.expectResourceVersionsByService, esTracker.resourceVersionsByService)
 		})
 	}
 }
 
-func TestEndpointSliceTrackerDelete(t *testing.T) {
+func TestEndpointSliceTrackerStaleSlices(t *testing.T) {
 	epSlice1 := &discovery.EndpointSlice{
 		ObjectMeta: metav1.ObjectMeta{
-			Name:            "example-1",
+			Name:       "example-1",
-			Namespace:       "ns1",
+			Namespace:  "ns1",
-			ResourceVersion: "rv1",
+			UID:        "original",
-			Labels:          map[string]string{discovery.LabelServiceName: "svc1"},
+			Generation: 1,
+			Labels:     map[string]string{discovery.LabelServiceName: "svc1"},
+		},
+	}
+
+	epSlice1NewerGen := epSlice1.DeepCopy()
+	epSlice1NewerGen.Generation = 2
+
+	testCases := []struct {
+		name         string
+		tracker      *endpointSliceTracker
+		serviceParam *v1.Service
+		slicesParam  []*discovery.EndpointSlice
+		expectNewer  bool
+	}{{
+		name: "empty tracker",
+		tracker: &endpointSliceTracker{
+			generationsByService: map[types.NamespacedName]generationsBySlice{},
+		},
+		serviceParam: &v1.Service{ObjectMeta: metav1.ObjectMeta{Name: "svc1", Namespace: "ns1"}},
+		slicesParam:  []*discovery.EndpointSlice{},
+		expectNewer:  false,
+	}, {
+		name: "empty slices",
+		tracker: &endpointSliceTracker{
+			generationsByService: map[types.NamespacedName]generationsBySlice{
+				{Name: "svc1", Namespace: "ns1"}: {},
+			},
+		},
+		serviceParam: &v1.Service{ObjectMeta: metav1.ObjectMeta{Name: "svc1", Namespace: "ns1"}},
+		slicesParam:  []*discovery.EndpointSlice{},
+		expectNewer:  false,
+	}, {
+		name: "matching slices",
+		tracker: &endpointSliceTracker{
+			generationsByService: map[types.NamespacedName]generationsBySlice{
+				{Name: "svc1", Namespace: "ns1"}: {
+					epSlice1.UID: epSlice1.Generation,
+				},
+			},
+		},
+		serviceParam: &v1.Service{ObjectMeta: metav1.ObjectMeta{Name: "svc1", Namespace: "ns1"}},
+		slicesParam:  []*discovery.EndpointSlice{epSlice1},
+		expectNewer:  false,
+	}, {
+		name: "newer slice in tracker",
+		tracker: &endpointSliceTracker{
+			generationsByService: map[types.NamespacedName]generationsBySlice{
+				{Name: "svc1", Namespace: "ns1"}: {
+					epSlice1.UID: epSlice1NewerGen.Generation,
+				},
+			},
+		},
+		serviceParam: &v1.Service{ObjectMeta: metav1.ObjectMeta{Name: "svc1", Namespace: "ns1"}},
+		slicesParam:  []*discovery.EndpointSlice{epSlice1},
+		expectNewer:  true,
+	}, {
+		name: "newer slice in params",
+		tracker: &endpointSliceTracker{
+			generationsByService: map[types.NamespacedName]generationsBySlice{
+				{Name: "svc1", Namespace: "ns1"}: {
+					epSlice1.UID: epSlice1.Generation,
+				},
+			},
+		},
+		serviceParam: &v1.Service{ObjectMeta: metav1.ObjectMeta{Name: "svc1", Namespace: "ns1"}},
+		slicesParam:  []*discovery.EndpointSlice{epSlice1NewerGen},
+		expectNewer:  false,
+	}}
+
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			actualNewer := tc.tracker.StaleSlices(tc.serviceParam, tc.slicesParam)
+			if actualNewer != tc.expectNewer {
+				t.Errorf("Expected %t, got %t", tc.expectNewer, actualNewer)
+			}
+		})
+	}
+}
+func TestEndpointSliceTrackerDeletion(t *testing.T) {
+	epSlice1 := &discovery.EndpointSlice{
+		ObjectMeta: metav1.ObjectMeta{
+			Name:       "example-1",
+			Namespace:  "ns1",
+			UID:        "original",
+			Generation: 1,
+			Labels:     map[string]string{discovery.LabelServiceName: "svc1"},
 		},
 	}
 
 	epSlice1DifferentNS := epSlice1.DeepCopy()
 	epSlice1DifferentNS.Namespace = "ns2"
+	epSlice1DifferentNS.UID = "diff-ns"
 
 	epSlice1DifferentService := epSlice1.DeepCopy()
 	epSlice1DifferentService.Labels[discovery.LabelServiceName] = "svc2"
+	epSlice1DifferentService.UID = "diff-svc"
 
-	epSlice1DifferentRV := epSlice1.DeepCopy()
+	epSlice1NewerGen := epSlice1.DeepCopy()
-	epSlice1DifferentRV.ResourceVersion = "rv2"
+	epSlice1NewerGen.Generation = 2
 
 	testCases := map[string]struct {
-		deleteParam *discovery.EndpointSlice
+		expectDeletionParam        *discovery.EndpointSlice
-		checksParam *discovery.EndpointSlice
+		checksParam                *discovery.EndpointSlice
-		expectHas   bool
+		deleteParam                *discovery.EndpointSlice
-		expectStale bool
+		expectHas                  bool
+		expectShouldSync           bool
+		expectedHandleDeletionResp bool
 	}{
 		"same slice": {
-			deleteParam: epSlice1,
+			expectDeletionParam:        epSlice1,
-			checksParam: epSlice1,
+			checksParam:                epSlice1,
-			expectHas:   false,
+			deleteParam:                epSlice1,
-			expectStale: true,
+			expectHas:                  true,
+			expectShouldSync:           true,
+			expectedHandleDeletionResp: true,
 		},
 		"different namespace": {
-			deleteParam: epSlice1DifferentNS,
+			expectDeletionParam:        epSlice1DifferentNS,
-			checksParam: epSlice1DifferentNS,
+			checksParam:                epSlice1DifferentNS,
-			expectHas:   false,
+			deleteParam:                epSlice1DifferentNS,
-			expectStale: true,
+			expectHas:                  true,
+			expectShouldSync:           true,
+			expectedHandleDeletionResp: false,
 		},
 		"different namespace, check original ep slice": {
-			deleteParam: epSlice1DifferentNS,
+			expectDeletionParam:        epSlice1DifferentNS,
-			checksParam: epSlice1,
+			checksParam:                epSlice1,
-			expectHas:   true,
+			deleteParam:                epSlice1DifferentNS,
-			expectStale: false,
+			expectHas:                  true,
+			expectShouldSync:           false,
+			expectedHandleDeletionResp: false,
 		},
 		"different service": {
-			deleteParam: epSlice1DifferentService,
+			expectDeletionParam:        epSlice1DifferentService,
-			checksParam: epSlice1DifferentService,
+			checksParam:                epSlice1DifferentService,
-			expectHas:   false,
+			deleteParam:                epSlice1DifferentService,
-			expectStale: true,
+			expectHas:                  true,
+			expectShouldSync:           true,
+			expectedHandleDeletionResp: false,
 		},
-		"different service, check original ep slice": {
+		"expectDelete different service, check original ep slice, delete original": {
-			deleteParam: epSlice1DifferentService,
+			expectDeletionParam:        epSlice1DifferentService,
-			checksParam: epSlice1,
+			checksParam:                epSlice1,
-			expectHas:   true,
+			deleteParam:                epSlice1,
-			expectStale: false,
+			expectHas:                  true,
+			expectShouldSync:           false,
+			expectedHandleDeletionResp: false,
 		},
-		"different resource version": {
+		"different generation": {
-			deleteParam: epSlice1DifferentRV,
+			expectDeletionParam:        epSlice1NewerGen,
-			checksParam: epSlice1DifferentRV,
+			checksParam:                epSlice1NewerGen,
-			expectHas:   false,
+			deleteParam:                epSlice1NewerGen,
-			expectStale: true,
+			expectHas:                  true,
+			expectShouldSync:           true,
+			expectedHandleDeletionResp: true,
 		},
-		"different resource version, check original ep slice": {
+		"expectDelete different generation, check original ep slice, delete original": {
-			deleteParam: epSlice1DifferentRV,
+			expectDeletionParam:        epSlice1NewerGen,
-			checksParam: epSlice1,
+			checksParam:                epSlice1,
-			expectHas:   false,
+			deleteParam:                epSlice1,
-			expectStale: true,
+			expectHas:                  true,
+			expectShouldSync:           true,
+			expectedHandleDeletionResp: true,
 		},
 	}
 
@@ -187,13 +288,20 @@ func TestEndpointSliceTrackerDelete(t *testing.T) {
 			esTracker := newEndpointSliceTracker()
 			esTracker.Update(epSlice1)
 
-			esTracker.Delete(tc.deleteParam)
+			esTracker.ExpectDeletion(tc.expectDeletionParam)
 			if esTracker.Has(tc.checksParam) != tc.expectHas {
 				t.Errorf("esTracker.Has(%+v) == %t, expected %t", tc.checksParam, esTracker.Has(tc.checksParam), tc.expectHas)
 			}
-			if esTracker.Stale(tc.checksParam) != tc.expectStale {
+			if esTracker.ShouldSync(tc.checksParam) != tc.expectShouldSync {
-				t.Errorf("esTracker.Stale(%+v) == %t, expected %t", tc.checksParam, esTracker.Stale(tc.checksParam), tc.expectStale)
+				t.Errorf("esTracker.ShouldSync(%+v) == %t, expected %t", tc.checksParam, esTracker.ShouldSync(tc.checksParam), tc.expectShouldSync)
 			}
+			if esTracker.HandleDeletion(epSlice1) != tc.expectedHandleDeletionResp {
+				t.Errorf("esTracker.ShouldSync(%+v) == %t, expected %t", epSlice1, esTracker.HandleDeletion(epSlice1), tc.expectedHandleDeletionResp)
+			}
+			if esTracker.Has(epSlice1) != false {
+				t.Errorf("esTracker.Has(%+v) == %t, expected false", epSlice1, esTracker.Has(epSlice1))
+			}
+
 		})
 	}
 }
@@ -203,48 +311,39 @@ func TestEndpointSliceTrackerDeleteService(t *testing.T) {
 	svcName2, svcNS2 := "svc2", "ns2"
 	epSlice1 := &discovery.EndpointSlice{
 		ObjectMeta: metav1.ObjectMeta{
-			Name:            "example-1",
+			Name:       "example-1",
-			Namespace:       svcNS1,
+			Namespace:  svcNS1,
-			ResourceVersion: "rv1",
+			Generation: 1,
-			Labels:          map[string]string{discovery.LabelServiceName: svcName1},
+			Labels:     map[string]string{discovery.LabelServiceName: svcName1},
 		},
 	}
 
 	testCases := map[string]struct {
-		updateParam                     *discovery.EndpointSlice
+		updateParam        *discovery.EndpointSlice
-		deleteServiceParam              *types.NamespacedName
+		deleteServiceParam *types.NamespacedName
-		expectHas                       bool
+		expectHas          bool
-		expectStale                     bool
+		expectShouldSync   bool
-		expectResourceVersionsByService map[types.NamespacedName]endpointSliceResourceVersions
+		expectGeneration   int64
 	}{
 		"same service": {
-			updateParam:                     epSlice1,
+			updateParam:        epSlice1,
-			deleteServiceParam:              &types.NamespacedName{Namespace: svcNS1, Name: svcName1},
+			deleteServiceParam: &types.NamespacedName{Namespace: svcNS1, Name: svcName1},
-			expectHas:                       false,
+			expectHas:          false,
-			expectStale:                     true,
+			expectShouldSync:   true,
-			expectResourceVersionsByService: map[types.NamespacedName]endpointSliceResourceVersions{},
 		},
 		"different namespace": {
 			updateParam:        epSlice1,
 			deleteServiceParam: &types.NamespacedName{Namespace: svcNS2, Name: svcName1},
 			expectHas:          true,
-			expectStale:        false,
+			expectShouldSync:   false,
-			expectResourceVersionsByService: map[types.NamespacedName]endpointSliceResourceVersions{
+			expectGeneration:   epSlice1.Generation,
-				{Namespace: epSlice1.Namespace, Name: "svc1"}: {
-					epSlice1.Name: epSlice1.ResourceVersion,
-				},
-			},
 		},
 		"different service": {
 			updateParam:        epSlice1,
 			deleteServiceParam: &types.NamespacedName{Namespace: svcNS1, Name: svcName2},
 			expectHas:          true,
-			expectStale:        false,
+			expectShouldSync:   false,
-			expectResourceVersionsByService: map[types.NamespacedName]endpointSliceResourceVersions{
+			expectGeneration:   epSlice1.Generation,
-				{Namespace: epSlice1.Namespace, Name: "svc1"}: {
-					epSlice1.Name: epSlice1.ResourceVersion,
-				},
-			},
 		},
 	}
 
@@ -256,10 +355,23 @@ func TestEndpointSliceTrackerDeleteService(t *testing.T) {
 			if esTracker.Has(tc.updateParam) != tc.expectHas {
 				t.Errorf("tc.tracker.Has(%+v) == %t, expected %t", tc.updateParam, esTracker.Has(tc.updateParam), tc.expectHas)
 			}
-			if esTracker.Stale(tc.updateParam) != tc.expectStale {
+			if esTracker.ShouldSync(tc.updateParam) != tc.expectShouldSync {
-				t.Errorf("tc.tracker.Stale(%+v) == %t, expected %t", tc.updateParam, esTracker.Stale(tc.updateParam), tc.expectStale)
+				t.Errorf("tc.tracker.ShouldSync(%+v) == %t, expected %t", tc.updateParam, esTracker.ShouldSync(tc.updateParam), tc.expectShouldSync)
+			}
+			if tc.expectGeneration != 0 {
+				serviceNN := types.NamespacedName{Namespace: epSlice1.Namespace, Name: "svc1"}
+				gfs, ok := esTracker.generationsByService[serviceNN]
+				if !ok {
+					t.Fatalf("expected tracker to have status for %s Service", serviceNN.Name)
+				}
+				generation, ok := gfs[epSlice1.UID]
+				if !ok {
+					t.Fatalf("expected tracker to have generation for %s EndpointSlice", epSlice1.Name)
+				}
+				if tc.expectGeneration != generation {
+					t.Fatalf("expected generation to be %d, got %d", tc.expectGeneration, generation)
+				}
 			}
-			assert.Equal(t, tc.expectResourceVersionsByService, esTracker.resourceVersionsByService)
 		})
 	}
 }

pkg/controller/endpointslice/errors.go

@@ -0,0 +1,30 @@
+/*
+Copyright 2021 The Kubernetes Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package endpointslice
+
+// StaleInformerCache errors indicate that the informer cache includes out of
+// date resources.
+type StaleInformerCache struct {
+	msg string
+}
+
+func (e *StaleInformerCache) Error() string { return e.msg }
+
+func isStaleInformerCacheErr(err error) bool {
+	_, ok := err.(*StaleInformerCache)
+	return ok
+}

pkg/controller/endpointslice/reconciler.go

@@ -101,7 +101,7 @@ func (r *reconciler) reconcile(service *corev1.Service, pods []*corev1.Pod, exis
 		if err != nil {
 			errs = append(errs, fmt.Errorf("Error deleting %s EndpointSlice for Service %s/%s: %v", sliceToDelete.Name, service.Namespace, service.Name, err))
 		} else {
-			r.endpointSliceTracker.Delete(sliceToDelete)
+			r.endpointSliceTracker.ExpectDeletion(sliceToDelete)
 			metrics.EndpointSliceChanges.WithLabelValues("delete").Inc()
 		}
 	}
@@ -293,7 +293,7 @@ func (r *reconciler) finalize(
 		if err != nil {
 			return fmt.Errorf("failed to delete %s EndpointSlice for Service %s/%s: %v", endpointSlice.Name, service.Namespace, service.Name, err)
 		}
-		r.endpointSliceTracker.Delete(endpointSlice)
+		r.endpointSliceTracker.ExpectDeletion(endpointSlice)
 		metrics.EndpointSliceChanges.WithLabelValues("delete").Inc()
 	}
 

pkg/controller/endpointslice/reconciler_test.go

@@ -65,7 +65,7 @@ func TestReconcileEmpty(t *testing.T) {
 	assert.Equal(t, svc.Name, slices[0].Labels[discovery.LabelServiceName])
 	assert.EqualValues(t, []discovery.EndpointPort{}, slices[0].Ports)
 	assert.EqualValues(t, []discovery.Endpoint{}, slices[0].Endpoints)
-	expectTrackedResourceVersion(t, r.endpointSliceTracker, &slices[0], "100")
+	expectTrackedGeneration(t, r.endpointSliceTracker, &slices[0], 1)
 	expectMetrics(t, expectedMetrics{desiredSlices: 1, actualSlices: 1, desiredEndpoints: 0, addedPerSync: 0, removedPerSync: 0, numCreated: 1, numUpdated: 0, numDeleted: 0})
 }
 
@@ -473,7 +473,7 @@ func TestReconcile1Pod(t *testing.T) {
 					t.Fatalf("Expected endpoint: %+v, got: %+v", expectedEndPointList[0], endpoint)
 				}
 
-				expectTrackedResourceVersion(t, r.endpointSliceTracker, &slice, "100")
+				expectTrackedGeneration(t, r.endpointSliceTracker, &slice, 1)
 
 				expectMetrics(t,
 					expectedMetrics{
@@ -516,7 +516,7 @@ func TestReconcile1EndpointSlice(t *testing.T) {
 	assert.Equal(t, svc.Name, slices[0].Labels[discovery.LabelServiceName])
 	assert.EqualValues(t, []discovery.EndpointPort{}, slices[0].Ports)
 	assert.EqualValues(t, []discovery.Endpoint{}, slices[0].Endpoints)
-	expectTrackedResourceVersion(t, r.endpointSliceTracker, &slices[0], "200")
+	expectTrackedGeneration(t, r.endpointSliceTracker, &slices[0], 1)
 	expectMetrics(t, expectedMetrics{desiredSlices: 1, actualSlices: 1, desiredEndpoints: 0, addedPerSync: 0, removedPerSync: 0, numCreated: 0, numUpdated: 1, numDeleted: 0})
 }
 
@@ -1436,14 +1436,17 @@ func expectActions(t *testing.T, actions []k8stesting.Action, num int, verb, res
 	}
 }
 
-func expectTrackedResourceVersion(t *testing.T, tracker *endpointSliceTracker, slice *discovery.EndpointSlice, expectedRV string) {
+func expectTrackedGeneration(t *testing.T, tracker *endpointSliceTracker, slice *discovery.EndpointSlice, expectedGeneration int64) {
-	rrv, _ := tracker.relatedResourceVersions(slice)
+	gfs, ok := tracker.generationsForSliceUnsafe(slice)
-	rv, tracked := rrv[slice.Name]
+	if !ok {
-	if !tracked {
+		t.Fatalf("Expected Service to be tracked for EndpointSlices %s", slice.Name)
+	}
+	generation, ok := gfs[slice.UID]
+	if !ok {
 		t.Fatalf("Expected EndpointSlice %s to be tracked", slice.Name)
 	}
-	if rv != expectedRV {
+	if generation != expectedGeneration {
-		t.Errorf("Expected ResourceVersion of %s to be %s, got %s", slice.Name, expectedRV, rv)
+		t.Errorf("Expected Generation of %s to be %d, got %d", slice.Name, expectedGeneration, generation)
 	}
 }
 

pkg/controller/endpointslice/utils_test.go

@@ -1001,13 +1001,13 @@ func newClientset() *fake.Clientset {
 			endpointSlice.ObjectMeta.Name = fmt.Sprintf("%s-%s", endpointSlice.ObjectMeta.GenerateName, rand.String(8))
 			endpointSlice.ObjectMeta.GenerateName = ""
 		}
-		endpointSlice.ObjectMeta.ResourceVersion = "100"
+		endpointSlice.Generation = 1
 
 		return false, endpointSlice, nil
 	}))
 	client.PrependReactor("update", "endpointslices", k8stesting.ReactionFunc(func(action k8stesting.Action) (bool, runtime.Object, error) {
 		endpointSlice := action.(k8stesting.CreateAction).GetObject().(*discovery.EndpointSlice)
-		endpointSlice.ObjectMeta.ResourceVersion = "200"
+		endpointSlice.Generation++
 		return false, endpointSlice, nil
 	}))