diff --git a/pkg/kubelet/util/manager/cache_based_manager.go b/pkg/kubelet/util/manager/cache_based_manager.go index a9321919c95..c291cfe91d9 100644 --- a/pkg/kubelet/util/manager/cache_based_manager.go +++ b/pkg/kubelet/util/manager/cache_based_manager.go @@ -29,6 +29,7 @@ import ( apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/types" "k8s.io/apimachinery/pkg/util/sets" "k8s.io/utils/clock" ) @@ -42,6 +43,7 @@ type GetObjectFunc func(string, string, metav1.GetOptions) (runtime.Object, erro type objectKey struct { namespace string name string + uid types.UID } // objectStoreItems is a single item stored in objectStore. @@ -226,7 +228,7 @@ func (c *cacheBasedManager) RegisterPod(pod *v1.Pod) { c.objectStore.AddReference(pod.Namespace, name) } var prev *v1.Pod - key := objectKey{namespace: pod.Namespace, name: pod.Name} + key := objectKey{namespace: pod.Namespace, name: pod.Name, uid: pod.UID} prev = c.registeredPods[key] c.registeredPods[key] = pod if prev != nil { @@ -243,7 +245,7 @@ func (c *cacheBasedManager) RegisterPod(pod *v1.Pod) { func (c *cacheBasedManager) UnregisterPod(pod *v1.Pod) { var prev *v1.Pod - key := objectKey{namespace: pod.Namespace, name: pod.Name} + key := objectKey{namespace: pod.Namespace, name: pod.Name, uid: pod.UID} c.lock.Lock() defer c.lock.Unlock() prev = c.registeredPods[key] diff --git a/pkg/kubelet/util/manager/cache_based_manager_test.go b/pkg/kubelet/util/manager/cache_based_manager_test.go index e56059680e7..1be96922885 100644 --- a/pkg/kubelet/util/manager/cache_based_manager_test.go +++ b/pkg/kubelet/util/manager/cache_based_manager_test.go @@ -30,6 +30,7 @@ import ( apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/types" "k8s.io/apimachinery/pkg/util/sets" clientset "k8s.io/client-go/kubernetes" @@ -335,10 +336,15 @@ type secretsToAttach struct { } func podWithSecrets(ns, podName string, toAttach secretsToAttach) *v1.Pod { + return podWithSecretsAndUID(ns, podName, "", toAttach) +} + +func podWithSecretsAndUID(ns, podName, podUID string, toAttach secretsToAttach) *v1.Pod { pod := &v1.Pod{ ObjectMeta: metav1.ObjectMeta{ Namespace: ns, Name: podName, + UID: types.UID(podUID), }, Spec: v1.PodSpec{}, } @@ -444,7 +450,7 @@ func TestRegisterIdempotence(t *testing.T) { refs := func(ns, name string) int { store.lock.Lock() defer store.lock.Unlock() - item, ok := store.items[objectKey{ns, name}] + item, ok := store.items[objectKey{namespace: ns, name: name}] if !ok { return 0 } @@ -531,7 +537,7 @@ func TestCacheRefcounts(t *testing.T) { refs := func(ns, name string) int { store.lock.Lock() defer store.lock.Unlock() - item, ok := store.items[objectKey{ns, name}] + item, ok := store.items[objectKey{namespace: ns, name: name}] if !ok { return 0 } @@ -549,6 +555,42 @@ func TestCacheRefcounts(t *testing.T) { assert.Equal(t, 0, refs("ns1", "s60")) assert.Equal(t, 1, refs("ns1", "s7")) assert.Equal(t, 1, refs("ns1", "s70")) + + // Check the interleaved registerpod/unregisterpod with identical names and different uids scenario + secret1 := secretsToAttach{ + containerEnvSecrets: []envSecrets{ + {envVarNames: []string{"secret1"}}, + }, + } + secret2 := secretsToAttach{ + containerEnvSecrets: []envSecrets{ + {envVarNames: []string{"secret2"}}, + }, + } + + // precondition: no references + assert.Equal(t, 0, refs("nsinterleaved", "secret1")) + assert.Equal(t, 0, refs("nsinterleaved", "secret2")) + + // add first pod that references secret1 only + manager.RegisterPod(podWithSecretsAndUID("nsinterleaved", "pod", "poduid1", secret1)) + assert.Equal(t, 1, refs("nsinterleaved", "secret1")) + assert.Equal(t, 0, refs("nsinterleaved", "secret2")) + + // add second pod that references secret2 only, retain references to secret1 + manager.RegisterPod(podWithSecretsAndUID("nsinterleaved", "pod", "poduid2", secret2)) + assert.Equal(t, 1, refs("nsinterleaved", "secret1")) + assert.Equal(t, 1, refs("nsinterleaved", "secret2")) + + // remove first pod that references secret1, retain references to secret2 + manager.UnregisterPod(podWithSecretsAndUID("nsinterleaved", "pod", "poduid1", secretsToAttach{})) + assert.Equal(t, 0, refs("nsinterleaved", "secret1")) + assert.Equal(t, 1, refs("nsinterleaved", "secret2")) + + // remove second pod that references secret2 + manager.UnregisterPod(podWithSecretsAndUID("nsinterleaved", "pod", "poduid2", secretsToAttach{})) + assert.Equal(t, 0, refs("nsinterleaved", "secret1")) + assert.Equal(t, 0, refs("nsinterleaved", "secret2")) } func TestCacheBasedSecretManager(t *testing.T) {