Skip to content

Conversation

@mumoshu
Copy link
Collaborator

@mumoshu mumoshu commented Feb 20, 2020

This is still WIP but I just wanted to submit this as a draft for the early review :)

The build passes and the implementation should be complete. But it's not tested at all. I'll make this out of the draft state once I've tested it on a real cluster.

UPDATE: This is tested E2E with running a Ginkgo suite on GitHub Actions. See make test and pkg/controllers/runnerset_controller_test.go.


RunnerSet is basically ReplicaSet for Runners.

It is responsible for maintaining number of runners to match the desired one. That is, it creates missing runners from .Spec.Template and deletes redundant runners.

Similar to ReplicaSet, this does not support rolling update of runners on its own. We might want to later add RunnerDeployment for that. But that's another story.

Notes:

  • RunnerSet has availableReplicas and readyReplicas under Status, which are showns in kubectl get output as AVAILABLE and READY respectively. These are borrowed from ReplicaSet.

Copy link
Contributor

@summerwind summerwind left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for the contribution! This change makes Runner easier to scale.
Can you please update CRD and RBAC with make manifests?

RunnerSet is basically ReplicaSet for Runners.

It is responsible for maintaining number of runners to match the desired one. That is, it creates missing runners from `.Spec.Template` and deletes redundant runners.

Similar to ReplicaSet, this does not support rolling update of runners on its own. We might want to later add `RunnerDeployment` for that. But that's another story.
@mumoshu mumoshu marked this pull request as ready for review February 24, 2020 01:34
@mumoshu
Copy link
Collaborator Author

mumoshu commented Feb 24, 2020

@summerwind I believe this is now ready for your review.

Not sure this can either be a long-term solution or not. But I think this still worth being included in this project as the baseline to compare future solutions against.

Background: As we've talked face-to-face, a StatefulSe-and-mutating-webhook based solution may replace this. More concretely, we may completely remove RunnerSet and instead just introduce RunnerDeployment based on the statefulset+mutating webhook solution in future. But until then this works

@mumoshu
Copy link
Collaborator Author

mumoshu commented Feb 25, 2020

I've added make tests to run E2E tests on GitHub Actions. I've confirmed it to work on my fork. Please see Run tests job in https://github.com/mumoshu/actions-runner-controller-ci/runs/459416632?check_suite_focus=true

mumoshu added a commit to mumoshu/actions-runner-controller that referenced this pull request Feb 27, 2020
Adds the initial version of RunnerDeployment that is intended to manage RunnerSets(actions#1), like Deployment manages ReplicaSets.

This is the initial version and therefore is bare bone. The only update strategy it supports is `Recreate`, which recreates the underlying RunnerSet when the runner template changes. I'd like to add `RollingUpdate` strategy once this is merged.

This depends on actions#1 so the diff contains that of actions#1, too. Please see only the latest commit for review.

Also see https://github.com/mumoshu/actions-runner-controller-ci/runs/471329823?check_suite_focus=true to confirm that `make tests` is passing after changes made in this commit.
@mumoshu mumoshu mentioned this pull request Feb 27, 2020
@summerwind
Copy link
Contributor

@mumoshu Thank you for adding tests in addition to the implementation!
I agree with using this as a "baseline" implementation.

The code looks good to me. Let's merge.

@summerwind summerwind merged commit 338da81 into actions:master Mar 1, 2020
mumoshu pushed a commit that referenced this pull request Jan 24, 2021
mumoshu pushed a commit that referenced this pull request Jan 24, 2021
* Add chart workflows (#1)

* Add chart workflows

* Fix publishing step in CI

Signed-off-by: David Young <davidy@funkypenguin.co.nz>

* Update CI on push-to-master (#3)

* Put helm installation step in the correct CI job

Signed-off-by: David Young <davidy@funkypenguin.co.nz>

* Put helm installation step in the correct CI job (#4)

* Update on-push-master-publish-chart.yml

* Remove references to certmanager dependency

Signed-off-by: David Young <davidy@funkypenguin.co.nz>

* Add ability to customize kube-rbac-proxy image

Signed-off-by: David Young <davidy@funkypenguin.co.nz>

* Only install cert-manager if we're going to spin up KinD

Signed-off-by: David Young <davidy@funkypenguin.co.nz>
bm1216 added a commit to THG-Site-Reliability-Engineering/actions-runner-controller that referenced this pull request Sep 5, 2022
* Add cmd line arg for enterprise url. Fix enterprise bug.

* Fix package import order

* Fix comment
justinsb pushed a commit to justinsb/actions-runner-controller that referenced this pull request Apr 24, 2025
Simple PoC for running a VM with a trampoline pod
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants