Repository: argoproj/argo
Pull request #4155 (2020-09-29 14:37:20)

chore: Deprecate unused RuntimeResolution field
Fixes https://github.com/argoproj/argo/issues/4102

simster7: @dtaniwaki Care to comment?

Patch:
diff --git a/.github/workflows/docker-image.yml b/.github/workflows/docker-image.yml
index 6320bf0d5..cb59fe9a9 100644
--- a/.github/workflows/docker-image.yml
+++ b/.github/workflows/docker-image.yml
@@ -12,12 +12,62 @@ defaults:
     shell: bash
 
 jobs:
-  build-linux:
-    name: Build & Push Linux Docker Images
+  build-linux-amd64:
+    name: Build & push linux/amd64
     runs-on: ubuntu-latest
     strategy:
       matrix:
-        platform: [linux/amd64, linux/arm64]
+        platform: [linux/amd64]
+        target: [workflow-controller, argocli, argoexec]
+    steps:
+      - uses: actions/checkout@v2
+
+      - name: Set up Docker Buildx
+        uses: crazy-max/ghaction-docker-buildx@v3
+
+      - name: Cache Docker layers
+        uses: actions/cache@v2
+        id: cache
+        with:
+          path: /tmp/.buildx-cache
+          key: ${{ runner.os }}-${{ matrix.platform }}-${{ matrix.target }}-buildx-${{ github.sha }}
+          restore-keys: |
+            ${{ runner.os }}-${{ matrix.platform }}-${{ matrix.target }}-buildx-
+
+      - name: Login to DockerHub
+        uses: docker/login-action@v1
+        with:
+          username: ${{ secrets.DOCKERIO_USERNAME }}
+          password: ${{ secrets.DOCKERIO_PASSWORD }}
+
+      - name: Docker Buildx
+        env:
+          DOCKERIO_ORG: ${{ secrets.DOCKERIO_ORG }}
+          PLATFORM: ${{ matrix.platform }}
+          TARGET: ${{ matrix.target }}
+        run: |
+          tag=$(basename $GITHUB_REF)
+          if [ $tag = "master" ]; then
+            tag="latest"
+          fi
+
+          tag_suffix=$(echo $PLATFORM | sed -r "s/\//-/g")
+          image_name="${DOCKERIO_ORG}/${TARGET}:${tag}-${tag_suffix}"
+
+          docker buildx build \
+            --cache-from "type=local,src=/tmp/.buildx-cache" \
+            --cache-to "type=local,dest=/tmp/.buildx-cache" \
+            --output "type=image,push=true" \
+            --platform="${PLATFORM}" \
+            --target $TARGET \
+            --tag $image_name .
+
+  build-linux-arm64:
+    name: Build & push linux/arm64
+    runs-on: ubuntu-latest
+    strategy:
+      matrix:
+        platform: [linux/arm64]
         target: [workflow-controller, argocli, argoexec]
     steps:
       - uses: actions/checkout@v2
@@ -63,7 +113,7 @@ jobs:
             --tag $image_name .
 
   build-windows:
-    name: Build & Push Windows Docker Images
+    name: Build & push windows
     runs-on: windows-2019
     steps:
       - uses: actions/checkout@v2
@@ -90,10 +140,41 @@ jobs:
             docker push $image_name
           done
 
+  push-linux-amd64-images:
+    name: Push manifest with linux/amd64
+    runs-on: ubuntu-latest
+    needs: [build-linux-amd64]
+    steps:
+      - uses: actions/checkout@v2
+      - name: Docker Login
+        uses: Azure/docker-login@v1
+        with:
+          username: ${{ secrets.DOCKERIO_USERNAME }}
+          password: ${{ secrets.DOCKERIO_PASSWORD }}
+      - name: Push Multiarch Image
+        env:
+          DOCKERIO_ORG: ${{ secrets.DOCKERIO_ORG }}
+        run: |
+          echo $(jq -c '. + { "experimental": "enabled" }' ${DOCKER_CONFIG}/config.json) > ${DOCKER_CONFIG}/config.json
+
+          docker_org=$DOCKERIO_ORG
+
+          tag=$(basename $GITHUB_REF)
+          if [ $tag = "master" ]; then
+            tag="latest"
+          fi
+
+          targets="workflow-controller argoexec argocli"
+          for target in $targets; do
+            image_name="${docker_org}/${target}:${tag}"
+            docker manifest create $image_name ${image_name}-linux-amd64
+            docker manifest push $image_name
+          done
+
   push-images:
