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

worker/containerd: NewWorkerOpt: remove workaround for named pipes #5399

Merged
merged 1 commit into from
Oct 8, 2024

Conversation

thaJeztah
Copy link
Member

relates to;

This workaround was added in 414edfa, pending changes in containerd. Those changes were included in containerd 1.7.12, which was vendored in bce2f79, so the workaround should no longer be needed.

I also reformatted the code, as there's no longer a need to wrap this function since the list of arguments has been reduced.

This workaround was added in 414edfa, pending
changes in containerd. Those changes were included in containerd 1.7.12, which
was vendored in bce2f79, so the workaround
should no longer be needed.

I also reformatted the code, as there's no longer a need to wrap this function
since the list of arguments has been reduced.

Signed-off-by: Sebastiaan van Stijn <[email protected]>
Copy link
Collaborator

@profnandaa profnandaa left a comment

Choose a reason for hiding this comment

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

LGTM.

address := workerOpts.Address

if goRuntime.GOOS == "windows" {
// TODO(profnandaa): once the upstream PR[1] is merged and
Copy link
Collaborator

Choose a reason for hiding this comment

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

thanks for helping with this TODO!

Copy link
Member Author

Choose a reason for hiding this comment

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

YW! I stumbled on it while working on some things in moby/moby, and it was low-hanging fruit 🥹😅

@crazy-max crazy-max added this to the v0.17.0 milestone Oct 8, 2024
@crazy-max crazy-max merged commit f3f34fb into moby:master Oct 8, 2024
92 checks passed
@thaJeztah thaJeztah deleted the remove_npipe_hack branch October 8, 2024 12:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants