Repository: argoproj/argo
Pull request #3301 (2020-06-25 15:05:33)

feat(controller): Add support for Docker workflow executor for Windows nodes
Argoexec already works with Windows Containers but didn't support `docker` workflow executor yet as Windows Containers require a different way of mounting the Docker socket (see https://github.com/kubernetes/kubernetes/pull/69484):
- Path: `\\.\pipe\docker_engine`
- Type: `null`
- ReadOnly: `false`

I adapted the workflow controller to allow for different volume setups for Windows or Linux Steps of a workflow. When the following `nodeSelector` entry is present for a step in a workflow, then the volume setup for Windows is used, else the setup for Linux is used:
```yaml
nodeSelector:
  kubernetes.io/os: windows
```

I also adapted the documentation as you not longer need to use the `kubelet` workflow executor.

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.
* [x] I have written unit and/or e2e tests for my change. PRs without these are unlikely to be merged.
  - not needed
* [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).


lippertmarkus: Thanks for reviewing, I fixed all conflicts now.

Patch:
diff --git a/docs/argo-server.md b/docs/argo-server.md
index 66b06d086..3e1515294 100644
--- a/docs/argo-server.md
+++ b/docs/argo-server.md
@@ -84,7 +84,7 @@ Then visit: http://127.0.0.1:8001
 kubectl proxy
 ```
 
-Then visit: http://127.0.0.1:8001/api/v1/namespaces/argo/services/argo-ui/proxy/
+Then visit: http://127.0.0.1:8001/api/v1/namespaces/argo/services/argo-server:web/proxy/
 
 NOTE: artifact download and webconsole is not supported using this method
 
diff --git a/docs/windows.md b/docs/windows.md
index a233a05c8..4d0de559b 100644
--- a/docs/windows.md
+++ b/docs/windows.md
@@ -2,21 +2,11 @@
 
 The Argo server and the workflow controller currently only run on Linux. The workflow executor however also runs on Windows nodes, meaning you can use Windows containers inside your workflows! Here are the steps to get started.
 
-##  Requirements
+## Requirements
 * Kubernetes 1.14 or later, supporting Windows nodes
 * Hybrid cluster containing Linux and Windows nodes like described in the [Kubernetes docs](https://kubernetes.io/docs/setup/production-environment/windows/user-guide-windows-containers/)
 * Argo configured and running like described [here](quick-start.md) 
 
-## Setting up the workflow executor
-
-Currently the worflow controller configuration doesn't support different configurations for the `dockerSockPath` based on the host OS. This means that the workflow executor, running in a Windows container can't use Docker for now.
-
-You therefore need to use `kubelet` or `k8sapi` instead in your workflow controller configmap:
-```yaml
-containerRuntimeExecutor: kubelet
-kubeletInsecure: true  # you can disable TLS verification of the kubelet executor for testing
-```
-
 ## Schedule workflows with Windows containers
 
 If you're running workflows in your hybrid Kubernetes cluster, always make sure to include a `nodeSelector` to run the steps on the correct host OS:
@@ -45,7 +35,7 @@ $ argo logs hello-windows-s9kk5
 hello-windows-s9kk5: "Hello from Windows Container!"
 ```
 
-## Bonus: Hybrid workflows
+## Schedule hybrid workflows
 
 You can also run different steps on different host OSs. This can for example be very helpful when you need to compile your application on Windows and Linux.
 
@@ -74,7 +64,7 @@ spec:
         args: ["echo", "Hello from Windows Container!"]
     - name: hello-linux
       nodeSelector:
-        beta.kubernetes.io/os: linux
+        kubernetes.io/os: linux
       container:
         image: alpine
         command: [echo]
diff --git a/examples/hello-hybrid.yaml b/examples/hello-hybrid.yaml
index 381b5a36b..485195ec0 100644
--- a/examples/hello-hybrid.yaml
+++ b/examples/hello-hybrid.yaml
@@ -21,7 +21,7 @@ spec:
         args: ["echo", "Hello from Windows Container!"]
     - name: hello-linux
       nodeSelector:
-        beta.kubernetes.io/os: linux
+        kubernetes.io/os: linux
       container:
         image: alpine
         command: [echo]
diff --git a/workflow/controller/workflowpod.go b/workflow/controller/workflowpod.go
index 55d98d5c3..063bdaf74 100644
--- a/workflow/controller/workflowpod.go
+++ b/workflow/controller/workflowpod.go
@@ -53,16 +53,48 @@ var (
 	hostPathSocket = apiv1.HostPathSocket
 )
 
-func (woc *wfOperationCtx) getVolumeMountDockerSock() apiv1.VolumeMount {
+func (woc *wfOperationCtx) getVolumeMountDockerSock(tmpl *wfv1.Template) apiv1.VolumeMount {
 	return apiv1.VolumeMount{
 		Name:      common.DockerSockVolumeName,
-		MountPath: "/var/run/docker.sock",
-		ReadOnly:  true,
+		MountPath: getDockerSockPath(tmpl),
+		ReadOnly:  getDockerSockReadOnly(tmpl),
 	}
 }
 
-func (woc *wfOperationCtx) getVolumeDockerSock() apiv1.Volume {
-	dockerSockPath := "/var/run/docker.sock"
+func getDockerSockReadOnly(tmpl *wfv1.Template) bool {
+	return !hasWindowsOSNodeSelector(tmpl.NodeSelector)
+}
+
+func getDockerSockPath(tmpl *wfv1.Template) string {
+	if hasWindowsOSNodeSelector(tmpl.NodeSelector) {
+		return "\\\\.\\pipe\\docker_engine"
+	}
+
+	return "/var/run/docker.sock"
+}
+
+func getVolumeHostPathType(tmpl *wfv1.Template) *apiv1.HostPathType {
+	if hasWindowsOSNodeSelector(tmpl.NodeSelector) {
+		return nil
+	}
+
+	return &hostPathSocket
+}
+
+func hasWindowsOSNodeSelector(nodeSelector map[string]string) bool {
+	if nodeSelector == nil {
+		return false
+	}
+
+	if os, keyExists := nodeSelector["kubernetes.io/os"]; keyExists && os == "windows" {
+		return true
+	}
+
+	return false
+}
+
+func (woc *wfOperationCtx) getVolumeDockerSock(tmpl *wfv1.Template) apiv1.Volume {
+	dockerSockPath := getDockerSockPath(tmpl)
 
 	if woc.controller.Config.DockerSockPath != "" {
 		dockerSockPath = woc.controller.Config.DockerSockPath
@@ -77,7 +109,7 @@ func (woc *wfOperationCtx) getVolumeDockerSock() apiv1.Volume {
 		VolumeSource: apiv1.VolumeSource{
 			HostPath: &apiv1.HostPathVolumeSource{
 				Path: dockerSockPath,
-				Type: &hostPathSocket,
+				Type: getVolumeHostPathType(tmpl),
 			},
 		},
 	}
@@ -133,7 +165,7 @@ func (woc *wfOperationCtx) createWorkflowPod(nodeName string, mainCtr apiv1.Cont
 		},
 		Spec: apiv1.PodSpec{
 			RestartPolicy:         apiv1.RestartPolicyNever,
-			Volumes:               woc.createVolumes(),
+			Volumes:               woc.createVolumes(tmpl),
 			ActiveDeadlineSeconds: activeDeadlineSeconds,
 			ImagePullSecrets:      woc.wfSpec.ImagePullSecrets,
 		},
@@ -363,7 +395,7 @@ func (woc *wfOperationCtx) newWaitContainer(tmpl *wfv1.Template) (*apiv1.Contain
 			ctr.SecurityContext.Privileged = pointer.BoolPtr(true)
 		}
 	case "", common.ContainerRuntimeExecutorDocker:
-		ctr.VolumeMounts = append(ctr.VolumeMounts, woc.getVolumeMountDockerSock())
+		ctr.VolumeMounts = append(ctr.VolumeMounts, woc.getVolumeMountDockerSock(tmpl))
 	}
 	return ctr, nil
 }
@@ -444,7 +476,7 @@ func (woc *wfOperationCtx) createEnvVars() []apiv1.EnvVar {
 	return execEnvVars
 }
 
-func (woc *wfOperationCtx) createVolumes() []apiv1.Volume {
+func (woc *wfOperationCtx) createVolumes(tmpl *wfv1.Template) []apiv1.Volume {
 	volumes := []apiv1.Volume{
 		volumePodMetadata,
 	}
@@ -466,7 +498,7 @@ func (woc *wfOperationCtx) createVolumes() []apiv1.Volume {
 	case common.ContainerRuntimeExecutorKubelet, common.ContainerRuntimeExecutorK8sAPI, common.ContainerRuntimeExecutorPNS:
 		return volumes
 	default:
-		return append(volumes, woc.getVolumeDockerSock())
+		return append(volumes, woc.getVolumeDockerSock(tmpl))
 	}
 }
 


Test patch:
diff --git a/workflow/controller/workflowpod_test.go b/workflow/controller/workflowpod_test.go
index 7cbddfe2a..7f6e4aa63 100644
--- a/workflow/controller/workflowpod_test.go
+++ b/workflow/controller/workflowpod_test.go
@@ -1112,6 +1112,62 @@ func TestPodSpecPatch(t *testing.T) {
 
 }
 
+var helloWindowsWf = `
+apiVersion: argoproj.io/v1alpha1
+kind: Workflow
+metadata:
+  name: hello-hybrid-win
+spec:
+  entrypoint: hello-win
+  templates:
+    - name: hello-win
+      nodeSelector:
+        kubernetes.io/os: windows
+      container:
+        image: mcr.microsoft.com/windows/nanoserver:1809
+        command: ["cmd", "/c"]
+        args: ["echo", "Hello from Windows Container!"]
+`
+
+var helloLinuxWf = `
+apiVersion: argoproj.io/v1alpha1
+kind: Workflow
+metadata:
+  name: hello-hybrid-lin
+spec:
+  entrypoint: hello-linux
+  templates:
+    - name: hello-linux
+      nodeSelector:
+        kubernetes.io/os: linux
+      container:
+        image: alpine
+        command: [echo]
+        args: ["Hello from Linux Container!"]
+`
+
+func TestHybridWfVolumesWindows(t *testing.T) {
+	wf := unmarshalWF(helloWindowsWf)
+	woc := newWoc(*wf)
+
+	mainCtr := woc.wfSpec.Templates[0].Container
+	pod, _ := woc.createWorkflowPod(wf.Name, *mainCtr, &wf.Spec.Templates[0], &createWorkflowPodOpts{})
+	assert.Equal(t, "\\\\.\\pipe\\docker_engine", pod.Spec.Containers[0].VolumeMounts[1].MountPath)
+	assert.Equal(t, false, pod.Spec.Containers[0].VolumeMounts[1].ReadOnly)
+	assert.Equal(t, (*apiv1.HostPathType)(nil), pod.Spec.Volumes[1].HostPath.Type)
+}
+
+func TestHybridWfVolumesLinux(t *testing.T) {
+	wf := unmarshalWF(helloLinuxWf)
+	woc := newWoc(*wf)
+
+	mainCtr := woc.wfSpec.Templates[0].Container
+	pod, _ := woc.createWorkflowPod(wf.Name, *mainCtr, &wf.Spec.Templates[0], &createWorkflowPodOpts{})
+	assert.Equal(t, "/var/run/docker.sock", pod.Spec.Containers[0].VolumeMounts[1].MountPath)
+	assert.Equal(t, true, pod.Spec.Containers[0].VolumeMounts[1].ReadOnly)
+	assert.Equal(t, &hostPathSocket, pod.Spec.Volumes[1].HostPath.Type)
+}
+
 var propagateMaxDuration = `
 name: retry-backoff
 retryStrategy: