[WIP] support for Hostport #335
Conversation
| nrc.cniConfFile = "/etc/cni/net.d/10-kuberouter.conf" | ||
| } | ||
| if _, err := os.Stat(nrc.cniConfFile); os.IsNotExist(err) { | ||
| return nil, errors.New("Invalid CNI conf file " + nrc.cniConfFile) |
There was a problem hiding this comment.
s/Invalid CNI conf/CNI conf file does not exist - the current error sounds like the file exists but it is invalid
| { | ||
| "type":"portmap", | ||
| "capabilities":{ | ||
| "snat":true, |
There was a problem hiding this comment.
is snat a requirement to use this?
There was a problem hiding this comment.
I think so. Though its not clear from https://github.com/containernetworking/plugins/tree/master/plugins/meta/portmap
There was a problem hiding this comment.
Seems like it sets up SNAT chain, might be good to double check if it conflicts with any SNAT chains kube-router creates
There was a problem hiding this comment.
@andrewsykim ok I checked, there is no conflict with any other rules.
Chain PREROUTING (policy ACCEPT 0 packets, 0 bytes)
pkts bytes target prot opt in out source destination
7 468 CNI-HOSTPORT-DNAT all -- * * 0.0.0.0/0 0.0.0.0/0 ADDRTYPE match dst-type LOCAL
Chain POSTROUTING (policy ACCEPT 24 packets, 1440 bytes)
pkts bytes target prot opt in out source destination
0 0 CNI-HOSTPORT-SNAT all -- * * 127.0.0.1 !127.0.0.1
Chain CNI-DN-1d1e1acf7cbbe4ef6121a (1 references)
pkts bytes target prot opt in out source destination
1 60 DNAT tcp -- * * 0.0.0.0/0 0.0.0.0/0 tcp dpt:8086 to:10.1.1.32:80
Chain CNI-HOSTPORT-DNAT (2 references)
pkts bytes target prot opt in out source destination
135 8148 CNI-DN-1d1e1acf7cbbe4ef6121a all -- * * 0.0.0.0/0 0.0.0.0/0 /* dnat name: "mynet" id: "2fd850ee20c7105656a5b5ccd4669c04f5a9ed8035f2f35c41a5aa7e6cf4e81f" */
Chain CNI-HOSTPORT-SNAT (1 references)
pkts bytes target prot opt in out source destination
0 0 CNI-SN-1d1e1acf7cbbe4ef6121a all -- * * 0.0.0.0/0 0.0.0.0/0 /* snat name: "mynet" id: "2fd850ee20c7105656a5b5ccd4669c04f5a9ed8035f2f35c41a5aa7e6cf4e81f" */
Chain CNI-SN-1d1e1acf7cbbe4ef6121a (1 references)
pkts bytes target prot opt in out source destination
0 0 MASQUERADE tcp -- * * 127.0.0.1 10.1.1.32 tcp dpt:80
| data: | ||
| cni-conf.json: | | ||
| { | ||
| "cniVersion":"0.3.0", |
There was a problem hiding this comment.
Isn't 0.6.0 the most recent stable version? Why 0.3.0?
There was a problem hiding this comment.
This is CNI spec version https://github.com/containernetworking/cni/blob/master/SPEC.md Plug-in's vesioning is different. I got to know about it for this PR :)
utils/pod_cidr.go
Outdated
| return fmt.Errorf("Failed to parse JSON from CNI conf file: %s", err.Error()) | ||
| } | ||
| config["ipam"].(map[string]interface{})["subnet"] = cidr | ||
| configJSON, _ := json.Marshal(config) |
There was a problem hiding this comment.
Seems like these three lines can be added after the if/else, instead of duplicating it
configJSON, _ := json.Marshal(config)
err = ioutil.WriteFile(cniConfFilePath, configJSON, 0644)
if err != nil {
return fmt.Errorf("Failed to insert subnet cidr into CNI conf file: %s", err.Error())
}
utils/pod_cidr.go
Outdated
| } | ||
| if strings.HasSuffix(cniConfFilePath, ".conflist") { | ||
| var config interface{} | ||
| err = json.Unmarshal(file, &config) |
There was a problem hiding this comment.
Wondering if it would help if we unmarshal this into libcni.NetworkConfig.
There was a problem hiding this comment.
NetConf structure is little wierd.
https://github.com/containernetworking/cni/blob/master/pkg/types/types.go#L70-L72
It does not have the CIDR, did not help much with unmarshalling.
- remove the hardcode cni conf file path - move it to env variable - keep it backward compatible even if env variable is not present - support both .conf and .conflist - daemonset with hostport support Fixes: cloudnativelabs#168
| } | ||
| var config interface{} | ||
| if strings.HasSuffix(cniConfFilePath, ".conflist") { | ||
| err = json.Unmarshal(file, &config) |
There was a problem hiding this comment.
Same with Unmarshal, seems like we can move it before the if/else, instead of duplicating it
|
lgtm, I think it would be nice to add test cases for this in here https://github.com/murali-reddy/kube-router/blob/master/utils/pod_cidr_test.go#L61 |
|
A couple questions/suggestions:
|
|
Yes, documentation change would be required. I will update the PR once i have working setup and be able to test portmap functionality. Right now i am struggling to get a setup where I can test. No changes are needed in Dockerfile. |
This PR addes support for hostport.
/etc/cni/net.d/10-kuberouter.conf.conflistis added so you can specify multiple plug-ins-- env variable set to
/etc/cni/net.d/10-kuberouter.conflist-- configmap with cni conf json with portmap plugin config
-- init container changes to add
.conflistfileImage
cloudnativelabs/kube-router-git:hostporthas the changes in this PR.