Add support for dockerhub rate-limiting

[Grimm75]

Add support for dockerhub API rate-limiting

Added function which wraps around client.Do() and:

• pauses when rate-limit is hit (HTTP err:429) for time given in retry-after header.
• checks the limit info from HTTP headers sent by API and will pause
  appropriately if rate-limit depletion is eminent.
• max. 2 attempts are made in addition to initial one before giving up.

And replaced calls to client.Do() with wrapped one.

See:
https://docs.docker.com/docker-hub/api/latest/#tag/rate-limiting

PS: Above docs are incorrect about x-retry-after header - name and content doesn't match observed headers from dockerhub, which are retry-after (without x-) and content is positive integer - number of seconds to wait, conforming to RFC9110 10.2.3.

Issue being fixed

Fixes #19967

Please indicate you've done the following:

• Well Written Title and Summary of the PR
• Label the PR as needed. "release-note/ignore-for-release, release-note/new-feature, release-note/update, release-note/enhancement, release-note/community, release-note/breaking-change, release-note/docs, release-note/infra, release-note/deprecation"
• Accepted the DCO. Commits without the DCO will delay acceptance.
• Made sure tests are passing and test coverage is added if needed.
• Considered the docs impact and opened a new docs issue or PR with docs changes if needed in website repository.

[Vad1mo]

this is a great contribution. thank you very much.. We need to give it a test

[Grimm75]

Thank you too, I just sqished some style related bugs, we'll see how it goes now.

[Grimm75]

fixed/explained in Fix: review - ... commits.

[MinerYang]

• add retry will increase your request to docker which actually would increase your risk of having rate limit error.
• response time will hang in there

[Grimm75]

• add retry will increase your request to docker which actually would increase your risk of having rate limit error.
• response time will hang in there

Fixed, 'retry' is now used ONLY when last response was 429 and we waited.

[wy65701436]

From the perspective of Harbor core, adding retry would result in more GET or HEAD requests to the upstream Docker registry, potentially increasing the chances of hitting rate limits.

Personally, I believe it's better to fail early and inform the client of the failure ASAP. This allows the client to decide whether to implement retry on their end. Hiding the retry functionality on the server side would inevitably increase response time, which could lead to timeouts for some clients. This approach doesn't seem practical to me.

[Grimm75]

From the perspective of Harbor core, adding retry would result in more GET or HEAD requests to the upstream Docker registry, potentially increasing the chances of hitting rate limits.

Personally, I believe it's better to fail early and inform the client of the failure ASAP. This allows the client to decide whether to implement retry on their end. Hiding the retry functionality on the server side would inevitably increase response time, which could lead to timeouts for some clients. This approach doesn't seem practical to me.

Retry was implemented in order to keep running replication alive during block time (in exchange for time waited).
When we are already blocked, there is no reason to do anything with dockerhub API.

But I can see your point and without retry, the solution is also more elegant:

https://github.com/Grimm75/harbor/blob/without-retry/src/pkg/reg/adapter/dockerhub/adapter.go#L129-L151

[Vad1mo]

@wy65701436 can you elaborate on what would be required to change in order to get this PRs merged.
It would be helpfuls for @Grimm75 to know what should be changed.

IMO, it definitely makes sense to be compliant and respect the Docker Hub rate limits.

[Grimm75]

@wy65701436, what should I do to get this PR accepted?

1. use current version
2. use current version + decrease attempts to '2' (try only once again if 1st response was 429)
3. use simple version without retry here
4. something else (please elaborate)

I'm OK with any solution that makes Harbor able to replicate projects with many tags.

[Grimm75]

Is there anybody to help me with this PR?

[codecov]

Codecov Report

Attention: Patch coverage is 27.27273% with 32 lines in your changes are missing coverage. Please review.

Project coverage is 67.47%. Comparing base (b7b8847) to head (901500f).
Report is 122 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main   #19984      +/-   ##
==========================================
- Coverage   67.56%   67.47%   -0.09%     
==========================================
  Files         991      998       +7     
  Lines      109181   110134     +953     
  Branches     2719     2724       +5     
==========================================
+ Hits        73768    74317     +549     
- Misses      31449    31831     +382     
- Partials     3964     3986      +22     

Flag	Coverage Δ
unittests	67.47% <27.27%> (-0.09%)	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files	Coverage Δ
src/pkg/reg/adapter/dockerhub/adapter.go	30.50% <27.27%> (-0.47%)	⬇️

... and 510 files with indirect coverage changes

[Vad1mo]

@wy65701436 can we get some indication for @Grimm75 in what direction to move?