From 51235b2edc9bfaab07b53e99115f1321957c6476 Mon Sep 17 00:00:00 2001 From: Ole Markus With Date: Fri, 26 Jun 2020 14:11:05 +0200 Subject: [PATCH 1/3] Deploy cilium etcd credentials if the cilium cluster exists --- nodeup/pkg/model/networking/cilium.go | 22 ++++++++++++++----- pkg/model/iam/iam_builder.go | 11 +++++++++- upup/models/bindata.go | 4 ++-- .../k8s-1.12.yaml.template | 4 ++-- 4 files changed, 30 insertions(+), 11 deletions(-) diff --git a/nodeup/pkg/model/networking/cilium.go b/nodeup/pkg/model/networking/cilium.go index cc80be2aa395f..38aac98b48cf5 100644 --- a/nodeup/pkg/model/networking/cilium.go +++ b/nodeup/pkg/model/networking/cilium.go @@ -37,20 +37,30 @@ var _ fi.ModelBuilder = &CiliumBuilder{} func (b *CiliumBuilder) Build(c *fi.ModelBuilderContext) error { networking := b.Cluster.Spec.Networking - if networking.Cilium == nil { - return nil - } + // As long as the cilium cluster exists, we should do this + ciliumEtcd := false - if err := b.buildBPFMount(c); err != nil { - return err + for _, cluster := range b.Cluster.Spec.EtcdClusters { + if cluster.Name == "cilium" { + ciliumEtcd = true + break + } } - if networking.Cilium.EtcdManaged { + if ciliumEtcd { if err := b.buildCiliumEtcdSecrets(c); err != nil { return err } } + if networking.Cilium == nil { + return nil + } + + if err := b.buildBPFMount(c); err != nil { + return err + } + return nil } diff --git a/pkg/model/iam/iam_builder.go b/pkg/model/iam/iam_builder.go index dfd9246db4d43..414e7d429c923 100644 --- a/pkg/model/iam/iam_builder.go +++ b/pkg/model/iam/iam_builder.go @@ -478,7 +478,16 @@ func ReadableStatePaths(cluster *kops.Cluster, role kops.InstanceGroupRole) ([]s } // @check if cilium is enabled as the CNI provider and permit access to the cilium etc client TLS certificate by default - if networkingSpec.Cilium != nil && networkingSpec.Cilium.EtcdManaged { + // As long as the cilium cluster exists, we should do this + ciliumEtcd := false + + for _, cluster := range cluster.Spec.EtcdClusters { + if cluster.Name == "cilium" { + ciliumEtcd = true + break + } + } + if networkingSpec.Cilium != nil && ciliumEtcd { paths = append(paths, "/pki/private/etcd-clients-ca-cilium/*") } } diff --git a/upup/models/bindata.go b/upup/models/bindata.go index e1b78ee8cdebe..d8626304f9204 100644 --- a/upup/models/bindata.go +++ b/upup/models/bindata.go @@ -4088,8 +4088,8 @@ data: - https://{{ $.MasterInternalName }}:4003 trusted-ca-file: '/var/lib/etcd-secrets/etcd-ca.crt' - key-file: '/var/lib/etcd-secrets/etcd-client.key' - cert-file: '/var/lib/etcd-secrets/etcd-client.crt' + key-file: '/var/lib/etcd-secrets/etcd-client-cilium.key' + cert-file: '/var/lib/etcd-secrets/etcd-client-cilium.crt' {{ end }} # Identity allocation mode selects how identities are shared between cilium diff --git a/upup/models/cloudup/resources/addons/networking.cilium.io/k8s-1.12.yaml.template b/upup/models/cloudup/resources/addons/networking.cilium.io/k8s-1.12.yaml.template index 8258844534a6c..d80c3d859e83e 100644 --- a/upup/models/cloudup/resources/addons/networking.cilium.io/k8s-1.12.yaml.template +++ b/upup/models/cloudup/resources/addons/networking.cilium.io/k8s-1.12.yaml.template @@ -28,8 +28,8 @@ data: - https://{{ $.MasterInternalName }}:4003 trusted-ca-file: '/var/lib/etcd-secrets/etcd-ca.crt' - key-file: '/var/lib/etcd-secrets/etcd-client.key' - cert-file: '/var/lib/etcd-secrets/etcd-client.crt' + key-file: '/var/lib/etcd-secrets/etcd-client-cilium.key' + cert-file: '/var/lib/etcd-secrets/etcd-client-cilium.crt' {{ end }} # Identity allocation mode selects how identities are shared between cilium From fea1aa0ae8a03055715965b63fb21d6dd0f9da09 Mon Sep 17 00:00:00 2001 From: Ole Markus With Date: Fri, 26 Jun 2020 14:11:30 +0200 Subject: [PATCH 2/3] Improve cilium feature documentation --- docs/networking/cilium.md | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-) diff --git a/docs/networking/cilium.md b/docs/networking/cilium.md index 93e03939031bd..ca2b409bcfa3a 100644 --- a/docs/networking/cilium.md +++ b/docs/networking/cilium.md @@ -27,7 +27,9 @@ kops create cluster \ ### Using etcd for agent state sync -By default, Cilium will use CRDs for synchronizing agent state. This can cause performance problems on larger clusters. As of kops 1.18, kops can manage an etcd cluster using etcd-manager dedicated for cilium agent state sync. The [Cilium docs](https://docs.cilium.io/en/stable/gettingstarted/k8s-install-external-etcd/) contains recommendations for this must be enabled. +This feature is in beta state as of kops 1.18. + +By default, Cilium will use CRDs for synchronizing agent state. This can cause performance problems on larger clusters. As of kops 1.18, kops can manage an etcd cluster using etcd-manager dedicated for cilium agent state sync. The [Cilium docs](https://docs.cilium.io/en/stable/gettingstarted/k8s-install-external-etcd/) contains recommendations for when this must be enabled. Add the following to `spec.etcdClusters`: Make sure `instanceGroup` match the other etcd clusters. @@ -43,6 +45,15 @@ Make sure `instanceGroup` match the other etcd clusters. name: cilium ``` +If this is an existing cluster, it is important that you roll the entire cluster so that all the nodes can connect to the new etcd cluster. + +```sh +kops update cluster +kops update cluster --yes +kops rolling-update cluster --force --yes + +``` + Then enable etcd as kvstore: ```yaml @@ -60,6 +71,8 @@ Read more about this in the [Cilium docs](https://docs.cilium.io/en/stable/getti Be aware that you need to use an AMI with at least Linux 4.19.57 for this feature to work. +Also be aware that while enabling this on an existing cluster is safe, disabling this is disruptive and requires you to run `kops rolling-upgrade cluster --cloudonly`. + ```yaml kubeProxy: enabled: false @@ -70,6 +83,8 @@ Be aware that you need to use an AMI with at least Linux 4.19.57 for this featur ### Enabling Cilium ENI IPAM +This feature is in beta state as of kops 1.18. + As of Kops 1.18, you can have Cilium provision AWS managed adresses and attach them directly to Pods much like Lyft VPC and AWS VPC. See [the Cilium docs for more information](https://docs.cilium.io/en/v1.6/concepts/ipam/eni/) When using ENI IPAM you need to disable masquerading in Cilium as well. From 2fd6e52af7b3fa28e4d09adb0822986d01e38d50 Mon Sep 17 00:00:00 2001 From: Ole Markus With Date: Sat, 27 Jun 2020 07:43:30 +0200 Subject: [PATCH 3/3] Apply suggestions from code review Co-authored-by: John Gardiner Myers --- docs/networking/cilium.md | 2 +- nodeup/pkg/model/networking/cilium.go | 2 +- pkg/model/iam/iam_builder.go | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/docs/networking/cilium.md b/docs/networking/cilium.md index ca2b409bcfa3a..d98dc3bca92e0 100644 --- a/docs/networking/cilium.md +++ b/docs/networking/cilium.md @@ -45,7 +45,7 @@ Make sure `instanceGroup` match the other etcd clusters. name: cilium ``` -If this is an existing cluster, it is important that you roll the entire cluster so that all the nodes can connect to the new etcd cluster. +If this is an existing cluster, it is important that you perform a rolling update on the entire cluster so that all the nodes can connect to the new etcd cluster. ```sh kops update cluster diff --git a/nodeup/pkg/model/networking/cilium.go b/nodeup/pkg/model/networking/cilium.go index 38aac98b48cf5..c3c243d651107 100644 --- a/nodeup/pkg/model/networking/cilium.go +++ b/nodeup/pkg/model/networking/cilium.go @@ -37,7 +37,7 @@ var _ fi.ModelBuilder = &CiliumBuilder{} func (b *CiliumBuilder) Build(c *fi.ModelBuilderContext) error { networking := b.Cluster.Spec.Networking - // As long as the cilium cluster exists, we should do this + // As long as the Cilium Etcd cluster exists, we should do this ciliumEtcd := false for _, cluster := range b.Cluster.Spec.EtcdClusters { diff --git a/pkg/model/iam/iam_builder.go b/pkg/model/iam/iam_builder.go index 414e7d429c923..6ed0c1b9f4ad3 100644 --- a/pkg/model/iam/iam_builder.go +++ b/pkg/model/iam/iam_builder.go @@ -478,7 +478,7 @@ func ReadableStatePaths(cluster *kops.Cluster, role kops.InstanceGroupRole) ([]s } // @check if cilium is enabled as the CNI provider and permit access to the cilium etc client TLS certificate by default - // As long as the cilium cluster exists, we should do this + // As long as the Cilium Etcd cluster exists, we should do this ciliumEtcd := false for _, cluster := range cluster.Spec.EtcdClusters {