github/kubernetes
1
0
Fork 0
mirror of https://github.com/k3s-io/kubernetes.git synced 2025-07-23 19:56:01 +00:00
Code Issues Projects Releases Wiki Activity

Clean up kublet secret and configmap unit test

Browse Source 
* Expected value comes before actual value in assert.Equal()
* Use assert.Equal() instead of assert.True() when possible
* Add a unit test that verifies no-op pod updates to the
  secret_manager and the configmap_manager
* Add a clarifying comment about why it's good to seemingly
  delete a secret on updates.
* Fix (for now, non-buggy) variable shadowing issue
This commit is contained in:
Joel Smith 2017-09-06 13:30:23 -06:00
parent 50c633182e
commit 58ae5a78f9
4 changed files with 62 additions and 29 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

5
pkg/kubelet/configmap/configmap_manager.go
View File

@ -282,6 +282,11 @@ func (c *cachingConfigMapManager) RegisterPod(pod *v1.Pod) {
c.registeredPods[key] = pod
if prev != nil {
for name := range getConfigMapNames(prev) {
// On an update, the .Add() call above will have re-incremented the
// ref count of any existing items, so any configmaps that are in both
// names and prev need to have their ref counts decremented. Any that
// are only in prev need to be completely removed. This unconditional
// call takes care of both cases.
c.configMapStore.Delete(prev.Namespace, name)
}
}

38
pkg/kubelet/configmap/configmap_manager_test.go
View File

@ -299,11 +299,11 @@ type configMapsToAttach struct {
volumes []string
}
func podWithConfigMaps(ns, name string, toAttach configMapsToAttach) *v1.Pod {
func podWithConfigMaps(ns, podName string, toAttach configMapsToAttach) *v1.Pod {
pod := &v1.Pod{
ObjectMeta: metav1.ObjectMeta{
Namespace: ns,
Name: name,
Name: podName,
},
Spec: v1.PodSpec{},
}
@ -454,6 +454,17 @@ func TestCacheRefcounts(t *testing.T) {
manager.RegisterPod(podWithConfigMaps("ns1", "other-name", s2))
manager.UnregisterPod(podWithConfigMaps("ns1", "other-name", s2))
s5 := configMapsToAttach{
containerEnvConfigMaps: []envConfigMaps{
{envVarNames: []string{"s7"}},
{envFromNames: []string{"s70"}},
},
}
// Check the no-op update scenario
manager.RegisterPod(podWithConfigMaps("ns1", "noop-pod", s5))
manager.RegisterPod(podWithConfigMaps("ns1", "noop-pod", s5))
refs := func(ns, name string) int {
store.lock.Lock()
defer store.lock.Unlock()
@ -463,17 +474,18 @@ func TestCacheRefcounts(t *testing.T) {
}
return item.refCount
}
assert.Equal(t, refs("ns1", "s1"), 1)
assert.Equal(t, refs("ns1", "s10"), 1)
assert.Equal(t, refs("ns1", "s2"), 1)
assert.Equal(t, refs("ns1", "s3"), 3)
assert.Equal(t, refs("ns1", "s30"), 2)
assert.Equal(t, refs("ns1", "s4"), 2)
assert.Equal(t, refs("ns1", "s5"), 4)
assert.Equal(t, refs("ns1", "s50"), 2)
assert.Equal(t, refs("ns1", "s6"), 0)
assert.Equal(t, refs("ns1", "s60"), 0)
assert.Equal(t, refs("ns1", "s7"), 0)
assert.Equal(t, 1, refs("ns1", "s1"))
assert.Equal(t, 1, refs("ns1", "s10"))
assert.Equal(t, 1, refs("ns1", "s2"))
assert.Equal(t, 3, refs("ns1", "s3"))
assert.Equal(t, 2, refs("ns1", "s30"))
assert.Equal(t, 2, refs("ns1", "s4"))
assert.Equal(t, 4, refs("ns1", "s5"))
assert.Equal(t, 2, refs("ns1", "s50"))
assert.Equal(t, 0, refs("ns1", "s6"))
assert.Equal(t, 0, refs("ns1", "s60"))
assert.Equal(t, 1, refs("ns1", "s7"))
assert.Equal(t, 1, refs("ns1", "s70"))
}
func TestCachingConfigMapManager(t *testing.T) {

5
pkg/kubelet/secret/secret_manager.go
View File

@ -282,6 +282,11 @@ func (c *cachingSecretManager) RegisterPod(pod *v1.Pod) {
c.registeredPods[key] = pod
if prev != nil {
for name := range getSecretNames(prev) {
// On an update, the .Add() call above will have re-incremented the
// ref count of any existing secrets, so any secrets that are in both
// names and prev need to have their ref counts decremented. Any that
// are only in prev need to be completely removed. This unconditional
// call takes care of both cases.
c.secretStore.Delete(prev.Namespace, name)
}
}

43
pkg/kubelet/secret/secret_manager_test.go
View File

@ -299,11 +299,11 @@ type secretsToAttach struct {
containerEnvSecrets []envSecrets
}
func podWithSecrets(ns, name string, toAttach secretsToAttach) *v1.Pod {
func podWithSecrets(ns, podName string, toAttach secretsToAttach) *v1.Pod {
pod := &v1.Pod{
ObjectMeta: metav1.ObjectMeta{
Namespace: ns,
Name: name,
Name: podName,
},
Spec: v1.PodSpec{},
}
@ -453,27 +453,38 @@ func TestCacheRefcounts(t *testing.T) {
manager.RegisterPod(podWithSecrets("ns1", "other-name", s2))
manager.UnregisterPod(podWithSecrets("ns1", "other-name", s2))
s5 := secretsToAttach{
containerEnvSecrets: []envSecrets{
{envVarNames: []string{"s7"}},
{envFromNames: []string{"s70"}},
},
}
// Check the no-op update scenario
manager.RegisterPod(podWithSecrets("ns1", "noop-pod", s5))
manager.RegisterPod(podWithSecrets("ns1", "noop-pod", s5))
// Now we have: 3 pods with s1, 2 pods with s2 and 2 pods with s3, 0 pods with s4.
verify := func(ns, name string, count int) bool {
refs := func(ns, name string) int {
store.lock.Lock()
defer store.lock.Unlock()
item, ok := store.items[objectKey{ns, name}]
if !ok {
return count == 0
return 0
}
return item.refCount == count
return item.refCount
}
assert.True(t, verify("ns1", "s1", 3))
assert.True(t, verify("ns1", "s10", 1))
assert.True(t, verify("ns1", "s2", 3))
assert.True(t, verify("ns1", "s3", 3))
assert.True(t, verify("ns1", "s30", 2))
assert.True(t, verify("ns1", "s4", 2))
assert.True(t, verify("ns1", "s5", 4))
assert.True(t, verify("ns1", "s50", 2))
assert.True(t, verify("ns1", "s6", 0))
assert.True(t, verify("ns1", "s60", 0))
assert.True(t, verify("ns1", "s7", 0))
assert.Equal(t, 3, refs("ns1", "s1"))
assert.Equal(t, 1, refs("ns1", "s10"))
assert.Equal(t, 3, refs("ns1", "s2"))
assert.Equal(t, 3, refs("ns1", "s3"))
assert.Equal(t, 2, refs("ns1", "s30"))
assert.Equal(t, 2, refs("ns1", "s4"))
assert.Equal(t, 4, refs("ns1", "s5"))
assert.Equal(t, 2, refs("ns1", "s50"))
assert.Equal(t, 0, refs("ns1", "s6"))
assert.Equal(t, 0, refs("ns1", "s60"))
assert.Equal(t, 1, refs("ns1", "s7"))
assert.Equal(t, 1, refs("ns1", "s70"))
}
func TestCachingSecretManager(t *testing.T) {