Skip to content

Commit

Permalink
refactor
Browse files Browse the repository at this point in the history
  • Loading branch information
@Tamas-Biro1
Tamas-Biro1 committed Nov 9, 2023
1 parent 964719d commit b6b32bc
Show file tree
Hide file tree
Showing 11 changed files with 175 additions and 95 deletions.
11 changes: 7 additions & 4 deletions api/v1/installation_types.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -403,8 +403,11 @@ const (
WindowsDataplaneHNS WindowsDataplaneOption = "HNS"
)

// +kubebuilder:validation:Enum=net.ipv4.tcp_keepalive_intvl;net.ipv4.tcp_keepalive_probes;net.ipv4.tcp_keepalive_time
type SysctlTuningKey string
type Sysctl struct {
// +kubebuilder:validation:Enum=net.ipv4.tcp_keepalive_intvl;net.ipv4.tcp_keepalive_probes;net.ipv4.tcp_keepalive_time
Key string `json:"key"`
Value string `json:"value"`
}

// CalicoNetworkSpec specifies configuration options for Calico provided pod networking.
type CalicoNetworkSpec struct {
Expand Down Expand Up @@ -467,9 +470,9 @@ type CalicoNetworkSpec struct {
// +kubebuilder:validation:Enum=Enabled;Disabled
ContainerIPForwarding *ContainerIPForwardingType `json:"containerIPForwarding,omitempty"`

// SysctlTuning configures sysctl parameters for tuning plugin
// Sysctl configures sysctl parameters for tuning plugin
// +optional
SysctlTuning *map[SysctlTuningKey]string `json:"sysctlTuning,omitempty"`
Sysctl []Sysctl `json:"sysctl,omitempty"`
}

// NodeAddressAutodetection provides configuration options for auto-detecting node addresses. At most one option
Expand Down
29 changes: 19 additions & 10 deletions api/v1/zz_generated.deepcopy.go
View file

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

23 changes: 9 additions & 14 deletions pkg/controller/installation/validation.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -337,25 +337,20 @@ func validateCustomResource(instance *operatorv1.Installation) error {
}
}

type TuningSpec struct {
Sysctl *map[operatorv1.SysctlTuningKey]string `json:"sysctl,omitempty"`
Type string `json:"type"`
}

