Update Calico add-on #38169

caseydavenport · 2016-12-06T04:32:44Z

What this PR does / why we need it:

Updates Calico to the latest version using self-hosted install as a DaemonSet, removes Calico's dependency on etcd.

Remove last bits of Calico salt
Failing on the master since no kube-proxy to access API.
Fix outgoing NAT
Tweak to work on both debian / GCI (not just GCI)
Add the portmap plugin for host port support

Maybe:

Add integration test

Which issue this PR fixes:

#32625

Try it out

Clone the PR, then:

make quick-release
export NETWORK_POLICY_PROVIDER=calico
export NODE_OS_DISTRIBUTION=gci
export MASTER_SIZE=n1-standard-4
./cluster/kube-up.sh

Release note:

The Calico version included in kube-up for GCE has been updated to v2.2.

k8s-reviewable · 2016-12-06T04:32:51Z

This change is

caseydavenport · 2016-12-06T07:18:12Z

Quoting #32625 (comment)

To avoid this in the future, I'd recommend adding a separate test suite that tests calico integration.

@vishh would you mind pointing me to the right place to add this?

eparis · 2016-12-06T14:14:17Z

@kubernetes/sig-network

thockin · 2016-12-08T05:03:53Z

nice!

caseydavenport · 2016-12-15T07:29:02Z