-    name: Push Multiplatform Images
+    name: Push manifest with all images
     runs-on: ubuntu-latest
-    needs: [build-linux, build-windows]
+    needs: [build-linux-arm64, build-windows, push-linux-amd64-images]
     steps:
       - uses: actions/checkout@v2
       - name: Docker Login
diff --git a/USERS.md b/USERS.md
index 87a2aa0c7..9fb5e39a8 100644
--- a/USERS.md
+++ b/USERS.md
@@ -61,6 +61,7 @@ Currently, the following organizations are **officially** using Argo Workflows:
 1. [New Relic](https://newrelic.com/)
 1. [Nikkei](https://www.nikkei.co.jp/nikkeiinfo/en/)
 1. [NVIDIA](https://www.nvidia.com/)
+1. [Onepanel](https://docs.onepanel.ai)
 1. [OVH](https://www.ovh.com/)
 1. [Peak AI](https://www.peak.ai/)
 1. [PDOK](https://www.pdok.nl/)
diff --git a/api/openapi-spec/swagger.json b/api/openapi-spec/swagger.json
index 05ab64e8b..2c2011208 100644
--- a/api/openapi-spec/swagger.json
+++ b/api/openapi-spec/swagger.json
@@ -3896,7 +3896,7 @@
           "type": "string"
         },
         "runtimeResolution": {
-          "description": "RuntimeResolution skips validation at creation time. By enabling this option, you can create the referred workflow template before the actual runtime.",
+          "description": "RuntimeResolution skips validation at creation time. By enabling this option, you can create the referred workflow template before the actual runtime. DEPRECATED: This value is not used anymore and is ignored",
           "type": "boolean"
         },
         "template": {
diff --git a/cmd/argo/commands/delete.go b/cmd/argo/commands/delete.go
index 4ad37adfc..441e54d13 100644
--- a/cmd/argo/commands/delete.go
+++ b/cmd/argo/commands/delete.go
@@ -2,6 +2,7 @@ package commands
 
 import (
 	"fmt"
+	"os"
 
 	"github.com/argoproj/pkg/errors"
 	"github.com/spf13/cobra"
@@ -34,6 +35,11 @@ func NewDeleteCommand() *cobra.Command {
   argo delete @latest
 `,
 		Run: func(cmd *cobra.Command, args []string) {
+			if len(args) == 0 && !(all || allNamespaces || flags.completed || flags.resubmitted || flags.prefix != "" || flags.labels != "" || flags.finishedAfter != "") {
+				cmd.HelpFunc()(cmd, args)
+				os.Exit(1)
+			}
+
 			ctx, apiClient := client.NewAPIClient()
 			serviceClient := apiClient.NewWorkflowServiceClient()
 			var workflows wfv1.Workflows
diff --git a/docs/fields.md b/docs/fields.md
index a4e2ef4aa..768c4d9b8 100644
--- a/docs/fields.md
+++ b/docs/fields.md
@@ -3293,7 +3293,7 @@ TemplateRef is a reference of template resource.
 |:----------:|:----------:|---------------|
 |`clusterScope`|`boolean`|ClusterScope indicates the referred template is cluster scoped (i.e. a ClusterWorkflowTemplate).|
 |`name`|`string`|Name is the resource name of the template.|
-|`runtimeResolution`|`boolean`|RuntimeResolution skips validation at creation time. By enabling this option, you can create the referred workflow template before the actual runtime.|
+|~`runtimeResolution`~|~`boolean`~|~RuntimeResolution skips validation at creation time. By enabling this option, you can create the referred workflow template before the actual runtime.~ DEPRECATED: This value is not used anymore and is ignored|
 |`template`|`string`|Template is the name of referred template in the resource.|
 
 ## MemoizationStatus
diff --git a/docs/swagger.md b/docs/swagger.md
index a806cec0f..42131f0c1 100644
--- a/docs/swagger.md
+++ b/docs/swagger.md
@@ -1579,7 +1579,7 @@ TemplateRef is a reference of template resource.
 | ---- | ---- | ----------- | -------- |
 | clusterScope | boolean | ClusterScope indicates the referred template is cluster scoped (i.e. a ClusterWorkflowTemplate). | No |
 | name | string | Name is the resource name of the template. | No |
-| runtimeResolution | boolean | RuntimeResolution skips validation at creation time. By enabling this option, you can create the referred workflow template before the actual runtime. | No |
+| runtimeResolution | boolean | RuntimeResolution skips validation at creation time. By enabling this option, you can create the referred workflow template before the actual runtime. DEPRECATED: This value is not used anymore and is ignored | No |
 | template | string | Template is the name of referred template in the resource. | No |
 
 #### io.argoproj.workflow.v1alpha1.UpdateCronWorkflowRequest
diff --git a/docs/variables.md b/docs/variables.md
index 9b22c53c3..831b35692 100644
--- a/docs/variables.md
+++ b/docs/variables.md
@@ -1,5 +1,33 @@
 # Workflow Variables
 
+Some fields in a workflow specification allow for variable references which are automatically substituted by Argo. 
+
+??? note "How to use variables"
+    Variables are enclosed in curly braces and **must not** include whitespace.
+
+    ``` yaml
+    apiVersion: argoproj.io/v1alpha1
+    kind: Workflow
+    metadata:
+    generateName: hello-world-parameters-
+    spec:
+    entrypoint: whalesay
+    arguments:
+        parameters:
+        - name: message
+        value: hello world
+    templates:
+    - name: whalesay
+        inputs:
+        parameters:
+        - name: message
+        container:
+        image: docker/whalesay
+        command: [cowsay]
+      # args: ["{{ inputs.parameters.message }}"]       <- bad
+        args: ["{{inputs.parameters.message}}"]         #  good
+    ```
+
 The following variables are made available to reference various metadata of a workflow:
 
 ## All Templates
diff --git a/mkdocs.yml b/mkdocs.yml
index 6d18944d7..fde989d35 100644
--- a/mkdocs.yml
+++ b/mkdocs.yml
@@ -14,6 +14,8 @@ google_analytics:
 markdown_extensions:
   - codehilite
   - admonition
+  - pymdownx.superfences
+  - pymdownx.details
   - toc:
       permalink: true
 nav:
diff --git a/pkg/apis/workflow/v1alpha1/generated.proto b/pkg/apis/workflow/v1alpha1/generated.proto
index d2b1b4b8e..c528de629 100644
--- a/pkg/apis/workflow/v1alpha1/generated.proto
+++ b/pkg/apis/workflow/v1alpha1/generated.proto
@@ -1145,6 +1145,7 @@ message TemplateRef {
 
   // RuntimeResolution skips validation at creation time.
   // By enabling this option, you can create the referred workflow template before the actual runtime.
+  // DEPRECATED: This value is not used anymore and is ignored
   optional bool runtimeResolution = 3;
 
   // ClusterScope indicates the referred template is cluster scoped (i.e. a ClusterWorkflowTemplate).
diff --git a/pkg/apis/workflow/v1alpha1/openapi_generated.go b/pkg/apis/workflow/v1alpha1/openapi_generated.go
index 397802f82..ce1ebd781 100644
--- a/pkg/apis/workflow/v1alpha1/openapi_generated.go
+++ b/pkg/apis/workflow/v1alpha1/openapi_generated.go
@@ -3757,7 +3757,7 @@ func schema_pkg_apis_workflow_v1alpha1_TemplateRef(ref common.ReferenceCallback)
 					},
 					"runtimeResolution": {
 						SchemaProps: spec.SchemaProps{
-							Description: "RuntimeResolution skips validation at creation time. By enabling this option, you can create the referred workflow template before the actual runtime.",
+							Description: "RuntimeResolution skips validation at creation time. By enabling this option, you can create the referred workflow template before the actual runtime. DEPRECATED: This value is not used anymore and is ignored",
 							Type:        []string{"boolean"},
 							Format:      "",
 						},
diff --git a/pkg/apis/workflow/v1alpha1/workflow_types.go b/pkg/apis/workflow/v1alpha1/workflow_types.go
index b1ebf6840..c70d834d9 100644
--- a/pkg/apis/workflow/v1alpha1/workflow_types.go
+++ b/pkg/apis/workflow/v1alpha1/workflow_types.go
@@ -953,6 +953,7 @@ type TemplateRef struct {
 	Template string `json:"template,omitempty" protobuf:"bytes,2,opt,name=template"`
 	// RuntimeResolution skips validation at creation time.
 	// By enabling this option, you can create the referred workflow template before the actual runtime.
+	// DEPRECATED: This value is not used anymore and is ignored
 	RuntimeResolution bool `json:"runtimeResolution,omitempty" protobuf:"varint,3,opt,name=runtimeResolution"`
 	// ClusterScope indicates the referred template is cluster scoped (i.e. a ClusterWorkflowTemplate).
 	ClusterScope bool `json:"clusterScope,omitempty" protobuf:"varint,4,opt,name=clusterScope"`
diff --git a/server/workflow/workflow_server.go b/server/workflow/workflow_server.go
index 5e77b77d7..c32ae8b41 100644
--- a/server/workflow/workflow_server.go
+++ b/server/workflow/workflow_server.go
@@ -261,7 +261,7 @@ func (s *workflowServer) DeleteWorkflow(ctx context.Context, req *workflowpkg.Wo
 	if err != nil {
 		return nil, err
 	}
-	err = auth.GetWfClient(ctx).ArgoprojV1alpha1().Workflows(wf.Namespace).Delete(wf.Name, &metav1.DeleteOptions{})
+	err = auth.GetWfClient(ctx).ArgoprojV1alpha1().Workflows(wf.Namespace).Delete(wf.Name, &metav1.DeleteOptions{PropagationPolicy: argoutil.GetDeletePropagation()})
 	if err != nil {
 		return nil, err
 	}
diff --git a/ui/src/app/workflows/components/workflow-dag/workflow-dag.tsx b/ui/src/app/workflows/components/workflow-dag/workflow-dag.tsx
index f1b9b7596..87ba9aaf9 100644
--- a/ui/src/app/workflows/components/workflow-dag/workflow-dag.tsx
+++ b/ui/src/app/workflows/components/workflow-dag/workflow-dag.tsx
@@ -310,7 +310,7 @@ export class WorkflowDag extends React.Component<WorkflowDagProps, WorkflowDagRe
             }
             return allNodes[nodeId].children.filter(child => allNodes[child]);
         };
-        const pushChildren = (nodeId: string, isExpanded: boolean): void => {
+        const pushChildren = (nodeId: string, isExpanded: boolean, queue: PrepareNode[]): void => {
             const children: string[] = getChildren(nodeId);
             if (!children) {
                 return;
@@ -349,45 +349,50 @@ export class WorkflowDag extends React.Component<WorkflowDagProps, WorkflowDagRe
             }
         };
 
-        const root: PrepareNode = {
-            nodeName: this.props.workflowName,
-            parent: '',
-            children: getChildren(this.props.workflowName)
-        };
-
-        const queue: PrepareNode[] = [root];
-        const consideredChildren: Set<string> = new Set<string>();
-        let previousCollapsed: string = '';
+        const traverse = (root: PrepareNode): void => {
+            const queue: PrepareNode[] = [root];
+            const consideredChildren: Set<string> = new Set<string>();
+            let previousCollapsed: string = '';
+
+            while (queue.length > 0) {
+                const item = queue.pop();
+
+                if (isCollapsedNode(item.nodeName)) {
+                    if (item.nodeName !== previousCollapsed) {
+                        nodes.push(item.nodeName);
+                        edges.push({v: item.parent, w: item.nodeName});
+                        previousCollapsed = item.nodeName;
+                    }
+                    continue;
+                }
 
-        while (queue.length > 0) {
-            const item = queue.pop();
+                const isExpanded: boolean = this.state.expandNodes.has('*') || this.state.expandNodes.has(item.nodeName);
+                nodes.push(item.nodeName);
+                edges.push({v: item.parent, w: item.nodeName});
 
-            if (isCollapsedNode(item.nodeName)) {
-                if (item.nodeName !== previousCollapsed) {
-                    nodes.push(item.nodeName);
-                    edges.push({v: item.parent, w: item.nodeName});
-                    previousCollapsed = item.nodeName;
+                // If we have already considered the children of this node, don't consider them again
+                if (consideredChildren.has(item.nodeName)) {
+                    continue;
                 }
-                continue;
-            }
+                consideredChildren.add(item.nodeName);
 
-            const isExpanded: boolean = this.state.expandNodes.has('*') || this.state.expandNodes.has(item.nodeName);
-            nodes.push(item.nodeName);
-            edges.push({v: item.parent, w: item.nodeName});
+                const node: NodeStatus = this.props.nodes[item.nodeName];
+                if (!node || node.phase === NODE_PHASE.OMITTED) {
+                    continue;
+                }
 
-            // If we have already considered the children of this node, don't consider them again
-            if (consideredChildren.has(item.nodeName)) {
-                continue;
+                pushChildren(node.id, isExpanded, queue);
             }
-            consideredChildren.add(item.nodeName);
+        };
 
-            const node: NodeStatus = this.props.nodes[item.nodeName];
-            if (!node || node.phase === NODE_PHASE.OMITTED) {
-                continue;
-            }
+        const workflowRoot: PrepareNode = {
+            nodeName: this.props.workflowName,
+            parent: '',
+            children: getChildren(this.props.workflowName)
+        };
 
-            pushChildren(node.id, isExpanded);
-        }
+        // Traverse the workflow from the root node
+        traverse(workflowRoot);
 
         const onExitHandlerNodeId = Object.values(allNodes).find(nodeId => nodeId.name === `${this.props.workflowName}.onExit`);
         if (onExitHandlerNodeId) {
@@ -397,6 +402,13 @@ export class WorkflowDag extends React.Component<WorkflowDagProps, WorkflowDagRe
                     edges.push({v, w: onExitHandlerNodeId.id});
                 }
             });
+            const onExitRoot: PrepareNode = {
+                nodeName: onExitHandlerNodeId.id,
+                parent: '',
+                children: getChildren(onExitHandlerNodeId.id)
+            };
+            // Traverse the onExit tree starting from the onExit node itself
+            traverse(onExitRoot);
         }
         return {nodes, edges};
     }
diff --git a/util/util.go b/util/util.go
index ca06ca5ba..4dd56adf1 100644
--- a/util/util.go
+++ b/util/util.go
@@ -3,6 +3,7 @@ package util
 import (
 	"fmt"
 	"io/ioutil"
+	"os"
 	"strconv"
 	"strings"
 
@@ -115,3 +116,13 @@ func RecoverWorkflowNameFromSelectorStringIfAny(selector string) string {
 	}
 	return ""
 }
+
+// getDeletePropagation return the default or configured DeletePropagation policy
+func GetDeletePropagation() *metav1.DeletionPropagation {
+	propagationPolicy := metav1.DeletePropagationBackground
+	envVal, ok := os.LookupEnv("WF_DEL_PROPAGATION_POLICY")
+	if ok && envVal != "" {
+		propagationPolicy = metav1.DeletionPropagation(envVal)
+	}
+	return &propagationPolicy
+}
diff --git a/workflow/ttlcontroller/ttlcontroller.go b/workflow/ttlcontroller/ttlcontroller.go
index ff8bbf6bc..294b4bcc0 100644
--- a/workflow/ttlcontroller/ttlcontroller.go
+++ b/workflow/ttlcontroller/ttlcontroller.go
@@ -16,6 +16,7 @@ import (
 
 	wfv1 "github.com/argoproj/argo/pkg/apis/workflow/v1alpha1"
 	wfclientset "github.com/argoproj/argo/pkg/client/clientset/versioned"
+	commonutil "github.com/argoproj/argo/util"
 	"github.com/argoproj/argo/workflow/common"
 	"github.com/argoproj/argo/workflow/util"
 )
@@ -177,8 +178,8 @@ func (c *Controller) deleteWorkflow(key string) error {
 	}
 	if c.ttlExpired(wf) {
 		log.Infof("Deleting TTL expired workflow %s/%s", wf.Namespace, wf.Name)
-		policy := metav1.DeletePropagationForeground
-		err = c.wfclientset.ArgoprojV1alpha1().Workflows(wf.Namespace).Delete(wf.Name, &metav1.DeleteOptions{PropagationPolicy: &policy})
+
+		err = c.wfclientset.ArgoprojV1alpha1().Workflows(wf.Namespace).Delete(wf.Name, &metav1.DeleteOptions{PropagationPolicy: commonutil.GetDeletePropagation()})
 		if err != nil {
 			return err
 		}
diff --git a/workflow/validate/validate.go b/workflow/validate/validate.go
index f173d8b5a..365ae61a2 100644
--- a/workflow/validate/validate.go
+++ b/workflow/validate/validate.go
@@ -327,11 +327,6 @@ Templates should be referenced from within a "steps" or a "dag" template. Here i
           template: %s`
 
 		out = fmt.Sprintf(out, tmpl.Name, tmpl.TemplateRef.Template, tmpl.TemplateRef.Name, tmpl.TemplateRef.Template)
-		if tmpl.TemplateRef.RuntimeResolution {
-			out += `
-          runtimeResolution: %t`
-			out = fmt.Sprintf(out, tmpl.TemplateRef.RuntimeResolution)
-		}
 	} else if tmpl.Template != "" {
 		out += `
         template: %s`
@@ -476,8 +471,7 @@ func (ctx *templateValidationCtx) validateTemplateHolder(tmplHolder wfv1.Templat
 			return nil, errors.New(errors.CodeBadRequest, "template name is required")
 		}
 		if tmplRef.RuntimeResolution {
-			// Let's see if the template exists at runtime.
-			return nil, nil
+			logrus.Warnf("the 'runtimeResolution' field is deprecated and ignored")
 		}
 	} else if tmplName != "" {
 		_, err := tmplCtx.GetTemplateByName(tmplName)


Test patch:
diff --git a/test/e2e/cli_test.go b/test/e2e/cli_test.go
index 20d5a92e4..0a63e8c2e 100644
--- a/test/e2e/cli_test.go
+++ b/test/e2e/cli_test.go
@@ -414,7 +414,7 @@ func (s *CLISuite) TestWorkflowDeleteNothing() {
 		When().
 		SubmitWorkflow().
 		RunCli([]string{"delete"}, func(t *testing.T, output string, err error) {
-			if assert.NoError(t, err) {
+			if assert.EqualError(t, err, "exit status 1") {
 				assert.NotContains(t, output, "deleted")
 			}
 		})
diff --git a/util/util_test.go b/util/util_test.go
index a4b9b7bf3..4c44d974d 100644
--- a/util/util_test.go
+++ b/util/util_test.go
@@ -1,8 +1,11 @@
 package util
 
 import (
+	"os"
 	"testing"
 
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+
 	"github.com/stretchr/testify/assert"
 )
 
@@ -64,3 +67,17 @@ func TestRecoverWorkflowNameFromSelectorString(t *testing.T) {
 		_ = RecoverWorkflowNameFromSelectorStringIfAny("whatever")
 	})
 }
+
+func TestGetDeletePropagation(t *testing.T) {
+	t.Run("GetDefaultPolicy", func(t *testing.T) {
+		assert.Equal(t, metav1.DeletePropagationBackground, *GetDeletePropagation())
+	})
+	t.Run("GetEnvPolicy", func(t *testing.T) {
+		os.Setenv("WF_DEL_PROPAGATION_POLICY", "Foreground")
+		assert.Equal(t, metav1.DeletePropagationForeground, *GetDeletePropagation())
+	})
+	t.Run("GetEnvPolicyWithEmpty", func(t *testing.T) {
+		os.Setenv("WF_DEL_PROPAGATION_POLICY", "")
+		assert.Equal(t, metav1.DeletePropagationBackground, *GetDeletePropagation())
+	})
+}
\ No newline at end of file