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

Add a timeout to all registry requests #1970

Merged
merged 2 commits into from Apr 24, 2019

Conversation

@2opremio
Copy link
Collaborator

2opremio commented Apr 24, 2019

(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 docker/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) {

This comment has been minimized.

Copy link
@2opremio

2opremio Apr 24, 2019

Author Collaborator

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?

This comment has been minimized.

Copy link
@squaremo

squaremo Apr 24, 2019

Member

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

This comment has been minimized.

Copy link
@2opremio

2opremio Apr 24, 2019

Author Collaborator

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 left a comment

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
2 checks passed
2 checks passed
ci/circleci: build Your tests passed on CircleCI!
Details
ci/circleci: e2e-testing Your tests passed on CircleCI!
Details
@2opremio 2opremio deleted the 2opremio:set-warmer-timeouts branch Apr 24, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.