Repository: argoproj/argo (go)

Issue:
Enable retryStrategy to be specified for non-leaf nodes
# Summary

Enable retryStrategy to be specified for non-leaf nodes, so for example it can be applied to a set of steps and they will all be retried if any fail.

# Motivation

For example if there are a pair of steps when one fetches some data and another processes the data, if the processing fails then this is useful if we want to retry the fetch too.

# Proposal

https://github.com/argoproj/argo/pull/1892

Pull request:
feat: Allow retry strategy on non-leaf nodes, eg for step groups. Fixes #1891
Checklist:

* [X] Either (a) I've created an [enhancement proposal](https://github.com/argoproj/argo/issues/new/choose) and discussed it with the community, (b) this is a bug fix, or (c) this does not need to be in the release notes.
* [X] The title of the PR is (a) [conventional](https://www.conventionalcommits.org/en/v1.0.0/), (b) states what changed, and (c) suffixes the related issues number. E.g. `"fix(controller): Updates such and such. Fixes #1234"`. Why? for the release notes. 
* [X] Optional. I've added My organization is added to the README.
* [X] I've signed the CLA and required builds are green. 

Fixes #1891

Patch:
diff --git a/api/openapi-spec/swagger.json b/api/openapi-spec/swagger.json
index 7bf0c0a07..fae466187 100644
--- a/api/openapi-spec/swagger.json
+++ b/api/openapi-spec/swagger.json
@@ -848,6 +848,12 @@
             "description": "The continue option should be set when retrieving more results from the server. Since this value is\nserver defined, clients may only use the continue value from a previous query result with identical\nquery parameters (except for the value of continue) and the server may reject a continue value it\ndoes not recognize. If the specified continue value is no longer valid whether due to expiration\n(generally five to fifteen minutes) or a configuration change on the server, the server will\nrespond with a 410 ResourceExpired error together with a continue token. If the client needs a\nconsistent list, it must restart their list without the continue field. Otherwise, the client may\nsend another list request with the token received with the 410 error, the server will respond with\na list starting from the next key, but from the latest snapshot, which is inconsistent from the\nprevious list results - objects that are created, modified, or deleted after the first list request\nwill be included in the response, as long as their keys are after the \"next key\".\n\nThis field is not supported when watch is true. Clients may start a watch from the last\nresourceVersion value returned by the server and not miss any modifications.",
             "name": "listOptions.continue",
             "in": "query"
+          },
+          {
+            "type": "string",
+            "description": "Fields to be included or excluded in the response. e.g. \"items.spec,items.status.phase\", \"-items.status.nodes\".",
+            "name": "fields",
+            "in": "query"
           }
         ],
         "responses": {
@@ -946,6 +952,12 @@
             "description": "When specified:\n- if unset, then the result is returned from remote storage based on quorum-read flag;\n- if it's 0, then we simply return what we currently have in cache, no guarantee;\n- if set to non zero, then the result is at least as fresh as given rv.",
             "name": "getOptions.resourceVersion",
             "in": "query"
+          },
+          {
+            "type": "string",
+            "description": "Fields to be included or excluded in the response. e.g. \"spec,status.phase\", \"-status.nodes\".",
+            "name": "fields",
+            "in": "query"
           }
         ],
         "responses": {
diff --git a/go.mod b/go.mod
index f4b609a76..5e890611c 100644
--- a/go.mod
+++ b/go.mod
@@ -6,7 +6,7 @@ require (
 	cloud.google.com/go v0.51.0 // indirect
 	github.com/Knetic/govaluate v3.0.1-0.20171022003610-9aa49832a739+incompatible
 	github.com/ajg/form v1.5.1 // indirect
-	github.com/argoproj/pkg v0.0.0-20200102163130-2dd1f3f6b4de
+	github.com/argoproj/pkg v0.0.0-20200226231057-d69f0b04da65
 	github.com/aws/aws-sdk-go v1.27.1 // indirect
 	github.com/beorn7/perks v1.0.1 // indirect
 	github.com/colinmarc/hdfs v1.1.4-0.20180805212432-9746310a4d31
diff --git a/go.sum b/go.sum
index ba4baacd5..de0c6872c 100644
--- a/go.sum
+++ b/go.sum
@@ -35,6 +35,8 @@ github.com/anmitsu/go-shlex v0.0.0-20161002113705-648efa622239/go.mod h1:2FmKhYU
 github.com/antihax/optional v0.0.0-20180407024304-ca021399b1a6/go.mod h1:V8iCPQYkqmusNa815XgQio277wI47sdRh1dUOLdyC6Q=
 github.com/argoproj/pkg v0.0.0-20200102163130-2dd1f3f6b4de h1:i19Yd0ERYAHtR84jnB8vmTuizk2q4Qw4NuayfRja5/A=
 github.com/argoproj/pkg v0.0.0-20200102163130-2dd1f3f6b4de/go.mod h1:2EZ44RG/CcgtPTwrRR0apOc7oU6UIw8GjCUJWZ8X3bM=
+github.com/argoproj/pkg v0.0.0-20200226231057-d69f0b04da65 h1:bBC9exkuWvqhyDDhoNr66BpE6B4f+vxxKIXKxg6E6us=
+github.com/argoproj/pkg v0.0.0-20200226231057-d69f0b04da65/go.mod h1:2EZ44RG/CcgtPTwrRR0apOc7oU6UIw8GjCUJWZ8X3bM=
 github.com/armon/go-socks5 v0.0.0-20160902184237-e75332964ef5 h1:0CwZNZbxp69SHPdPJAN/hZIm0C4OItdklCFmMRWYpio=
 github.com/armon/go-socks5 v0.0.0-20160902184237-e75332964ef5/go.mod h1:wHh0iHkYZB8zMSxRWpUBQtwG5a7fFgvEO+odwuTv2gs=
 github.com/aws/aws-sdk-go v1.27.1 h1:MXnqY6SlWySaZAqNnXThOvjRFdiiOuKtC6i7baFdNdU=
diff --git a/pkg/apiclient/workflow/forwarder_overwrite.go b/pkg/apiclient/workflow/forwarder_overwrite.go
index 98750c4c7..b9d097522 100644
--- a/pkg/apiclient/workflow/forwarder_overwrite.go
+++ b/pkg/apiclient/workflow/forwarder_overwrite.go
@@ -1,10 +1,12 @@
 package workflow
 
 import (
-	"github.com/argoproj/argo/util/http"
+	"github.com/argoproj/pkg/grpc/http"
 )
 
 func init() {
 	forward_WorkflowService_WatchWorkflows_0 = http.StreamForwarder
 	forward_WorkflowService_PodLogs_0 = http.StreamForwarder
+	forward_WorkflowService_ListWorkflows_0 = http.UnaryForwarder
+	forward_WorkflowService_GetWorkflow_0 = http.UnaryForwarder
 }
diff --git a/pkg/apiclient/workflow/workflow.proto b/pkg/apiclient/workflow/workflow.proto
index 781f795e3..c7c4d0001 100644
--- a/pkg/apiclient/workflow/workflow.proto
+++ b/pkg/apiclient/workflow/workflow.proto
@@ -24,11 +24,15 @@ message WorkflowGetRequest {
     string name = 1;
     string namespace = 2;
     k8s.io.apimachinery.pkg.apis.meta.v1.GetOptions getOptions = 3;
+    // Fields to be included or excluded in the response. e.g. "spec,status.phase", "-status.nodes"
+    string fields = 4;
 }
 
 message WorkflowListRequest {
     string namespace = 1;
     k8s.io.apimachinery.pkg.apis.meta.v1.ListOptions listOptions = 2;
+    // Fields to be included or excluded in the response. e.g. "items.spec,items.status.phase", "-items.status.nodes"
+    string fields = 3;
 }
 
 message WorkflowResubmitRequest {
diff --git a/pkg/apiclient/workflow/workflow.swagger.json b/pkg/apiclient/workflow/workflow.swagger.json
index 4f4861ae5..dc917dd94 100644
--- a/pkg/apiclient/workflow/workflow.swagger.json
+++ b/pkg/apiclient/workflow/workflow.swagger.json
@@ -173,6 +173,13 @@
             "in": "query",
             "required": false,
             "type": "string"
+          },
+          {
+            "name": "fields",
+            "description": "Fields to be included or excluded in the response. e.g. \"items.spec,items.status.phase\", \"-items.status.nodes\".",
+            "in": "query",
+            "required": false,
+            "type": "string"
           }
         ],
         "tags": [
@@ -272,6 +279,13 @@
             "in": "query",
             "required": false,
             "type": "string"
+          },
+          {
+            "name": "fields",
+            "description": "Fields to be included or excluded in the response. e.g. \"spec,status.phase\", \"-status.nodes\".",
+            "in": "query",
+            "required": false,
+            "type": "string"
           }
         ],
         "tags": [
diff --git a/util/http/forwarders.go b/util/http/forwarders.go
deleted file mode 100644
index 0ab0b3e47..000000000
--- a/util/http/forwarders.go
+++ /dev/null
@@ -1,48 +0,0 @@
-package http
-
-import (
-	"encoding/json"
-	"fmt"
-	"net/http"
-
-	"github.com/golang/protobuf/proto"
-	"github.com/grpc-ecosystem/grpc-gateway/runtime"
-	"golang.org/x/net/context"
-)
-
-type sseMarshaller struct {
-	runtime.Marshaler
-}
-
-func (m *sseMarshaller) ContentType() string {
-	return "text/event-stream"
-}
-
-func (m *sseMarshaller) Marshal(v interface{}) ([]byte, error) {
-	dataBytes, err := json.Marshal(v)
-	if err != nil {
-		return nil, err
-	}
-	return []byte(fmt.Sprintf("data: %s \n\n", string(dataBytes))), nil
-}
-
-var (
-	StreamForwarder = func(
-		ctx context.Context,
-		mux *runtime.ServeMux,
-		marshaler runtime.Marshaler,
-		w http.ResponseWriter,
-		req *http.Request,
-		recv func() (proto.Message, error),
-		opts ...func(context.Context, http.ResponseWriter, proto.Message) error,
-	) {
-		if req.Header.Get("Accept") == "text/event-stream" {
-			w.Header().Set("Content-Type", "text/event-stream")
-			w.Header().Set("Transfer-Encoding", "chunked")
-			w.Header().Set("X-Content-Type-Options", "nosniff")
-			runtime.ForwardResponseStream(ctx, mux, &sseMarshaller{marshaler}, w, req, recv, opts...)
-		} else {
-			runtime.ForwardResponseStream(ctx, mux, marshaler, w, req, recv, opts...)
-		}
-	}
-)
diff --git a/workflow/controller/operator.go b/workflow/controller/operator.go
index fcba59605..ca7aacd3c 100644
--- a/workflow/controller/operator.go
+++ b/workflow/controller/operator.go
@@ -624,9 +624,8 @@ func (woc *wfOperationCtx) processNodeRetries(node *wfv1.NodeStatus, retryStrate
 		if time.Now().Before(waitingDeadline) {
 			retryMessage := fmt.Sprintf("Retrying in %s", humanize.Duration(time.Until(waitingDeadline)))
 			return woc.markNodePhase(node.Name, node.Phase, retryMessage), false, nil
-		} else {
-			node = woc.markNodePhase(node.Name, node.Phase, "")
 		}
+		node = woc.markNodePhase(node.Name, node.Phase, "")
 	}
 
 	var retryOnFailed bool
@@ -1312,7 +1311,7 @@ func (woc *wfOperationCtx) executeTemplate(nodeName string, orgTmpl wfv1.Templat
 	// the container. The status of this node should be "Success" if any
 	// of the retries succeed. Otherwise, it is "Failed".
 	retryNodeName := ""
-	if processedTmpl.IsLeaf() && processedTmpl.RetryStrategy != nil {
+	if processedTmpl.RetryStrategy != nil {
 		retryNodeName = nodeName
 		retryParentNode := node
 		if retryParentNode == nil {
@@ -1337,19 +1336,20 @@ func (woc *wfOperationCtx) executeTemplate(nodeName string, orgTmpl wfv1.Templat
 		}
 		if lastChildNode != nil && !lastChildNode.Completed() {
 			// Last child node is still running.
-			return retryParentNode, nil
-		}
-
-		// Create a new child node and append it to the retry node.
-		nodeName = fmt.Sprintf("%s(%d)", retryNodeName, len(retryParentNode.Children))
-		woc.addChildNode(retryNodeName, nodeName)
-		node = nil
-
-		// Change the `pod.name` variable to the new retry node name
-		if processedTmpl.IsPodType() {
-			processedTmpl, err = common.SubstituteParams(processedTmpl, map[string]string{}, map[string]string{common.LocalVarPodName: woc.wf.NodeID(nodeName)})
-			if err != nil {
-				return woc.initializeNodeOrMarkError(node, nodeName, wfv1.NodeTypeSkipped, templateScope, orgTmpl, boundaryID, err), err
+			nodeName = lastChildNode.Name
+			node = lastChildNode
+		} else {
+			// Create a new child node and append it to the retry node.
+			nodeName = fmt.Sprintf("%s(%d)", retryNodeName, len(retryParentNode.Children))
+			woc.addChildNode(retryNodeName, nodeName)
+			node = nil
+
+			// Change the `pod.name` variable to the new retry node name
+			if processedTmpl.IsPodType() {
+				processedTmpl, err = common.SubstituteParams(processedTmpl, map[string]string{}, map[string]string{common.LocalVarPodName: woc.wf.NodeID(nodeName)})
+				if err != nil {
+					return woc.initializeNodeOrMarkError(node, nodeName, wfv1.NodeTypeSkipped, templateScope, orgTmpl, boundaryID, err), err
+				}
 			}
 		}
 	}
@@ -1383,9 +1383,16 @@ func (woc *wfOperationCtx) executeTemplate(nodeName string, orgTmpl wfv1.Templat
 	}
 	node = woc.getNodeByName(node.Name)
 
-	// Swap the node back to retry node.
+	// Swap the node back to retry node
 	if retryNodeName != "" {
-		node = woc.getNodeByName(retryNodeName)
+		retryNode := woc.getNodeByName(retryNodeName)
+		if !retryNode.Completed() && node.Completed() { //if the retry child has completed we need to update outself
+			node, err = woc.executeTemplate(retryNodeName, orgTmpl, tmplCtx, args, boundaryID)
+			if err != nil {
+				return woc.markNodeError(node.Name, err), err
+			}
+		}
+		node = retryNode
 	}
 
 	return node, nil
diff --git a/workflow/validate/validate.go b/workflow/validate/validate.go
index 5295fc218..475717111 100644
--- a/workflow/validate/validate.go
+++ b/workflow/validate/validate.go
@@ -457,9 +457,6 @@ func validateNonLeaf(tmpl *wfv1.Template) error {
 	if tmpl.ActiveDeadlineSeconds != nil {
 		return errors.Errorf(errors.CodeBadRequest, "templates.%s.activeDeadlineSeconds is only valid for leaf templates", tmpl.Name)
 	}
-	if tmpl.RetryStrategy != nil {
-		return errors.Errorf(errors.CodeBadRequest, "templates.%s.retryStrategy is only valid for container templates", tmpl.Name)
-	}
 	return nil
 }
 


Test patch:
diff --git a/test/e2e/argo_server_test.go b/test/e2e/argo_server_test.go
index 241bc6e53..a5dd9f6c0 100644
--- a/test/e2e/argo_server_test.go
+++ b/test/e2e/argo_server_test.go
@@ -462,6 +462,21 @@ func (s *ArgoServerSuite) TestWorkflowService() {
 			NotNull()
 	})
 
+	s.Run("ListWithFields", func() {
+		j := s.e(s.T()).GET("/api/v1/workflows/argo").
+			WithQuery("listOptions.labelSelector", "argo-e2e=subject").
+			WithQuery("fields", "-items.status.nodes,items.status.finishedAt,items.status.startedAt").
+			Expect().
+			Status(200).
+			JSON()
+		j.
+			Path("$.items").
+			Array().
+			Length().
+			Equal(1)
+		j.Path("$.items[0].status").Object().ContainsKey("phase").NotContainsKey("nodes")
+	})
+
 	s.Run("Get", func() {
 		j := s.e(s.T()).GET("/api/v1/workflows/argo/test").
 			Expect().
@@ -480,6 +495,15 @@ func (s *ArgoServerSuite) TestWorkflowService() {
 			Status(404)
 	})
 
+	s.Run("GetWithFields", func() {
+		j := s.e(s.T()).GET("/api/v1/workflows/argo/test").
+			WithQuery("fields", "status.phase").
+			Expect().
+			Status(200).
+			JSON()
+		j.Path("$.status").Object().ContainsKey("phase").NotContainsKey("nodes")
+	})
+
 	s.Run("Suspend", func() {
 		s.e(s.T()).PUT("/api/v1/workflows/argo/test/suspend").
 			Expect().
diff --git a/workflow/controller/controller_test.go b/workflow/controller/controller_test.go
index a26e73fdf..d981f60b4 100644
--- a/workflow/controller/controller_test.go
+++ b/workflow/controller/controller_test.go
@@ -83,13 +83,18 @@ func unmarshalWFTmpl(yamlStr string) *wfv1.WorkflowTemplate {
 	return &wftmpl
 }
 
-// makePodsRunning acts like a pod controller and simulates the transition of pods transitioning into a running state
-func makePodsRunning(t *testing.T, kubeclientset kubernetes.Interface, namespace string) {
+// makePodsPhase acts like a pod controller and simulates the transition of pods transitioning into a specified state
+func makePodsPhase(t *testing.T, phase apiv1.PodPhase, kubeclientset kubernetes.Interface, namespace string) {
 	podcs := kubeclientset.CoreV1().Pods(namespace)
 	pods, err := podcs.List(metav1.ListOptions{})
 	assert.NoError(t, err)
 	for _, pod := range pods.Items {
-		pod.Status.Phase = apiv1.PodRunning
-		_, _ = podcs.Update(&pod)
+		if pod.Status.Phase == "" {
+			pod.Status.Phase = phase
+			if phase == apiv1.PodFailed {
+				pod.Status.Message = "Pod failed"
+			}
+			_, _ = podcs.Update(&pod)
+		}
 	}
 }
diff --git a/workflow/controller/operator_test.go b/workflow/controller/operator_test.go
index 797e261ad..17e5bc45c 100644
--- a/workflow/controller/operator_test.go
+++ b/workflow/controller/operator_test.go
@@ -405,6 +405,80 @@ func TestAssessNodeStatus(t *testing.T) {
 	}
 }
 
+var workflowStepRetry = `
+apiVersion: argoproj.io/v1alpha1
+kind: Workflow
+metadata:
+  name: step-retry
+spec:
+  entrypoint: step-retry
+  templates:
+  - name: step-retry
+    retryStrategy:
+      limit: 1
+    steps:
+      - - name: whalesay-success
+          arguments:
+            parameters:
+            - name: message
+              value: success
+          template: whalesay
+      - - name: whalesay-failure
+          arguments:
+            parameters:
+            - name: message
+              value: failure
+          template: whalesay
+
+  - name: whalesay
+    inputs:
+      parameters:
+        - name: message
+    container:
+      image: docker/whalesay:latest
+      command: [sh, -c]
+      args: ["cowsay {{inputs.parameters.message}}"]
+`
+
+// TestWorkflowParallelismLimit verifies parallelism at a workflow level is honored.
+func TestWorkflowStepRetry(t *testing.T) {
+	controller := newController()
+	wfcset := controller.wfclientset.ArgoprojV1alpha1().Workflows("")
+	wf := unmarshalWF(workflowStepRetry)
+	wf, err := wfcset.Create(wf)
+	assert.Nil(t, err)
+	wf, err = wfcset.Get(wf.ObjectMeta.Name, metav1.GetOptions{})
+	assert.Nil(t, err)
+	woc := newWorkflowOperationCtx(wf, controller)
+	woc.operate()
+	pods, err := controller.kubeclientset.CoreV1().Pods("").List(metav1.ListOptions{})
+	assert.Nil(t, err)
+	assert.Equal(t, 1, len(pods.Items))
+
+	//complete the first pod
+	makePodsPhase(t, apiv1.PodSucceeded, controller.kubeclientset, wf.ObjectMeta.Namespace)
+	wf, err = wfcset.Get(wf.ObjectMeta.Name, metav1.GetOptions{})
+	assert.Nil(t, err)
+	woc = newWorkflowOperationCtx(wf, controller)
+	woc.operate()
+
+	// fail the second pod
+	makePodsPhase(t, apiv1.PodFailed, controller.kubeclientset, wf.ObjectMeta.Namespace)
+	wf, err = wfcset.Get(wf.ObjectMeta.Name, metav1.GetOptions{})
+	assert.Nil(t, err)
+	woc = newWorkflowOperationCtx(wf, controller)
+	woc.operate()
+	pods, err = controller.kubeclientset.CoreV1().Pods("").List(metav1.ListOptions{})
+	assert.Nil(t, err)
+	assert.Equal(t, 3, len(pods.Items))
+	assert.Equal(t, "cowsay success", pods.Items[0].Spec.Containers[1].Args[0])
+	assert.Equal(t, "cowsay failure", pods.Items[1].Spec.Containers[1].Args[0])
+
+	//verify that after the cowsay failure pod failed, we are retrying cowsay success
+	assert.Equal(t, "cowsay success", pods.Items[2].Spec.Containers[1].Args[0])
+
+}
+
 var workflowParallelismLimit = `
 apiVersion: argoproj.io/v1alpha1
 kind: Workflow
@@ -451,7 +525,7 @@ func TestWorkflowParallelismLimit(t *testing.T) {
 	assert.NoError(t, err)
 	assert.Equal(t, 2, len(pods.Items))
 	// operate again and make sure we don't schedule any more pods
-	makePodsRunning(t, controller.kubeclientset, wf.ObjectMeta.Namespace)
+	makePodsPhase(t, apiv1.PodRunning, controller.kubeclientset, wf.ObjectMeta.Namespace)
 	wf, err = wfcset.Get(wf.ObjectMeta.Name, metav1.GetOptions{})
 	assert.NoError(t, err)
 	// wfBytes, _ := json.MarshalIndent(wf, "", "  ")
@@ -510,7 +584,7 @@ func TestStepsTemplateParallelismLimit(t *testing.T) {
 	assert.Equal(t, 2, len(pods.Items))
 
 	// operate again and make sure we don't schedule any more pods
-	makePodsRunning(t, controller.kubeclientset, wf.ObjectMeta.Namespace)
+	makePodsPhase(t, apiv1.PodRunning, controller.kubeclientset, wf.ObjectMeta.Namespace)
 	wf, err = wfcset.Get(wf.ObjectMeta.Name, metav1.GetOptions{})
 	assert.NoError(t, err)
 	// wfBytes, _ := json.MarshalIndent(wf, "", "  ")
@@ -566,7 +640,7 @@ func TestDAGTemplateParallelismLimit(t *testing.T) {
 	assert.Equal(t, 2, len(pods.Items))
 
 	// operate again and make sure we don't schedule any more pods
-	makePodsRunning(t, controller.kubeclientset, wf.ObjectMeta.Namespace)
+	makePodsPhase(t, apiv1.PodRunning, controller.kubeclientset, wf.ObjectMeta.Namespace)
 	wf, err = wfcset.Get(wf.ObjectMeta.Name, metav1.GetOptions{})
 	assert.NoError(t, err)
 	// wfBytes, _ := json.MarshalIndent(wf, "", "  ")
@@ -2135,7 +2209,7 @@ func TestEventTimeout(t *testing.T) {
 	assert.NoError(t, err)
 	woc := newWorkflowOperationCtx(wf, controller)
 	woc.operate()
-	makePodsRunning(t, controller.kubeclientset, wf.ObjectMeta.Namespace)
+	makePodsPhase(t, apiv1.PodRunning, controller.kubeclientset, wf.ObjectMeta.Namespace)
 	wf, err = wfcset.Get(wf.ObjectMeta.Name, metav1.GetOptions{})
 	assert.NoError(t, err)
 	woc = newWorkflowOperationCtx(wf, controller)
diff --git a/workflow/validate/validate_test.go b/workflow/validate/validate_test.go
index c76fd0bf2..4e2ae0ab6 100644
--- a/workflow/validate/validate_test.go
+++ b/workflow/validate/validate_test.go
@@ -1066,34 +1066,6 @@ func TestLeafWithParallelism(t *testing.T) {
 	}
 }
 
-var nonLeafWithRetryStrategy = `
-apiVersion: argoproj.io/v1alpha1
-kind: Workflow
-metadata:
-  name: non-leaf-with-retry-strategy
-spec:
-  entrypoint: non-leaf-with-retry-strategy
-  templates:
-  - name: non-leaf-with-retry-strategy
-    retryStrategy:
-      limit: 4
-    steps:
-    - - name: try
-        template: try
-  - name: try
-    container:
-      image: debian:9.4
-      command: [sh, -c]
-      args: ["kubectl version"]
-`
-
-func TestNonLeafWithRetryStrategy(t *testing.T) {
-	err := validate(nonLeafWithRetryStrategy)
-	if assert.NotNil(t, err) {
-		assert.Contains(t, err.Error(), "is only valid")
-	}
-}
-
 var invalidStepsArgumentNoFromOrLocation = `
 apiVersion: argoproj.io/v1alpha1
 kind: Workflow