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

Read zone information from PowerFlex secret #810

Merged
merged 6 commits into from
Dec 5, 2024
Merged
Show file tree
Hide file tree
Changes from 5 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
53 changes: 53 additions & 0 deletions pkg/drivers/powerflex.go
Original file line number Diff line number Diff line change
Expand Up @@ -346,3 +346,56 @@ 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)

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 {
SystemID string `json:"systemId"`
Zone struct {
Label string `json:"label"`
} `json:"zone"`
Copy link
Contributor

Choose a reason for hiding this comment

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

Zones in the secret will have both the 'name' and the 'label' fields, right? Do we need to keep track of both of those?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ahh, right, confirmed with Trevor. Was confused with a different acceptance criteria:

zone:
  label: <user defined label name>=<zone name>

Fixed. Also, fixed the AC.

}

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)
}
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 != "" {
Copy link
Contributor

@ChristianAtDell ChristianAtDell Dec 5, 2024

Choose a reason for hiding this comment

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

I think that your code is accessing the secret as if it only has one field, and that field is in the format of "<labelkey>=<zone name>"? Which is what my other two comments are referring to. But I don't believe that's the structure of the proposed zoning secrets.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed

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
}
125 changes: 125 additions & 0 deletions pkg/drivers/powerflex_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 (
Expand Down Expand Up @@ -194,3 +196,126 @@ func TestModifyPowerflexCR(t *testing.T) {
})
}
}

func TestExtractZonesFromSecret(t *testing.T) {
emptyConfigData := ``
invalidConfigData := `
- username: "admin"
-
`
dataWithNoSystemID := `
- 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"
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
Copy link
Contributor

Choose a reason for hiding this comment

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

Secret should have both 'labelKey' and 'name' fields, I think-- check with @tdawe or double-check the latest proposed spec

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, you're right. Thanks! confirmed with Trevor. Was confused with a different acceptance criteria:
Fixed. Also, fixed the AC.

`
dataWithoutZone := `
- username: "admin"
password: "password"
systemID: "2b11bb111111bb1b"
endpoint: "https://127.0.0.2"
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{
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": "US-EAST"}, "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
},
"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(emptyConfigData),
},
}

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(dataWithNoSystemID),
},
}

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(invalidConfigData),
},
}

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(ctx, client, "vxflexos", secret)
if wantErr {
assert.NotNil(t, err)
} else {
assert.Nil(t, err)
assert.Equal(t, wantZones, zones)
}
})
}
}
Loading