Initial support for calico kubernetes backend #3127

blakebarnett · 2017-08-02T21:37:49Z

This will enable locking down access to etcd from the nodes as well as support for the new NetworkPolicy API in k8s 1.7

Update to Calico 2.4
Disable opening the etcd port

k8s-ci-robot · 2017-08-02T21:37:57Z

Hi @blakebarnett. Thanks for your PR.

I'm waiting for a kubernetes member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

I understand the commands that are listed here.

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-08-02T21:38:11Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: blakebarnett
We suggest the following additional approver: justinsb

Assign the PR to them by writing /assign @justinsb in a comment when ready.

The full list of commands accepted by this bot can be found here.

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

OWNERS

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

blakebarnett · 2017-08-02T23:28:15Z

/assign @justinsb

blakebarnett · 2017-08-02T23:30:21Z

pkg/openapi/openapi_generated.go

@@ -1577,6 +1569,27 @@ func GetOpenAPIDefinitions(ref openapi.ReferenceCallback) map[string]openapi.Ope
 			},
 			Dependencies: []string{},
 		},
+		"k8s.io/kops/pkg/apis/kops/v1alpha1.Assets": {


I'm not sure why the apimachinery generated this bit...

Not entirely sure, but my guess is that you're just picking up that it wasn't run before... Don't worry about it anyway, although it highlights that we need to get this under CI testing lest people forget.

blakebarnett · 2017-08-02T23:31:25Z

There were a few suprises when I ran make apimachinery with the new changes in master. Did I do something wrong there?

blakebarnett · 2017-08-04T21:54:53Z

@justinsb looks like switching the backend during a rolling-update isn't going to work automatically. What's the best way to determine if this is a first time cluster deploy? I'll make it refuse to change backends if it's not a new cluster for now.

justinsb · 2017-08-06T03:05:07Z

What's the best way to determine if this is a first time cluster deploy?

This came up in office hours with respect to etcd also (enabling TLS & maybe thinking about allowing etcd3). So I did #3147 / #3148 - that adds a ValidateClusterUpdate validation, where you could prevent this change. It infers the status from the cloudprovider, which is nice for initial population, but we'll probably eventually put the Status into the Cluster object (like other k8s objects).

Because we know we can do this, the fact that switching won't work needn't be a blocker to getting this PR merged. I'll take a pass at the code now.

justinsb

Generallly looks great. We don't want a separate 1.7 manifest unless it is actually needed though...

justinsb · 2017-08-06T03:06:13Z

channels/alpha

@@ -17,6 +17,9 @@ spec:
    networking:
      kubenet: {}
  kubernetesVersions:
+  - range: ">=1.7.0"


Does this relate to this PR (and if so, why :-) )? I made similar changes to the alpha channel but I don't think we've promoted it to stable yet.

I'm happy to leave it out, but there's the issue of having addons not change unless the channel version changes. It's probably not a problem for this PR though, for now we can't make an upgrade work anyway. I can remove.

justinsb · 2017-08-06T03:08:20Z

pkg/model/firewall.go

@@ -196,7 +196,7 @@ func (b *FirewallModelBuilder) applyNodeToMasterBlockSpecificPorts(c *fi.ModelBu
 	udpRanges := []portRange{{From: 1, To: 65535}}
 	protocols := []Protocol{}

-	if b.Cluster.Spec.Networking.Calico != nil {
+	if b.Cluster.Spec.Networking.Calico != nil && b.Cluster.Spec.Networking.Calico.KubernetesDataStore != true {


Nit: !x is probably more idiomatic than x != true, but I actually find your version easier to read. So either works :-)

Yeah I've always found that idiom in go to be super easy to miss

justinsb · 2017-08-06T03:09:40Z

pkg/openapi/openapi_generated.go

@@ -1577,6 +1569,27 @@ func GetOpenAPIDefinitions(ref openapi.ReferenceCallback) map[string]openapi.Ope
 			},
 			Dependencies: []string{},
 		},
