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

cluster: Renew the context after communicating with the registry #31586

Merged
merged 1 commit into from Mar 14, 2017

Conversation

Projects
None yet
6 participants
@aaronlehmann
Contributor

aaronlehmann commented Mar 7, 2017

When pinning by digest, the registry might be slow or unresponsive. This
could cause the context to already be expired by the time UpdateService
or CreateService is called. We want digest pinning to be a best-effort
operation, so it's problematic if a slow or misbehaving registry
prevents the service operation from completing. Replace the context
after communicating with the registry, so we have a fresh timeout for
the gRPC call.

Fixes #31427

cc @nishanttotla

We might want to backport this to a stable release.

cluster: Renew the context after communicating with the registry
When pinning by digest, the registry might be slow or unresponsive. This
could cause the context to already be expired by the time UpdateService
or CreateService is called. We want digest pinning to be a best-effort
operation, so it's problematic if a slow or misbehaving registry
prevents the service operation from completing. Replace the context
after communicating with the registry, so we have a fresh timeout for
the gRPC call.

Signed-off-by: Aaron Lehmann <aaron.lehmann@docker.com>
@nishanttotla

This comment has been minimized.

Show comment
Hide comment
@nishanttotla

nishanttotla Mar 7, 2017

Contributor

Code LGTM

I'll try to find a way to test this PR

Contributor

nishanttotla commented Mar 7, 2017

Code LGTM

I'll try to find a way to test this PR

@vdemeester

LGTM 🐯

@thaJeztah

LGTM

@thaJeztah thaJeztah referenced this pull request Mar 14, 2017

Merged

bump 17.04.0-rc1 #31811

@vieux vieux merged commit 08bbd43 into moby:master Mar 14, 2017

5 of 6 checks passed

z Jenkins build is being scheduled
Details
dco-signed All commits are signed
experimental Jenkins build Docker-PRs-experimental 31373 has succeeded
Details
janky Jenkins build Docker-PRs 39991 has succeeded
Details
powerpc Jenkins build Docker-PRs-powerpc 415 has succeeded
Details
windowsRS1 Jenkins build Docker-PRs-WoW-RS1 11061 has succeeded
Details

@GordonTheTurtle GordonTheTurtle added this to the 17.04.0 milestone Mar 14, 2017

@thaJeztah

This comment has been minimized.

Show comment
Hide comment
@thaJeztah

thaJeztah Mar 15, 2017

Member

We might want to backport this to a stable release.

If we need this into 17.03, can you prepare a cherry-pick?

Member

thaJeztah commented Mar 15, 2017

We might want to backport this to a stable release.

If we need this into 17.03, can you prepare a cherry-pick?

@aaronlehmann aaronlehmann deleted the aaronlehmann:digest-pin-context branch Mar 15, 2017

@aaronlehmann

This comment has been minimized.

Show comment
Hide comment
@aaronlehmann

aaronlehmann Mar 15, 2017

Contributor

Do you mean I should open a PR against 17.03.x?

Contributor

aaronlehmann commented Mar 15, 2017

Do you mean I should open a PR against 17.03.x?

@thaJeztah

This comment has been minimized.

Show comment
Hide comment
@thaJeztah

thaJeztah Mar 15, 2017

Member

@aaronlehmann yes, if you have time; I think there's another PR that needs to go into 17.03.1, so possibly there's gonna be an rc2; if not, we can always close the PR

Member

thaJeztah commented Mar 15, 2017

@aaronlehmann yes, if you have time; I think there's another PR that needs to go into 17.03.1, so possibly there's gonna be an rc2; if not, we can always close the PR

@aaronlehmann

This comment has been minimized.

Show comment
Hide comment
@aaronlehmann

aaronlehmann Mar 15, 2017

Contributor

Opened #31861

Contributor

aaronlehmann commented Mar 15, 2017

Opened #31861

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment