Skip to content

Add FifoIO to expose fifos directly to client#1405

Merged
dmcgowan merged 1 commit intocontainerd:masterfrom
crosbymichael:fifos
Aug 22, 2017
Merged

Add FifoIO to expose fifos directly to client#1405
dmcgowan merged 1 commit intocontainerd:masterfrom
crosbymichael:fifos

Conversation

@crosbymichael
Copy link
Copy Markdown
Member

This allows clients an easier way to interact with the fifos for a
container without having to use the built in copyIO functions when
opening fifos.

It's nothing that clients could not have already coded but since we use
this type of functionality in the tests it makes sense to add an
implementation here.

Fixes #1402

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

Comment thread container_linux_test.go Outdated
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I'd rather not lose the test for Windows 👼

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

There is not much of a difference with directIO for windows because of the pipe and Accept conn

@codecov-io
Copy link
Copy Markdown

codecov-io commented Aug 21, 2017

Codecov Report

Merging #1405 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #1405   +/-   ##
=======================================
  Coverage   40.81%   40.81%           
=======================================
  Files          23       23           
  Lines        2923     2923           
=======================================
  Hits         1193     1193           
  Misses       1452     1452           
  Partials      278      278

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 1046064...a8b21da. Read the comment docs.

This allows clients an easier way to interact with the fifos for a
container without having to use the built in copyIO functions when
opening fifos.

It's nothing that clients could not have already coded but since we use
this type of functionality in the tests it makes sense to add an
implementation here.

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

@mlaventure i think we should review and merge this to fix the issues we are seeing in PRs with the attach tests. I have it on my todo list to reenable this for windows

Copy link
Copy Markdown
Contributor

@mlaventure mlaventure left a comment

Choose a reason for hiding this comment

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

Fair enough.

LGTM

@dmcgowan
Copy link
Copy Markdown
Member

LGTM

@dmcgowan dmcgowan merged commit 4028add into containerd:master Aug 22, 2017
@crosbymichael crosbymichael deleted the fifos branch August 23, 2017 13:09
mauriciovasquezbernal pushed a commit to kinvolk/containerd that referenced this pull request Nov 13, 2020
reload cni network config if has fs change events
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.

4 participants