From 16666e1bbaca46c5ebb1024f2886cedc2838d989 Mon Sep 17 00:00:00 2001 From: Dmitry Chepurovskiy Date: Fri, 22 Sep 2023 13:41:50 +0300 Subject: [PATCH] Fix #2809 : replace TLS dockerd connection with unix socket (#2833) Co-authored-by: Bassem Dghaidi <568794+Link-@users.noreply.github.com> --- .../templates/_helpers.tpl | 37 +++++++------------ .../tests/template_test.go | 32 ++++++---------- charts/gha-runner-scale-set/values.yaml | 23 +++++++----- 3 files changed, 39 insertions(+), 53 deletions(-) diff --git a/charts/gha-runner-scale-set/templates/_helpers.tpl b/charts/gha-runner-scale-set/templates/_helpers.tpl index 80cc4aff..2f667ab4 100644 --- a/charts/gha-runner-scale-set/templates/_helpers.tpl +++ b/charts/gha-runner-scale-set/templates/_helpers.tpl @@ -93,19 +93,26 @@ volumeMounts: {{- define "gha-runner-scale-set.dind-container" -}} image: docker:dind +args: + - dockerd + - --host=unix:///run/docker/docker.sock + - --group=$(DOCKER_GROUP_GID) +env: + - name: DOCKER_GROUP_GID + value: "123" securityContext: privileged: true volumeMounts: - name: work mountPath: /home/runner/_work - - name: dind-cert - mountPath: /certs/client + - name: dind-sock + mountPath: /run/docker - name: dind-externals mountPath: /home/runner/externals {{- end }} {{- define "gha-runner-scale-set.dind-volume" -}} -- name: dind-cert +- name: dind-sock emptyDir: {} - name: dind-externals emptyDir: {} @@ -185,8 +192,6 @@ volumeMounts: {{- end }} {{- end }} {{- $setDockerHost := 1 }} - {{- $setDockerTlsVerify := 1 }} - {{- $setDockerCertPath := 1 }} {{- $setRunnerWaitDocker := 1 }} {{- $setNodeExtraCaCerts := 0 }} {{- $setRunnerUpdateCaCerts := 0 }} @@ -200,12 +205,6 @@ env: {{- if eq $env.name "DOCKER_HOST" }} {{- $setDockerHost = 0 }} {{- end }} - {{- if eq $env.name "DOCKER_TLS_VERIFY" }} - {{- $setDockerTlsVerify = 0 }} - {{- end }} - {{- if eq $env.name "DOCKER_CERT_PATH" }} - {{- $setDockerCertPath = 0 }} - {{- end }} {{- if eq $env.name "RUNNER_WAIT_FOR_DOCKER_IN_SECONDS" }} {{- $setRunnerWaitDocker = 0 }} {{- end }} @@ -220,15 +219,7 @@ env: {{- end }} {{- if $setDockerHost }} - name: DOCKER_HOST - value: tcp://localhost:2376 - {{- end }} - {{- if $setDockerTlsVerify }} - - name: DOCKER_TLS_VERIFY - value: "1" - {{- end }} - {{- if $setDockerCertPath }} - - name: DOCKER_CERT_PATH - value: /certs/client + value: unix:///run/docker/docker.sock {{- end }} {{- if $setRunnerWaitDocker }} - name: RUNNER_WAIT_FOR_DOCKER_IN_SECONDS @@ -254,7 +245,7 @@ volumeMounts: {{- if eq $volMount.name "work" }} {{- $mountWork = 0 }} {{- end }} - {{- if eq $volMount.name "dind-cert" }} + {{- if eq $volMount.name "dind-sock" }} {{- $mountDindCert = 0 }} {{- end }} {{- if eq $volMount.name "github-server-tls-cert" }} @@ -268,8 +259,8 @@ volumeMounts: mountPath: /home/runner/_work {{- end }} {{- if $mountDindCert }} - - name: dind-cert - mountPath: /certs/client + - name: dind-sock + mountPath: /run/docker readOnly: true {{- end }} {{- if $mountGitHubServerTLS }} diff --git a/charts/gha-runner-scale-set/tests/template_test.go b/charts/gha-runner-scale-set/tests/template_test.go index 89ec7a2d..f85426b3 100644 --- a/charts/gha-runner-scale-set/tests/template_test.go +++ b/charts/gha-runner-scale-set/tests/template_test.go @@ -767,7 +767,7 @@ func TestTemplateRenderedAutoScalingRunnerSet_DinD_ExtraVolumes(t *testing.T) { helm.UnmarshalK8SYaml(t, output, &ars) assert.Len(t, ars.Spec.Template.Spec.Volumes, 5, "Volumes should be 5") - assert.Equal(t, "dind-cert", ars.Spec.Template.Spec.Volumes[0].Name, "Volume name should be dind-cert") + assert.Equal(t, "dind-sock", ars.Spec.Template.Spec.Volumes[0].Name, "Volume name should be dind-sock") assert.Equal(t, "dind-externals", ars.Spec.Template.Spec.Volumes[1].Name, "Volume name should be dind-externals") assert.Equal(t, "work", ars.Spec.Template.Spec.Volumes[2].Name, "Volume name should be work") assert.Equal(t, "/data", ars.Spec.Template.Spec.Volumes[2].HostPath.Path, "Volume host path should be /data") @@ -863,40 +863,36 @@ func TestTemplateRenderedAutoScalingRunnerSet_EnableDinD(t *testing.T) { assert.Len(t, ars.Spec.Template.Spec.Containers, 2, "Template.Spec should have 2 container") assert.Equal(t, "runner", ars.Spec.Template.Spec.Containers[0].Name) assert.Equal(t, "ghcr.io/actions/actions-runner:latest", ars.Spec.Template.Spec.Containers[0].Image) - assert.Len(t, ars.Spec.Template.Spec.Containers[0].Env, 4, "The runner container should have 4 env vars, DOCKER_HOST, DOCKER_TLS_VERIFY, DOCKER_CERT_PATH and RUNNER_WAIT_FOR_DOCKER_IN_SECONDS") + assert.Len(t, ars.Spec.Template.Spec.Containers[0].Env, 2, "The runner container should have 2 env vars, DOCKER_HOST and RUNNER_WAIT_FOR_DOCKER_IN_SECONDS") assert.Equal(t, "DOCKER_HOST", ars.Spec.Template.Spec.Containers[0].Env[0].Name) - assert.Equal(t, "tcp://localhost:2376", ars.Spec.Template.Spec.Containers[0].Env[0].Value) - assert.Equal(t, "DOCKER_TLS_VERIFY", ars.Spec.Template.Spec.Containers[0].Env[1].Name) - assert.Equal(t, "1", ars.Spec.Template.Spec.Containers[0].Env[1].Value) - assert.Equal(t, "DOCKER_CERT_PATH", ars.Spec.Template.Spec.Containers[0].Env[2].Name) - assert.Equal(t, "/certs/client", ars.Spec.Template.Spec.Containers[0].Env[2].Value) - assert.Equal(t, "RUNNER_WAIT_FOR_DOCKER_IN_SECONDS", ars.Spec.Template.Spec.Containers[0].Env[3].Name) - assert.Equal(t, "120", ars.Spec.Template.Spec.Containers[0].Env[3].Value) + assert.Equal(t, "unix:///run/docker/docker.sock", ars.Spec.Template.Spec.Containers[0].Env[0].Value) + assert.Equal(t, "RUNNER_WAIT_FOR_DOCKER_IN_SECONDS", ars.Spec.Template.Spec.Containers[0].Env[1].Name) + assert.Equal(t, "120", ars.Spec.Template.Spec.Containers[0].Env[1].Value) - assert.Len(t, ars.Spec.Template.Spec.Containers[0].VolumeMounts, 2, "The runner container should have 2 volume mounts, dind-cert and work") + assert.Len(t, ars.Spec.Template.Spec.Containers[0].VolumeMounts, 2, "The runner container should have 2 volume mounts, dind-sock and work") assert.Equal(t, "work", ars.Spec.Template.Spec.Containers[0].VolumeMounts[0].Name) assert.Equal(t, "/home/runner/_work", ars.Spec.Template.Spec.Containers[0].VolumeMounts[0].MountPath) assert.False(t, ars.Spec.Template.Spec.Containers[0].VolumeMounts[0].ReadOnly) - assert.Equal(t, "dind-cert", ars.Spec.Template.Spec.Containers[0].VolumeMounts[1].Name) - assert.Equal(t, "/certs/client", ars.Spec.Template.Spec.Containers[0].VolumeMounts[1].MountPath) + assert.Equal(t, "dind-sock", ars.Spec.Template.Spec.Containers[0].VolumeMounts[1].Name) + assert.Equal(t, "/run/docker", ars.Spec.Template.Spec.Containers[0].VolumeMounts[1].MountPath) assert.True(t, ars.Spec.Template.Spec.Containers[0].VolumeMounts[1].ReadOnly) assert.Equal(t, "dind", ars.Spec.Template.Spec.Containers[1].Name) assert.Equal(t, "docker:dind", ars.Spec.Template.Spec.Containers[1].Image) assert.True(t, *ars.Spec.Template.Spec.Containers[1].SecurityContext.Privileged) - assert.Len(t, ars.Spec.Template.Spec.Containers[1].VolumeMounts, 3, "The dind container should have 3 volume mounts, dind-cert, work and externals") + assert.Len(t, ars.Spec.Template.Spec.Containers[1].VolumeMounts, 3, "The dind container should have 3 volume mounts, dind-sock, work and externals") assert.Equal(t, "work", ars.Spec.Template.Spec.Containers[1].VolumeMounts[0].Name) assert.Equal(t, "/home/runner/_work", ars.Spec.Template.Spec.Containers[1].VolumeMounts[0].MountPath) - assert.Equal(t, "dind-cert", ars.Spec.Template.Spec.Containers[1].VolumeMounts[1].Name) - assert.Equal(t, "/certs/client", ars.Spec.Template.Spec.Containers[1].VolumeMounts[1].MountPath) + assert.Equal(t, "dind-sock", ars.Spec.Template.Spec.Containers[1].VolumeMounts[1].Name) + assert.Equal(t, "/run/docker", ars.Spec.Template.Spec.Containers[1].VolumeMounts[1].MountPath) assert.Equal(t, "dind-externals", ars.Spec.Template.Spec.Containers[1].VolumeMounts[2].Name) assert.Equal(t, "/home/runner/externals", ars.Spec.Template.Spec.Containers[1].VolumeMounts[2].MountPath) assert.Len(t, ars.Spec.Template.Spec.Volumes, 3, "Volumes should be 3") - assert.Equal(t, "dind-cert", ars.Spec.Template.Spec.Volumes[0].Name, "Volume name should be dind-cert") + assert.Equal(t, "dind-sock", ars.Spec.Template.Spec.Volumes[0].Name, "Volume name should be dind-sock") assert.Equal(t, "dind-externals", ars.Spec.Template.Spec.Volumes[1].Name, "Volume name should be dind-externals") assert.Equal(t, "work", ars.Spec.Template.Spec.Volumes[2].Name, "Volume name should be work") assert.NotNil(t, ars.Spec.Template.Spec.Volumes[2].EmptyDir, "Volume work should be an emptyDir") @@ -1833,10 +1829,6 @@ func TestTemplateRenderedAutoScalingRunnerSet_DinDMergePodSpec(t *testing.T) { assert.Equal(t, "tcp://localhost:9999", ars.Spec.Template.Spec.Containers[0].Env[0].Value, "DOCKER_HOST should be set to `tcp://localhost:9999`") assert.Equal(t, "MY_NODE_NAME", ars.Spec.Template.Spec.Containers[0].Env[1].Name, "MY_NODE_NAME should be set") assert.Equal(t, "spec.nodeName", ars.Spec.Template.Spec.Containers[0].Env[1].ValueFrom.FieldRef.FieldPath, "MY_NODE_NAME should be set to `spec.nodeName`") - assert.Equal(t, "DOCKER_TLS_VERIFY", ars.Spec.Template.Spec.Containers[0].Env[2].Name, "DOCKER_TLS_VERIFY should be set") - assert.Equal(t, "1", ars.Spec.Template.Spec.Containers[0].Env[2].Value, "DOCKER_TLS_VERIFY should be set to `1`") - assert.Equal(t, "DOCKER_CERT_PATH", ars.Spec.Template.Spec.Containers[0].Env[3].Name, "DOCKER_CERT_PATH should be set") - assert.Equal(t, "/certs/client", ars.Spec.Template.Spec.Containers[0].Env[3].Value, "DOCKER_CERT_PATH should be set to `/certs/client`") assert.Equal(t, "work", ars.Spec.Template.Spec.Containers[0].VolumeMounts[0].Name, "VolumeMount name should be work") assert.Equal(t, "/work", ars.Spec.Template.Spec.Containers[0].VolumeMounts[0].MountPath, "VolumeMount mountPath should be /work") assert.Equal(t, "others", ars.Spec.Template.Spec.Containers[0].VolumeMounts[1].Name, "VolumeMount name should be others") diff --git a/charts/gha-runner-scale-set/values.yaml b/charts/gha-runner-scale-set/values.yaml index 96b3d474..64642edb 100644 --- a/charts/gha-runner-scale-set/values.yaml +++ b/charts/gha-runner-scale-set/values.yaml @@ -122,32 +122,35 @@ template: ## command: ["/home/runner/run.sh"] ## env: ## - name: DOCKER_HOST - ## value: tcp://localhost:2376 - ## - name: DOCKER_TLS_VERIFY - ## value: "1" - ## - name: DOCKER_CERT_PATH - ## value: /certs/client + ## value: unix:///run/docker/docker.sock ## volumeMounts: ## - name: work ## mountPath: /home/runner/_work - ## - name: dind-cert - ## mountPath: /certs/client + ## - name: dind-sock + ## mountPath: /run/docker ## readOnly: true ## - name: dind ## image: docker:dind + ## args: + ## - dockerd + ## - --host=unix:///run/docker/docker.sock + ## - --group=$(DOCKER_GROUP_GID) + ## env: + ## - name: DOCKER_GROUP_GID + ## value: "123" ## securityContext: ## privileged: true ## volumeMounts: ## - name: work ## mountPath: /home/runner/_work - ## - name: dind-cert - ## mountPath: /certs/client + ## - name: dind-sock + ## mountPath: /run/docker ## - name: dind-externals ## mountPath: /home/runner/externals ## volumes: ## - name: work ## emptyDir: {} - ## - name: dind-cert + ## - name: dind-sock ## emptyDir: {} ## - name: dind-externals ## emptyDir: {}