From 6c0535a939b0d0c5e6cbd649319092e06012bacd Mon Sep 17 00:00:00 2001 From: Wojciech Tyczynski Date: Mon, 30 Jan 2017 13:37:48 +0100 Subject: [PATCH] Use secret TTL annotation in secret manager --- pkg/controller/ttl/ttlcontroller.go | 2 +- pkg/kubelet/kubelet.go | 13 +- pkg/kubelet/secret/secret_manager.go | 47 ++++++- pkg/kubelet/secret/secret_manager_test.go | 143 ++++++++++++++++++++-- 4 files changed, 185 insertions(+), 20 deletions(-) diff --git a/pkg/controller/ttl/ttlcontroller.go b/pkg/controller/ttl/ttlcontroller.go index 3a92d64d18e..daab92eba6e 100644 --- a/pkg/controller/ttl/ttlcontroller.go +++ b/pkg/controller/ttl/ttlcontroller.go @@ -284,13 +284,13 @@ func (ttlc *TTLController) updateNodeIfNeeded(key string) error { } return err } - desiredTTL := ttlc.getDesiredTTLSeconds() desiredTTL := ttlc.getDesiredTTLSeconds() currentTTL, ok := getIntFromAnnotation(node, v1.ObjectTTLAnnotationKey) if ok && currentTTL == desiredTTL { return nil } + objCopy, err := api.Scheme.DeepCopy(node) if err != nil { return err diff --git a/pkg/kubelet/kubelet.go b/pkg/kubelet/kubelet.go index 9dfa1f71122..b513b1d5f30 100644 --- a/pkg/kubelet/kubelet.go +++ b/pkg/kubelet/kubelet.go @@ -406,11 +406,6 @@ func NewMainKubelet(kubeCfg *componentconfig.KubeletConfiguration, kubeDeps *Kub } containerRefManager := kubecontainer.NewRefManager() - secretManager, err := secret.NewCachingSecretManager(kubeDeps.KubeClient) - if err != nil { - return nil, fmt.Errorf("failed to initialize secret manager: %v", err) - } - oomWatcher := NewOOMWatcher(kubeDeps.CAdvisorInterface, kubeDeps.Recorder) klet := &Kubelet{ @@ -436,7 +431,6 @@ func NewMainKubelet(kubeCfg *componentconfig.KubeletConfiguration, kubeDeps *Kub recorder: kubeDeps.Recorder, cadvisor: kubeDeps.CAdvisorInterface, diskSpaceManager: diskSpaceManager, - secretManager: secretManager, cloud: kubeDeps.Cloud, autoDetectCloudProvider: (componentconfigv1alpha1.AutoDetectCloudProvider == kubeCfg.CloudProvider), nodeRef: nodeRef, @@ -471,6 +465,13 @@ func NewMainKubelet(kubeCfg *componentconfig.KubeletConfiguration, kubeDeps *Kub experimentalHostUserNamespaceDefaulting: utilfeature.DefaultFeatureGate.Enabled(features.ExperimentalHostUserNamespaceDefaultingGate), } + secretManager, err := secret.NewCachingSecretManager( + kubeDeps.KubeClient, secret.GetObjectTTLFromNodeFunc(klet.GetNode)) + if err != nil { + return nil, fmt.Errorf("failed to initialize secret manager: %v", err) + } + klet.secretManager = secretManager + if klet.experimentalHostUserNamespaceDefaulting { glog.Infof("Experimental host user namespace defaulting is enabled.") } diff --git a/pkg/kubelet/secret/secret_manager.go b/pkg/kubelet/secret/secret_manager.go index 207fa853c0f..17248307544 100644 --- a/pkg/kubelet/secret/secret_manager.go +++ b/pkg/kubelet/secret/secret_manager.go @@ -18,6 +18,7 @@ package secret import ( "fmt" + "strconv" "sync" "time" @@ -32,6 +33,10 @@ import ( "k8s.io/client-go/util/clock" ) +const ( + defaultTTL = time.Minute +) + type Manager interface { // Get secret by secret namespace and name. GetSecret(namespace, name string) (*v1.Secret, error) @@ -67,6 +72,8 @@ func (s *simpleSecretManager) RegisterPod(pod *v1.Pod) { func (s *simpleSecretManager) UnregisterPod(pod *v1.Pod) { } +type GetObjectTTLFunc func() (time.Duration, bool) + type objectKey struct { namespace string name string @@ -93,15 +100,18 @@ type secretStore struct { lock sync.Mutex items map[objectKey]*secretStoreItem - ttl time.Duration + + defaultTTL time.Duration + getTTL GetObjectTTLFunc } -func newSecretStore(kubeClient clientset.Interface, clock clock.Clock, ttl time.Duration) *secretStore { +func newSecretStore(kubeClient clientset.Interface, clock clock.Clock, getTTL GetObjectTTLFunc, ttl time.Duration) *secretStore { return &secretStore{ kubeClient: kubeClient, clock: clock, items: make(map[objectKey]*secretStoreItem), - ttl: ttl, + defaultTTL: ttl, + getTTL: getTTL, } } @@ -149,6 +159,31 @@ func (s *secretStore) Delete(namespace, name string) { } } +func GetObjectTTLFromNodeFunc(getNode func() (*v1.Node, error)) GetObjectTTLFunc { + return func() (time.Duration, bool) { + node, err := getNode() + if err != nil { + return time.Duration(0), false + } + if node != nil && node.Annotations != nil { + if value, ok := node.Annotations[v1.ObjectTTLAnnotationKey]; ok { + if intValue, err := strconv.Atoi(value); err == nil { + return time.Duration(intValue) * time.Second, true + } + } + } + return time.Duration(0), false + } +} + +func (s *secretStore) isSecretFresh(data *secretData) bool { + secretTTL := s.defaultTTL + if ttl, ok := s.getTTL(); ok { + secretTTL = ttl + } + return s.clock.Now().Before(data.lastUpdateTime.Add(secretTTL)) +} + func (s *secretStore) Get(namespace, name string) (*v1.Secret, error) { key := objectKey{namespace: namespace, name: name} @@ -172,7 +207,7 @@ func (s *secretStore) Get(namespace, name string) (*v1.Secret, error) { // needed and return data. data.Lock() defer data.Unlock() - if data.err != nil || !s.clock.Now().Before(data.lastUpdateTime.Add(s.ttl)) { + if data.err != nil || !s.isSecretFresh(data) { opts := metav1.GetOptions{} if data.secret != nil && data.err == nil { // This is just a periodic refresh of a secret we successfully fetched previously. @@ -212,9 +247,9 @@ type cachingSecretManager struct { registeredPods map[objectKey]*v1.Pod } -func NewCachingSecretManager(kubeClient clientset.Interface) (Manager, error) { +func NewCachingSecretManager(kubeClient clientset.Interface, getTTL GetObjectTTLFunc) (Manager, error) { csm := &cachingSecretManager{ - secretStore: newSecretStore(kubeClient, clock.RealClock{}, time.Minute), + secretStore: newSecretStore(kubeClient, clock.RealClock{}, getTTL, defaultTTL), registeredPods: make(map[objectKey]*v1.Pod), } return csm, nil diff --git a/pkg/kubelet/secret/secret_manager_test.go b/pkg/kubelet/secret/secret_manager_test.go index 1fee59897d2..b809d23b94b 100644 --- a/pkg/kubelet/secret/secret_manager_test.go +++ b/pkg/kubelet/secret/secret_manager_test.go @@ -46,9 +46,13 @@ func checkSecret(t *testing.T, store *secretStore, ns, name string, shouldExist } } +func noObjectTTL() (time.Duration, bool) { + return time.Duration(0), false +} + func TestSecretStore(t *testing.T) { fakeClient := &fake.Clientset{} - store := newSecretStore(fakeClient, clock.RealClock{}, 0) + store := newSecretStore(fakeClient, clock.RealClock{}, noObjectTTL, 0) store.Add("ns1", "name1") store.Add("ns2", "name2") store.Add("ns1", "name1") @@ -82,7 +86,7 @@ func TestSecretStore(t *testing.T) { func TestSecretStoreDeletingSecret(t *testing.T) { fakeClient := &fake.Clientset{} - store := newSecretStore(fakeClient, clock.RealClock{}, 0) + store := newSecretStore(fakeClient, clock.RealClock{}, noObjectTTL, 0) store.Add("ns", "name") result := &v1.Secret{ObjectMeta: metav1.ObjectMeta{Namespace: "ns", Name: "name", ResourceVersion: "10"}} @@ -112,7 +116,7 @@ func TestSecretStoreDeletingSecret(t *testing.T) { func TestSecretStoreGetAlwaysRefresh(t *testing.T) { fakeClient := &fake.Clientset{} fakeClock := clock.NewFakeClock(time.Now()) - store := newSecretStore(fakeClient, fakeClock, 0) + store := newSecretStore(fakeClient, fakeClock, noObjectTTL, 0) for i := 0; i < 10; i++ { store.Add(fmt.Sprintf("ns-%d", i), fmt.Sprintf("name-%d", i)) @@ -139,7 +143,7 @@ func TestSecretStoreGetAlwaysRefresh(t *testing.T) { func TestSecretStoreGetNeverRefresh(t *testing.T) { fakeClient := &fake.Clientset{} fakeClock := clock.NewFakeClock(time.Now()) - store := newSecretStore(fakeClient, fakeClock, time.Minute) + store := newSecretStore(fakeClient, fakeClock, noObjectTTL, time.Minute) for i := 0; i < 10; i++ { store.Add(fmt.Sprintf("ns-%d", i), fmt.Sprintf("name-%d", i)) @@ -160,6 +164,131 @@ func TestSecretStoreGetNeverRefresh(t *testing.T) { assert.Equal(t, 10, len(actions), "unexpected actions: %#v", actions) } +func TestCustomTTL(t *testing.T) { + ttl := time.Duration(0) + ttlExists := false + customTTL := func() (time.Duration, bool) { + return ttl, ttlExists + } + + fakeClient := &fake.Clientset{} + fakeClock := clock.NewFakeClock(time.Time{}) + store := newSecretStore(fakeClient, fakeClock, customTTL, time.Minute) + + store.Add("ns", "name") + store.Get("ns", "name") + fakeClient.ClearActions() + + // Set 0-ttl and see if that works. + ttl = time.Duration(0) + ttlExists = true + store.Get("ns", "name") + actions := fakeClient.Actions() + assert.Equal(t, 1, len(actions), "unexpected actions: %#v", actions) + fakeClient.ClearActions() + + // Set 5-minute ttl and see if this works. + ttl = time.Duration(5) * time.Minute + store.Get("ns", "name") + actions = fakeClient.Actions() + assert.Equal(t, 0, len(actions), "unexpected actions: %#v", actions) + // Still no effect after 4 minutes. + fakeClock.Step(4 * time.Minute) + store.Get("ns", "name") + actions = fakeClient.Actions() + assert.Equal(t, 0, len(actions), "unexpected actions: %#v", actions) + // Now it should have an effect. + fakeClock.Step(time.Minute) + store.Get("ns", "name") + actions = fakeClient.Actions() + assert.Equal(t, 1, len(actions), "unexpected actions: %#v", actions) + fakeClient.ClearActions() + + // Now remove the custom ttl and see if that works. + ttlExists = false + fakeClock.Step(55 * time.Second) + store.Get("ns", "name") + actions = fakeClient.Actions() + assert.Equal(t, 0, len(actions), "unexpected actions: %#v", actions) + // Pass the minute and it should be triggered now. + fakeClock.Step(5 * time.Second) + store.Get("ns", "name") + actions = fakeClient.Actions() + assert.Equal(t, 1, len(actions), "unexpected actions: %#v", actions) +} + +func TestParseNodeAnnotation(t *testing.T) { + testCases := []struct { + node *v1.Node + err error + exists bool + ttl time.Duration + }{ + { + node: nil, + err: fmt.Errorf("error"), + exists: false, + }, + { + node: &v1.Node{ + ObjectMeta: metav1.ObjectMeta{ + Name: "node", + }, + }, + exists: false, + }, + { + node: &v1.Node{ + ObjectMeta: metav1.ObjectMeta{ + Name: "node", + Annotations: map[string]string{}, + }, + }, + exists: false, + }, + { + node: &v1.Node{ + ObjectMeta: metav1.ObjectMeta{ + Name: "node", + Annotations: map[string]string{v1.ObjectTTLAnnotationKey: "bad"}, + }, + }, + exists: false, + }, + { + node: &v1.Node{ + ObjectMeta: metav1.ObjectMeta{ + Name: "node", + Annotations: map[string]string{v1.ObjectTTLAnnotationKey: "0"}, + }, + }, + exists: true, + ttl: time.Duration(0), + }, + { + node: &v1.Node{ + ObjectMeta: metav1.ObjectMeta{ + Name: "node", + Annotations: map[string]string{v1.ObjectTTLAnnotationKey: "60"}, + }, + }, + exists: true, + ttl: time.Minute, + }, + } + for i, testCase := range testCases { + getNode := func() (*v1.Node, error) { return testCase.node, testCase.err } + ttl, exists := GetObjectTTLFromNodeFunc(getNode)() + if exists != testCase.exists { + t.Errorf("%d: incorrect parsing: %t", i, exists) + continue + } + if exists && ttl != testCase.ttl { + t.Errorf("%d: incorrect ttl: %v", i, ttl) + } + } +} + type envSecrets struct { envVarNames []string envFromNames []string @@ -215,7 +344,7 @@ func podWithSecrets(ns, name string, toAttach secretsToAttach) *v1.Pod { func TestCacheInvalidation(t *testing.T) { fakeClient := &fake.Clientset{} fakeClock := clock.NewFakeClock(time.Now()) - store := newSecretStore(fakeClient, fakeClock, time.Minute) + store := newSecretStore(fakeClient, fakeClock, noObjectTTL, time.Minute) manager := &cachingSecretManager{ secretStore: store, registeredPods: make(map[objectKey]*v1.Pod), @@ -273,7 +402,7 @@ func TestCacheInvalidation(t *testing.T) { func TestCacheRefcounts(t *testing.T) { fakeClient := &fake.Clientset{} fakeClock := clock.NewFakeClock(time.Now()) - store := newSecretStore(fakeClient, fakeClock, time.Minute) + store := newSecretStore(fakeClient, fakeClock, noObjectTTL, time.Minute) manager := &cachingSecretManager{ secretStore: store, registeredPods: make(map[objectKey]*v1.Pod), @@ -349,7 +478,7 @@ func TestCacheRefcounts(t *testing.T) { func TestCachingSecretManager(t *testing.T) { fakeClient := &fake.Clientset{} - secretStore := newSecretStore(fakeClient, clock.RealClock{}, 0) + secretStore := newSecretStore(fakeClient, clock.RealClock{}, noObjectTTL, 0) manager := &cachingSecretManager{ secretStore: secretStore, registeredPods: make(map[objectKey]*v1.Pod),