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

client checks kernel module in /sys/module for WSL2 bridge networking #17306

Merged
merged 2 commits into from
Jun 6, 2023

Conversation

jeteve
Copy link
Contributor

@jeteve jeteve commented May 24, 2023

This is a solution for issue #17305 and will enable nomad to have greater chances to detect the bridge kernel module on WSL2 hosts, thus enabling bridge networking.

Closes #17305

@hashicorp-cla
Copy link

hashicorp-cla commented May 24, 2023

CLA assistant check
All committers have signed the CLA.

Copy link
Contributor

@lgfa29 lgfa29 left a comment

Choose a reason for hiding this comment

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

Thanks for the contribution @jeteve!

I left some minor comments about error handling and variable naming but I was able to get this to work after the small adjustment described in microsoft/WSL#6044 (comment) to enable legacy mode for iptables. Did you run into this as well?

I was also not able to get the docker driver to work (the network pre-run hook fails with the error failed to Statfs "/var/run/docker/netns/...), but that looks like a fix for another time 😄

Using the exec driver in bridge mode worked well 👍

I will also ping @shoenig for another check since he has more experience on this area than me.

client/fingerprint/bridge_linux.go Outdated Show resolved Hide resolved
client/fingerprint/bridge_linux.go Outdated Show resolved Hide resolved
@jeteve jeteve force-pushed the 17305-bridge-detection-wsl branch from bd7c4f6 to ee72bea Compare June 3, 2023 19:57
@jeteve
Copy link
Contributor Author

jeteve commented Jun 3, 2023

Hi @lgfa29 , thanks a lot for your review. I've pushed some amended code.

Copy link
Contributor

@lgfa29 lgfa29 left a comment

Choose a reason for hiding this comment

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

Changes look good to me, thanks @jeteve!

I just pushed a commit to add a CHANGELOG entry.

@lgfa29 lgfa29 added the backport/1.5.x backport to 1.5.x release line label Jun 5, 2023
@jeteve
Copy link
Contributor Author

jeteve commented Jun 6, 2023

Thanks a lot @lgfa29 , and apologies for skipping the CHANGELOG myself.

I see all checks are good except 'Vercel – nomad'. I assume unblocking this is part of your release flow?

Thanks!

Copy link
Member

@shoenig shoenig left a comment

Choose a reason for hiding this comment

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

LGTM!

@lgfa29
Copy link
Contributor

lgfa29 commented Jun 6, 2023

I see all checks are good except 'Vercel – nomad'. I assume unblocking this is part of your release flow?

I tried to approve it a few times but it doesn't seem to be working 😅

But that's not a problem for this PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport/1.5.x backport to 1.5.x release line
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Lookup bridge kernel module in /sys/module/ to enable running on WSL2
4 participants