From 560c8300ec7733080554d3048d4937b15b58889a Mon Sep 17 00:00:00 2001 From: "Dr. Stefan Schimanski" Date: Wed, 14 Oct 2015 14:22:13 +0200 Subject: [PATCH] Enforce an initial empty SET PodConfig In PodConfigNotificationIncremental PodConfig mode, when no pods are available for a source, the Merge function correctly concluded that neither ADD, UPDATE nor REMOVE updates are to be sent to the kubelet. But as a consequence the kubelet will not mark that source as seen. This is usually not a problem for the apiserver source. But it is a problem for an empty "file" source, e.g. by passing an empty directory to the kubelet for static pods. Then the file source will never be seen and the kubelet will stay in its special not-all-source-seen mode. --- pkg/kubelet/config/config.go | 12 +++++++----- pkg/kubelet/config/config_test.go | 25 +++++++++++++++++++++++++ 2 files changed, 32 insertions(+), 5 deletions(-) diff --git a/pkg/kubelet/config/config.go b/pkg/kubelet/config/config.go index 99a08b69d3c6..1d6fc9cff26e 100644 --- a/pkg/kubelet/config/config.go +++ b/pkg/kubelet/config/config.go @@ -153,7 +153,9 @@ func (s *podStorage) Merge(source string, change interface{}) error { s.updateLock.Lock() defer s.updateLock.Unlock() + seenBefore := s.sourcesSeen.Has(source) adds, updates, deletes := s.merge(source, change) + firstSet := !seenBefore && s.sourcesSeen.Has(source) // deliver update notifications switch s.mode { @@ -161,7 +163,7 @@ func (s *podStorage) Merge(source string, change interface{}) error { if len(deletes.Pods) > 0 { s.updates <- *deletes } - if len(adds.Pods) > 0 { + if len(adds.Pods) > 0 || firstSet { s.updates <- *adds } if len(updates.Pods) > 0 { @@ -169,15 +171,15 @@ func (s *podStorage) Merge(source string, change interface{}) error { } case PodConfigNotificationSnapshotAndUpdates: + if len(deletes.Pods) > 0 || len(adds.Pods) > 0 || firstSet { + s.updates <- kubelet.PodUpdate{Pods: s.MergedState().([]*api.Pod), Op: kubelet.SET, Source: source} + } if len(updates.Pods) > 0 { s.updates <- *updates } - if len(deletes.Pods) > 0 || len(adds.Pods) > 0 { - s.updates <- kubelet.PodUpdate{Pods: s.MergedState().([]*api.Pod), Op: kubelet.SET, Source: source} - } case PodConfigNotificationSnapshot: - if len(updates.Pods) > 0 || len(deletes.Pods) > 0 || len(adds.Pods) > 0 { + if len(updates.Pods) > 0 || len(deletes.Pods) > 0 || len(adds.Pods) > 0 || firstSet { s.updates <- kubelet.PodUpdate{Pods: s.MergedState().([]*api.Pod), Op: kubelet.SET, Source: source} } diff --git a/pkg/kubelet/config/config_test.go b/pkg/kubelet/config/config_test.go index b2db9114d783..05370c5e7788 100644 --- a/pkg/kubelet/config/config_test.go +++ b/pkg/kubelet/config/config_test.go @@ -267,6 +267,31 @@ func TestNewPodAddedUpdatedSet(t *testing.T) { CreatePodUpdate(kubelet.UPDATE, TestSource, pod)) } +func TestInitialEmptySet(t *testing.T) { + for _, test := range []struct { + mode PodConfigNotificationMode + op kubelet.PodOperation + }{ + {PodConfigNotificationIncremental, kubelet.ADD}, + {PodConfigNotificationSnapshot, kubelet.SET}, + {PodConfigNotificationSnapshotAndUpdates, kubelet.SET}, + } { + channel, ch, _ := createPodConfigTester(test.mode) + + // should register an empty PodUpdate operation + podUpdate := CreatePodUpdate(kubelet.SET, TestSource) + channel <- podUpdate + expectPodUpdate(t, ch, CreatePodUpdate(test.op, TestSource)) + + // should ignore following empty sets + podUpdate = CreatePodUpdate(kubelet.SET, TestSource) + channel <- podUpdate + podUpdate = CreatePodUpdate(kubelet.ADD, TestSource, CreateValidPod("foo", "new")) + channel <- podUpdate + expectPodUpdate(t, ch, CreatePodUpdate(test.op, TestSource, CreateValidPod("foo", "new"))) + } +} + func TestPodUpdateAnnotations(t *testing.T) { channel, ch, _ := createPodConfigTester(PodConfigNotificationIncremental)