+		"k8s.io/kops/pkg/apis/kops/v1alpha1.Assets": {


Not entirely sure, but my guess is that you're just picking up that it wasn't run before... Don't worry about it anyway, although it highlights that we need to get this under CI testing lest people forget.

justinsb · 2017-08-06T03:10:57Z

upup/models/cloudup/resources/addons/networking.projectcalico.org/k8s-1.7.yaml.template

+  namespace: kube-system
+data:
+{{- if not (.Networking.Calico.KubernetesDataStore)}}
+  # The calico-etcd PetSet service IP:port


I think we're supposed to call them StatefulSets now :-) But NBD.

Ah that's remnants, not something I added :) I'll change it

justinsb · 2017-08-06T03:13:19Z

upup/pkg/fi/cloudup/bootstrapchannelbuilder.go

+		}
+
+		{
+			location := key + "/k8s-1.7.yaml"


So we only need a new manifest if the manifests are actually different. 1.6 is really >= 1.6, and pre-1.6 is really < 1.6. The change being that 1.6 changed the node taints/tolerations (both the values and it moved from an annotation to a field), and that 1.6 introduced RBAC.

I'd prefer not to have a duplicate version if 1.7 & 1.6 are actually the same. But of course, if they are different, then they have to be.

Are they different (and if so why :-) ) ?

The main difference for calico is that there are some changes that only work with 1.7 (the NetworkPolicy changes) AFAIK. I don't think it's incompatible with 1.6 at all though, so it's probably safe to leave out the 1.7 manifest.

At some point I would really like to extract the version requirements for bundled addons like this into the addon manifests, so changes to the channel like this would never be necessary.

justinsb · 2017-08-06T03:14:21Z

/ok-to-test

caseydavenport · 2017-08-07T07:06:01Z

docs/networking.md