if instance.Spec.CalicoNetwork.SysctlTuning != nil {
if instance.Spec.CalicoNetwork.Sysctl != nil {
// CNI tuning plugin
pluginData := instance.Spec.CalicoNetwork.SysctlTuning
pluginData := instance.Spec.CalicoNetwork.Sysctl

// sysctl settings
allowedKeys := map[operatorv1.SysctlTuningKey]struct{}{
"net.ipv4.tcp_keepalive_intvl": {},
"net.ipv4.tcp_keepalive_probes": {},
"net.ipv4.tcp_keepalive_time": {},
allowedKeys := map[string]bool{
"net.ipv4.tcp_keepalive_intvl": true,
"net.ipv4.tcp_keepalive_probes": true,
"net.ipv4.tcp_keepalive_time": true,
}

for key, value := range *pluginData {
if _, allowed := allowedKeys[key]; !allowed {
return fmt.Errorf("value %s not allowed in spec.calicoNetwork.sysctlTuning", value)
for _, setting := range pluginData {
if allowedKeys[setting.Key] {
return fmt.Errorf("key %s is not allowed in spec.calicoNetwork.sysctl", setting.Key)
}
}

Expand Down
16 changes: 12 additions & 4 deletions pkg/controller/installation/validation_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -412,10 +412,18 @@ var _ = Describe("Installation validation tests", func() {
})

It("should error on not-allowed CNI sysctl tuning plugin config", func() {
instance.Spec.CalicoNetwork.SysctlTuning = &map[operator.SysctlTuningKey]string{
"net.ipv4.not_allowed_setting": "15",
"net.ipv4.tcp_keepalive_probes": "6",
"net.ipv4.tcp_keepalive_time": "40",
instance.Spec.CalicoNetwork.Sysctl = []operator.Sysctl{
{
Key: "net.ipv4.tcp_keepalive_intvl",
Value: "15",
}, {
Key: "net.ipv4.tcp_keepalive_probes",
Value: "6",
},
{
Key: "net.ipv4.tcp_keepalive_time",
Value: "40",
},
}
err := validateCustomResource(instance)
Expect(err).To(HaveOccurred())
Expand Down
22 changes: 11 additions & 11 deletions pkg/controller/migration/convert/network.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -200,8 +200,8 @@ func handleCalicoCNI(c *components, install *operatorv1.Installation) error {
}

type TuningSpec struct {
Sysctl *map[operatorv1.SysctlTuningKey]string `json:"sysctl,omitempty"`
Type string `json:"type"`
Sysctl []operatorv1.Sysctl `json:"sysctl,omitempty"`
Type string `json:"type"`
}

// CNI tuning plugin
Expand All @@ -218,20 +218,20 @@ func handleCalicoCNI(c *components, install *operatorv1.Installation) error {
}

// sysctl settings
allowedKeys := map[operatorv1.SysctlTuningKey]struct{}{
"net.ipv4.tcp_keepalive_intvl": {},
"net.ipv4.tcp_keepalive_probes": {},
"net.ipv4.tcp_keepalive_time": {},
allowedKeys := map[string]bool{
"net.ipv4.tcp_keepalive_intvl": true,
"net.ipv4.tcp_keepalive_probes": true,
"net.ipv4.tcp_keepalive_time": true,
}
sysctlTuning := make(map[operatorv1.SysctlTuningKey]string)
for key, value := range *tuningSpecData.Sysctl {
if _, allowed := allowedKeys[key]; allowed {
sysctlTuning[key] = value
sysctlTuning := []operatorv1.Sysctl{}
for _, setting := range tuningSpecData.Sysctl {
if allowedKeys[setting.Key] {
return fmt.Errorf("key %s is not allowed in spec.calicoNetwork.sysctl", setting.Key)
}
}

if len(sysctlTuning) > 0 {
install.Spec.CalicoNetwork.SysctlTuning = &sysctlTuning
install.Spec.CalicoNetwork.Sysctl = sysctlTuning
}
}

Expand Down
52 changes: 36 additions & 16 deletions pkg/controller/migration/convert/network_test.go
View file
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// Copyright (c) 2022 Tigera, Inc. All rights reserved.
// Copyright (c) 2023 Tigera, Inc. All rights reserved.

// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
Expand Down Expand Up @@ -782,11 +782,20 @@ var _ = Describe("Convert network tests", func() {
},
{
"type": "tuning",
"sysctl": {
"net.ipv4.tcp_keepalive_intvl": "15",
"net.ipv4.tcp_keepalive_probes": "6",
"net.ipv4.tcp_keepalive_time": "40"
}
"sysctl": [
{
"key": "net.ipv4.tcp_keepalive_intvl",
"value": "15"
},
{
"key": "net.ipv4.tcp_keepalive_probes",
"value": "6"
},
{
"key": "net.ipv4.tcp_keepalive_time",
"value": "40"
}
]
}
]
}`,
Expand All @@ -795,11 +804,19 @@ var _ = Describe("Convert network tests", func() {
cfg, err := Convert(ctx, c)
Expect(err).ToNot(HaveOccurred())
Expect(cfg).ToNot(BeNil())
Expect(cfg.Spec.CalicoNetwork.SysctlTuning).ToNot(BeNil())
Expect(*cfg.Spec.CalicoNetwork.SysctlTuning).To(Equal(map[string]string{
"net.ipv4.tcp_keepalive_time": "40",
"net.ipv4.tcp_keepalive_intvl": "15",
"net.ipv4.tcp_keepalive_probes": "6",
Expect(cfg.Spec.CalicoNetwork.Sysctl).ToNot(BeNil())
Expect(cfg.Spec.CalicoNetwork.Sysctl).To(Equal([]operatorv1.Sysctl{
{
Key: "net.ipv4.tcp_keepalive_intvl",
Value: "15",
}, {
Key: "net.ipv4.tcp_keepalive_probes",
Value: "6",
},
{
Key: "net.ipv4.tcp_keepalive_time",
Value: "40",
},
}))
})

Expand Down Expand Up @@ -829,9 +846,12 @@ var _ = Describe("Convert network tests", func() {
},
{
"type": "tuning",
"sysctl": {
"net.ipv4.not_allowed": "40"
}
"sysctl": [
{
"key": "net.ipv4.not_allowed",
"value": "40"
}
]
}
]
}`,
Expand All @@ -840,7 +860,7 @@ var _ = Describe("Convert network tests", func() {
cfg, err := Convert(ctx, c)
Expect(err).ToNot(HaveOccurred())
Expect(cfg).ToNot(BeNil())
Expect(cfg.Spec.CalicoNetwork.SysctlTuning).To(BeNil())
Expect(cfg.Spec.CalicoNetwork.Sysctl).To(BeNil())
})

It("not allowed sysctl tuning in config", func() {
Expand Down Expand Up @@ -874,7 +894,7 @@ var _ = Describe("Convert network tests", func() {
cfg, err := Convert(ctx, c)
Expect(err).ToNot(HaveOccurred())
Expect(cfg).ToNot(BeNil())
Expect(cfg.Spec.CalicoNetwork.SysctlTuning).To(BeNil())
Expect(cfg.Spec.CalicoNetwork.Sysctl).To(BeNil())
})
})

Expand Down
4 changes: 2 additions & 2 deletions pkg/controller/utils/merge.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -303,9 +303,9 @@ func mergeCalicoNetwork(cfg, override *operatorv1.CalicoNetworkSpec) *operatorv1
out.ContainerIPForwarding = override.ContainerIPForwarding
}

switch compareFields(out.SysctlTuning, override.SysctlTuning) {
switch compareFields(out.Sysctl, override.Sysctl) {
case BOnlySet, Different:
out.SysctlTuning = override.SysctlTuning
out.Sysctl = override.Sysctl
}
return out
}
Expand Down
26 changes: 17 additions & 9 deletions pkg/controller/utils/merge_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -479,26 +479,34 @@ var _ = Describe("Installation merge tests", func() {
}),
)

_sysctlTuningA := map[opv1.SysctlTuningKey]string{
"net.ipv4.tcp_keepalive_time": "40",
"net.ipv4.tcp_keepalive_intvl": "15",
"net.ipv4.tcp_keepalive_probes": "6",
_sysctlTuningA := []opv1.Sysctl{
{
Key: "net.ipv4.tcp_keepalive_intvl",
Value: "15",
}, {
Key: "net.ipv4.tcp_keepalive_probes",
Value: "6",
},
{
Key: "net.ipv4.tcp_keepalive_time",
Value: "40",
},
}
_sysctlTuningB := map[opv1.SysctlTuningKey]string{}
DescribeTable("merge CNI Tuning", func(main, second, expect *map[opv1.SysctlTuningKey]string) {
_sysctlTuningB := []opv1.Sysctl{}
DescribeTable("merge CNI Tuning", func(main, second, expect []opv1.Sysctl) {
m := opv1.InstallationSpec{}
s := opv1.InstallationSpec{}
if main != nil {
m.CalicoNetwork = &opv1.CalicoNetworkSpec{SysctlTuning: main}
m.CalicoNetwork = &opv1.CalicoNetworkSpec{Sysctl: main}
}
if second != nil {
s.CalicoNetwork = &opv1.CalicoNetworkSpec{SysctlTuning: second}
s.CalicoNetwork = &opv1.CalicoNetworkSpec{Sysctl: second}
}
inst := OverrideInstallationSpec(m, s)
if expect == nil {
Expect(inst.CalicoNetwork).To(BeNil())
} else {
Expect(*inst.CalicoNetwork.SysctlTuning).To(Equal(*expect))
Expect(inst.CalicoNetwork.Sysctl).To(Equal(expect))
}
},
Entry("Both unset", nil, nil, nil),
Expand Down
47 changes: 35 additions & 12 deletions pkg/crds/operator/operator.tigera.io_installations.yaml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1483,12 +1483,23 @@ spec:
on interfaces that do not match the given regex.
type: string
type: object
sysctlTuning:
additionalProperties:
type: string
description: SysctlTuning configures sysctl parameters for tuning
plugin
type: object
sysctl:
description: Sysctl configures sysctl parameters for tuning plugin
items:
properties:
key:
enum:
- net.ipv4.tcp_keepalive_intvl
- net.ipv4.tcp_keepalive_probes
- net.ipv4.tcp_keepalive_time
type: string
value:
type: string
required:
- key
- value
type: object
type: array
windowsDataplane:
description: 'WindowsDataplane is used to select the dataplane
used for Windows nodes. In particular, it causes the operator
Expand Down Expand Up @@ -10591,12 +10602,24 @@ spec:
on interfaces that do not match the given regex.
type: string
type: object
sysctlTuning:
additionalProperties:
type: string
description: SysctlTuning configures sysctl parameters for
tuning plugin
type: object
sysctl:
description: Sysctl configures sysctl parameters for tuning
plugin
items:
properties:
key:
enum:
- net.ipv4.tcp_keepalive_intvl
- net.ipv4.tcp_keepalive_probes
- net.ipv4.tcp_keepalive_time
type: string
value:
type: string
required:
- key
- value
type: object
type: array
windowsDataplane:
description: 'WindowsDataplane is used to select the dataplane
used for Windows nodes. In particular, it causes the operator
Expand Down
6 changes: 3 additions & 3 deletions pkg/render/node.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -716,9 +716,9 @@ func (c *nodeComponent) createTuningPlugin() map[string]interface{} {
// tuning plugin (sysctl)
tuningPlugin := map[string]interface{}{
"type": "tuning",
"sysctl": map[string]string{},
"sysctl": []operatorv1.Sysctl{},
}
tuningPlugin["sysctl"] = *c.cfg.Installation.CalicoNetwork.SysctlTuning
tuningPlugin["sysctl"] = c.cfg.Installation.CalicoNetwork.Sysctl

return tuningPlugin
}
Expand All @@ -742,7 +742,7 @@ func (c *nodeComponent) nodeCNIConfigMap() *corev1.ConfigMap {
}

// optional tuning plugin
if c.cfg.Installation.CalicoNetwork.SysctlTuning != nil {
if c.cfg.Installation.CalicoNetwork.Sysctl != nil {
plugins = append(plugins, c.createTuningPlugin())
}

Expand Down
Loading

0 comments on commit b6b32bc

Please sign in to comment.