fix(controller): Correct fail workflow when pod is deleted with --for…

…ce. Fixes #3097 (#3469)
argoproj · Jul 14, 2020 · 675ce29 · 675ce29
1 parent 194a213
commit 675ce29
Show file tree

Hide file tree

Showing 3 changed files with 19 additions and 8 deletions.
diff --git a/workflow/controller/controller.go b/workflow/controller/controller.go
@@ -644,30 +644,34 @@ func (wfc *WorkflowController) newPodInformer() cache.SharedIndexInformer {
 		cache.ResourceEventHandlerFuncs{
 			AddFunc: func(obj interface{}) {
 				key, err := cache.MetaNamespaceKeyFunc(obj)
-				if err == nil {
-					wfc.podQueue.Add(key)
+				if err != nil {
+					return
 				}
+				wfc.podQueue.Add(key)
 			},
 			UpdateFunc: func(old, new interface{}) {
+				key, err := cache.MetaNamespaceKeyFunc(new)
+				if err != nil {
+					return
+				}
 				oldPod, newPod := old.(*apiv1.Pod), new.(*apiv1.Pod)
 				if oldPod.ResourceVersion == newPod.ResourceVersion {
 					return
 				}
 				if !pod.SignificantPodChange(oldPod, newPod) {
+					log.WithField("key", key).Info("insignificant pod change")
 					return
 				}
-				key, err := cache.MetaNamespaceKeyFunc(new)
-				if err == nil {
-					wfc.podQueue.Add(key)
-				}
+				wfc.podQueue.Add(key)
 			},
 			DeleteFunc: func(obj interface{}) {
 				// IndexerInformer uses a delta queue, therefore for deletes we have to use this
 				// key function.
 				key, err := cache.DeletionHandlingMetaNamespaceKeyFunc(obj)
-				if err == nil {
-					wfc.podQueue.Add(key)
+				if err != nil {
+					return
 				}
+				wfc.podQueue.Add(key)
 			},
 		},
 	)

diff --git a/workflow/controller/pod/significant.go b/workflow/controller/pod/significant.go
@@ -9,6 +9,7 @@ func SignificantPodChange(from *apiv1.Pod, to *apiv1.Pod) bool {
 		from.Status.Phase != to.Status.Phase ||
 		from.Status.Message != to.Status.Message ||
 		from.Status.PodIP != to.Status.PodIP ||
+		from.GetDeletionTimestamp() != to.GetDeletionTimestamp() ||
 		significantContainerStatusesChange(from.Status.ContainerStatuses, to.Status.ContainerStatuses) ||
 		significantContainerStatusesChange(from.Status.InitContainerStatuses, to.Status.InitContainerStatuses)
 }

diff --git a/workflow/controller/pod/significant_test.go b/workflow/controller/pod/significant_test.go
@@ -5,10 +5,16 @@ import (
 
 	"github.com/stretchr/testify/assert"
 	corev1 "k8s.io/api/core/v1"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 )
 
 func Test_SgnificantPodChange(t *testing.T) {
 	assert.False(t, SignificantPodChange(&corev1.Pod{}, &corev1.Pod{}), "No change")
+
+	t.Run("DeletionTimestamp", func(t *testing.T) {
+		now := metav1.Now()
+		assert.True(t, SignificantPodChange(&corev1.Pod{}, &corev1.Pod{ObjectMeta: metav1.ObjectMeta{DeletionTimestamp: &now}}), "deletion timestamp change")
+	})
 	t.Run("Spec", func(t *testing.T) {
 		assert.True(t, SignificantPodChange(&corev1.Pod{}, &corev1.Pod{Spec: corev1.PodSpec{NodeName: "from"}}), "Node name change")