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

Explore alternatives to rate limited GitHub API for clusterctl #3982

Closed
fabriziopandini opened this issue Dec 3, 2020 · 29 comments
Closed
Labels
area/clusterctl Issues or PRs related to clusterctl help wanted Denotes an issue that needs help from a contributor. Must meet "help wanted" guidelines. kind/feature Categorizes issue or PR as related to a new feature. lifecycle/frozen Indicates that an issue or PR should not be auto-closed due to staleness. needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release.

Comments

@fabriziopandini
Copy link
Member

User Story

As a user, I would like to use clusterctl without the risk of incurring in GitHub API rate limits.

Detailed Description

Clusterctl fetches artifacts from the provider's repository, and if the repository is hosted on GitHub this operation is subject to GitHub API rate limits.

#2450 already raised this problem, and #2848 implemented caching so now clusterctl init executes 8 API calls (+3 for each additional provider), which is ~1/6 of the total number of allowed calls in 1hr (without a GitHub token).

While this is generally ok for the majority of the users, there is still a chance that user (especially developers or CI tools) incur in this limit, so we should explore alternatives for overcoming these limits e.g.

/kind feature
/area clusterctl

@k8s-ci-robot k8s-ci-robot added kind/feature Categorizes issue or PR as related to a new feature. area/clusterctl Issues or PRs related to clusterctl labels Dec 3, 2020
@fabriziopandini
Copy link
Member Author

/milestone v0.4.0

@k8s-ci-robot k8s-ci-robot added this to the v0.4.0 milestone Dec 3, 2020
@fabriziopandini
Copy link
Member Author

/help

@k8s-ci-robot
Copy link
Contributor

@fabriziopandini:
This request has been marked as needing help from a contributor.

Please ensure the request meets the requirements listed here.

If this request no longer meets these requirements, the label can be removed
by commenting with the /remove-help command.

In response to this:

/help

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@k8s-ci-robot k8s-ci-robot added the help wanted Denotes an issue that needs help from a contributor. Must meet "help wanted" guidelines. label Dec 3, 2020
@vincepri
Copy link
Member

vincepri commented Dec 4, 2020

I'd make use of Go module proxy + raw.github (wherever possible)

@fejta-bot
Copy link

Issues go stale after 90d of inactivity.
Mark the issue as fresh with /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.

If this issue is safe to close now please do so with /close.

Send feedback to sig-contributor-experience at kubernetes/community.
/lifecycle stale

@k8s-ci-robot k8s-ci-robot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Mar 4, 2021
@vincepri
Copy link
Member

vincepri commented Mar 4, 2021

/lifecycle frozen

@k8s-ci-robot k8s-ci-robot added lifecycle/frozen Indicates that an issue or PR should not be auto-closed due to staleness. and removed lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. labels Mar 4, 2021
@vincepri
Copy link
Member

Can we prioritize this issue a bit more? GitHub rate limits are pretty fast to hit and that might impact the user experience, could we move to use go modules like we do in

rawURL := url.URL{
Scheme: "https",
Host: "proxy.golang.org",
Path: path.Join(gomodule, "@v", "/list"),
}
resp, err := http.Get(rawURL.String())
if err != nil {
return "", err
}
defer resp.Body.Close()
out, err := io.ReadAll(resp.Body)
if err != nil {
return "", err
}
parsedTags := semver.Versions{}
for _, line := range strings.Split(string(out), "\n") {
if strings.HasPrefix(line, "v") {
parsedTags = append(parsedTags, semver.MustParse(strings.TrimPrefix(line, "v")))
}
}
sort.Sort(parsedTags)
?

@vincepri
Copy link
Member

/milestone v1.0

@k8s-ci-robot k8s-ci-robot modified the milestones: v0.4, v1.0 Oct 19, 2021
@vincepri vincepri modified the milestones: v1.0, v1.1 Oct 22, 2021
@fabriziopandini fabriziopandini modified the milestones: v1.1, v1.2 Feb 3, 2022
@CecileRobertMichon
Copy link
Contributor

/priority important-soon

Users (especially developers) are regularly hitting this issue.

@k8s-ci-robot k8s-ci-robot added the priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release. label Apr 26, 2022
@fabriziopandini fabriziopandini added the triage/accepted Indicates an issue or PR is ready to be actively worked on. label Jul 29, 2022
@fabriziopandini fabriziopandini removed this from the v1.2 milestone Jul 29, 2022
@fabriziopandini fabriziopandini removed the triage/accepted Indicates an issue or PR is ready to be actively worked on. label Jul 29, 2022
@fabriziopandini
Copy link
Member Author

/triage accepted
/help-wanted

@k8s-ci-robot k8s-ci-robot added the triage/accepted Indicates an issue or PR is ready to be actively worked on. label Aug 5, 2022
@sbueringer
Copy link
Member