+```
+  networking:
+    calico:
+      kubernetesDataStore: true


I'd suggest switching this to something like:

datastore: kubernetes | etcd

Just in case someone wants a third option there some day.

- Update to Calico 2.4 - Disable opening the etcd port This will enable locking down access to etcd from the nodes as well as support for the new NetworkPolicy API in k8s 1.7

- Also use RollingUpdate for updateStrategy for the calico-node DaemonSet, which seems safer for updates. - Adds a separate serviceAccount for calico-node and calico-policy-controller, calico-node has many more required privileges.

- Add the path to the manifest visitor error output to make it easier to track down weird manifest errors

blakebarnett · 2017-08-07T21:47:26Z

After speaking to @caseydavenport it's probably best to hold off on merging this until Calico 2.5 comes out and the move from TPR -> CRD is done. In the meantime I'll work on a migration script to try switching this live on a cluster.

- Switch from a bool kubernetesDataStore option to `datastore` and set the default to `etcd` - Remove the 1.7 version manifest

justinsb · 2017-08-17T11:47:11Z

Do let me know if / when this is ready for review @blakebarnett

k8s-github-robot · 2017-08-18T00:03:41Z

@blakebarnett PR needs rebase

blakebarnett · 2017-08-25T18:37:19Z

Looks like 2.5.0 is out and they have a migration process documented. I'll see if it's feasible to automate and try updating this next week: https://github.com/projectcalico/calico/blob/master/upgrade/v2.5/README.md#1-before-you-begin

justinsb · 2017-10-10T09:47:22Z

Hi @blakebarnett sorry this one fell off my radar a bit - where are we on calico versions now?

blakebarnett · 2017-10-10T17:13:41Z

No worries, it fell off of mine also. I probably need to start fresh on this one, probably won't get it done for 1.8

chrislovecnm · 2017-10-11T05:02:55Z

2.5 just came out a couple of weeks ago. We need it for 1.8 network policies ;)

blakebarnett · 2017-10-11T19:12:57Z

You mean 2.6.x :)

Yeah, we should update the version separate from this also.

fejta-bot · 2018-02-20T17:38:01Z

Issues go stale after 90d of inactivity.
Mark the issue as fresh with /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/lifecycle stale

justinsb · 2018-02-26T05:52:44Z

What's the state of this one @blakebarnett ?

blakebarnett · 2018-02-26T18:29:32Z

Will probably be a month or so before I can come back do it, we can close if someone else wants to take a stab.

fejta-bot · 2018-03-28T18:46:53Z

Stale issues rot after 30d of inactivity.
Mark the issue as fresh with /remove-lifecycle rotten.
Rotten issues close after an additional 30d of inactivity.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/lifecycle rotten
/remove-lifecycle stale

blakebarnett · 2018-03-28T18:48:56Z

I don't have time to work on this for now.

k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Aug 2, 2017

k8s-ci-robot assigned justinsb Aug 2, 2017

blakebarnett commented Aug 2, 2017

View reviewed changes

blakebarnett force-pushed the bdb/calico_k8s_backend branch from 4cb06ad to dc56d76 Compare August 4, 2017 22:43

justinsb reviewed Aug 6, 2017

View reviewed changes

k8s-ci-robot removed the needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. label Aug 6, 2017

caseydavenport reviewed Aug 7, 2017

View reviewed changes

Blake added 6 commits August 7, 2017 11:03

Initial support for calico kubernetes backend

b02721c

- Update to Calico 2.4 - Disable opening the etcd port This will enable locking down access to etcd from the nodes as well as support for the new NetworkPolicy API in k8s 1.7

Add RBAC requirements for k8s backend

36aa186

- Also use RollingUpdate for updateStrategy for the calico-node DaemonSet, which seems safer for updates. - Adds a separate serviceAccount for calico-node and calico-policy-controller, calico-node has many more required privileges.

Remove blank annotations in the service accounts

2f7bfd4

- Add the path to the manifest visitor error output to make it easier to track down weird manifest errors

Make channel required versions match properly

59baabb

Add some docs on enabling kdd

b3335c7

typos

5b23877

chrislovecnm added the WIP: Work In Progress label Aug 7, 2017

Incorporate review feedback

238dc94

- Switch from a bool kubernetesDataStore option to `datastore` and set the default to `etcd` - Remove the 1.7 version manifest

blakebarnett force-pushed the bdb/calico_k8s_backend branch from dc56d76 to 238dc94 Compare August 7, 2017 23:25

k8s-github-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Aug 18, 2017

justinsb added this to the 1.8.0 milestone Sep 25, 2017

justinsb added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. and removed WIP: Work In Progress labels Nov 4, 2017

justinsb modified the milestones: 1.8.0, 1.8.1 Nov 22, 2017

k8s-ci-robot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Feb 20, 2018

justinsb modified the milestones: 1.8.1, 1.9 Feb 21, 2018

k8s-ci-robot added lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. and removed lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. labels Mar 28, 2018

blakebarnett closed this Mar 28, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Initial support for calico kubernetes backend #3127

Initial support for calico kubernetes backend #3127

blakebarnett commented Aug 2, 2017 •

edited

Loading

k8s-ci-robot commented Aug 2, 2017

k8s-github-robot commented Aug 2, 2017

blakebarnett commented Aug 2, 2017

blakebarnett Aug 2, 2017

justinsb Aug 6, 2017

blakebarnett commented Aug 2, 2017 •

edited

Loading

blakebarnett commented Aug 4, 2017

justinsb commented Aug 6, 2017

justinsb left a comment

justinsb Aug 6, 2017

blakebarnett Aug 7, 2017

justinsb Aug 6, 2017

blakebarnett Aug 7, 2017

justinsb Aug 6, 2017

justinsb Aug 6, 2017

blakebarnett Aug 7, 2017

justinsb Aug 6, 2017

blakebarnett Aug 7, 2017

justinsb commented Aug 6, 2017

caseydavenport Aug 7, 2017

blakebarnett Aug 7, 2017

blakebarnett commented Aug 7, 2017

justinsb commented Aug 17, 2017

k8s-github-robot commented Aug 18, 2017

blakebarnett commented Aug 25, 2017

justinsb commented Oct 10, 2017

blakebarnett commented Oct 10, 2017

chrislovecnm commented Oct 11, 2017

blakebarnett commented Oct 11, 2017

fejta-bot commented Feb 20, 2018

justinsb commented Feb 26, 2018

blakebarnett commented Feb 26, 2018

fejta-bot commented Mar 28, 2018

blakebarnett commented Mar 28, 2018

Initial support for calico kubernetes backend #3127

Initial support for calico kubernetes backend #3127

Conversation

blakebarnett commented Aug 2, 2017 • edited Loading

k8s-ci-robot commented Aug 2, 2017

k8s-github-robot commented Aug 2, 2017

blakebarnett commented Aug 2, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

blakebarnett commented Aug 2, 2017 • edited Loading

blakebarnett commented Aug 4, 2017

justinsb commented Aug 6, 2017

justinsb left a comment

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

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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

justinsb commented Aug 6, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

blakebarnett commented Aug 7, 2017

justinsb commented Aug 17, 2017

k8s-github-robot commented Aug 18, 2017

blakebarnett commented Aug 25, 2017

justinsb commented Oct 10, 2017

blakebarnett commented Oct 10, 2017

chrislovecnm commented Oct 11, 2017

blakebarnett commented Oct 11, 2017

fejta-bot commented Feb 20, 2018

justinsb commented Feb 26, 2018

blakebarnett commented Feb 26, 2018

fejta-bot commented Mar 28, 2018

blakebarnett commented Mar 28, 2018

blakebarnett commented Aug 2, 2017 •

edited

Loading

blakebarnett commented Aug 2, 2017 •

edited

Loading