Skip to content
This repository has been archived by the owner on Nov 1, 2022. It is now read-only.

Add a timeout to all registry requests #1970

Merged
merged 2 commits into from
Apr 24, 2019

Conversation

2opremio
Copy link
Contributor

(Hopefully) fixes #1940 and #1808

Registry request could get stuck indefinitely due to a combination of:

  1. The repository client we use:

    This has been addressed by Honor contexts passed to registry client methods distribution/distribution#2905 , prompting me to replace https://github.com/docker/distribution by https://github.com/2opremio/distribution until the PR is merged.

  2. We didn't set any context deadline we just set the context Background()

cacheClient: cacheClient,
func newRepoCacheManager(now time.Time,
repoID image.Name, clientFactory registry.ClientFactory, creds registry.Credentials, repoClientTimeout time.Duration,
burst int, trace bool, logger log.Logger, cacheClient Client) (*repoCacheManager, error) {
Copy link
Contributor Author

@2opremio 2opremio Apr 24, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think the amount of parameters (which comes from the previous refactoring I made) is reasonable at all.

I tried a few alternatives but they were worse. Any suggestions?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some people (including me occasionally) wrap optional values in a config struct, which is passed by value.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Uhm, only one (or maybe two) are really optional.

@hiddeco hiddeco added this to the v1.12.1 milestone Apr 24, 2019
Copy link
Member

@hiddeco hiddeco left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lets fix the amount of parameters in a second round, fixing the bug itself is more important. 🚢

@2opremio 2opremio merged commit d4b0ae7 into fluxcd:master Apr 24, 2019
@2opremio 2opremio deleted the set-warmer-timeouts branch April 24, 2019 15:59
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Registry cache warmer stops logging
3 participants