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

Windows CNI for overlay (vxlan) and host-gw (l2bridge) modes #85

Closed
wants to merge 1 commit into from

Conversation

rakelkar
Copy link
Contributor

@rakelkar rakelkar commented Nov 1, 2017

This is an early PR to get feedback on a resolution to #80. Proposes changes to add CNI plugins for windows into the CNI repository. The plugins use the Windows Kernel Host Networking Service (HNS) APIs via the HCSSHIM package to configure an IP address on the windows network stack. Depends on #77.

TODO:

@squeed
Copy link
Member

squeed commented Nov 6, 2017

This seems pretty reasonable. One important thing - modules in pkg should have docblocks on all functions.

@gkudra-msft
Copy link

I like this. A couple of points:

  • To be clear, plugin-specific parameters are part of the top-level configuration (like ipMasq and endpointMacPrefix)?

  • The existing (private) wincni.exe binary would no longer exist. Instead, the unified code would be a part of this pkg/hns/* package. This is where code interpreting additionalArgs into HNS policy would go. Is the structure of the additionalArgs different than raw HNS policy, and if so, should there be docs for that?

  • A feature (or "capability") doesn't seem like it belongs in either of these. The Linux model for capabilities such as port mapping seems to have separate plugins. For Windows, it makes sense (to me) to put them into the hns package. Will they be implicitly applied when the plugin passes them along? My gut (and analysis of the code) says no; how, then, do we do that cleanly?

@rakelkar
Copy link
Contributor Author

@gkudra-msft

  1. endpointMacPrefix is not part of top-level configuration since l2bridge mode doesn't need it.
  2. The current code already supports merging additionalArgs - the structure is identical.
  3. I think portmapping is a chained plugin to enable all Linux plugins to use it. For windows it depends on whether policies can be set after the endpoint is created. If yes, makes sense to have a windows verson of portmap if no, then the right thing to do it to put helpers into pkg/hns and use while creating endpoints

@madhanrm
Copy link

will update the PR with review comments

@madhanrm
Copy link

Sample build output:
madhanm@NetAppLinux:~/repo/gopath/src/github.com/containernetworking/plugins$ bash build.sh
Building plugins
flannel
portmap
tuning
bridge
host-device
ipvlan
loopback
macvlan
ptp
vlan
l2bridge.exe
overlay.exe
dhcp
host-local
sample

@madhanrm madhanrm force-pushed the windowsCni branch 5 times, most recently from ef5bf7e to ab985f5 Compare December 16, 2017 09:01
@madhanrm
Copy link

Fixed the Windows CI test.

@madhanrm
Copy link

@squeed Can you look into this and get this merged? Is there any open items here?

@rakelkar
Copy link
Contributor Author

@madhanrm see TODO in PR description. CNI maintainers asked for integration tests like other plugins.

@madhanrm
Copy link

@rakelkar Yes, that has to be done in a separate PR. This PR is already huge.

@rakelkar
Copy link
Contributor Author

rakelkar commented Dec 21, 2017 via email

@madhanrm
Copy link

madhanrm commented Jan 2, 2018

Vendor changes are required to get the current windows Appveyor test running and passing. We should be adding the integration test separately, once we have the dependencies merged.

@madhanrm
Copy link

madhanrm commented Jan 2, 2018

ping @rosenhouse , @squeed

@rosenhouse
Copy link
Contributor

Sorry for the delay, most of us were/still-are on holiday.

Please squash and follow guidelines for commit message.

@PatrickLang
Copy link

@madhanrm - can you fix the travis break?

Is this just waiting on review or are there other issues blocking this from moving forward?

thxCode added a commit to thxCode/containernetworking-plugins that referenced this pull request Aug 23, 2018
thxCode added a commit to thxCode/containernetworking-plugins that referenced this pull request Aug 23, 2018
thxCode added a commit to thxCode/containernetworking-plugins that referenced this pull request Aug 23, 2018
thxCode added a commit to thxCode/containernetworking-plugins that referenced this pull request Aug 23, 2018
Patch for containernetworking#85

+ Windows cni plugins are added
   (*) hostgw (l2bridge)
   (*) vxlan (overlay)
+ Windows netconf unit test
+ Fix appveyor config to run the test
+ Build Release support for windows plugins
thxCode added a commit to thxCode/containernetworking-plugins that referenced this pull request Aug 23, 2018
@benmoss
Copy link

benmoss commented Aug 28, 2018

@thxCode opened a PR with fixes here: #193

rosenhouse pushed a commit to rosenhouse/plugins that referenced this pull request Aug 29, 2018
rosenhouse pushed a commit to rosenhouse/plugins that referenced this pull request Aug 29, 2018
rosenhouse pushed a commit to rosenhouse/plugins that referenced this pull request Aug 29, 2018
rosenhouse pushed a commit to rosenhouse/plugins that referenced this pull request Aug 29, 2018
+ Windows cni plugins are added
   (*) hostgw (l2bridge)
   (*) vxlan (overlay)
+ Windows netconf unit test
+ Fix appveyor config to run the test
+ Build Release support for windows plugins

Based on containernetworking#85

Co-authored-by: rakelkar <[email protected]>
Co-authored-by: Madhan Raj Mookkandy <[email protected]>
rosenhouse pushed a commit to rosenhouse/plugins that referenced this pull request Aug 29, 2018
thxCode added a commit to thxCode/containernetworking-plugins that referenced this pull request Aug 30, 2018
thxCode added a commit to thxCode/containernetworking-plugins that referenced this pull request Aug 30, 2018
thxCode added a commit to thxCode/containernetworking-plugins that referenced this pull request Aug 30, 2018
@daschott
Copy link
Contributor

daschott commented Sep 4, 2018

@rosenhouse @thxCode thank you for the insight and help for this PR. Are there any outstanding issues or actions needed to merge this initial PR in?

thxCode added a commit to thxCode/containernetworking-plugins that referenced this pull request Sep 20, 2018
Patch for containernetworking#85

+ Windows cni plugins are added
   (*) win-bridge (hostgw)
   (*) win-overlay (vxlan)
+ Windows netconf unit test
+ Fix appveyor config to run the test
+ Build release support for windows plugins

Address comments

From:
    - containernetworking#85
    - rakelkar@0049c64
thxCode added a commit to thxCode/containernetworking-plugins that referenced this pull request Sep 20, 2018
Patch for containernetworking#85

+ Windows cni plugins are added
   (*) win-bridge (hostgw)
   (*) win-overlay (vxlan)
+ Windows netconf unit test
+ Fix appveyor config to run the test
+ Build release support for windows plugins

Address comments

From:
    - containernetworking#85
    - rakelkar@0049c64
thxCode added a commit to thxCode/containernetworking-plugins that referenced this pull request Sep 20, 2018
Patch for containernetworking#85

+ Windows cni plugins are added
   (*) win-bridge (hostgw)
   (*) win-overlay (vxlan)
+ Windows netconf unit test
+ Fix appveyor config to run the test
+ Build release support for windows plugins

Address comments

From:
    - containernetworking#85
    - rakelkar@0049c64
thxCode added a commit to thxCode/containernetworking-plugins that referenced this pull request Sep 20, 2018
Patch for containernetworking#85

+ Windows cni plugins are added
   (*) win-bridge (hostgw)
   (*) win-overlay (vxlan)
+ Windows netconf unit test
+ Fix appveyor config to run the test
+ Build release support for windows plugins

Address comments

From:
    - containernetworking#85
    - rakelkar@0049c64
thxCode added a commit to thxCode/containernetworking-plugins that referenced this pull request Sep 20, 2018
Patch for containernetworking#85

+ Windows cni plugins are added
   (*) win-bridge (hostgw)
   (*) win-overlay (vxlan)
+ Windows netconf unit test
+ Fix appveyor config to run the test
+ Build release support for windows plugins

Address comments

From:
    - containernetworking#85
    - rakelkar@0049c64
thxCode added a commit to thxCode/containernetworking-plugins that referenced this pull request Sep 20, 2018
Patch for containernetworking#85

+ Windows cni plugins are added
   (*) win-bridge (hostgw)
   (*) win-overlay (vxlan)
+ Windows netconf unit test
+ Fix appveyor config to run the test
+ Build release support for windows plugins

Address comments

From:
    - containernetworking#85
    - rakelkar@0049c64
thxCode added a commit to thxCode/containernetworking-plugins that referenced this pull request Sep 20, 2018
Patch for containernetworking#85

+ Windows cni plugins are added
   (*) win-bridge (hostgw)
   (*) win-overlay (vxlan)
+ Windows netconf unit test
+ Fix appveyor config to run the test
+ Build release support for windows plugins

Address comments

From:
    - containernetworking#85
    - rakelkar@0049c64
thxCode added a commit to thxCode/containernetworking-plugins that referenced this pull request Sep 20, 2018
Patch for containernetworking#85

+ Windows cni plugins are added
   (*) win-bridge (hostgw)
   (*) win-overlay (vxlan)
+ Windows netconf unit test
+ Fix appveyor config to run the test
+ Build release support for windows plugins

Address comments

From:
    - containernetworking#85
    - rakelkar@0049c64
thxCode added a commit to thxCode/containernetworking-plugins that referenced this pull request Sep 20, 2018
Patch for containernetworking#85

+ Windows cni plugins are added
   (*) win-bridge (hostgw)
   (*) win-overlay (vxlan)
+ Windows netconf unit test
+ Fix appveyor config to run the test
+ Build release support for windows plugins

Address comments

From:
    - containernetworking#85
    - rakelkar@0049c64
@dcbw
Copy link
Member

dcbw commented Sep 20, 2018

Merged #193 as a continuation of this one.

@dcbw dcbw closed this Sep 20, 2018
plwhite pushed a commit to plwhite/plugins that referenced this pull request Oct 10, 2018
Patch for containernetworking#85

+ Windows cni plugins are added
   (*) win-bridge (hostgw)
   (*) win-overlay (vxlan)
+ Windows netconf unit test
+ Fix appveyor config to run the test
+ Build release support for windows plugins

Address comments

From:
    - containernetworking#85
    - rakelkar@0049c64
SchSeba pushed a commit to SchSeba/plugins that referenced this pull request Nov 18, 2018
Patch for containernetworking#85

+ Windows cni plugins are added
   (*) win-bridge (hostgw)
   (*) win-overlay (vxlan)
+ Windows netconf unit test
+ Fix appveyor config to run the test
+ Build release support for windows plugins

Address comments

From:
    - containernetworking#85
    - rakelkar@0049c64
apreuavar6 added a commit to apreuavar6/cni-plugins that referenced this pull request Aug 11, 2024
Patch for containernetworking/plugins#85

+ Windows cni plugins are added
   (*) win-bridge (hostgw)
   (*) win-overlay (vxlan)
+ Windows netconf unit test
+ Fix appveyor config to run the test
+ Build release support for windows plugins

Address comments

From:
    - containernetworking/plugins#85
    - rakelkar/plugins@0049c64
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.