diff --git a/controllers/actions.summerwind.net/new_runner_pod_test.go b/controllers/actions.summerwind.net/new_runner_pod_test.go index 4d3b419d..fb9b6653 100644 --- a/controllers/actions.summerwind.net/new_runner_pod_test.go +++ b/controllers/actions.summerwind.net/new_runner_pod_test.go @@ -76,9 +76,12 @@ func TestNewRunnerPod(t *testing.T) { }, }, { - Name: "certs-client", + Name: "docker-sock", VolumeSource: corev1.VolumeSource{ - EmptyDir: &corev1.EmptyDirVolumeSource{}, + EmptyDir: &corev1.EmptyDirVolumeSource{ + Medium: corev1.StorageMediumMemory, + SizeLimit: resource.NewScaledQuantity(1, resource.Mega), + }, }, }, }, @@ -137,15 +140,7 @@ func TestNewRunnerPod(t *testing.T) { }, { 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: []corev1.VolumeMount{ @@ -158,9 +153,8 @@ func TestNewRunnerPod(t *testing.T) { MountPath: "/runner/_work", }, { - Name: "certs-client", - MountPath: "/certs/client", - ReadOnly: true, + Name: "docker-sock", + MountPath: "/run/docker", }, }, ImagePullPolicy: corev1.PullAlways, @@ -169,10 +163,15 @@ func TestNewRunnerPod(t *testing.T) { { Name: "docker", Image: "default-docker-image", + Args: []string{ + "dockerd", + "--host=unix:///run/docker/docker.sock", + "--group=$(DOCKER_GROUP_GID)", + }, Env: []corev1.EnvVar{ { - Name: "DOCKER_TLS_CERTDIR", - Value: "/certs", + Name: "DOCKER_GROUP_GID", + Value: "121", }, }, VolumeMounts: []corev1.VolumeMount{ @@ -181,8 +180,8 @@ func TestNewRunnerPod(t *testing.T) { MountPath: "/runner", }, { - Name: "certs-client", - MountPath: "/certs/client", + Name: "docker-sock", + MountPath: "/run/docker", }, { Name: "work", @@ -485,9 +484,12 @@ func TestNewRunnerPod(t *testing.T) { }, }, { - Name: "certs-client", + Name: "docker-sock", VolumeSource: corev1.VolumeSource{ - EmptyDir: &corev1.EmptyDirVolumeSource{}, + EmptyDir: &corev1.EmptyDirVolumeSource{ + Medium: corev1.StorageMediumMemory, + SizeLimit: resource.NewScaledQuantity(1, resource.Mega), + }, }, }, } @@ -501,9 +503,8 @@ func TestNewRunnerPod(t *testing.T) { MountPath: "/runner", }, { - Name: "certs-client", - MountPath: "/certs/client", - ReadOnly: true, + Name: "docker-sock", + MountPath: "/run/docker", }, } }), @@ -527,9 +528,12 @@ func TestNewRunnerPod(t *testing.T) { }, }, { - Name: "certs-client", + Name: "docker-sock", VolumeSource: corev1.VolumeSource{ - EmptyDir: &corev1.EmptyDirVolumeSource{}, + EmptyDir: &corev1.EmptyDirVolumeSource{ + Medium: corev1.StorageMediumMemory, + SizeLimit: resource.NewScaledQuantity(1, resource.Mega), + }, }, }, } @@ -606,9 +610,12 @@ func TestNewRunnerPodFromRunnerController(t *testing.T) { }, }, { - Name: "certs-client", + Name: "docker-sock", VolumeSource: corev1.VolumeSource{ - EmptyDir: &corev1.EmptyDirVolumeSource{}, + EmptyDir: &corev1.EmptyDirVolumeSource{ + Medium: corev1.StorageMediumMemory, + SizeLimit: resource.NewScaledQuantity(1, resource.Mega), + }, }, }, }, @@ -667,15 +674,7 @@ func TestNewRunnerPodFromRunnerController(t *testing.T) { }, { 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", }, { Name: "RUNNER_NAME", @@ -696,9 +695,8 @@ func TestNewRunnerPodFromRunnerController(t *testing.T) { MountPath: "/runner/_work", }, { - Name: "certs-client", - MountPath: "/certs/client", - ReadOnly: true, + Name: "docker-sock", + MountPath: "/run/docker", }, }, ImagePullPolicy: corev1.PullAlways, @@ -707,10 +705,15 @@ func TestNewRunnerPodFromRunnerController(t *testing.T) { { Name: "docker", Image: "default-docker-image", + Args: []string{ + "dockerd", + "--host=unix:///run/docker/docker.sock", + "--group=$(DOCKER_GROUP_GID)", + }, Env: []corev1.EnvVar{ { - Name: "DOCKER_TLS_CERTDIR", - Value: "/certs", + Name: "DOCKER_GROUP_GID", + Value: "121", }, }, VolumeMounts: []corev1.VolumeMount{ @@ -719,8 +722,8 @@ func TestNewRunnerPodFromRunnerController(t *testing.T) { MountPath: "/runner", }, { - Name: "certs-client", - MountPath: "/certs/client", + Name: "docker-sock", + MountPath: "/run/docker", }, { Name: "work", @@ -1079,6 +1082,10 @@ func TestNewRunnerPodFromRunnerController(t *testing.T) { Name: "work", MountPath: "/runner/_work", }, + { + Name: "docker-sock", + MountPath: "/run/docker", + }, }, }, }, @@ -1097,9 +1104,12 @@ func TestNewRunnerPodFromRunnerController(t *testing.T) { }, }, { - Name: "certs-client", + Name: "docker-sock", VolumeSource: corev1.VolumeSource{ - EmptyDir: &corev1.EmptyDirVolumeSource{}, + EmptyDir: &corev1.EmptyDirVolumeSource{ + Medium: corev1.StorageMediumMemory, + SizeLimit: resource.NewScaledQuantity(1, resource.Mega), + }, }, }, workGenericEphemeralVolume, @@ -1110,13 +1120,12 @@ func TestNewRunnerPodFromRunnerController(t *testing.T) { MountPath: "/runner/_work", }, { - Name: "runner", - MountPath: "/runner", + Name: "docker-sock", + MountPath: "/run/docker", }, { - Name: "certs-client", - MountPath: "/certs/client", - ReadOnly: true, + Name: "runner", + MountPath: "/runner", }, } }), @@ -1144,9 +1153,12 @@ func TestNewRunnerPodFromRunnerController(t *testing.T) { }, }, { - Name: "certs-client", + Name: "docker-sock", VolumeSource: corev1.VolumeSource{ - EmptyDir: &corev1.EmptyDirVolumeSource{}, + EmptyDir: &corev1.EmptyDirVolumeSource{ + Medium: corev1.StorageMediumMemory, + SizeLimit: resource.NewScaledQuantity(1, resource.Mega), + }, }, }, workGenericEphemeralVolume, diff --git a/controllers/actions.summerwind.net/runner_controller.go b/controllers/actions.summerwind.net/runner_controller.go index c208d1a2..4fa00968 100644 --- a/controllers/actions.summerwind.net/runner_controller.go +++ b/controllers/actions.summerwind.net/runner_controller.go @@ -30,6 +30,7 @@ import ( "github.com/go-logr/logr" kerrors "k8s.io/apimachinery/pkg/api/errors" + "k8s.io/apimachinery/pkg/api/resource" "k8s.io/apimachinery/pkg/runtime" "k8s.io/client-go/tools/record" ctrl "sigs.k8s.io/controller-runtime" @@ -1001,6 +1002,35 @@ func newRunnerPodWithContainerMode(containerMode string, template corev1.Pod, ru ) } + // explicitly invoke `dockerd` to avoid automatic TLS / TCP binding + dockerdContainer.Args = append([]string{ + "dockerd", + "--host=unix:///run/docker/docker.sock", + }, dockerdContainer.Args...) + + // this must match a GID for the user in the runner image + // default matches GitHub Actions infra (and default runner images + // for actions-runner-controller) so typically should not need to be + // overridden + if ok, _ := envVarPresent("DOCKER_GROUP_GID", dockerdContainer.Env); !ok { + dockerdContainer.Env = append(dockerdContainer.Env, + corev1.EnvVar{ + Name: "DOCKER_GROUP_GID", + Value: "121", + }) + } + dockerdContainer.Args = append(dockerdContainer.Args, "--group=$(DOCKER_GROUP_GID)") + + // ideally, we could mount the socket directly at `/var/run/docker.sock` + // to use the default, but that's not practical since it won't exist + // when the container starts, so can't use subPath on the volume mount + runnerContainer.Env = append(runnerContainer.Env, + corev1.EnvVar{ + Name: "DOCKER_HOST", + Value: "unix:///run/docker/docker.sock", + }, + ) + if ok, _ := workVolumePresent(pod.Spec.Volumes); !ok { pod.Spec.Volumes = append(pod.Spec.Volumes, corev1.Volume{ @@ -1014,9 +1044,12 @@ func newRunnerPodWithContainerMode(containerMode string, template corev1.Pod, ru pod.Spec.Volumes = append(pod.Spec.Volumes, corev1.Volume{ - Name: "certs-client", + Name: "docker-sock", VolumeSource: corev1.VolumeSource{ - EmptyDir: &corev1.EmptyDirVolumeSource{}, + EmptyDir: &corev1.EmptyDirVolumeSource{ + Medium: corev1.StorageMediumMemory, + SizeLimit: resource.NewScaledQuantity(1, resource.Mega), + }, }, }, ) @@ -1030,28 +1063,14 @@ func newRunnerPodWithContainerMode(containerMode string, template corev1.Pod, ru ) } - runnerContainer.VolumeMounts = append(runnerContainer.VolumeMounts, - corev1.VolumeMount{ - Name: "certs-client", - MountPath: "/certs/client", - ReadOnly: true, - }, - ) - - runnerContainer.Env = append(runnerContainer.Env, []corev1.EnvVar{ - { - Name: "DOCKER_HOST", - Value: "tcp://localhost:2376", - }, - { - Name: "DOCKER_TLS_VERIFY", - Value: "1", - }, - { - Name: "DOCKER_CERT_PATH", - Value: "/certs/client", - }, - }...) + if ok, _ := volumeMountPresent("docker-sock", runnerContainer.VolumeMounts); !ok { + runnerContainer.VolumeMounts = append(runnerContainer.VolumeMounts, + corev1.VolumeMount{ + Name: "docker-sock", + MountPath: "/run/docker", + }, + ) + } // Determine the volume mounts assigned to the docker sidecar. In case extra mounts are included in the RunnerSpec, append them to the standard // set of mounts. See https://github.com/actions/actions-runner-controller/issues/435 for context. @@ -1060,14 +1079,16 @@ func newRunnerPodWithContainerMode(containerMode string, template corev1.Pod, ru Name: runnerVolumeName, MountPath: runnerVolumeMountPath, }, - { - Name: "certs-client", - MountPath: "/certs/client", - }, } - mountPresent, _ := workVolumeMountPresent(dockerdContainer.VolumeMounts) - if !mountPresent { + if p, _ := volumeMountPresent("docker-sock", dockerdContainer.VolumeMounts); !p { + dockerVolumeMounts = append(dockerVolumeMounts, corev1.VolumeMount{ + Name: "docker-sock", + MountPath: "/run/docker", + }) + } + + if p, _ := workVolumeMountPresent(dockerdContainer.VolumeMounts); !p { dockerVolumeMounts = append(dockerVolumeMounts, corev1.VolumeMount{ Name: "work", MountPath: workDir, @@ -1078,11 +1099,6 @@ func newRunnerPodWithContainerMode(containerMode string, template corev1.Pod, ru dockerdContainer.Image = defaultDockerImage } - dockerdContainer.Env = append(dockerdContainer.Env, corev1.EnvVar{ - Name: "DOCKER_TLS_CERTDIR", - Value: "/certs", - }) - if dockerdContainer.SecurityContext == nil { dockerdContainer.SecurityContext = &corev1.SecurityContext{ Privileged: &privileged, @@ -1273,6 +1289,15 @@ func removeFinalizer(finalizers []string, finalizerName string) ([]string, bool) return result, removed } +func envVarPresent(name string, items []corev1.EnvVar) (bool, int) { + for index, item := range items { + if item.Name == name { + return true, index + } + } + return false, -1 +} + func workVolumePresent(items []corev1.Volume) (bool, int) { for index, item := range items { if item.Name == "work" { @@ -1283,12 +1308,16 @@ func workVolumePresent(items []corev1.Volume) (bool, int) { } func workVolumeMountPresent(items []corev1.VolumeMount) (bool, int) { + return volumeMountPresent("work", items) +} + +func volumeMountPresent(name string, items []corev1.VolumeMount) (bool, int) { for index, item := range items { - if item.Name == "work" { + if item.Name == name { return true, index } } - return false, 0 + return false, -1 } func applyWorkVolumeClaimTemplateToPod(pod *corev1.Pod, workVolumeClaimTemplate *v1alpha1.WorkVolumeClaimTemplate, workDir string) error { diff --git a/test/e2e/e2e_test.go b/test/e2e/e2e_test.go index 48e5fc2d..f6681558 100644 --- a/test/e2e/e2e_test.go +++ b/test/e2e/e2e_test.go @@ -1096,7 +1096,6 @@ func installActionsWorkflow(t *testing.T, testName, runnerLabel, testResultCMNam if !kubernetesContainerMode { setupBuildXActionWith := &testing.With{ BuildkitdFlags: "--debug", - Endpoint: "mycontext", // As the consequence of setting `install: false`, it doesn't install buildx as an alias to `docker build` // so we need to use `docker buildx build` in the next step Install: false, @@ -1122,16 +1121,24 @@ func installActionsWorkflow(t *testing.T, testName, runnerLabel, testResultCMNam setupBuildXActionWith.Driver = "docker" dockerfile = "Dockerfile.nocache" } - steps = append(steps, - testing.Step{ + + useCustomDockerContext := os.Getenv("ARC_E2E_USE_CUSTOM_DOCKER_CONTEXT") != "" + if useCustomDockerContext { + setupBuildXActionWith.Endpoint = "mycontext" + + steps = append(steps, testing.Step{ // https://github.com/docker/buildx/issues/413#issuecomment-710660155 // To prevent setup-buildx-action from failing with: // error: could not create a builder instance with TLS data loaded from environment. Please use `docker context create ` to create a context for current environment and then create a builder instance with `docker buildx create ` Run: "docker context create mycontext", }, - testing.Step{ - Run: "docker context use mycontext", - }, + testing.Step{ + Run: "docker context use mycontext", + }, + ) + } + + steps = append(steps, testing.Step{ Name: "Set up Docker Buildx", Uses: "docker/setup-buildx-action@v1",