Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add extraArgs config for kube-router #3902

Merged
merged 1 commit into from
Jan 12, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 6 additions & 4 deletions docs/configuration.md
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,7 @@ spec:
mtu: 0
peerRouterASNs: ""
peerRouterIPs: ""
extraArgs:
nodeLocalLoadBalancing:
enabled: false
envoyProxy:
Expand Down Expand Up @@ -213,15 +214,16 @@ 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). |
| `peerRouterIPs` | DEPRECATED: Use extraArgs with peerRouterIPs instead. Comma-separated list of [global peer addresses](https://github.com/cloudnativelabs/kube-router/blob/master/docs/bgp.md#global-external-bgp-peers). |
| `peerRouterASNs` | DEPRECATED: Use extraArgs with peerRouterASNs instead. 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). |
| `ipMasq` | IP masquerade for traffic originating from the pod network, and destined outside of it (default: false) |
| `ipMasq` | IP masquerade for traffic originating from the pod network, and destined outside of it (default: false) |
| `extraArgs` | Extra arguments to pass to kube-router. Can be also used to override any k0s managed args. For reference, see kube-router [documentation](https://github.com/cloudnativelabs/kube-router/blob/master/docs/user-guide.md#command-line-options). (default: empty) |

**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)).

Expand Down
5 changes: 5 additions & 0 deletions pkg/apis/k0s/v1beta1/kuberouter.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,9 +33,14 @@ type KubeRouter struct {
// IP masquerade for traffic originating from the pod network, and destined outside of it (default: false)
IPMasq bool `json:"ipMasq"`
// Comma-separated list of global peer addresses
// DEPRECATED: Use extraArgs with peerRouterASNs instead
PeerRouterASNs string `json:"peerRouterASNs"`
// Comma-separated list of global peer ASNs
// DEPRECATED: Use extraArgs with peerRouterIPs instead
PeerRouterIPs string `json:"peerRouterIPs"`
// ExtraArgs are extra arguments to pass to kube-router
// Can be also used to override the default k0s managed kube-router arguments
jnummelin marked this conversation as resolved.
Show resolved Hide resolved
ExtraArgs map[string]string `json:"extraArgs,omitempty"`
}

// +kubebuilder:validation:Enum=Enabled;Allowed;Disabled
Expand Down
9 changes: 8 additions & 1 deletion pkg/apis/k0s/v1beta1/zz_generated.deepcopy.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

71 changes: 43 additions & 28 deletions pkg/component/controller/kuberouter.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,9 @@ import (
"bytes"
"context"
"fmt"
"reflect"

"github.com/k0sproject/k0s/internal/pkg/stringmap"
"github.com/k0sproject/k0s/internal/pkg/templatewriter"
"github.com/k0sproject/k0s/pkg/apis/k0s/v1beta1"
"github.com/k0sproject/k0s/pkg/component/manager"
Expand Down Expand Up @@ -49,12 +51,12 @@ type kubeRouterConfig struct {
MetricsPort int
CNIInstallerImage string
CNIImage string
GlobalHairpin bool
CNIHairpin bool
IPMasq bool
PeerRouterIPs string
PeerRouterASNs string
PullPolicy string
Args []string
}

// NewKubeRouter creates new KubeRouter reconciler component
Expand All @@ -73,25 +75,26 @@ 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) {
func getHairpinConfig(krc *v1beta1.KubeRouter) (cniHairpin bool, globalHairpin bool) {
// 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
cniHairpin = true
globalHairpin = true
jnummelin marked this conversation as resolved.
Show resolved Hide resolved
}
case v1beta1.HairpinDisabled:
cfg.CNIHairpin = false
cfg.GlobalHairpin = false
cniHairpin = false
globalHairpin = false
case v1beta1.HairpinAllowed:
cfg.CNIHairpin = true
cfg.GlobalHairpin = false
cniHairpin = true
globalHairpin = false
case v1beta1.HairpinEnabled:
cfg.CNIHairpin = true
cfg.GlobalHairpin = true
cniHairpin = true
globalHairpin = true
}
return
}

// Reconcile detects changes in configuration and applies them to the component
Expand All @@ -106,20 +109,44 @@ func (k *KubeRouter) Reconcile(_ context.Context, clusterConfig *v1beta1.Cluster
return fmt.Errorf("cannot change CNI provider from %s to %s", existingCNI, constant.CNIProviderKubeRouter)
}

