Repository: argoproj/argo (go)

Issue:
Fix TestWorkflowRetry flakey test
```
2020-09-22T20:15:08.1567699Z --- FAIL: TestCLIWithServerSuite (211.00s)
2020-09-22T20:15:08.1568639Z     --- FAIL: TestCLIWithServerSuite/TestWorkflowRetry (11.23s)
2020-09-22T20:15:08.1570802Z         cli_test.go:643: 
2020-09-22T20:15:08.1571195Z             	Error Trace:	cli_test.go:643
2020-09-22T20:15:08.1571600Z             	            				when.go:244
2020-09-22T20:15:08.1571958Z             	            				when.go:253
2020-09-22T20:15:08.1572319Z             	            				cli_test.go:642
2020-09-22T20:15:08.1572766Z             	Error:      	Received unexpected error:
2020-09-22T20:15:08.1573197Z             	            	exit status 1
2020-09-22T20:15:08.1573968Z             	Test:       	TestCLIWithServerSuite/TestWorkflowRetry
```

https://github.com/argoproj/argo/runs/1150921691?check_suite_focus=true
https://github.com/argoproj/argo/suites/1229456411/artifacts/18490770

Pull request:
fix(cli): Add retry to retry ;). Fixes #4101
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 is a chore.
* [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"`.  
* [x] I've signed the CLA.
* [ ] ~~I have written unit and/or e2e tests for my change. PRs without these are unlikely to be merged.~~
* [x] My builds are green. Try syncing with master if they are not. 
* [x] My organization is added to [USERS.md](https://github.com/argoproj/argo/blob/master/USERS.md).

Fixes #4101

Patch:
diff --git a/.github/workflows/docker-image.yml b/.github/workflows/docker-image.yml
index ebdd6a406..6320bf0d5 100644
--- a/.github/workflows/docker-image.yml
+++ b/.github/workflows/docker-image.yml
@@ -15,37 +15,52 @@ jobs:
   build-linux:
     name: Build & Push Linux Docker Images
     runs-on: ubuntu-latest
+    strategy:
+      matrix:
+        platform: [linux/amd64, linux/arm64]
+        target: [workflow-controller, argocli, argoexec]
     steps:
       - uses: actions/checkout@v2
-      - name: Docker Login
-        uses: Azure/docker-login@v1
+
+      - 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: Build & Push Linux Docker Images
+
+      - name: Docker Buildx
         env:
           DOCKERIO_ORG: ${{ secrets.DOCKERIO_ORG }}
-        # https://github.com/marketplace/actions/docker-buildx#build-and-push-to-dockerhub
+          PLATFORM: ${{ matrix.platform }}
+          TARGET: ${{ matrix.target }}
         run: |
-          /usr/bin/docker pull -q multiarch/qemu-user-static:latest
-          /usr/bin/docker run --rm --privileged multiarch/qemu-user-static:latest --reset -p yes --credential yes
-          /usr/bin/docker buildx create --name builder --driver docker-container --use
-          /usr/bin/docker buildx inspect --bootstrap
-
           tag=$(basename $GITHUB_REF)
           if [ $tag = "master" ]; then
             tag="latest"
           fi
-          targets="workflow-controller argocli"
-          for target in $targets; do
-            docker buildx build \
-              --output "type=image,push=true" \
-              --platform="linux/amd64,linux/arm64" \
-              --target $target \
-              --tag "${DOCKERIO_ORG}/${target}:${tag}-linux" \
-              --tag "${DOCKERIO_ORG}/${target}:${tag}" .
-          done
-          /usr/bin/docker buildx rm builder
+
+          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-windows:
     name: Build & Push Windows Docker Images
@@ -76,7 +91,7 @@ jobs:
           done
 
   push-images:
-    name: Build & Push Multiplatform Images
+    name: Push Multiplatform Images
     runs-on: ubuntu-latest
     needs: [build-linux, build-windows]
     steps:
@@ -90,11 +105,6 @@ jobs:
         env:
           DOCKERIO_ORG: ${{ secrets.DOCKERIO_ORG }}
         run: |
-          /usr/bin/docker pull -q multiarch/qemu-user-static:latest
-          /usr/bin/docker run --rm --privileged multiarch/qemu-user-static:latest --reset -p yes --credential yes
-          /usr/bin/docker buildx create --name builder --driver docker-container --use
-          /usr/bin/docker buildx inspect --bootstrap
-
           echo $(jq -c '. + { "experimental": "enabled" }' ${DOCKER_CONFIG}/config.json) > ${DOCKER_CONFIG}/config.json
 
           docker_org=$DOCKERIO_ORG
@@ -103,23 +113,16 @@ jobs:
           if [ $tag = "master" ]; then
             tag="latest"
           fi
-          targets="argoexec"
+
+          targets="workflow-controller argoexec argocli"
           for target in $targets; do
             image_name="${docker_org}/${target}:${tag}"
 
-            docker buildx build \
-              --push \
-              --platform="linux/arm64" \
-              --target ${target} \
-              --tag "${image_name}-linux-arm64" .
-
-            docker buildx build \
-              --push \
-              --platform="linux/amd64" \
-              --target ${target} \
-              --tag "${image_name}-linux-amd64" .
-
-            docker manifest create $image_name ${image_name}-windows ${image_name}-linux-arm64 ${image_name}-linux-amd64
+            if [ $target = "argoexec" ]; then
+              docker manifest create $image_name ${image_name}-linux-arm64 ${image_name}-linux-amd64 ${image_name}-windows
+            else
+              docker manifest create $image_name ${image_name}-linux-arm64 ${image_name}-linux-amd64
+            fi
 
             docker manifest push $image_name
           done
\ No newline at end of file
diff --git a/Dockerfile.windows b/Dockerfile.windows
index a3c0b6b42..2976110ba 100644
--- a/Dockerfile.windows
+++ b/Dockerfile.windows
@@ -19,7 +19,7 @@ RUN iex ((new-object net.webclient).DownloadString('https://chocolatey.org/insta
 
 # install golang, dep and other tools
 RUN choco install golang --version=$env:GOLANG_VERSION ; \
-    choco install make dep docker-cli git.portable
+    choco install make dep docker-cli git.portable 7zip.portable
 
 ####################################################################################################
 # argoexec-base
@@ -41,6 +41,7 @@ RUN mkdir C:\app && \
 
 COPY --from=builder C:/ProgramData/chocolatey/lib/docker-cli/tools/docker.exe C:/app/docker.exe
 COPY --from=builder C:/tools/git C:/app/git
+COPY --from=builder C:/ProgramData/chocolatey/lib/7zip.portable/tools/7z-extra/x64/7za.exe C:/app/7za.exe
 
 # add binaries to path
 USER Administrator
diff --git a/USERS.md b/USERS.md
index 17d15b0e3..d6042fa35 100644
--- a/USERS.md
+++ b/USERS.md
@@ -85,3 +85,4 @@ Currently, the following organizations are **officially** using Argo Workflows:
 1. [Tulip](https://tulip.com/)
 1. [Wavefront](https://www.wavefront.com/)
 1. [Wellcome Trust](https://wellcome.ac.uk/)
+1. [Helio](https://helio.exchange)
diff --git a/workflow/common/util.go b/workflow/common/util.go
index a7a00cd87..fc2366adf 100644
--- a/workflow/common/util.go
+++ b/workflow/common/util.go
@@ -8,6 +8,7 @@ import (
 	"net/http"
 	"os/exec"
 	"regexp"
+	"runtime"
 	"strconv"
 	"strings"
 	"time"
@@ -404,20 +405,33 @@ func Replace(fstTmpl *fasttemplate.Template, replaceMap map[string]string, allow
 }
 
 // RunCommand is a convenience function to run/log a command and log the stderr upon failure
-func RunCommand(name string, arg ...string) error {
+func RunCommand(name string, arg ...string) ([]byte, error) {
 	cmd := exec.Command(name, arg...)
 	cmdStr := strings.Join(cmd.Args, " ")
 	log.Info(cmdStr)
-	_, err := cmd.Output()
+	out, err := cmd.Output()
 	if err != nil {
 		if exErr, ok := err.(*exec.ExitError); ok {
 			errOutput := string(exErr.Stderr)
 			log.Errorf("`%s` failed: %s", cmdStr, errOutput)
-			return errors.InternalError(strings.TrimSpace(errOutput))
+			return nil, errors.InternalError(strings.TrimSpace(errOutput))
 		}
-		return errors.InternalWrapError(err)
+		return nil, errors.InternalWrapError(err)
 	}
-	return nil
+	return out, nil
+}
+
+// RunShellCommand is a convenience function to use RunCommand for shell executions. It's os-specific
+// and runs `cmd` in windows.
+func RunShellCommand(arg ...string) ([]byte, error) {
+	name := "sh"
+	shellFlag := "-c"
+	if runtime.GOOS == "windows" {
+		name = "cmd"
+		shellFlag = "/c"
+	}
+	arg = append([]string{shellFlag}, arg...)
+	return RunCommand(name, arg...)
 }
 
 const patchRetries = 5
diff --git a/workflow/controller/operator.go b/workflow/controller/operator.go
index 1be02b7f2..a771edd3b 100644
--- a/workflow/controller/operator.go
+++ b/workflow/controller/operator.go
@@ -3037,19 +3037,20 @@ func (woc *wfOperationCtx) loadExecutionSpec() (wfv1.TemplateReferenceHolder, wf
 		}
 	}
 
-	//In WorkflowTemplateRef scenario, we need to merge the Workflow Default, Workflow spec and storedWorkflowspec for execWf.
+	//In WorkflowTemplateRef scenario, we need to merge the  Workflow spec, :StoredWorkflowspec and Workflow Default for execWf.
+	// Overlay
 	targetWf := wfv1.Workflow{Spec: *woc.wf.Spec.DeepCopy()}
-	err := woc.controller.setWorkflowDefaults(&targetWf)
+
+	err := wfutil.MergeTo(&wfv1.Workflow{Spec: *woc.wf.Status.StoredWorkflowSpec}, &targetWf)
 	if err != nil {
-		log.WithFields(log.Fields{"key": woc.wf.Name, "error": err}).Warn("Failed to apply default workflow values")
 		return nil, executionParameters, err
 	}
 
-	err = wfutil.MergeTo(&wfv1.Workflow{Spec: *woc.wf.Status.StoredWorkflowSpec}, &targetWf)
+	err = woc.controller.setWorkflowDefaults(&targetWf)
 	if err != nil {
+		log.WithFields(log.Fields{"key": woc.wf.Name, "error": err}).Warn("Failed to apply default workflow values")
 		return nil, executionParameters, err
 	}
-
 	// Setting the merged workflow to executable workflow
 	woc.execWf = &targetWf
 
diff --git a/workflow/executor/docker/docker.go b/workflow/executor/docker/docker.go
index 7bc052133..247503504 100644
--- a/workflow/executor/docker/docker.go
+++ b/workflow/executor/docker/docker.go
@@ -8,6 +8,7 @@ import (
 	"io/ioutil"
 	"os"
 	"os/exec"
+	"runtime"
 	"strconv"
 	"strings"
 	"sync"
@@ -36,34 +37,18 @@ func (d *DockerExecutor) GetFileContents(containerID string, sourcePath string)
 	// content from the tar archive and output into stdout. In this way, we do not need to
 	// create and copy the content into a file from the wait container.
 	dockerCpCmd := fmt.Sprintf("docker cp -a %s:%s - | tar -ax -O", containerID, sourcePath)
-	cmd := exec.Command("sh", "-c", dockerCpCmd)
-	log.Info(cmd.Args)
-	out, err := cmd.Output()
+	out, err := common.RunShellCommand(dockerCpCmd)
 	if err != nil {
-		if exErr, ok := err.(*exec.ExitError); ok {
-			log.Errorf("`%s` stderr:\n%s", cmd.Args, string(exErr.Stderr))
-		}
-		return "", errors.InternalWrapError(err)
+		return "", err
 	}
 	return string(out), nil
 }
 
 func (d *DockerExecutor) CopyFile(containerID string, sourcePath string, destPath string, compressionLevel int) error {
 	log.Infof("Archiving %s:%s to %s", containerID, sourcePath, destPath)
-	var levelFlag string
-	switch compressionLevel {
-	case gzip.NoCompression:
-		// best we can do - if we skip gzip it's a different file
-		levelFlag = "-1"
-	case gzip.DefaultCompression:
-		// use cmd default
-		levelFlag = ""
-	default:
-		// -1 through -9 (or error)
-		levelFlag = "-" + strconv.Itoa(compressionLevel)
-	}
-	dockerCpCmd := fmt.Sprintf("docker cp -a %s:%s - | gzip %s > %s", containerID, sourcePath, levelFlag, destPath)
-	err := common.RunCommand("sh", "-c", dockerCpCmd)
+
+	dockerCpCmd := getDockerCpCmd(containerID, sourcePath, compressionLevel, destPath)
+	_, err := common.RunShellCommand(dockerCpCmd)
 	if err != nil {
 		return err
 	}
@@ -182,7 +167,8 @@ func (d *DockerExecutor) WaitInit() error {
 
 // Wait for the container to complete
 func (d *DockerExecutor) Wait(containerID string) error {
-	return common.RunCommand("docker", "wait", containerID)
+	_, err := common.RunCommand("docker", "wait", containerID)
+	return err
 }
 
 // killContainers kills a list of containerIDs first with a SIGTERM then with a SIGKILL after a grace period
@@ -190,7 +176,7 @@ func (d *DockerExecutor) Kill(containerIDs []string) error {
 	killArgs := append([]string{"kill", "--signal", "TERM"}, containerIDs...)
 	// docker kill will return with an error if a container has terminated already, which is not an error in this case.
 	// We therefore ignore any error. docker wait that follows will re-raise any other error with the container.
-	err := common.RunCommand("docker", killArgs...)
+	_, err := common.RunCommand("docker", killArgs...)
 	if err != nil {
 		log.Warningf("Ignored error from 'docker kill --signal TERM': %s", err)
 	}
@@ -219,3 +205,28 @@ func (d *DockerExecutor) Kill(containerIDs []string) error {
 	log.Infof("Containers %s killed successfully", containerIDs)
 	return nil
 }
+
+// getDockerCpCmd uses os-specific code to run `docker cp` and gzip/7zip to copy gzipped data from another
+// container.
+func getDockerCpCmd(containerID, sourcePath string, compressionLevel int, destPath string) string {
+	gzipCmd := "gzip %s > %s"
+	levelFlagParam := "-"
+	if runtime.GOOS == "windows" {
+		gzipCmd = "7za.exe a -tgzip -si %s %s"
+		levelFlagParam = "-mx"
+	}
+
+	var levelFlag string
+	switch compressionLevel {
+	case gzip.NoCompression:
+		// best we can do - if we skip gzip it's a different file
+		levelFlag = levelFlagParam + "1"
+	case gzip.DefaultCompression:
+		// use cmd default
+		levelFlag = ""
+	default:
+		// -1 through -9 (or error)
+		levelFlag = levelFlagParam + strconv.Itoa(compressionLevel)
+	}
+	return fmt.Sprintf("docker cp -a %s:%s - | %s", containerID, sourcePath, fmt.Sprintf(gzipCmd, levelFlag, destPath))
+}
diff --git a/workflow/executor/executor.go b/workflow/executor/executor.go
index 98a90e1b8..60413b492 100644
--- a/workflow/executor/executor.go
+++ b/workflow/executor/executor.go
@@ -854,7 +854,7 @@ func untar(tarPath string, destPath string) error {
 	if err != nil {
 		return errors.InternalWrapError(err)
 	}
-	err = common.RunCommand("tar", "-xf", tarPath, "-C", tmpDir)
+	_, err = common.RunCommand("tar", "-xf", tarPath, "-C", tmpDir)
 	if err != nil {
 		return err
 	}
diff --git a/workflow/util/util.go b/workflow/util/util.go
index 582f50c4f..e51fb4633 100644
--- a/workflow/util/util.go
+++ b/workflow/util/util.go
@@ -665,7 +665,19 @@ func RetryWorkflow(kubeClient kubernetes.Interface, hydrator hydrator.Interface,
 	default:
 		return nil, errors.Errorf(errors.CodeBadRequest, "workflow must be Failed/Error to retry")
 	}
+	var updated *wfv1.Workflow
+	err := wait.ExponentialBackoff(retry.DefaultRetry, func() (bool, error) {
+		var err error
+		updated, err = retryWorkflow(kubeClient, hydrator, wfClient, wf, restartSuccessful, nodeFieldSelector)
+		return err == nil, err
+	})
+	if err != nil {
+		return nil, err
+	}
+	return updated, err
+}
 
+func retryWorkflow(kubeClient kubernetes.Interface, hydrator hydrator.Interface, wfClient v1alpha1.WorkflowInterface, wf *wfv1.Workflow, restartSuccessful bool, nodeFieldSelector string) (*wfv1.Workflow, error) {
 	err := hydrator.Hydrate(wf)
 	if err != nil {
 		return nil, err


Test patch:
diff --git a/workflow/controller/operator_workflow_template_ref_test.go b/workflow/controller/operator_workflow_template_ref_test.go
index 0d1a9fdfe..a75b1a423 100644
--- a/workflow/controller/operator_workflow_template_ref_test.go
+++ b/workflow/controller/operator_workflow_template_ref_test.go
@@ -63,6 +63,42 @@ func TestWorkflowTemplateRefWithWorkflowTemplateArgs(t *testing.T) {
 		assert.Equal(t, "test", woc.globalParams["workflow.parameters.param1"])
 
 	})
+
+	t.Run("CheckMergingWFDefaults", func(t *testing.T) {
+		wfDefaultActiveS := int64(5)
+		cancel, controller := newController(wf, wftmpl)
+		defer cancel()
+		controller.Config.WorkflowDefaults = &wfv1.Workflow{Spec: wfv1.WorkflowSpec{
+			ActiveDeadlineSeconds: &wfDefaultActiveS,
+		},
+		}
+		woc := newWorkflowOperationCtx(wf, controller)
+		woc.operate()
+		assert.Equal(t, wfDefaultActiveS, *woc.execWf.Spec.ActiveDeadlineSeconds)
+
+	})
+	t.Run("CheckMergingWFTandWF", func(t *testing.T) {
+		wfActiveS := int64(10)
+		wftActiveS := int64(10)
+		wfDefaultActiveS := int64(5)
+
+		wftmpl.Spec.ActiveDeadlineSeconds = &wftActiveS
+		cancel, controller := newController(wf, wftmpl)
+		defer cancel()
+		controller.Config.WorkflowDefaults = &wfv1.Workflow{Spec: wfv1.WorkflowSpec{
+			ActiveDeadlineSeconds: &wfDefaultActiveS,
+		},
+		}
+		wf.Spec.ActiveDeadlineSeconds = &wfActiveS
+		woc := newWorkflowOperationCtx(wf, controller)
+		woc.operate()
+		assert.Equal(t, wfActiveS, *woc.execWf.Spec.ActiveDeadlineSeconds)
+
+		wf.Spec.ActiveDeadlineSeconds = nil
+		woc = newWorkflowOperationCtx(wf, controller)
+		woc.operate()
+		assert.Equal(t, wftActiveS, *woc.execWf.Spec.ActiveDeadlineSeconds)
+	})
 }
 
 const wfWithStatus = `