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

Workaround - cannot parse clusterNetworkPrefix and endpointMacPrefix #5

Open
wants to merge 20 commits into
base: windowsCni
Choose a base branch
from

Conversation

ptylenda
Copy link

@ptylenda ptylenda commented Mar 4, 2018

NOTE: This pull request is not a fix itself, as I am not a Go programmer. So it is only a workaround, but it points out an issue that could be fixed in a go-ish way.

I have noticed that in case of l2bridge plugin clusterNetworkPrefix does not get unmarshalled and endpoint hns policies have "" when using IPMasq. Same goes for endpointMacPrefix.

This behaviour is caused by lower-/upper-case and json unmarshalling https://stackoverflow.com/questions/28228393/json-unmarshal-returning-blank-structure. After simple change of the field names to upper case, there are problems with unmarshalling raw string to net.IPNet, so I have changed this to a raw string as a workaround.

rakelkar and others added 16 commits February 3, 2018 23:14
Support delegating to Windows CNI - use a separate WindowsDelegate field as the format of Windows properties is different from the schema supported by Linux under the delegate field. This will users to configure both Linux and Windows together and also to use Windows specific configuration for Windows clusters.

flannel: Remove WindowsDelegate in favor of OS specific config files
Resolves PR review comments by:
removing the WindowsDelegate attribute,
fix coditions to avoid tabs
remove duplicate checks
remove redundant map pointer
remove redundant ip fns

