Skip to content

Add client.Reconnect API#2149

Merged
dmcgowan merged 1 commit intocontainerd:masterfrom
crosbymichael:reconnect
Feb 21, 2018
Merged

Add client.Reconnect API#2149
dmcgowan merged 1 commit intocontainerd:masterfrom
crosbymichael:reconnect

Conversation

@crosbymichael
Copy link
Copy Markdown
Member

This adds a reconnect api to the client so that the client instance
stays the same and on reconnect, all tasks and containers with
references to the *Client have the correct connection.

Signed-off-by: Michael Crosby [email protected]

@cpuguy83
Copy link
Copy Markdown
Member

Thanks! This works well, tested in moby/moby.

Comment thread client.go Outdated
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

no connector available

This adds a reconnect api to the client so that the client instance
stays the same and on reconnect, all tasks and containers with
references to the *Client have the correct connection.

Signed-off-by: Michael Crosby <[email protected]>
@cpuguy83
Copy link
Copy Markdown
Member

I guess a WithConnector for people using NewWithConn would help as well.

@codecov-io
Copy link
Copy Markdown

codecov-io commented Feb 21, 2018

Codecov Report

Merging #2149 into master will decrease coverage by 0.03%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2149      +/-   ##
==========================================
- Coverage   45.56%   45.53%   -0.04%     
==========================================
  Files          80       80              
  Lines        8726     8726              
==========================================
- Hits         3976     3973       -3     
- Misses       4105     4107       +2     
- Partials      645      646       +1
Flag Coverage Δ
#linux 50.24% <ø> (ø) ⬆️
#windows 41.09% <ø> (-0.04%) ⬇️
Impacted Files Coverage Δ
gc/scheduler/scheduler.go 67.96% <0%> (-1.46%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 255ad41...7b653dc. Read the comment docs.

Copy link
Copy Markdown
Member

@estesp estesp left a comment

Choose a reason for hiding this comment

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

LGTM

@dmcgowan
Copy link
Copy Markdown
Member

I guess a WithConnector for people using NewWithConn would help as well.

Resolve in this PR?

LGTM as is, adding an opt in a follow up seems reasonable and would allow for some interesting flexibility.

@cpuguy83
Copy link
Copy Markdown
Member

SGTM

@dmcgowan dmcgowan merged commit fc87dce into containerd:master Feb 21, 2018
@crosbymichael crosbymichael deleted the reconnect branch February 22, 2018 16:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants