diff --git a/controllers/actions.summerwind.net/integration_test.go b/controllers/actions.summerwind.net/integration_test.go index d10184eb84..2dc34ca7b4 100644 --- a/controllers/actions.summerwind.net/integration_test.go +++ b/controllers/actions.summerwind.net/integration_test.go @@ -105,12 +105,14 @@ func SetupIntegrationTest(ctx2 context.Context) *testEnvironment { Log: logf.Log, Recorder: mgr.GetEventRecorderFor("runnerreplicaset-controller"), GitHubClient: multiClient, - RunnerImage: "example/runner:test", - DockerImage: "example/docker:test", Name: controllerName("runner"), RegistrationRecheckInterval: time.Millisecond * 100, RegistrationRecheckJitter: time.Millisecond * 10, UnregistrationRetryDelay: 1 * time.Second, + RunnerPodDefaults: RunnerPodDefaults{ + RunnerImage: "example/runner:test", + DockerImage: "example/docker:test", + }, } err = runnerController.SetupWithManager(mgr) Expect(err).NotTo(HaveOccurred(), "failed to setup runner controller") diff --git a/controllers/actions.summerwind.net/new_runner_pod_test.go b/controllers/actions.summerwind.net/new_runner_pod_test.go index fb9b6653a8..dfd0b2faf5 100644 --- a/controllers/actions.summerwind.net/new_runner_pod_test.go +++ b/controllers/actions.summerwind.net/new_runner_pod_test.go @@ -15,6 +15,21 @@ import ( "sigs.k8s.io/controller-runtime/pkg/client" ) +func newRunnerPod(template corev1.Pod, runnerSpec arcv1alpha1.RunnerConfig, githubBaseURL string, d RunnerPodDefaults) (corev1.Pod, error) { + return newRunnerPodWithContainerMode("", template, runnerSpec, githubBaseURL, d) +} + +func setEnv(c *corev1.Container, name, value string) { + for j := range c.Env { + e := &c.Env[j] + + if e.Name == name { + e.Value = value + return + } + } +} + func newWorkGenericEphemeralVolume(t *testing.T, storageReq string) corev1.Volume { GBs, err := resource.ParseQuantity(storageReq) if err != nil { @@ -171,7 +186,7 @@ func TestNewRunnerPod(t *testing.T) { Env: []corev1.EnvVar{ { Name: "DOCKER_GROUP_GID", - Value: "121", + Value: "1234", }, }, VolumeMounts: []corev1.VolumeMount{ @@ -397,6 +412,50 @@ func TestNewRunnerPod(t *testing.T) { config: arcv1alpha1.RunnerConfig{}, want: newTestPod(base, nil), }, + { + description: "it should respect DOCKER_GROUP_GID of the dockerd sidecar container", + template: corev1.Pod{ + Spec: corev1.PodSpec{ + Containers: []corev1.Container{ + { + Name: "docker", + Env: []corev1.EnvVar{ + { + Name: "DOCKER_GROUP_GID", + Value: "2345", + }, + }, + }, + }, + }, + }, + config: arcv1alpha1.RunnerConfig{}, + want: newTestPod(base, func(p *corev1.Pod) { + setEnv(&p.Spec.Containers[1], "DOCKER_GROUP_GID", "2345") + }), + }, + { + description: "it should add DOCKER_GROUP_GID=1001 to the dockerd sidecar container for Ubuntu 20.04 runners", + template: corev1.Pod{}, + config: arcv1alpha1.RunnerConfig{ + Image: "ghcr.io/summerwind/actions-runner:ubuntu-20.04-20210726-1", + }, + want: newTestPod(base, func(p *corev1.Pod) { + setEnv(&p.Spec.Containers[1], "DOCKER_GROUP_GID", "1001") + p.Spec.Containers[0].Image = "ghcr.io/summerwind/actions-runner:ubuntu-20.04-20210726-1" + }), + }, + { + description: "it should add DOCKER_GROUP_GID=121 to the dockerd sidecar container for Ubuntu 22.04 runners", + template: corev1.Pod{}, + config: arcv1alpha1.RunnerConfig{ + Image: "ghcr.io/summerwind/actions-runner:ubuntu-22.04-20210726-1", + }, + want: newTestPod(base, func(p *corev1.Pod) { + setEnv(&p.Spec.Containers[1], "DOCKER_GROUP_GID", "121") + p.Spec.Containers[0].Image = "ghcr.io/summerwind/actions-runner:ubuntu-22.04-20210726-1" + }), + }, { description: "dockerdWithinRunnerContainer=true should set privileged=true and omit the dind sidecar container", template: corev1.Pod{}, @@ -552,7 +611,14 @@ func TestNewRunnerPod(t *testing.T) { for i := range testcases { tc := testcases[i] t.Run(tc.description, func(t *testing.T) { - got, err := newRunnerPod(tc.template, tc.config, defaultRunnerImage, defaultRunnerImagePullSecrets, defaultDockerImage, defaultDockerRegistryMirror, githubBaseURL, false) + got, err := newRunnerPod(tc.template, tc.config, githubBaseURL, RunnerPodDefaults{ + RunnerImage: defaultRunnerImage, + RunnerImagePullSecrets: defaultRunnerImagePullSecrets, + DockerImage: defaultDockerImage, + DockerRegistryMirror: defaultDockerRegistryMirror, + DockerGID: "1234", + UseRunnerStatusUpdateHook: false, + }) require.NoError(t, err) require.Equal(t, tc.want, got) }) @@ -713,7 +779,7 @@ func TestNewRunnerPodFromRunnerController(t *testing.T) { Env: []corev1.EnvVar{ { Name: "DOCKER_GROUP_GID", - Value: "121", + Value: "1234", }, }, VolumeMounts: []corev1.VolumeMount{ @@ -1171,6 +1237,7 @@ func TestNewRunnerPodFromRunnerController(t *testing.T) { defaultRunnerImage = "default-runner-image" defaultRunnerImagePullSecrets = []string{} defaultDockerImage = "default-docker-image" + defaultDockerGID = "1234" defaultDockerRegistryMirror = "" githubBaseURL = "api.github.com" ) @@ -1190,12 +1257,15 @@ func TestNewRunnerPodFromRunnerController(t *testing.T) { t.Run(tc.description, func(t *testing.T) { r := &RunnerReconciler{ - RunnerImage: defaultRunnerImage, - RunnerImagePullSecrets: defaultRunnerImagePullSecrets, - DockerImage: defaultDockerImage, - DockerRegistryMirror: defaultDockerRegistryMirror, - GitHubClient: multiClient, - Scheme: scheme, + GitHubClient: multiClient, + Scheme: scheme, + RunnerPodDefaults: RunnerPodDefaults{ + RunnerImage: defaultRunnerImage, + RunnerImagePullSecrets: defaultRunnerImagePullSecrets, + DockerImage: defaultDockerImage, + DockerRegistryMirror: defaultDockerRegistryMirror, + DockerGID: defaultDockerGID, + }, } got, err := r.newPod(tc.runner) require.NoError(t, err) diff --git a/controllers/actions.summerwind.net/runner_controller.go b/controllers/actions.summerwind.net/runner_controller.go index 4fa00968fd..c0632ad5d1 100644 --- a/controllers/actions.summerwind.net/runner_controller.go +++ b/controllers/actions.summerwind.net/runner_controller.go @@ -68,15 +68,24 @@ type RunnerReconciler struct { Recorder record.EventRecorder Scheme *runtime.Scheme GitHubClient *MultiGitHubClient - RunnerImage string - RunnerImagePullSecrets []string - DockerImage string - DockerRegistryMirror string Name string RegistrationRecheckInterval time.Duration RegistrationRecheckJitter time.Duration - UseRunnerStatusUpdateHook bool UnregistrationRetryDelay time.Duration + + RunnerPodDefaults RunnerPodDefaults +} + +type RunnerPodDefaults struct { + RunnerImage string + RunnerImagePullSecrets []string + DockerImage string + DockerRegistryMirror string + // The default Docker group ID to use for the dockerd sidecar container. + // Ubuntu 20.04 runner images assumes 1001 and the 22.04 variant assumes 121 by default. + DockerGID string + + UseRunnerStatusUpdateHook bool } // +kubebuilder:rbac:groups=actions.summerwind.dev,resources=runners,verbs=get;list;watch;create;update;patch;delete @@ -145,7 +154,7 @@ func (r *RunnerReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctr ready := runnerPodReady(&pod) - if (runner.Status.Phase != phase || runner.Status.Ready != ready) && !r.UseRunnerStatusUpdateHook || runner.Status.Phase == "" && r.UseRunnerStatusUpdateHook { + if (runner.Status.Phase != phase || runner.Status.Ready != ready) && !r.RunnerPodDefaults.UseRunnerStatusUpdateHook || runner.Status.Phase == "" && r.RunnerPodDefaults.UseRunnerStatusUpdateHook { if pod.Status.Phase == corev1.PodRunning { // Seeing this message, you can expect the runner to become `Running` soon. log.V(1).Info( @@ -292,7 +301,7 @@ func (r *RunnerReconciler) processRunnerCreation(ctx context.Context, runner v1a return ctrl.Result{}, err } - needsServiceAccount := runner.Spec.ServiceAccountName == "" && (r.UseRunnerStatusUpdateHook || runner.Spec.ContainerMode == "kubernetes") + needsServiceAccount := runner.Spec.ServiceAccountName == "" && (r.RunnerPodDefaults.UseRunnerStatusUpdateHook || runner.Spec.ContainerMode == "kubernetes") if needsServiceAccount { serviceAccount := &corev1.ServiceAccount{ ObjectMeta: metav1.ObjectMeta{ @@ -306,7 +315,7 @@ func (r *RunnerReconciler) processRunnerCreation(ctx context.Context, runner v1a rules := []rbacv1.PolicyRule{} - if r.UseRunnerStatusUpdateHook { + if r.RunnerPodDefaults.UseRunnerStatusUpdateHook { rules = append(rules, []rbacv1.PolicyRule{ { APIGroups: []string{"actions.summerwind.dev"}, @@ -583,7 +592,7 @@ func (r *RunnerReconciler) newPod(runner v1alpha1.Runner) (corev1.Pod, error) { } } - pod, err := newRunnerPodWithContainerMode(runner.Spec.ContainerMode, template, runner.Spec.RunnerConfig, r.RunnerImage, r.RunnerImagePullSecrets, r.DockerImage, r.DockerRegistryMirror, ghc.GithubBaseURL, r.UseRunnerStatusUpdateHook) + pod, err := newRunnerPodWithContainerMode(runner.Spec.ContainerMode, template, runner.Spec.RunnerConfig, ghc.GithubBaseURL, r.RunnerPodDefaults) if err != nil { return pod, err } @@ -634,7 +643,7 @@ func (r *RunnerReconciler) newPod(runner v1alpha1.Runner) (corev1.Pod, error) { if runnerSpec.ServiceAccountName != "" { pod.Spec.ServiceAccountName = runnerSpec.ServiceAccountName - } else if r.UseRunnerStatusUpdateHook || runner.Spec.ContainerMode == "kubernetes" { + } else if r.RunnerPodDefaults.UseRunnerStatusUpdateHook || runner.Spec.ContainerMode == "kubernetes" { pod.Spec.ServiceAccountName = runner.ObjectMeta.Name } @@ -754,13 +763,19 @@ func runnerHookEnvs(pod *corev1.Pod) ([]corev1.EnvVar, error) { }, nil } -func newRunnerPodWithContainerMode(containerMode string, template corev1.Pod, runnerSpec v1alpha1.RunnerConfig, defaultRunnerImage string, defaultRunnerImagePullSecrets []string, defaultDockerImage, defaultDockerRegistryMirror string, githubBaseURL string, useRunnerStatusUpdateHook bool) (corev1.Pod, error) { +func newRunnerPodWithContainerMode(containerMode string, template corev1.Pod, runnerSpec v1alpha1.RunnerConfig, githubBaseURL string, d RunnerPodDefaults) (corev1.Pod, error) { var ( privileged bool = true dockerdInRunner bool = runnerSpec.DockerdWithinRunnerContainer != nil && *runnerSpec.DockerdWithinRunnerContainer dockerEnabled bool = runnerSpec.DockerEnabled == nil || *runnerSpec.DockerEnabled ephemeral bool = runnerSpec.Ephemeral == nil || *runnerSpec.Ephemeral dockerdInRunnerPrivileged bool = dockerdInRunner + + defaultRunnerImage = d.RunnerImage + defaultRunnerImagePullSecrets = d.RunnerImagePullSecrets + defaultDockerImage = d.DockerImage + defaultDockerRegistryMirror = d.DockerRegistryMirror + useRunnerStatusUpdateHook = d.UseRunnerStatusUpdateHook ) if containerMode == "kubernetes" { @@ -1013,10 +1028,22 @@ func newRunnerPodWithContainerMode(containerMode string, template corev1.Pod, ru // for actions-runner-controller) so typically should not need to be // overridden if ok, _ := envVarPresent("DOCKER_GROUP_GID", dockerdContainer.Env); !ok { + gid := d.DockerGID + // We default to gid 121 for Ubuntu 22.04 images + // See below for more details + // - https://github.com/actions/actions-runner-controller/issues/2490#issuecomment-1501561923 + // - https://github.com/actions/actions-runner-controller/blob/8869ad28bb5a1daaedefe0e988571fe1fb36addd/runner/actions-runner.ubuntu-20.04.dockerfile#L14 + // - https://github.com/actions/actions-runner-controller/blob/8869ad28bb5a1daaedefe0e988571fe1fb36addd/runner/actions-runner.ubuntu-22.04.dockerfile#L12 + if strings.Contains(runnerContainer.Image, "22.04") { + gid = "121" + } else if strings.Contains(runnerContainer.Image, "20.04") { + gid = "1001" + } + dockerdContainer.Env = append(dockerdContainer.Env, corev1.EnvVar{ Name: "DOCKER_GROUP_GID", - Value: "121", + Value: gid, }) } dockerdContainer.Args = append(dockerdContainer.Args, "--group=$(DOCKER_GROUP_GID)") @@ -1240,10 +1267,6 @@ func newRunnerPodWithContainerMode(containerMode string, template corev1.Pod, ru return *pod, nil } -func newRunnerPod(template corev1.Pod, runnerSpec v1alpha1.RunnerConfig, defaultRunnerImage string, defaultRunnerImagePullSecrets []string, defaultDockerImage, defaultDockerRegistryMirror string, githubBaseURL string, useRunnerStatusUpdateHookEphemeralRole bool) (corev1.Pod, error) { - return newRunnerPodWithContainerMode("", template, runnerSpec, defaultRunnerImage, defaultRunnerImagePullSecrets, defaultDockerImage, defaultDockerRegistryMirror, githubBaseURL, useRunnerStatusUpdateHookEphemeralRole) -} - func (r *RunnerReconciler) SetupWithManager(mgr ctrl.Manager) error { name := "runner-controller" if r.Name != "" { diff --git a/controllers/actions.summerwind.net/runnerset_controller.go b/controllers/actions.summerwind.net/runnerset_controller.go index f937237ff8..5fd825a218 100644 --- a/controllers/actions.summerwind.net/runnerset_controller.go +++ b/controllers/actions.summerwind.net/runnerset_controller.go @@ -45,13 +45,10 @@ type RunnerSetReconciler struct { Recorder record.EventRecorder Scheme *runtime.Scheme - CommonRunnerLabels []string - GitHubClient *MultiGitHubClient - RunnerImage string - RunnerImagePullSecrets []string - DockerImage string - DockerRegistryMirror string - UseRunnerStatusUpdateHook bool + CommonRunnerLabels []string + GitHubClient *MultiGitHubClient + + RunnerPodDefaults RunnerPodDefaults } // +kubebuilder:rbac:groups=actions.summerwind.dev,resources=runnersets,verbs=get;list;watch;create;update;patch;delete @@ -231,7 +228,7 @@ func (r *RunnerSetReconciler) newStatefulSet(ctx context.Context, runnerSet *v1a githubBaseURL := ghc.GithubBaseURL - pod, err := newRunnerPodWithContainerMode(runnerSet.Spec.RunnerConfig.ContainerMode, template, runnerSet.Spec.RunnerConfig, r.RunnerImage, r.RunnerImagePullSecrets, r.DockerImage, r.DockerRegistryMirror, githubBaseURL, r.UseRunnerStatusUpdateHook) + pod, err := newRunnerPodWithContainerMode(runnerSet.Spec.RunnerConfig.ContainerMode, template, runnerSet.Spec.RunnerConfig, githubBaseURL, r.RunnerPodDefaults) if err != nil { return nil, err } diff --git a/main.go b/main.go index aee0322572..543db603ab 100644 --- a/main.go +++ b/main.go @@ -45,6 +45,7 @@ import ( const ( defaultRunnerImage = "summerwind/actions-runner:latest" defaultDockerImage = "docker:dind" + defaultDockerGID = "1001" ) var scheme = runtime.NewScheme() @@ -76,18 +77,15 @@ func main() { autoScalingRunnerSetOnly bool enableLeaderElection bool disableAdmissionWebhook bool - runnerStatusUpdateHook bool leaderElectionId string port int syncPeriod time.Duration defaultScaleDownDelay time.Duration - runnerImage string runnerImagePullSecrets stringSlice + runnerPodDefaults actionssummerwindnet.RunnerPodDefaults - dockerImage string - dockerRegistryMirror string namespace string logLevel string logFormat string @@ -108,10 +106,11 @@ func main() { flag.BoolVar(&enableLeaderElection, "enable-leader-election", false, "Enable leader election for controller manager. Enabling this will ensure there is only one active controller manager.") flag.StringVar(&leaderElectionId, "leader-election-id", "actions-runner-controller", "Controller id for leader election.") - flag.StringVar(&runnerImage, "runner-image", defaultRunnerImage, "The image name of self-hosted runner container to use by default if one isn't defined in yaml.") - flag.StringVar(&dockerImage, "docker-image", defaultDockerImage, "The image name of docker sidecar container to use by default if one isn't defined in yaml.") + flag.StringVar(&runnerPodDefaults.RunnerImage, "runner-image", defaultRunnerImage, "The image name of self-hosted runner container to use by default if one isn't defined in yaml.") + flag.StringVar(&runnerPodDefaults.DockerImage, "docker-image", defaultDockerImage, "The image name of docker sidecar container to use by default if one isn't defined in yaml.") + flag.StringVar(&runnerPodDefaults.DockerGID, "docker-gid", defaultDockerGID, "The default GID of docker group in the docker sidecar container. Use 1001 for dockerd sidecars of Ubuntu 20.04 runners 121 for Ubuntu 22.04.") flag.Var(&runnerImagePullSecrets, "runner-image-pull-secret", "The default image-pull secret name for self-hosted runner container.") - flag.StringVar(&dockerRegistryMirror, "docker-registry-mirror", "", "The default Docker Registry Mirror used by runners.") + flag.StringVar(&runnerPodDefaults.DockerRegistryMirror, "docker-registry-mirror", "", "The default Docker Registry Mirror used by runners.") flag.StringVar(&c.Token, "github-token", c.Token, "The personal access token of GitHub.") flag.StringVar(&c.EnterpriseURL, "github-enterprise-url", c.EnterpriseURL, "Enterprise URL to be used for your GitHub API calls") flag.Int64Var(&c.AppID, "github-app-id", c.AppID, "The application ID of GitHub App.") @@ -122,7 +121,7 @@ func main() { flag.StringVar(&c.BasicauthUsername, "github-basicauth-username", c.BasicauthUsername, "Username for GitHub basic auth to use instead of PAT or GitHub APP in case it's running behind a proxy API") flag.StringVar(&c.BasicauthPassword, "github-basicauth-password", c.BasicauthPassword, "Password for GitHub basic auth to use instead of PAT or GitHub APP in case it's running behind a proxy API") flag.StringVar(&c.RunnerGitHubURL, "runner-github-url", c.RunnerGitHubURL, "GitHub URL to be used by runners during registration") - flag.BoolVar(&runnerStatusUpdateHook, "runner-status-update-hook", false, "Use custom RBAC for runners (role, role binding and service account).") + flag.BoolVar(&runnerPodDefaults.UseRunnerStatusUpdateHook, "runner-status-update-hook", false, "Use custom RBAC for runners (role, role binding and service account).") flag.DurationVar(&defaultScaleDownDelay, "default-scale-down-delay", actionssummerwindnet.DefaultScaleDownDelay, "The approximate delay for a scale down followed by a scale up, used to prevent flapping (down->up->down->... loop)") flag.IntVar(&port, "port", 9443, "The port to which the admission webhook endpoint should bind") flag.DurationVar(&syncPeriod, "sync-period", 1*time.Minute, "Determines the minimum frequency at which K8s resources managed by this controller are reconciled.") @@ -135,6 +134,8 @@ func main() { flag.Var(&autoScalerImagePullSecrets, "auto-scaler-image-pull-secrets", "The default image-pull secret name for auto-scaler listener container.") flag.Parse() + runnerPodDefaults.RunnerImagePullSecrets = runnerImagePullSecrets + log, err := logging.NewLogger(logLevel, logFormat) if err != nil { fmt.Fprintf(os.Stderr, "Error: creating logger: %v\n", err) @@ -255,16 +256,11 @@ func main() { ) runnerReconciler := &actionssummerwindnet.RunnerReconciler{ - Client: mgr.GetClient(), - Log: log.WithName("runner"), - Scheme: mgr.GetScheme(), - GitHubClient: multiClient, - DockerImage: dockerImage, - DockerRegistryMirror: dockerRegistryMirror, - UseRunnerStatusUpdateHook: runnerStatusUpdateHook, - // Defaults for self-hosted runner containers - RunnerImage: runnerImage, - RunnerImagePullSecrets: runnerImagePullSecrets, + Client: mgr.GetClient(), + Log: log.WithName("runner"), + Scheme: mgr.GetScheme(), + GitHubClient: multiClient, + RunnerPodDefaults: runnerPodDefaults, } if err = runnerReconciler.SetupWithManager(mgr); err != nil { @@ -296,17 +292,12 @@ func main() { } runnerSetReconciler := &actionssummerwindnet.RunnerSetReconciler{ - Client: mgr.GetClient(), - Log: log.WithName("runnerset"), - Scheme: mgr.GetScheme(), - CommonRunnerLabels: commonRunnerLabels, - DockerImage: dockerImage, - DockerRegistryMirror: dockerRegistryMirror, - GitHubClient: multiClient, - // Defaults for self-hosted runner containers - RunnerImage: runnerImage, - RunnerImagePullSecrets: runnerImagePullSecrets, - UseRunnerStatusUpdateHook: runnerStatusUpdateHook, + Client: mgr.GetClient(), + Log: log.WithName("runnerset"), + Scheme: mgr.GetScheme(), + CommonRunnerLabels: commonRunnerLabels, + GitHubClient: multiClient, + RunnerPodDefaults: runnerPodDefaults, } if err = runnerSetReconciler.SetupWithManager(mgr); err != nil { @@ -319,8 +310,9 @@ func main() { "version", build.Version, "default-scale-down-delay", defaultScaleDownDelay, "sync-period", syncPeriod, - "default-runner-image", runnerImage, - "default-docker-image", dockerImage, + "default-runner-image", runnerPodDefaults.RunnerImage, + "default-docker-image", runnerPodDefaults.DockerImage, + "default-docker-gid", runnerPodDefaults.DockerGID, "common-runnner-labels", commonRunnerLabels, "leader-election-enabled", enableLeaderElection, "leader-election-id", leaderElectionId,