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

runtime: Use server-side throttling when available #270

Merged
merged 2 commits into from
May 11, 2022

Conversation

aryan9600
Copy link
Member

Checks if the PriorityAndFairness flow control filter is enabled to
disable client side throttling, leveraging server side throttling
instead, since client-go respects the Retry-After response header set
by the APF filter.

Fixes: #269

Signed-off-by: Sanskar Jaiswal jaiswalsanskar078@gmail.com

@aryan9600 aryan9600 requested a review from stefanprodan May 10, 2022 16:42
// GetConfigOrDie wraps ctrl.GetConfigOrDie and checks if the Kubernetes apiserver
// has PriorityAndFairness flow control filter enabled. If true, it returns a rest.Config
// with client side throttling disabled. Otherwise, it returns a modified rest.Config
// configured with the provided Options.
func GetConfigOrDie(opts Options) *rest.Config {
Copy link
Member

Choose a reason for hiding this comment

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

While working on fluxcd/helm-controller#480, I discovered this func does not have a non-panicing counterpart.

I think this would be useful to have in situations where you work with the option, but not the configuration (like the above PR).

runtime/client/client.go Outdated Show resolved Hide resolved
@stefanprodan stefanprodan added the area/runtime Controller runtime related issues and pull requests label May 11, 2022
@stefanprodan
Copy link
Member

@aryan9600 please test this code with the Flux RBAC and see if we need to allow any new endpoints. Looking at the flowcontrol code seems that access to the version API is required.

@aryan9600
Copy link
Member Author

I don't think that's the case, since only the hostURL is returned and the api versioned path is dropped here. I'll check once anyway to be sure.

Copy link
Member

@stefanprodan stefanprodan left a comment

Choose a reason for hiding this comment

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

LGTM

Thanks @aryan9600

@stefanprodan
Copy link
Member

@aryan9600 please rebase

aryan9600 added 2 commits May 11, 2022 16:42
Checks if the PriorityAndFairness flow control filter is enabled to
disable client side throttling, leveraging server side throttling
instead, since client-go respects the `Retry-After` response header set
by the APF filter

Signed-off-by: Sanskar Jaiswal <jaiswalsanskar078@gmail.com>
Signed-off-by: Sanskar Jaiswal <jaiswalsanskar078@gmail.com>
@aryan9600 aryan9600 force-pushed the server-side-throttling branch from cedb123 to 5909ab5 Compare May 11, 2022 11:12
@stefanprodan stefanprodan changed the title use server side throttling if enabled runtime: Use server-side throttling when available May 11, 2022
@stefanprodan stefanprodan merged commit ed96ab7 into fluxcd:main May 11, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/runtime Controller runtime related issues and pull requests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

runtime: Use server-side throttling
4 participants