Skip to content
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

Make EphemeralRunnerController MaxConcurrentReconciles configurable #3832

Merged
merged 4 commits into from
Dec 11, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 8 additions & 6 deletions controllers/actions.github.com/ephemeralrunner_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -823,12 +823,14 @@ func (r *EphemeralRunnerReconciler) deleteRunnerFromService(ctx context.Context,
}

// SetupWithManager sets up the controller with the Manager.
func (r *EphemeralRunnerReconciler) SetupWithManager(mgr ctrl.Manager) error {
return ctrl.NewControllerManagedBy(mgr).
For(&v1alpha1.EphemeralRunner{}).
Owns(&corev1.Pod{}).
WithEventFilter(predicate.ResourceVersionChangedPredicate{}).
Complete(r)
func (r *EphemeralRunnerReconciler) SetupWithManager(mgr ctrl.Manager, opts ...Option) error {
return builderWithOptions(
ctrl.NewControllerManagedBy(mgr).
For(&v1alpha1.EphemeralRunner{}).
Owns(&corev1.Pod{}).
WithEventFilter(predicate.ResourceVersionChangedPredicate{}),
opts,
).Complete(r)
}

func runnerContainerStatus(pod *corev1.Pod) *corev1.ContainerStatus {
Expand Down
90 changes: 90 additions & 0 deletions controllers/actions.github.com/options.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,90 @@
package actionsgithubcom

import (
"fmt"
"os"
"strconv"

"sigs.k8s.io/controller-runtime/pkg/builder"
"sigs.k8s.io/controller-runtime/pkg/controller"
)

// Options is the optional configuration for the controllers, which can be
// set via command-line flags or environment variables.
type Options struct {
// RunnerMaxConcuncurrentReconciles is the maximum number of concurrent Reconciles which can be run
// by the EphemeralRunnerController.
RunnerMaxConcuncurrentReconciles int
}

// OptionsWithDefault returns the default options.
// This is here to maintain the options and their default values in one place,
// rather than having to correlate those in multiple places.
func OptionsWithDefault() Options {
return Options{
RunnerMaxConcuncurrentReconciles: 1,
}
}

// LoadEnv loads the options from the environment variables.
// This updates the option value only if the environment variable is set.
// If the option is already set (via a command-line flag), the value from the environment variable takes precedence.
func (o *Options) LoadEnv() error {
v, err := o.getEnvInt("RUNNER_MAX_CONCURRENT_RECONCILES")
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I added this env just to make it easy to give it a shot without modifying the chart.

Instead, we can modify the manager args template there

Copy link
Member

Choose a reason for hiding this comment

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

@mumoshu - I recommend we go with modifying the chart and adding this as an extra element under flags:

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@Link- Thanks! Addressed in 0bebb66

if err != nil {
return err
}

if v != nil {
o.RunnerMaxConcuncurrentReconciles = *v
}

return nil
}

func (o *Options) getEnvInt(name string) (*int, error) {
s := os.Getenv(name)
if s == "" {
return nil, nil
}

v, err := strconv.Atoi(s)
if err != nil {
return nil, fmt.Errorf("failed to convert %s=%s to int: %w", name, s, err)
}

return &v, nil
}

type Option func(*controller.Options)

// WithMaxConcurrentReconciles sets the maximum number of concurrent Reconciles which can be run.
//
// This is useful to improve the throughput of the controller, but it may also increase the load on the API server and
// the external service (e.g. GitHub API). The default value is 1, as defined by the controller-runtime.
//
// See https://github.com/actions/actions-runner-controller/issues/3021 for more information
// on real-world use cases and the potential impact of this option.
func WithMaxConcurrentReconciles(n int) Option {
return func(b *controller.Options) {
b.MaxConcurrentReconciles = n
}
}

// builderWithOptions applies the given options to the provided builder, if any.
// This is a helper function to avoid the need to import the controller-runtime package in every reconciler source file
// and the command package that creates the controller.
// This is also useful for reducing code duplication around setting controller options in
// multiple reconcilers.
func builderWithOptions(b *builder.Builder, opts []Option) *builder.Builder {
if len(opts) == 0 {
return b
}

var controllerOpts controller.Options
for _, opt := range opts {
opt(&controllerOpts)
}

return b.WithOptions(controllerOpts)
}
11 changes: 10 additions & 1 deletion main.go
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,8 @@ func main() {

autoScalerImagePullSecrets stringSlice

opts = actionsgithubcom.OptionsWithDefault()

commonRunnerLabels commaSeparatedStringSlice
)
var c github.Config
Expand Down Expand Up @@ -136,6 +138,7 @@ func main() {
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.")
flag.IntVar(&opts.RunnerMaxConcuncurrentReconciles, "runner-max-concurrent-reconciles", opts.RunnerMaxConcuncurrentReconciles, "The maximum number of concurrent reconciles which can be run by the EphemeralRunner controller. Increase this value to improve the throughput of the controller, but it may also increase the load on the API server and the external service (e.g. GitHub API).")
flag.Var(&commonRunnerLabels, "common-runner-labels", "Runner labels in the K1=V1,K2=V2,... format that are inherited all the runners created by the controller. See https://github.com/actions/actions-runner-controller/issues/321 for more information")
flag.StringVar(&namespace, "watch-namespace", "", "The namespace to watch for custom resources. Set to empty for letting it watch for all namespaces.")
flag.StringVar(&watchSingleNamespace, "watch-single-namespace", "", "Restrict to watch for custom resources in a single namespace.")
Expand All @@ -156,6 +159,12 @@ func main() {
}
c.Log = &log

if err := opts.LoadEnv(); err != nil {
fmt.Fprintf(os.Stderr, "Error: loading environment variables: %v\n", err)
os.Exit(1)
}
log.Info("Using options", "runner-max-concurrent-reconciles", opts.RunnerMaxConcuncurrentReconciles)

if !autoScalingRunnerSetOnly {
ghClient, err = c.NewClient()
if err != nil {
Expand Down Expand Up @@ -285,7 +294,7 @@ func main() {
Scheme: mgr.GetScheme(),
ActionsClient: actionsMultiClient,
ResourceBuilder: rb,
}).SetupWithManager(mgr); err != nil {
}).SetupWithManager(mgr, actionsgithubcom.WithMaxConcurrentReconciles(opts.RunnerMaxConcuncurrentReconciles)); err != nil {
log.Error(err, "unable to create controller", "controller", "EphemeralRunner")
os.Exit(1)
}
Expand Down
Loading