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

Bump containerd client #36684

Merged
merged 2 commits into from Apr 19, 2018
Merged

Bump containerd client #36684

merged 2 commits into from Apr 19, 2018

Conversation

@cpuguy83
Copy link
Contributor

cpuguy83 commented Mar 23, 2018

  • Bumps containerd client
  • Uses containerd client's new Reconnect() API to reconnect cached clients instead of replacing them (leaving stale clients in other cached objects).
@@ -114,6 +114,13 @@ type client struct {
containers map[string]*container
}

func (c *client) reconnect() error {

This comment has been minimized.

Copy link
@cpuguy83

cpuguy83 Mar 23, 2018

Author Contributor

We probably need to catch connection failures in normal requests and trigger a reconnect, but we aren't handling these cases at all (or ever AFAIK).

This comment has been minimized.

Copy link
@mlaventure

mlaventure Apr 18, 2018

Contributor

Those requests would just fail until the connection monitor reconnects normally.

@thaJeztah

This comment has been minimized.

Copy link
Member

thaJeztah commented Mar 23, 2018

Will this change be part of the upcoming containerd 1.1.0 release? Wondering if we can get back to released versions of the dependency (or at least something on a release branch)

@cpuguy83

This comment has been minimized.

Copy link
Contributor Author

cpuguy83 commented Mar 23, 2018

I think so, a release branch has not be cut yet.

@cpuguy83

This comment has been minimized.

Copy link
Contributor Author

cpuguy83 commented Mar 23, 2018

The new client API is not really "new" anymore since it's been a couple of weeks, btw.

@thaJeztah

This comment has been minimized.

Copy link
Member

thaJeztah commented Mar 23, 2018

Alright, so probably safe to assume it’ll be in 1.1.0, and once that’s released, we could switch to a tagged version, thanks!

@cpuguy83 cpuguy83 force-pushed the cpuguy83:bump_containerd_client branch from 271b782 to 5554c31 Mar 27, 2018
@codecov

This comment has been minimized.

Copy link

codecov bot commented Mar 27, 2018

Codecov Report

❗️ No coverage uploaded for pull request base (master@b6a7d02). Click here to learn what that means.
The diff coverage is 0%.

@@            Coverage Diff            @@
##             master   #36684   +/-   ##
=========================================
  Coverage          ?   35.33%           
=========================================
  Files             ?      614           
  Lines             ?    46345           
  Branches          ?        0           
=========================================
  Hits              ?    16375           
  Misses            ?    27797           
  Partials          ?     2173
@cpuguy83 cpuguy83 force-pushed the cpuguy83:bump_containerd_client branch from 5554c31 to f7a5424 Mar 27, 2018
cpuguy83 added 2 commits Mar 28, 2018
This does not bump the containerd binary.
Picks last commit before go1.10 switch, which is not currently supported
in moby.

Signed-off-by: Brian Goff <cpuguy83@gmail.com>
This fixes an issue where the containerd client is cached in a container
object in libcontainerd and becomes stale after containerd is restarted.

Signed-off-by: Brian Goff <cpuguy83@gmail.com>
@cpuguy83 cpuguy83 force-pushed the cpuguy83:bump_containerd_client branch from f7a5424 to 2c682d5 Mar 28, 2018
@mlaventure mlaventure added the rebuild/* label Apr 9, 2018
@crosbymichael

This comment has been minimized.

Copy link
Member

crosbymichael commented Apr 17, 2018

LGTM

Copy link
Contributor

mlaventure left a comment

LGTM

Copy link
Member

vdemeester left a comment

LGTM 🐯

@vdemeester vdemeester merged commit 8bb5a28 into moby:master Apr 19, 2018
8 of 9 checks passed
8 of 9 checks passed
codecov/patch 0% of diff hit (target 50%)
Details
codecov/project No report found to compare against
Details
dco-signed All commits are signed
experimental Jenkins build Docker-PRs-experimental 40023 has succeeded
Details
janky Jenkins build Docker-PRs 48894 has succeeded
Details
powerpc Jenkins build Docker-PRs-powerpc 9343 has succeeded
Details
vendor Jenkins build Docker-PRs-vendor 4184 has succeeded
Details
windowsRS1 Jenkins build Docker-PRs-WoW-RS1 20218 has succeeded
Details
z Jenkins build Docker-PRs-s390x 9164 has succeeded
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
6 participants
You can’t perform that action at this time.