-
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
Make EphemeralRunnerController MaxConcurrentReconciles configurable #3832
Merged
+78
−7
Merged
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit
Hold shift + click to select a range
0d88dae
Make EphemeralRunnerController MaxConcurrentReconciles configurable
mumoshu b0f52ba
Increase the default EphemeralRunnerController RunnerMaxConcuncurrent…
mumoshu 0bebb66
Prefer flags over envvar for RunnerMaxConcurrentReconciles
mumoshu f8b1adc
Merge branch 'master' into runner-concurrent-reconcile
Link- File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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") | ||
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) | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
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
actions-runner-controller/charts/gha-runner-scale-set-controller/templates/deployment.yaml
Line 50 in 8b36ea9
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.
@mumoshu - I recommend we go with modifying the chart and adding this as an extra element under
flags
:actions-runner-controller/charts/gha-runner-scale-set-controller/values.yaml
Line 97 in 8b36ea9
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.
@Link- Thanks! Addressed in 0bebb66