From 3cf8234d015733c6398b2843b94309c1fac71a1c Mon Sep 17 00:00:00 2001 From: John Gardiner Myers Date: Sun, 13 Jun 2021 16:42:17 -0700 Subject: [PATCH 1/2] Cilium: disable masquerade by default when in ENI IPAM mode --- docs/networking/cilium.md | 14 +++++++++++--- pkg/apis/kops/networking.go | 2 +- pkg/apis/kops/v1alpha2/networking.go | 2 +- pkg/apis/kops/v1alpha2/zz_generated.deepcopy.go | 5 +++++ pkg/apis/kops/validation/validation.go | 2 +- pkg/apis/kops/validation/validation_test.go | 11 +++++------ pkg/apis/kops/zz_generated.deepcopy.go | 5 +++++ pkg/commands/set_cluster_test.go | 4 ++-- pkg/model/components/cilium.go | 4 ++++ .../k8s-1.12-v1.8.yaml.template | 2 +- .../k8s-1.12-v1.9.yaml.template | 2 +- .../k8s-1.16-v1.10.yaml.template | 2 +- 12 files changed, 38 insertions(+), 17 deletions(-) diff --git a/docs/networking/cilium.md b/docs/networking/cilium.md index bce11f4838f65..d146a7fda797d 100644 --- a/docs/networking/cilium.md +++ b/docs/networking/cilium.md @@ -103,11 +103,19 @@ kops rolling-update cluster --yes ### Enabling Cilium ENI IPAM -This feature is in beta state as of kOps 1.18. +{{ kops_feature_table(kops_added_default='1.18') }} + +This feature is in beta state. -As of kOps 1.18, you can have Cilium provision AWS managed addresses 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/) +You can have Cilium provision AWS managed addresses 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/) + +```yaml + networking: + cilium: + ipam: eni +``` -When using ENI IPAM you need to disable masquerading in Cilium as well. +In kOps versions before 1.22, when using ENI IPAM you need to explicitly disable masquerading in Cilium as well. ```yaml networking: diff --git a/pkg/apis/kops/networking.go b/pkg/apis/kops/networking.go index 3e445fbb8d516..ec3edbf686d6c 100644 --- a/pkg/apis/kops/networking.go +++ b/pkg/apis/kops/networking.go @@ -396,7 +396,7 @@ type CiliumNetworkingSpec struct { // Setting this has no effect. LogstashProbeTimer uint32 `json:"logstashProbeTimer,omitempty"` // DisableMasquerade disables masquerading traffic to external destinations behind the node IP. - DisableMasquerade bool `json:"disableMasquerade,omitempty"` + DisableMasquerade *bool `json:"disableMasquerade,omitempty"` // Nat6Range is not implemented and may be removed in the future. // Setting this has no effect. Nat46Range string `json:"nat46Range,omitempty"` diff --git a/pkg/apis/kops/v1alpha2/networking.go b/pkg/apis/kops/v1alpha2/networking.go index 447006f5ceb38..1b6e7c210d098 100644 --- a/pkg/apis/kops/v1alpha2/networking.go +++ b/pkg/apis/kops/v1alpha2/networking.go @@ -394,7 +394,7 @@ type CiliumNetworkingSpec struct { // Setting this has no effect. LogstashProbeTimer uint32 `json:"logstashProbeTimer,omitempty"` // DisableMasquerade disables masquerading traffic to external destinations behind the node IP. - DisableMasquerade bool `json:"disableMasquerade,omitempty"` + DisableMasquerade *bool `json:"disableMasquerade,omitempty"` // Nat6Range is not implemented and may be removed in the future. // Setting this has no effect. Nat46Range string `json:"nat46Range,omitempty"` diff --git a/pkg/apis/kops/v1alpha2/zz_generated.deepcopy.go b/pkg/apis/kops/v1alpha2/zz_generated.deepcopy.go index 792b8c85057e4..6324224ae5692 100644 --- a/pkg/apis/kops/v1alpha2/zz_generated.deepcopy.go +++ b/pkg/apis/kops/v1alpha2/zz_generated.deepcopy.go @@ -531,6 +531,11 @@ func (in *CiliumNetworkingSpec) DeepCopyInto(out *CiliumNetworkingSpec) { (*out)[key] = val } } + if in.DisableMasquerade != nil { + in, out := &in.DisableMasquerade, &out.DisableMasquerade + *out = new(bool) + **out = **in + } if in.EnableRemoteNodeIdentity != nil { in, out := &in.EnableRemoteNodeIdentity, &out.EnableRemoteNodeIdentity *out = new(bool) diff --git a/pkg/apis/kops/validation/validation.go b/pkg/apis/kops/validation/validation.go index a5b9e9fe75206..d6466fcfef88a 100644 --- a/pkg/apis/kops/validation/validation.go +++ b/pkg/apis/kops/validation/validation.go @@ -855,7 +855,7 @@ func validateNetworkingCilium(cluster *kops.Cluster, v *kops.CiliumNetworkingSpe if c.CloudProvider != string(kops.CloudProviderAWS) { allErrs = append(allErrs, field.Forbidden(fldPath.Child("ipam"), "Cilum ENI IPAM is supported only in AWS")) } - if !v.DisableMasquerade { + if v.DisableMasquerade != nil && !*v.DisableMasquerade { allErrs = append(allErrs, field.Forbidden(fldPath.Child("disableMasquerade"), "Masquerade must be disabled when ENI IPAM is used")) } } diff --git a/pkg/apis/kops/validation/validation_test.go b/pkg/apis/kops/validation/validation_test.go index 79b03f7d44030..6f12aedf2dfc8 100644 --- a/pkg/apis/kops/validation/validation_test.go +++ b/pkg/apis/kops/validation/validation_test.go @@ -790,8 +790,7 @@ func Test_Validate_Cilium(t *testing.T) { }, { Cilium: kops.CiliumNetworkingSpec{ - DisableMasquerade: true, - Ipam: "eni", + Ipam: "eni", }, Spec: kops.ClusterSpec{ CloudProvider: "aws", @@ -799,7 +798,7 @@ func Test_Validate_Cilium(t *testing.T) { }, { Cilium: kops.CiliumNetworkingSpec{ - DisableMasquerade: true, + DisableMasquerade: fi.Bool(true), Ipam: "eni", }, Spec: kops.ClusterSpec{ @@ -814,7 +813,8 @@ func Test_Validate_Cilium(t *testing.T) { }, { Cilium: kops.CiliumNetworkingSpec{ - Ipam: "eni", + DisableMasquerade: fi.Bool(false), + Ipam: "eni", }, Spec: kops.ClusterSpec{ CloudProvider: "aws", @@ -823,8 +823,7 @@ func Test_Validate_Cilium(t *testing.T) { }, { Cilium: kops.CiliumNetworkingSpec{ - DisableMasquerade: true, - Ipam: "eni", + Ipam: "eni", }, Spec: kops.ClusterSpec{ CloudProvider: "gce", diff --git a/pkg/apis/kops/zz_generated.deepcopy.go b/pkg/apis/kops/zz_generated.deepcopy.go index 32c2381e665b4..2601e3dfc5a34 100644 --- a/pkg/apis/kops/zz_generated.deepcopy.go +++ b/pkg/apis/kops/zz_generated.deepcopy.go @@ -615,6 +615,11 @@ func (in *CiliumNetworkingSpec) DeepCopyInto(out *CiliumNetworkingSpec) { (*out)[key] = val } } + if in.DisableMasquerade != nil { + in, out := &in.DisableMasquerade, &out.DisableMasquerade + *out = new(bool) + **out = **in + } if in.EnableRemoteNodeIdentity != nil { in, out := &in.EnableRemoteNodeIdentity, &out.EnableRemoteNodeIdentity *out = new(bool) diff --git a/pkg/commands/set_cluster_test.go b/pkg/commands/set_cluster_test.go index 8caeedeccfec5..3a06647699fc8 100644 --- a/pkg/commands/set_cluster_test.go +++ b/pkg/commands/set_cluster_test.go @@ -288,7 +288,7 @@ func TestSetClusterFields(t *testing.T) { Spec: kops.ClusterSpec{ Networking: &kops.NetworkingSpec{ Cilium: &kops.CiliumNetworkingSpec{ - DisableMasquerade: true, + DisableMasquerade: fi.Bool(true), }, }, }, @@ -367,7 +367,7 @@ func TestSetCiliumFields(t *testing.T) { Cilium: &kops.CiliumNetworkingSpec{ Ipam: "eni", EnableNodePort: true, - DisableMasquerade: true, + DisableMasquerade: fi.Bool(true), }, }, }, diff --git a/pkg/model/components/cilium.go b/pkg/model/components/cilium.go index 0d0ca8ff5beac..2dde9f22a8cb4 100644 --- a/pkg/model/components/cilium.go +++ b/pkg/model/components/cilium.go @@ -85,6 +85,10 @@ func (b *CiliumOptionsBuilder) BuildOptions(o interface{}) error { } } + if c.DisableMasquerade == nil { + c.DisableMasquerade = fi.Bool(c.Ipam == "eni") + } + if c.Tunnel == "" { if c.Ipam == "eni" { c.Tunnel = "disabled" diff --git a/upup/models/cloudup/resources/addons/networking.cilium.io/k8s-1.12-v1.8.yaml.template b/upup/models/cloudup/resources/addons/networking.cilium.io/k8s-1.12-v1.8.yaml.template index b373dfcf31163..1a9ac51c14e77 100644 --- a/upup/models/cloudup/resources/addons/networking.cilium.io/k8s-1.12-v1.8.yaml.template +++ b/upup/models/cloudup/resources/addons/networking.cilium.io/k8s-1.12-v1.8.yaml.template @@ -149,7 +149,7 @@ data: # - auto (automatically detect the container runtime) # container-runtime: "{{ .ContainerRuntimeLabels }}" - masquerade: "{{- if .DisableMasquerade -}}false{{- else -}}true{{- end -}}" + masquerade: "{{- if WithDefaultBool .DisableMasquerade false -}}false{{- else -}}true{{- end -}}" install-iptables-rules: "{{- if .IPTablesRulesNoinstall -}}false{{- else -}}true{{- end -}}" auto-direct-node-routes: "{{ .AutoDirectNodeRoutes }}" {{ if .EnableHostReachableServices }} diff --git a/upup/models/cloudup/resources/addons/networking.cilium.io/k8s-1.12-v1.9.yaml.template b/upup/models/cloudup/resources/addons/networking.cilium.io/k8s-1.12-v1.9.yaml.template index c290bfb5c7026..14d9d9cf7d60a 100644 --- a/upup/models/cloudup/resources/addons/networking.cilium.io/k8s-1.12-v1.9.yaml.template +++ b/upup/models/cloudup/resources/addons/networking.cilium.io/k8s-1.12-v1.9.yaml.template @@ -170,7 +170,7 @@ data: # - auto (automatically detect the container runtime) # container-runtime: "{{ .ContainerRuntimeLabels }}" - masquerade: "{{- if .DisableMasquerade -}}false{{- else -}}true{{- end -}}" + masquerade: "{{- if WithDefaultBool .DisableMasquerade false -}}false{{- else -}}true{{- end -}}" install-iptables-rules: "{{- if .IPTablesRulesNoinstall -}}false{{- else -}}true{{- end -}}" auto-direct-node-routes: "{{ .AutoDirectNodeRoutes }}" {{ if .EnableHostReachableServices }} diff --git a/upup/models/cloudup/resources/addons/networking.cilium.io/k8s-1.16-v1.10.yaml.template b/upup/models/cloudup/resources/addons/networking.cilium.io/k8s-1.16-v1.10.yaml.template index 2633fbf71b2ad..81615356810da 100644 --- a/upup/models/cloudup/resources/addons/networking.cilium.io/k8s-1.16-v1.10.yaml.template +++ b/upup/models/cloudup/resources/addons/networking.cilium.io/k8s-1.16-v1.10.yaml.template @@ -170,7 +170,7 @@ data: # - auto (automatically detect the container runtime) # container-runtime: "{{ .ContainerRuntimeLabels }}" - masquerade: "{{- if .DisableMasquerade -}}false{{- else -}}true{{- end -}}" + masquerade: "{{- if WithDefaultBool .DisableMasquerade false -}}false{{- else -}}true{{- end -}}" install-iptables-rules: "{{- if .IPTablesRulesNoinstall -}}false{{- else -}}true{{- end -}}" auto-direct-node-routes: "{{ .AutoDirectNodeRoutes }}" {{ if .EnableHostReachableServices }} From 0b7f6e308254a7a9f4beaa5196bdc5bb2c7ece67 Mon Sep 17 00:00:00 2001 From: John Gardiner Myers Date: Sun, 13 Jun 2021 16:58:55 -0700 Subject: [PATCH 2/2] Remove dead code --- pkg/model/components/cilium.go | 9 +-------- 1 file changed, 1 insertion(+), 8 deletions(-) diff --git a/pkg/model/components/cilium.go b/pkg/model/components/cilium.go index 2dde9f22a8cb4..4e80e16c96fa5 100644 --- a/pkg/model/components/cilium.go +++ b/pkg/model/components/cilium.go @@ -17,7 +17,6 @@ limitations under the License. package components import ( - "github.com/blang/semver/v4" "k8s.io/apimachinery/pkg/api/resource" "k8s.io/kops/pkg/apis/kops" "k8s.io/kops/pkg/wellknownports" @@ -43,8 +42,6 @@ func (b *CiliumOptionsBuilder) BuildOptions(o interface{}) error { c.Version = "v1.10.0" } - version, _ := semver.ParseTolerant(c.Version) - if c.BPFCTGlobalAnyMax == 0 { c.BPFCTGlobalAnyMax = 262144 @@ -78,11 +75,7 @@ func (b *CiliumOptionsBuilder) BuildOptions(o interface{}) error { } if c.Ipam == "" { - if version.Minor >= 8 { - c.Ipam = "kubernetes" - } else { - c.Ipam = "hostscope" - } + c.Ipam = "kubernetes" } if c.DisableMasquerade == nil {