CNI networking installation support #621

chrislovecnm · 2016-10-09T04:20:46Z

Work on getting cni networks such a weave functioning. I did encounter this probably kubernetes/kubernetes#20379

Oct  9 04:09:44 ip-172-20-111-165 kubelet[999]: E1009 04:09:44.019978     999 kubelet.go:2095] Failed killing the pod "cassandra-5": failed to "TeardownNetwork" for "cassandra-5_default" with TeardownNetworkError: "Failed to teardown network for pod \"b4f4f534-8dd5-11e6-a44d-02c1f7bba931\" using network plugins \"cni\": no such file or directory"

This is now tested an ready for master. I will be testing an install above the 50 node limit to fully vet.

krisnova · 2016-10-09T04:29:33Z

upup/pkg/api/networking.go

@@ -16,7 +17,12 @@ type KubenetNetworkingSpec struct {
 }

 // ExternalNetworkingSpec is the specification for networking that is implemented by a Daemonset
+// It also uses kubenet
+type ExternalNetworkingSpec struct {


Just curious - what are we using the empty struct for?

chrislovecnm · 2016-10-10T03:39:28Z

Yah noticed that too, just modeled the original code. @justinsb?

krisnova · 2016-10-10T03:41:25Z

upup/models/config/components/kubelet/_networking_cni/_k8s_1_4/kubelet.cni.1_4.options

+  # AWS MTU is 9001
+  NetworkPluginMTU: 9001
+
+# TODO: Having to duplicate MasterKubelet & Kubelet feels wrong


What feels wrong here? Generally vagrant TODO's make me at least want to raise a question :)

Comments ported over from the files I stole from kubenet. I can remove.

krisnova · 2016-10-10T03:44:53Z

upup/pkg/fi/cloudup/tagbuilder.go

@@ -23,6 +23,9 @@ func buildCloudupTags(cluster *api.Cluster) (map[string]struct{}, error) {
 		// external is based on kubenet
 		tags["_networking_kubenet"] = struct{}{}
 		tags["_networking_external"] = struct{}{}
+	} else if networking.CNI != nil {
+		// external is based on cni, weave, flannel, etc
+		tags["_networking_cni"] = struct{}{}


More empty structs? Are these just place holders for future members or something?

Almost answered in French. Again just modeling code that was done before. We can cleanup if you care to recommend a pattern.

This is the "best way" to do sets in golang

chrislovecnm · 2016-10-10T22:20:50Z

should solve #537 as well

chrislovecnm · 2016-10-10T22:21:19Z

Should solve #315 as well

WillPlatnick · 2016-10-11T22:40:53Z

I am so excited for this. Thanks @chrislovecnm

chrislovecnm · 2016-10-12T20:44:28Z

@WillPlatnick looking good, just needing more testing.

kzwang · 2016-10-12T22:39:34Z

upup/models/config/components/kubelet/_networking_cni/_k8s_1_5/kubelet.cni.1_5.options

@@ -0,0 +1,7 @@
+Kubelet:
+  # AWS MTU is 9001
+  NetworkPluginMTU: 9001


Hi @chrislovecnm , I got a question about this MTU. According to Weave document, the MTU should be 8950 as there is an overhead of 50 bytes for Vxlan, will this be handled automatically?

Another question is I can see kubenet is using the MTU passed to the plugin (https://github.com/kubernetes/kubernetes/blob/master/pkg/kubelet/network/kubenet/kubenet_linux.go#L128), however, for cni it doesn't seem it's using the MTU value (https://github.com/kubernetes/kubernetes/blob/master/pkg/kubelet/network/cni/cni.go#L144). Is this been handled somewhere else?

I am going to remove it. Thanks

offlinehacker · 2016-10-14T14:34:38Z

We are testing on new cluster, and it seems to work without problems so far, i also did some basic policy testing which also seems to work.

krisnova

LGTM

chrislovecnm · 2016-10-15T15:41:56Z

I need more testing and squash my commits prettier :)

chrislovecnm · 2016-10-18T21:22:27Z

@JuJu227 if you could take a look, specifically at the documentation, would be much appreciated.

@kris-nova & @justinsb the commit are squashed by me already.

JuJu227 · 2016-10-19T11:38:48Z

upup/pkg/fi/cloudup/populatecluster_test.go

+			{Name: "us-west-2a", CIDR: "172.20.2.0/27"},
+			{Name: "us-west-2b", CIDR: "172.20.2.32/27"},
+			{Name: "us-west-2c", CIDR: "172.20.2.64/27"},
+		}*/


Suggest removing dead code.

krisnova · 2016-10-19T12:58:55Z

@chrislovecnm - this looks good.. From a code/documentation standpoint this all makes sense. Will have to defer to you and @justinsb for functionality success.

cleaning up TODOs updated with a unit test

removing MTU options that we do not need working on getting file structure up

more test cleanup

justinsb · 2016-10-20T02:19:41Z

upup/pkg/fi/cloudup/tagbuilder_test.go

+	UpdatePolicy      string
+}
+
+func buildCluster(clusterArgs interface{}) *api.Cluster {


Not sure why this isn't just clusterArgs *ClusterParams ?

justinsb · 2016-10-20T04:05:54Z

LGTM... found a few problems:

We hit Daemonsets silently fail if resources are insufficent kubernetes#31369 when using a machine with a single core - m4.large fixes it.
Have to SSH to the master to install the weave DS
When a node first comes up it isn't really ready; this self heals though:

  1m    1m      1       {kubelet ip-172-20-68-75.ec2.internal}          Warning FailedSync      Error syncing pod, skipping: failed to "SetupNetwork" for "guestbook-03xsn_default" with SetupNetworkError: "Failed to setup network for pod \"guestbook-03xsn_default(4c4a9f7b-9676-11e6-b502-061a7a80de7a)\" using network plugins \"cni\": unable to allocate IP address: Post http://127.0.0.1:6784/ip/416e5685e89795642fa79a0959dfbf687aa452fdba527c0ff11ad8dd1519c652: dial tcp 127.0.0.1:6784: getsockopt: connection refused; Skipping pod"

If we can change the pod IP range away from 10.0.0.0/8 that will avoid conflicts with EC2 classic. We default to 100.64.x.x, if we can honor that it's probably best
At 125 nodes, started seeing this error:

E1020 03:58:52.832357       5 endpoints_controller.go:514] Endpoints "guestbook" is invalid: [subsets[0].addresses[52].nodeName: Forbidden: Cannot change NodeName for 10.38.0.1 to ip-172-20-92-169.ec2.internal, subsets[0].addresses[97].nodeName: Forbidden: Cannot change NodeName for 10.42.128.1 to ip-172-20-91-115.ec2.internal]

