Enable Flannel to support Windows for host-gw and VXLAN #832

rakelkar · 2017-10-04T05:05:15Z

Description

This PR enables Flannel to build and run on windows and support configuring Kubernetes using the host-gw and VXLAN backend. 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.

The change requires a related change to the Flannel CNI to enable it to delegate to Windows CNI.
PRs open on CNI:
flannel CNI for windows: containernetworking/plugins#76
host-local IPAM for windows: containernetworking/plugins#77

Todos

Tests
Documentation

This change is the first required step to supporting flannel on Windows

This uses new Windows Host Networking Service (HNS) features to configure networking for the host-gw mode Fixes flannel-io#833

tomdee

Thanks for this PR. I've taken a look and it's looking broadly OK, though I do have a few questions/comments.

Common code
What are your thoughts on all the duplicate code in hostgw_*.go? The Windows and Linux versions share quite a bit of code, do you think it should be pulled out?
Testing
How will the flannel.exe binary be automatically tested? TravisCI doesn't support windows so we need some alternative.
You'll need to add some windows specific tests before this can be merged.
Maintenance
Who is going to be responsible for ongoing maintenance for Windows support in flannel? I am likely going to need help in this area since I don't have a Windows computer. I'm thinking of things like Windows specific test failing, reviewing Windows specific PR and providing Windows support.
Documentation
This PR will need to include some Windows specific documentation
Build
This PR will need to include some way of building and releasing the Windows flannel binary

Also, you shouldn't have added logrus as a submodule in vendor. If you need it to be updated you should do it through glide.

tomdee · 2017-10-13T23:13:27Z

backend/hostgw/hostgw_network_windows.go

+	"github.com/coreos/flannel/backend"
+	"github.com/coreos/flannel/subnet"
+
+	netroute "github.com/rakelkar/gonetsh/netroute"


This will need to be added to the glide.* files

tomdee · 2017-10-13T23:16:15Z

backend/hostgw/hostgw_windows.go

+import (
+	"fmt"
+
+	"github.com/Microsoft/hcsshim"


this will also need to be added to glide

tomdee · 2017-10-13T23:19:03Z

backend/vxlan/vxlan_windows.go

@@ -0,0 +1,25 @@
+// +build windows


Do you need this file since vxlan isn't currently supported on Windows?

Merged VXLAN support

tomdee · 2017-10-13T23:19:36Z

network/ipmasq_windows.go

@@ -0,0 +1,34 @@
+// +build windows


Why have a windows version of this file?

Had to move some IPMASQ functions out of main - see updated file.

rakelkar · 2017-10-14T18:54:13Z

Thanks for taking a look Tom. Your comments are very reasonable. I'm going to update the PR with changes I've made for VXLAN and address the easy fixes like removing logrus and adding some unit tests. Will get back to you on how best to build/integration test after I get a chance to huddle with others on the Windows networking team. Just letting you know so you're not surprised to see the PR increase in feature scope before your comments are addressed - this will allow me to avoid merge madness.

…

On Fri, Oct 13, 2017 at 4:28 PM, Tom Denham ***@***.***> wrote: ***@***.**** requested changes on this pull request. Thanks for this PR. I've taken a look and it's looking broadly OK, though I do have a few questions/comments. - Common code What are your thoughts on all the duplicate code in hostgw_*.go? The Windows and Linux versions share quite a bit of code, do you think it should be pulled out? - Testing How will the flannel.exe binary be automatically tested? TravisCI doesn't support windows so we need some alternative. You'll need to add some windows specific tests before this can be merged. - Maintenance Who is going to be responsible for ongoing maintenance for Windows support in flannel? I am likely going to need help in this area since I don't have a Windows computer. I'm thinking of things like Windows specific test failing, reviewing Windows specific PR and providing Windows support. - Documentation This PR will need to include some Windows specific documentation Also, you shouldn't have added logrus as a submodule in vendor. If you need it to be updated you should do it through glide. ------------------------------ In backend/hostgw/hostgw_network_windows.go <#832 (comment)>: > +// See the License for the specific language governing permissions and +// limitations under the License. + +package hostgw + +import ( + "sync" + "time" + + log "github.com/golang/glog" + "golang.org/x/net/context" + + "github.com/coreos/flannel/backend" + "github.com/coreos/flannel/subnet" + + netroute "github.com/rakelkar/gonetsh/netroute" This will need to be added to the glide.* files ------------------------------ In backend/hostgw/hostgw_windows.go <#832 (comment)>: > +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package hostgw + +import ( + "fmt" + + "github.com/Microsoft/hcsshim" this will also need to be added to glide ------------------------------ In backend/vxlan/vxlan_windows.go <#832 (comment)>: > @@ -0,0 +1,25 @@ +// +build windows Do you need this file since vxlan isn't currently supported on Windows? ------------------------------ In network/ipmasq_windows.go <#832 (comment)>: > @@ -0,0 +1,34 @@ +// +build windows Why have a windows version of this file? — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#832 (review)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ADQphVIlA5QEGEzKQh_Nbv4QL5NTfcA2ks5sr_IhgaJpZM4PtGcf> .

-- Rakesh Kelkar

* Fix build errors after rebase by moving Linux specific for IPMASQ from main.go to ipmasq.go * delete unused cniConf struct and redundant comment * Support for vxlan (overlay) backend in Windows. * Makes windows VXLAN MAC prefix configurable Records subnet add/remove time Recycles remote endpoints if PA address has changed Sets remote MAC and PA on remote endpoint Adds config checks to reduce user error Adds unit tests

rakelkar · 2017-10-14T20:00:51Z

Snuff out logrus (it snuck in)

tomdee · 2017-10-20T16:04:23Z

@rakelkar If it's possible to submit smaller PRs then that's helpful for me. e.g. moving the ipmasq code out of main might be a generally applicable change which doesn't depend on the Windows changes.

rakelkar · 2017-10-20T17:04:46Z

Can do..

…

On Oct 20, 2017 9:14 AM, "Tom Denham" ***@***.***> wrote: @rakelkar <https://github.com/rakelkar> If it's possible to submit smaller PRs then that's helpful for me. e.g. moving the ipmasq code out of main might be a generally applicable change which doesn't depend on the Windows changes. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#832 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ADQpheSFml_G2st5OIjYHffbkBQptlZBks5suMSTgaJpZM4PtGcf> .

rakelkar · 2017-10-23T20:55:30Z

@tomdee We are planning to break this PR up into the following new PRs:
PR1: ipmasq to build for windows - this is a just refactor, hopefully can be approved without too much controversy :)
PR2: build for windows, appveyor, windows iface code+gonetsh via glide
This PR will focus on getting Flannel to build on windows, with a little bit of functionality to determine the default interface.
PR3a: hostgw
This PR will add _windows files for hostgw functionality
PR3b: vxlan
This PR will add _windows files for vxlan functionality