cniHairpin, globalHairpin := getHairpinConfig(clusterConfig.Spec.Network.KubeRouter)

args := stringmap.StringMap{
// k0s set default args
"run-router": "true",
"run-firewall": "true",
"run-service-proxy": "false",
"bgp-graceful-restart": "true",
// Args from config values
"auto-mtu": fmt.Sprintf("%t", clusterConfig.Spec.Network.KubeRouter.AutoMTU),
"metrics-port": fmt.Sprintf("%d", clusterConfig.Spec.Network.KubeRouter.MetricsPort),
"hairpin-mode": fmt.Sprintf("%t", globalHairpin),
}

// We should not add peering flags if the values are empty
if clusterConfig.Spec.Network.KubeRouter.PeerRouterASNs != "" {
args["peer-router-asns"] = clusterConfig.Spec.Network.KubeRouter.PeerRouterASNs
}
if clusterConfig.Spec.Network.KubeRouter.PeerRouterIPs != "" {
args["peer-router-ips"] = clusterConfig.Spec.Network.KubeRouter.PeerRouterIPs
}

// Override or add args from config
args.Merge(clusterConfig.Spec.Network.KubeRouter.ExtraArgs)

cfg := kubeRouterConfig{
AutoMTU: clusterConfig.Spec.Network.KubeRouter.AutoMTU,
MTU: clusterConfig.Spec.Network.KubeRouter.MTU,
MetricsPort: clusterConfig.Spec.Network.KubeRouter.MetricsPort,
PeerRouterIPs: clusterConfig.Spec.Network.KubeRouter.PeerRouterIPs,
PeerRouterASNs: clusterConfig.Spec.Network.KubeRouter.PeerRouterASNs,
IPMasq: clusterConfig.Spec.Network.KubeRouter.IPMasq,
CNIHairpin: cniHairpin,
CNIImage: clusterConfig.Spec.Images.KubeRouter.CNI.URI(),
CNIInstallerImage: clusterConfig.Spec.Images.KubeRouter.CNIInstaller.URI(),
PullPolicy: clusterConfig.Spec.Images.DefaultPullPolicy,
Args: args.ToDashedArgs(),
}
getHairpinConfig(&cfg, clusterConfig.Spec.Network.KubeRouter)

if cfg == k.previousConfig {
if reflect.DeepEqual(k.previousConfig, cfg) {
k.log.Info("config matches with previous, not reconciling anything")
return nil
}
Expand Down Expand Up @@ -278,20 +305,8 @@ spec:
image: {{ .CNIImage }}
imagePullPolicy: {{ .PullPolicy }}
args:
- "--run-router=true"
- "--run-firewall=true"
- "--run-service-proxy=false"
- "--bgp-graceful-restart=true"
- "--metrics-port={{ .MetricsPort }}"
- "--hairpin-mode={{ .GlobalHairpin }}"
{{- if not .AutoMTU }}
- "--auto-mtu=false"
{{- end }}
{{- if .PeerRouterIPs }}
- "--peer-router-ips={{ .PeerRouterIPs }}"
{{- end }}
{{- if .PeerRouterASNs }}
- "--peer-router-asns={{ .PeerRouterASNs }}"
{{- range .Args }}
- {{ . | printf "%q" }}
{{- end }}
env:
- name: NODE_NAME
Expand Down
72 changes: 57 additions & 15 deletions pkg/component/controller/kuberouter_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -77,40 +77,50 @@ func TestKubeRouterConfig(t *testing.T) {
}

type hairpinTest struct {
krc *v1beta1.KubeRouter
result kubeRouterConfig
krc *v1beta1.KubeRouter
resultCNIHairpin bool
resultGlobalHairpin bool
}

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: true},
resultCNIHairpin: true,
resultGlobalHairpin: true,
},
{
krc: &v1beta1.KubeRouter{Hairpin: v1beta1.HairpinUndefined, HairpinMode: false},
result: kubeRouterConfig{CNIHairpin: false, GlobalHairpin: false},
krc: &v1beta1.KubeRouter{Hairpin: v1beta1.HairpinUndefined, HairpinMode: false},
resultCNIHairpin: false,
resultGlobalHairpin: false,
},
{
krc: &v1beta1.KubeRouter{Hairpin: v1beta1.HairpinAllowed, HairpinMode: true},
result: kubeRouterConfig{CNIHairpin: true, GlobalHairpin: false},
krc: &v1beta1.KubeRouter{Hairpin: v1beta1.HairpinAllowed, HairpinMode: true},
resultCNIHairpin: true,
resultGlobalHairpin: false,
},
{
krc: &v1beta1.KubeRouter{Hairpin: v1beta1.HairpinDisabled, HairpinMode: true},
result: kubeRouterConfig{CNIHairpin: false, GlobalHairpin: false},
krc: &v1beta1.KubeRouter{Hairpin: v1beta1.HairpinDisabled, HairpinMode: true},
resultCNIHairpin: false,
resultGlobalHairpin: false,
},
{
krc: &v1beta1.KubeRouter{Hairpin: v1beta1.HairpinEnabled, HairpinMode: false},
result: kubeRouterConfig{CNIHairpin: true, GlobalHairpin: true},
krc: &v1beta1.KubeRouter{Hairpin: v1beta1.HairpinEnabled, HairpinMode: false},
resultCNIHairpin: true,
resultGlobalHairpin: 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)
cniHairpin, globalHairpin := getHairpinConfig(test.krc)
if cniHairpin != test.resultCNIHairpin {
t.Fatalf("CNI hairpin configuration (%#v) does not match exepected output (%#v) ", cfg, test.resultCNIHairpin)
}
if globalHairpin != test.resultGlobalHairpin {
t.Fatalf("Global hairpin configuration (%#v) does not match exepected output (%#v) ", cfg, test.resultGlobalHairpin)
}

}
}

