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

Release notes API rate limiting throws 403 #2225

Closed
saschagrunert opened this issue Aug 25, 2021 · 8 comments · Fixed by #2324 or kubernetes-sigs/release-sdk#11
Closed

Release notes API rate limiting throws 403 #2225

saschagrunert opened this issue Aug 25, 2021 · 8 comments · Fixed by #2324 or kubernetes-sigs/release-sdk#11
Labels
area/release-eng Issues or PRs related to the Release Engineering subproject kind/bug Categorizes issue or PR as related to a bug. priority/critical-urgent Highest priority. Must be actively worked on as someone's top priority right now. sig/release Categorizes an issue or PR as relevant to SIG Release.
Milestone

Comments

@saschagrunert
Copy link
Member

The release notes tool seems to get a new API response from GitHub if the rate limit kicks in:

level=fatal msg="gathering release notes: listing release notes: gathering notes: GET https://api.github.com/repos/cri-o/cri-o/commits/1aaf47484dcf1206663610c622a1b37a84e2e91b/pulls?page=1&per_page=100&state=closed: 403 You have exceeded a secondary rate limit. Please wait a few minutes before you try again. []"

Ref: https://github.com/cri-o/cri-o/runs/3422342329?check_suite_focus=true

It seems that we can mitigate by checking for the error and waiting a bit longer before trying it again.

@saschagrunert saschagrunert added kind/bug Categorizes issue or PR as related to a bug. sig/release Categorizes an issue or PR as relevant to SIG Release. area/release-eng Issues or PRs related to the Release Engineering subproject labels Aug 25, 2021
@saschagrunert
Copy link
Member Author

/help
/priority important-soon
/good-first-issue

@k8s-ci-robot
Copy link
Contributor

@saschagrunert:
This request has been marked as suitable for new contributors.

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-good-first-issue command.

In response to this:

/help
/priority important-soon
/good-first-issue

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 priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release. good first issue Denotes an issue ready for a new contributor, according to the "help wanted" guidelines. help wanted Denotes an issue that needs help from a contributor. Must meet "help wanted" guidelines. and removed needs-priority labels Aug 25, 2021
@saschagrunert
Copy link
Member Author

cc @kubernetes/release-managers

@sandipanpanda
Copy link
Member

Hi there, I am a new contributor to Kubernetes and hopped in seeing good first issue label, can you please give a little insight on how to get started working on this issue?

@saschagrunert
Copy link
Member Author

Sounds great @sandipanpanda, thank you for taking this opportunity. I think a good starting point would be to reproduce the issue with the release-notes tool: https://github.com/kubernetes/release/tree/master/cmd/release-notes

After that, we can try to catch the error in the retry logic of the release notes gatherer, which is using the go-github API. The retry logic can be found there:

func GithubErrChecker(maxTries int, sleeper func(time.Duration)) func(error) bool {
try := 0
return func(err error) bool {
if err == nil {
return false
}
if try >= maxTries {
logrus.Errorf("Max retries (%d) reached, not retrying anymore: %v", maxTries, err)
return false
}
try++
if aerr, ok := err.(*github.AbuseRateLimitError); ok {
waitDuration := defaultGithubSleep
if d := aerr.RetryAfter; d != nil {
waitDuration = *d
}
logrus.
WithField("err", aerr).
Infof("Hit the abuse rate limit on try %d, sleeping for %s", try, waitDuration)
sleeper(waitDuration)
return true
}
return false
}
}

@rayandas
Copy link
Member

rayandas commented Sep 1, 2021

Hi @sandipanpanda are you working on this issue? If not then I can take this up and give it a try.

@rayandas
Copy link
Member

rayandas commented Sep 2, 2021

/assign

@justaugustus
Copy link
Member

This just burned a stage and should have higher priority.
/remove-priority important-soon
/priority critical-urgent
/milestone v1.23
cc: @kubernetes/release-engineering
ref: https://kubernetes.slack.com/archives/CJH2GBF7Y/p1631645686102400?thread_ts=1631606375.087500&cid=CJH2GBF7Y

@k8s-ci-robot k8s-ci-robot added priority/critical-urgent Highest priority. Must be actively worked on as someone's top priority right now. and removed priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release. labels Sep 14, 2021
@k8s-ci-robot k8s-ci-robot added this to the v1.23 milestone Sep 14, 2021
@justaugustus justaugustus removed help wanted Denotes an issue that needs help from a contributor. Must meet "help wanted" guidelines. good first issue Denotes an issue ready for a new contributor, according to the "help wanted" guidelines. labels Sep 14, 2021
@rayandas rayandas removed their assignment Sep 15, 2021
saschagrunert added a commit to saschagrunert/release-sdk that referenced this issue Oct 4, 2021
We now check for the `github.RateLimitError` and use its result to wait
before an actual retry. This should avoid hitting the secondary rate
limit, as described there:

- https://docs.github.com/en/rest/overview/resources-in-the-rest-api#rate-limiting
- https://docs.github.com/en/rest/guides/best-practices-for-integrators

Refers to kubernetes/release#2225

Signed-off-by: Sascha Grunert <sgrunert@redhat.com>
saschagrunert added a commit to saschagrunert/release-sdk that referenced this issue Oct 4, 2021
We now check for the `github.RateLimitError` and use its result to wait
before an actual retry. This should avoid hitting the secondary rate
limit, as described there:

- https://docs.github.com/en/rest/overview/resources-in-the-rest-api#rate-limiting
- https://docs.github.com/en/rest/guides/best-practices-for-integrators

Refers to kubernetes/release#2225

Signed-off-by: Sascha Grunert <sgrunert@redhat.com>
puerco added a commit to puerco/release that referenced this issue Nov 17, 2021
The last couple of releases have been hitting a new secondary rate limit
in the GitHub API. This PR makes some changes to play a bit more fair with
the API.

Now, the notes gatherer interprets the GitHub API responde. If the
secondary rate limit was hit, the functions that make a lot of calls
will now sleep for a minute+random before respawning.

Fixes kubernetes#2225
Fixes kubernetes#2302

Signed-off-by: Adolfo García Veytia (Puerco) <adolfo.garcia@uservers.net>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/release-eng Issues or PRs related to the Release Engineering subproject kind/bug Categorizes issue or PR as related to a bug. priority/critical-urgent Highest priority. Must be actively worked on as someone's top priority right now. sig/release Categorizes an issue or PR as relevant to SIG Release.
Projects
None yet
5 participants