flannel: Format windows test file
flannel: move Windows tests to Ginkgo
Add logic to support vxlan on Windows (rakelkar#2)
Flannel: Add support for Windows Overlay (vxlan) mode
Windows Overlay mode requires the network to be set to the cluster CIDR.
So for overlay setup host-local as IPAM with the pod CIDR as the ip range

First cut bridge plugin
add overlay strawman and share dup code
Hns helper functions
windows cni: add tests and update flannel CNI to use windows CNI
flannel: revert test that moved to windows cni
flannel: remove unused import
flannel: update readme for windows
win-overlay: update readme to fix minor copy paste errors
Build/Release support for windows plugins
Fixed review comments

Fixed Windows netconf test
Updated appveyor to deploy the pre-requisite
Fix build.sh to build windows only for amd64
Vendor commit for Windows support
…akelkar#3)

overlay: 
Add: details to error messages in overlay_windows.go
Fix: building json request to hns endpoints (Policies was malformed)
Fix: updated tests to cover netconf policy marshalling fix
* fix no valid addresses caused by lack of  ip's version
* fix nit
* fmt err
Support delegating to Windows CNI - use a separate WindowsDelegate field as the format of Windows properties is different from the schema supported by Linux under the delegate field. This will users to configure both Linux and Windows together and also to use Windows specific configuration for Windows clusters.

flannel: Remove WindowsDelegate in favor of OS specific config files
Resolves PR review comments by:
removing the WindowsDelegate attribute,
fix coditions to avoid tabs
remove duplicate checks
remove redundant map pointer
remove redundant ip fns

flannel: Format windows test file
flannel: move Windows tests to Ginkgo
Add logic to support vxlan on Windows (rakelkar#2)
Flannel: Add support for Windows Overlay (vxlan) mode
Windows Overlay mode requires the network to be set to the cluster CIDR.
So for overlay setup host-local as IPAM with the pod CIDR as the ip range

First cut bridge plugin
add overlay strawman and share dup code
Hns helper functions
windows cni: add tests and update flannel CNI to use windows CNI
flannel: revert test that moved to windows cni
flannel: remove unused import
flannel: update readme for windows
win-overlay: update readme to fix minor copy paste errors
Build/Release support for windows plugins
Fixed review comments

Fixed Windows netconf test
Updated appveyor to deploy the pre-requisite
Fix build.sh to build windows only for amd64

Update vendor golang.org/x/sys

Vendor commit for Windows support
…akelkar#3)

overlay: 
Add: details to error messages in overlay_windows.go
Fix: building json request to hns endpoints (Policies was malformed)
Fix: updated tests to cover netconf policy marshalling fix
* fix no valid addresses caused by lack of  ip's version
* fix nit
* fmt err
Fixed the Comparison to be Case insensitive for Network Type

Fix build error
DNSServerList: strings.Join(result.DNS.Nameservers, ","),
DNSSuffix: result.DNS.Domain,
DNSServerList: strings.Join(n.DNS.Nameservers, ","),
DNSSuffix: n.DNS.Domain,
Copy link
Author

Choose a reason for hiding this comment

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

@rakelkar , I am not sure if this is the proper way, but result variable is always empty when returned from IPAM plugin. Here are how r and result look like:

2018/03/05 01:54:01 Data: &types020.Result{CNIVersion:"0.2.0", IP4:(*types020.IPConfig)(0xc0421a22a0), IP6:(*types020.IPConfig)(nil), DNS:types.DNS{Nameservers:[]string(nil), Domain:"", Search:[]string(nil), Options:[]string(nil)}}
2018/03/05 01:54:01 Data: &current.Result{CNIVersion:"0.3.1", Interfaces:[]*current.Interface(nil), IPs:[]*current.IPConfig{(*current.IPConfig)(0xc0421a23c0)}, Routes:[]*types.Route{}, DNS:types.DNS{Nameservers:[]string(nil), Domain:"", Search:[]string(nil), Options:[]string(nil)}}

after passing

{"IPMasq":true,"clusterNetworkPrefix":"10.200.0.0/16","cniVersion":"0.2.0","dns":{"Domain":"default.svc.cluster.local","Nameservers":["10.201.0.10"]},"ipam":{"subnet":"10.200.9.0/24","type":"host-local"},"name":"cbr0","type":"win-l2bridge"}

But netConfig looks okay and using n.DNS.Nameservers and n.DNS.Domain gives proper result, the container interface is configured with DNS as expected:

Ethernet adapter vEthernet (d54bd7bb5e200d550d7b0ba1ac70db5340b0806a945790279e86989ee4819571_cbr0):

   Connection-specific DNS Suffix  . : default.svc.cluster.local
   Description . . . . . . . . . . . : Hyper-V Virtual Ethernet Adapter #5
   Physical Address. . . . . . . . . : 00-15-5D-D5-0A-C1
   DHCP Enabled. . . . . . . . . . . : No
   Autoconfiguration Enabled . . . . : Yes
   Link-local IPv6 Address . . . . . : fe80::418a:ae59:763b:50cc%29(Preferred)
   IPv4 Address. . . . . . . . . . . : 10.200.9.72(Preferred)
   Subnet Mask . . . . . . . . . . . : 255.255.255.0
   Default Gateway . . . . . . . . . : 10.200.9.2
   DNS Servers . . . . . . . . . . . : 10.201.0.10
   NetBIOS over Tcpip. . . . . . . . : Disabled

Copy link
Author

Choose a reason for hiding this comment

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

Well it is almost surely something wrong with host-local IPAM, using n.DNS is just a workaround then

@@ -100,14 +100,22 @@ func cmdAdd(args *skel.CmdArgs) error {

// NAT based on the the configured cluster network
if n.IPMasq {
n.ApplyOutboundNatPolicy(n.clusterNetworkPrefix.String())
n.ApplyOutboundNatPolicy(n.ClusterNetworkPrefix)
cmd := exec.Command("powershell",
Copy link
Author

Choose a reason for hiding this comment

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

And this is how creating a new NetNat can be worked around and making NAT fully functional with kubernetes on windows

Copy link

Choose a reason for hiding this comment

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

@ptylenda

Hi, I manually build host-local.exe and l2bridge from this pr, these binaries will auto create NetNat rule for me. The pod on windows can reach outside address but the pod-to pod communication is broken when these pods locate on different windows node. After I manually delete these nat rule, the pod-to-pod communication is recovered but the pod can not reach outside addresses.

@madhanrm madhanrm force-pushed the windowsCni branch 3 times, most recently from 9484738 to 5f6aabf Compare March 6, 2018 18:38
@ptylenda
Copy link
Author

ptylenda commented Mar 7, 2018

@Calory, true, as we talked on Slack, this creates problems with intra pod communication, looks like outbound NAT exceptions are not working properly :( so this pull request does not make much sense for now, I will leave it only for "demo" purposes and other changes in json unmarshalling.

BTW I will be off for a few weeks, we will see what happens when I am back, maybe everything gets fixed ;)

@madhanrm madhanrm force-pushed the windowsCni branch 3 times, most recently from ebc2540 to d9e340a Compare June 10, 2018 01:47
@madhanrm madhanrm force-pushed the windowsCni branch 2 times, most recently from ed18059 to d0f397c Compare June 11, 2018 23:22
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.

5 participants