rakelkar · 2017-10-23T21:54:31Z

Maintenance: The team that own the Windows Host Networking Services has agreed to own maintenance of these features. This is ofcourse dependent on automated testing, still poking about to figure out best to automate.

michmike · 2017-10-27T20:55:30Z

cc: @alinbalutoiu

tomdee · 2017-11-24T03:50:11Z

Closing this PR as it's now being done in stages.

This was referenced Oct 4, 2017

Support Windows in host-gw and VXLAN modes #833

Closed

Support delegating to Windows CNI from Flannel CNI containernetworking/plugins#76

Closed

rakelkar force-pushed the rakelkar/windows-hostgw branch from 1f6a757 to 9eb1ff4 Compare October 5, 2017 00:29

rakelkar and others added 2 commits October 4, 2017 17:34

flannel: get flannel to build and run on windows

a00bfc2

This change is the first required step to supporting flannel on Windows

backend: Support host-gw backed on Windows

4e73b4c

This uses new Windows Host Networking Service (HNS) features to configure networking for the host-gw mode Fixes flannel-io#833

rakelkar force-pushed the rakelkar/windows-hostgw branch from 9eb1ff4 to 4e73b4c Compare October 5, 2017 00:34

tomdee suggested changes Oct 13, 2017

View reviewed changes

rakelkar added 2 commits October 14, 2017 11:55

Merge branch 'master' into rakelkar/windows-hostgw

dfc4099

rakelkar changed the title ~~Enable Flannel to support Windows for host-gw~~ Enable Flannel to support Windows for host-gw and VXLAN Oct 14, 2017

revert logrus addition - its not needed

31b0b86

jroggeman mentioned this pull request Oct 25, 2017

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

Merged

tomdee added needs/rework platform/windows labels Oct 31, 2017

jroggeman mentioned this pull request Nov 14, 2017

backend: Get flannel building on windows with stubs #879

Merged

tomdee closed this Nov 24, 2017

jroggeman mentioned this pull request Nov 28, 2017

flannel: Get flannel running on windows #891

Merged

This was referenced Jan 16, 2018

host-gw: add windows support #921

Closed

vxlan: add windows support #922

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Enable Flannel to support Windows for host-gw and VXLAN #832

Enable Flannel to support Windows for host-gw and VXLAN #832

rakelkar commented Oct 4, 2017 •

edited

Loading

tomdee left a comment •

edited

Loading

tomdee Oct 13, 2017

tomdee Oct 13, 2017

tomdee Oct 13, 2017

rakelkar Oct 14, 2017

tomdee Oct 13, 2017

rakelkar Oct 14, 2017

rakelkar commented Oct 14, 2017 via email

rakelkar commented Oct 14, 2017

tomdee commented Oct 20, 2017

rakelkar commented Oct 20, 2017 via email

rakelkar commented Oct 23, 2017

rakelkar commented Oct 23, 2017

michmike commented Oct 27, 2017

tomdee commented Nov 24, 2017

Enable Flannel to support Windows for host-gw and VXLAN #832

Enable Flannel to support Windows for host-gw and VXLAN #832

Conversation

rakelkar commented Oct 4, 2017 • edited Loading

Description

Todos

tomdee left a comment • edited Loading

Choose a reason for hiding this comment

tomdee Oct 13, 2017

Choose a reason for hiding this comment

tomdee Oct 13, 2017

Choose a reason for hiding this comment

tomdee Oct 13, 2017

Choose a reason for hiding this comment

rakelkar Oct 14, 2017

Choose a reason for hiding this comment

tomdee Oct 13, 2017

Choose a reason for hiding this comment

rakelkar Oct 14, 2017

Choose a reason for hiding this comment

rakelkar commented Oct 14, 2017 via email

rakelkar commented Oct 14, 2017

tomdee commented Oct 20, 2017

rakelkar commented Oct 20, 2017 via email

rakelkar commented Oct 23, 2017

rakelkar commented Oct 23, 2017

michmike commented Oct 27, 2017

tomdee commented Nov 24, 2017

rakelkar commented Oct 4, 2017 •

edited

Loading

tomdee left a comment •

edited

Loading