From 971a3f036ec628d0c5d4290ddf6528c95483341d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pawe=C5=82=20Bojanowski?= Date: Fri, 13 Dec 2024 11:40:04 +0100 Subject: [PATCH 1/4] add test case reproducing convert bug MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Paweł Bojanowski --- config/legacyconfig/migrate_test.go | 161 ++++++++++++++++++++++++++++ 1 file changed, 161 insertions(+) diff --git a/config/legacyconfig/migrate_test.go b/config/legacyconfig/migrate_test.go index 774dbbc9a..d4c9e4c5a 100644 --- a/config/legacyconfig/migrate_test.go +++ b/config/legacyconfig/migrate_test.go @@ -489,6 +489,167 @@ controlPlane: `, ExpectedErr: "migrate legacy k8s values: config is already in correct format", }, + { + Name: "statefulset affinity added", + Distro: "k8s", + In: `isolation: + # nodeProxyPermission: + # enabled: true + enabled: true + podSecurityStandard: baseline + resourceQuota: + enabled: true + quota: + count/endpoints: null + count/pods: null + count/services: null + count/configmaps: null + count/secrets: null + count/persistentvolumeclaims: null + limits.cpu: 256 + limits.memory: 1Ti + requests.storage: 10Ti + requests.ephemeral-storage: null + requests.memory: 128Gi + requests.cpu: 120 + services.loadbalancers: null + services.nodeports: null + limitRange: + enabled: true + defaultRequest: + cpu: 24m + memory: 32Mi + ephemeral-storage: null + default: + ephemeral-storage: null + memory: 2Gi + cpu: 512m + # max: + # cpu: 32 + # memory: 64Gi + # ephemeral-storage: 512Gi + networkPolicy: + enabled: false +storage: + className: px-pool +sync: + secrets: + enabled: true + nodes: + enabled: true + networkpolicies: + enabled: true + hoststorageclasses: + enabled: true +# enableHA: true +embeddedEtcd: + enabled: true +syncer: + resources: + limits: + cpu: '8' + ephemeral-storage: 8Gi + memory: 10Gi + # extraArgs: + # - '--sync-labels=namespace,aussiebb.io/,..aussiebb.io/' + replicas: 3 + labels: + aussiebb.io/profile: "true" + storage: + size: 50Gi + className: px-pool-etcd + affinity: + podAntiAffinity: + requiredDuringSchedulingIgnoredDuringExecution: + - labelSelector: + matchExpressions: + - key: app + operator: In + values: + - vcluster + topologyKey: "kubernetes.io/hostname" +coredns: + replicas: 3 + resources: + limits: + cpu: '2' + memory: '1Gi' +api: + extraArgs: + - "-v=4"`, + Expected: `controlPlane: + backingStore: + etcd: + embedded: + enabled: true + coredns: + deployment: + replicas: 3 + resources: + limits: + cpu: "2" + memory: 1Gi + distro: + k8s: + apiServer: + extraArgs: + - -v=4 + enabled: true + statefulSet: + affinity: + podAntiAffinity: + requiredDuringSchedulingIgnoredDuringExecution: + - labelSelector: + matchExpressions: + - key: app + operator: In + values: + - vcluster + topologyKey: "kubernetes.io/hostname" + highAvailability: + replicas: 3 + persistence: + volumeClaim: + size: 50Gi + storageClass: px-pool-etcd + resources: + limits: + cpu: "8" + memory: 10Gi + scheduling: + podManagementPolicy: OrderedReady +policies: + limitRange: + default: + cpu: 512m + memory: 2Gi + ephemeral-storage: null + defaultRequest: + cpu: 24m + memory: 32Mi + ephemeral-storage: null + enabled: true + podSecurityStandard: baseline + resourceQuota: + enabled: true + quota: + limits.cpu: 256 + limits.memory: 1Ti + requests.cpu: 120 + requests.memory: 128Gi + requests.storage: 10Ti + requests.ephemeral-storage: null +sync: + fromHost: + nodes: + enabled: true + storageClasses: + enabled: true + toHost: + networkPolicies: + enabled: true`, + ExpectedErr: "", + }, } for _, testCase := range testCases { From 0ebd42a04991c6e58c4fc7e6ae1f384d60777fa3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pawe=C5=82=20Bojanowski?= Date: Fri, 13 Dec 2024 13:02:13 +0100 Subject: [PATCH 2/4] fix vcluster config convert overwritting statefulSet affinity; do not prune ephemeral-storage null values MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Paweł Bojanowski --- config/diff.go | 11 ++++++- config/legacyconfig/migrate.go | 3 +- config/legacyconfig/migrate_test.go | 50 +++++++++++++++-------------- 3 files changed, 38 insertions(+), 26 deletions(-) diff --git a/config/diff.go b/config/diff.go index c879ba13a..1c548276d 100644 --- a/config/diff.go +++ b/config/diff.go @@ -12,6 +12,14 @@ import ( // ErrUnsupportedType is returned if the type is not implemented var ErrUnsupportedType = errors.New("unsupported type") +// includeNilValuesForKeys prevents removing certain keys with null values. +// This is useful when default value for a given field is not nil, user set this field for null in oldConfig, +// and we want to keep this field as null in the newConfig. +var includeNilValuesForKeys = map[string]struct{}{ + "ephemeral-storage": {}, + "requests.ephemeral-storage": {}, +} + func Diff(fromConfig *Config, toConfig *Config) (string, error) { // convert to map[string]interface{} fromRaw := map[string]interface{}{} @@ -112,7 +120,8 @@ func prune(in interface{}) interface{} { for k, v := range inType { inType[k] = prune(v) - if inType[k] == nil { + _, ok := includeNilValuesForKeys[k] + if inType[k] == nil && !ok { delete(inType, k) } } diff --git a/config/legacyconfig/migrate.go b/config/legacyconfig/migrate.go index 0c0489e75..f8cedd0ba 100644 --- a/config/legacyconfig/migrate.go +++ b/config/legacyconfig/migrate.go @@ -274,13 +274,14 @@ func convertBaseValues(oldConfig BaseHelm, newConfig *config.Config) error { } newConfig.Networking.Advanced.FallbackHostCluster = oldConfig.FallbackHostDNS + newConfig.ControlPlane.StatefulSet.Labels = oldConfig.Labels newConfig.ControlPlane.StatefulSet.Annotations = oldConfig.Annotations newConfig.ControlPlane.StatefulSet.Pods.Labels = oldConfig.PodLabels newConfig.ControlPlane.StatefulSet.Pods.Annotations = oldConfig.PodAnnotations newConfig.ControlPlane.StatefulSet.Scheduling.Tolerations = oldConfig.Tolerations newConfig.ControlPlane.StatefulSet.Scheduling.NodeSelector = oldConfig.NodeSelector - newConfig.ControlPlane.StatefulSet.Scheduling.Affinity = oldConfig.Affinity + newConfig.ControlPlane.StatefulSet.Scheduling.Affinity = mergeMaps(newConfig.ControlPlane.StatefulSet.Scheduling.Affinity, oldConfig.Affinity) newConfig.ControlPlane.StatefulSet.Scheduling.PriorityClassName = oldConfig.PriorityClassName newConfig.Networking.ReplicateServices.FromHost = oldConfig.MapServices.FromHost diff --git a/config/legacyconfig/migrate_test.go b/config/legacyconfig/migrate_test.go index d4c9e4c5a..ef92d8cc9 100644 --- a/config/legacyconfig/migrate_test.go +++ b/config/legacyconfig/migrate_test.go @@ -596,16 +596,6 @@ api: - -v=4 enabled: true statefulSet: - affinity: - podAntiAffinity: - requiredDuringSchedulingIgnoredDuringExecution: - - labelSelector: - matchExpressions: - - key: app - operator: In - values: - - vcluster - topologyKey: "kubernetes.io/hostname" highAvailability: replicas: 3 persistence: @@ -617,17 +607,27 @@ api: cpu: "8" memory: 10Gi scheduling: + affinity: + podAntiAffinity: + requiredDuringSchedulingIgnoredDuringExecution: + - labelSelector: + matchExpressions: + - key: app + operator: In + values: + - vcluster + topologyKey: kubernetes.io/hostname podManagementPolicy: OrderedReady policies: limitRange: default: cpu: 512m + ephemeral-storage: null memory: 2Gi - ephemeral-storage: null defaultRequest: cpu: 24m + ephemeral-storage: null memory: 32Mi - ephemeral-storage: null enabled: true podSecurityStandard: baseline resourceQuota: @@ -636,9 +636,9 @@ policies: limits.cpu: 256 limits.memory: 1Ti requests.cpu: 120 + requests.ephemeral-storage: null requests.memory: 128Gi requests.storage: 10Ti - requests.ephemeral-storage: null sync: fromHost: nodes: @@ -653,18 +653,20 @@ sync: } for _, testCase := range testCases { - out, err := MigrateLegacyConfig(testCase.Distro, testCase.In) - if err != nil { - if testCase.ExpectedErr != "" && testCase.ExpectedErr == err.Error() { - continue - } + t.Run(testCase.Name, func(t *testing.T) { + out, err := MigrateLegacyConfig(testCase.Distro, testCase.In) + if err != nil { + if testCase.ExpectedErr != "" && testCase.ExpectedErr == err.Error() { + return + } - t.Fatalf("Test case %s failed with: %v", testCase.Name, err) - } + t.Fatalf("Test case %s failed with: %v", testCase.Name, err) + } - if strings.TrimSpace(testCase.Expected) != strings.TrimSpace(out) { - t.Log(out) - } - assert.Equal(t, strings.TrimSpace(testCase.Expected), strings.TrimSpace(out), testCase.Name) + if strings.TrimSpace(testCase.Expected) != strings.TrimSpace(out) { + t.Log(out) + } + assert.Equal(t, strings.TrimSpace(testCase.Expected), strings.TrimSpace(out), testCase.Name) + }) } } From 9bb08def4207a6e57ea6f8bdb32d5dd0a33038dc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pawe=C5=82=20Bojanowski?= Date: Mon, 16 Dec 2024 10:28:13 +0100 Subject: [PATCH 3/4] do not prune default null values MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Paweł Bojanowski --- config/diff.go | 13 ---- config/legacyconfig/migrate_test.go | 94 +++++++++++++++++++++++++---- 2 files changed, 82 insertions(+), 25 deletions(-) diff --git a/config/diff.go b/config/diff.go index 1c548276d..82cc4c2f9 100644 --- a/config/diff.go +++ b/config/diff.go @@ -12,14 +12,6 @@ import ( // ErrUnsupportedType is returned if the type is not implemented var ErrUnsupportedType = errors.New("unsupported type") -// includeNilValuesForKeys prevents removing certain keys with null values. -// This is useful when default value for a given field is not nil, user set this field for null in oldConfig, -// and we want to keep this field as null in the newConfig. -var includeNilValuesForKeys = map[string]struct{}{ - "ephemeral-storage": {}, - "requests.ephemeral-storage": {}, -} - func Diff(fromConfig *Config, toConfig *Config) (string, error) { // convert to map[string]interface{} fromRaw := map[string]interface{}{} @@ -120,12 +112,7 @@ func prune(in interface{}) interface{} { for k, v := range inType { inType[k] = prune(v) - _, ok := includeNilValuesForKeys[k] - if inType[k] == nil && !ok { - delete(inType, k) - } } - if len(inType) == 0 { return nil } diff --git a/config/legacyconfig/migrate_test.go b/config/legacyconfig/migrate_test.go index ef92d8cc9..b00a8a532 100644 --- a/config/legacyconfig/migrate_test.go +++ b/config/legacyconfig/migrate_test.go @@ -28,7 +28,10 @@ func TestMigration(t *testing.T) { enabled: true statefulSet: scheduling: - podManagementPolicy: OrderedReady`, + podManagementPolicy: OrderedReady +external: + platform: + apiKey: null`, }, { Name: "k3s with deprecated serviceCIDR", @@ -42,7 +45,10 @@ serviceCIDR: 10.96.0.0/16 enabled: true statefulSet: scheduling: - podManagementPolicy: OrderedReady`, + podManagementPolicy: OrderedReady +external: + platform: + apiKey: null`, }, { Name: "Simple k0s", @@ -53,7 +59,10 @@ serviceCIDR: 10.96.0.0/16 enabled: true statefulSet: scheduling: - podManagementPolicy: OrderedReady`, + podManagementPolicy: OrderedReady +external: + platform: + apiKey: null`, }, { Name: "Plugin k3s", @@ -68,8 +77,14 @@ serviceCIDR: 10.96.0.0/16 statefulSet: scheduling: podManagementPolicy: OrderedReady +external: + platform: + apiKey: null plugin: test: + rbac: + clusterRole: null + role: null version: v2`, }, { @@ -89,6 +104,9 @@ plugin: statefulSet: scheduling: podManagementPolicy: OrderedReady +external: + platform: + apiKey: null sync: fromHost: ingressClasses: @@ -143,7 +161,10 @@ experimental: kind: Secret version: v1beta1 multiNamespaceMode: - enabled: true`, + enabled: true +external: + platform: + apiKey: null`, }, { Name: "persistence false", @@ -160,7 +181,10 @@ experimental: volumeClaim: enabled: false scheduling: - podManagementPolicy: OrderedReady`, + podManagementPolicy: OrderedReady +external: + platform: + apiKey: null`, }, { Name: "vcluster env", @@ -178,7 +202,10 @@ experimental: value: postgres://username:password@hostname:5432/k3s statefulSet: scheduling: - podManagementPolicy: OrderedReady`, + podManagementPolicy: OrderedReady +external: + platform: + apiKey: null`, }, { Name: "high availability", @@ -207,7 +234,10 @@ coredns: highAvailability: replicas: 3 scheduling: - podManagementPolicy: OrderedReady`, + podManagementPolicy: OrderedReady +external: + platform: + apiKey: null`, }, { Name: "fallback host dns", @@ -221,6 +251,9 @@ pro: true`, statefulSet: scheduling: podManagementPolicy: OrderedReady +external: + platform: + apiKey: null networking: advanced: fallbackHostCluster: true @@ -245,6 +278,9 @@ pro: true`, statefulSet: scheduling: podManagementPolicy: OrderedReady +external: + platform: + apiKey: null policies: limitRange: enabled: true @@ -273,6 +309,9 @@ policies: statefulSet: scheduling: podManagementPolicy: OrderedReady +external: + platform: + apiKey: null sync: fromHost: nodes: @@ -298,7 +337,9 @@ sync: statefulSet: scheduling: podManagementPolicy: OrderedReady -`, +external: + platform: + apiKey: null`, }, { Name: "embedded etcd", @@ -315,7 +356,10 @@ sync: enabled: true statefulSet: scheduling: - podManagementPolicy: OrderedReady`, + podManagementPolicy: OrderedReady +external: + platform: + apiKey: null`, }, { Name: "scheduler", @@ -337,6 +381,9 @@ sync: statefulSet: scheduling: podManagementPolicy: OrderedReady +external: + platform: + apiKey: null sync: fromHost: csiNodes: @@ -365,7 +412,10 @@ syncer: repository: loft-sh/test tag: abc scheduling: - podManagementPolicy: OrderedReady`, + podManagementPolicy: OrderedReady +external: + platform: + apiKey: null`, }, { Name: "binariesVolume", @@ -387,7 +437,10 @@ syncer: persistentVolumeClaim: claimName: my-pvc scheduling: - podManagementPolicy: OrderedReady`, + podManagementPolicy: OrderedReady +external: + platform: + apiKey: null`, ExpectedErr: "", }, { @@ -410,6 +463,9 @@ syncer: statefulSet: scheduling: podManagementPolicy: OrderedReady +external: + platform: + apiKey: null policies: limitRange: enabled: true @@ -442,7 +498,10 @@ policies: limits: memory: 10Gi scheduling: - podManagementPolicy: OrderedReady`, + podManagementPolicy: OrderedReady +external: + platform: + apiKey: null`, ExpectedErr: "", }, { @@ -618,6 +677,9 @@ api: - vcluster topologyKey: kubernetes.io/hostname podManagementPolicy: OrderedReady +external: + platform: + apiKey: null policies: limitRange: default: @@ -633,12 +695,20 @@ policies: resourceQuota: enabled: true quota: + count/configmaps: null + count/endpoints: null + count/persistentvolumeclaims: null + count/pods: null + count/secrets: null + count/services: null limits.cpu: 256 limits.memory: 1Ti requests.cpu: 120 requests.ephemeral-storage: null requests.memory: 128Gi requests.storage: 10Ti + services.loadbalancers: null + services.nodeports: null sync: fromHost: nodes: From 909d12307a361fa39dc8ea0c59b9e18200a44054 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pawe=C5=82=20Bojanowski?= Date: Wed, 18 Dec 2024 09:46:55 +0100 Subject: [PATCH 4/4] prune keys for empty maps etc. but keep them for null values MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Paweł Bojanowski --- config/diff.go | 6 ++ config/legacyconfig/migrate_test.go | 95 +++++------------------------ 2 files changed, 20 insertions(+), 81 deletions(-) diff --git a/config/diff.go b/config/diff.go index 82cc4c2f9..a38d2059b 100644 --- a/config/diff.go +++ b/config/diff.go @@ -112,10 +112,16 @@ func prune(in interface{}) interface{} { for k, v := range inType { inType[k] = prune(v) + // delete key only if original value was not nil (but for example empty map), + // otherwise we want to keep null as a value + if inType[k] == nil && v != nil { + delete(inType, k) + } } if len(inType) == 0 { return nil } + return inType default: return in diff --git a/config/legacyconfig/migrate_test.go b/config/legacyconfig/migrate_test.go index b00a8a532..307f85897 100644 --- a/config/legacyconfig/migrate_test.go +++ b/config/legacyconfig/migrate_test.go @@ -28,10 +28,7 @@ func TestMigration(t *testing.T) { enabled: true statefulSet: scheduling: - podManagementPolicy: OrderedReady -external: - platform: - apiKey: null`, + podManagementPolicy: OrderedReady`, }, { Name: "k3s with deprecated serviceCIDR", @@ -45,10 +42,7 @@ serviceCIDR: 10.96.0.0/16 enabled: true statefulSet: scheduling: - podManagementPolicy: OrderedReady -external: - platform: - apiKey: null`, + podManagementPolicy: OrderedReady`, }, { Name: "Simple k0s", @@ -59,10 +53,7 @@ external: enabled: true statefulSet: scheduling: - podManagementPolicy: OrderedReady -external: - platform: - apiKey: null`, + podManagementPolicy: OrderedReady`, }, { Name: "Plugin k3s", @@ -77,14 +68,8 @@ external: statefulSet: scheduling: podManagementPolicy: OrderedReady -external: - platform: - apiKey: null plugin: test: - rbac: - clusterRole: null - role: null version: v2`, }, { @@ -104,9 +89,6 @@ plugin: statefulSet: scheduling: podManagementPolicy: OrderedReady -external: - platform: - apiKey: null sync: fromHost: ingressClasses: @@ -161,10 +143,7 @@ experimental: kind: Secret version: v1beta1 multiNamespaceMode: - enabled: true -external: - platform: - apiKey: null`, + enabled: true`, }, { Name: "persistence false", @@ -181,10 +160,7 @@ external: volumeClaim: enabled: false scheduling: - podManagementPolicy: OrderedReady -external: - platform: - apiKey: null`, + podManagementPolicy: OrderedReady`, }, { Name: "vcluster env", @@ -202,10 +178,7 @@ external: value: postgres://username:password@hostname:5432/k3s statefulSet: scheduling: - podManagementPolicy: OrderedReady -external: - platform: - apiKey: null`, + podManagementPolicy: OrderedReady`, }, { Name: "high availability", @@ -234,10 +207,7 @@ coredns: highAvailability: replicas: 3 scheduling: - podManagementPolicy: OrderedReady -external: - platform: - apiKey: null`, + podManagementPolicy: OrderedReady`, }, { Name: "fallback host dns", @@ -251,9 +221,6 @@ pro: true`, statefulSet: scheduling: podManagementPolicy: OrderedReady -external: - platform: - apiKey: null networking: advanced: fallbackHostCluster: true @@ -278,9 +245,6 @@ pro: true`, statefulSet: scheduling: podManagementPolicy: OrderedReady -external: - platform: - apiKey: null policies: limitRange: enabled: true @@ -309,9 +273,6 @@ policies: statefulSet: scheduling: podManagementPolicy: OrderedReady -external: - platform: - apiKey: null sync: fromHost: nodes: @@ -336,10 +297,7 @@ sync: enabled: true statefulSet: scheduling: - podManagementPolicy: OrderedReady -external: - platform: - apiKey: null`, + podManagementPolicy: OrderedReady`, }, { Name: "embedded etcd", @@ -356,10 +314,7 @@ external: enabled: true statefulSet: scheduling: - podManagementPolicy: OrderedReady -external: - platform: - apiKey: null`, + podManagementPolicy: OrderedReady`, }, { Name: "scheduler", @@ -381,9 +336,6 @@ external: statefulSet: scheduling: podManagementPolicy: OrderedReady -external: - platform: - apiKey: null sync: fromHost: csiNodes: @@ -412,10 +364,7 @@ syncer: repository: loft-sh/test tag: abc scheduling: - podManagementPolicy: OrderedReady -external: - platform: - apiKey: null`, + podManagementPolicy: OrderedReady`, }, { Name: "binariesVolume", @@ -437,10 +386,7 @@ external: persistentVolumeClaim: claimName: my-pvc scheduling: - podManagementPolicy: OrderedReady -external: - platform: - apiKey: null`, + podManagementPolicy: OrderedReady`, ExpectedErr: "", }, { @@ -463,9 +409,6 @@ external: statefulSet: scheduling: podManagementPolicy: OrderedReady -external: - platform: - apiKey: null policies: limitRange: enabled: true @@ -498,10 +441,7 @@ policies: limits: memory: 10Gi scheduling: - podManagementPolicy: OrderedReady -external: - platform: - apiKey: null`, + podManagementPolicy: OrderedReady`, ExpectedErr: "", }, { @@ -522,9 +462,7 @@ controlPlane: tag: v1.30.2-k3s2 statefulSet: scheduling: - podManagementPolicy: OrderedReady - -`, + podManagementPolicy: OrderedReady`, ExpectedErr: "migrate legacy k3s values: config is already in correct format", }, { @@ -543,9 +481,7 @@ controlPlane: enabled: true statefulSet: scheduling: - podManagementPolicy: OrderedReady - -`, + podManagementPolicy: OrderedReady`, ExpectedErr: "migrate legacy k8s values: config is already in correct format", }, { @@ -677,9 +613,6 @@ api: - vcluster topologyKey: kubernetes.io/hostname podManagementPolicy: OrderedReady -external: - platform: - apiKey: null policies: limitRange: default: