From 9800423260f81cb8d957c295ac1dea2e82114366 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Juan=20Luis=20de=20Sousa-Valadas=20Casta=C3=B1o?= Date: Wed, 16 Nov 2022 17:33:53 +0100 Subject: [PATCH] Enable hairpin for kube-router by default MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit We agreed that the most logical decision was to enable hairpin by default. The old configuration doesn't reflect our real needs, for this reason we decided to deprecate the old configuration and add a new configuration. This commit handles all these details. Fixes https://github.com/k0sproject/k0s/issues/1953 Co-authored-by: Tom Wieczorek Signed-off-by: Juan Luis de Sousa-Valadas Castaño --- docs/configuration.md | 19 ++++---- inttest/kuberouter/kuberouter_hairpin_test.go | 2 - .../k0s.k0sproject.io/v1beta1/kuberouter.go | 20 +++++++- pkg/component/controller/kuberouter.go | 30 ++++++++++-- pkg/component/controller/kuberouter_test.go | 46 +++++++++++++++++-- .../k0s.k0sproject.io_clusterconfigs.yaml | 15 +++++- 6 files changed, 109 insertions(+), 23 deletions(-) diff --git a/docs/configuration.md b/docs/configuration.md index 15df0ad52043..f57b535c68ff 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -66,7 +66,7 @@ spec: peerRouterIPs: "" peerRouterASNs: "" autoMTU: true - hairpinMode: false + hairpin: Enabled kubeProxy: disabled: false mode: iptables @@ -211,14 +211,15 @@ CALICO_IPV6POOL_CIDR: "{{ spec.network.dualStack.IPv6podCIDR }}" #### `spec.network.kuberouter` -| Element | Description | -| ---------------- |----------------------------------------------------------------------------------------------------------------------------------------------------| -| `autoMTU` | Autodetection of used MTU (default: `true`). | -| `mtu` | Override MTU setting, if `autoMTU` must be set to `false`). | -| `metricsPort` | Kube-router metrics server port. Set to 0 to disable metrics (default: `8080`). | -| `peerRouterIPs` | Comma-separated list of [global peer addresses](https://github.com/cloudnativelabs/kube-router/blob/master/docs/bgp.md#global-external-bgp-peers). | -| `peerRouterASNs` | Comma-separated list of [global peer ASNs](https://github.com/cloudnativelabs/kube-router/blob/master/docs/bgp.md#global-external-bgp-peers). | -| `hairpinMode` | Activate hairpinMode (default: `false`) (https://github.com/cloudnativelabs/kube-router/blob/master/docs/user-guide.md#hairpin-mode) | +| Element | Description | +| ---------------- |---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------| +| `autoMTU` | Autodetection of used MTU (default: `true`). | +| `mtu` | Override MTU setting, if `autoMTU` must be set to `false`). | +| `metricsPort` | Kube-router metrics server port. Set to 0 to disable metrics (default: `8080`). | +| `peerRouterIPs` | Comma-separated list of [global peer addresses](https://github.com/cloudnativelabs/kube-router/blob/master/docs/bgp.md#global-external-bgp-peers). | +| `peerRouterASNs` | Comma-separated list of [global peer ASNs](https://github.com/cloudnativelabs/kube-router/blob/master/docs/bgp.md#global-external-bgp-peers). | +| `hairpin` | Hairpin mode, supported modes `Enabled`: enabled cluster wide, `Allowed`: must be allowed per service [using annotations](https://github.com/cloudnativelabs/kube-router/blob/master/docs/user-guide.md#hairpin-mode), `Disabled`: doesn't work at all (default: Enabled) | +| `hairpinMode` | **Deprecated** Use `hairpin` instead. If both `hairpin` and `hairpinMode` are defined, this is ignored. If only hairpinMode is configured explicitly activates hairpinMode (https://github.com/cloudnativelabs/kube-router/blob/master/docs/user-guide.md#hairpin-mode). | **Note**: Kube-router allows many networking aspects to be configured per node, service, and pod (for more information, refer to the [Kube-router user guide](https://github.com/cloudnativelabs/kube-router/blob/master/docs/user-guide.md)). diff --git a/inttest/kuberouter/kuberouter_hairpin_test.go b/inttest/kuberouter/kuberouter_hairpin_test.go index 91c6117ec78e..e340b311babc 100644 --- a/inttest/kuberouter/kuberouter_hairpin_test.go +++ b/inttest/kuberouter/kuberouter_hairpin_test.go @@ -100,8 +100,6 @@ const k0sConfigWithHairpinning = ` spec: network: provider: kuberouter - kuberouter: - hairpinMode: true ` const podManifest = ` diff --git a/pkg/apis/k0s.k0sproject.io/v1beta1/kuberouter.go b/pkg/apis/k0s.k0sproject.io/v1beta1/kuberouter.go index f5b75c06c2ba..5252b70e29a2 100644 --- a/pkg/apis/k0s.k0sproject.io/v1beta1/kuberouter.go +++ b/pkg/apis/k0s.k0sproject.io/v1beta1/kuberouter.go @@ -24,19 +24,35 @@ type KubeRouter struct { MTU int `json:"mtu"` // Kube-router metrics server port. Set to 0 to disable metrics (default: 8080) MetricsPort int `json:"metricsPort"` - // Activate Hairpin Mode (allow a Pod behind a Service to communicate to its own ClusterIP:Port) - HairpinMode bool `json:"hairpinMode"` + // Admits three values: "Enabled" enables it globally, "Allowed" allows but services must be annotated explicitly and "Disabled" + // Defaults to "Enabled" + // +kubebuilder:default=Enabled + Hairpin Hairpin `json:"hairpin"` + // DEPRECATED: Use hairpin instead. Activates Hairpin Mode (allow a Pod behind a Service to communicate to its own ClusterIP:Port) + HairpinMode bool `json:"hairpinMode,omitempty"` // Comma-separated list of global peer addresses PeerRouterASNs string `json:"peerRouterASNs"` // Comma-separated list of global peer ASNs PeerRouterIPs string `json:"peerRouterIPs"` } +// +kubebuilder:validation:Enum=Enabled;Allowed;Disabled +type Hairpin string + +const ( + HairpinEnabled Hairpin = "Enabled" + HairpinAllowed Hairpin = "Allowed" + HairpinDisabled Hairpin = "Disabled" + // Necessary for backwards compatibility with HairpinMode + HairpinUndefined Hairpin = "" +) + // DefaultKubeRouter returns the default config for kube-router func DefaultKubeRouter() *KubeRouter { return &KubeRouter{ MTU: 0, AutoMTU: true, MetricsPort: 8080, + Hairpin: HairpinEnabled, } } diff --git a/pkg/component/controller/kuberouter.go b/pkg/component/controller/kuberouter.go index 0d9ec063fd48..0dd53fd3fd89 100644 --- a/pkg/component/controller/kuberouter.go +++ b/pkg/component/controller/kuberouter.go @@ -48,7 +48,8 @@ type kubeRouterConfig struct { MetricsPort int CNIInstallerImage string CNIImage string - HairpinMode bool + GlobalHairpin bool + CNIHairpin bool PeerRouterIPs string PeerRouterASNs string PullPolicy string @@ -70,6 +71,27 @@ func (k *KubeRouter) Init(_ context.Context) error { return nil } // Stop no-op as nothing running func (k *KubeRouter) Stop() error { return nil } +func getHairpinConfig(cfg *kubeRouterConfig, krc *v1beta1.KubeRouter) { + // Configure hairpin + switch krc.Hairpin { + case v1beta1.HairpinUndefined: + // If Hairpin is undefined, then we honor HairpinMode + if krc.HairpinMode { + cfg.CNIHairpin = true + cfg.GlobalHairpin = true + } + case v1beta1.HairpinDisabled: + cfg.CNIHairpin = false + cfg.GlobalHairpin = false + case v1beta1.HairpinAllowed: + cfg.CNIHairpin = true + cfg.GlobalHairpin = false + case v1beta1.HairpinEnabled: + cfg.CNIHairpin = true + cfg.GlobalHairpin = true + } +} + // Reconcile detects changes in configuration and applies them to the component func (k *KubeRouter) Reconcile(_ context.Context, clusterConfig *v1beta1.ClusterConfig) error { logrus.Debug("reconcile method called for: KubeRouter") @@ -88,11 +110,11 @@ func (k *KubeRouter) Reconcile(_ context.Context, clusterConfig *v1beta1.Cluster MetricsPort: clusterConfig.Spec.Network.KubeRouter.MetricsPort, PeerRouterIPs: clusterConfig.Spec.Network.KubeRouter.PeerRouterIPs, PeerRouterASNs: clusterConfig.Spec.Network.KubeRouter.PeerRouterASNs, - HairpinMode: clusterConfig.Spec.Network.KubeRouter.HairpinMode, CNIImage: clusterConfig.Spec.Images.KubeRouter.CNI.URI(), CNIInstallerImage: clusterConfig.Spec.Images.KubeRouter.CNIInstaller.URI(), PullPolicy: clusterConfig.Spec.Images.DefaultPullPolicy, } + getHairpinConfig(&cfg, clusterConfig.Spec.Network.KubeRouter) if cfg == k.previousConfig { k.log.Info("config matches with previous, not reconciling anything") @@ -149,7 +171,7 @@ data: "auto-mtu": {{ .AutoMTU }}, "bridge":"kube-bridge", "isDefaultGateway":true, - "hairpinMode": {{ .HairpinMode }}, + "hairpinMode": {{ .CNIHairpin }}, "ipam":{ "type":"host-local" } @@ -258,7 +280,7 @@ spec: - "--run-service-proxy=false" - "--bgp-graceful-restart=true" - "--metrics-port={{ .MetricsPort }}" - - "--hairpin-mode={{ .HairpinMode }}" + - "--hairpin-mode={{ .GlobalHairpin }}" {{- if .PeerRouterIPs }} - "--peer-router-ips={{ .PeerRouterIPs }}" {{- end }} diff --git a/pkg/component/controller/kuberouter_test.go b/pkg/component/controller/kuberouter_test.go index a5d21a1d1e29..03e0ea53a584 100644 --- a/pkg/component/controller/kuberouter_test.go +++ b/pkg/component/controller/kuberouter_test.go @@ -44,7 +44,7 @@ func TestKubeRouterConfig(t *testing.T) { cfg.Spec.Network.KubeRouter.MTU = 1450 cfg.Spec.Network.KubeRouter.PeerRouterASNs = "12345,67890" cfg.Spec.Network.KubeRouter.PeerRouterIPs = "1.2.3.4,4.3.2.1" - cfg.Spec.Network.KubeRouter.HairpinMode = true + cfg.Spec.Network.KubeRouter.Hairpin = v1beta1.HairpinAllowed saver := inMemorySaver{} kr := NewKubeRouter(k0sVars, saver) @@ -61,7 +61,7 @@ func TestKubeRouterConfig(t *testing.T) { require.NotNil(t, ds) require.Contains(t, ds.Spec.Template.Spec.Containers[0].Args, "--peer-router-ips=1.2.3.4,4.3.2.1") require.Contains(t, ds.Spec.Template.Spec.Containers[0].Args, "--peer-router-asns=12345,67890") - require.Contains(t, ds.Spec.Template.Spec.Containers[0].Args, "--hairpin-mode=true") + require.Contains(t, ds.Spec.Template.Spec.Containers[0].Args, "--hairpin-mode=false") cm, err := findConfig(resources) require.NoError(t, err) @@ -74,6 +74,44 @@ func TestKubeRouterConfig(t *testing.T) { require.Equal(t, true, p.Dig("hairpinMode")) } +type hairpinTest struct { + krc *v1beta1.KubeRouter + result kubeRouterConfig +} + +func TestGetHairpinConfig(t *testing.T) { + hairpinTests := []hairpinTest{ + { + krc: &v1beta1.KubeRouter{Hairpin: v1beta1.HairpinUndefined, HairpinMode: true}, + result: kubeRouterConfig{CNIHairpin: true, GlobalHairpin: true}, + }, + { + krc: &v1beta1.KubeRouter{Hairpin: v1beta1.HairpinUndefined, HairpinMode: false}, + result: kubeRouterConfig{CNIHairpin: false, GlobalHairpin: false}, + }, + { + krc: &v1beta1.KubeRouter{Hairpin: v1beta1.HairpinAllowed, HairpinMode: true}, + result: kubeRouterConfig{CNIHairpin: true, GlobalHairpin: false}, + }, + { + krc: &v1beta1.KubeRouter{Hairpin: v1beta1.HairpinDisabled, HairpinMode: true}, + result: kubeRouterConfig{CNIHairpin: false, GlobalHairpin: false}, + }, + { + krc: &v1beta1.KubeRouter{Hairpin: v1beta1.HairpinEnabled, HairpinMode: false}, + result: kubeRouterConfig{CNIHairpin: true, GlobalHairpin: true}, + }, + } + + for _, test := range hairpinTests { + cfg := &kubeRouterConfig{} + getHairpinConfig(cfg, test.krc) + if cfg.CNIHairpin != test.result.CNIHairpin || cfg.GlobalHairpin != test.result.GlobalHairpin { + t.Fatalf("Hairpin configuration (%#v) does not match exepected output (%#v) ", cfg, test.result) + } + } +} + func TestKubeRouterDefaultManifests(t *testing.T) { k0sVars := constant.GetConfig(t.TempDir()) cfg := v1beta1.DefaultClusterConfig() @@ -94,7 +132,7 @@ func TestKubeRouterDefaultManifests(t *testing.T) { require.NoError(t, err) require.NotNil(t, ds) - assert.Contains(t, ds.Spec.Template.Spec.Containers[0].Args, "--hairpin-mode=false") + assert.Contains(t, ds.Spec.Template.Spec.Containers[0].Args, "--hairpin-mode=true") cm, err := findConfig(resources) require.NoError(t, err) @@ -104,7 +142,7 @@ func TestKubeRouterDefaultManifests(t *testing.T) { require.NoError(t, err) require.Equal(t, true, p.Dig("auto-mtu")) require.Nil(t, p.Dig("mtu")) - require.Equal(t, false, p.Dig("hairpinMode")) + require.Equal(t, true, p.Dig("hairpinMode")) } func findConfig(resources []*unstructured.Unstructured) (corev1.ConfigMap, error) { diff --git a/static/manifests/v1beta1/CustomResourceDefinition/k0s.k0sproject.io_clusterconfigs.yaml b/static/manifests/v1beta1/CustomResourceDefinition/k0s.k0sproject.io_clusterconfigs.yaml index b818de49e0ec..2229f28755e2 100644 --- a/static/manifests/v1beta1/CustomResourceDefinition/k0s.k0sproject.io_clusterconfigs.yaml +++ b/static/manifests/v1beta1/CustomResourceDefinition/k0s.k0sproject.io_clusterconfigs.yaml @@ -357,9 +357,20 @@ spec: autoMTU: description: 'Auto-detection of used MTU (default: true)' type: boolean + hairpin: + default: Enabled + description: 'Admits three values: "Enabled" enables it globally, + "Allowed" allows but services must be annotated explicitly + and "Disabled" Defaults to "Enabled"' + enum: + - Enabled + - Allowed + - Disabled + type: string hairpinMode: - description: Activate Hairpin Mode (allow a Pod behind a Service - to communicate to its own ClusterIP:Port) + description: 'DEPRECATED: Use hairpin instead. Activates Hairpin + Mode (allow a Pod behind a Service to communicate to its + own ClusterIP:Port)' type: boolean metricsPort: description: 'Kube-router metrics server port. Set to 0 to