diff --git a/.github/workflows/runners.yaml b/.github/workflows/runners.yaml index 508236ac..7809f186 100644 --- a/.github/workflows/runners.yaml +++ b/.github/workflows/runners.yaml @@ -22,6 +22,7 @@ on: env: RUNNER_VERSION: 2.294.0 DOCKER_VERSION: 20.10.12 + RUNNER_CONTAINER_HOOKS_VERSION: 0.1.2 DOCKERHUB_USERNAME: summerwind jobs: @@ -65,6 +66,7 @@ jobs: build-args: | RUNNER_VERSION=${{ env.RUNNER_VERSION }} DOCKER_VERSION=${{ env.DOCKER_VERSION }} + RUNNER_CONTAINER_HOOKS_VERSION=${{ env.RUNNER_CONTAINER_HOOKS_VERSION }} tags: | ${{ env.DOCKERHUB_USERNAME }}/${{ matrix.name }}:v${{ env.RUNNER_VERSION }}-${{ matrix.os-name }}-${{ matrix.os-version }} ${{ env.DOCKERHUB_USERNAME }}/${{ matrix.name }}:v${{ env.RUNNER_VERSION }}-${{ matrix.os-name }}-${{ matrix.os-version }}-${{ steps.vars.outputs.sha_short }} diff --git a/README.md b/README.md index 1d061532..4c4605ab 100644 --- a/README.md +++ b/README.md @@ -30,6 +30,7 @@ ToC: - [Autoscaling to/from 0](#autoscaling-tofrom-0) - [Scheduled Overrides](#scheduled-overrides) - [Runner with DinD](#runner-with-dind) + - [Runner with k8s jobs](#runner-with-k8s-jobs) - [Additional Tweaks](#additional-tweaks) - [Custom Volume mounts](#custom-volume-mounts) - [Runner Labels](#runner-labels) @@ -1164,6 +1165,36 @@ spec: This also helps with resources, as you don't need to give resources separately to docker and runner. +### Runner with K8s Jobs + +When using the default runner, jobs that use a container will run in docker. This necessitates privileged mode, either on the runner pod or the sidecar container + +By setting the container mode, you can instead invoke these jobs using a [kubernetes implementation](https://github.com/actions/runner-container-hooks/tree/main/packages/k8s) while not executing in privileged mode. + +The runner will dynamically spin up pods and k8s jobs in the runner's namespace to run the workflow, so a `workVolumeClaimTemplate` is required for the runner's working directory, and a service account with the [appropriate permissions.](https://github.com/actions/runner-container-hooks/tree/main/packages/k8s#pre-requisites) + +There are some [limitations](https://github.com/actions/runner-container-hooks/tree/main/packages/k8s#limitations) to this approach, mainly [job containers](https://docs.github.com/en/actions/using-jobs/running-jobs-in-a-container) are required on all workflows. + +```yaml +# runner.yaml +apiVersion: actions.summerwind.dev/v1alpha1 +kind: Runner +metadata: + name: example-runner +spec: + repository: example/myrepo + containerMode: kubernetes + serviceAccountName: my-service-account + workVolumeClaimTemplate: + storageClassName: "my-dynamic-storage-class" + accessModes: + - ReadWriteOnce + resources: + requests: + storage: 10Gi + env: [] +``` + ### Additional Tweaks You can pass details through the spec selector. Here's an eg. of what you may like to do: diff --git a/api/v1alpha1/runner_types.go b/api/v1alpha1/runner_types.go index fa1e363c..2a06dced 100644 --- a/api/v1alpha1/runner_types.go +++ b/api/v1alpha1/runner_types.go @@ -18,6 +18,7 @@ package v1alpha1 import ( "errors" + "fmt" "k8s.io/apimachinery/pkg/api/resource" @@ -71,6 +72,9 @@ type RunnerConfig struct { VolumeSizeLimit *resource.Quantity `json:"volumeSizeLimit,omitempty"` // +optional VolumeStorageMedium *string `json:"volumeStorageMedium,omitempty"` + + // +optional + ContainerMode string `json:"containerMode,omitempty"` } // RunnerPodSpec defines the desired pod spec fields of the runner pod @@ -157,6 +161,9 @@ type RunnerPodSpec struct { // +optional DnsConfig *corev1.PodDNSConfig `json:"dnsConfig,omitempty"` + + // +optional + WorkVolumeClaimTemplate *WorkVolumeClaimTemplate `json:"workVolumeClaimTemplate,omitempty"` } // ValidateRepository validates repository field. @@ -182,6 +189,29 @@ func (rs *RunnerSpec) ValidateRepository() error { return nil } +func (rs *RunnerSpec) ValidateWorkVolumeClaimTemplate() error { + if rs.ContainerMode != "kubernetes" { + return nil + } + + if rs.WorkVolumeClaimTemplate == nil { + return errors.New("Spec.ContainerMode: kubernetes must have workVolumeClaimTemplate field specified") + } + + return rs.WorkVolumeClaimTemplate.validate() +} + +func (rs *RunnerSpec) ValidateIsServiceAccountNameSet() error { + if rs.ContainerMode != "kubernetes" { + return nil + } + + if rs.ServiceAccountName == "" { + return errors.New("service account name is required if container mode is kubernetes") + } + return nil +} + // RunnerStatus defines the observed state of Runner type RunnerStatus struct { // Turns true only if the runner pod is ready. @@ -210,6 +240,51 @@ type RunnerStatusRegistration struct { ExpiresAt metav1.Time `json:"expiresAt"` } +type WorkVolumeClaimTemplate struct { + StorageClassName string `json:"storageClassName"` + AccessModes []corev1.PersistentVolumeAccessMode `json:"accessModes"` + Resources corev1.ResourceRequirements `json:"resources"` +} + +func (w *WorkVolumeClaimTemplate) validate() error { + if w.AccessModes == nil || len(w.AccessModes) == 0 { + return errors.New("Access mode should have at least one mode specified") + } + + for _, accessMode := range w.AccessModes { + switch accessMode { + case corev1.ReadWriteOnce, corev1.ReadWriteMany: + default: + return fmt.Errorf("Access mode %v is not supported", accessMode) + } + } + return nil +} + +func (w *WorkVolumeClaimTemplate) V1Volume() corev1.Volume { + return corev1.Volume{ + Name: "work", + VolumeSource: corev1.VolumeSource{ + Ephemeral: &corev1.EphemeralVolumeSource{ + VolumeClaimTemplate: &corev1.PersistentVolumeClaimTemplate{ + Spec: corev1.PersistentVolumeClaimSpec{ + AccessModes: w.AccessModes, + StorageClassName: &w.StorageClassName, + Resources: w.Resources, + }, + }, + }, + }, + } +} + +func (w *WorkVolumeClaimTemplate) V1VolumeMount(mountPath string) corev1.VolumeMount { + return corev1.VolumeMount{ + MountPath: mountPath, + Name: "work", + } +} + // +kubebuilder:object:root=true // +kubebuilder:subresource:status // +kubebuilder:printcolumn:JSONPath=".spec.enterprise",name=Enterprise,type=string diff --git a/api/v1alpha1/runner_webhook.go b/api/v1alpha1/runner_webhook.go index 861fdce6..be833cf1 100644 --- a/api/v1alpha1/runner_webhook.go +++ b/api/v1alpha1/runner_webhook.go @@ -76,6 +76,16 @@ func (r *Runner) Validate() error { errList = append(errList, field.Invalid(field.NewPath("spec", "repository"), r.Spec.Repository, err.Error())) } + err = r.Spec.ValidateWorkVolumeClaimTemplate() + if err != nil { + errList = append(errList, field.Invalid(field.NewPath("spec", "workVolumeClaimTemplate"), r.Spec.WorkVolumeClaimTemplate, err.Error())) + } + + err = r.Spec.ValidateIsServiceAccountNameSet() + if err != nil { + errList = append(errList, field.Invalid(field.NewPath("spec", "serviceAccountName"), r.Spec.ServiceAccountName, err.Error())) + } + if len(errList) > 0 { return apierrors.NewInvalid(r.GroupVersionKind().GroupKind(), r.Name, errList) } diff --git a/api/v1alpha1/runnerdeployment_webhook.go b/api/v1alpha1/runnerdeployment_webhook.go index 91a034d6..d0242899 100644 --- a/api/v1alpha1/runnerdeployment_webhook.go +++ b/api/v1alpha1/runnerdeployment_webhook.go @@ -76,6 +76,16 @@ func (r *RunnerDeployment) Validate() error { errList = append(errList, field.Invalid(field.NewPath("spec", "template", "spec", "repository"), r.Spec.Template.Spec.Repository, err.Error())) } + err = r.Spec.Template.Spec.ValidateWorkVolumeClaimTemplate() + if err != nil { + errList = append(errList, field.Invalid(field.NewPath("spec", "template", "spec", "workVolumeClaimTemplate"), r.Spec.Template.Spec.WorkVolumeClaimTemplate, err.Error())) + } + + err = r.Spec.Template.Spec.ValidateIsServiceAccountNameSet() + if err != nil { + errList = append(errList, field.Invalid(field.NewPath("spec", "template", "spec", "serviceAccountName"), r.Spec.Template.Spec.ServiceAccountName, err.Error())) + } + if len(errList) > 0 { return apierrors.NewInvalid(r.GroupVersionKind().GroupKind(), r.Name, errList) } diff --git a/api/v1alpha1/runnerreplicaset_webhook.go b/api/v1alpha1/runnerreplicaset_webhook.go index b728d57d..3a970da8 100644 --- a/api/v1alpha1/runnerreplicaset_webhook.go +++ b/api/v1alpha1/runnerreplicaset_webhook.go @@ -76,6 +76,16 @@ func (r *RunnerReplicaSet) Validate() error { errList = append(errList, field.Invalid(field.NewPath("spec", "template", "spec", "repository"), r.Spec.Template.Spec.Repository, err.Error())) } + err = r.Spec.Template.Spec.ValidateWorkVolumeClaimTemplate() + if err != nil { + errList = append(errList, field.Invalid(field.NewPath("spec", "template", "spec", "workVolumeClaimTemplate"), r.Spec.Template.Spec.WorkVolumeClaimTemplate, err.Error())) + } + + err = r.Spec.Template.Spec.ValidateIsServiceAccountNameSet() + if err != nil { + errList = append(errList, field.Invalid(field.NewPath("spec", "template", "spec", "serviceAccountName"), r.Spec.Template.Spec.ServiceAccountName, err.Error())) + } + if len(errList) > 0 { return apierrors.NewInvalid(r.GroupVersionKind().GroupKind(), r.Name, errList) } diff --git a/api/v1alpha1/runnerset_types.go b/api/v1alpha1/runnerset_types.go index f5e697b7..58ef755e 100644 --- a/api/v1alpha1/runnerset_types.go +++ b/api/v1alpha1/runnerset_types.go @@ -33,6 +33,12 @@ type RunnerSetSpec struct { // +nullable EffectiveTime *metav1.Time `json:"effectiveTime,omitempty"` + // +optional + ServiceAccountName string `json:"serviceAccountName,omitempty"` + + // +optional + WorkVolumeClaimTemplate *WorkVolumeClaimTemplate `json:"workVolumeClaimTemplate,omitempty"` + appsv1.StatefulSetSpec `json:",inline"` } diff --git a/api/v1alpha1/zz_generated.deepcopy.go b/api/v1alpha1/zz_generated.deepcopy.go index ce3e2aec..9315b459 100644 --- a/api/v1alpha1/zz_generated.deepcopy.go +++ b/api/v1alpha1/zz_generated.deepcopy.go @@ -741,6 +741,11 @@ func (in *RunnerPodSpec) DeepCopyInto(out *RunnerPodSpec) { *out = new(v1.PodDNSConfig) (*in).DeepCopyInto(*out) } + if in.WorkVolumeClaimTemplate != nil { + in, out := &in.WorkVolumeClaimTemplate, &out.WorkVolumeClaimTemplate + *out = new(WorkVolumeClaimTemplate) + (*in).DeepCopyInto(*out) + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new RunnerPodSpec. @@ -939,6 +944,11 @@ func (in *RunnerSetSpec) DeepCopyInto(out *RunnerSetSpec) { in, out := &in.EffectiveTime, &out.EffectiveTime *out = (*in).DeepCopy() } + if in.WorkVolumeClaimTemplate != nil { + in, out := &in.WorkVolumeClaimTemplate, &out.WorkVolumeClaimTemplate + *out = new(WorkVolumeClaimTemplate) + (*in).DeepCopyInto(*out) + } in.StatefulSetSpec.DeepCopyInto(&out.StatefulSetSpec) } @@ -1126,6 +1136,27 @@ func (in *ScheduledOverride) DeepCopy() *ScheduledOverride { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *WorkVolumeClaimTemplate) DeepCopyInto(out *WorkVolumeClaimTemplate) { + *out = *in + if in.AccessModes != nil { + in, out := &in.AccessModes, &out.AccessModes + *out = make([]v1.PersistentVolumeAccessMode, len(*in)) + copy(*out, *in) + } + in.Resources.DeepCopyInto(&out.Resources) +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new WorkVolumeClaimTemplate. +func (in *WorkVolumeClaimTemplate) DeepCopy() *WorkVolumeClaimTemplate { + if in == nil { + return nil + } + out := new(WorkVolumeClaimTemplate) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *WorkflowJobSpec) DeepCopyInto(out *WorkflowJobSpec) { *out = *in diff --git a/charts/actions-runner-controller/crds/actions.summerwind.dev_runnerdeployments.yaml b/charts/actions-runner-controller/crds/actions.summerwind.dev_runnerdeployments.yaml index 77add743..7241ad90 100644 --- a/charts/actions-runner-controller/crds/actions.summerwind.dev_runnerdeployments.yaml +++ b/charts/actions-runner-controller/crds/actions.summerwind.dev_runnerdeployments.yaml @@ -574,6 +574,8 @@ spec: type: object automountServiceAccountToken: type: boolean + containerMode: + type: string containers: items: description: A single application container that you want to run within a pod. @@ -5176,6 +5178,41 @@ spec: type: array workDir: type: string + workVolumeClaimTemplate: + properties: + accessModes: + items: + type: string + type: array + resources: + description: ResourceRequirements describes the compute resource requirements. + properties: + limits: + additionalProperties: + anyOf: + - type: integer + - type: string + pattern: ^(\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))(([KMGTPE]i)|[numkMGTPE]|([eE](\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))))?$ + x-kubernetes-int-or-string: true + description: 'Limits describes the maximum amount of compute resources allowed. More info: https://kubernetes.io/docs/concepts/configuration/manage-resources-containers/' + type: object + requests: + additionalProperties: + anyOf: + - type: integer + - type: string + pattern: ^(\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))(([KMGTPE]i)|[numkMGTPE]|([eE](\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))))?$ + x-kubernetes-int-or-string: true + description: 'Requests describes the minimum amount of compute resources required. If Requests is omitted for a container, it defaults to Limits if that is explicitly specified, otherwise to an implementation-defined value. More info: https://kubernetes.io/docs/concepts/configuration/manage-resources-containers/' + type: object + type: object + storageClassName: + type: string + required: + - accessModes + - resources + - storageClassName + type: object type: object type: object required: diff --git a/charts/actions-runner-controller/crds/actions.summerwind.dev_runnerreplicasets.yaml b/charts/actions-runner-controller/crds/actions.summerwind.dev_runnerreplicasets.yaml index 549a9eac..19771690 100644 --- a/charts/actions-runner-controller/crds/actions.summerwind.dev_runnerreplicasets.yaml +++ b/charts/actions-runner-controller/crds/actions.summerwind.dev_runnerreplicasets.yaml @@ -571,6 +571,8 @@ spec: type: object automountServiceAccountToken: type: boolean + containerMode: + type: string containers: items: description: A single application container that you want to run within a pod. @@ -5173,6 +5175,41 @@ spec: type: array workDir: type: string + workVolumeClaimTemplate: + properties: + accessModes: + items: + type: string + type: array + resources: + description: ResourceRequirements describes the compute resource requirements. + properties: + limits: + additionalProperties: + anyOf: + - type: integer + - type: string + pattern: ^(\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))(([KMGTPE]i)|[numkMGTPE]|([eE](\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))))?$ + x-kubernetes-int-or-string: true + description: 'Limits describes the maximum amount of compute resources allowed. More info: https://kubernetes.io/docs/concepts/configuration/manage-resources-containers/' + type: object + requests: + additionalProperties: + anyOf: + - type: integer + - type: string + pattern: ^(\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))(([KMGTPE]i)|[numkMGTPE]|([eE](\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))))?$ + x-kubernetes-int-or-string: true + description: 'Requests describes the minimum amount of compute resources required. If Requests is omitted for a container, it defaults to Limits if that is explicitly specified, otherwise to an implementation-defined value. More info: https://kubernetes.io/docs/concepts/configuration/manage-resources-containers/' + type: object + type: object + storageClassName: + type: string + required: + - accessModes + - resources + - storageClassName + type: object type: object type: object required: diff --git a/charts/actions-runner-controller/crds/actions.summerwind.dev_runners.yaml b/charts/actions-runner-controller/crds/actions.summerwind.dev_runners.yaml index 2953410d..6cba75dc 100644 --- a/charts/actions-runner-controller/crds/actions.summerwind.dev_runners.yaml +++ b/charts/actions-runner-controller/crds/actions.summerwind.dev_runners.yaml @@ -512,6 +512,8 @@ spec: type: object automountServiceAccountToken: type: boolean + containerMode: + type: string containers: items: description: A single application container that you want to run within a pod. @@ -5114,6 +5116,41 @@ spec: type: array workDir: type: string + workVolumeClaimTemplate: + properties: + accessModes: + items: + type: string + type: array + resources: + description: ResourceRequirements describes the compute resource requirements. + properties: + limits: + additionalProperties: + anyOf: + - type: integer + - type: string + pattern: ^(\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))(([KMGTPE]i)|[numkMGTPE]|([eE](\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))))?$ + x-kubernetes-int-or-string: true + description: 'Limits describes the maximum amount of compute resources allowed. More info: https://kubernetes.io/docs/concepts/configuration/manage-resources-containers/' + type: object + requests: + additionalProperties: + anyOf: + - type: integer + - type: string + pattern: ^(\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))(([KMGTPE]i)|[numkMGTPE]|([eE](\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))))?$ + x-kubernetes-int-or-string: true + description: 'Requests describes the minimum amount of compute resources required. If Requests is omitted for a container, it defaults to Limits if that is explicitly specified, otherwise to an implementation-defined value. More info: https://kubernetes.io/docs/concepts/configuration/manage-resources-containers/' + type: object + type: object + storageClassName: + type: string + required: + - accessModes + - resources + - storageClassName + type: object type: object status: description: RunnerStatus defines the observed state of Runner diff --git a/charts/actions-runner-controller/crds/actions.summerwind.dev_runnersets.yaml b/charts/actions-runner-controller/crds/actions.summerwind.dev_runnersets.yaml index 9f089d6e..c6b8e301 100644 --- a/charts/actions-runner-controller/crds/actions.summerwind.dev_runnersets.yaml +++ b/charts/actions-runner-controller/crds/actions.summerwind.dev_runnersets.yaml @@ -46,6 +46,8 @@ spec: spec: description: RunnerSetSpec defines the desired state of RunnerSet properties: + containerMode: + type: string dockerEnabled: type: boolean dockerMTU: @@ -134,6 +136,8 @@ spec: description: matchLabels is a map of {key,value} pairs. A single {key,value} in the matchLabels map is equivalent to an element of matchExpressions, whose key field is "key", the operator is "In", and the values array contains only "value". The requirements are ANDed. type: object type: object + serviceAccountName: + type: string serviceName: description: 'serviceName is the name of the service that governs this StatefulSet. This service must exist before the StatefulSet, and is responsible for the network identity of the set. Pods get DNS/hostnames that follow the pattern: pod-specific-string.serviceName.default.svc.cluster.local where "pod-specific-string" is managed by the StatefulSet controller.' type: string @@ -4445,6 +4449,41 @@ spec: type: string workDir: type: string + workVolumeClaimTemplate: + properties: + accessModes: + items: + type: string + type: array + resources: + description: ResourceRequirements describes the compute resource requirements. + properties: + limits: + additionalProperties: + anyOf: + - type: integer + - type: string + pattern: ^(\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))(([KMGTPE]i)|[numkMGTPE]|([eE](\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))))?$ + x-kubernetes-int-or-string: true + description: 'Limits describes the maximum amount of compute resources allowed. More info: https://kubernetes.io/docs/concepts/configuration/manage-resources-containers/' + type: object + requests: + additionalProperties: + anyOf: + - type: integer + - type: string + pattern: ^(\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))(([KMGTPE]i)|[numkMGTPE]|([eE](\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))))?$ + x-kubernetes-int-or-string: true + description: 'Requests describes the minimum amount of compute resources required. If Requests is omitted for a container, it defaults to Limits if that is explicitly specified, otherwise to an implementation-defined value. More info: https://kubernetes.io/docs/concepts/configuration/manage-resources-containers/' + type: object + type: object + storageClassName: + type: string + required: + - accessModes + - resources + - storageClassName + type: object required: - selector - serviceName diff --git a/charts/actions-runner-controller/templates/manager_role.yaml b/charts/actions-runner-controller/templates/manager_role.yaml index 9b6a8395..d5a41f2d 100644 --- a/charts/actions-runner-controller/templates/manager_role.yaml +++ b/charts/actions-runner-controller/templates/manager_role.yaml @@ -250,3 +250,11 @@ rules: - patch - update - watch +- apiGroups: + - "" + resources: + - secrets + verbs: + - get + - list + - watch diff --git a/config/crd/bases/actions.summerwind.dev_runnerdeployments.yaml b/config/crd/bases/actions.summerwind.dev_runnerdeployments.yaml index 77add743..7241ad90 100644 --- a/config/crd/bases/actions.summerwind.dev_runnerdeployments.yaml +++ b/config/crd/bases/actions.summerwind.dev_runnerdeployments.yaml @@ -574,6 +574,8 @@ spec: type: object automountServiceAccountToken: type: boolean + containerMode: + type: string containers: items: description: A single application container that you want to run within a pod. @@ -5176,6 +5178,41 @@ spec: type: array workDir: type: string + workVolumeClaimTemplate: + properties: + accessModes: + items: + type: string + type: array + resources: + description: ResourceRequirements describes the compute resource requirements. + properties: + limits: + additionalProperties: + anyOf: + - type: integer + - type: string + pattern: ^(\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))(([KMGTPE]i)|[numkMGTPE]|([eE](\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))))?$ + x-kubernetes-int-or-string: true + description: 'Limits describes the maximum amount of compute resources allowed. More info: https://kubernetes.io/docs/concepts/configuration/manage-resources-containers/' + type: object + requests: + additionalProperties: + anyOf: + - type: integer + - type: string + pattern: ^(\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))(([KMGTPE]i)|[numkMGTPE]|([eE](\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))))?$ + x-kubernetes-int-or-string: true + description: 'Requests describes the minimum amount of compute resources required. If Requests is omitted for a container, it defaults to Limits if that is explicitly specified, otherwise to an implementation-defined value. More info: https://kubernetes.io/docs/concepts/configuration/manage-resources-containers/' + type: object + type: object + storageClassName: + type: string + required: + - accessModes + - resources + - storageClassName + type: object type: object type: object required: diff --git a/config/crd/bases/actions.summerwind.dev_runnerreplicasets.yaml b/config/crd/bases/actions.summerwind.dev_runnerreplicasets.yaml index 549a9eac..19771690 100644 --- a/config/crd/bases/actions.summerwind.dev_runnerreplicasets.yaml +++ b/config/crd/bases/actions.summerwind.dev_runnerreplicasets.yaml @@ -571,6 +571,8 @@ spec: type: object automountServiceAccountToken: type: boolean + containerMode: + type: string containers: items: description: A single application container that you want to run within a pod. @@ -5173,6 +5175,41 @@ spec: type: array workDir: type: string + workVolumeClaimTemplate: + properties: + accessModes: + items: + type: string + type: array + resources: + description: ResourceRequirements describes the compute resource requirements. + properties: + limits: + additionalProperties: + anyOf: + - type: integer + - type: string + pattern: ^(\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))(([KMGTPE]i)|[numkMGTPE]|([eE](\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))))?$ + x-kubernetes-int-or-string: true + description: 'Limits describes the maximum amount of compute resources allowed. More info: https://kubernetes.io/docs/concepts/configuration/manage-resources-containers/' + type: object + requests: + additionalProperties: + anyOf: + - type: integer + - type: string + pattern: ^(\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))(([KMGTPE]i)|[numkMGTPE]|([eE](\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))))?$ + x-kubernetes-int-or-string: true + description: 'Requests describes the minimum amount of compute resources required. If Requests is omitted for a container, it defaults to Limits if that is explicitly specified, otherwise to an implementation-defined value. More info: https://kubernetes.io/docs/concepts/configuration/manage-resources-containers/' + type: object + type: object + storageClassName: + type: string + required: + - accessModes + - resources + - storageClassName + type: object type: object type: object required: diff --git a/config/crd/bases/actions.summerwind.dev_runners.yaml b/config/crd/bases/actions.summerwind.dev_runners.yaml index 2953410d..6cba75dc 100644 --- a/config/crd/bases/actions.summerwind.dev_runners.yaml +++ b/config/crd/bases/actions.summerwind.dev_runners.yaml @@ -512,6 +512,8 @@ spec: type: object automountServiceAccountToken: type: boolean + containerMode: + type: string containers: items: description: A single application container that you want to run within a pod. @@ -5114,6 +5116,41 @@ spec: type: array workDir: type: string + workVolumeClaimTemplate: + properties: + accessModes: + items: + type: string + type: array + resources: + description: ResourceRequirements describes the compute resource requirements. + properties: + limits: + additionalProperties: + anyOf: + - type: integer + - type: string + pattern: ^(\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))(([KMGTPE]i)|[numkMGTPE]|([eE](\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))))?$ + x-kubernetes-int-or-string: true + description: 'Limits describes the maximum amount of compute resources allowed. More info: https://kubernetes.io/docs/concepts/configuration/manage-resources-containers/' + type: object + requests: + additionalProperties: + anyOf: + - type: integer + - type: string + pattern: ^(\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))(([KMGTPE]i)|[numkMGTPE]|([eE](\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))))?$ + x-kubernetes-int-or-string: true + description: 'Requests describes the minimum amount of compute resources required. If Requests is omitted for a container, it defaults to Limits if that is explicitly specified, otherwise to an implementation-defined value. More info: https://kubernetes.io/docs/concepts/configuration/manage-resources-containers/' + type: object + type: object + storageClassName: + type: string + required: + - accessModes + - resources + - storageClassName + type: object type: object status: description: RunnerStatus defines the observed state of Runner diff --git a/config/crd/bases/actions.summerwind.dev_runnersets.yaml b/config/crd/bases/actions.summerwind.dev_runnersets.yaml index 9f089d6e..c6b8e301 100644 --- a/config/crd/bases/actions.summerwind.dev_runnersets.yaml +++ b/config/crd/bases/actions.summerwind.dev_runnersets.yaml @@ -46,6 +46,8 @@ spec: spec: description: RunnerSetSpec defines the desired state of RunnerSet properties: + containerMode: + type: string dockerEnabled: type: boolean dockerMTU: @@ -134,6 +136,8 @@ spec: description: matchLabels is a map of {key,value} pairs. A single {key,value} in the matchLabels map is equivalent to an element of matchExpressions, whose key field is "key", the operator is "In", and the values array contains only "value". The requirements are ANDed. type: object type: object + serviceAccountName: + type: string serviceName: description: 'serviceName is the name of the service that governs this StatefulSet. This service must exist before the StatefulSet, and is responsible for the network identity of the set. Pods get DNS/hostnames that follow the pattern: pod-specific-string.serviceName.default.svc.cluster.local where "pod-specific-string" is managed by the StatefulSet controller.' type: string @@ -4445,6 +4449,41 @@ spec: type: string workDir: type: string + workVolumeClaimTemplate: + properties: + accessModes: + items: + type: string + type: array + resources: + description: ResourceRequirements describes the compute resource requirements. + properties: + limits: + additionalProperties: + anyOf: + - type: integer + - type: string + pattern: ^(\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))(([KMGTPE]i)|[numkMGTPE]|([eE](\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))))?$ + x-kubernetes-int-or-string: true + description: 'Limits describes the maximum amount of compute resources allowed. More info: https://kubernetes.io/docs/concepts/configuration/manage-resources-containers/' + type: object + requests: + additionalProperties: + anyOf: + - type: integer + - type: string + pattern: ^(\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))(([KMGTPE]i)|[numkMGTPE]|([eE](\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))))?$ + x-kubernetes-int-or-string: true + description: 'Requests describes the minimum amount of compute resources required. If Requests is omitted for a container, it defaults to Limits if that is explicitly specified, otherwise to an implementation-defined value. More info: https://kubernetes.io/docs/concepts/configuration/manage-resources-containers/' + type: object + type: object + storageClassName: + type: string + required: + - accessModes + - resources + - storageClassName + type: object required: - selector - serviceName diff --git a/config/rbac/role.yaml b/config/rbac/role.yaml index e0fda3a1..bea27013 100644 --- a/config/rbac/role.yaml +++ b/config/rbac/role.yaml @@ -249,3 +249,12 @@ rules: - patch - update - watch +- apiGroups: + - "" + resources: + - secrets + verbs: + - delete + - get + - list + - watch diff --git a/controllers/constants.go b/controllers/constants.go index d1f5d7b3..bb36ba7a 100644 --- a/controllers/constants.go +++ b/controllers/constants.go @@ -9,7 +9,8 @@ const ( const ( // This names requires at least one slash to work. // See https://github.com/google/knative-gcp/issues/378 - runnerPodFinalizerName = "actions.summerwind.dev/runner-pod" + runnerPodFinalizerName = "actions.summerwind.dev/runner-pod" + runnerLinkedResourcesFinalizerName = "actions.summerwind.dev/linked-resources" annotationKeyPrefix = "actions-runner/" @@ -61,4 +62,7 @@ const ( EnvVarRunnerName = "RUNNER_NAME" EnvVarRunnerToken = "RUNNER_TOKEN" + + // defaultHookPath is path to the hook script used when the "containerMode: kubernetes" is specified + defaultRunnerHookPath = "/runner/k8s/index.js" ) diff --git a/controllers/runner_controller.go b/controllers/runner_controller.go index 0f9a3737..a696ea06 100644 --- a/controllers/runner_controller.go +++ b/controllers/runner_controller.go @@ -18,7 +18,9 @@ package controllers import ( "context" + "errors" "fmt" + "strconv" "strings" "time" @@ -76,6 +78,7 @@ type RunnerReconciler struct { // +kubebuilder:rbac:groups=actions.summerwind.dev,resources=runners/finalizers,verbs=get;list;watch;create;update;patch;delete // +kubebuilder:rbac:groups=actions.summerwind.dev,resources=runners/status,verbs=get;update;patch // +kubebuilder:rbac:groups=core,resources=pods,verbs=get;list;watch;create;update;patch;delete +// +kubebuilder:rbac:groups=core,resources=secrets,verbs=get;list;watch;delete // +kubebuilder:rbac:groups=core,resources=pods/finalizers,verbs=get;list;watch;create;update;patch;delete // +kubebuilder:rbac:groups=core,resources=events,verbs=create;patch @@ -112,6 +115,7 @@ func (r *RunnerReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctr // Pod was not found return r.processRunnerDeletion(runner, ctx, log, nil) } + return r.processRunnerDeletion(runner, ctx, log, &pod) } @@ -412,7 +416,17 @@ func (r *RunnerReconciler) newPod(runner v1alpha1.Runner) (corev1.Pod, error) { template.Spec.SecurityContext = runner.Spec.SecurityContext template.Spec.EnableServiceLinks = runner.Spec.EnableServiceLinks - pod, err := newRunnerPod(runner.Name, template, runner.Spec.RunnerConfig, r.RunnerImage, r.RunnerImagePullSecrets, r.DockerImage, r.DockerRegistryMirror, r.GitHubClient.GithubBaseURL) + if runner.Spec.ContainerMode == "kubernetes" { + workDir := runner.Spec.WorkDir + if workDir == "" { + workDir = "/runner/_work" + } + if err := applyWorkVolumeClaimTemplateToPod(&template, runner.Spec.WorkVolumeClaimTemplate, workDir); err != nil { + return corev1.Pod{}, err + } + } + + pod, err := newRunnerPodWithContainerMode(runner.Spec.ContainerMode, runner.Name, template, runner.Spec.RunnerConfig, r.RunnerImage, r.RunnerImagePullSecrets, r.DockerImage, r.DockerRegistryMirror, r.GitHubClient.GithubBaseURL) if err != nil { return pod, err } @@ -424,6 +438,9 @@ func (r *RunnerReconciler) newPod(runner v1alpha1.Runner) (corev1.Pod, error) { // if operater provides a work volume mount, use that isPresent, _ := workVolumeMountPresent(runnerSpec.VolumeMounts) if isPresent { + if runnerSpec.ContainerMode == "kubernetes" { + return pod, errors.New("volume mount \"work\" should be specified by workVolumeClaimTemplate in container mode kubernetes") + } // remove work volume since it will be provided from runnerSpec.Volumes // if we don't remove it here we would get a duplicate key error, i.e. two volumes named work _, index := workVolumeMountPresent(pod.Spec.Containers[0].VolumeMounts) @@ -437,6 +454,9 @@ func (r *RunnerReconciler) newPod(runner v1alpha1.Runner) (corev1.Pod, error) { // if operator provides a work volume. use that isPresent, _ := workVolumePresent(runnerSpec.Volumes) if isPresent { + if runnerSpec.ContainerMode == "kubernetes" { + return pod, errors.New("volume \"work\" should be specified by workVolumeClaimTemplate in container mode kubernetes") + } _, index := workVolumePresent(pod.Spec.Volumes) // remove work volume since it will be provided from runnerSpec.Volumes @@ -446,6 +466,7 @@ func (r *RunnerReconciler) newPod(runner v1alpha1.Runner) (corev1.Pod, error) { pod.Spec.Volumes = append(pod.Spec.Volumes, runnerSpec.Volumes...) } + if len(runnerSpec.InitContainers) != 0 { pod.Spec.InitContainers = append(pod.Spec.InitContainers, runnerSpec.InitContainers...) } @@ -530,7 +551,45 @@ func mutatePod(pod *corev1.Pod, token string) *corev1.Pod { return updated } -func newRunnerPod(runnerName string, template corev1.Pod, runnerSpec v1alpha1.RunnerConfig, defaultRunnerImage string, defaultRunnerImagePullSecrets []string, defaultDockerImage, defaultDockerRegistryMirror string, githubBaseURL string) (corev1.Pod, error) { +func runnerHookEnvs(pod *corev1.Pod) ([]corev1.EnvVar, error) { + isRequireSameNode, err := isRequireSameNode(pod) + if err != nil { + return nil, err + } + + return []corev1.EnvVar{ + { + Name: "ACTIONS_RUNNER_CONTAINER_HOOKS", + Value: defaultRunnerHookPath, + }, + { + Name: "ACTIONS_RUNNER_REQUIRE_JOB_CONTAINER", + Value: "true", + }, + { + Name: "ACTIONS_RUNNER_POD_NAME", + ValueFrom: &corev1.EnvVarSource{ + FieldRef: &corev1.ObjectFieldSelector{ + FieldPath: "metadata.name", + }, + }, + }, + { + Name: "ACTIONS_RUNNER_JOB_NAMESPACE", + ValueFrom: &corev1.EnvVarSource{ + FieldRef: &corev1.ObjectFieldSelector{ + FieldPath: "metadata.namespace", + }, + }, + }, + corev1.EnvVar{ + Name: "ACTIONS_RUNNER_REQUIRE_SAME_NODE", + Value: strconv.FormatBool(isRequireSameNode), + }, + }, nil +} + +func newRunnerPodWithContainerMode(containerMode string, runnerName string, template corev1.Pod, runnerSpec v1alpha1.RunnerConfig, defaultRunnerImage string, defaultRunnerImagePullSecrets []string, defaultDockerImage, defaultDockerRegistryMirror string, githubBaseURL string) (corev1.Pod, error) { var ( privileged bool = true dockerdInRunner bool = runnerSpec.DockerdWithinRunnerContainer != nil && *runnerSpec.DockerdWithinRunnerContainer @@ -539,6 +598,12 @@ func newRunnerPod(runnerName string, template corev1.Pod, runnerSpec v1alpha1.Ru dockerdInRunnerPrivileged bool = dockerdInRunner ) + if containerMode == "kubernetes" { + dockerdInRunner = false + dockerEnabled = false + dockerdInRunnerPrivileged = false + } + template = *template.DeepCopy() // This label selector is used by default when rd.Spec.Selector is empty. @@ -625,6 +690,17 @@ func newRunnerPod(runnerName string, template corev1.Pod, runnerSpec v1alpha1.Ru } } + if containerMode == "kubernetes" { + if dockerdContainer != nil { + template.Spec.Containers = append(template.Spec.Containers[:dockerdContainerIndex], template.Spec.Containers[dockerdContainerIndex+1:]...) + } + if runnerContainerIndex < runnerContainerIndex { + runnerContainerIndex-- + } + dockerdContainer = nil + dockerdContainerIndex = -1 + } + if runnerContainer == nil { runnerContainerIndex = -1 runnerContainer = &corev1.Container{ @@ -655,6 +731,13 @@ func newRunnerPod(runnerName string, template corev1.Pod, runnerSpec v1alpha1.Ru } runnerContainer.Env = append(runnerContainer.Env, env...) + if containerMode == "kubernetes" { + hookEnvs, err := runnerHookEnvs(&template) + if err != nil { + return corev1.Pod{}, err + } + runnerContainer.Env = append(runnerContainer.Env, hookEnvs...) + } if runnerContainer.SecurityContext == nil { runnerContainer.SecurityContext = &corev1.SecurityContext{} @@ -879,6 +962,10 @@ func newRunnerPod(runnerName string, template corev1.Pod, runnerSpec v1alpha1.Ru return *pod, nil } +func newRunnerPod(runnerName string, template corev1.Pod, runnerSpec v1alpha1.RunnerConfig, defaultRunnerImage string, defaultRunnerImagePullSecrets []string, defaultDockerImage, defaultDockerRegistryMirror string, githubBaseURL string) (corev1.Pod, error) { + return newRunnerPodWithContainerMode("", runnerName, template, runnerSpec, defaultRunnerImage, defaultRunnerImagePullSecrets, defaultDockerImage, defaultDockerRegistryMirror, githubBaseURL) +} + func (r *RunnerReconciler) SetupWithManager(mgr ctrl.Manager) error { name := "runner-controller" if r.Name != "" { @@ -941,3 +1028,71 @@ func workVolumeMountPresent(items []corev1.VolumeMount) (bool, int) { } return false, 0 } + +func applyWorkVolumeClaimTemplateToPod(pod *corev1.Pod, workVolumeClaimTemplate *v1alpha1.WorkVolumeClaimTemplate, workDir string) error { + if workVolumeClaimTemplate == nil { + return errors.New("work volume claim template must be specified in container mode kubernetes") + } + for i := range pod.Spec.Volumes { + if pod.Spec.Volumes[i].Name == "work" { + return fmt.Errorf("Work volume should not be specified in container mode kubernetes. workVolumeClaimTemplate field should be used instead.") + } + } + pod.Spec.Volumes = append(pod.Spec.Volumes, workVolumeClaimTemplate.V1Volume()) + + var runnerContainer *corev1.Container + for i := range pod.Spec.Containers { + if pod.Spec.Containers[i].Name == "runner" { + runnerContainer = &pod.Spec.Containers[i] + break + } + } + + if runnerContainer == nil { + return fmt.Errorf("runner container is not present when applying work volume claim template") + } + + if isPresent, _ := workVolumeMountPresent(runnerContainer.VolumeMounts); isPresent { + return fmt.Errorf("volume mount \"work\" should not be present on the runner container in container mode kubernetes") + } + + runnerContainer.VolumeMounts = append(runnerContainer.VolumeMounts, workVolumeClaimTemplate.V1VolumeMount(workDir)) + + return nil +} + +// isRequireSameNode specifies for the runner in kubernetes mode wether it should +// schedule jobs to the same node where the runner is +// +// This function should only be called in containerMode: kubernetes +func isRequireSameNode(pod *corev1.Pod) (bool, error) { + isPresent, index := workVolumePresent(pod.Spec.Volumes) + if !isPresent { + return true, errors.New("internal error: work volume mount must exist in containerMode: kubernetes") + } + + if pod.Spec.Volumes[index].Ephemeral == nil || pod.Spec.Volumes[index].Ephemeral.VolumeClaimTemplate == nil { + return true, errors.New("containerMode: kubernetes should have pod.Spec.Volumes[].Ephemeral.VolumeClaimTemplate set") + } + + for _, accessMode := range pod.Spec.Volumes[index].Ephemeral.VolumeClaimTemplate.Spec.AccessModes { + switch accessMode { + case corev1.ReadWriteOnce: + return true, nil + case corev1.ReadWriteMany: + default: + return true, errors.New("actions-runner-controller supports ReadWriteOnce and ReadWriteMany modes only") + } + } + return false, nil +} + +func overwriteRunnerEnv(runner *v1alpha1.Runner, key string, value string) { + for i := range runner.Spec.Env { + if runner.Spec.Env[i].Name == key { + runner.Spec.Env[i].Value = value + return + } + } + runner.Spec.Env = append(runner.Spec.Env, corev1.EnvVar{Name: key, Value: value}) +} diff --git a/controllers/runner_pod_controller.go b/controllers/runner_pod_controller.go index 9e9ed9c4..3bfdcbaf 100644 --- a/controllers/runner_pod_controller.go +++ b/controllers/runner_pod_controller.go @@ -20,6 +20,7 @@ import ( "context" "errors" "fmt" + "sync" "time" "github.com/go-logr/logr" @@ -50,6 +51,7 @@ type RunnerPodReconciler struct { } // +kubebuilder:rbac:groups=core,resources=pods,verbs=get;list;watch;update;patch;delete +// +kubebuilder:rbac:groups=core,resources=secrets,verbs=get;list;watch // +kubebuilder:rbac:groups=core,resources=events,verbs=create;patch func (r *RunnerPodReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) { @@ -77,6 +79,7 @@ func (r *RunnerPodReconciler) Reconcile(ctx context.Context, req ctrl.Request) ( } var enterprise, org, repo string + var isContainerMode bool for _, e := range envvars { switch e.Name { @@ -86,13 +89,20 @@ func (r *RunnerPodReconciler) Reconcile(ctx context.Context, req ctrl.Request) ( org = e.Value case EnvVarRepo: repo = e.Value + case "ACTIONS_RUNNER_CONTAINER_HOOKS": + isContainerMode = true } } if runnerPod.ObjectMeta.DeletionTimestamp.IsZero() { finalizers, added := addFinalizer(runnerPod.ObjectMeta.Finalizers, runnerPodFinalizerName) - if added { + var cleanupFinalizersAdded bool + if isContainerMode { + finalizers, cleanupFinalizersAdded = addFinalizer(finalizers, runnerLinkedResourcesFinalizerName) + } + + if added || cleanupFinalizersAdded { newRunner := runnerPod.DeepCopy() newRunner.ObjectMeta.Finalizers = finalizers @@ -108,6 +118,27 @@ func (r *RunnerPodReconciler) Reconcile(ctx context.Context, req ctrl.Request) ( } else { log.V(2).Info("Seen deletion-timestamp is already set") + if finalizers, removed := removeFinalizer(runnerPod.ObjectMeta.Finalizers, runnerLinkedResourcesFinalizerName); removed { + if err := r.cleanupRunnerLinkedPods(ctx, &runnerPod, log); err != nil { + log.Info("Runner-linked pods clean up that has failed due to an error. If this persists, please manually remove the runner-linked pods to unblock ARC", "err", err.Error()) + return ctrl.Result{Requeue: true, RequeueAfter: 30 * time.Second}, nil + } + if err := r.cleanupRunnerLinkedSecrets(ctx, &runnerPod, log); err != nil { + log.Info("Runner-linked secrets clean up that has failed due to an error. If this persists, please manually remove the runner-linked secrets to unblock ARC", "err", err.Error()) + return ctrl.Result{Requeue: true, RequeueAfter: 30 * time.Second}, nil + } + patchedPod := runnerPod.DeepCopy() + patchedPod.ObjectMeta.Finalizers = finalizers + + if err := r.Patch(ctx, patchedPod, client.MergeFrom(&runnerPod)); err != nil { + log.Error(err, "Failed to update runner for finalizer linked resources removal") + return ctrl.Result{}, err + } + + // Otherwise the subsequent patch request can revive the removed finalizer and it will trigger a unnecessary reconcilation + runnerPod = *patchedPod + } + finalizers, removed := removeFinalizer(runnerPod.ObjectMeta.Finalizers, runnerPodFinalizerName) if removed { @@ -222,3 +253,93 @@ func (r *RunnerPodReconciler) SetupWithManager(mgr ctrl.Manager) error { Named(name). Complete(r) } + +func (r *RunnerPodReconciler) cleanupRunnerLinkedPods(ctx context.Context, pod *corev1.Pod, log logr.Logger) error { + var runnerLinkedPodList corev1.PodList + if err := r.List(ctx, &runnerLinkedPodList, client.InNamespace(pod.Namespace), client.MatchingLabels( + map[string]string{ + "runner-pod": pod.ObjectMeta.Name, + }, + )); err != nil { + return fmt.Errorf("failed to list runner-linked pods: %w", err) + } + + var ( + wg sync.WaitGroup + errs []error + ) + for _, p := range runnerLinkedPodList.Items { + if !p.ObjectMeta.DeletionTimestamp.IsZero() { + continue + } + + p := p + wg.Add(1) + go func() { + defer wg.Done() + if err := r.Delete(ctx, &p); err != nil { + if kerrors.IsNotFound(err) || kerrors.IsGone(err) { + return + } + errs = append(errs, fmt.Errorf("delete pod %q error: %v", p.ObjectMeta.Name, err)) + } + }() + } + + wg.Wait() + + if len(errs) > 0 { + for _, err := range errs { + log.Error(err, "failed to remove runner-linked pod") + } + return errors.New("failed to remove some runner linked pods") + } + + return nil +} + +func (r *RunnerPodReconciler) cleanupRunnerLinkedSecrets(ctx context.Context, pod *corev1.Pod, log logr.Logger) error { + log.V(2).Info("Listing runner-linked secrets to be deleted", "ns", pod.Namespace) + + var runnerLinkedSecretList corev1.SecretList + if err := r.List(ctx, &runnerLinkedSecretList, client.InNamespace(pod.Namespace), client.MatchingLabels( + map[string]string{ + "runner-pod": pod.ObjectMeta.Name, + }, + )); err != nil { + return fmt.Errorf("failed to list runner-linked secrets: %w", err) + } + + var ( + wg sync.WaitGroup + errs []error + ) + for _, s := range runnerLinkedSecretList.Items { + if !s.ObjectMeta.DeletionTimestamp.IsZero() { + continue + } + + s := s + wg.Add(1) + go func() { + defer wg.Done() + if err := r.Delete(ctx, &s); err != nil { + if kerrors.IsNotFound(err) || kerrors.IsGone(err) { + return + } + errs = append(errs, fmt.Errorf("delete secret %q error: %v", s.ObjectMeta.Name, err)) + } + }() + } + + wg.Wait() + + if len(errs) > 0 { + for _, err := range errs { + log.Error(err, "failed to remove runner-linked secret") + } + return errors.New("failed to remove some runner linked secrets") + } + + return nil +} diff --git a/controllers/runnerset_controller.go b/controllers/runnerset_controller.go index 5aae774f..ed9f0e8a 100644 --- a/controllers/runnerset_controller.go +++ b/controllers/runnerset_controller.go @@ -195,7 +195,31 @@ func (r *RunnerSetReconciler) newStatefulSet(runnerSet *v1alpha1.RunnerSet) (*ap Spec: runnerSetWithOverrides.StatefulSetSpec.Template.Spec, } - pod, err := newRunnerPod(runnerSet.Name, template, runnerSet.Spec.RunnerConfig, r.RunnerImage, r.RunnerImagePullSecrets, r.DockerImage, r.DockerRegistryMirror, r.GitHubBaseURL) + if runnerSet.Spec.RunnerConfig.ContainerMode == "kubernetes" { + found := false + for i := range template.Spec.Containers { + if template.Spec.Containers[i].Name == containerName { + found = true + } + } + if !found { + template.Spec.Containers = append(template.Spec.Containers, corev1.Container{ + Name: "runner", + }) + } + + workDir := runnerSet.Spec.RunnerConfig.WorkDir + if workDir == "" { + workDir = "/runner/_work" + } + if err := applyWorkVolumeClaimTemplateToPod(&template, runnerSet.Spec.WorkVolumeClaimTemplate, workDir); err != nil { + return nil, err + } + + template.Spec.ServiceAccountName = runnerSet.Spec.ServiceAccountName + } + + pod, err := newRunnerPodWithContainerMode(runnerSet.Spec.RunnerConfig.ContainerMode, runnerSet.Name, template, runnerSet.Spec.RunnerConfig, r.RunnerImage, r.RunnerImagePullSecrets, r.DockerImage, r.DockerRegistryMirror, r.GitHubBaseURL) if err != nil { return nil, err } diff --git a/controllers/utils_test.go b/controllers/utils_test.go index df0f57e3..5fdb9aaf 100644 --- a/controllers/utils_test.go +++ b/controllers/utils_test.go @@ -3,6 +3,9 @@ package controllers import ( "reflect" "testing" + + "github.com/actions-runner-controller/actions-runner-controller/api/v1alpha1" + corev1 "k8s.io/api/core/v1" ) func Test_filterLabels(t *testing.T) { @@ -32,3 +35,94 @@ func Test_filterLabels(t *testing.T) { }) } } + +func Test_workVolumeClaimTemplateVolumeV1VolumeTransformation(t *testing.T) { + storageClassName := "local-storage" + workVolumeClaimTemplate := v1alpha1.WorkVolumeClaimTemplate{ + StorageClassName: storageClassName, + AccessModes: []corev1.PersistentVolumeAccessMode{corev1.ReadWriteOnce, corev1.ReadWriteMany}, + Resources: corev1.ResourceRequirements{}, + } + want := corev1.Volume{ + Name: "work", + VolumeSource: corev1.VolumeSource{ + Ephemeral: &corev1.EphemeralVolumeSource{ + VolumeClaimTemplate: &corev1.PersistentVolumeClaimTemplate{ + Spec: corev1.PersistentVolumeClaimSpec{ + AccessModes: []corev1.PersistentVolumeAccessMode{corev1.ReadWriteOnce, corev1.ReadWriteMany}, + StorageClassName: &storageClassName, + Resources: corev1.ResourceRequirements{}, + }, + }, + }, + }, + } + + got := workVolumeClaimTemplate.V1Volume() + + if got.Name != want.Name { + t.Errorf("want name %q, got %q\n", want.Name, got.Name) + } + + if got.VolumeSource.Ephemeral == nil { + t.Fatal("work volume claim template should transform itself into Ephemeral volume source\n") + } + + if got.VolumeSource.Ephemeral.VolumeClaimTemplate == nil { + t.Fatal("work volume claim template should have ephemeral volume claim template set\n") + } + + gotClassName := *got.VolumeSource.Ephemeral.VolumeClaimTemplate.Spec.StorageClassName + wantClassName := *want.VolumeSource.Ephemeral.VolumeClaimTemplate.Spec.StorageClassName + if gotClassName != wantClassName { + t.Errorf("expected storage class name %q, got %q\n", wantClassName, gotClassName) + } + + gotAccessModes := got.VolumeSource.Ephemeral.VolumeClaimTemplate.Spec.AccessModes + wantAccessModes := want.VolumeSource.Ephemeral.VolumeClaimTemplate.Spec.AccessModes + if len(gotAccessModes) != len(wantAccessModes) { + t.Fatalf("access modes lengths missmatch: got %v, expected %v\n", gotAccessModes, wantAccessModes) + } + + diff := make(map[corev1.PersistentVolumeAccessMode]int, len(wantAccessModes)) + for _, am := range wantAccessModes { + diff[am]++ + } + + for _, am := range gotAccessModes { + _, ok := diff[am] + if !ok { + t.Errorf("got access mode %v that is not in the wanted access modes\n", am) + } + + diff[am]-- + if diff[am] == 0 { + delete(diff, am) + } + } + + if len(diff) != 0 { + t.Fatalf("got access modes did not take every access mode into account\nactual: %v expected: %v\n", gotAccessModes, wantAccessModes) + } +} + +func Test_workVolumeClaimTemplateV1VolumeMount(t *testing.T) { + + workVolumeClaimTemplate := v1alpha1.WorkVolumeClaimTemplate{ + StorageClassName: "local-storage", + AccessModes: []corev1.PersistentVolumeAccessMode{corev1.ReadWriteOnce, corev1.ReadWriteMany}, + Resources: corev1.ResourceRequirements{}, + } + + mountPath := "/test/_work" + want := corev1.VolumeMount{ + MountPath: mountPath, + Name: "work", + } + + got := workVolumeClaimTemplate.V1VolumeMount(mountPath) + + if want != got { + t.Fatalf("expected volume mount %+v, actual %+v\n", want, got) + } +} diff --git a/runner/Makefile b/runner/Makefile index 43ea727b..cf0f1c20 100644 --- a/runner/Makefile +++ b/runner/Makefile @@ -5,6 +5,7 @@ TAG ?= latest TARGETPLATFORM ?= $(shell arch) RUNNER_VERSION ?= 2.294.0 +RUNNER_CONTAINER_HOOKS_VERSION ?= 0.1.2 DOCKER_VERSION ?= 20.10.12 # default list of platforms for which multiarch image is built @@ -28,6 +29,7 @@ docker-build-ubuntu: docker build \ --build-arg TARGETPLATFORM=${TARGETPLATFORM} \ --build-arg RUNNER_VERSION=${RUNNER_VERSION} \ + --build-arg RUNNER_CONTAINER_HOOKS_VERSION=${RUNNER_CONTAINER_HOOKS_VERSION} \ --build-arg DOCKER_VERSION=${DOCKER_VERSION} \ -f actions-runner.dockerfile \ -t ${NAME}:${TAG} . @@ -50,12 +52,14 @@ docker-buildx-ubuntu: fi docker buildx build --platform ${PLATFORMS} \ --build-arg RUNNER_VERSION=${RUNNER_VERSION} \ + --build-arg RUNNER_CONTAINER_HOOKS_VERSION=${RUNNER_CONTAINER_HOOKS_VERSION} \ --build-arg DOCKER_VERSION=${DOCKER_VERSION} \ -f actions-runner.dockerfile \ -t "${NAME}:${TAG}" \ . ${PUSH_ARG} docker buildx build --platform ${PLATFORMS} \ --build-arg RUNNER_VERSION=${RUNNER_VERSION} \ + --build-arg RUNNER_CONTAINER_HOOKS_VERSION=${RUNNER_CONTAINER_HOOKS_VERSION} \ --build-arg DOCKER_VERSION=${DOCKER_VERSION} \ -f actions-runner-dind.dockerfile \ -t "${DIND_RUNNER_NAME}:${TAG}" \ diff --git a/runner/actions-runner.dockerfile b/runner/actions-runner.dockerfile index 46105700..baa3ebbe 100644 --- a/runner/actions-runner.dockerfile +++ b/runner/actions-runner.dockerfile @@ -2,6 +2,7 @@ FROM ubuntu:20.04 ARG TARGETPLATFORM ARG RUNNER_VERSION=2.294.0 +ARG RUNNER_CONTAINER_HOOKS_VERSION=0.1.2 ARG DOCKER_CHANNEL=stable ARG DOCKER_VERSION=20.10.12 ARG DUMB_INIT_VERSION=1.2.5 @@ -105,6 +106,11 @@ RUN export ARCH=$(echo ${TARGETPLATFORM} | cut -d / -f2) \ && apt-get install -y libyaml-dev \ && rm -rf /var/lib/apt/lists/* +RUN cd "$RUNNER_ASSETS_DIR" \ + && curl -f -L -o runner-container-hooks.zip https://github.com/actions/runner-container-hooks/releases/download/v${RUNNER_CONTAINER_HOOKS_VERSION}/actions-runner-hooks-k8s-${RUNNER_CONTAINER_HOOKS_VERSION}.zip \ + && unzip ./runner-container-hooks.zip -d ./k8s \ + && rm runner-container-hooks.zip + ENV RUNNER_TOOL_CACHE=/opt/hostedtoolcache RUN mkdir /opt/hostedtoolcache \ && chgrp docker /opt/hostedtoolcache \