justinsb · 2016-10-20T04:06:08Z

Merging - we can address those issues post merge!

justinsb · 2016-10-27T16:05:34Z

@chrislovecnm do we have issues to track the the 5 snags above?

googlebot added the cla: yes label Oct 9, 2016

This was referenced Oct 9, 2016

Error messages when pod is deleted weaveworks-experiments/weave-kube#32

Closed

networking set as external launches kubelet with kubenet option #620

Closed

krisnova reviewed Oct 10, 2016

View reviewed changes

chrislovecnm mentioned this pull request Oct 10, 2016

Easy way to configure CNI and/or weave #537

Closed

chrislovecnm force-pushed the cni-networks branch 2 times, most recently from 13a023c to 29dafb6 Compare October 11, 2016 20:44

chrislovecnm force-pushed the cni-networks branch from 29dafb6 to bef000e Compare October 12, 2016 20:42

chrislovecnm assigned justinsb Oct 12, 2016

kzwang reviewed Oct 12, 2016

View reviewed changes

chrislovecnm force-pushed the cni-networks branch 2 times, most recently from 6b00f9b to 03ac69e Compare October 14, 2016 18:05

krisnova approved these changes Oct 15, 2016

View reviewed changes

Buzer mentioned this pull request Oct 15, 2016

kubectl not using all of the yaml documents in some cases kubernetes/kubernetes#34886

