RunnerDeployment 'initContainers' getting overwritten

[donovanmuller]

Describe the bug

I am trying to use initContainers in a RunnerDeployment spec. However, once applied, it appears that the initContainers value are overwritten to an empty value, which then results in this error:

ERROR    controller-runtime.controller    Reconciler error    {"controller": "runner-controller", "request": "actions-runners/test-rf9v5-kjks5", "error": "Runner.actions.summerwind.dev \"test-rf9v5-kjks5\" is invalid: spec.initContainers.name: Required value"}

Checks

• My actions-runner-controller version (v0.19.0) does support the feature
• I'm using an unreleased version of the controller I built from HEAD of the default branch

To Reproduce
Steps to reproduce the behavior:

1. Apply the following RunnerDeployment spec:

apiVersion: actions.summerwind.dev/v1alpha1
kind: RunnerDeployment
metadata:    
  name: test
  namespace: actions-runners
spec:
  replicas: 1
  template:
    metadata: {}
    spec:
      dockerdWithinRunnerContainer: true
      ephemeral: true
      image: summerwind/actions-runner-dind:v2.278.0-ubuntu-20.04
      initContainers:
      - command:
          - sh
          - '-c'
          - 'echo "I just want to run..."'
        image: 'alpine'
        name: test
      labels:
      - test
      repository: test/test

2. Describe the created RunnerDeployment and notice that the initContainers now have a single empty value:

$ k get runnerdeployments.actions.summerwind.dev test -n actions-runners -oyaml
apiVersion: actions.summerwind.dev/v1alpha1
kind: RunnerDeployment
metadata:
  annotations:
    kubectl.kubernetes.io/last-applied-configuration: |
      {"apiVersion":"actions.summerwind.dev/v1alpha1","kind":"RunnerDeployment","metadata":{"annotations":{},"name":"test","namespace":"actions-runners"},"spec":{"replicas":1,"template":{"metadata":{},"spec":{"dockerdWithinRunnerContainer":true,"ephemeral":true,"image":"summerwind/actions-runner-dind:v2.278.0-ubuntu-20.04","initContainers":[{"command":["sh","-c","echo \"I just want to run...\""],"image":"alpine","name":"test"}],"labels":["test"],"repository":"test/test"}}}}
  creationTimestamp: "2021-08-28T09:40:18Z"
  generation: 1
  name: test
  namespace: actions-runners
  resourceVersion: "7900359"
  uid: a39eed1b-8edc-4da5-b6e8-9577586ba0cc
spec:
  replicas: 1
  selector: null
  template:
    metadata: {}
    spec:
      dockerdContainerResources: {}
      dockerdWithinRunnerContainer: true
      ephemeral: true
      image: summerwind/actions-runner-dind:v2.278.0-ubuntu-20.04
      initContainers:
      - {}
      labels:
      - test
      repository: test/test
      resources: {}

Expected behavior

The initContainers are applied to the runner Pod as defined in the RunnerDeployment

Environment (please complete the following information):

• Controller Version v0.19.0
• Deployment Method Helm
• Helm Chart Version 0.12.7

Additional context

Just to try rule out the possibility that other controllers/mutating webhooks were mutating the RunnerDeployment I tested with a simple Pod spec with initContainers but that worked as expected.

[mumoshu]

@donovanmuller Hey! Thanks for reporting. This does sound like a bug. But I'm not sure how it could happen in the current implementation.

We only append InitContainers from the runner spec to the pod spec. And it should be sufficient and enough to make it work

https://github.com/actions-runner-controller/actions-runner-controller/blob/c424d1afee693d0f2297df53d663f8a8137b9e51/controllers/runner_controller.go#L878-L880

Do you have any insight on it? 🤔

[donovanmuller]

@mumoshu agreed, I also couldn't see anything obvious that would overwrite initContainers and which is why I tested with a standard Pod definition.

I'll have to check the cluster audit logs to try see what is modifying the RunnerDeployment after it's applied initially. Will update once I find something...

[donovanmuller]

@mumoshu I've found the issue. initContainers (and others like sidecarContainers) are pruned by the api-server.