Expand Down Expand Up @@ -182,6 +192,38 @@ func TestKubeRouterManualMTUManifests(t *testing.T) {
require.Equal(t, float64(1234), p.Dig("mtu"))
}

func TestExtraArgs(t *testing.T) {
k0sVars, err := config.NewCfgVars(nil, t.TempDir())
require.NoError(t, err)
cfg := v1beta1.DefaultClusterConfig()
cfg.Spec.Network.Calico = nil
cfg.Spec.Network.Provider = "kuberouter"
cfg.Spec.Network.KubeRouter = v1beta1.DefaultKubeRouter()
cfg.Spec.Network.KubeRouter.ExtraArgs = map[string]string{
// Add some random arg
"foo": "bar",
// Override the default arg
"run-firewall": "false",
}

saver := inMemorySaver{}
kr := NewKubeRouter(k0sVars, saver)
require.NoError(t, kr.Reconcile(context.Background(), cfg))
require.NoError(t, kr.Stop())

manifestData, foundRaw := saver["kube-router.yaml"]
require.True(t, foundRaw, "must have manifests for kube-router")

resources, err := testutil.ParseManifests(manifestData)
require.NoError(t, err)
ds, err := findDaemonset(resources)
require.NoError(t, err)
require.NotNil(t, ds)

assert.Contains(t, ds.Spec.Template.Spec.Containers[0].Args, "--run-firewall=false")
assert.Contains(t, ds.Spec.Template.Spec.Containers[0].Args, "--foo=bar")
}

func findConfig(resources []*unstructured.Unstructured) (corev1.ConfigMap, error) {
var cm corev1.ConfigMap
for _, r := range resources {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -428,6 +428,13 @@ spec:
autoMTU:
description: 'Auto-detection of used MTU (default: true)'
type: boolean
extraArgs:
additionalProperties:
type: string
Comment on lines +432 to +433

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is not a valid flag and causes kube-router to error if this property is defined

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure what you're referring to? The additionalProperties stuff is OpenAPI-specific and not referring to actual kube-router flags.

description: ExtraArgs are extra arguments to pass to kube-router
Can be also used to override the default k0s managed kube-router
arguments
type: object
hairpin:
default: Enabled
description: 'Admits three values: "Enabled" enables it globally,
Expand Down Expand Up @@ -456,10 +463,12 @@ spec:
false)
type: integer
peerRouterASNs:
description: Comma-separated list of global peer addresses
description: 'Comma-separated list of global peer addresses
DEPRECATED: Use extraArgs with peerRouterASNs instead'
type: string
peerRouterIPs:
description: Comma-separated list of global peer ASNs
description: 'Comma-separated list of global peer ASNs DEPRECATED:
Use extraArgs with peerRouterIPs instead'
type: string
type: object
nodeLocalLoadBalancing:
Expand Down
Loading