sbueringer commented Aug 26, 2022

Hm I think that should not happen. Never had the problem on my machine. Maybe there is a way to confirm that the token / your account is really over the rate limit?

Sorry super stupid question but I assume the GITHUB_TOKEN env var is exported?

@chrischdi
Copy link
Member

I'll take a look and try to at least implement the improvement mentioned by @vincepri here: #3982 (comment) 😃

@vladimirvivien
Copy link

vladimirvivien commented Aug 26, 2022

I created a fresh PAT and it seems to work (I may have tested with an old PAT). I will keep an eye and report if it happens again.

The PAT requirement should be documented though.

@ykakarap
Copy link
Contributor

The PAT requirement should be documented though.

+1. I will open an issue for this.

@fabriziopandini
Copy link
Member Author

fabriziopandini commented Sep 22, 2022

After reading the list of releases from go modules, the next big step is to find a way to download release assets without using GitHub API

@sbueringer
Copy link
Member

sbueringer commented Sep 22, 2022

Might be a stupid question but as we only have the sources in the go modules (https://pkg.go.dev/sigs.k8s.io/cluster-api#section-directories), from where else can we get the built/rendered release assets?

@fabriziopandini
Copy link
Member Author

I did not research the topic extensively, but if we can build an url like https://github.com/kubernetes-sigs/cluster-api/releases/download/v1.2.2/bootstrap-components.yaml with the info we have, I assume we can download from there without passing from the GitHub API, similar to when I click to links in the asset section in https://github.com/kubernetes-sigs/cluster-api/releases/tag/v1.2.2 page

In other words, we will continue to get the assets from GitHub but using HTTP calls instead of the the GitHub APIs

@k8s-triage-robot
Copy link

This issue is labeled with priority/important-soon but has not been updated in over 90 days, and should be re-triaged.
Important-soon issues must be staffed and worked on either currently, or very soon, ideally in time for the next release.

You can:

  • Confirm that this issue is still relevant with /triage accepted (org members only)
  • Deprioritize it with /priority important-longterm or /priority backlog
  • Close this issue with /close

For more details on the triage process, see https://www.kubernetes.dev/docs/guide/issue-triage/

/remove-triage accepted

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

/triage accepted

@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 Feb 9, 2023
@mjnovice
Copy link

mjnovice commented May 4, 2023

Any updates on this ?

@killianmuldoon
Copy link
Contributor

@mjnovice - nobody has followed up on this as a general case as far as I know. Have you run into this issue?

@mjnovice
Copy link

mjnovice commented May 4, 2023

@killianmuldoon we have a pipeline which creates and destroys clusterAPI on kind cluster, and we constantly get throttled by this.

@mjnovice
Copy link

mjnovice commented May 4, 2023

Any way we can circumvent this ?

@killianmuldoon
Copy link
Contributor

Any way we can circumvent this ?

Circumvention for this is to use a github token, which should get rid of most of the rate limiting, or else use a local repository with the yamls that github normally tries to get from the internet. You should also pin versions to prevent clustertctl from looking for it.

@ykakarap
Copy link
Contributor

ykakarap commented May 9, 2023

+1 to what @killianmuldoon said. The bulk of the calls that clusterctl makes are to look up versions and download files from the github releases. There were some improvements made to try and redude the gihtub aoi calls by using goporxy to look up versions but the best ways to avoid rate limiting are as Killian mentioned:

  • Use a github token
  • Use a local repository
  • Pin versions (this only helps with version look ups but will still need to access github to download files)

@k8s-triage-robot
Copy link

This issue is labeled with priority/important-soon but has not been updated in over 90 days, and should be re-triaged.
Important-soon issues must be staffed and worked on either currently, or very soon, ideally in time for the next release.

You can:

  • Confirm that this issue is still relevant with /triage accepted (org members only)
  • Deprioritize it with /priority important-longterm or /priority backlog
  • Close this issue with /close

For more details on the triage process, see https://www.kubernetes.dev/docs/guide/issue-triage/

/remove-triage accepted

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

fabriziopandini commented Jan 19, 2024

The remaining items in this issue have been addressed by #9237 and #9236

/close

@k8s-ci-robot
Copy link
Contributor

@fabriziopandini: Closing this issue.

In response to this:

The remaining items in this issue should now be addressed by #9237 and #9236

/close

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/clusterctl Issues or PRs related to clusterctl help wanted Denotes an issue that needs help from a contributor. Must meet "help wanted" guidelines. kind/feature Categorizes issue or PR as related to a new feature. lifecycle/frozen Indicates that an issue or PR should not be auto-closed due to staleness. needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release.
Projects
None yet
Development

No branches or pull requests