Rate limiting: Use conditional requests for GitHub commit status updates

[fadi-bwa]

We use notification-controller to update our GitHub repos' commit statuses. Since there are a couple of hundred of those repositories and we reconcile our Kustomizations quite often, we get rate limited by GitHub.

Since we'd like to keep our short reconciliation interval I'm proposing to implement conditional requests for requests made to GitHub by the notification-controller.

All that's required for that is to save the value of the etag header from the last request made to a repository and then add its value to a new header if-none-match on the next request.

Happy to share any additional info you need or help with the implementation if I can, please just ping me - thanks!

[makkes]

Do I understand correct that you have set .spec.interval to a rather low value (which one exactly?) and the content of the Git repository doesn't change as often as that? In that case, notification-controller wouldn't try to update the commit status.

Which request exactly is rate-limited by GitHub for you?

[fadi-bwa]

Ah sorry, should've attached an error message. See this:

{
    "level": "error",
    "ts": "2024-11-11T12:47:41.888Z",
    "logger": "event-server",
    "msg": "failed to send notification",
    "eventInvolvedObject": {
        "kind": "Kustomization",
        "namespace": "ns",
        "name": "app",
        "uid": "123",
        "apiVersion": "kustomize.toolkit.fluxcd.io/v1",
        "resourceVersion": "1328084087"
    },
    "Alert": {
        "name": "alert",
        "namespace": "ns"
    },
    "error": "could not list commit statuses: GET https://api.github.com/repos/org/repo/commits/912873/statuses?per_page=50: 403 API rate limit exceeded for user ID 93142435. If you reach out to GitHub Support for  help, please include the request ID 2800:197694:FAE359:FD89B0:6731FCED and timestamp 2024-11-11 12:47:41 UTC. [rate reset in 20m09s]"
}

I think this is because the code tries to list commit statuses to decide whether to send an update or not.
And yes, we're on a very short (1m) interval at the moment.

[makkes]

And yes, we're on a very short (1m) interval at the moment.

I know you said you'd like to keep it that way but I wonder why, because dialing that up would be the easiest fix. The interval on Kustomization mainly serves to rollback manual changes to the resources. If you have a reasonably low interval on the sources, changes to those will also trigger reconciliations of all Kustomizations.

[fadi-bwa]

We're keeping it that low to make sure our Kustomizations are deployed early. But I think this might have been a misconception on our side - it looks like we should keep the GitRepository's spec.interval low for that purpose and set the Kustomization's interval to something much longer - correct?

As in:

• Check the repo every minute, deploy changes if need be (which triggers a notification)
• Check the Kustomization every 10 minutes (which will always also trigger a notification)

[makkes]

it looks like we should keep the GitRepository's spec.interval low for that purpose and set the Kustomization's interval to something much longer - correct?

Exactly. I usually set the Kustomization interval to 1 hour on my clusters to keep the noise low.

[fadi-bwa]

OK, great! We'll change our setup accordingly, thank you!