diff --git a/pkg/controller/daemon/controller.go b/pkg/controller/daemon/controller.go index 96c10514881..50a5293deaa 100644 --- a/pkg/controller/daemon/controller.go +++ b/pkg/controller/daemon/controller.go @@ -65,8 +65,9 @@ const ( // DaemonSetsController is responsible for synchronizing DaemonSet objects stored // in the system with actual running pods. type DaemonSetsController struct { - kubeClient clientset.Interface - podControl controller.PodControlInterface + kubeClient clientset.Interface + eventRecorder record.EventRecorder + podControl controller.PodControlInterface // An dsc is temporarily suspended after creating/deleting these many replicas. // It resumes normal action after observing the watch events for them. @@ -105,7 +106,8 @@ func NewDaemonSetsController(kubeClient clientset.Interface, resyncPeriod contro eventBroadcaster.StartRecordingToSink(&unversionedcore.EventSinkImpl{kubeClient.Core().Events("")}) dsc := &DaemonSetsController{ - kubeClient: kubeClient, + kubeClient: kubeClient, + eventRecorder: eventBroadcaster.NewRecorder(api.EventSource{Component: "daemonset-controller"}), podControl: controller.RealPodControl{ KubeClient: kubeClient, Recorder: eventBroadcaster.NewRecorder(api.EventSource{Component: "daemon-set"}), @@ -131,7 +133,7 @@ func NewDaemonSetsController(kubeClient clientset.Interface, resyncPeriod contro AddFunc: func(obj interface{}) { ds := obj.(*extensions.DaemonSet) glog.V(4).Infof("Adding daemon set %s", ds.Name) - dsc.enqueueDaemonSet(obj) + dsc.enqueueDaemonSet(ds) }, UpdateFunc: func(old, cur interface{}) { oldDS := old.(*extensions.DaemonSet) @@ -152,12 +154,12 @@ func NewDaemonSetsController(kubeClient clientset.Interface, resyncPeriod contro } glog.V(4).Infof("Updating daemon set %s", oldDS.Name) - dsc.enqueueDaemonSet(cur) + dsc.enqueueDaemonSet(curDS) }, DeleteFunc: func(obj interface{}) { ds := obj.(*extensions.DaemonSet) glog.V(4).Infof("Deleting daemon set %s", ds.Name) - dsc.enqueueDaemonSet(obj) + dsc.enqueueDaemonSet(ds) }, }, ) @@ -246,10 +248,10 @@ func (dsc *DaemonSetsController) enqueueAllDaemonSets() { } } -func (dsc *DaemonSetsController) enqueueDaemonSet(obj interface{}) { - key, err := controller.KeyFunc(obj) +func (dsc *DaemonSetsController) enqueueDaemonSet(ds *extensions.DaemonSet) { + key, err := controller.KeyFunc(ds) if err != nil { - glog.Errorf("Couldn't get key for object %+v: %v", obj, err) + glog.Errorf("Couldn't get key for object %+v: %v", ds, err) return } @@ -623,6 +625,12 @@ func (dsc *DaemonSetsController) syncDaemonSet(key string) error { } ds := obj.(*extensions.DaemonSet) + everything := unversioned.LabelSelector{} + if reflect.DeepEqual(ds.Spec.Selector, &everything) { + dsc.eventRecorder.Eventf(ds, api.EventTypeWarning, "SelectingAll", "This controller is selecting all pods. Skipping sync.") + return nil + } + // Don't process a daemon set until all its creations and deletions have been processed. // For example if daemon set foo asked for 3 new daemon pods in the previous call to manage, // then we do not want to call manage on foo until the daemon pods have been created. diff --git a/pkg/controller/daemon/controller_test.go b/pkg/controller/daemon/controller_test.go index 7728b4c07cc..861f9df8304 100644 --- a/pkg/controller/daemon/controller_test.go +++ b/pkg/controller/daemon/controller_test.go @@ -167,7 +167,7 @@ func TestSimpleDaemonSetLaunchesPods(t *testing.T) { syncAndValidateDaemonSets(t, manager, ds, podControl, 5, 0) } -// DaemonSets without node selectors should launch pods on every node. +// DaemonSets should do nothing if there aren't any nodes func TestNoNodesDoesNothing(t *testing.T) { manager, podControl := newTestController() ds := newDaemonSet("foo") @@ -175,7 +175,8 @@ func TestNoNodesDoesNothing(t *testing.T) { syncAndValidateDaemonSets(t, manager, ds, podControl, 0, 0) } -// DaemonSets without node selectors should launch pods on every node. +// DaemonSets without node selectors should launch on a single node in a +// single node cluster. func TestOneNodeDaemonLaunchesPod(t *testing.T) { manager, podControl := newTestController() manager.nodeStore.Add(newNode("only-node", nil)) @@ -350,6 +351,41 @@ func TestNoPortConflictNodeDaemonLaunchesPod(t *testing.T) { syncAndValidateDaemonSets(t, manager, ds, podControl, 1, 0) } +// DaemonSetController should not sync DaemonSets with empty pod selectors. +// +// issue https://github.com/kubernetes/kubernetes/pull/23223 +func TestPodIsNotDeletedByDaemonsetWithEmptyLabelSelector(t *testing.T) { + manager, podControl := newTestController() + manager.nodeStore.Store.Add(newNode("node1", nil)) + // Create pod not controlled by a daemonset. + manager.podStore.Add(&api.Pod{ + ObjectMeta: api.ObjectMeta{ + Labels: map[string]string{"bang": "boom"}, + Namespace: api.NamespaceDefault, + }, + Spec: api.PodSpec{ + NodeName: "node1", + }, + }) + + // Create a misconfigured DaemonSet. An empty pod selector is invalid but could happen + // if we upgrade and make a backwards incompatible change. + // + // The node selector matches no nodes which mimics the behavior of kubectl delete. + // + // The DaemonSet should not schedule pods and should not delete scheduled pods in + // this case even though it's empty pod selector matches all pods. The DaemonSetController + // should detect this misconfiguration and choose not to sync the DaemonSet. We should + // not observe a deletion of the pod on node1. + ds := newDaemonSet("foo") + ls := unversioned.LabelSelector{} + ds.Spec.Selector = &ls + ds.Spec.Template.Spec.NodeSelector = map[string]string{"foo": "bar"} + manager.dsStore.Add(ds) + + syncAndValidateDaemonSets(t, manager, ds, podControl, 0, 0) +} + // Controller should not create pods on nodes which have daemon pods, and should remove excess pods from nodes that have extra pods. func TestDealsWithExistingPods(t *testing.T) { manager, podControl := newTestController()