Merge pull request #109137 from wojtek-t/fix_multiple_values_indexer

Fix issues in indexer caused by object changing the number of index values

staging/src/k8s.io/client-go/tools/cache/thread_safe_store.go

@@ -280,18 +280,15 @@ func (c *threadSafeMap) updateIndices(oldObj interface{}, newObj interface{}, ke
 			c.indices[name] = index
 		}
 
+		if len(indexValues) == 1 && len(oldIndexValues) == 1 && indexValues[0] == oldIndexValues[0] {
+			// We optimize for the most common case where indexFunc returns a single value which has not been changed
+			continue
+		}
+
 		for _, value := range oldIndexValues {
-			// We optimize for the most common case where indexFunc returns a single value.
-			if len(indexValues) == 1 && value == indexValues[0] {
-				continue
-			}
 			c.deleteKeyFromIndex(key, value, index)
 		}
 		for _, value := range indexValues {
-			// We optimize for the most common case where indexFunc returns a single value.
-			if len(oldIndexValues) == 1 && value == oldIndexValues[0] {
-				continue
-			}
 			c.addKeyToIndex(key, value, index)
 		}
 	}

staging/src/k8s.io/client-go/tools/cache/thread_safe_store_test.go

@@ -18,7 +18,11 @@ package cache
 
 import (
 	"fmt"
+	"strings"
 	"testing"
+
+	"github.com/google/go-cmp/cmp"
+	"github.com/stretchr/testify/assert"
 )
 
 func TestThreadSafeStoreDeleteRemovesEmptySetsFromIndex(t *testing.T) {
@@ -92,6 +96,75 @@ func TestThreadSafeStoreAddKeepsNonEmptySetPostDeleteFromIndex(t *testing.T) {
 	}
 }
 
+func TestThreadSafeStoreIndexingFunctionsWithMultipleValues(t *testing.T) {
+	testIndexer := "testIndexer"
+
+	indexers := Indexers{
+		testIndexer: func(obj interface{}) ([]string, error) {
+			return strings.Split(obj.(string), ","), nil
+		},
+	}
+
+	indices := Indices{}
+	store := NewThreadSafeStore(indexers, indices).(*threadSafeMap)
+
+	store.Add("key1", "foo")
+	store.Add("key2", "bar")
+
+	assert := assert.New(t)
+
+	compare := func(key string, expected []string) error {
+		values := store.indices[testIndexer][key].List()
+		if cmp.Equal(values, expected) {
+			return nil
+		}
+		return fmt.Errorf("unexpected index for key %s, diff=%s", key, cmp.Diff(values, expected))
+	}
+
+	assert.NoError(compare("foo", []string{"key1"}))
+	assert.NoError(compare("bar", []string{"key2"}))
+
+	store.Update("key2", "foo,bar")
+
+	assert.NoError(compare("foo", []string{"key1", "key2"}))
+	assert.NoError(compare("bar", []string{"key2"}))
+
+	store.Update("key1", "foo,bar")
+
+	assert.NoError(compare("foo", []string{"key1", "key2"}))
+	assert.NoError(compare("bar", []string{"key1", "key2"}))
+
+	store.Add("key3", "foo,bar,baz")
+
+	assert.NoError(compare("foo", []string{"key1", "key2", "key3"}))
+	assert.NoError(compare("bar", []string{"key1", "key2", "key3"}))
+	assert.NoError(compare("baz", []string{"key3"}))
+
+	store.Update("key1", "foo")
+
+	assert.NoError(compare("foo", []string{"key1", "key2", "key3"}))
+	assert.NoError(compare("bar", []string{"key2", "key3"}))
+	assert.NoError(compare("baz", []string{"key3"}))
+
+	store.Update("key2", "bar")
+
+	assert.NoError(compare("foo", []string{"key1", "key3"}))
+	assert.NoError(compare("bar", []string{"key2", "key3"}))
+	assert.NoError(compare("baz", []string{"key3"}))
+
+	store.Delete("key1")
+
+	assert.NoError(compare("foo", []string{"key3"}))
+	assert.NoError(compare("bar", []string{"key2", "key3"}))
+	assert.NoError(compare("baz", []string{"key3"}))
+
+	store.Delete("key3")
+
+	assert.NoError(compare("foo", []string{}))
+	assert.NoError(compare("bar", []string{"key2"}))
+	assert.NoError(compare("baz", []string{}))
+}
+
 func BenchmarkIndexer(b *testing.B) {
 	testIndexer := "testIndexer"