From 53f8769f410b76f824ba9778c965530bc9442550 Mon Sep 17 00:00:00 2001 From: Aaron Tye Date: Tue, 3 Dec 2024 15:24:57 +0000 Subject: [PATCH 1/6] start tests --- pkg/drivers/powerflex.go | 4 ++ pkg/drivers/powerflex_test.go | 71 +++++++++++++++++++++++++++++++++++ 2 files changed, 75 insertions(+) diff --git a/pkg/drivers/powerflex.go b/pkg/drivers/powerflex.go index 84d6c7b10..f99770406 100644 --- a/pkg/drivers/powerflex.go +++ b/pkg/drivers/powerflex.go @@ -346,3 +346,7 @@ func ModifyPowerflexCR(yamlString string, cr csmv1.ContainerStorageModule, fileT } return yamlString } + +func ExtractZonesFromSecret(kube client.Client, namespace string, secret string) (map[string]string, error) { + return nil, nil +} diff --git a/pkg/drivers/powerflex_test.go b/pkg/drivers/powerflex_test.go index 19fbb2eec..57799933f 100644 --- a/pkg/drivers/powerflex_test.go +++ b/pkg/drivers/powerflex_test.go @@ -22,7 +22,9 @@ import ( "github.com/dell/csm-operator/tests/shared/crclient" "github.com/stretchr/testify/assert" corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/client/fake" ) var ( @@ -194,3 +196,72 @@ func TestModifyPowerflexCR(t *testing.T) { }) } } + +func TestExtractZonesFromSecret(t *testing.T) { + dataWithZone := ` +- username: "admin" + password: "password" + systemID: "2b11bb111111bb1b" + endpoint: "https://127.0.0.2" + skipCertificateValidation: true + mdm: "10.0.0.3,10.0.0.4" + zone: + label: topology.kubernetes.io/zone:"US-EAST" +` + dataWithoutZone := ` +- username: "admin" + password: "password" + systemID: "2b11bb111111bb1b" + endpoint: "https://127.0.0.2" + skipCertificateValidation: true + mdm: "10.0.0.3,10.0.0.4" +` + + tests := map[string]func() (client.WithWatch, map[string]string, string, bool){ + "success with zone": func() (client.WithWatch, map[string]string, string, bool) { + secret := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "vxflexos-config", + Namespace: "vxflexos", + }, + Data: map[string][]byte{ + "config": []byte(dataWithZone), + }, + } + + client := fake.NewClientBuilder().WithObjects(secret).Build() + return client, map[string]string{"topology.kubernetes.io/zone": "devops"}, "vxflexos-config", false + }, + "success no zone": func() (client.WithWatch, map[string]string, string, bool) { + secret := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "vxflexos-config", + Namespace: "vxflexos", + }, + Data: map[string][]byte{ + "config": []byte(dataWithoutZone), + }, + } + + client := fake.NewClientBuilder().WithObjects(secret).Build() + return client, map[string]string{}, "vxflexos-config", false + }, + "error getting secret": func() (client.WithWatch, map[string]string, string, bool) { + client := fake.NewClientBuilder().Build() + return client, nil, "vxflexos-not-found", true + }, + } + + for name, tc := range tests { + t.Run(name, func(t *testing.T) { + client, wantZones, secret, wantErr := tc() + zones, err := ExtractZonesFromSecret(client, "vxflexos", secret) + if wantErr { + assert.NotNil(t, err) + } else { + assert.Nil(t, err) + assert.Equal(t, wantZones, zones) + } + }) + } +} From b409d8ab56748dcb4d795592d71eea0f60b80115 Mon Sep 17 00:00:00 2001 From: Evgeny Uglov Date: Tue, 3 Dec 2024 17:39:42 +0000 Subject: [PATCH 2/6] Implement zone info extraction --- pkg/drivers/powerflex.go | 52 +++++++++++++++++++++++++++++++++-- pkg/drivers/powerflex_test.go | 23 +++++++++++++--- 2 files changed, 69 insertions(+), 6 deletions(-) diff --git a/pkg/drivers/powerflex.go b/pkg/drivers/powerflex.go index f99770406..f0279eb86 100644 --- a/pkg/drivers/powerflex.go +++ b/pkg/drivers/powerflex.go @@ -347,6 +347,54 @@ func ModifyPowerflexCR(yamlString string, cr csmv1.ContainerStorageModule, fileT return yamlString } -func ExtractZonesFromSecret(kube client.Client, namespace string, secret string) (map[string]string, error) { - return nil, nil +func ExtractZonesFromSecret(ctx context.Context, kube client.Client, namespace string, secret string) (map[string]string, error) { + log := logger.GetLogger(ctx) + + arraySecret, err := utils.GetSecret(ctx, secret, namespace, kube) + if err != nil { + return nil, fmt.Errorf("reading secret [%s] error [%s]", secret, err) + } + + type StorageArrayConfig struct { + Username string `json:"username"` + Password string `json:"password"` + SystemID string `json:"systemId"` + Endpoint string `json:"endpoint"` + SkipCertificateValidation bool `json:"skipCertificateValidation,omitempty"` + IsDefault bool `json:"isDefault,omitempty"` + MDM string `json:"mdm"` + Zone struct { + Label string `json:"label"` + } `json:"zone"` + } + + data := arraySecret.Data + configBytes := data["config"] + zonesMapData := make(map[string]string) + + if string(configBytes) != "" { + yamlConfig := make([]StorageArrayConfig, 0) + configs, err := yaml.JSONToYAML(configBytes) + if err != nil { + return nil, fmt.Errorf("unable to parse multi-array configuration[%v]", err) + } + _ = yaml.Unmarshal(configs, &yamlConfig) + + for _, configParam := range yamlConfig { + if configParam.Zone.Label != "" { + keyVal := strings.Split(configParam.Zone.Label, "=") + if len(keyVal) == 2 { + zonesMapData[keyVal[0]] = keyVal[1] + log.Infof("Zoning information configured for systemID %s: %v ", configParam.SystemID, zonesMapData) + } + } else { + log.Info("Zoning information not found in the array config. Continue with topology-unaware driver installation mode") + return zonesMapData, nil + } + } + } else { + return nil, fmt.Errorf("Array details are not provided in vxflexos-config secret") + } + + return zonesMapData, nil } diff --git a/pkg/drivers/powerflex_test.go b/pkg/drivers/powerflex_test.go index 57799933f..9a106444c 100644 --- a/pkg/drivers/powerflex_test.go +++ b/pkg/drivers/powerflex_test.go @@ -198,6 +198,7 @@ func TestModifyPowerflexCR(t *testing.T) { } func TestExtractZonesFromSecret(t *testing.T) { + emptySecret := `` dataWithZone := ` - username: "admin" password: "password" @@ -206,7 +207,7 @@ func TestExtractZonesFromSecret(t *testing.T) { skipCertificateValidation: true mdm: "10.0.0.3,10.0.0.4" zone: - label: topology.kubernetes.io/zone:"US-EAST" + label: topology.kubernetes.io/zone=US-EAST ` dataWithoutZone := ` - username: "admin" @@ -216,7 +217,7 @@ func TestExtractZonesFromSecret(t *testing.T) { skipCertificateValidation: true mdm: "10.0.0.3,10.0.0.4" ` - + ctx := context.Background() tests := map[string]func() (client.WithWatch, map[string]string, string, bool){ "success with zone": func() (client.WithWatch, map[string]string, string, bool) { secret := &corev1.Secret{ @@ -230,7 +231,7 @@ func TestExtractZonesFromSecret(t *testing.T) { } client := fake.NewClientBuilder().WithObjects(secret).Build() - return client, map[string]string{"topology.kubernetes.io/zone": "devops"}, "vxflexos-config", false + return client, map[string]string{"topology.kubernetes.io/zone": "US-EAST"}, "vxflexos-config", false }, "success no zone": func() (client.WithWatch, map[string]string, string, bool) { secret := &corev1.Secret{ @@ -250,12 +251,26 @@ func TestExtractZonesFromSecret(t *testing.T) { client := fake.NewClientBuilder().Build() return client, nil, "vxflexos-not-found", true }, + "error parsing empty secret": func() (client.WithWatch, map[string]string, string, bool) { + secret := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "vxflexos-config", + Namespace: "vxflexos", + }, + Data: map[string][]byte{ + "config": []byte(emptySecret), + }, + } + + client := fake.NewClientBuilder().WithObjects(secret).Build() + return client, nil, "vxflexos-config", true + }, } for name, tc := range tests { t.Run(name, func(t *testing.T) { client, wantZones, secret, wantErr := tc() - zones, err := ExtractZonesFromSecret(client, "vxflexos", secret) + zones, err := ExtractZonesFromSecret(ctx, client, "vxflexos", secret) if wantErr { assert.NotNil(t, err) } else { From e5393b310166f2e5a8d2d33d1a3bdab5ff5f0365 Mon Sep 17 00:00:00 2001 From: Evgeny Uglov Date: Tue, 3 Dec 2024 19:12:29 +0000 Subject: [PATCH 3/6] Add new err checks and tests --- pkg/drivers/powerflex.go | 18 ++++++++-------- pkg/drivers/powerflex_test.go | 39 +++++++++++++++++++++++++++++++++++ 2 files changed, 48 insertions(+), 9 deletions(-) diff --git a/pkg/drivers/powerflex.go b/pkg/drivers/powerflex.go index f0279eb86..948e4b3fd 100644 --- a/pkg/drivers/powerflex.go +++ b/pkg/drivers/powerflex.go @@ -356,14 +356,8 @@ func ExtractZonesFromSecret(ctx context.Context, kube client.Client, namespace s } type StorageArrayConfig struct { - Username string `json:"username"` - Password string `json:"password"` - SystemID string `json:"systemId"` - Endpoint string `json:"endpoint"` - SkipCertificateValidation bool `json:"skipCertificateValidation,omitempty"` - IsDefault bool `json:"isDefault,omitempty"` - MDM string `json:"mdm"` - Zone struct { + SystemID string `json:"systemId"` + Zone struct { Label string `json:"label"` } `json:"zone"` } @@ -378,9 +372,15 @@ func ExtractZonesFromSecret(ctx context.Context, kube client.Client, namespace s if err != nil { return nil, fmt.Errorf("unable to parse multi-array configuration[%v]", err) } - _ = yaml.Unmarshal(configs, &yamlConfig) + err = yaml.Unmarshal(configs, &yamlConfig) + if err != nil { + return nil, fmt.Errorf("unable to unmarshal multi-array configuration[%v]", err) + } for _, configParam := range yamlConfig { + if configParam.SystemID == "" { + return nil, fmt.Errorf("invalid value for SystemID") + } if configParam.Zone.Label != "" { keyVal := strings.Split(configParam.Zone.Label, "=") if len(keyVal) == 2 { diff --git a/pkg/drivers/powerflex_test.go b/pkg/drivers/powerflex_test.go index 9a106444c..cfe90fe94 100644 --- a/pkg/drivers/powerflex_test.go +++ b/pkg/drivers/powerflex_test.go @@ -199,6 +199,17 @@ func TestModifyPowerflexCR(t *testing.T) { func TestExtractZonesFromSecret(t *testing.T) { emptySecret := `` + invalidSecret := ` +- username: "admin" + - +` + secretWithNoSystemID := ` +- username: "admin" + password: "password" + endpoint: "https://127.0.0.2" + skipCertificateValidation: true + mdm: "10.0.0.3,10.0.0.4" +` dataWithZone := ` - username: "admin" password: "password" @@ -262,6 +273,34 @@ func TestExtractZonesFromSecret(t *testing.T) { }, } + client := fake.NewClientBuilder().WithObjects(secret).Build() + return client, nil, "vxflexos-config", true + }, + "error with no system id": func() (client.WithWatch, map[string]string, string, bool) { + secret := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "vxflexos-config", + Namespace: "vxflexos", + }, + Data: map[string][]byte{ + "config": []byte(secretWithNoSystemID), + }, + } + + client := fake.NewClientBuilder().WithObjects(secret).Build() + return client, nil, "vxflexos-config", true + }, + "error unmarshaling config": func() (client.WithWatch, map[string]string, string, bool) { + secret := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "vxflexos-config", + Namespace: "vxflexos", + }, + Data: map[string][]byte{ + "config": []byte(invalidSecret), + }, + } + client := fake.NewClientBuilder().WithObjects(secret).Build() return client, nil, "vxflexos-config", true }, From 2de4999edebac629c31b445840ee56f908a36650 Mon Sep 17 00:00:00 2001 From: Evgeny Uglov Date: Tue, 3 Dec 2024 19:36:45 +0000 Subject: [PATCH 4/6] Fix lint errors --- pkg/drivers/powerflex_test.go | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/pkg/drivers/powerflex_test.go b/pkg/drivers/powerflex_test.go index cfe90fe94..a2dde684b 100644 --- a/pkg/drivers/powerflex_test.go +++ b/pkg/drivers/powerflex_test.go @@ -198,12 +198,12 @@ func TestModifyPowerflexCR(t *testing.T) { } func TestExtractZonesFromSecret(t *testing.T) { - emptySecret := `` - invalidSecret := ` + emptyConfigData := `` + invalidConfigData := ` - username: "admin" - ` - secretWithNoSystemID := ` + dataWithNoSystemID := ` - username: "admin" password: "password" endpoint: "https://127.0.0.2" @@ -269,7 +269,7 @@ func TestExtractZonesFromSecret(t *testing.T) { Namespace: "vxflexos", }, Data: map[string][]byte{ - "config": []byte(emptySecret), + "config": []byte(emptyConfigData), }, } @@ -283,7 +283,7 @@ func TestExtractZonesFromSecret(t *testing.T) { Namespace: "vxflexos", }, Data: map[string][]byte{ - "config": []byte(secretWithNoSystemID), + "config": []byte(dataWithNoSystemID), }, } @@ -297,7 +297,7 @@ func TestExtractZonesFromSecret(t *testing.T) { Namespace: "vxflexos", }, Data: map[string][]byte{ - "config": []byte(invalidSecret), + "config": []byte(invalidConfigData), }, } From e73cd05b7e0c0d9b3404ce3325da6395f0a681f3 Mon Sep 17 00:00:00 2001 From: Evgeny Uglov Date: Tue, 3 Dec 2024 20:29:21 +0000 Subject: [PATCH 5/6] Add method description --- pkg/drivers/powerflex.go | 1 + 1 file changed, 1 insertion(+) diff --git a/pkg/drivers/powerflex.go b/pkg/drivers/powerflex.go index 948e4b3fd..873eb81db 100644 --- a/pkg/drivers/powerflex.go +++ b/pkg/drivers/powerflex.go @@ -347,6 +347,7 @@ func ModifyPowerflexCR(yamlString string, cr csmv1.ContainerStorageModule, fileT return yamlString } +// ExtractZonesFromSecret - Reads the array config secret and returns the zone label information func ExtractZonesFromSecret(ctx context.Context, kube client.Client, namespace string, secret string) (map[string]string, error) { log := logger.GetLogger(ctx) From 40bc0b828c02cf919a0fec1fc3fc1ec5bd06a797 Mon Sep 17 00:00:00 2001 From: Evgeny Uglov Date: Thu, 5 Dec 2024 18:26:24 +0000 Subject: [PATCH 6/6] Address comments --- pkg/drivers/powerflex.go | 10 +++++----- pkg/drivers/powerflex_test.go | 5 +++-- 2 files changed, 8 insertions(+), 7 deletions(-) diff --git a/pkg/drivers/powerflex.go b/pkg/drivers/powerflex.go index 873eb81db..56d88e4ba 100644 --- a/pkg/drivers/powerflex.go +++ b/pkg/drivers/powerflex.go @@ -359,7 +359,8 @@ func ExtractZonesFromSecret(ctx context.Context, kube client.Client, namespace s type StorageArrayConfig struct { SystemID string `json:"systemId"` Zone struct { - Label string `json:"label"` + Name string `json:"name"` + LabelKey string `json:"labelKey"` } `json:"zone"` } @@ -382,10 +383,9 @@ func ExtractZonesFromSecret(ctx context.Context, kube client.Client, namespace s if configParam.SystemID == "" { return nil, fmt.Errorf("invalid value for SystemID") } - if configParam.Zone.Label != "" { - keyVal := strings.Split(configParam.Zone.Label, "=") - if len(keyVal) == 2 { - zonesMapData[keyVal[0]] = keyVal[1] + if configParam.Zone.LabelKey != "" { + if configParam.Zone.Name != "" { + zonesMapData[configParam.Zone.LabelKey] = configParam.Zone.Name log.Infof("Zoning information configured for systemID %s: %v ", configParam.SystemID, zonesMapData) } } else { diff --git a/pkg/drivers/powerflex_test.go b/pkg/drivers/powerflex_test.go index a2dde684b..79b14ecab 100644 --- a/pkg/drivers/powerflex_test.go +++ b/pkg/drivers/powerflex_test.go @@ -218,7 +218,8 @@ func TestExtractZonesFromSecret(t *testing.T) { skipCertificateValidation: true mdm: "10.0.0.3,10.0.0.4" zone: - label: topology.kubernetes.io/zone=US-EAST + name: "US-EAST" + labelKey: "zone.csi-vxflexos.dellemc.com" ` dataWithoutZone := ` - username: "admin" @@ -242,7 +243,7 @@ func TestExtractZonesFromSecret(t *testing.T) { } client := fake.NewClientBuilder().WithObjects(secret).Build() - return client, map[string]string{"topology.kubernetes.io/zone": "US-EAST"}, "vxflexos-config", false + return client, map[string]string{"zone.csi-vxflexos.dellemc.com": "US-EAST"}, "vxflexos-config", false }, "success no zone": func() (client.WithWatch, map[string]string, string, bool) { secret := &corev1.Secret{