From 165178e27206b39c79bb8419da90f8bac07f0134 Mon Sep 17 00:00:00 2001 From: Don Khan Date: Wed, 4 Dec 2024 10:53:22 -0500 Subject: [PATCH 1/7] Implement a pre-init phase to sae node specific MDM lists. --- go.mod | 1 + go.sum | 2 + main.go | 12 + service/controller.go | 4 +- .../features/array-config/simple-config.yaml | 7 + service/preinit.go | 166 ++++++ service/preinit_test.go | 530 ++++++++++++++++++ service/service.go | 25 +- 8 files changed, 735 insertions(+), 12 deletions(-) create mode 100644 service/features/array-config/simple-config.yaml create mode 100644 service/preinit.go create mode 100644 service/preinit_test.go diff --git a/go.mod b/go.mod index 9b1a9793..7f1e2835 100644 --- a/go.mod +++ b/go.mod @@ -74,6 +74,7 @@ require ( github.com/spf13/afero v1.11.0 // indirect github.com/spf13/cast v1.7.0 // indirect github.com/spf13/pflag v1.0.5 // indirect + github.com/stretchr/objx v0.5.2 // indirect github.com/subosito/gotenv v1.6.0 // indirect github.com/x448/float16 v0.8.4 // indirect go.etcd.io/etcd/api/v3 v3.5.17 // indirect diff --git a/go.sum b/go.sum index 3c13a90e..b7c70d04 100644 --- a/go.sum +++ b/go.sum @@ -445,6 +445,8 @@ github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+ github.com/stretchr/objx v0.1.1/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME= github.com/stretchr/objx v0.4.0/go.mod h1:YvHI0jy2hoMjB+UWwv71VJQ9isScKT/TqJzVSSt89Yw= github.com/stretchr/objx v0.5.0/go.mod h1:Yh+to48EsGEfYuaHDzXPcE3xhTkx73EhmCGUpEOglKo= +github.com/stretchr/objx v0.5.2 h1:xuMeJ0Sdp5ZMRXx/aWO6RZxdr3beISkG5/G/aIRr3pY= +github.com/stretchr/objx v0.5.2/go.mod h1:FRsXN1f5AsAjCGJKqEizvkpNtU+EGNCLh3NxZ/8L+MA= github.com/stretchr/testify v1.2.2/go.mod h1:a8OnRcib4nhh0OaRAV+Yts87kKdq0PP7pXfy6kDkUVs= github.com/stretchr/testify v1.3.0/go.mod h1:M5WIy9Dh21IEIfnGCwXGc5bZfKNJtfHm1UVUgZn+9EI= github.com/stretchr/testify v1.4.0/go.mod h1:j7eGeouHqKxXV5pUuKE4zz7dFj8WfuZ+81PSLYec5m4= diff --git a/main.go b/main.go index 718b6ea7..66355ec2 100644 --- a/main.go +++ b/main.go @@ -57,6 +57,18 @@ func main() { service.DriverConfigParamsFile = *driverConfigParamsfile service.KubeConfig = *kubeconfig + // Run the service as a pre-init step. + if os.Getenv(gocsi.EnvVarMode) == "MDM-Info" { + fmt.Fprintf(os.Stdout, "PowerFlex Container Storage Interface (CSI) Plugin starting in pre-init mode.") + svc := service.NewPreInitService() + err := svc.PreInit() + if err != nil { + _, _ = fmt.Fprintf(os.Stderr, "failed to complete pre-init: %v", err) + os.Exit(1) + } + os.Exit(0) + } + run := func(ctx context.Context) { gocsi.Run(ctx, service.Name, "A PowerFlex Container Storage Interface (CSI) Plugin", usage, provider.New()) diff --git a/service/controller.go b/service/controller.go index 26a0d645..f83a78e5 100644 --- a/service/controller.go +++ b/service/controller.go @@ -828,9 +828,7 @@ func (s *service) createVolumeFromSnapshot(req *csi.CreateVolumeRequest, snapID := getVolumeIDFromCsiVolumeID(snapshotSource.SnapshotId) srcVol, err := s.getVolByID(snapID, systemID) if err != nil { - if err != nil { - return nil, status.Errorf(codes.NotFound, "Snapshot not found: %s, error: %s", snapshotSource.SnapshotId, err.Error()) - } + return nil, status.Errorf(codes.NotFound, "Snapshot not found: %s, error: %s", snapshotSource.SnapshotId, err.Error()) } // Validate the size is the same. if int64(srcVol.SizeInKb) != sizeInKbytes { diff --git a/service/features/array-config/simple-config.yaml b/service/features/array-config/simple-config.yaml new file mode 100644 index 00000000..d91a345a --- /dev/null +++ b/service/features/array-config/simple-config.yaml @@ -0,0 +1,7 @@ +- username: "user" + password: "pass" + systemID: "systemid" + endpoint: "https://192.168.1.12" + skipCertificateValidation: true + isDefault: true + mdm: "192.168.1.10,192.168.1.11" diff --git a/service/preinit.go b/service/preinit.go new file mode 100644 index 00000000..3e96117b --- /dev/null +++ b/service/preinit.go @@ -0,0 +1,166 @@ +// Copyright © 2024 Dell Inc. or its subsidiaries. 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. +// You may obtain a copy of the License at +// http://www.apache.org/licenses/LICENSE-2.0 +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// + +package service + +import ( + "fmt" + "io/fs" + "os" + "strings" + + "golang.org/x/net/context" +) + +const ( + defaultNodeMdmsFile = "/data/node_mdms.txt" +) + +type PreInitService interface { + PreInit() error +} + +func NewPreInitService() PreInitService { + return &service{} +} + +type ArrayConfigurationProvider interface { + GetArrayConfiguration() ([]*ArrayConnectionData, error) +} + +type FileWriterProvider interface { + WriteFile(filename string, data []byte, perm os.FileMode) error +} + +type DefaultArrayConfigurationProvider struct{} + +func (s *DefaultArrayConfigurationProvider) GetArrayConfiguration() ([]*ArrayConnectionData, error) { + arrayConfig, err := getArrayConfig(nil) + if err != nil { + return nil, err + } + + connectionData := make([]*ArrayConnectionData, 0) + for _, v := range arrayConfig { + connectionData = append(connectionData, v) + } + + return connectionData, nil +} + +type DefaultFileWriterProvider struct{} + +func (s *DefaultFileWriterProvider) WriteFile(filename string, data []byte, perm os.FileMode) error { + return os.WriteFile(filename, data, perm) +} + +var ( + arrayConfigurationProviderImpl ArrayConfigurationProvider = &DefaultArrayConfigurationProvider{} + fileWriterProviderImpl FileWriterProvider = &DefaultFileWriterProvider{} + nodeMdmsFile = defaultNodeMdmsFile +) + +func (s *service) PreInit() error { + Log.Infof("PreInit running") + + arrayConfig, err := arrayConfigurationProviderImpl.GetArrayConfiguration() + if err != nil { + return err + } + + labelKey, err := getLabelKey(arrayConfig) + if err != nil { + return err + } + + var mdmData string + + if labelKey == "" { + Log.Debug("No zone key found, will configure all MDMs") + sb := strings.Builder{} + for _, connectionData := range arrayConfig { + if connectionData.Mdm != "" { + if sb.Len() > 0 { + sb.WriteString(",") + } + sb.WriteString(connectionData.Mdm) + } + } + mdmData = sb.String() + } else { + Log.Infof("Zone key detected, will configure MDMs for this node, key: %s", labelKey) + nodeLabels, err := s.GetNodeLabels(context.Background()) + if err != nil { + return err + } + zone := nodeLabels[labelKey] + + if zone == "" { + return fmt.Errorf("No zone found, cannot configure this node") + } + + Log.Infof("Zone found, will configure MDMs for this node, zone: %s", zone) + mdmData, err = getMdmList(arrayConfig, labelKey, zone) + if err != nil { + return err + } + } + + Log.Infof("Saving MDM list to %s, MDM=%s", nodeMdmsFile, mdmData) + err = fileWriterProviderImpl.WriteFile(nodeMdmsFile, []byte(fmt.Sprintf("MDM=%s\n", mdmData)), fs.FileMode(0o444)) + return err +} + +// Returns a string with the comma separated list of MDM addresses given a +// key and zone. The ordering of the MDM addresses is not guaranteed. An error is +// returned if either the key or zone are empty. +func getMdmList(connectionData []*ArrayConnectionData, key, zone string) (string, error) { + if key == "" { + return "", fmt.Errorf("key is empty") + } + if zone == "" { + return "", fmt.Errorf("zone is empty") + } + + sb := &strings.Builder{} + for _, connectionData := range connectionData { + if connectionData.Mdm != "" && connectionData.Zone.LabelKey == key && connectionData.Zone.Name == zone { + if sb.Len() > 0 { + sb.WriteString(",") + } + sb.WriteString(connectionData.Mdm) + } + } + + return sb.String(), nil +} + +// Returns the label key for the given set of array configurations. +// It is expected that the value for labelKey is the same for all arrays. +// An empty string is returned if the labelKey is not present in all arrays. +// An error is returned if the key cannot be determined. +func getLabelKey(connectionData []*ArrayConnectionData) (string, error) { + if len(connectionData) == 0 { + return "", fmt.Errorf("array connection data is empty") + } + + labelKey := connectionData[0].Zone.LabelKey + + for _, v := range connectionData { + if v.Zone.LabelKey != labelKey { + return "", fmt.Errorf("zone label key is not the same for all arrays") + } + } + + return labelKey, nil +} diff --git a/service/preinit_test.go b/service/preinit_test.go new file mode 100644 index 00000000..07325cbb --- /dev/null +++ b/service/preinit_test.go @@ -0,0 +1,530 @@ +// Copyright © 2024 Dell Inc. or its subsidiaries. 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. +// You may obtain a copy of the License at +// http://www.apache.org/licenses/LICENSE-2.0 +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// + +package service + +import ( + "fmt" + "io/fs" + "os" + "reflect" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/mock" + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/client-go/kubernetes/fake" +) + +type MockArrayConfigurationProvider struct { + mock.Mock +} + +func (mock *MockArrayConfigurationProvider) GetArrayConfiguration() ([]*ArrayConnectionData, error) { + args := mock.Called() + return args.Get(0).([]*ArrayConnectionData), args.Error(1) +} + +type MockFileWriterProvider struct { + mock.Mock +} + +func (mock *MockFileWriterProvider) WriteFile(filename string, data []byte, perm os.FileMode) error { + args := mock.Called(filename, data, perm) + return args.Error(0) +} + +func TestNewPreInitService(t *testing.T) { + svc := NewPreInitService() + assert.NotNil(t, svc) +} + +func TestPreInit(t *testing.T) { + tests := []struct { + name string + connectionInfo []*ArrayConnectionData + connectionError error + nodeInfo *corev1.Node + errorExpected bool + expectedResult string + }{ + { + name: "should error on no connection info", + connectionInfo: []*ArrayConnectionData{}, + errorExpected: true, + expectedResult: "array connection data is empty", + }, + { + name: "should handle error getting connection info", + connectionInfo: nil, + connectionError: fmt.Errorf("don't care about error text"), + errorExpected: true, + expectedResult: "don't care about error text", + }, + { + name: "should error when zone labels different", + connectionInfo: []*ArrayConnectionData{ + { + Zone: ZoneInfo{ + LabelKey: "key1", + Name: "zone1", + }, + }, + { + Zone: ZoneInfo{ + LabelKey: "key2", + Name: "zone1", + }, + }, + }, + errorExpected: true, + expectedResult: "zone label key is not the same for all arrays", + }, + { + name: "should configure all MDMs when no zone label single array", + connectionInfo: []*ArrayConnectionData{ + { + Mdm: "192.168.1.1,192.168.1.2", + }, + }, + errorExpected: false, + expectedResult: "192.168.1.1,192.168.1.2", + }, + { + name: "should configure all MDMs when no zone label multi array", + connectionInfo: []*ArrayConnectionData{ + { + Mdm: "192.168.1.1,192.168.1.2", + }, + { + Mdm: "192.168.2.1,192.168.2.2", + }, + }, + errorExpected: false, + expectedResult: "192.168.1.1,192.168.1.2,192.168.2.1,192.168.2.2", + }, + { + name: "should fail if zones configured but unable to fetch node labels", + connectionInfo: []*ArrayConnectionData{ + { + Mdm: "192.168.1.1,192.168.1.2", + Zone: ZoneInfo{ + LabelKey: "key1", + Name: "zone1", + }, + }, + }, + errorExpected: true, + expectedResult: "rpc error: code = Internal desc = Unable to fetch the node labels. Error: nodes \"\" not found", + }, + { + name: "should fail if node label not found for node", + connectionInfo: []*ArrayConnectionData{ + { + Mdm: "192.168.1.1,192.168.1.2", + Zone: ZoneInfo{ + LabelKey: "key1", + Name: "zone1", + }, + }, + }, + nodeInfo: &corev1.Node{ + ObjectMeta: metav1.ObjectMeta{ + Name: "", + Labels: map[string]string{"label1": "value1", "label2": "value2"}, + }, + }, + errorExpected: true, + expectedResult: "No zone found, cannot configure this node", + }, + { + name: "should configure MDMs for only array matching zone label", + connectionInfo: []*ArrayConnectionData{ + { + Mdm: "192.168.1.1,192.168.1.2", + Zone: ZoneInfo{ + LabelKey: "key1", + Name: "zone1", + }, + }, + { + Mdm: "192.168.2.1,192.168.2.2", + Zone: ZoneInfo{ + LabelKey: "key1", + Name: "zone2", + }, + }, + }, + nodeInfo: &corev1.Node{ + ObjectMeta: metav1.ObjectMeta{ + Name: "", + Labels: map[string]string{"key1": "zone1", "label2": "value2"}, + }, + }, + errorExpected: false, + expectedResult: "192.168.1.1,192.168.1.2", + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + mockArrayConfigurationProvider := &MockArrayConfigurationProvider{} + mockFileWriterProvider := &MockFileWriterProvider{} + + arrayConfigurationProviderImpl = mockArrayConfigurationProvider + fileWriterProviderImpl = mockFileWriterProvider + + if test.nodeInfo == nil { + K8sClientset = fake.NewClientset() + } else { + K8sClientset = fake.NewClientset(test.nodeInfo) + } + + svc := NewPreInitService() + + mockArrayConfigurationProvider.On("GetArrayConfiguration").Return(test.connectionInfo, test.connectionError) + mockFileWriterProvider.On("WriteFile", mock.Anything, mock.Anything, mock.Anything).Return(nil) + + err := svc.PreInit() + if test.errorExpected { + assert.Equal(t, test.expectedResult, err.Error()) + } else { + assert.Nil(t, err) + mockFileWriterProvider.AssertCalled(t, "WriteFile", nodeMdmsFile, + []byte(fmt.Sprintf("MDM=%s\n", test.expectedResult)), fs.FileMode(0o444)) + } + }) + } +} + +// This test will use the default providers to exercise the +// production use case. +func TestPreInitWithDefaultProviders(t *testing.T) { + arrayConfigurationProviderImpl = &DefaultArrayConfigurationProvider{} + fileWriterProviderImpl = &DefaultFileWriterProvider{} + K8sClientset = fake.NewClientset(&corev1.Node{ + ObjectMeta: metav1.ObjectMeta{ + Name: "", + }, + }) + ArrayConfigFile = "features/array-config/simple-config.yaml" + nodeMdmsFile = "test/tmp/node_mdms.txt" + svc := NewPreInitService() + + _ = os.Remove(nodeMdmsFile) + err := svc.PreInit() + + assert.Nil(t, err) + assert.FileExists(t, nodeMdmsFile) +} + +func TestGetMdmList(t *testing.T) { + tests := []struct { + name string + connectionInfo []*ArrayConnectionData + key string + zone string + errorExpected bool + expectedResult string + }{ + { + name: "key is empty", + connectionInfo: []*ArrayConnectionData{}, + key: "", + zone: "testZone", + errorExpected: true, + expectedResult: "", + }, + { + name: "zone is empty", + connectionInfo: []*ArrayConnectionData{}, + key: "testKey", + zone: "", + errorExpected: true, + expectedResult: "", + }, + { + name: "key and zone are not empty with no arrays", + connectionInfo: []*ArrayConnectionData{}, + key: "testKey", + zone: "testZone", + errorExpected: false, + expectedResult: "", + }, + { + name: "no zone info and no MDMs", + connectionInfo: []*ArrayConnectionData{ + { + SystemID: "testSystemID", + }, + }, + key: "testKey", + zone: "testZone", + errorExpected: false, + expectedResult: "", + }, + { + name: "no zone info with MDMs", + connectionInfo: []*ArrayConnectionData{ + { + SystemID: "testSystemID", + Mdm: "192.168.0.10,192.168.0.20", + }, + }, + key: "testKey", + zone: "testZone", + errorExpected: false, + expectedResult: "", + }, + + { + name: "single MDM", + connectionInfo: []*ArrayConnectionData{ + { + SystemID: "testSystemID", + Mdm: "192.168.0.10", + Zone: ZoneInfo{Name: "testZone", LabelKey: "testKey"}, + }, + }, + key: "testKey", + zone: "testZone", + errorExpected: false, + expectedResult: "192.168.0.10", + }, + { + name: "two MDMs", + connectionInfo: []*ArrayConnectionData{ + { + SystemID: "testSystemID", + Mdm: "192.168.0.10,192.168.0.20", + Zone: ZoneInfo{Name: "testZone", LabelKey: "testKey"}, + }, + }, + key: "testKey", + zone: "testZone", + errorExpected: false, + expectedResult: "192.168.0.10,192.168.0.20", + }, + { + name: "two arrays", + connectionInfo: []*ArrayConnectionData{ + { + SystemID: "testSystemID1", + Mdm: "192.168.0.10,192.168.0.20", + Zone: ZoneInfo{Name: "testZone", LabelKey: "testKey"}, + }, + { + SystemID: "testSystemID2", + Mdm: "192.168.1.10,192.168.1.20", + Zone: ZoneInfo{Name: "testZone", LabelKey: "testKey"}, + }, + }, + key: "testKey", + zone: "testZone", + errorExpected: false, + expectedResult: "192.168.0.10,192.168.0.20,192.168.1.10,192.168.1.20", + }, + { + name: "two arrays with multiple zones 1", + connectionInfo: []*ArrayConnectionData{ + { + SystemID: "testSystemID1", + Mdm: "192.168.0.10,192.168.0.20", + Zone: ZoneInfo{Name: "testZone1", LabelKey: "testKey"}, + }, + { + SystemID: "testSystemID2", + Mdm: "192.168.1.10,192.168.1.20", + Zone: ZoneInfo{Name: "testZone2", LabelKey: "testKey"}, + }, + }, + key: "testKey", + zone: "testZone1", + errorExpected: false, + expectedResult: "192.168.0.10,192.168.0.20", + }, + { + name: "two arrays with multiple zones 2", + connectionInfo: []*ArrayConnectionData{ + { + SystemID: "testSystemID1", + Mdm: "192.168.0.10,192.168.0.20", + Zone: ZoneInfo{Name: "testZone1", LabelKey: "testKey"}, + }, + { + SystemID: "testSystemID2", + Mdm: "192.168.1.10,192.168.1.20", + Zone: ZoneInfo{Name: "testZone2", LabelKey: "testKey"}, + }, + }, + key: "testKey", + zone: "testZone2", + errorExpected: false, + expectedResult: "192.168.1.10,192.168.1.20", + }, + { + name: "two arrays in same zone with different keys 1", + connectionInfo: []*ArrayConnectionData{ + { + SystemID: "testSystemID1", + Mdm: "192.168.0.10,192.168.0.20", + Zone: ZoneInfo{Name: "testZone", LabelKey: "testKey1"}, + }, + { + SystemID: "testSystemID2", + Mdm: "192.168.1.10,192.168.1.20", + Zone: ZoneInfo{Name: "testZone", LabelKey: "testKey2"}, + }, + }, + key: "testKey1", + zone: "testZone", + errorExpected: false, + expectedResult: "192.168.0.10,192.168.0.20", + }, + { + name: "two arrays in same zone with different keys 2", + connectionInfo: []*ArrayConnectionData{ + { + SystemID: "testSystemID1", + Mdm: "192.168.0.10,192.168.0.20", + Zone: ZoneInfo{Name: "testZone", LabelKey: "testKey1"}, + }, + { + SystemID: "testSystemID2", + Mdm: "192.168.1.10,192.168.1.20", + Zone: ZoneInfo{Name: "testZone", LabelKey: "testKey2"}, + }, + }, + key: "testKey2", + zone: "testZone", + errorExpected: false, + expectedResult: "192.168.1.10,192.168.1.20", + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + result, err := getMdmList(test.connectionInfo, test.key, test.zone) + if (err != nil) != test.errorExpected { + t.Errorf("getMdmList() error: '%v', expected: '%v'", err, test.errorExpected) + return + } + if !reflect.DeepEqual(test.expectedResult, result) { + t.Errorf("getMdmList() = '%v', expected '%v'", result, test.expectedResult) + } + }) + } +} + +func TestGetLabelKey(t *testing.T) { + tests := []struct { + name string + connectionInfo []*ArrayConnectionData + errorExpected bool + expectedResult string + }{ + { + name: "no connection info", + connectionInfo: []*ArrayConnectionData{}, + errorExpected: true, + expectedResult: "array connection data is empty", + }, + { + name: "zone is empty", + connectionInfo: []*ArrayConnectionData{ + { + SystemID: "testSystemID1", + }, + { + SystemID: "testSystemID2", + }, + }, + errorExpected: false, + expectedResult: "", + }, + { + name: "zone is not empty with different keys", + connectionInfo: []*ArrayConnectionData{ + { + SystemID: "testSystemID1", + Zone: ZoneInfo{Name: "testZone", LabelKey: "testKey1"}, + }, + { + SystemID: "testSystemID2", + Zone: ZoneInfo{Name: "testZone", LabelKey: "testKey2"}, + }, + }, + errorExpected: true, + expectedResult: "label key is not the same for all arrays", + }, + { + name: "mix of empty and non empty zones", + connectionInfo: []*ArrayConnectionData{ + { + SystemID: "testSystemID1", + }, + { + SystemID: "testSystemID2", + Zone: ZoneInfo{Name: "testZone", LabelKey: "testKey1"}, + }, + }, + errorExpected: true, + expectedResult: "label key is not the same for all arrays", + }, + { + name: "same key in all zones", + connectionInfo: []*ArrayConnectionData{ + { + SystemID: "testSystemID1", + Zone: ZoneInfo{Name: "testZone", LabelKey: "testKey1"}, + }, + { + SystemID: "testSystemID2", + Zone: ZoneInfo{Name: "testZone", LabelKey: "testKey1"}, + }, + }, + errorExpected: false, + expectedResult: "testKey1", + }, + { + name: "case sensitivity test for key", + connectionInfo: []*ArrayConnectionData{ + { + SystemID: "testSystemID1", + Zone: ZoneInfo{Name: "testZone", LabelKey: "testKey1"}, + }, + { + SystemID: "testSystemID2", + Zone: ZoneInfo{Name: "testZone", LabelKey: "TestKey1"}, + }, + }, + errorExpected: true, + expectedResult: "label key is not the same for all arrays", + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + result, err := getLabelKey(test.connectionInfo) + if (err != nil) != test.errorExpected { + t.Errorf("getLabelKey() error: '%v', expected: '%v'", err, test.errorExpected) + return + } + + if err == nil && !reflect.DeepEqual(test.expectedResult, result) { + t.Errorf("getLabelKey() = '%v', expected '%v'", result, test.expectedResult) + } + }) + } +} diff --git a/service/service.go b/service/service.go index 30ac4c70..8c002e62 100644 --- a/service/service.go +++ b/service/service.go @@ -107,15 +107,22 @@ var Log = logrus.New() // ArrayConnectionData contains data required to connect to array type ArrayConnectionData struct { - SystemID string `json:"systemID"` - Username string `json:"username"` - Password string `json:"password"` - Endpoint string `json:"endpoint"` - SkipCertificateValidation bool `json:"skipCertificateValidation,omitempty"` - Insecure bool `json:"insecure,omitempty"` - IsDefault bool `json:"isDefault,omitempty"` - AllSystemNames string `json:"allSystemNames"` - NasName string `json:"nasName"` + SystemID string `json:"systemID"` + Username string `json:"username"` + Password string `json:"password"` + Endpoint string `json:"endpoint"` + SkipCertificateValidation bool `json:"skipCertificateValidation,omitempty"` + Insecure bool `json:"insecure,omitempty"` + IsDefault bool `json:"isDefault,omitempty"` + AllSystemNames string `json:"allSystemNames"` + NasName string `json:"nasName"` + Mdm string `json:"mdm,omitempty"` + Zone ZoneInfo `json:"zone,omitempty"` +} + +type ZoneInfo struct { + Name string `json:"name"` + LabelKey string `json:"labelKey"` } // Manifest is the SP's manifest. From 72b50176f2ea04a1f0ed06d6239326fafec8193b Mon Sep 17 00:00:00 2001 From: Don Khan Date: Mon, 9 Dec 2024 10:01:21 -0500 Subject: [PATCH 2/7] Separate array MDMs with & --- service/preinit.go | 7 ++++--- service/preinit_test.go | 25 ++++++++++++++++++++++--- 2 files changed, 26 insertions(+), 6 deletions(-) diff --git a/service/preinit.go b/service/preinit.go index 3e96117b..4a578e33 100644 --- a/service/preinit.go +++ b/service/preinit.go @@ -91,7 +91,7 @@ func (s *service) PreInit() error { for _, connectionData := range arrayConfig { if connectionData.Mdm != "" { if sb.Len() > 0 { - sb.WriteString(",") + sb.WriteString("&") } sb.WriteString(connectionData.Mdm) } @@ -122,7 +122,8 @@ func (s *service) PreInit() error { } // Returns a string with the comma separated list of MDM addresses given a -// key and zone. The ordering of the MDM addresses is not guaranteed. An error is +// key and zone. The MDMs for each array is separated by an ampersand. +// The ordering of the MDM addresses is not guaranteed. An error is // returned if either the key or zone are empty. func getMdmList(connectionData []*ArrayConnectionData, key, zone string) (string, error) { if key == "" { @@ -136,7 +137,7 @@ func getMdmList(connectionData []*ArrayConnectionData, key, zone string) (string for _, connectionData := range connectionData { if connectionData.Mdm != "" && connectionData.Zone.LabelKey == key && connectionData.Zone.Name == zone { if sb.Len() > 0 { - sb.WriteString(",") + sb.WriteString("&") } sb.WriteString(connectionData.Mdm) } diff --git a/service/preinit_test.go b/service/preinit_test.go index 07325cbb..23a9fb07 100644 --- a/service/preinit_test.go +++ b/service/preinit_test.go @@ -112,7 +112,7 @@ func TestPreInit(t *testing.T) { }, }, errorExpected: false, - expectedResult: "192.168.1.1,192.168.1.2,192.168.2.1,192.168.2.2", + expectedResult: "192.168.1.1,192.168.1.2&192.168.2.1,192.168.2.2", }, { name: "should fail if zones configured but unable to fetch node labels", @@ -219,7 +219,7 @@ func TestPreInitWithDefaultProviders(t *testing.T) { }, }) ArrayConfigFile = "features/array-config/simple-config.yaml" - nodeMdmsFile = "test/tmp/node_mdms.txt" + nodeMdmsFile = fmt.Sprintf("%s/node_mdms.txt", t.TempDir()) svc := NewPreInitService() _ = os.Remove(nodeMdmsFile) @@ -333,7 +333,26 @@ func TestGetMdmList(t *testing.T) { key: "testKey", zone: "testZone", errorExpected: false, - expectedResult: "192.168.0.10,192.168.0.20,192.168.1.10,192.168.1.20", + expectedResult: "192.168.0.10,192.168.0.20&192.168.1.10,192.168.1.20", + }, + { + name: "two arrays with one MDM each", + connectionInfo: []*ArrayConnectionData{ + { + SystemID: "testSystemID1", + Mdm: "192.168.0.10", + Zone: ZoneInfo{Name: "testZone", LabelKey: "testKey"}, + }, + { + SystemID: "testSystemID2", + Mdm: "192.168.1.10", + Zone: ZoneInfo{Name: "testZone", LabelKey: "testKey"}, + }, + }, + key: "testKey", + zone: "testZone", + errorExpected: false, + expectedResult: "192.168.0.10&192.168.1.10", }, { name: "two arrays with multiple zones 1", From 84ce0a20683f077bcb7e20516933ae392a2abdfe Mon Sep 17 00:00:00 2001 From: Don Khan Date: Mon, 9 Dec 2024 13:51:45 -0500 Subject: [PATCH 3/7] Use lowercase mfm-info --- main.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/main.go b/main.go index 66355ec2..c80a4a73 100644 --- a/main.go +++ b/main.go @@ -58,7 +58,7 @@ func main() { service.KubeConfig = *kubeconfig // Run the service as a pre-init step. - if os.Getenv(gocsi.EnvVarMode) == "MDM-Info" { + if os.Getenv(gocsi.EnvVarMode) == "mdm-info" { fmt.Fprintf(os.Stdout, "PowerFlex Container Storage Interface (CSI) Plugin starting in pre-init mode.") svc := service.NewPreInitService() err := svc.PreInit() From 9eca8c28112ff60172577c07b748bfca834f9b74 Mon Sep 17 00:00:00 2001 From: Don Khan Date: Mon, 9 Dec 2024 14:41:02 -0500 Subject: [PATCH 4/7] Address PR comments: * Log errors and return new wrapped error struct. * When the node label is missing or empty configure an empty MDM list. --- service/preinit.go | 23 +++++++++++++---------- service/preinit_test.go | 34 +++++++++++++++++++++++++++------- 2 files changed, 40 insertions(+), 17 deletions(-) diff --git a/service/preinit.go b/service/preinit.go index 4a578e33..c73d7893 100644 --- a/service/preinit.go +++ b/service/preinit.go @@ -75,12 +75,12 @@ func (s *service) PreInit() error { arrayConfig, err := arrayConfigurationProviderImpl.GetArrayConfiguration() if err != nil { - return err + return fmt.Errorf("unable to get array configuration: %v", err) } labelKey, err := getLabelKey(arrayConfig) if err != nil { - return err + return fmt.Errorf("unable to get zone label key: %v", err) } var mdmData string @@ -101,18 +101,21 @@ func (s *service) PreInit() error { Log.Infof("Zone key detected, will configure MDMs for this node, key: %s", labelKey) nodeLabels, err := s.GetNodeLabels(context.Background()) if err != nil { - return err + return fmt.Errorf("unable to get node labels: %v", err) } - zone := nodeLabels[labelKey] - if zone == "" { - return fmt.Errorf("No zone found, cannot configure this node") + zone, ok := nodeLabels[labelKey] + + if ok && zone == "" { + Log.Errorf("node key found but zone is missing, will not configure MDMs for this node, key: %s", labelKey) } - Log.Infof("Zone found, will configure MDMs for this node, zone: %s", zone) - mdmData, err = getMdmList(arrayConfig, labelKey, zone) - if err != nil { - return err + if zone != "" { + Log.Infof("zone found, will configure MDMs for this node, zone: %s", zone) + mdmData, err = getMdmList(arrayConfig, labelKey, zone) + if err != nil { + return fmt.Errorf("unable to get MDM list: %v", err) + } } } diff --git a/service/preinit_test.go b/service/preinit_test.go index 23a9fb07..c7763251 100644 --- a/service/preinit_test.go +++ b/service/preinit_test.go @@ -63,14 +63,14 @@ func TestPreInit(t *testing.T) { name: "should error on no connection info", connectionInfo: []*ArrayConnectionData{}, errorExpected: true, - expectedResult: "array connection data is empty", + expectedResult: "unable to get zone label key: array connection data is empty", }, { name: "should handle error getting connection info", connectionInfo: nil, connectionError: fmt.Errorf("don't care about error text"), errorExpected: true, - expectedResult: "don't care about error text", + expectedResult: "unable to get array configuration: don't care about error text", }, { name: "should error when zone labels different", @@ -89,7 +89,7 @@ func TestPreInit(t *testing.T) { }, }, errorExpected: true, - expectedResult: "zone label key is not the same for all arrays", + expectedResult: "unable to get zone label key: zone label key is not the same for all arrays", }, { name: "should configure all MDMs when no zone label single array", @@ -126,10 +126,10 @@ func TestPreInit(t *testing.T) { }, }, errorExpected: true, - expectedResult: "rpc error: code = Internal desc = Unable to fetch the node labels. Error: nodes \"\" not found", + expectedResult: "unable to get node labels: rpc error: code = Internal desc = Unable to fetch the node labels. Error: nodes \"\" not found", }, { - name: "should fail if node label not found for node", + name: "should configure empty MDM list if node label not found for node", connectionInfo: []*ArrayConnectionData{ { Mdm: "192.168.1.1,192.168.1.2", @@ -145,8 +145,28 @@ func TestPreInit(t *testing.T) { Labels: map[string]string{"label1": "value1", "label2": "value2"}, }, }, - errorExpected: true, - expectedResult: "No zone found, cannot configure this node", + errorExpected: false, + expectedResult: "", + }, + { + name: "should configure empty MDM list if node label present but value is nil", + connectionInfo: []*ArrayConnectionData{ + { + Mdm: "192.168.1.1,192.168.1.2", + Zone: ZoneInfo{ + LabelKey: "key1", + Name: "zone1", + }, + }, + }, + nodeInfo: &corev1.Node{ + ObjectMeta: metav1.ObjectMeta{ + Name: "", + Labels: map[string]string{"key1": "", "label2": "value2"}, + }, + }, + errorExpected: false, + expectedResult: "", }, { name: "should configure MDMs for only array matching zone label", From e7735dc16ff443926c42f1cd824781830521fdf7 Mon Sep 17 00:00:00 2001 From: Don Khan Date: Mon, 9 Dec 2024 16:05:14 -0500 Subject: [PATCH 5/7] Use escaped ampersand for multiple arrays. --- service/preinit.go | 4 ++-- service/preinit_test.go | 6 +++--- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/service/preinit.go b/service/preinit.go index c73d7893..7dafdff1 100644 --- a/service/preinit.go +++ b/service/preinit.go @@ -91,7 +91,7 @@ func (s *service) PreInit() error { for _, connectionData := range arrayConfig { if connectionData.Mdm != "" { if sb.Len() > 0 { - sb.WriteString("&") + sb.WriteString("\\&") } sb.WriteString(connectionData.Mdm) } @@ -140,7 +140,7 @@ func getMdmList(connectionData []*ArrayConnectionData, key, zone string) (string for _, connectionData := range connectionData { if connectionData.Mdm != "" && connectionData.Zone.LabelKey == key && connectionData.Zone.Name == zone { if sb.Len() > 0 { - sb.WriteString("&") + sb.WriteString("\\&") } sb.WriteString(connectionData.Mdm) } diff --git a/service/preinit_test.go b/service/preinit_test.go index c7763251..2a32e2fc 100644 --- a/service/preinit_test.go +++ b/service/preinit_test.go @@ -112,7 +112,7 @@ func TestPreInit(t *testing.T) { }, }, errorExpected: false, - expectedResult: "192.168.1.1,192.168.1.2&192.168.2.1,192.168.2.2", + expectedResult: "192.168.1.1,192.168.1.2\\&192.168.2.1,192.168.2.2", }, { name: "should fail if zones configured but unable to fetch node labels", @@ -353,7 +353,7 @@ func TestGetMdmList(t *testing.T) { key: "testKey", zone: "testZone", errorExpected: false, - expectedResult: "192.168.0.10,192.168.0.20&192.168.1.10,192.168.1.20", + expectedResult: "192.168.0.10,192.168.0.20\\&192.168.1.10,192.168.1.20", }, { name: "two arrays with one MDM each", @@ -372,7 +372,7 @@ func TestGetMdmList(t *testing.T) { key: "testKey", zone: "testZone", errorExpected: false, - expectedResult: "192.168.0.10&192.168.1.10", + expectedResult: "192.168.0.10\\&192.168.1.10", }, { name: "two arrays with multiple zones 1", From 18d9fbbb6d0aefe1bc7f557654e36a541d4b6b75 Mon Sep 17 00:00:00 2001 From: Don Khan Date: Mon, 9 Dec 2024 17:20:43 -0500 Subject: [PATCH 6/7] Fix doc string. --- service/preinit.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/service/preinit.go b/service/preinit.go index 7dafdff1..a785e049 100644 --- a/service/preinit.go +++ b/service/preinit.go @@ -124,7 +124,7 @@ func (s *service) PreInit() error { return err } -// Returns a string with the comma separated list of MDM addresses given a +// Returns a string with the list of MDM addresses given a // key and zone. The MDMs for each array is separated by an ampersand. // The ordering of the MDM addresses is not guaranteed. An error is // returned if either the key or zone are empty. From 11aea87980d696d829feb8829df11b4cbb78200f Mon Sep 17 00:00:00 2001 From: Don Khan Date: Tue, 10 Dec 2024 06:36:30 -0500 Subject: [PATCH 7/7] Use NODENAME if not set in service options. --- service/preinit_test.go | 10 ++++++---- service/service.go | 10 +++++++++- 2 files changed, 15 insertions(+), 5 deletions(-) diff --git a/service/preinit_test.go b/service/preinit_test.go index 2a32e2fc..b37322eb 100644 --- a/service/preinit_test.go +++ b/service/preinit_test.go @@ -126,7 +126,7 @@ func TestPreInit(t *testing.T) { }, }, errorExpected: true, - expectedResult: "unable to get node labels: rpc error: code = Internal desc = Unable to fetch the node labels. Error: nodes \"\" not found", + expectedResult: "unable to get node labels: rpc error: code = Internal desc = Unable to fetch the node labels. Error: nodes \"testnode\" not found", }, { name: "should configure empty MDM list if node label not found for node", @@ -141,7 +141,7 @@ func TestPreInit(t *testing.T) { }, nodeInfo: &corev1.Node{ ObjectMeta: metav1.ObjectMeta{ - Name: "", + Name: "testnode", Labels: map[string]string{"label1": "value1", "label2": "value2"}, }, }, @@ -161,7 +161,7 @@ func TestPreInit(t *testing.T) { }, nodeInfo: &corev1.Node{ ObjectMeta: metav1.ObjectMeta{ - Name: "", + Name: "testnode", Labels: map[string]string{"key1": "", "label2": "value2"}, }, }, @@ -188,7 +188,7 @@ func TestPreInit(t *testing.T) { }, nodeInfo: &corev1.Node{ ObjectMeta: metav1.ObjectMeta{ - Name: "", + Name: "testnode", Labels: map[string]string{"key1": "zone1", "label2": "value2"}, }, }, @@ -211,6 +211,8 @@ func TestPreInit(t *testing.T) { K8sClientset = fake.NewClientset(test.nodeInfo) } + t.Setenv("NODENAME", "testnode") + svc := NewPreInitService() mockArrayConfigurationProvider.On("GetArrayConfiguration").Return(test.connectionInfo, test.connectionError) diff --git a/service/service.go b/service/service.go index 8c002e62..9df53977 100644 --- a/service/service.go +++ b/service/service.go @@ -1812,8 +1812,16 @@ func (s *service) GetNodeLabels(_ context.Context) (map[string]string, error) { K8sClientset = k8sutils.Clientset } + nodeName := s.opts.KubeNodeName + if nodeName == "" { + Log.Infof("Using env variable for node name") + nodeName = os.Getenv("NODENAME") + } + + Log.Infof("Using: %s as nodeName", nodeName) + // access the API to fetch node object - node, err := K8sClientset.CoreV1().Nodes().Get(context.TODO(), s.opts.KubeNodeName, v1.GetOptions{}) + node, err := K8sClientset.CoreV1().Nodes().Get(context.TODO(), nodeName, v1.GetOptions{}) if err != nil { return nil, status.Error(codes.Internal, GetMessage("Unable to fetch the node labels. Error: %v", err)) }