Looks like the PR that removed the legacy "configure-cbr0" mode also removed some handy outgoing NAT behavior that this PR was depending on (or rather it's now kubenet only).

Calico can do its own NAT (and usually doesn't rely on the above), but I haven't added support for that feature yet to the "etcdless" mode used in this PR, so that's the next step to move this forward.

roberthbailey · 2017-01-31T06:57:10Z

Is this still WIP?

caseydavenport · 2017-01-31T07:06:15Z

@roberthbailey Yes, sorry, I forgot about this guy. Just a few tweaks I need to make - will do it this week.

caseydavenport · 2017-02-08T02:32:05Z

It's still a bit icky.

I can't mount /opt/cni/bin (the location for CNI binaries on debian, etc) when running on GCI, which means that I can only make the manifest work on one or the other.

How do we feel about making GCI and debian both look in /home/kubernetes/bin for CNI plugins so that at least they're consistent?

freehan · 2017-02-10T18:39:35Z

How do we feel about making GCI and debian both look in /home/kubernetes/bin for CNI plugins so that at least they're consistent?

Probably better to put it in /home/kubernetes/bin/cni. There are a bunch of stuff in the directory already.

caseydavenport · 2017-02-23T02:00:23Z

@freehan I've left it as /home/kubernetes/bin for the moment, since that is what the default GCI scripts configure right now and I'm worried about breaking other CNI providers that might expect that location. https://github.com/kubernetes/kubernetes/blob/master/cluster/gce/gci/configure-helper.sh#L637

Do you think it's OK to change?

caseydavenport · 2017-02-23T16:57:21Z

This PR currently wants this: #41943

caseydavenport · 2017-02-23T16:57:30Z

@k8s-bot gce etcd3 e2e test this

freehan · 2017-02-24T19:42:55Z

/lgtm

roberthbailey · 2017-02-25T06:25:41Z

cluster/addons/calico-policy-controller/calico-node.yaml

+        kubernetes.io/cluster-service: "true"
+        k8s-app: calico-node
+      annotations:
+        scheduler.alpha.kubernetes.io/critical-pod: ''


If this runs in a cluster w/o alpha features enabled, do these annotations get ignored?

roberthbailey · 2017-02-25T06:26:18Z

cluster/addons/calico-policy-controller/calico-node.yaml

+            - name: CALICO_NETWORKING_BACKEND
+              value: "none"
+            - name: CALICO_IPV4POOL_CIDR
+              value: "10.244.0.0/16"


What does this CIDR range do? Should it be configurable?

This CIDR range is used by Calico to perform outgoing NAT for Pods to access the internet, etc and is required as of k8s v1.5 since the kubelet stopped doing NAT for CNI plugins.

It probably should be configurable if the cluster CIDR is configurable (they should have the same value). I think that would mean making this file into a template.

roberthbailey · 2017-04-18T14:49:08Z

@caseydavenport -- are you still working on this PR? The labels indicate that you need to set a release note before it will merge.

caseydavenport · 2017-04-30T01:09:42Z

@roberthbailey sorry for the delay in response, have been out of the office for a few weeks.

I've already got a release note in the description, but I'd like to rebase and retest this before merging.

dnardo · 2017-05-08T15:58:33Z

@caseydavenport Any chance we can get this in this week?

caseydavenport · 2017-05-08T23:31:58Z

@dnardo just spun another cluster up today with this and noticed that something has changed in the last rebase that is causing the master components to fail.

I need to investigate the cause of that - will post back with more information. I'm really hoping to get this merged this week provided I can sort out this new issue.

caseydavenport · 2017-05-09T21:52:35Z

Interestingly enough, it looks like the problem I'm seeing is that the kubelet is trying to evict the controller manager upon installation of Calico on the node.

May 09 21:36:04 kubernetes-master kubelet[1587]: I0509 21:36:04.886477    1587 preemption.go:101] preemption: attempting to evict pods [&Pod{ObjectMeta:k8s_io_apimachinery_pkg_apis_meta_v1.ObjectMeta{Name:kube-controller-manager-kubernetes-master

Not sure yet what the right fix is - might just be to stop running Calico on the master somehow, or adjust resource requests, or.. something else?

caseydavenport · 2017-05-10T00:55:11Z

@dnardo Yeah, can confirm that this works just swell when I set MASTER_SIZE=n1-standard-4.

caseydavenport · 2017-05-12T22:42:28Z

@dnardo @roberthbailey I think this is ready to go in now.

dnardo · 2017-05-12T23:21:04Z

lgtm

dnardo · 2017-05-14T00:07:03Z

cluster/gce/config-default.sh

@@ -122,7 +122,7 @@ ENABLE_CLUSTER_MONITORING="${KUBE_ENABLE_CLUSTER_MONITORING:-influxdb}"
 # of fluentd running on a node, kubelet need to mark node on which
 # fluentd is not running as a manifest pod with appropriate label.
 # TODO(piosz): remove this in 1.8
-NODE_LABELS="${KUBE_NODE_LABELS:-beta.kubernetes.io/fluentd-ds-ready=true}"
+NODE_LABELS="${KUBE_NODE_LABELS:-beta.kubernetes.io/fluentd-ds-ready=true,beta.kubernetes.io/calico-ds-ready=true}"


I think we may want to make this conditional on NETWORK_POLICY_PROVIDER=calico ?

Why would this be beta.kubernetes.io/calico-ds-ready=true ? Shouldn't it be something like: beta.calicoproject.org/ds-ready=true ?

I guess my question comes down to 'who defined this as 'beta' and 'why does it belong in the kubernetes.io org?

@eparis I was mostly modeling this off the annotation used for the fluentd DaemonSet for consistency. The designation as beta has not had a formal agreement by anyone. projectcalico.org/ds-ready=true sounds fine to me.

@dnardo yes, I think you're right. Will update.

caseydavenport · 2017-05-15T16:58:12Z

@k8s-bot pull-kubernetes-federation-e2e-gce test this
@k8s-bot cvm gke e2e test this

caseydavenport · 2017-05-15T20:48:37Z

/release-note

caseydavenport · 2017-05-16T12:57:12Z

@dnardo @roberthbailey @eparis I think I've made all the markups - please take a look. Thanks!

roberthbailey · 2017-05-17T07:13:09Z

cluster/gce/container-linux/configure-helper.sh

+
+    # Replace the cluster cidr.
+    local -r calico_file="${dst_dir}/calico-policy-controller/calico-node.yaml"
+    sed -i -e "s@__CLUSTER_CIDR__@${CLUSTER_IP_RANGE:-'10.244.0.0/16'}@g" "${calico_file}"


I don't understand the default value here. It doesn't match the value in config-default.sh (https://github.com/kubernetes/kubernetes/blob/master/cluster/gce/config-default.sh#L89) and I'm wondering if this script should just fail if the CIDR isn't set rather than falling back to a value that will likely result in a non-functional cluster.

Yeah, that's wrong - it should just use the value in config-default.sh.

roberthbailey · 2017-05-17T07:14:08Z

cluster/addons/calico-policy-controller/calico-node.yaml

+        # container programs network policy and routes on each
+        # host.
+        - name: calico-node
+          image: calico/node:v1.2.0


I assume this pulls from dockerhub. Most (all?) of the other cluster addons pull from gcr.io, so it'd be nice to use that here as well for consistency.

roberthbailey · 2017-05-19T18:47:56Z

/lgtm
/approve

k8s-github-robot · 2017-05-19T18:48:15Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: caseydavenport, freehan, roberthbailey

Needs approval from an approver in each of these OWNERS Files:

~~cluster/OWNERS~~ [roberthbailey]

You can indicate your approval by writing /approve in a comment
You can cancel your approval by writing /approve cancel in a comment

k8s-ci-robot · 2017-05-20T02:15:04Z

@caseydavenport: The following test(s) failed:

Test name	Commit	Details	Rerun command
Jenkins GKE smoke e2e	0622a6292bf527b9c93a524a790ff19686929b85	link	`@k8s-bot cvm gke e2e test this`
pull-kubernetes-federation-e2e-gce	`63744a8`	link	`@k8s-bot pull-kubernetes-federation-e2e-gce test this`

Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

k8s-github-robot · 2017-05-20T02:38:59Z

Automatic merge from submit-queue

k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Dec 6, 2016

caseydavenport force-pushed the calico-daemonset branch from b24cc48 to 175fe62 Compare December 6, 2016 04:33

k8s-github-robot assigned eparis Dec 6, 2016

k8s-github-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. release-note-label-needed labels Dec 6, 2016

caseydavenport force-pushed the calico-daemonset branch 2 times, most recently from dbc960a to e2bf14f Compare December 15, 2016 02:09

k8s-github-robot assigned roberthbailey and unassigned eparis Jan 30, 2017

apelisse assigned eparis Jan 31, 2017

caseydavenport force-pushed the calico-daemonset branch from e2bf14f to 1919cda Compare February 7, 2017 22:49

caseydavenport changed the title ~~[WIP] Update Calico add-on~~ Update Calico add-on Feb 8, 2017

caseydavenport force-pushed the calico-daemonset branch from 1919cda to 7823db6 Compare February 14, 2017 02:58

caseydavenport mentioned this pull request Feb 16, 2017

Update Calico instructions kubernetes/website#2545

Closed

caseydavenport force-pushed the calico-daemonset branch from 7823db6 to d7a68f9 Compare February 23, 2017 02:27

caseydavenport mentioned this pull request Feb 24, 2017

cluster/gce: set cni-bin-dir for kubelet on debian #42079

Closed

roberthbailey reviewed Feb 25, 2017

View reviewed changes

caseydavenport force-pushed the calico-daemonset branch from 0fca57a to b17b394 Compare May 5, 2017 21:36

caseydavenport force-pushed the calico-daemonset branch 3 times, most recently from ff012f5 to c100e5a Compare May 12, 2017 22:41

dnardo reviewed May 14, 2017

View reviewed changes

caseydavenport force-pushed the calico-daemonset branch from c100e5a to 2b9b7f0 Compare May 14, 2017 17:27

k8s-github-robot added release-note Denotes a PR that will be considered when it comes time to generate release notes. and removed release-note-label-needed labels May 15, 2017

roberthbailey reviewed May 17, 2017

View reviewed changes

Update Calico add-on

63744a8

caseydavenport force-pushed the calico-daemonset branch from 2b9b7f0 to 63744a8 Compare May 17, 2017 22:04

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label May 19, 2017

k8s-github-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label May 19, 2017

k8s-github-robot merged commit a9d0403 into kubernetes:master May 20, 2017

caseydavenport deleted the calico-daemonset branch May 20, 2017 02:44

Update Calico add-on #38169

Update Calico add-on #38169

Conversation

caseydavenport commented Dec 6, 2016 • edited Loading

k8s-reviewable commented Dec 6, 2016

caseydavenport commented Dec 6, 2016

eparis commented Dec 6, 2016

thockin commented Dec 8, 2016

caseydavenport commented Dec 15, 2016

roberthbailey commented Jan 31, 2017

caseydavenport commented Jan 31, 2017

caseydavenport commented Feb 8, 2017

freehan commented Feb 10, 2017 • edited Loading

caseydavenport commented Feb 23, 2017

caseydavenport commented Feb 23, 2017

caseydavenport commented Feb 23, 2017

freehan commented Feb 24, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

roberthbailey commented Apr 18, 2017

caseydavenport commented Apr 30, 2017

dnardo commented May 8, 2017

caseydavenport commented May 8, 2017

caseydavenport commented May 9, 2017

caseydavenport commented May 10, 2017 • edited Loading

caseydavenport commented May 12, 2017

dnardo commented May 12, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

caseydavenport commented May 15, 2017

caseydavenport commented May 15, 2017

caseydavenport commented May 16, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

roberthbailey commented May 19, 2017

k8s-github-robot commented May 19, 2017

k8s-ci-robot commented May 20, 2017 • edited Loading

k8s-github-robot commented May 20, 2017

caseydavenport commented Dec 6, 2016 •

edited

Loading

freehan commented Feb 10, 2017 •

edited

Loading

caseydavenport commented May 10, 2017 •

edited

Loading

k8s-ci-robot commented May 20, 2017 •

edited

Loading