-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
WIP: Use StatefulSet to run multiple runners #4
Conversation
// RunnerSpec defines the desired state of Runner | ||
type RunnerSpec struct { | ||
// +kubebuilder:validation:MinLength=3 | ||
// +kubebuilder:validation:Pattern=`^[^/]+/[^/]+$` | ||
Repository string `json:"repository"` | ||
|
||
// +optional | ||
Image string `json:"image"` | ||
Replicas *int32 `json:"replicas,omitempty"` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you use RunnerSet
or something similar for managing stateful sets instead of pods?
Doing so will allow me to deploy both pod-based and stateful-set-based runners for comparison against real workloads.
// If that makes sense, I'll rename the current RunnnerSet
that I've added in #1 to RunnerReplicaSet
to not collide and make mine look like a less recommended way.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just submitted #6 for the rename
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for your helpful comment and PR!
Using the RunnerSet
resource for StatefulSet based management is a great idea. I'm going to merge #6 after reviewing.
To hand over the name `RunnerSet` to the new StatefulSet-based implementation of that being developed at actions#4
* 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>
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions. |
@mumoshu PRs are included by the stale bot at the moment, if any are they are going to hang around they need one of the exempt labels. Alternatively you can reconfigure the stale config to monitor issues only if you prefer, whatever you think is best 🤷 |
Let me close this due to inactivity and conflicts, but feel free to restart another pull request. Thanks anyway! |
No description provided.