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

Ensure we use the least amount of http Clients possible #8598

Open
sbueringer opened this issue May 3, 2023 · 4 comments
Open

Ensure we use the least amount of http Clients possible #8598

sbueringer opened this issue May 3, 2023 · 4 comments
Assignees
Labels
kind/feature Categorizes issue or PR as related to a new feature. priority/important-longterm Important over the long term, but may not be staffed and/or may need multiple releases to complete. triage/accepted Indicates an issue or PR is ready to be actively worked on.

Comments

@sbueringer
Copy link
Member

What would you like to be added (User Story)?

In CR v0.15 there was a change which allows us to reduce the amount of http Clients used and thus the amount of TCP connections.

Let's take a look at our controllers and see if we can further optimize.

Detailed Description

Anything else you would like to add?

No response

Label(s) to be applied

/kind feature
One or more /area label. See https://github.com/kubernetes-sigs/cluster-api/labels?q=area for the list of labels.

@k8s-ci-robot k8s-ci-robot added kind/feature Categorizes issue or PR as related to a new feature. needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. labels May 3, 2023
@sbueringer
Copy link
Member Author

/triage accepted
/assign

@k8s-ci-robot k8s-ci-robot added triage/accepted Indicates an issue or PR is ready to be actively worked on. and removed needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. labels May 3, 2023
@fabriziopandini
Copy link
Member

is there a link or some more info about the CR change?

@sbueringer
Copy link
Member Author

sbueringer commented May 4, 2023

PR: kubernetes-sigs/controller-runtime#2122

Draft release notes:

  • Make *http.Client configurable and use/share the same client by default (📖 Update README to point to 2020 meeting notes. #2122)
    • When using the default Manager configuration, no immediate changes are needed.
    • client/apiutil.NewDynamicRESTMapper signature has changed and now requires an *http.Client as parameter.
    • cluster.Cluster interface requires GetHTTPClient() method which must return an already configured, non-nil, *http.Client for the Cluster. When using cluster.New to create Clusters, the client is created internally if not specified as an Options field.
    • cluster.Options.MapperProvider field now requires a *rest.Config and *http.Client.

I already partially implement this in my bump PR. I just want to check (also in the context of scale testing) if it can be improved a bit more.

@fabriziopandini
Copy link
Member

/priority important-longterm

@k8s-ci-robot k8s-ci-robot added the priority/important-longterm Important over the long term, but may not be staffed and/or may need multiple releases to complete. label Apr 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/feature Categorizes issue or PR as related to a new feature. priority/important-longterm Important over the long term, but may not be staffed and/or may need multiple releases to complete. triage/accepted Indicates an issue or PR is ready to be actively worked on.
Projects
None yet
Development

No branches or pull requests

3 participants