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

network: move ipmasq management into platform-specific files #854

Merged
merged 1 commit into from
Nov 10, 2017

Conversation

jroggeman
Copy link
Contributor

Description

As requested in #832 , we're breaking up the changes into smaller commits. This PR moves the Setup, monitoring, and teardown of IPMasq settings into platform-specific files. This is one of the requirements to get Flannel to build on Windows.

From the original PR (#832):

The goal is to enable users to provision a Kubernetes cluster with windows nodes in VXLAN mode or mixed windows and Linux nodes in host-gw mode.

Copy link
Contributor

@rakelkar rakelkar left a comment

Choose a reason for hiding this comment

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

:shipit:

@jroggeman
Copy link
Contributor Author

@tomdee This addresses your comment from #832

@tomdee
Copy link
Contributor

tomdee commented Oct 28, 2017

The tests are failing. I restarted it as it looks like a google connectivity problem.

Copy link
Contributor

@tomdee tomdee left a comment

Choose a reason for hiding this comment

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

Just one comment, otherwise looks good.

@@ -1,3 +1,5 @@
// +build linux
Copy link
Contributor

Choose a reason for hiding this comment

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

What about just naming the file _linux.go?

@tomdee
Copy link
Contributor

tomdee commented Oct 31, 2017

Since there is no actual Windows specific code in here, why not use this PR to just move iptables stuff out of main.go but not do any file renaming?

@jroggeman
Copy link
Contributor Author

@tomdee updated review to only be the minor refactoring, can you take a look?

@tomdee
Copy link
Contributor

tomdee commented Nov 3, 2017

Looks good. If you squash the commits I can merge.

@jroggeman
Copy link
Contributor Author

Squashed changes are pushed :)

@tomdee
Copy link
Contributor

tomdee commented Nov 10, 2017

Looks good, merging.

@tomdee tomdee merged commit ab36802 into flannel-io:master Nov 10, 2017
@jroggeman jroggeman deleted the jroggeman/ipmasq branch November 10, 2017 19:01
@rakelkar
Copy link
Contributor

#833

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.

3 participants