Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Support arbitrarily setting privileged: true for runner container #1383

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 23 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -435,6 +435,17 @@ spec:
requests:
cpu: "2.0"
memory: "4Gi"
# This is an advanced configuration. Don't touch it unless you know what you're doing.
securityContext:
# Usually, the runner container's privileged field is derived from dockerdWithinRunnerContainer.
# But in the case where you need to run privileged job steps even if you don't use docker/don't need dockerd within the runner container,
# just specified `privileged: true` like this.
# See https://github.com/actions-runner-controller/actions-runner-controller/issues/1282
# Do note that specifying `privileged: false` while using dind is very likely to fail, even if you use some vm-based container runtimes
# like firecracker and kata. Basically they run containers within dedicated micro vms and so
# it's more like you can use `privileged: true` safer with those runtimes.
#
# privileged: true
- name: docker
resources:
limits:
Expand Down Expand Up @@ -1125,6 +1136,18 @@ spec:
# This must match the name of a RuntimeClass resource available on the cluster.
# More info: https://kubernetes.io/docs/concepts/containers/runtime-class
runtimeClassName: "runc"
# This is an advanced configuration. Don't touch it unless you know what you're doing.
containers:
- name: runner
# Usually, the runner container's privileged field is derived from dockerdWithinRunnerContainer.
# But in the case where you need to run privileged job steps even if you don't use docker/don't need dockerd within the runner container,
# just specified `privileged: true` like this.
# See https://github.com/actions-runner-controller/actions-runner-controller/issues/1282
# Do note that specifying `privileged: false` while using dind is very likely to fail, even if you use some vm-based container runtimes
# like firecracker and kata. Basically they run containers within dedicated micro vms and so
# it's more like you can use `privileged: true` safer with those runtimes.
#
# privileged: true
```

### Custom Volume mounts
Expand Down
5 changes: 2 additions & 3 deletions controllers/new_runner_pod_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -400,8 +400,7 @@ func TestNewRunnerPod(t *testing.T) {
DockerEnabled: boolPtr(false),
},
want: newTestPod(dockerDisabled, func(p *corev1.Pod) {
// TODO
// p.Spec.Containers[0].SecurityContext.Privileged = boolPtr(true)
p.Spec.Containers[0].SecurityContext.Privileged = boolPtr(true)
}),
},
}
Expand Down Expand Up @@ -904,7 +903,7 @@ func TestNewRunnerPodFromRunnerController(t *testing.T) {
},

want: newTestPod(dockerDisabled, func(p *corev1.Pod) {
// p.Spec.Containers[0].SecurityContext.Privileged = boolPtr(true)
p.Spec.Containers[0].SecurityContext.Privileged = boolPtr(true)
}),
},
}
Expand Down
52 changes: 41 additions & 11 deletions controllers/runner_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -349,25 +349,52 @@ func (r *RunnerReconciler) newPod(runner v1alpha1.Runner) (corev1.Pod, error) {

if len(runner.Spec.Containers) == 0 {
template.Spec.Containers = append(template.Spec.Containers, corev1.Container{
Name: "runner",
ImagePullPolicy: runner.Spec.ImagePullPolicy,
EnvFrom: runner.Spec.EnvFrom,
Env: runner.Spec.Env,
Resources: runner.Spec.Resources,
Name: "runner",
})

if (runner.Spec.DockerEnabled == nil || *runner.Spec.DockerEnabled) && (runner.Spec.DockerdWithinRunnerContainer == nil || !*runner.Spec.DockerdWithinRunnerContainer) {
template.Spec.Containers = append(template.Spec.Containers, corev1.Container{
Name: "docker",
VolumeMounts: runner.Spec.DockerVolumeMounts,
Resources: runner.Spec.DockerdContainerResources,
Env: runner.Spec.DockerEnv,
Name: "docker",
})
}
} else {
template.Spec.Containers = runner.Spec.Containers
}

for i, c := range template.Spec.Containers {
switch c.Name {
case "runner":
if c.ImagePullPolicy == "" {
template.Spec.Containers[i].ImagePullPolicy = runner.Spec.ImagePullPolicy
}
if len(c.EnvFrom) == 0 {
template.Spec.Containers[i].EnvFrom = runner.Spec.EnvFrom
}
if len(c.Env) == 0 {
template.Spec.Containers[i].Env = runner.Spec.Env
}
if len(c.Resources.Requests) == 0 {
template.Spec.Containers[i].Resources.Requests = runner.Spec.Resources.Requests
}
if len(c.Resources.Limits) == 0 {
template.Spec.Containers[i].Resources.Limits = runner.Spec.Resources.Limits
}
case "docker":
if len(c.VolumeMounts) == 0 {
template.Spec.Containers[i].VolumeMounts = runner.Spec.DockerVolumeMounts
}
if len(c.Resources.Limits) == 0 {
template.Spec.Containers[i].Resources.Limits = runner.Spec.DockerdContainerResources.Limits
}
if len(c.Resources.Requests) == 0 {
template.Spec.Containers[i].Resources.Requests = runner.Spec.DockerdContainerResources.Requests
}
if len(c.Env) == 0 {
template.Spec.Containers[i].Env = runner.Spec.DockerEnv
}
}
}

template.Spec.SecurityContext = runner.Spec.SecurityContext
template.Spec.EnableServiceLinks = runner.Spec.EnableServiceLinks

Expand Down Expand Up @@ -624,8 +651,11 @@ func newRunnerPod(runnerName string, template corev1.Pod, runnerSpec v1alpha1.Ru
if runnerContainer.SecurityContext == nil {
runnerContainer.SecurityContext = &corev1.SecurityContext{}
}
// Runner need to run privileged if it contains DinD
runnerContainer.SecurityContext.Privileged = &dockerdInRunnerPrivileged

if runnerContainer.SecurityContext.Privileged == nil {
// Runner need to run privileged if it contains DinD
runnerContainer.SecurityContext.Privileged = &dockerdInRunnerPrivileged
}

pod := template.DeepCopy()

Expand Down