Enable hairpin for kube-router by default

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 #1953 Co-authored-by: Tom Wieczorek <twz123@users.noreply.github.com> Signed-off-by: Juan Luis de Sousa-Valadas Castaño <jvaladas@mirantis.com>
k0sproject · Dec 15, 2022 · 9800423 · 9800423
1 parent 4af3de3
commit 9800423
Show file tree

Hide file tree

Showing 6 changed files with 109 additions and 23 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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