This is because the RunnerDeployment and related CRD's had the default preserveUnknownFields: true when using apiextensions.k8s.io/v1beta1, however, since #629 and related PR's the apiVersion was bumped to apiextensions.k8s.io/v1, which now has a default of preserveUnknownFields: false and now why fields with non-structural schema's like initContainers and sidecarContainers are being pruned.

You can test this by applying the v1beta1 CRD, creating a test RunnerDeployment with a initContainer, which works as expected and then delete the v1beta1 CRD and apply the latest v1 CRD and notice how the same RunnerDeployment is now no longer valid.

$ cat /tmp/test-runner.yaml
apiVersion: actions.summerwind.dev/v1alpha1
kind: RunnerDeployment
metadata:
  name: test-runner
spec:
  template:
    spec:
      dockerMTU: 9001
      dockerdWithinRunnerContainer: true
      ephemeral: true
      image: 'summerwind/actions-runner-dind:v2.278.0-ubuntu-20.04'
      initContainers:
        - name: copy-deps
          command:
            - sh
            - '-c'
            - echo "Hello"
          image: apline
      repository: test/test

$ kubectl apply -f https://raw.githubusercontent.com/actions-runner-controller/actions-runner-controller/ce48dc58e685d8bf48d369fe114434374aebc3c7/charts/actions-runner-controller/crds/actions.summerwind.dev_runnerdeployments.yaml
Warning: apiextensions.k8s.io/v1beta1 CustomResourceDefinition is deprecated in v1.16+, unavailable in v1.22+; use apiextensions.k8s.io/v1 CustomResourceDefinition
customresourcedefinition.apiextensions.k8s.io/runnerdeployments.actions.summerwind.dev created
$ kubectl apply -f /tmp/test-runner.yaml -n default
runnerdeployment.actions.summerwind.dev/test-runner created
$ kubectl get runnerdeployments.actions.summerwind.dev test-runner -n default -oyaml
apiVersion: actions.summerwind.dev/v1alpha1
kind: RunnerDeployment
metadata:
  annotations:
    kubectl.kubernetes.io/last-applied-configuration: |
      {"apiVersion":"actions.summerwind.dev/v1alpha1","kind":"RunnerDeployment","metadata":{"annotations":{},"name":"test-runner","namespace":"default"},"spec":{"template":{"spec":{"dockerMTU":9001,"dockerdWithinRunnerContainer":true,"ephemeral":true,"image":"summerwind/actions-runner-dind:v2.278.0-ubuntu-20.04","initContainers":[{"command":["sh","-c","echo \"Hello\""],"image":"apline","name":"copy-deps"}],"repository":"test/test"}}}}
  creationTimestamp: "2021-09-10T13:29:48Z"
  generation: 1
  name: test-runner
  namespace: default
  resourceVersion: "62817"
  uid: d5e1252f-3487-465c-bac3-7587bb6ed6e3
spec:
  template:
    spec:
      dockerMTU: 9001
      dockerdWithinRunnerContainer: true
      ephemeral: true
      image: summerwind/actions-runner-dind:v2.278.0-ubuntu-20.04
      initContainers:
      - command:
        - sh
        - -c
        - echo "Hello"
        image: apline
        name: copy-deps
      repository: test/test

$ # delete the CRD, if you apply the new CRD it will add `preserveUnknownFields: true` when migrating from v1beta1 to v1
$ kubectl delete crd runnerdeployments.actions.summerwind.dev
customresourcedefinition.apiextensions.k8s.io "runnerdeployments.actions.summerwind.dev" deleted
$ kubectl apply -f https://raw.githubusercontent.com/actions-runner-controller/actions-runner-controller/master/charts/actions-runner-controller/crds/actions.summerwind.dev_runnerdeployments.yaml
customresourcedefinition.apiextensions.k8s.io/runnerdeployments.actions.summerwind.dev created
$ kubectl apply -f /tmp/test-runner.yaml -n default
The RunnerDeployment "test-runner" is invalid: spec.template.spec.initContainers.name: Required value

Where validation now fails because the initContainers field is pruned by the api-server since preserveUnknownFields: false

[donovanmuller]

This should be solved by #803