Closed

This was referenced Oct 16, 2016

Set roadmap for 1.5 and 1.6 releases #663

Closed

Support virtual networking addon #247

Closed

chrislovecnm force-pushed the cni-networks branch 2 times, most recently from 19ed4c8 to c24836a Compare October 18, 2016 21:15

chrislovecnm changed the title ~~[WIP] CNI networks~~ CNI networking installation support Oct 18, 2016

JuJu227 reviewed Oct 19, 2016

View reviewed changes

chrislovecnm added 6 commits October 19, 2016 09:20

adding model and options to handle cni networking

6e3bece

set network type

54f72be

cleaning up TODOs updated with a unit test

updating unit test to work with mock data

6171f07

removing MTU options that we do not need working on getting file structure up

Adding correct tag files for cni

1e5e180

go fmt and unit test fixed

c85c5fd

more test cleanup

Documenation for the new stuff and cleaning up test

a914626

chrislovecnm force-pushed the cni-networks branch from 7e77a27 to a914626 Compare October 19, 2016 15:20

justinsb reviewed Oct 20, 2016

View reviewed changes

justinsb merged commit 8255dbf into kubernetes:master Oct 20, 2016

chrislovecnm mentioned this pull request Oct 21, 2016

CNI Networking at scale #711

Closed

bboreham mentioned this pull request Oct 31, 2016

weave-kube race at install weaveworks/weave#2570

Closed

razic mentioned this pull request Nov 6, 2016

WIP: Support Weave #819

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

CNI networking installation support #621

CNI networking installation support #621

chrislovecnm commented Oct 9, 2016 •

edited

Loading

krisnova Oct 9, 2016

chrislovecnm commented Oct 10, 2016

krisnova Oct 10, 2016

chrislovecnm Oct 10, 2016

krisnova Oct 10, 2016

chrislovecnm Oct 10, 2016 •

edited

Loading

justinsb Oct 20, 2016

chrislovecnm commented Oct 10, 2016

chrislovecnm commented Oct 10, 2016

WillPlatnick commented Oct 11, 2016

chrislovecnm commented Oct 12, 2016

kzwang Oct 12, 2016

chrislovecnm Oct 13, 2016

offlinehacker commented Oct 14, 2016

krisnova left a comment

chrislovecnm commented Oct 15, 2016

chrislovecnm commented Oct 18, 2016 •

edited

Loading

JuJu227 Oct 19, 2016

chrislovecnm Oct 19, 2016

krisnova commented Oct 19, 2016

justinsb Oct 20, 2016

justinsb commented Oct 20, 2016

justinsb commented Oct 20, 2016

justinsb commented Oct 27, 2016

CNI networking installation support #621

CNI networking installation support #621

Conversation

chrislovecnm commented Oct 9, 2016 • edited Loading

Choose a reason for hiding this comment

chrislovecnm commented Oct 10, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

chrislovecnm Oct 10, 2016 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

chrislovecnm commented Oct 10, 2016

chrislovecnm commented Oct 10, 2016

WillPlatnick commented Oct 11, 2016

chrislovecnm commented Oct 12, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

offlinehacker commented Oct 14, 2016

krisnova left a comment

Choose a reason for hiding this comment

chrislovecnm commented Oct 15, 2016

chrislovecnm commented Oct 18, 2016 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

krisnova commented Oct 19, 2016

Choose a reason for hiding this comment

justinsb commented Oct 20, 2016

justinsb commented Oct 20, 2016

justinsb commented Oct 27, 2016

chrislovecnm commented Oct 9, 2016 •

edited

Loading

chrislovecnm Oct 10, 2016 •

edited

Loading

chrislovecnm commented Oct 18, 2016 •

edited

Loading