Merge pull request kubernetes#25913 from pweil-/ds-tombstone

Automatic merge from submit-queue daemonset handle DeletedFinalStateUnknown During an e2e run in OpenShift we ran into the DS controller panic when handling `DeletedFinalStateUnknown`. This PR checks for `DeletedFinalStateUnknown` and queues the embedded object if it is a `DaemonSet`. @mikedanese - would you mind taking a look? @deads2k ``` panic: interface conversion: interface is cache.DeletedFinalStateUnknown, not *extensions.DaemonSet goroutine 4369 [running]: k8s.io/kubernetes/pkg/controller/daemon.func·005(0x2f8a0c0, 0xc20b559680) /data/src/github.com/openshift/origin/Godeps/_workspace/src/k8s.io/kubernetes/pkg/controller/daemon/controller.go:160 +0x50 k8s.io/kubernetes/pkg/controller/framework.ResourceEventHandlerFuncs.OnDelete(0xc20a0ae090, 0xc20a0ae0a0, 0xc20a0ae0b0, 0x2f8a0c0, 0xc20b559680) /data/src/github.com/openshift/origin/Godeps/_workspace/src/k8s.io/kubernetes/pkg/controller/framework/controller.go:178 +0x41 k8s.io/kubernetes/pkg/controller/framework.(*ResourceEventHandlerFuncs).OnDelete(0xc20b8ebf20, 0x2f8a0c0, 0xc20b559680) <autogenerated>:25 +0xb5 k8s.io/kubernetes/pkg/controller/framework.func·001(0x2f8a280, 0xc20b5522e0, 0x0, 0x0) /data/src/github.com/openshift/origin/Godeps/_workspace/src/k8s.io/kubernetes/pkg/controller/framework/controller.go:248 +0x4be k8s.io/kubernetes/pkg/controller/framework.(*Controller).processLoop(0xc20bb727e0) /data/src/github.com/openshift/origin/Godeps/_workspace/src/k8s.io/kubernetes/pkg/controller/framework/controller.go:122 +0x6f k8s.io/kubernetes/pkg/controller/framework.*Controller.(k8s.io/kubernetes/pkg/controller/framework.processLoop)·fm() /data/src/github.com/openshift/origin/Godeps/_workspace/src/k8s.io/kubernetes/pkg/controller/framework/controller.go:97 +0x27 k8s.io/kubernetes/pkg/util/wait.func·001() /data/src/github.com/openshift/origin/Godeps/_workspace/src/k8s.io/kubernetes/pkg/util/wait/wait.go:66 +0x61 k8s.io/kubernetes/pkg/util/wait.JitterUntil(0xc209f8cfb8, 0x3b9aca00, 0x0, 0xc2080543c0) /data/src/github.com/openshift/origin/Godeps/_workspace/src/k8s.io/kubernetes/pkg/util/wait/wait.go:67 +0x8f k8s.io/kubernetes/pkg/util/wait.Until(0xc209f8cfb8, 0x3b9aca00, 0xc2080543c0) /data/src/github.com/openshift/origin/Godeps/_workspace/src/k8s.io/kubernetes/pkg/util/wait/wait.go:47 +0x4a k8s.io/kubernetes/pkg/controller/framework.(*Controller).Run(0xc20bb727e0, 0xc2080543c0) /data/src/github.com/openshift/origin/Godeps/_workspace/src/k8s.io/kubernetes/pkg/controller/framework/controller.go:97 +0x1fb created by k8s.io/kubernetes/pkg/controller/daemon.(*DaemonSetsController).Run /data/src/github.com/openshift/origin/Godeps/_workspace/src/k8s.io/kubernetes/pkg/controller/daemon/controller.go:212 +0xae ``` https://ci.openshift.redhat.com/jenkins/job/test_pull_requests_origin_check/1002/artifact/origin/artifacts/test-cmd/logs/openshift.log
ajwdev · May 31, 2016 · d1277e3 · d1277e3
2 parents 8a29f67 + 4d6fee7
commit d1277e3
Show file tree

Hide file tree

Showing 2 changed files with 31 additions and 5 deletions.
diff --git a/pkg/controller/daemon/controller.go b/pkg/controller/daemon/controller.go
@@ -168,11 +168,7 @@ func NewDaemonSetsController(podInformer framework.SharedIndexInformer, kubeClie
 				glog.V(4).Infof("Updating daemon set %s", oldDS.Name)
 				dsc.enqueueDaemonSet(curDS)
 			},
-			DeleteFunc: func(obj interface{}) {
-				ds := obj.(*extensions.DaemonSet)
-				glog.V(4).Infof("Deleting daemon set %s", ds.Name)
-				dsc.enqueueDaemonSet(ds)
-			},
+			DeleteFunc: dsc.deleteDaemonset,
 		},
 	)
 
@@ -217,6 +213,24 @@ func NewDaemonSetsControllerFromClient(kubeClient clientset.Interface, resyncPer
 	return dsc
 }
 
+func (dsc *DaemonSetsController) deleteDaemonset(obj interface{}) {
+	ds, ok := obj.(*extensions.DaemonSet)
+	if !ok {
+		tombstone, ok := obj.(cache.DeletedFinalStateUnknown)
+		if !ok {
+			glog.Errorf("Couldn't get object from tombstone %+v", obj)
+			return
+		}
+		ds, ok = tombstone.Obj.(*extensions.DaemonSet)
+		if !ok {
+			glog.Errorf("Tombstone contained object that is not a DaemonSet %+v", obj)
+			return
+		}
+	}
+	glog.V(4).Infof("Deleting daemon set %s", ds.Name)
+	dsc.enqueueDaemonSet(ds)
+}
+
 // Run begins watching and syncing daemon sets.
 func (dsc *DaemonSetsController) Run(workers int, stopCh <-chan struct{}) {
 	defer utilruntime.HandleCrash()

diff --git a/pkg/controller/daemon/controller_test.go b/pkg/controller/daemon/controller_test.go
@@ -158,6 +158,18 @@ func syncAndValidateDaemonSets(t *testing.T, manager *DaemonSetsController, ds *
 	validateSyncDaemonSets(t, podControl, expectedCreates, expectedDeletes)
 }
 
+func TestDeleteFinalStateUnknown(t *testing.T) {
+	manager, _ := newTestController()
+	addNodes(manager.nodeStore.Store, 0, 1, nil)
+	ds := newDaemonSet("foo")
+	// DeletedFinalStateUnknown should queue the embedded DS if found.
+	manager.deleteDaemonset(cache.DeletedFinalStateUnknown{Key: "foo", Obj: ds})
+	enqueuedKey, _ := manager.queue.Get()
+	if enqueuedKey.(string) != "default/foo" {
+		t.Errorf("expected delete of DeletedFinalStateUnknown to enqueue the daemonset but found: %#v", enqueuedKey)
+	}
+}
+
 // DaemonSets without node selectors should launch pods on every node.
 func TestSimpleDaemonSetLaunchesPods(t *testing.T) {
 	manager, podControl := newTestController()