From dbe9cb0436ce55988608b3596a0ce3fbea0680a0 Mon Sep 17 00:00:00 2001 From: Fernando Alfaro Campos Date: Tue, 3 Dec 2024 14:26:22 -0500 Subject: [PATCH 01/40] Add snapshot check and topology add during zone volume creation --- service/controller.go | 30 +++++++++++++++++++++++++++++- 1 file changed, 29 insertions(+), 1 deletion(-) diff --git a/service/controller.go b/service/controller.go index 25bfbf8c..c8fb84ed 100644 --- a/service/controller.go +++ b/service/controller.go @@ -221,6 +221,21 @@ func (s *service) CreateVolume( // Handle Zone topology, which happens when node is annotated with a matching zone label if len(zoneTargetMap) != 0 && accessibility != nil && len(accessibility.GetPreferred()) > 0 { + contentSource := req.GetVolumeContentSource() + var snapshotSourceSystemID string + if contentSource != nil { + Log.Infof("[CreateVolume] Volume has a content source - we are a snapshot: %+v", contentSource) + + snapshotSource := contentSource.GetSnapshot() + if snapshotSource != nil { + Log.Infof("[CreateVolume] Volume has a content source - we are a snapshot: %+v", snapshotSource) + snapshotSourceSystemID = s.getSystemIDFromCsiVolumeID(snapshotSource.SnapshotId) + Log.Infof("[CreateVolume] Snapshot source systemID: %s", snapshotSourceSystemID) + } + } else { + Log.Infoln("[CreateVolume] Volume does not have a content source - not a snapshot") + } + for _, topo := range accessibility.GetPreferred() { for topoLabel, zoneName := range topo.Segments { Log.Infof("Zoning based on label %s", s.opts.zoneLabelKey) @@ -231,6 +246,11 @@ func (s *service) CreateVolume( continue } + if snapshotSourceSystemID != "" && zoneTarget.systemID != snapshotSourceSystemID { + Log.Infof("systemID %s does not match snapshot source systemID %s", zoneTarget.systemID, snapshotSourceSystemID) + continue + } + protectionDomain = string(zoneTarget.protectionDomain) storagePool = string(zoneTarget.pool) systemID = zoneTarget.systemID @@ -558,7 +578,15 @@ func (s *service) CreateVolume( snapshotSource := contentSource.GetSnapshot() if snapshotSource != nil { Log.Printf("snapshot %s specified as volume content source", snapshotSource.SnapshotId) - return s.createVolumeFromSnapshot(req, snapshotSource, name, size, storagePool) + snapshotVolumeResponse, err := s.createVolumeFromSnapshot(req, snapshotSource, name, size, storagePool) + if err != nil { + return nil, err + } + + snapshotVolumeResponse.Volume.AccessibleTopology = volumeTopology + + Log.Printf("[FERNANDO] Snapshot Volume Response: %+v", snapshotVolumeResponse) + return snapshotVolumeResponse, nil } } From 19bcb43b42e5ded1f484e2097355e26a88f60c24 Mon Sep 17 00:00:00 2001 From: Trevor Dawe Date: Wed, 4 Dec 2024 20:58:33 +0000 Subject: [PATCH 02/40] Add topology checks for clones --- service/controller.go | 28 +++++++++++++++++++--------- 1 file changed, 19 insertions(+), 9 deletions(-) diff --git a/service/controller.go b/service/controller.go index c8fb84ed..74185b26 100644 --- a/service/controller.go +++ b/service/controller.go @@ -222,15 +222,19 @@ func (s *service) CreateVolume( // Handle Zone topology, which happens when node is annotated with a matching zone label if len(zoneTargetMap) != 0 && accessibility != nil && len(accessibility.GetPreferred()) > 0 { contentSource := req.GetVolumeContentSource() - var snapshotSourceSystemID string + var sourceSystemID string if contentSource != nil { - Log.Infof("[CreateVolume] Volume has a content source - we are a snapshot: %+v", contentSource) + Log.Infof("[CreateVolume] Volume has a content source - we are a snapshot or clone: %+v", contentSource) snapshotSource := contentSource.GetSnapshot() + cloneSource := contentSource.GetVolume() + if snapshotSource != nil { - Log.Infof("[CreateVolume] Volume has a content source - we are a snapshot: %+v", snapshotSource) - snapshotSourceSystemID = s.getSystemIDFromCsiVolumeID(snapshotSource.SnapshotId) - Log.Infof("[CreateVolume] Snapshot source systemID: %s", snapshotSourceSystemID) + sourceSystemID = s.getSystemIDFromCsiVolumeID(snapshotSource.SnapshotId) + Log.Infof("[CreateVolume] Snapshot source systemID: %s", sourceSystemID) + } else if cloneSource != nil { + sourceSystemID = s.getSystemIDFromCsiVolumeID(cloneSource.VolumeId) + Log.Infof("[CreateVolume] Clone source systemID: %s", sourceSystemID) } } else { Log.Infoln("[CreateVolume] Volume does not have a content source - not a snapshot") @@ -246,8 +250,8 @@ func (s *service) CreateVolume( continue } - if snapshotSourceSystemID != "" && zoneTarget.systemID != snapshotSourceSystemID { - Log.Infof("systemID %s does not match snapshot source systemID %s", zoneTarget.systemID, snapshotSourceSystemID) + if sourceSystemID != "" && zoneTarget.systemID != sourceSystemID { + Log.Infof("systemID %s does not match snapshot/clone source systemID %s", zoneTarget.systemID, sourceSystemID) continue } @@ -572,8 +576,14 @@ func (s *service) CreateVolume( if contentSource != nil { volumeSource := contentSource.GetVolume() if volumeSource != nil { - Log.Printf("volume %s specified as volume content source", volumeSource.VolumeId) - return s.Clone(req, volumeSource, name, size, storagePool) + cloneResponse, err := s.Clone(req, volumeSource, name, size, storagePool) + if err != nil { + return nil, err + } + + cloneResponse.Volume.AccessibleTopology = volumeTopology + + return cloneResponse, nil } snapshotSource := contentSource.GetSnapshot() if snapshotSource != nil { From 559f607f46535e8441a01670668312f1b4d8ead8 Mon Sep 17 00:00:00 2001 From: Fernando Alfaro Campos Date: Thu, 5 Dec 2024 12:00:54 -0500 Subject: [PATCH 03/40] Add zone snapshot and restore e2e test --- test/e2e/e2e.go | 139 +++++++++++++++++++-- test/e2e/features/e2e.feature | 14 +++ test/e2e/templates/zone-wait/snapshot.yaml | 51 ++++++++ 3 files changed, 197 insertions(+), 7 deletions(-) create mode 100644 test/e2e/templates/zone-wait/snapshot.yaml diff --git a/test/e2e/e2e.go b/test/e2e/e2e.go index 26611d17..15694aa3 100644 --- a/test/e2e/e2e.go +++ b/test/e2e/e2e.go @@ -20,6 +20,7 @@ import ( "fmt" "log" "os/exec" + "strconv" "strings" "time" @@ -38,11 +39,12 @@ const ( ) type feature struct { - errs []error - zoneNodeMapping map[string]string - zoneKey string - supportedZones []string - cordonedNode string + errs []error + zoneNodeMapping map[string]string + zoneKey string + supportedZones []string + cordonedNode string + zoneReplicaCount int32 } func (f *feature) aVxFlexOSService() error { @@ -214,13 +216,13 @@ func (f *feature) deleteZoneVolumes(fileLocation string) error { } func (f *feature) checkStatfulSetStatus() error { - log.Println("[checkStatfulSetStatus] checking statefulset status") - err := f.isStatefulSetReady() if err != nil { return err } + log.Println("[checkStatfulSetStatus] Statefulset and zone pods are ready") + return nil } @@ -244,6 +246,7 @@ func (f *feature) isStatefulSetReady() error { // Everything should be ready. if *sts.Spec.Replicas == sts.Status.ReadyReplicas { ready = true + f.zoneReplicaCount = sts.Status.ReadyReplicas break } @@ -339,6 +342,125 @@ func (f *feature) checkPodsForCordonRun() error { return nil } +func (f *feature) createZoneSnapshotsAndRestore(location string) error { + log.Println("[createZoneSnapshotsAndRestore] Creating snapshots and restores") + templateFile := "templates/" + location + "/snapshot.yaml" + updatedTemplateFile := "templates/" + location + "/snapshot-updated.yaml" + + for i := 0; i < int(f.zoneReplicaCount); i++ { + time.Sleep(10 * time.Second) + + copyFile := exec.Command("cp", templateFile, updatedTemplateFile) + b, err := copyFile.CombinedOutput() + if err != nil { + return fmt.Errorf("failed to copy template file: %v\nErrMessage:\n%s", err, string(b)) + } + + // Update iteration and apply... + err = replaceInFile("ITERATION", strconv.Itoa(i), updatedTemplateFile) + if err != nil { + return err + } + + createSnapshot := "kubectl apply -f " + updatedTemplateFile + _, err = execLocalCommand(createSnapshot) + if err != nil { + return err + } + } + + log.Println("[createZoneSnapshotsAndRestore] Snapshots and restores created") + + return nil +} + +func (f *feature) deleteZoneSnapshotsAndRestore(location string) error { + log.Println("[createZoneSnapshotsAndRestore] Deleting restores and snapshots") + templateFile := "templates/" + location + "/snapshot.yaml" + updatedTemplateFile := "templates/" + location + "/snapshot-updated.yaml" + + for i := 0; i < int(f.zoneReplicaCount); i++ { + time.Sleep(10 * time.Second) + + copyFile := exec.Command("cp", templateFile, updatedTemplateFile) + b, err := copyFile.CombinedOutput() + if err != nil { + return fmt.Errorf("failed to copy template file: %v\nErrMessage:\n%s", err, string(b)) + } + + // Update iteration and apply... + err = replaceInFile("ITERATION", strconv.Itoa(i), updatedTemplateFile) + if err != nil { + return err + } + + createSnapshot := "kubectl delete -f " + updatedTemplateFile + _, err = execLocalCommand(createSnapshot) + if err != nil { + return err + } + } + + log.Println("[createZoneSnapshotsAndRestore] Snapshots and restores deleted") + + return nil +} + +func (f *feature) areAllRestoresRunning() error { + log.Println("[areAllRestoresRunning] Checking if all restores are running") + + complete := false + attempts := 0 + for attempts < 15 { + getZonePods := "kubectl get pods -n " + testNamespace + " -o jsonpath='{.items}'" + result, err := execLocalCommand(getZonePods) + if err != nil { + return err + } + + pods := []v1Core.Pod{} + err = json.Unmarshal(result, &pods) + if err != nil { + return err + } + + runningCount := 0 + for _, pod := range pods { + if !strings.Contains(pod.ObjectMeta.Name, "snapshot-maz-restore") { + continue + } + + if pod.Status.Phase == "Running" { + runningCount++ + } + } + + if runningCount != int(f.zoneReplicaCount) { + time.Sleep(10 * time.Second) + continue + } else { + complete = true + break + } + } + + if !complete { + return fmt.Errorf("all restores not running, check pods status starting with snapshot-maz-restore and then try again") + } + + return nil +} + +func replaceInFile(old, new, templateFile string) error { + cmdString := "s|" + old + "|" + new + "|g" + cmd := exec.Command("sed", "-i", cmdString, templateFile) + err := cmd.Run() + if err != nil { + return fmt.Errorf("failed to substitute %s with %s in file %s: %s", old, new, templateFile, err.Error()) + } + return nil +} + func execLocalCommand(command string) ([]byte, error) { var buf bytes.Buffer cmd := exec.Command("bash", "-c", command) @@ -364,4 +486,7 @@ func InitializeScenario(s *godog.ScenarioContext) { s.Step(`^check the statefulset for zones$`, f.checkStatfulSetStatus) s.Step(`^cordon one node$`, f.cordonNode) s.Step(`^ensure pods aren't scheduled incorrectly and still running$`, f.checkPodsForCordonRun) + s.Step(`^create snapshots for zone volumes and restore in "([^"]*)"$`, f.createZoneSnapshotsAndRestore) + s.Step(`^delete snapshots for zone volumes and restore in "([^"]*)"$`, f.deleteZoneSnapshotsAndRestore) + s.Step(`^all zone restores are running$`, f.areAllRestoresRunning) } diff --git a/test/e2e/features/e2e.feature b/test/e2e/features/e2e.feature index aa73d824..cd706471 100644 --- a/test/e2e/features/e2e.feature +++ b/test/e2e/features/e2e.feature @@ -29,3 +29,17 @@ Feature: VxFlex OS CSI interface | secret | namespace | location | | "vxflexos-config" | "vxflexos" | "zone-wait" | + @zone + Scenario: Create zone voume and snapshots + Given a VxFlexOS service + And verify driver is configured and running correctly + And verify zone information from secret in namespace + Then create zone volume and pod in + And check the statefulset for zones + Then create snapshots for zone volumes and restore in + And all zone restores are running + Then delete snapshots for zone volumes and restore in + Then delete zone volume and pod in + Examples: + | secret | namespace | location | + | "vxflexos-config" | "vxflexos" | "zone-wait" | diff --git a/test/e2e/templates/zone-wait/snapshot.yaml b/test/e2e/templates/zone-wait/snapshot.yaml new file mode 100644 index 00000000..45c8be56 --- /dev/null +++ b/test/e2e/templates/zone-wait/snapshot.yaml @@ -0,0 +1,51 @@ +--- +apiVersion: snapshot.storage.k8s.io/v1 +kind: VolumeSnapshot +metadata: + name: snapshot-maz-ITERATION + namespace: vxflexos-test +spec: + volumeSnapshotClassName: vxflexos-snapclass + source: + persistentVolumeClaimName: multi-az-pvc-vxflextest-az-ITERATION +--- +apiVersion: v1 +kind: PersistentVolumeClaim +metadata: + name: snapshot-maz-pvcITERATION + namespace: vxflexos-test +spec: + dataSource: + name: snapshot-maz-ITERATION + kind: VolumeSnapshot + apiGroup: snapshot.storage.k8s.io + accessModes: + - ReadWriteOnce + resources: + requests: + storage: 8Gi +--- +apiVersion: v1 +kind: Pod +metadata: + name: snapshot-maz-restore-podITERATION + namespace: vxflexos-test +spec: + containers: + - name: busybox + image: quay.io/quay/busybox:latest + command: ["/bin/sleep", "3600"] + volumeMounts: + - mountPath: "/data0" + name: multi-az-pvc + resources: + requests: + cpu: "100m" + memory: "128Mi" + limits: + cpu: "200m" + memory: "256Mi" + volumes: + - name: multi-az-pvc + persistentVolumeClaim: + claimName: snapshot-maz-pvcITERATION From c86b87cc82c5e341a51f9bdbd8768b1318679ca9 Mon Sep 17 00:00:00 2001 From: Fernando Alfaro Campos Date: Thu, 5 Dec 2024 14:10:04 -0500 Subject: [PATCH 04/40] Address failed PR checks --- test/e2e/e2e.go | 25 +++++++++-------- test/e2e/templates/zone-wait/snapshot.yaml | 32 +++++++++++----------- 2 files changed, 29 insertions(+), 28 deletions(-) diff --git a/test/e2e/e2e.go b/test/e2e/e2e.go index 15694aa3..a4706621 100644 --- a/test/e2e/e2e.go +++ b/test/e2e/e2e.go @@ -350,8 +350,8 @@ func (f *feature) createZoneSnapshotsAndRestore(location string) error { for i := 0; i < int(f.zoneReplicaCount); i++ { time.Sleep(10 * time.Second) - copyFile := exec.Command("cp", templateFile, updatedTemplateFile) - b, err := copyFile.CombinedOutput() + cpCmd := "cp " + templateFile + " " + updatedTemplateFile + b, err := execLocalCommand(cpCmd) if err != nil { return fmt.Errorf("failed to copy template file: %v\nErrMessage:\n%s", err, string(b)) } @@ -382,8 +382,8 @@ func (f *feature) deleteZoneSnapshotsAndRestore(location string) error { for i := 0; i < int(f.zoneReplicaCount); i++ { time.Sleep(10 * time.Second) - copyFile := exec.Command("cp", templateFile, updatedTemplateFile) - b, err := copyFile.CombinedOutput() + cpCmd := "cp " + templateFile + " " + updatedTemplateFile + b, err := execLocalCommand(cpCmd) if err != nil { return fmt.Errorf("failed to copy template file: %v\nErrMessage:\n%s", err, string(b)) } @@ -438,10 +438,11 @@ func (f *feature) areAllRestoresRunning() error { if runningCount != int(f.zoneReplicaCount) { time.Sleep(10 * time.Second) continue - } else { - complete = true - break } + + complete = true + break + } if !complete { @@ -451,12 +452,12 @@ func (f *feature) areAllRestoresRunning() error { return nil } -func replaceInFile(old, new, templateFile string) error { - cmdString := "s|" + old + "|" + new + "|g" - cmd := exec.Command("sed", "-i", cmdString, templateFile) - err := cmd.Run() +func replaceInFile(oldString, newString, templateFile string) error { + cmdString := "s|" + oldString + "|" + newString + "|g" + replaceCmd := fmt.Sprintf("sed -i '%s' %s", cmdString, templateFile) + _, err := execLocalCommand(replaceCmd) if err != nil { - return fmt.Errorf("failed to substitute %s with %s in file %s: %s", old, new, templateFile, err.Error()) + return fmt.Errorf("failed to substitute %s with %s in file %s: %s", oldString, newString, templateFile, err.Error()) } return nil } diff --git a/test/e2e/templates/zone-wait/snapshot.yaml b/test/e2e/templates/zone-wait/snapshot.yaml index 45c8be56..6d325ef5 100644 --- a/test/e2e/templates/zone-wait/snapshot.yaml +++ b/test/e2e/templates/zone-wait/snapshot.yaml @@ -32,20 +32,20 @@ metadata: namespace: vxflexos-test spec: containers: - - name: busybox - image: quay.io/quay/busybox:latest - command: ["/bin/sleep", "3600"] - volumeMounts: - - mountPath: "/data0" - name: multi-az-pvc - resources: - requests: - cpu: "100m" - memory: "128Mi" - limits: - cpu: "200m" - memory: "256Mi" + - name: busybox + image: quay.io/quay/busybox:latest + command: ["/bin/sleep", "3600"] + volumeMounts: + - mountPath: "/data0" + name: multi-az-pvc + resources: + requests: + cpu: "100m" + memory: "128Mi" + limits: + cpu: "200m" + memory: "256Mi" volumes: - - name: multi-az-pvc - persistentVolumeClaim: - claimName: snapshot-maz-pvcITERATION + - name: multi-az-pvc + persistentVolumeClaim: + claimName: snapshot-maz-pvcITERATION From e22ab6250a0078043cbbbbb59b04a87bf64e0694 Mon Sep 17 00:00:00 2001 From: Fernando Alfaro Campos Date: Thu, 5 Dec 2024 14:20:37 -0500 Subject: [PATCH 05/40] Update README --- test/e2e/README.md | 1 + 1 file changed, 1 insertion(+) diff --git a/test/e2e/README.md b/test/e2e/README.md index 77416699..6225317f 100644 --- a/test/e2e/README.md +++ b/test/e2e/README.md @@ -22,3 +22,4 @@ The following tests are implemented and run during the `zone-e2e` test. 1. Creates a stateful set of 7 replicas and ensures that everything is up and ready with the zone configuration. 2. Cordons a node (marks it as unschedulable), creates 7 volumes/pods, and ensures that none gets scheduled on the cordoned node. +3. Creates a stateful set of 7 replicas, creates a snapshot and restore pod for each and ensure that they are all running. From 6ded989b27ada2d774994fb80a10b7d4f5bb910e Mon Sep 17 00:00:00 2001 From: Fernando Alfaro Campos Date: Fri, 6 Dec 2024 16:52:29 -0500 Subject: [PATCH 06/40] Add setNodePodLabel and zone probing for nodes --- service/controller.go | 40 +++++++++++++++++++++++++++++--- service/identity.go | 4 ++-- service/node.go | 2 ++ service/service.go | 49 ++++++++++++++++++++++++++++++++++++--- service/step_defs_test.go | 2 +- 5 files changed, 88 insertions(+), 9 deletions(-) diff --git a/service/controller.go b/service/controller.go index 74185b26..89bf8235 100644 --- a/service/controller.go +++ b/service/controller.go @@ -2270,13 +2270,18 @@ func (s *service) getSystemCapacity(ctx context.Context, systemID, protectionDom adminClient := s.adminClients[systemID] system := s.systems[systemID] + if adminClient == nil || system == nil { + return 0, fmt.Errorf("can't find adminClient or system by id %s", systemID) + } + + Log.Printf("[FERNANDO] adminClient: %v, system: %v", adminClient, system) var statsFunc func() (*siotypes.Statistics, error) // Default to get Capacity of system statsFunc = system.GetStatistics - if len(spName) > 0 { + if len(spName) > 0 && spName[0] != "" { // if storage pool is given, get capacity of storage pool pdID, err := s.getProtectionDomainIDFromName(systemID, protectionDomain) if err != nil { @@ -2319,6 +2324,8 @@ func (s *service) getCapacityForAllSystems(ctx context.Context, protectionDomain var systemCapacity int64 var err error + Log.Printf("[FERNANDO] Get capacity for system: %s, pool %d, protection domain %s", array.SystemID, len(spName), protectionDomain) + if len(spName) > 0 && spName[0] != "" { systemCapacity, err = s.getSystemCapacity(ctx, array.SystemID, protectionDomain, spName[0]) } else { @@ -2546,13 +2553,38 @@ func (s *service) ControllerGetCapabilities( // systemProbeAll will iterate through all arrays in service.opts.arrays and probe them. If failed, it logs // the failed system name -func (s *service) systemProbeAll(ctx context.Context) error { +func (s *service) systemProbeAll(ctx context.Context, zoneLabel string) error { // probe all arrays - Log.Infof("Probing all arrays. Number of arrays: %d", len(s.opts.arrays)) + // Log.Infof("Probing all arrays. Number of arrays: %d", len(s.opts.arrays)) + Log.Infoln("Probing all associated arrays") allArrayFail := true errMap := make(map[string]error) + zone := "" + + if zoneLabel != "" { + labels, err := GetNodeLabels(ctx, s) + if err != nil { + return err + } + + Log.Infof("Listing labels: %v", labels) + + if val, ok := labels[zoneLabel]; ok { + Log.Infof("probing zoneLabel %s, zone value: %s", zoneLabel, val) + zone = val + + } else { + return fmt.Errorf("label %s not found", zoneLabel) + } + } for _, array := range s.opts.arrays { + // If zone information is available, use it to probe the array + if array.AvailabilityZone != nil && array.AvailabilityZone.Name != ZoneName(zone) { + Log.Warnf("array %s zone %s does not match %s, not pinging this array\n", array.SystemID, array.AvailabilityZone.Name, zone) + continue + } + err := s.systemProbe(ctx, array) systemID := array.SystemID if err == nil { @@ -2612,6 +2644,8 @@ func (s *service) systemProbe(_ context.Context, array *ArrayConnectionData) err } } + Log.Printf("Login to VxFlexOS Gateway, system=%s, endpoint=%s, user=%s\n", systemID, array.Endpoint, array.Username) + if s.adminClients[systemID].GetToken() == "" { _, err := s.adminClients[systemID].Authenticate(&goscaleio.ConfigConnect{ Endpoint: array.Endpoint, diff --git a/service/identity.go b/service/identity.go index b7ec6a2f..3d733c82 100644 --- a/service/identity.go +++ b/service/identity.go @@ -80,7 +80,7 @@ func (s *service) Probe( Log.Debug("Probe called") if !strings.EqualFold(s.mode, "node") { Log.Debug("systemProbe") - if err := s.systemProbeAll(ctx); err != nil { + if err := s.systemProbeAll(ctx, ""); err != nil { Log.Printf("error in systemProbeAll: %s", err.Error()) return nil, err } @@ -107,7 +107,7 @@ func (s *service) ProbeController(ctx context.Context, ) { if !strings.EqualFold(s.mode, "node") { Log.Debug("systemProbe") - if err := s.systemProbeAll(ctx); err != nil { + if err := s.systemProbeAll(ctx, ""); err != nil { Log.Printf("error in systemProbeAll: %s", err.Error()) return nil, err } diff --git a/service/node.go b/service/node.go index b591ae33..298b898d 100644 --- a/service/node.go +++ b/service/node.go @@ -761,6 +761,8 @@ func (s *service) NodeGetInfo( if zone, ok := labels[s.opts.zoneLabelKey]; ok { topology[s.opts.zoneLabelKey] = zone + + _ = s.SetPodZoneLabel(ctx, topology) } for _, array := range s.opts.arrays { diff --git a/service/service.go b/service/service.go index 28a9e50f..c96fa864 100644 --- a/service/service.go +++ b/service/service.go @@ -666,7 +666,7 @@ func (s *service) getIPAddressByInterface(interfaceName string, networkInterface } func (s *service) checkNFS(ctx context.Context, systemID string) (bool, error) { - err := s.systemProbeAll(ctx) + err := s.systemProbeAll(ctx, s.opts.zoneLabelKey) if err != nil { return false, err } @@ -715,7 +715,7 @@ func (s *service) doProbe(ctx context.Context) error { defer px.Unlock() if !strings.EqualFold(s.mode, "node") { - if err := s.systemProbeAll(ctx); err != nil { + if err := s.systemProbeAll(ctx, ""); err != nil { return err } } @@ -723,7 +723,8 @@ func (s *service) doProbe(ctx context.Context) error { // Do a node probe if !strings.EqualFold(s.mode, "controller") { // Probe all systems managed by driver - if err := s.systemProbeAll(ctx); err != nil { + Log.Infof("[doProbe] nodeProbe - zone label: %s", s.opts.zoneLabelKey) + if err := s.systemProbeAll(ctx, s.opts.zoneLabelKey); err != nil { return err } @@ -1844,6 +1845,48 @@ func (s *service) GetNodeLabels(_ context.Context) (map[string]string, error) { return node.Labels, nil } +func (s *service) SetPodZoneLabel(_ context.Context, zoneLabel map[string]string) error { + if K8sClientset == nil { + err := k8sutils.CreateKubeClientSet() + if err != nil { + return status.Error(codes.Internal, GetMessage("init client failed with error: %v", err)) + } + K8sClientset = k8sutils.Clientset + } + + Log.Println("SetPodZoneLabel") + + // access the API to fetch node object + pods, err := K8sClientset.CoreV1().Pods(DriverNamespace).List(context.TODO(), v1.ListOptions{}) + if err != nil { + return status.Error(codes.Internal, GetMessage("Unable to fetch the node labels. Error: %v", err)) + } + + podName := "" + for _, pod := range pods.Items { + if pod.Spec.NodeName == s.opts.KubeNodeName && pod.Labels["app"] != "" { + podName = pod.Name + } + } + + pod, err := K8sClientset.CoreV1().Pods(DriverNamespace).Get(context.TODO(), podName, v1.GetOptions{}) + if err != nil { + return status.Error(codes.Internal, GetMessage("Unable to fetch the node labels. Error: %v", err)) + } + + for key, value := range zoneLabel { + Log.Printf("Setting Label: Key: %s, Value: %s for pod: %s\n", key, value, podName) + pod.Labels[key] = value + } + + _, err = K8sClientset.CoreV1().Pods(DriverNamespace).Update(context.TODO(), pod, v1.UpdateOptions{}) + if err != nil { + return status.Error(codes.Internal, GetMessage("Unable to fetch the node labels. Error: %v", err)) + } + + return nil +} + func (s *service) GetNodeUID(_ context.Context) (string, error) { if K8sClientset == nil { err := k8sutils.CreateKubeClientSet() diff --git a/service/step_defs_test.go b/service/step_defs_test.go index 3c4f7c76..64f65de0 100644 --- a/service/step_defs_test.go +++ b/service/step_defs_test.go @@ -4256,7 +4256,7 @@ func (f *feature) iUseConfig(filename string) error { return fmt.Errorf("get zone key label from secret: %s", err.Error()) } fmt.Printf("****************************************************** s.opts.arrays %v\n", f.service.opts.arrays) - f.service.systemProbeAll(context.Background()) + f.service.systemProbeAll(context.Background(), "") f.adminClient = f.service.adminClients[arrayID] if f.adminClient == nil { return fmt.Errorf("adminClient nil") From 22d00d6b0f449b530cf5d5ae6597c590fba89cd9 Mon Sep 17 00:00:00 2001 From: Fernando Alfaro Campos Date: Fri, 6 Dec 2024 16:59:52 -0500 Subject: [PATCH 07/40] rb --- service/controller.go | 3 +-- service/service.go | 1 + 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/service/controller.go b/service/controller.go index 89bf8235..83d977f4 100644 --- a/service/controller.go +++ b/service/controller.go @@ -2572,7 +2572,6 @@ func (s *service) systemProbeAll(ctx context.Context, zoneLabel string) error { if val, ok := labels[zoneLabel]; ok { Log.Infof("probing zoneLabel %s, zone value: %s", zoneLabel, val) zone = val - } else { return fmt.Errorf("label %s not found", zoneLabel) } @@ -2580,7 +2579,7 @@ func (s *service) systemProbeAll(ctx context.Context, zoneLabel string) error { for _, array := range s.opts.arrays { // If zone information is available, use it to probe the array - if array.AvailabilityZone != nil && array.AvailabilityZone.Name != ZoneName(zone) { + if strings.EqualFold(s.mode, "node") && array.AvailabilityZone != nil && array.AvailabilityZone.Name != ZoneName(zone) { Log.Warnf("array %s zone %s does not match %s, not pinging this array\n", array.SystemID, array.AvailabilityZone.Name, zone) continue } diff --git a/service/service.go b/service/service.go index c96fa864..f43b28b4 100644 --- a/service/service.go +++ b/service/service.go @@ -715,6 +715,7 @@ func (s *service) doProbe(ctx context.Context) error { defer px.Unlock() if !strings.EqualFold(s.mode, "node") { + Log.Infof("[doProbe] controllerProbe - zone label: %s", s.opts.zoneLabelKey) if err := s.systemProbeAll(ctx, ""); err != nil { return err } From ebd8f370bfb28c59d0e69b0750494b3ee4db7c3c Mon Sep 17 00:00:00 2001 From: Fernando Alfaro Campos Date: Mon, 9 Dec 2024 14:41:40 -0500 Subject: [PATCH 08/40] Update modified goscaleio --- go.mod | 7 +++---- go.sum | 13 ++++++------- 2 files changed, 9 insertions(+), 11 deletions(-) diff --git a/go.mod b/go.mod index d4733d8c..0d92b50b 100644 --- a/go.mod +++ b/go.mod @@ -6,7 +6,6 @@ module github.com/dell/csi-vxflexos/v2 go 1.23 require ( - github.com/akutz/memconn v0.1.0 github.com/apparentlymart/go-cidr v1.1.0 github.com/container-storage-interface/spec v1.6.0 github.com/cucumber/godog v0.12.1 @@ -16,14 +15,14 @@ require ( github.com/dell/dell-csi-extensions/volumeGroupSnapshot v1.7.0 github.com/dell/gocsi v1.12.0 github.com/dell/gofsutil v1.17.0 - github.com/dell/goscaleio v1.17.0 + github.com/dell/goscaleio v1.17.2-0.20241209165307-dcbadc33ab2e github.com/fsnotify/fsnotify v1.5.1 github.com/google/uuid v1.6.0 github.com/gorilla/mux v1.8.0 github.com/kubernetes-csi/csi-lib-utils v0.9.1 github.com/sirupsen/logrus v1.9.3 github.com/spf13/viper v1.10.1 - github.com/stretchr/testify v1.7.0 + github.com/stretchr/testify v1.9.0 golang.org/x/net v0.28.0 google.golang.org/grpc v1.67.1 google.golang.org/protobuf v1.34.2 @@ -73,7 +72,7 @@ require ( go.uber.org/multierr v1.6.0 // indirect go.uber.org/zap v1.17.0 // indirect golang.org/x/oauth2 v0.22.0 // indirect - golang.org/x/sys v0.25.0 // indirect + golang.org/x/sys v0.27.0 // indirect golang.org/x/term v0.23.0 // indirect golang.org/x/text v0.17.0 // indirect golang.org/x/time v0.0.0-20210723032227-1f47c861a9ac // indirect diff --git a/go.sum b/go.sum index c1280d1b..7fb2ec2d 100644 --- a/go.sum +++ b/go.sum @@ -52,8 +52,6 @@ github.com/PuerkitoBio/urlesc v0.0.0-20160726150825-5bd2802263f2/go.mod h1:uGdko github.com/PuerkitoBio/urlesc v0.0.0-20170810143723-de5bf2ad4578/go.mod h1:uGdkoq3SwY9Y+13GIhn11/XLaGBb4BfwItxLd5jeuXE= github.com/akutz/gosync v0.1.0 h1:naxPT/aDYDh79PMwM3XmencmNQeYmpNFSZy4ZE9zIW0= github.com/akutz/gosync v0.1.0/go.mod h1:I8I4aiqJI1nqaeYOOB1WS+CgRJVVPqhct9Y4njywM84= -github.com/akutz/memconn v0.1.0 h1:NawI0TORU4hcOMsMr11g7vwlCdkYeLKXBcxWu2W/P8A= -github.com/akutz/memconn v0.1.0/go.mod h1:Jo8rI7m0NieZyLI5e2CDlRdRqRRB4S7Xp77ukDjH+Fw= github.com/alecthomas/template v0.0.0-20160405071501-a0175ee3bccc/go.mod h1:LOuyumcjzFXgccqObfd/Ljyb9UuFJ6TxHnclSeseNhc= github.com/alecthomas/template v0.0.0-20190718012654-fb15b899a751/go.mod h1:LOuyumcjzFXgccqObfd/Ljyb9UuFJ6TxHnclSeseNhc= github.com/alecthomas/units v0.0.0-20151022065526-2efee857e7cf/go.mod h1:ybxpYRFXyAe+OPACYpWeL0wqObRcbAqCMya13uyzqw0= @@ -116,8 +114,8 @@ github.com/dell/gocsi v1.12.0 h1:Dn/8f2BLovo57T/aC5pP/4Eqz4h6WX8SbX+hxT5NlvQ= github.com/dell/gocsi v1.12.0/go.mod h1:hJURrmDrXDGW4xVtgi5Kx6zUsU3ht9l+nlreNx33rf0= github.com/dell/gofsutil v1.17.0 h1:QA6gUb1mz8kXNEN4eEx47OHCz8nSqZrrCnaDUYmV5EY= github.com/dell/gofsutil v1.17.0/go.mod h1:PN2hWl/pVLQiTsFR0X1x+GfhfOrfW8pGgH5xGcGMeFs= -github.com/dell/goscaleio v1.17.0 h1:x+RfTgLW6fCwVpMgKjbGPXtwioK7KO7CBNQ54E0jLl0= -github.com/dell/goscaleio v1.17.0/go.mod h1:dB1a2wXevGps25VAda+6WDp+NTUdgMZXvQVM0YOBpX8= +github.com/dell/goscaleio v1.17.2-0.20241209165307-dcbadc33ab2e h1:Y+F8YP3ceH6XRz0phFV5VpS2Pmoi8f0Vtg261/C2pZo= +github.com/dell/goscaleio v1.17.2-0.20241209165307-dcbadc33ab2e/go.mod h1:7bX3rL8JWMmdifGr/UeD/Ju9wbkHUqvKDrbdu7XyGm8= github.com/dgrijalva/jwt-go v3.2.0+incompatible/go.mod h1:E3ru+11k8xSBh+hMPgOLZmtrrCbhqsmaPHjLKYnJCaQ= github.com/dgryski/go-sip13 v0.0.0-20181026042036-e10d5fee7954/go.mod h1:vAd38F8PWV+bWy6jNmig1y/TA+kYO4g3RSRF0IAv0no= github.com/docker/spdystream v0.0.0-20160310174837-449fdfce4d96/go.mod h1:Qh8CwZgvJUkLughtfhJv5dyTYa91l1fOUCrgjqmcifM= @@ -432,8 +430,9 @@ github.com/stretchr/testify v1.2.2/go.mod h1:a8OnRcib4nhh0OaRAV+Yts87kKdq0PP7pXf github.com/stretchr/testify v1.3.0/go.mod h1:M5WIy9Dh21IEIfnGCwXGc5bZfKNJtfHm1UVUgZn+9EI= github.com/stretchr/testify v1.4.0/go.mod h1:j7eGeouHqKxXV5pUuKE4zz7dFj8WfuZ+81PSLYec5m4= github.com/stretchr/testify v1.5.1/go.mod h1:5W2xD1RspED5o8YsWQXVCued0rvSQ+mT+I5cxcmMvtA= -github.com/stretchr/testify v1.7.0 h1:nwc3DEeHmmLAfoZucVR881uASk0Mfjw8xYJ99tb5CcY= github.com/stretchr/testify v1.7.0/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg= +github.com/stretchr/testify v1.9.0 h1:HtqpIVDClZ4nwg75+f6Lvsy/wHu+3BoSGCbBAcpTsTg= +github.com/stretchr/testify v1.9.0/go.mod h1:r2ic/lqez/lEtzL7wO/rwa5dbSLXVDPFyf8C91i36aY= github.com/subosito/gotenv v1.2.0 h1:Slr1R9HxAlEKefgq5jn9U+DnETlIUa6HfgEzj0g5d7s= github.com/subosito/gotenv v1.2.0/go.mod h1:N0PQaV/YGNqwC0u51sEeR/aUtSLEXKX9iv69rRypqCw= github.com/thecodeteam/gosync v0.1.0 h1:RcD9owCaiK0Jg1rIDPgirdcLCL1jCD6XlDVSg0MfHmE= @@ -604,8 +603,8 @@ golang.org/x/sys v0.0.0-20210603081109-ebe580a85c40/go.mod h1:oPkhp1MJrh7nUepCBc golang.org/x/sys v0.0.0-20210616094352-59db8d763f22/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.0.0-20210630005230-0f9fa26af87c/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.0.0-20220715151400-c0bba94af5f8/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= -golang.org/x/sys v0.25.0 h1:r+8e+loiHxRqhXVl6ML1nO3l1+oFoWbnlu2Ehimmi34= -golang.org/x/sys v0.25.0/go.mod h1:/VUhepiaJMQUp4+oa/7Zr1D23ma6VTLIYjOOTFZPUcA= +golang.org/x/sys v0.27.0 h1:wBqf8DvsY9Y/2P8gAfPDEYNuS30J4lPHJxXSb/nJZ+s= +golang.org/x/sys v0.27.0/go.mod h1:/VUhepiaJMQUp4+oa/7Zr1D23ma6VTLIYjOOTFZPUcA= golang.org/x/term v0.0.0-20201117132131-f5c789dd3221/go.mod h1:Nr5EML6q2oocZ2LXRh80K7BxOlk5/8JxuGnuhpl+muw= golang.org/x/term v0.0.0-20201126162022-7de9c90e9dd1/go.mod h1:bj7SfCRtBDWHUb9snDiAeCFNEtKQo2Wmx5Cou7ajbmo= golang.org/x/term v0.0.0-20210220032956-6a3ed077a48d/go.mod h1:bj7SfCRtBDWHUb9snDiAeCFNEtKQo2Wmx5Cou7ajbmo= From d4897b05030d4ec9bdfb731b0afaaa8524c8afbe Mon Sep 17 00:00:00 2001 From: Fernando Alfaro Campos Date: Mon, 9 Dec 2024 14:42:24 -0500 Subject: [PATCH 09/40] Use modified Authentication in goscaleio --- service/controller.go | 6 +++--- service/identity.go | 9 ++++++--- service/service.go | 7 +++---- test/integration/step_defs_test.go | 2 +- 4 files changed, 13 insertions(+), 11 deletions(-) diff --git a/service/controller.go b/service/controller.go index 83d977f4..ad53a10e 100644 --- a/service/controller.go +++ b/service/controller.go @@ -2604,7 +2604,7 @@ func (s *service) systemProbeAll(ctx context.Context, zoneLabel string) error { } // systemProbe will probe the given array -func (s *service) systemProbe(_ context.Context, array *ArrayConnectionData) error { +func (s *service) systemProbe(ctx context.Context, array *ArrayConnectionData) error { // Check that we have the details needed to login to the Gateway if array.Endpoint == "" { return status.Error(codes.FailedPrecondition, @@ -2646,7 +2646,7 @@ func (s *service) systemProbe(_ context.Context, array *ArrayConnectionData) err Log.Printf("Login to VxFlexOS Gateway, system=%s, endpoint=%s, user=%s\n", systemID, array.Endpoint, array.Username) if s.adminClients[systemID].GetToken() == "" { - _, err := s.adminClients[systemID].Authenticate(&goscaleio.ConfigConnect{ + _, err := s.adminClients[systemID].Authenticate(ctx, &goscaleio.ConfigConnect{ Endpoint: array.Endpoint, Username: array.Username, Password: array.Password, @@ -3619,7 +3619,7 @@ func (s *service) CreateReplicationConsistencyGroupSnapshot(client *goscaleio.Cl rcg := goscaleio.NewReplicationConsistencyGroup(client) rcg.ReplicationConsistencyGroup = group - response, err := rcg.CreateReplicationConsistencyGroupSnapshot(false) + response, err := rcg.CreateReplicationConsistencyGroupSnapshot() if err != nil { return nil, err } diff --git a/service/identity.go b/service/identity.go index 3d733c82..fec0a803 100644 --- a/service/identity.go +++ b/service/identity.go @@ -16,6 +16,7 @@ package service import ( "fmt" "strings" + "time" "golang.org/x/net/context" @@ -77,10 +78,12 @@ func (s *service) Probe( _ *csi.ProbeRequest) ( *csi.ProbeResponse, error, ) { - Log.Debug("Probe called") + Log.Infof("[Probe] Probe context: %v", ctx) + newCtx, _ := context.WithTimeout(ctx, 5*time.Second) + Log.Infof("[Probe] New Probe context: %v", newCtx) if !strings.EqualFold(s.mode, "node") { - Log.Debug("systemProbe") - if err := s.systemProbeAll(ctx, ""); err != nil { + Log.Infoln("[Probe] FERNANDO we are probing the controller") + if err := s.systemProbeAll(newCtx, ""); err != nil { Log.Printf("error in systemProbeAll: %s", err.Error()) return nil, err } diff --git a/service/service.go b/service/service.go index f43b28b4..84859106 100644 --- a/service/service.go +++ b/service/service.go @@ -542,10 +542,9 @@ func (s *service) BeforeServe( // Update the ConfigMap with the Interface IPs s.updateConfigMap(s.getIPAddressByInterface, ConfigMapFilePath) - if _, ok := csictx.LookupEnv(ctx, "X_CSI_VXFLEXOS_NO_PROBE_ON_START"); !ok { - return s.doProbe(ctx) - } - return nil + return s.doProbe(ctx) + + // return nil } func (s *service) updateConfigMap(getIPAddressByInterfacefunc GetIPAddressByInterfacefunc, configFilePath string) { diff --git a/test/integration/step_defs_test.go b/test/integration/step_defs_test.go index 8463b6e2..fb2892c3 100644 --- a/test/integration/step_defs_test.go +++ b/test/integration/step_defs_test.go @@ -114,7 +114,7 @@ func (f *feature) getGoscaleioClient() (client *goscaleio.Client, err error) { if err != nil { log.Fatalf("err getting client: %v", err) } - _, err = client.Authenticate(&goscaleio.ConfigConnect{ + _, err = client.Authenticate(context.Background(), &goscaleio.ConfigConnect{ Username: a.Username, Password: a.Password, Endpoint: a.Endpoint, From 0e71a3566ffc38f72764a09513901720cab1ad5d Mon Sep 17 00:00:00 2001 From: "Lau, Luke" Date: Wed, 11 Dec 2024 11:54:36 -0500 Subject: [PATCH 10/40] set timeout for probe calls with long response time --- service/controller.go | 6 +++++- service/identity.go | 6 ++---- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/service/controller.go b/service/controller.go index ad53a10e..2fee83d2 100644 --- a/service/controller.go +++ b/service/controller.go @@ -2584,7 +2584,11 @@ func (s *service) systemProbeAll(ctx context.Context, zoneLabel string) error { continue } - err := s.systemProbe(ctx, array) + // Expect responses from Pflex within 60s + subCtx, cancel := context.WithTimeout(ctx, time.Second*60) + err := s.systemProbe(subCtx, array) + cancel() + systemID := array.SystemID if err == nil { Log.Infof("array %s probed successfully", systemID) diff --git a/service/identity.go b/service/identity.go index fec0a803..543f47be 100644 --- a/service/identity.go +++ b/service/identity.go @@ -16,7 +16,6 @@ package service import ( "fmt" "strings" - "time" "golang.org/x/net/context" @@ -79,11 +78,10 @@ func (s *service) Probe( *csi.ProbeResponse, error, ) { Log.Infof("[Probe] Probe context: %v", ctx) - newCtx, _ := context.WithTimeout(ctx, 5*time.Second) - Log.Infof("[Probe] New Probe context: %v", newCtx) + if !strings.EqualFold(s.mode, "node") { Log.Infoln("[Probe] FERNANDO we are probing the controller") - if err := s.systemProbeAll(newCtx, ""); err != nil { + if err := s.systemProbeAll(ctx, ""); err != nil { Log.Printf("error in systemProbeAll: %s", err.Error()) return nil, err } From 0483cc080c1b48b0c0690264d14d90e87539370a Mon Sep 17 00:00:00 2001 From: "Lau, Luke" Date: Wed, 11 Dec 2024 12:00:30 -0500 Subject: [PATCH 11/40] clarify comment --- service/controller.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/service/controller.go b/service/controller.go index 2fee83d2..05df8bd0 100644 --- a/service/controller.go +++ b/service/controller.go @@ -2584,7 +2584,8 @@ func (s *service) systemProbeAll(ctx context.Context, zoneLabel string) error { continue } - // Expect responses from Pflex within 60s + // Set an upper limit to pflex response times so we can probe all arrays + // before the original context times out. subCtx, cancel := context.WithTimeout(ctx, time.Second*60) err := s.systemProbe(subCtx, array) cancel() From e0671d0c284576d350deb74620f570d81c730a34 Mon Sep 17 00:00:00 2001 From: "Lau, Luke" Date: Wed, 11 Dec 2024 15:23:19 -0500 Subject: [PATCH 12/40] handle when one array has a long response time. - dispatch array probe as a set of goroutines to prevent blocking. - update GetCapacity to only get capacity for the pflex array in a zone, using availableTopology from the request. --- service/controller.go | 62 +++++++++++++++++++++++++++++++++---------- 1 file changed, 48 insertions(+), 14 deletions(-) diff --git a/service/controller.go b/service/controller.go index 05df8bd0..bfe379de 100644 --- a/service/controller.go +++ b/service/controller.go @@ -2376,6 +2376,22 @@ func (s *service) GetCapacity( } } + // If using availability zones, get capacity for the system in the zone + // using accessible topology parameter from k8s. + if s.opts.zoneLabelKey != "" { + for topoKey, topoValue := range req.AccessibleTopology.Segments { + if topoKey == s.opts.zoneLabelKey { + for _, array := range s.opts.arrays { + if topoValue == string(array.AvailabilityZone.Name) { + systemID = array.SystemID + break + } + } + break + } + } + } + if systemID == "" { // Get capacity of storage pool spname in all systems, return total capacity capacity, err = s.getCapacityForAllSystems(ctx, "", spname) @@ -2557,7 +2573,7 @@ func (s *service) systemProbeAll(ctx context.Context, zoneLabel string) error { // probe all arrays // Log.Infof("Probing all arrays. Number of arrays: %d", len(s.opts.arrays)) Log.Infoln("Probing all associated arrays") - allArrayFail := true + probeSucceeded := false errMap := make(map[string]error) zone := "" @@ -2577,6 +2593,7 @@ func (s *service) systemProbeAll(ctx context.Context, zoneLabel string) error { } } + probeSuccess := make(chan bool, len(s.opts.arrays)) for _, array := range s.opts.arrays { // If zone information is available, use it to probe the array if strings.EqualFold(s.mode, "node") && array.AvailabilityZone != nil && array.AvailabilityZone.Name != ZoneName(zone) { @@ -2584,23 +2601,40 @@ func (s *service) systemProbeAll(ctx context.Context, zoneLabel string) error { continue } - // Set an upper limit to pflex response times so we can probe all arrays - // before the original context times out. - subCtx, cancel := context.WithTimeout(ctx, time.Second*60) - err := s.systemProbe(subCtx, array) - cancel() + // Run probe requests in parallel so that if one takes a long time to respond + // others are not blocked. So long as one array is online, we can provision storage. + go func() { + err := s.systemProbe(ctx, array) - systemID := array.SystemID - if err == nil { - Log.Infof("array %s probed successfully", systemID) - allArrayFail = false - } else { - errMap[systemID] = err - Log.Errorf("array %s probe failed: %v", array.SystemID, err) + systemID := array.SystemID + if err == nil { + Log.Infof("array %s probed successfully", systemID) + probeSuccess <- true + } else { + errMap[systemID] = err + Log.Errorf("array %s probe failed: %v", array.SystemID, err) + probeSuccess <- false + } + }() + } + + for range s.opts.arrays { + select { + case <-ctx.Done(): + // In case all calls are stuck waiting for responses and context times out + Log.Errorf("[LUKE] probe timed out") + break + case success := <-probeSuccess: + // We only need one successful probe to continue + probeSucceeded = probeSucceeded || success + } + if probeSucceeded { + Log.Infof("[LUKE] probe success, moving on, returning true") + break } } - if allArrayFail { + if !probeSucceeded { return status.Error(codes.FailedPrecondition, fmt.Sprintf("All arrays are not working. Could not proceed further: %v", errMap)) } From 979ebeaacd6bfbe26388e676f4b5ed66174b07f7 Mon Sep 17 00:00:00 2001 From: "Lau, Luke" Date: Wed, 11 Dec 2024 15:31:17 -0500 Subject: [PATCH 13/40] optimize GetCapacity --- service/controller.go | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/service/controller.go b/service/controller.go index bfe379de..a7b7f349 100644 --- a/service/controller.go +++ b/service/controller.go @@ -2379,14 +2379,13 @@ func (s *service) GetCapacity( // If using availability zones, get capacity for the system in the zone // using accessible topology parameter from k8s. if s.opts.zoneLabelKey != "" { - for topoKey, topoValue := range req.AccessibleTopology.Segments { - if topoKey == s.opts.zoneLabelKey { - for _, array := range s.opts.arrays { - if topoValue == string(array.AvailabilityZone.Name) { - systemID = array.SystemID - break - } - } + zoneLabel, ok := req.AccessibleTopology.Segments[s.opts.zoneLabelKey] + if !ok { + Log.Infof("could not get availability zone from accessible topology. Getting capacity for all systems") + } + for _, array := range s.opts.arrays { + if zoneLabel == string(array.AvailabilityZone.Name) { + systemID = array.SystemID break } } From 8204ed1a686ae35ec618a0a1e9a4820990bb0358 Mon Sep 17 00:00:00 2001 From: "Lau, Luke" Date: Wed, 11 Dec 2024 15:32:17 -0500 Subject: [PATCH 14/40] optimize GetCapacity --- service/controller.go | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/service/controller.go b/service/controller.go index a7b7f349..e80f9c1e 100644 --- a/service/controller.go +++ b/service/controller.go @@ -2382,11 +2382,12 @@ func (s *service) GetCapacity( zoneLabel, ok := req.AccessibleTopology.Segments[s.opts.zoneLabelKey] if !ok { Log.Infof("could not get availability zone from accessible topology. Getting capacity for all systems") - } - for _, array := range s.opts.arrays { - if zoneLabel == string(array.AvailabilityZone.Name) { - systemID = array.SystemID - break + } else { + for _, array := range s.opts.arrays { + if zoneLabel == string(array.AvailabilityZone.Name) { + systemID = array.SystemID + break + } } } } From a88e0591ac861113867b52ff71cce2512f7e2bc5 Mon Sep 17 00:00:00 2001 From: "Lau, Luke" Date: Wed, 11 Dec 2024 16:45:25 -0500 Subject: [PATCH 15/40] tidying --- service/controller.go | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/service/controller.go b/service/controller.go index e80f9c1e..c6a3d397 100644 --- a/service/controller.go +++ b/service/controller.go @@ -2593,7 +2593,8 @@ func (s *service) systemProbeAll(ctx context.Context, zoneLabel string) error { } } - probeSuccess := make(chan bool, len(s.opts.arrays)) + probeRequestCount := len(s.opts.arrays) + probeSuccess := make(chan bool, probeRequestCount) for _, array := range s.opts.arrays { // If zone information is available, use it to probe the array if strings.EqualFold(s.mode, "node") && array.AvailabilityZone != nil && array.AvailabilityZone.Name != ZoneName(zone) { @@ -2603,6 +2604,7 @@ func (s *service) systemProbeAll(ctx context.Context, zoneLabel string) error { // Run probe requests in parallel so that if one takes a long time to respond // others are not blocked. So long as one array is online, we can provision storage. + // Useful in controller mode but not necessarily node mode go func() { err := s.systemProbe(ctx, array) @@ -2618,16 +2620,17 @@ func (s *service) systemProbeAll(ctx context.Context, zoneLabel string) error { }() } - for range s.opts.arrays { + for range probeRequestCount { select { case <-ctx.Done(): - // In case all calls are stuck waiting for responses and context times out + // handle case when all probe reqs are stuck waiting for responses and context times out Log.Errorf("[LUKE] probe timed out") break case success := <-probeSuccess: // We only need one successful probe to continue probeSucceeded = probeSucceeded || success } + if probeSucceeded { Log.Infof("[LUKE] probe success, moving on, returning true") break From 825b2d7e43b4c97e42c842f9dace84d503686685 Mon Sep 17 00:00:00 2001 From: "Lau, Luke" Date: Thu, 12 Dec 2024 14:36:31 -0500 Subject: [PATCH 16/40] revert changes to probe calls due to data race --- service/controller.go | 46 ++++++++++--------------------------------- 1 file changed, 10 insertions(+), 36 deletions(-) diff --git a/service/controller.go b/service/controller.go index c6a3d397..fb3509d5 100644 --- a/service/controller.go +++ b/service/controller.go @@ -2573,7 +2573,7 @@ func (s *service) systemProbeAll(ctx context.Context, zoneLabel string) error { // probe all arrays // Log.Infof("Probing all arrays. Number of arrays: %d", len(s.opts.arrays)) Log.Infoln("Probing all associated arrays") - probeSucceeded := false + allArrayFail := true errMap := make(map[string]error) zone := "" @@ -2593,8 +2593,6 @@ func (s *service) systemProbeAll(ctx context.Context, zoneLabel string) error { } } - probeRequestCount := len(s.opts.arrays) - probeSuccess := make(chan bool, probeRequestCount) for _, array := range s.opts.arrays { // If zone information is available, use it to probe the array if strings.EqualFold(s.mode, "node") && array.AvailabilityZone != nil && array.AvailabilityZone.Name != ZoneName(zone) { @@ -2602,42 +2600,18 @@ func (s *service) systemProbeAll(ctx context.Context, zoneLabel string) error { continue } - // Run probe requests in parallel so that if one takes a long time to respond - // others are not blocked. So long as one array is online, we can provision storage. - // Useful in controller mode but not necessarily node mode - go func() { - err := s.systemProbe(ctx, array) - - systemID := array.SystemID - if err == nil { - Log.Infof("array %s probed successfully", systemID) - probeSuccess <- true - } else { - errMap[systemID] = err - Log.Errorf("array %s probe failed: %v", array.SystemID, err) - probeSuccess <- false - } - }() - } - - for range probeRequestCount { - select { - case <-ctx.Done(): - // handle case when all probe reqs are stuck waiting for responses and context times out - Log.Errorf("[LUKE] probe timed out") - break - case success := <-probeSuccess: - // We only need one successful probe to continue - probeSucceeded = probeSucceeded || success - } - - if probeSucceeded { - Log.Infof("[LUKE] probe success, moving on, returning true") - break + err := s.systemProbe(ctx, array) + systemID := array.SystemID + if err == nil { + Log.Infof("array %s probed successfully", systemID) + allArrayFail = false + } else { + errMap[systemID] = err + Log.Errorf("array %s probe failed: %v", array.SystemID, err) } } - if !probeSucceeded { + if allArrayFail { return status.Error(codes.FailedPrecondition, fmt.Sprintf("All arrays are not working. Could not proceed further: %v", errMap)) } From 21e7043099aeb9f893f4fb3886b6cb4dbcdcd176 Mon Sep 17 00:00:00 2001 From: "Lau, Luke" Date: Thu, 12 Dec 2024 16:48:12 -0500 Subject: [PATCH 17/40] update test context creation --- service/step_defs_test.go | 234 +++++++++++++++++++------------------- 1 file changed, 117 insertions(+), 117 deletions(-) diff --git a/service/step_defs_test.go b/service/step_defs_test.go index 64f65de0..5e12a085 100644 --- a/service/step_defs_test.go +++ b/service/step_defs_test.go @@ -326,9 +326,9 @@ func (f *feature) getService() *service { return f.service } var opts Opts - ctx := new(context.Context) + ctx := context.Background() var err error - opts.arrays, err = getArrayConfig(*ctx) + opts.arrays, err = getArrayConfig(ctx) if err != nil { log.Printf("Read arrays from config file failed: %s\n", err) } @@ -462,9 +462,9 @@ func (f *feature) aValidDynamicLogChange(file, expectedLevel string) error { // GetPluginInfo func (f *feature) iCallGetPluginInfo() error { - ctx := new(context.Context) + ctx := context.Background() req := new(csi.GetPluginInfoRequest) - f.getPluginInfoResponse, f.err = f.service.GetPluginInfo(*ctx, req) + f.getPluginInfoResponse, f.err = f.service.GetPluginInfo(ctx, req) if f.err != nil { return f.err } @@ -513,9 +513,9 @@ func (f *feature) aValidGetPlugInfoResponseIsReturned() error { } func (f *feature) iCallGetPluginCapabilities() error { - ctx := new(context.Context) + ctx := context.Background() req := new(csi.GetPluginCapabilitiesRequest) - f.getPluginCapabilitiesResponse, f.err = f.service.GetPluginCapabilities(*ctx, req) + f.getPluginCapabilitiesResponse, f.err = f.service.GetPluginCapabilities(ctx, req) if f.err != nil { return f.err } @@ -538,12 +538,12 @@ func (f *feature) aValidGetPluginCapabilitiesResponseIsReturned() error { } func (f *feature) iCallProbe() error { - ctx := new(context.Context) + ctx := context.Background() req := new(csi.ProbeRequest) f.checkGoRoutines("before probe") f.service.opts.AutoProbe = true f.service.mode = "controller" - f.probeResponse, f.err = f.service.Probe(*ctx, req) + f.probeResponse, f.err = f.service.Probe(ctx, req) f.checkGoRoutines("after probe") return nil } @@ -700,7 +700,7 @@ func (f *feature) iSpecifyCreateVolumeMountRequest(fstype string) error { } func (f *feature) iCallCreateVolume(name string) error { - ctx := new(context.Context) + ctx := context.Background() if f.createVolumeRequest == nil { req := getTypicalCreateVolumeRequest() f.createVolumeRequest = req @@ -715,7 +715,7 @@ func (f *feature) iCallCreateVolume(name string) error { fmt.Println("I am in iCallCreateVolume fn.....") - f.createVolumeResponse, f.err = f.service.CreateVolume(*ctx, req) + f.createVolumeResponse, f.err = f.service.CreateVolume(ctx, req) if f.err != nil { log.Printf("CreateVolume called failed: %s\n", f.err.Error()) } @@ -727,7 +727,7 @@ func (f *feature) iCallCreateVolume(name string) error { } func (f *feature) iCallValidateVolumeHostConnectivity() error { - ctx := new(context.Context) + ctx := context.Background() sdcID := f.service.opts.SdcGUID sdcGUID := strings.ToUpper(sdcID) @@ -764,7 +764,7 @@ func (f *feature) iCallValidateVolumeHostConnectivity() error { VolumeIds: volIDs, } - connect, err := f.service.ValidateVolumeHostConnectivity(*ctx, req) + connect, err := f.service.ValidateVolumeHostConnectivity(ctx, req) if err != nil { f.err = errors.New(err.Error()) return nil @@ -1026,7 +1026,7 @@ func (f *feature) iSpecifyNoStoragePool() error { } func (f *feature) iCallCreateVolumeSize(name string, size int64) error { - ctx := new(context.Context) + ctx := context.Background() var req *csi.CreateVolumeRequest if f.createVolumeRequest == nil { req = getTypicalCreateVolumeRequest() @@ -1040,7 +1040,7 @@ func (f *feature) iCallCreateVolumeSize(name string, size int64) error { req.Name = name f.createVolumeRequest = req - f.createVolumeResponse, f.err = f.service.CreateVolume(*ctx, req) + f.createVolumeResponse, f.err = f.service.CreateVolume(ctx, req) if f.err != nil { log.Printf("CreateVolumeSize called failed: %s\n", f.err.Error()) } @@ -1052,7 +1052,7 @@ func (f *feature) iCallCreateVolumeSize(name string, size int64) error { } func (f *feature) iCallCreateVolumeSizeNFS(name string, size int64) error { - ctx := new(context.Context) + ctx := context.Background() var req *csi.CreateVolumeRequest if f.createVolumeRequest == nil { req = getTypicalNFSCreateVolumeRequest() @@ -1066,7 +1066,7 @@ func (f *feature) iCallCreateVolumeSizeNFS(name string, size int64) error { req.Name = name f.createVolumeRequest = req - f.createVolumeResponse, f.err = f.service.CreateVolume(*ctx, req) + f.createVolumeResponse, f.err = f.service.CreateVolume(ctx, req) if f.err != nil { log.Printf("CreateVolumeSize called failed: %s\n", f.err.Error()) } @@ -1610,7 +1610,7 @@ func (f *feature) getControllerDeleteVolumeRequestNFS(accessType string) *csi.De } func (f *feature) iCallPublishVolumeWith(arg1 string) error { - ctx := new(context.Context) + ctx := context.Background() req := f.publishVolumeRequest if f.publishVolumeRequest == nil { req = f.getControllerPublishVolumeRequest(arg1) @@ -1618,7 +1618,7 @@ func (f *feature) iCallPublishVolumeWith(arg1 string) error { } log.Printf("Calling controllerPublishVolume") - f.publishVolumeResponse, f.err = f.service.ControllerPublishVolume(*ctx, req) + f.publishVolumeResponse, f.err = f.service.ControllerPublishVolume(ctx, req) if f.err != nil { log.Printf("PublishVolume call failed: %s\n", f.err.Error()) } @@ -1626,7 +1626,7 @@ func (f *feature) iCallPublishVolumeWith(arg1 string) error { } func (f *feature) iCallPublishVolumeWithNFS(arg1 string) error { - ctx := new(context.Context) + ctx := context.Background() req := f.publishVolumeRequest if f.publishVolumeRequest == nil { req = f.getControllerPublishVolumeRequestNFS(arg1) @@ -1683,7 +1683,7 @@ func (f *feature) iCallPublishVolumeWithNFS(arg1 string) error { } log.Printf("Calling controllerPublishVolume") - f.publishVolumeResponse, f.err = f.service.ControllerPublishVolume(*ctx, req) + f.publishVolumeResponse, f.err = f.service.ControllerPublishVolume(ctx, req) if f.err != nil { log.Printf("PublishVolume call failed: %s\n", f.err.Error()) } @@ -1806,14 +1806,14 @@ func (f *feature) getControllerUnpublishVolumeRequestNFS() *csi.ControllerUnpubl } func (f *feature) iCallUnpublishVolume() error { - ctx := new(context.Context) + ctx := context.Background() req := f.unpublishVolumeRequest if f.unpublishVolumeRequest == nil { req = f.getControllerUnpublishVolumeRequest() f.unpublishVolumeRequest = req } log.Printf("Calling controllerUnpublishVolume: %s", req.VolumeId) - f.unpublishVolumeResponse, f.err = f.service.ControllerUnpublishVolume(*ctx, req) + f.unpublishVolumeResponse, f.err = f.service.ControllerUnpublishVolume(ctx, req) if f.err != nil { log.Printf("UnpublishVolume call failed: %s\n", f.err.Error()) } @@ -1821,7 +1821,7 @@ func (f *feature) iCallUnpublishVolume() error { } func (f *feature) iCallUnpublishVolumeNFS() error { - ctx := new(context.Context) + ctx := context.Background() req := f.unpublishVolumeRequest if f.unpublishVolumeRequest == nil { req = f.getControllerUnpublishVolumeRequestNFS() @@ -1855,7 +1855,7 @@ func (f *feature) iCallUnpublishVolumeNFS() error { } log.Printf("Calling controllerUnpublishVolume: %s", req.VolumeId) - f.unpublishVolumeResponse, f.err = f.service.ControllerUnpublishVolume(*ctx, req) + f.unpublishVolumeResponse, f.err = f.service.ControllerUnpublishVolume(ctx, req) if f.err != nil { log.Printf("UnpublishVolume call failed: %s\n", f.err.Error()) } @@ -1877,50 +1877,50 @@ func (f *feature) theNumberOfSDCMappingsIs(arg1 int) error { } func (f *feature) iCallNodeGetInfo() error { - ctx := new(context.Context) + ctx := context.Background() req := new(csi.NodeGetInfoRequest) f.service.opts.SdcGUID = "9E56672F-2F4B-4A42-BFF4-88B6846FBFDA" GetNodeLabels = mockGetNodeLabels GetNodeUID = mockGetNodeUID - f.nodeGetInfoResponse, f.err = f.service.NodeGetInfo(*ctx, req) + f.nodeGetInfoResponse, f.err = f.service.NodeGetInfo(ctx, req) return nil } func (f *feature) iCallNodeGetInfoWithValidVolumeLimitNodeLabels() error { f.setFakeNode() - ctx := new(context.Context) + ctx := context.Background() req := new(csi.NodeGetInfoRequest) f.service.opts.SdcGUID = "9E56672F-2F4B-4A42-BFF4-88B6846FBFDA" GetNodeLabels = mockGetNodeLabelsWithVolumeLimits - f.nodeGetInfoResponse, f.err = f.service.NodeGetInfo(*ctx, req) + f.nodeGetInfoResponse, f.err = f.service.NodeGetInfo(ctx, req) fmt.Printf("MaxVolumesPerNode: %v", f.nodeGetInfoResponse.MaxVolumesPerNode) return nil } func (f *feature) iCallNodeGetInfoWithInvalidVolumeLimitNodeLabels() error { - ctx := new(context.Context) + ctx := context.Background() req := new(csi.NodeGetInfoRequest) f.service.opts.SdcGUID = "9E56672F-2F4B-4A42-BFF4-88B6846FBFDA" GetNodeLabels = mockGetNodeLabelsWithInvalidVolumeLimits - f.nodeGetInfoResponse, f.err = f.service.NodeGetInfo(*ctx, req) + f.nodeGetInfoResponse, f.err = f.service.NodeGetInfo(ctx, req) return nil } func (f *feature) iCallNodeGetInfoWithValidNodeUID() error { - ctx := new(context.Context) + ctx := context.Background() req := new(csi.NodeGetInfoRequest) GetNodeUID = mockGetNodeUID f.service.opts.SdcGUID = "" - f.nodeGetInfoResponse, f.err = f.service.NodeGetInfo(*ctx, req) + f.nodeGetInfoResponse, f.err = f.service.NodeGetInfo(ctx, req) fmt.Printf("NodeGetInfoResponse: %v", f.nodeGetInfoResponse) return nil } func (f *feature) iCallGetNodeUID() error { f.setFakeNode() - ctx := new(context.Context) + ctx := context.Background() nodeUID := "" - nodeUID, err := f.service.GetNodeUID(*ctx) + nodeUID, err := f.service.GetNodeUID(ctx) fmt.Printf("Node UID: %v", nodeUID) if err != nil { @@ -2012,8 +2012,8 @@ func (f *feature) iCallGetNodeLabelsWithInvalidNode() error { func (f *feature) iCallGetNodeLabelsWithUnsetKubernetesClient() error { K8sClientset = nil - ctx := new(context.Context) - f.nodeLabels, f.err = f.service.GetNodeLabels(*ctx) + ctx := context.Background() + f.nodeLabels, f.err = f.service.GetNodeLabels(ctx) return nil } @@ -2025,17 +2025,17 @@ func (f *feature) iCallGetNodeUIDWithInvalidNode() error { func (f *feature) iCallGetNodeUIDWithUnsetKubernetesClient() error { K8sClientset = nil - ctx := new(context.Context) - f.nodeUID, f.err = f.service.GetNodeUID(*ctx) + ctx := context.Background() + f.nodeUID, f.err = f.service.GetNodeUID(ctx) return nil } func (f *feature) iCallNodeProbe() error { - ctx := new(context.Context) + ctx := context.Background() req := new(csi.ProbeRequest) f.checkGoRoutines("before probe") f.service.mode = "node" - f.probeResponse, f.err = f.service.Probe(*ctx, req) + f.probeResponse, f.err = f.service.Probe(ctx, req) f.checkGoRoutines("after probe") return nil } @@ -2082,14 +2082,14 @@ func (f *feature) theVolumeLimitIsSet() error { } func (f *feature) iCallDeleteVolumeWith(arg1 string) error { - ctx := new(context.Context) + ctx := context.Background() req := f.deleteVolumeRequest if f.deleteVolumeRequest == nil { req = f.getControllerDeleteVolumeRequest(arg1) f.deleteVolumeRequest = req } log.Printf("Calling DeleteVolume") - f.deleteVolumeResponse, f.err = f.service.DeleteVolume(*ctx, req) + f.deleteVolumeResponse, f.err = f.service.DeleteVolume(ctx, req) if f.err != nil { log.Printf("DeleteVolume called failed: %s\n", f.err.Error()) } @@ -2097,14 +2097,14 @@ func (f *feature) iCallDeleteVolumeWith(arg1 string) error { } func (f *feature) iCallDeleteVolumeWithBad(arg1 string) error { - ctx := new(context.Context) + ctx := context.Background() req := f.deleteVolumeRequest if f.deleteVolumeRequest == nil { req = f.getControllerDeleteVolumeRequestBad(arg1) f.deleteVolumeRequest = req } log.Printf("Calling DeleteVolume") - f.deleteVolumeResponse, f.err = f.service.DeleteVolume(*ctx, req) + f.deleteVolumeResponse, f.err = f.service.DeleteVolume(ctx, req) if f.err != nil { log.Printf("DeleteVolume called failed: %s\n", f.err.Error()) } @@ -2112,14 +2112,14 @@ func (f *feature) iCallDeleteVolumeWithBad(arg1 string) error { } func (f *feature) iCallDeleteVolumeNFSWith(arg1 string) error { - ctx := new(context.Context) + ctx := context.Background() req := f.deleteVolumeRequest if f.deleteVolumeRequest == nil { req = f.getControllerDeleteVolumeRequestNFS(arg1) f.deleteVolumeRequest = req } log.Printf("Calling DeleteVolume") - f.deleteVolumeResponse, f.err = f.service.DeleteVolume(*ctx, req) + f.deleteVolumeResponse, f.err = f.service.DeleteVolume(ctx, req) if f.err != nil { log.Printf("DeleteVolume called failed: %s\n", f.err.Error()) } @@ -2147,7 +2147,7 @@ func (f *feature) theVolumeIsAlreadyMappedToAnSDC() error { } func (f *feature) iCallGetCapacityWithStoragePool(arg1 string) error { - ctx := new(context.Context) + ctx := context.Background() req := new(csi.GetCapacityRequest) if arg1 != "" { parameters := make(map[string]string) @@ -2155,7 +2155,7 @@ func (f *feature) iCallGetCapacityWithStoragePool(arg1 string) error { req.Parameters = parameters } log.Printf("Calling GetCapacity") - f.getCapacityResponse, f.err = f.service.GetCapacity(*ctx, req) + f.getCapacityResponse, f.err = f.service.GetCapacity(ctx, req) if f.err != nil { log.Printf("GetCapacity call failed: %s\n", f.err.Error()) return nil @@ -2205,10 +2205,10 @@ func (f *feature) iCallControllerGetCapabilities(isHealthMonitorEnabled string) if isHealthMonitorEnabled == "true" { f.service.opts.IsHealthMonitorEnabled = true } - ctx := new(context.Context) + ctx := context.Background() req := new(csi.ControllerGetCapabilitiesRequest) log.Printf("Calling ControllerGetCapabilities") - f.controllerGetCapabilitiesResponse, f.err = f.service.ControllerGetCapabilities(*ctx, req) + f.controllerGetCapabilitiesResponse, f.err = f.service.ControllerGetCapabilities(ctx, req) if f.err != nil { log.Printf("ControllerGetCapabilities call failed: %s\n", f.err.Error()) return f.err @@ -2263,7 +2263,7 @@ func (f *feature) iCallListVolumesWith(maxEntriesString, startingToken string) e return err } - ctx := new(context.Context) + ctx := context.Background() req := f.listVolumesRequest if f.listVolumesRequest == nil { switch st := startingToken; st { @@ -2286,7 +2286,7 @@ func (f *feature) iCallListVolumesWith(maxEntriesString, startingToken string) e f.listVolumesRequest = req } log.Printf("Calling ListVolumes with req=%+v", f.listVolumesRequest) - f.listVolumesResponse, f.err = f.service.ListVolumes(*ctx, req) + f.listVolumesResponse, f.err = f.service.ListVolumes(ctx, req) if f.err != nil { log.Printf("ListVolume called failed: %s\n", f.err.Error()) } else { @@ -2349,7 +2349,7 @@ func (f *feature) aValidControllerGetCapabilitiesResponseIsReturned() error { } func (f *feature) iCallCloneVolume() error { - ctx := new(context.Context) + ctx := context.Background() req := getTypicalCreateVolumeRequest() req.Name = "clone" if f.invalidVolumeID { @@ -2366,7 +2366,7 @@ func (f *feature) iCallCloneVolume() error { req.VolumeContentSource = new(csi.VolumeContentSource) req.VolumeContentSource.Type = &csi.VolumeContentSource_Volume{Volume: source} req.AccessibilityRequirements = new(csi.TopologyRequirement) - f.createVolumeResponse, f.err = f.service.CreateVolume(*ctx, req) + f.createVolumeResponse, f.err = f.service.CreateVolume(ctx, req) if f.err != nil { fmt.Printf("Error on CreateVolume from volume: %s\n", f.err.Error()) } @@ -2376,7 +2376,7 @@ func (f *feature) iCallCloneVolume() error { //nolint:revive func (f *feature) iCallValidateVolumeCapabilitiesWithVoltypeAccessFstype(voltype, access, fstype string) error { - ctx := new(context.Context) + ctx := context.Background() req := new(csi.ValidateVolumeCapabilitiesRequest) if f.invalidVolumeID || f.createVolumeResponse == nil { req.VolumeId = badVolumeID2 @@ -2417,7 +2417,7 @@ func (f *feature) iCallValidateVolumeCapabilitiesWithVoltypeAccessFstype(voltype capabilities = append(capabilities, capability) req.VolumeCapabilities = capabilities log.Printf("Calling ValidateVolumeCapabilities %#v", accessMode) - f.validateVolumeCapabilitiesResponse, f.err = f.service.ValidateVolumeCapabilities(*ctx, req) + f.validateVolumeCapabilitiesResponse, f.err = f.service.ValidateVolumeCapabilities(ctx, req) if f.err != nil { return nil } @@ -2896,8 +2896,8 @@ func (f *feature) iCallNodePublishVolumeNFS(arg1 string) error { func (f *feature) iCallUnmountPrivMount() error { gofsutil.GOFSMock.InduceGetMountsError = true - ctx := new(context.Context) - err := unmountPrivMount(*ctx, nil, "/foo/bar") + ctx := context.Background() + err := unmountPrivMount(ctx, nil, "/foo/bar") fmt.Printf("unmountPrivMount getMounts error: %s\n", err.Error()) // getMounts induced error if err != nil { @@ -2917,7 +2917,7 @@ func (f *feature) iCallUnmountPrivMount() error { gofsutil.GOFSMock.InduceGetMountsError = false gofsutil.GOFSMock.InduceUnmountError = true - err = unmountPrivMount(*ctx, nil, target) + err = unmountPrivMount(ctx, nil, target) fmt.Printf("unmountPrivMount unmount error: %s\n", err) if err != nil { f.err = errors.New("error in unmountPrivMount") @@ -3271,9 +3271,9 @@ func (f *feature) iCallBeforeServe() error { } func (f *feature) iCallNodeStageVolume() error { - ctx := new(context.Context) + ctx := context.Background() req := new(csi.NodeStageVolumeRequest) - _, f.err = f.service.NodeStageVolume(*ctx, req) + _, f.err = f.service.NodeStageVolume(ctx, req) return nil } @@ -3322,7 +3322,7 @@ func (f *feature) iCallNodeExpandVolume(volPath string) error { } func (f *feature) iCallNodeGetVolumeStats() error { - ctx := new(context.Context) + ctx := context.Background() VolumeID := sdcVolume1 VolumePath := datadir @@ -3352,7 +3352,7 @@ func (f *feature) iCallNodeGetVolumeStats() error { req := &csi.NodeGetVolumeStatsRequest{VolumeId: VolumeID, VolumePath: VolumePath} - f.nodeGetVolumeStatsResponse, f.err = f.service.NodeGetVolumeStats(*ctx, req) + f.nodeGetVolumeStatsResponse, f.err = f.service.NodeGetVolumeStats(ctx, req) return nil } @@ -3458,12 +3458,12 @@ func (f *feature) iCallNodeUnstageVolumeWith(anError string) error { } func (f *feature) iCallNodeGetCapabilities(isHealthMonitorEnabled string) error { - ctx := new(context.Context) + ctx := context.Background() if isHealthMonitorEnabled == "true" { f.service.opts.IsHealthMonitorEnabled = true } req := new(csi.NodeGetCapabilitiesRequest) - f.nodeGetCapabilitiesResponse, f.err = f.service.NodeGetCapabilities(*ctx, req) + f.nodeGetCapabilitiesResponse, f.err = f.service.NodeGetCapabilities(ctx, req) return nil } @@ -3654,7 +3654,7 @@ func (f *feature) aValidCreateVolumeSnapshotGroupResponse() error { } func (f *feature) iCallCreateSnapshot(snapName string) error { - ctx := new(context.Context) + ctx := context.Background() if len(f.volumeIDList) == 0 { f.volumeIDList = append(f.volumeIDList, "00000000") @@ -3689,15 +3689,15 @@ func (f *feature) iCallCreateSnapshot(snapName string) error { } fmt.Println("snapName is: ", snapName) - fmt.Println("ctx: ", *ctx) + fmt.Println("ctx: ", ctx) fmt.Println("req: ", req) - f.createSnapshotResponse, f.err = f.service.CreateSnapshot(*ctx, req) + f.createSnapshotResponse, f.err = f.service.CreateSnapshot(ctx, req) return nil } func (f *feature) iCallCreateSnapshotNFS(snapName string) error { - ctx := new(context.Context) + ctx := context.Background() req := &csi.CreateSnapshotRequest{ SourceVolumeId: "14dbbf5617523654" + "/" + fileSystemNameToID["volume1"], @@ -3713,10 +3713,10 @@ func (f *feature) iCallCreateSnapshotNFS(snapName string) error { } fmt.Println("snapName is: ", snapName) - fmt.Println("ctx: ", *ctx) + fmt.Println("ctx: ", ctx) fmt.Println("req: ", req) - f.createSnapshotResponse, f.err = f.service.CreateSnapshot(*ctx, req) + f.createSnapshotResponse, f.err = f.service.CreateSnapshot(ctx, req) return nil } @@ -3740,7 +3740,7 @@ func (f *feature) aValidSnapshot() error { } func (f *feature) iCallDeleteSnapshot() error { - ctx := new(context.Context) + ctx := context.Background() req := &csi.DeleteSnapshotRequest{SnapshotId: goodSnapID, Secrets: make(map[string]string)} req.Secrets["x"] = "y" if f.invalidVolumeID { @@ -3748,12 +3748,12 @@ func (f *feature) iCallDeleteSnapshot() error { } else if f.noVolumeID { req.SnapshotId = "" } - _, f.err = f.service.DeleteSnapshot(*ctx, req) + _, f.err = f.service.DeleteSnapshot(ctx, req) return nil } func (f *feature) iCallDeleteSnapshotNFS() error { - ctx := new(context.Context) + ctx := context.Background() var req *csi.DeleteSnapshotRequest = new(csi.DeleteSnapshotRequest) if fileSystemNameToID["snap1"] == "" { req = &csi.DeleteSnapshotRequest{SnapshotId: "14dbbf5617523654" + "/" + "1111111", Secrets: make(map[string]string)} @@ -3762,7 +3762,7 @@ func (f *feature) iCallDeleteSnapshotNFS() error { } req.Secrets["x"] = "y" - _, f.err = f.service.DeleteSnapshot(*ctx, req) + _, f.err = f.service.DeleteSnapshot(ctx, req) return nil } @@ -3793,7 +3793,7 @@ func (f *feature) aValidSnapshotConsistencyGroup() error { } func (f *feature) iCallCreateVolumeFromSnapshot() error { - ctx := new(context.Context) + ctx := context.Background() req := getTypicalCreateVolumeRequest() req.Name = "volumeFromSnap" if f.wrongCapacity { @@ -3805,7 +3805,7 @@ func (f *feature) iCallCreateVolumeFromSnapshot() error { source := &csi.VolumeContentSource_SnapshotSource{SnapshotId: goodSnapID} req.VolumeContentSource = new(csi.VolumeContentSource) req.VolumeContentSource.Type = &csi.VolumeContentSource_Snapshot{Snapshot: source} - f.createVolumeResponse, f.err = f.service.CreateVolume(*ctx, req) + f.createVolumeResponse, f.err = f.service.CreateVolume(ctx, req) if f.err != nil { fmt.Printf("Error on CreateVolume from snap: %s\n", f.err.Error()) } @@ -3813,7 +3813,7 @@ func (f *feature) iCallCreateVolumeFromSnapshot() error { } func (f *feature) iCallCreateVolumeFromSnapshotNFS() error { - ctx := new(context.Context) + ctx := context.Background() req := getTypicalNFSCreateVolumeRequest() req.Name = "volumeFromSnap" if f.wrongCapacity { @@ -3825,7 +3825,7 @@ func (f *feature) iCallCreateVolumeFromSnapshotNFS() error { source := &csi.VolumeContentSource_SnapshotSource{SnapshotId: "14dbbf5617523654" + "/" + fileSystemNameToID["snap1"]} req.VolumeContentSource = new(csi.VolumeContentSource) req.VolumeContentSource.Type = &csi.VolumeContentSource_Snapshot{Snapshot: source} - f.createVolumeResponse, f.err = f.service.CreateVolume(*ctx, req) + f.createVolumeResponse, f.err = f.service.CreateVolume(ctx, req) if f.err != nil { fmt.Printf("Error on CreateVolume from snap: %s\n", f.err.Error()) } @@ -3867,7 +3867,7 @@ func (f *feature) iCallListSnapshotsWithMaxentriesAndStartingtoken(maxEntriesStr if err != nil { return nil } - ctx := new(context.Context) + ctx := context.Background() // ignoring integer overflow issue, will not be an issue if maxEntries is less than 2147483647 // #nosec G115 @@ -3875,7 +3875,7 @@ func (f *feature) iCallListSnapshotsWithMaxentriesAndStartingtoken(maxEntriesStr f.listSnapshotsRequest = req log.Printf("Calling ListSnapshots with req=%+v", f.listVolumesRequest) - f.listSnapshotsResponse, f.err = f.service.ListSnapshots(*ctx, req) + f.listSnapshotsResponse, f.err = f.service.ListSnapshots(ctx, req) if f.err != nil { log.Printf("ListSnapshots called failed: %s\n", f.err.Error()) } @@ -3888,7 +3888,7 @@ func (f *feature) iCallListSnapshotsForVolume(arg1 string) error { sourceVolumeID = altVolumeID } - ctx := new(context.Context) + ctx := context.Background() req := &csi.ListSnapshotsRequest{SourceVolumeId: sourceVolumeID} req.StartingToken = "0" req.MaxEntries = 100 @@ -3900,7 +3900,7 @@ func (f *feature) iCallListSnapshotsForVolume(arg1 string) error { f.listSnapshotsRequest = req log.Printf("Calling ListSnapshots with req=%+v", f.listSnapshotsRequest) - f.listSnapshotsResponse, f.err = f.service.ListSnapshots(*ctx, req) + f.listSnapshotsResponse, f.err = f.service.ListSnapshots(ctx, req) if f.err != nil { log.Printf("ListSnapshots called failed: %s\n", f.err.Error()) } @@ -3908,11 +3908,11 @@ func (f *feature) iCallListSnapshotsForVolume(arg1 string) error { } func (f *feature) iCallListSnapshotsForSnapshot(arg1 string) error { - ctx := new(context.Context) + ctx := context.Background() req := &csi.ListSnapshotsRequest{SnapshotId: arg1} f.listSnapshotsRequest = req log.Printf("Calling ListSnapshots with req=%+v", f.listVolumesRequest) - f.listSnapshotsResponse, f.err = f.service.ListSnapshots(*ctx, req) + f.listSnapshotsResponse, f.err = f.service.ListSnapshots(ctx, req) if f.err != nil { log.Printf("ListSnapshots called failed: %s\n", f.err.Error()) } @@ -4153,8 +4153,8 @@ func (f *feature) iCallGetSystemNameError() error { stepHandlersErrors.PodmonNodeProbeError = true // Unable to probe system with ID: - ctx := new(context.Context) - f.err = f.service.systemProbe(*ctx, badarray) + ctx := context.Background() + f.err = f.service.systemProbe(ctx, badarray) return nil } @@ -4168,9 +4168,9 @@ func (f *feature) iCallGetSystemName() error { func (f *feature) iCallNodeGetAllSystems() error { // lookup the system names for a couple of systems // This should not generate an error as systems without names are supported - ctx := new(context.Context) + ctx := context.Background() badarray := f.service.opts.arrays[arrayID] - f.err = f.service.systemProbe(*ctx, badarray) + f.err = f.service.systemProbe(ctx, badarray) return nil } @@ -4213,8 +4213,8 @@ func (f *feature) anInvalidMaxVolumesPerNode() error { } func (f *feature) iCallGetArrayConfig() error { - ctx := new(context.Context) - _, err := getArrayConfig(*ctx) + ctx := context.Background() + _, err := getArrayConfig(ctx) if err != nil { f.err = err } @@ -4229,8 +4229,8 @@ func (f *feature) iCallgetArrayInstallationID(systemID string) error { } func (f *feature) iCallSetQoSParameters(systemID string, sdcID string, bandwidthLimit string, iopsLimit string, volumeName string, csiVolID string, nodeID string) error { - ctx := new(context.Context) - f.err = f.service.setQoSParameters(*ctx, systemID, sdcID, bandwidthLimit, iopsLimit, volumeName, csiVolID, nodeID) + ctx := context.Background() + f.err = f.service.setQoSParameters(ctx, systemID, sdcID, bandwidthLimit, iopsLimit, volumeName, csiVolID, nodeID) if f.err != nil { fmt.Printf("error in setting QoS parameters for volume %s : %s\n", volumeName, f.err.Error()) } @@ -4273,8 +4273,8 @@ func (f *feature) iUseConfig(filename string) error { func (f *feature) iCallGetReplicationCapabilities() error { req := &replication.GetReplicationCapabilityRequest{} - ctx := new(context.Context) - f.replicationCapabilitiesResponse, f.err = f.service.GetReplicationCapabilities(*ctx, req) + ctx := context.Background() + f.replicationCapabilitiesResponse, f.err = f.service.GetReplicationCapabilities(ctx, req) log.Printf("GetReplicationCapabilities returned %+v", f.replicationCapabilitiesResponse) return nil } @@ -4353,7 +4353,7 @@ func (f *feature) iSetApproveSdcEnabled(approveSDCEnabled string) error { } func (f *feature) iCallCreateRemoteVolume() error { - ctx := new(context.Context) + ctx := context.Background() req := &replication.CreateRemoteVolumeRequest{} if f.createVolumeResponse == nil { return errors.New("iCallCreateRemoteVolume: f.createVolumeResponse is nil") @@ -4369,7 +4369,7 @@ func (f *feature) iCallCreateRemoteVolume() error { f.service.WithRP(KeyReplicationRemoteStoragePool): "viki_pool_HDD_20181031", f.service.WithRP(KeyReplicationRemoteSystem): "15dbbf5617523655", } - _, f.err = f.service.CreateRemoteVolume(*ctx, req) + _, f.err = f.service.CreateRemoteVolume(ctx, req) if f.err != nil { fmt.Printf("CreateRemoteVolumeRequest returned error: %s", f.err) } @@ -4377,7 +4377,7 @@ func (f *feature) iCallCreateRemoteVolume() error { } func (f *feature) iCallDeleteLocalVolume(name string) error { - ctx := new(context.Context) + ctx := context.Background() replicatedVolName := "replicated-" + name volumeHandle := arrayID2 + "-" + volumeNameToID[replicatedVolName] @@ -4395,7 +4395,7 @@ func (f *feature) iCallDeleteLocalVolume(name string) error { VolumeHandle: volumeHandle, } - _, f.err = f.service.DeleteLocalVolume(*ctx, req) + _, f.err = f.service.DeleteLocalVolume(ctx, req) if f.err != nil { fmt.Printf("DeleteLocalVolume returned error: %s", f.err) } @@ -4404,7 +4404,7 @@ func (f *feature) iCallDeleteLocalVolume(name string) error { } func (f *feature) iCallCreateStorageProtectionGroup() error { - ctx := new(context.Context) + ctx := context.Background() parameters := make(map[string]string) // Must be repeatable. @@ -4445,12 +4445,12 @@ func (f *feature) iCallCreateStorageProtectionGroup() error { if stepHandlersErrors.BadVolIDError { req.VolumeHandle = "0%0" } - f.createStorageProtectionGroupResponse, f.err = f.service.CreateStorageProtectionGroup(*ctx, req) + f.createStorageProtectionGroupResponse, f.err = f.service.CreateStorageProtectionGroup(ctx, req) return nil } func (f *feature) iCallCreateStorageProtectionGroupWith(arg1, arg2, arg3 string) error { - ctx := new(context.Context) + ctx := context.Background() parameters := make(map[string]string) // Must be repeatable. @@ -4467,12 +4467,12 @@ func (f *feature) iCallCreateStorageProtectionGroupWith(arg1, arg2, arg3 string) Parameters: parameters, } - f.createStorageProtectionGroupResponse, f.err = f.service.CreateStorageProtectionGroup(*ctx, req) + f.createStorageProtectionGroupResponse, f.err = f.service.CreateStorageProtectionGroup(ctx, req) return nil } func (f *feature) iCallGetStorageProtectionGroupStatus() error { - ctx := new(context.Context) + ctx := context.Background() attributes := make(map[string]string) replicationGroupConsistMode = defaultConsistencyMode @@ -4482,13 +4482,13 @@ func (f *feature) iCallGetStorageProtectionGroupStatus() error { ProtectionGroupId: f.createStorageProtectionGroupResponse.LocalProtectionGroupId, ProtectionGroupAttributes: attributes, } - _, f.err = f.service.GetStorageProtectionGroupStatus(*ctx, req) + _, f.err = f.service.GetStorageProtectionGroupStatus(ctx, req) return nil } func (f *feature) iCallGetStorageProtectionGroupStatusWithStateAndMode(arg1, arg2 string) error { - ctx := new(context.Context) + ctx := context.Background() attributes := make(map[string]string) replicationGroupState = arg1 @@ -4499,7 +4499,7 @@ func (f *feature) iCallGetStorageProtectionGroupStatusWithStateAndMode(arg1, arg ProtectionGroupId: f.createStorageProtectionGroupResponse.LocalProtectionGroupId, ProtectionGroupAttributes: attributes, } - _, f.err = f.service.GetStorageProtectionGroupStatus(*ctx, req) + _, f.err = f.service.GetStorageProtectionGroupStatus(ctx, req) return nil } @@ -4511,12 +4511,12 @@ func (f *feature) iCallDeleteVolume(name string) error { for name, id := range volumeNameToID { fmt.Printf("volNameToID name %s id %s\n", name, id) } - ctx := new(context.Context) + ctx := context.Background() req := f.getControllerDeleteVolumeRequest("single-writer") id := arrayID + "-" + volumeNameToID[name] log.Printf("iCallDeleteVolume name %s to ID %s", name, id) req.VolumeId = id - f.deleteVolumeResponse, f.err = f.service.DeleteVolume(*ctx, req) + f.deleteVolumeResponse, f.err = f.service.DeleteVolume(ctx, req) if f.err != nil { fmt.Printf("DeleteVolume error: %s", f.err) } @@ -4524,19 +4524,19 @@ func (f *feature) iCallDeleteVolume(name string) error { } func (f *feature) iCallDeleteStorageProtectionGroup() error { - ctx := new(context.Context) + ctx := context.Background() attributes := make(map[string]string) attributes[f.service.opts.replicationContextPrefix+"systemName"] = arrayID req := &replication.DeleteStorageProtectionGroupRequest{ ProtectionGroupId: f.createStorageProtectionGroupResponse.LocalProtectionGroupId, ProtectionGroupAttributes: attributes, } - f.deleteStorageProtectionGroupResponse, f.err = f.service.DeleteStorageProtectionGroup(*ctx, req) + f.deleteStorageProtectionGroupResponse, f.err = f.service.DeleteStorageProtectionGroup(ctx, req) return nil } func (f *feature) iCallExecuteAction(arg1 string) error { - ctx := new(context.Context) + ctx := context.Background() attributes := make(map[string]string) remoteAttributes := make(map[string]string) @@ -4575,7 +4575,7 @@ func (f *feature) iCallExecuteAction(arg1 string) error { ActionTypes: &action, } - _, f.err = f.service.ExecuteAction(*ctx, req) + _, f.err = f.service.ExecuteAction(ctx, req) return nil } @@ -4707,7 +4707,7 @@ func getZoneEnabledRequest(zoneLabelName string) *csi.CreateVolumeRequest { } func (f *feature) iCallCreateVolumeWithZones(name string) error { - ctx := new(context.Context) + ctx := context.Background() if f.createVolumeRequest == nil { req := getZoneEnabledRequest(f.service.opts.zoneLabelKey) f.createVolumeRequest = req @@ -4717,7 +4717,7 @@ func (f *feature) iCallCreateVolumeWithZones(name string) error { fmt.Println("I am in iCallCreateVolume fn.....") - f.createVolumeResponse, f.err = f.service.CreateVolume(*ctx, req) + f.createVolumeResponse, f.err = f.service.CreateVolume(ctx, req) if f.err != nil { log.Printf("CreateVolume called failed: %s\n", f.err.Error()) } @@ -4734,12 +4734,12 @@ func mockGetNodeLabelsWithZone(_ context.Context, s *service) (map[string]string } func (f *feature) iCallNodeGetInfoWithZoneLabels() error { - ctx := new(context.Context) + ctx := context.Background() req := new(csi.NodeGetInfoRequest) f.service.opts.SdcGUID = "9E56672F-2F4B-4A42-BFF4-88B6846FBFDA" GetNodeLabels = mockGetNodeLabelsWithZone GetNodeUID = mockGetNodeUID - f.nodeGetInfoResponse, f.err = f.service.NodeGetInfo(*ctx, req) + f.nodeGetInfoResponse, f.err = f.service.NodeGetInfo(ctx, req) return nil } From cf89ec3a25d5f43e4a869f4bb53a9357bd46fee3 Mon Sep 17 00:00:00 2001 From: "Lau, Luke" Date: Fri, 13 Dec 2024 11:17:22 -0500 Subject: [PATCH 18/40] update goscaleio for context impl --- go.mod | 2 +- go.sum | 2 ++ service/controller.go | 2 +- 3 files changed, 4 insertions(+), 2 deletions(-) diff --git a/go.mod b/go.mod index 0d92b50b..6581957e 100644 --- a/go.mod +++ b/go.mod @@ -15,7 +15,7 @@ require ( github.com/dell/dell-csi-extensions/volumeGroupSnapshot v1.7.0 github.com/dell/gocsi v1.12.0 github.com/dell/gofsutil v1.17.0 - github.com/dell/goscaleio v1.17.2-0.20241209165307-dcbadc33ab2e + github.com/dell/goscaleio v1.17.2-0.20241213145027-141cfe292cfa github.com/fsnotify/fsnotify v1.5.1 github.com/google/uuid v1.6.0 github.com/gorilla/mux v1.8.0 diff --git a/go.sum b/go.sum index 7fb2ec2d..f28649c4 100644 --- a/go.sum +++ b/go.sum @@ -116,6 +116,8 @@ github.com/dell/gofsutil v1.17.0 h1:QA6gUb1mz8kXNEN4eEx47OHCz8nSqZrrCnaDUYmV5EY= github.com/dell/gofsutil v1.17.0/go.mod h1:PN2hWl/pVLQiTsFR0X1x+GfhfOrfW8pGgH5xGcGMeFs= github.com/dell/goscaleio v1.17.2-0.20241209165307-dcbadc33ab2e h1:Y+F8YP3ceH6XRz0phFV5VpS2Pmoi8f0Vtg261/C2pZo= github.com/dell/goscaleio v1.17.2-0.20241209165307-dcbadc33ab2e/go.mod h1:7bX3rL8JWMmdifGr/UeD/Ju9wbkHUqvKDrbdu7XyGm8= +github.com/dell/goscaleio v1.17.2-0.20241213145027-141cfe292cfa h1:9honWWT9xEcI0OWyLtiWIDCaMAEpBAeyyzW+KPRVh10= +github.com/dell/goscaleio v1.17.2-0.20241213145027-141cfe292cfa/go.mod h1:7bX3rL8JWMmdifGr/UeD/Ju9wbkHUqvKDrbdu7XyGm8= github.com/dgrijalva/jwt-go v3.2.0+incompatible/go.mod h1:E3ru+11k8xSBh+hMPgOLZmtrrCbhqsmaPHjLKYnJCaQ= github.com/dgryski/go-sip13 v0.0.0-20181026042036-e10d5fee7954/go.mod h1:vAd38F8PWV+bWy6jNmig1y/TA+kYO4g3RSRF0IAv0no= github.com/docker/spdystream v0.0.0-20160310174837-449fdfce4d96/go.mod h1:Qh8CwZgvJUkLughtfhJv5dyTYa91l1fOUCrgjqmcifM= diff --git a/service/controller.go b/service/controller.go index fb3509d5..50e0d75d 100644 --- a/service/controller.go +++ b/service/controller.go @@ -2662,7 +2662,7 @@ func (s *service) systemProbe(ctx context.Context, array *ArrayConnectionData) e Log.Printf("Login to VxFlexOS Gateway, system=%s, endpoint=%s, user=%s\n", systemID, array.Endpoint, array.Username) if s.adminClients[systemID].GetToken() == "" { - _, err := s.adminClients[systemID].Authenticate(ctx, &goscaleio.ConfigConnect{ + _, err := s.adminClients[systemID].WithContext(ctx).Authenticate(&goscaleio.ConfigConnect{ Endpoint: array.Endpoint, Username: array.Username, Password: array.Password, From cf06c7f4b70c66ea2b66c08dd07b17415057aeaf Mon Sep 17 00:00:00 2001 From: "Lau, Luke" Date: Fri, 13 Dec 2024 11:17:59 -0500 Subject: [PATCH 19/40] clarify return value for Probe() --- service/identity.go | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/service/identity.go b/service/identity.go index 543f47be..d19184f3 100644 --- a/service/identity.go +++ b/service/identity.go @@ -93,10 +93,9 @@ func (s *service) Probe( return nil, err } } - ready := new(wrapperspb.BoolValue) - ready.Value = true - rep := new(csi.ProbeResponse) - rep.Ready = ready + rep := &csi.ProbeResponse{ + Ready: wrapperspb.Bool(true), + } Log.Debug(fmt.Sprintf("Probe returning: %v", rep.Ready.GetValue())) return rep, nil From 0b0d1359a37656b884ef1d15543e4557e988e0cb Mon Sep 17 00:00:00 2001 From: "Lau, Luke" Date: Fri, 13 Dec 2024 13:10:26 -0500 Subject: [PATCH 20/40] fix merge issue --- service/controller.go | 19 ------------------- 1 file changed, 19 deletions(-) diff --git a/service/controller.go b/service/controller.go index eba30880..9263832c 100644 --- a/service/controller.go +++ b/service/controller.go @@ -221,25 +221,6 @@ func (s *service) CreateVolume( // Handle Zone topology, which happens when node is annotated with a matching zone label if len(zoneTargetMap) != 0 && accessibility != nil && len(accessibility.GetPreferred()) > 0 { - contentSource := req.GetVolumeContentSource() - var sourceSystemID string - if contentSource != nil { - Log.Infof("[CreateVolume] Volume has a content source - we are a snapshot or clone: %+v", contentSource) - - snapshotSource := contentSource.GetSnapshot() - cloneSource := contentSource.GetVolume() - - if snapshotSource != nil { - sourceSystemID = s.getSystemIDFromCsiVolumeID(snapshotSource.SnapshotId) - Log.Infof("[CreateVolume] Snapshot source systemID: %s", sourceSystemID) - } else if cloneSource != nil { - sourceSystemID = s.getSystemIDFromCsiVolumeID(cloneSource.VolumeId) - Log.Infof("[CreateVolume] Clone source systemID: %s", sourceSystemID) - } - } else { - Log.Infoln("[CreateVolume] Volume does not have a content source - not a snapshot") - } - contentSource := req.GetVolumeContentSource() var sourceSystemID string if contentSource != nil { From 809940303f16ce01a281d24407a4b5c0a0b82489 Mon Sep 17 00:00:00 2001 From: "Lau, Luke" Date: Mon, 16 Dec 2024 15:19:48 -0500 Subject: [PATCH 21/40] use withContext from goscaleio --- go.mod | 2 +- go.sum | 4 ++++ service/controller.go | 2 +- 3 files changed, 6 insertions(+), 2 deletions(-) diff --git a/go.mod b/go.mod index 6581957e..28af9f89 100644 --- a/go.mod +++ b/go.mod @@ -15,7 +15,7 @@ require ( github.com/dell/dell-csi-extensions/volumeGroupSnapshot v1.7.0 github.com/dell/gocsi v1.12.0 github.com/dell/gofsutil v1.17.0 - github.com/dell/goscaleio v1.17.2-0.20241213145027-141cfe292cfa + github.com/dell/goscaleio v1.17.2-0.20241213215244-2164caaef4ab github.com/fsnotify/fsnotify v1.5.1 github.com/google/uuid v1.6.0 github.com/gorilla/mux v1.8.0 diff --git a/go.sum b/go.sum index f28649c4..7b338a59 100644 --- a/go.sum +++ b/go.sum @@ -118,6 +118,10 @@ github.com/dell/goscaleio v1.17.2-0.20241209165307-dcbadc33ab2e h1:Y+F8YP3ceH6XR github.com/dell/goscaleio v1.17.2-0.20241209165307-dcbadc33ab2e/go.mod h1:7bX3rL8JWMmdifGr/UeD/Ju9wbkHUqvKDrbdu7XyGm8= github.com/dell/goscaleio v1.17.2-0.20241213145027-141cfe292cfa h1:9honWWT9xEcI0OWyLtiWIDCaMAEpBAeyyzW+KPRVh10= github.com/dell/goscaleio v1.17.2-0.20241213145027-141cfe292cfa/go.mod h1:7bX3rL8JWMmdifGr/UeD/Ju9wbkHUqvKDrbdu7XyGm8= +github.com/dell/goscaleio v1.17.2-0.20241213204026-19006b56eb26 h1:Kg6MSwBmAlmUDWRKiG0YJRv1xd8qi9+mW7vAVNnghj4= +github.com/dell/goscaleio v1.17.2-0.20241213204026-19006b56eb26/go.mod h1:7bX3rL8JWMmdifGr/UeD/Ju9wbkHUqvKDrbdu7XyGm8= +github.com/dell/goscaleio v1.17.2-0.20241213215244-2164caaef4ab h1:DYWY7fs8v1VbmzF2pAx7peZtKhkppW5NCIyHCJN1fS4= +github.com/dell/goscaleio v1.17.2-0.20241213215244-2164caaef4ab/go.mod h1:7bX3rL8JWMmdifGr/UeD/Ju9wbkHUqvKDrbdu7XyGm8= github.com/dgrijalva/jwt-go v3.2.0+incompatible/go.mod h1:E3ru+11k8xSBh+hMPgOLZmtrrCbhqsmaPHjLKYnJCaQ= github.com/dgryski/go-sip13 v0.0.0-20181026042036-e10d5fee7954/go.mod h1:vAd38F8PWV+bWy6jNmig1y/TA+kYO4g3RSRF0IAv0no= github.com/docker/spdystream v0.0.0-20160310174837-449fdfce4d96/go.mod h1:Qh8CwZgvJUkLughtfhJv5dyTYa91l1fOUCrgjqmcifM= diff --git a/service/controller.go b/service/controller.go index 9263832c..ee178631 100644 --- a/service/controller.go +++ b/service/controller.go @@ -2671,7 +2671,7 @@ func (s *service) systemProbe(ctx context.Context, array *ArrayConnectionData) e // initialize system if needed if s.systems[systemID] == nil { - system, err := s.adminClients[systemID].FindSystem( + system, err := s.adminClients[systemID].WithContext(ctx).FindSystem( array.SystemID, array.SystemID, "") if err != nil { return status.Errorf(codes.FailedPrecondition, From 59788192c455a3864655dd79ccf350319c44f76c Mon Sep 17 00:00:00 2001 From: "Lau, Luke" Date: Mon, 16 Dec 2024 15:21:54 -0500 Subject: [PATCH 22/40] refactor changes and add UT --- service/controller.go | 81 ++++++++---- service/controller_test.go | 263 +++++++++++++++++++++++++++++++++++++ 2 files changed, 316 insertions(+), 28 deletions(-) create mode 100644 service/controller_test.go diff --git a/service/controller.go b/service/controller.go index ee178631..7f37d402 100644 --- a/service/controller.go +++ b/service/controller.go @@ -2375,16 +2375,9 @@ func (s *service) GetCapacity( // If using availability zones, get capacity for the system in the zone // using accessible topology parameter from k8s. if s.opts.zoneLabelKey != "" { - zoneLabel, ok := req.AccessibleTopology.Segments[s.opts.zoneLabelKey] - if !ok { - Log.Infof("could not get availability zone from accessible topology. Getting capacity for all systems") - } else { - for _, array := range s.opts.arrays { - if zoneLabel == string(array.AvailabilityZone.Name) { - systemID = array.SystemID - break - } - } + systemID, err = s.getSystemIDFromZoneLabelKey(req) + if err != nil { + return nil, status.Errorf(codes.Internal, "%s", err.Error()) } } @@ -2430,6 +2423,28 @@ func (s *service) GetCapacity( }, nil } +// getSystemIDFromZoneLabelKey returns the system ID associated with the zoneLabelKey if zoneLabelKey is set and +// contains an associated zone name. Returns an empty string otherwise. +func (s *service) getSystemIDFromZoneLabelKey(req *csi.GetCapacityRequest) (systemID string, err error) { + zoneName, ok := req.AccessibleTopology.Segments[s.opts.zoneLabelKey] + if !ok { + Log.Infof("could not get availability zone from accessible topology. Getting capacity for all systems") + return "", nil + } + + // find the systemID with the matching zone name + for _, array := range s.opts.arrays { + if zoneName == string(array.AvailabilityZone.Name) { + systemID = array.SystemID + break + } + } + if systemID == "" { + return "", fmt.Errorf("could not find an array assigned to zone '%s'", zoneName) + } + return systemID, nil +} + func (s *service) getMaximumVolumeSize(systemID string) (int64, error) { valueInCache, found := getCachedMaximumVolumeSize(systemID) if !found || valueInCache < 0 { @@ -2563,36 +2578,46 @@ func (s *service) ControllerGetCapabilities( }, nil } +func (s *service) getZoneFromZoneLabelKey(ctx context.Context, zoneLabelKey string) (zone string, err error) { + if zoneLabelKey == "" { + return "", nil + } + + labels, err := GetNodeLabels(ctx, s) + if err != nil { + return "", err + } + + Log.Infof("Listing labels: %v", labels) + + if val, ok := labels[zoneLabelKey]; ok { + Log.Infof("probing zoneLabel %s, zone value: %s", zoneLabelKey, val) + return val, nil + } + + return "", fmt.Errorf("label %s not found", zoneLabelKey) +} + // systemProbeAll will iterate through all arrays in service.opts.arrays and probe them. If failed, it logs // the failed system name -func (s *service) systemProbeAll(ctx context.Context, zoneLabel string) error { +func (s *service) systemProbeAll(ctx context.Context, zoneLabelKey string) error { // probe all arrays // Log.Infof("Probing all arrays. Number of arrays: %d", len(s.opts.arrays)) Log.Infoln("Probing all associated arrays") allArrayFail := true errMap := make(map[string]error) - zone := "" - if zoneLabel != "" { - labels, err := GetNodeLabels(ctx, s) - if err != nil { - return err - } - - Log.Infof("Listing labels: %v", labels) - - if val, ok := labels[zoneLabel]; ok { - Log.Infof("probing zoneLabel %s, zone value: %s", zoneLabel, val) - zone = val - } else { - return fmt.Errorf("label %s not found", zoneLabel) - } + zoneName, err := s.getZoneFromZoneLabelKey(ctx, zoneLabelKey) + if err != nil { + return err } for _, array := range s.opts.arrays { // If zone information is available, use it to probe the array - if strings.EqualFold(s.mode, "node") && array.AvailabilityZone != nil && array.AvailabilityZone.Name != ZoneName(zone) { - Log.Warnf("array %s zone %s does not match %s, not pinging this array\n", array.SystemID, array.AvailabilityZone.Name, zone) + if strings.EqualFold(s.mode, "node") && array.AvailabilityZone != nil && array.AvailabilityZone.Name != ZoneName(zoneName) { + // Driver node containers should not probe arrays that exist outside their assigned zone + // Driver controller container should probe all arrays + Log.Warnf("array %s zone %s does not match %s, not pinging this array\n", array.SystemID, array.AvailabilityZone.Name, zoneName) continue } diff --git a/service/controller_test.go b/service/controller_test.go new file mode 100644 index 00000000..d4d532f4 --- /dev/null +++ b/service/controller_test.go @@ -0,0 +1,263 @@ +// Copyright © 2019-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 ( + "errors" + "sync" + "testing" + + csi "github.com/container-storage-interface/spec/lib/go/csi" + sio "github.com/dell/goscaleio" + siotypes "github.com/dell/goscaleio/types/v1" + "golang.org/x/net/context" +) + +func Test_service_getZoneFromZoneLabelKey(t *testing.T) { + type fields struct { + opts Opts + adminClients map[string]*sio.Client + systems map[string]*sio.System + mode string + volCache []*siotypes.Volume + volCacheRWL sync.RWMutex + volCacheSystemID string + snapCache []*siotypes.Volume + snapCacheRWL sync.RWMutex + snapCacheSystemID string + privDir string + storagePoolIDToName map[string]string + statisticsCounter int + volumePrefixToSystems map[string][]string + connectedSystemNameToID map[string]string + } + type args struct { + ctx context.Context + zoneLabelKey string + } + tests := []struct { + name string + fields fields + args args + wantZone string + wantErr bool + getNodeLabelFunc func(ctx context.Context, s *service) (map[string]string, error) + }{ + { + name: "get good zone label", + args: args{ + ctx: context.Background(), + zoneLabelKey: "topology.kubernetes.io/zone", + }, + wantZone: "zoneA", + wantErr: false, + getNodeLabelFunc: func(ctx context.Context, s *service) (map[string]string, error) { + nodeLabels := map[string]string{"topology.kubernetes.io/zone": "zoneA"} + return nodeLabels, nil + }, + }, + { + name: "use bad zone label key", + args: args{ + ctx: context.Background(), + zoneLabelKey: "badkey", + }, + wantZone: "", + wantErr: true, + getNodeLabelFunc: func(ctx context.Context, s *service) (map[string]string, error) { + return nil, nil + }, + }, + { + name: "fail to get node labels", + args: args{ + ctx: context.Background(), + zoneLabelKey: "unimportant", + }, + wantZone: "", + wantErr: true, + getNodeLabelFunc: func(ctx context.Context, s *service) (map[string]string, error) { + return nil, errors.New("") + }, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + s := &service{ + opts: tt.fields.opts, + adminClients: tt.fields.adminClients, + systems: tt.fields.systems, + mode: tt.fields.mode, + volCache: tt.fields.volCache, + volCacheRWL: tt.fields.volCacheRWL, + volCacheSystemID: tt.fields.volCacheSystemID, + snapCache: tt.fields.snapCache, + snapCacheRWL: tt.fields.snapCacheRWL, + snapCacheSystemID: tt.fields.snapCacheSystemID, + privDir: tt.fields.privDir, + storagePoolIDToName: tt.fields.storagePoolIDToName, + statisticsCounter: tt.fields.statisticsCounter, + volumePrefixToSystems: tt.fields.volumePrefixToSystems, + connectedSystemNameToID: tt.fields.connectedSystemNameToID, + } + GetNodeLabels = tt.getNodeLabelFunc + gotZone, err := s.getZoneFromZoneLabelKey(tt.args.ctx, tt.args.zoneLabelKey) + if (err != nil) != tt.wantErr { + t.Errorf("service.getZoneFromZoneLabelKey() error = %v, wantErr %v", err, tt.wantErr) + return + } + if gotZone != tt.wantZone { + t.Errorf("service.getZoneFromZoneLabelKey() = %v, want %v", gotZone, tt.wantZone) + } + }) + } +} + +func Test_service_getSystemIDFromZoneLabelKey(t *testing.T) { + type fields struct { + opts Opts + adminClients map[string]*sio.Client + systems map[string]*sio.System + mode string + volCache []*siotypes.Volume + volCacheRWL sync.RWMutex + volCacheSystemID string + snapCache []*siotypes.Volume + snapCacheRWL sync.RWMutex + snapCacheSystemID string + privDir string + storagePoolIDToName map[string]string + statisticsCounter int + volumePrefixToSystems map[string][]string + connectedSystemNameToID map[string]string + } + type args struct { + req *csi.GetCapacityRequest + } + const validSystemID = "valid-id" + const validTopologyKey = "topology.kubernetes.io/zone" + const validZone = "zoneA" + + tests := []struct { + name string + fields fields + args args + wantSystemID string + wantErr bool + }{ + { + name: "get a valid system ID", + wantErr: false, + wantSystemID: validSystemID, + args: args{ + req: &csi.GetCapacityRequest{ + AccessibleTopology: &csi.Topology{ + Segments: map[string]string{ + validTopologyKey: validZone, + }, + }, + }, + }, + fields: fields{ + opts: Opts{ + zoneLabelKey: "topology.kubernetes.io/zone", + arrays: map[string]*ArrayConnectionData{ + "array1": { + SystemID: validSystemID, + AvailabilityZone: &AvailabilityZone{ + Name: validZone, + }, + }, + }, + }, + }, + }, + { + name: "topology not passed with csi request", + wantErr: false, + wantSystemID: "", + args: args{ + req: &csi.GetCapacityRequest{ + AccessibleTopology: &csi.Topology{ + // don't pass any topology info with the request + Segments: map[string]string{}, + }, + }, + }, + fields: fields{ + opts: Opts{ + zoneLabelKey: "topology.kubernetes.io/zone", + }, + }, + }, + { + name: "zone name missing in secret", + wantErr: true, + wantSystemID: "", + args: args{ + req: &csi.GetCapacityRequest{ + AccessibleTopology: &csi.Topology{ + Segments: map[string]string{ + validTopologyKey: validZone, + }, + }, + }, + }, + fields: fields{ + opts: Opts{ + zoneLabelKey: "topology.kubernetes.io/zone", + arrays: map[string]*ArrayConnectionData{ + "array1": { + SystemID: validSystemID, + AvailabilityZone: &AvailabilityZone{ + // ensure the zone name will not match the topology key value + // in the request + Name: validZone + "no-match", + }, + }, + }, + }, + }, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + s := &service{ + opts: tt.fields.opts, + adminClients: tt.fields.adminClients, + systems: tt.fields.systems, + mode: tt.fields.mode, + volCache: tt.fields.volCache, + volCacheRWL: tt.fields.volCacheRWL, + volCacheSystemID: tt.fields.volCacheSystemID, + snapCache: tt.fields.snapCache, + snapCacheRWL: tt.fields.snapCacheRWL, + snapCacheSystemID: tt.fields.snapCacheSystemID, + privDir: tt.fields.privDir, + storagePoolIDToName: tt.fields.storagePoolIDToName, + statisticsCounter: tt.fields.statisticsCounter, + volumePrefixToSystems: tt.fields.volumePrefixToSystems, + connectedSystemNameToID: tt.fields.connectedSystemNameToID, + } + gotSystemID, err := s.getSystemIDFromZoneLabelKey(tt.args.req) + if (err != nil) != tt.wantErr { + t.Errorf("service.getSystemIDFromZoneLabelKey() error = %v, wantErr %v", err, tt.wantErr) + return + } + if gotSystemID != tt.wantSystemID { + t.Errorf("service.getSystemIDFromZoneLabelKey() = %v, want %v", gotSystemID, tt.wantSystemID) + } + }) + } +} From d0ff2fc9e023d7f0f85aec2b799c68aa81367828 Mon Sep 17 00:00:00 2001 From: "Lau, Luke" Date: Tue, 17 Dec 2024 10:08:33 -0500 Subject: [PATCH 23/40] remove debug logs --- service/controller.go | 5 ----- service/identity.go | 2 +- 2 files changed, 1 insertion(+), 6 deletions(-) diff --git a/service/controller.go b/service/controller.go index 7f37d402..63104220 100644 --- a/service/controller.go +++ b/service/controller.go @@ -2270,8 +2270,6 @@ func (s *service) getSystemCapacity(ctx context.Context, systemID, protectionDom return 0, fmt.Errorf("can't find adminClient or system by id %s", systemID) } - Log.Printf("[FERNANDO] adminClient: %v, system: %v", adminClient, system) - var statsFunc func() (*siotypes.Statistics, error) // Default to get Capacity of system @@ -2320,8 +2318,6 @@ func (s *service) getCapacityForAllSystems(ctx context.Context, protectionDomain var systemCapacity int64 var err error - Log.Printf("[FERNANDO] Get capacity for system: %s, pool %d, protection domain %s", array.SystemID, len(spName), protectionDomain) - if len(spName) > 0 && spName[0] != "" { systemCapacity, err = s.getSystemCapacity(ctx, array.SystemID, protectionDomain, spName[0]) } else { @@ -2602,7 +2598,6 @@ func (s *service) getZoneFromZoneLabelKey(ctx context.Context, zoneLabelKey stri // the failed system name func (s *service) systemProbeAll(ctx context.Context, zoneLabelKey string) error { // probe all arrays - // Log.Infof("Probing all arrays. Number of arrays: %d", len(s.opts.arrays)) Log.Infoln("Probing all associated arrays") allArrayFail := true errMap := make(map[string]error) diff --git a/service/identity.go b/service/identity.go index d19184f3..761d0be4 100644 --- a/service/identity.go +++ b/service/identity.go @@ -80,7 +80,7 @@ func (s *service) Probe( Log.Infof("[Probe] Probe context: %v", ctx) if !strings.EqualFold(s.mode, "node") { - Log.Infoln("[Probe] FERNANDO we are probing the controller") + Log.Debug("systemProbe") if err := s.systemProbeAll(ctx, ""); err != nil { Log.Printf("error in systemProbeAll: %s", err.Error()) return nil, err From addfc10e74a7cc89ed2ca8e081ac36f08e41156d Mon Sep 17 00:00:00 2001 From: "Lau, Luke" Date: Tue, 17 Dec 2024 10:43:35 -0500 Subject: [PATCH 24/40] support context when adding pod labels --- service/service.go | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/service/service.go b/service/service.go index 84859106..44b49742 100644 --- a/service/service.go +++ b/service/service.go @@ -1845,7 +1845,7 @@ func (s *service) GetNodeLabels(_ context.Context) (map[string]string, error) { return node.Labels, nil } -func (s *service) SetPodZoneLabel(_ context.Context, zoneLabel map[string]string) error { +func (s *service) SetPodZoneLabel(ctx context.Context, zoneLabel map[string]string) error { if K8sClientset == nil { err := k8sutils.CreateKubeClientSet() if err != nil { @@ -1857,7 +1857,7 @@ func (s *service) SetPodZoneLabel(_ context.Context, zoneLabel map[string]string Log.Println("SetPodZoneLabel") // access the API to fetch node object - pods, err := K8sClientset.CoreV1().Pods(DriverNamespace).List(context.TODO(), v1.ListOptions{}) + pods, err := K8sClientset.CoreV1().Pods(DriverNamespace).List(ctx, v1.ListOptions{}) if err != nil { return status.Error(codes.Internal, GetMessage("Unable to fetch the node labels. Error: %v", err)) } @@ -1869,7 +1869,7 @@ func (s *service) SetPodZoneLabel(_ context.Context, zoneLabel map[string]string } } - pod, err := K8sClientset.CoreV1().Pods(DriverNamespace).Get(context.TODO(), podName, v1.GetOptions{}) + pod, err := K8sClientset.CoreV1().Pods(DriverNamespace).Get(ctx, podName, v1.GetOptions{}) if err != nil { return status.Error(codes.Internal, GetMessage("Unable to fetch the node labels. Error: %v", err)) } @@ -1879,9 +1879,9 @@ func (s *service) SetPodZoneLabel(_ context.Context, zoneLabel map[string]string pod.Labels[key] = value } - _, err = K8sClientset.CoreV1().Pods(DriverNamespace).Update(context.TODO(), pod, v1.UpdateOptions{}) + _, err = K8sClientset.CoreV1().Pods(DriverNamespace).Update(ctx, pod, v1.UpdateOptions{}) if err != nil { - return status.Error(codes.Internal, GetMessage("Unable to fetch the node labels. Error: %v", err)) + return status.Error(codes.Internal, GetMessage("Unable to update the node labels. Error: %v", err)) } return nil From 1e0c2330cf8a565402c0021ba422d386b5cbb719 Mon Sep 17 00:00:00 2001 From: "Lau, Luke" Date: Tue, 17 Dec 2024 11:03:20 -0500 Subject: [PATCH 25/40] add tests to SetPodZoneLabel --- service/service_test.go | 152 +++++++++++++++++++++++++++++++++++++++- 1 file changed, 151 insertions(+), 1 deletion(-) diff --git a/service/service_test.go b/service/service_test.go index ee3809cb..f3fab9ff 100644 --- a/service/service_test.go +++ b/service/service_test.go @@ -1,4 +1,4 @@ -// Copyright © 2019-2022 Dell Inc. or its subsidiaries. All Rights Reserved. +// Copyright © 2019-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. @@ -14,13 +14,20 @@ package service import ( + "context" "fmt" "net/http" "os" + "sync" "testing" "time" "github.com/cucumber/godog" + sio "github.com/dell/goscaleio" + siotypes "github.com/dell/goscaleio/types/v1" + v1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/client-go/kubernetes/fake" ) func TestMain(m *testing.M) { @@ -56,3 +63,146 @@ func TestMain(m *testing.M) { os.Exit(status) } + +func Test_service_SetPodZoneLabel(t *testing.T) { + type fields struct { + opts Opts + adminClients map[string]*sio.Client + systems map[string]*sio.System + mode string + volCache []*siotypes.Volume + volCacheSystemID string + snapCache []*siotypes.Volume + snapCacheSystemID string + privDir string + storagePoolIDToName map[string]string + statisticsCounter int + volumePrefixToSystems map[string][]string + connectedSystemNameToID map[string]string + } + + type args struct { + ctx context.Context + zoneLabel map[string]string + } + + const validZoneName = "zoneA" + const validZoneLabelKey = "topology.kubernetes.io/zone" + const validAppName = "test-app" + const validAppLabelKey = "app" + const validNodeName = "kube-node-name" + validAppLabels := map[string]string{validAppLabelKey: validAppName} + + tests := []struct { + name string + fields fields + args args + initTest func(s *service) + wantErr bool + }{ + { + // happy path test + name: "add zone labels to a pod", + wantErr: false, + args: args{ + ctx: context.Background(), + zoneLabel: map[string]string{ + validZoneLabelKey: validZoneName, + }, + }, + fields: fields{ + opts: Opts{ + KubeNodeName: validNodeName, + }, + }, + initTest: func(s *service) { + // setup fake k8s client and create a pod to perform tests against + K8sClientset = fake.NewSimpleClientset() + podClient := K8sClientset.CoreV1().Pods(DriverNamespace) + + // create test pod + _, err := podClient.Create(context.Background(), &v1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: validAppName, + Labels: validAppLabels, + }, + Spec: v1.PodSpec{ + NodeName: s.opts.KubeNodeName, + }, + }, metav1.CreateOptions{}) + if err != nil { + t.Errorf("error creating test pod error = %v", err) + } + }, + }, + { + // Attempt to set pod labels when the k8s client cannot get pods + name: "when 'list pods' k8s client request fails", + wantErr: true, + args: args{ + ctx: context.Background(), + zoneLabel: map[string]string{ + validZoneLabelKey: validZoneName, + }, + }, + fields: fields{ + opts: Opts{ + KubeNodeName: validNodeName, + }, + }, + initTest: func(s *service) { + // create a client, but do not create any pods so the request + // to list pods fails + K8sClientset = fake.NewSimpleClientset() + }, + }, + { + name: "clientset is nil and fails to create one", + wantErr: true, + args: args{ + ctx: context.Background(), + zoneLabel: map[string]string{ + validZoneLabelKey: validZoneName, + }, + }, + fields: fields{ + opts: Opts{ + KubeNodeName: validNodeName, + }, + }, + initTest: func(s *service) { + // setup clientset to nil to force creation + // Creation should fail because tests are not run in a cluster + K8sClientset = nil + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + s := &service{ + opts: tt.fields.opts, + adminClients: tt.fields.adminClients, + systems: tt.fields.systems, + mode: tt.fields.mode, + volCache: tt.fields.volCache, + volCacheRWL: sync.RWMutex{}, + volCacheSystemID: tt.fields.volCacheSystemID, + snapCache: tt.fields.snapCache, + snapCacheRWL: sync.RWMutex{}, + snapCacheSystemID: tt.fields.snapCacheSystemID, + privDir: tt.fields.privDir, + storagePoolIDToName: tt.fields.storagePoolIDToName, + statisticsCounter: tt.fields.statisticsCounter, + volumePrefixToSystems: tt.fields.volumePrefixToSystems, + connectedSystemNameToID: tt.fields.connectedSystemNameToID, + } + + tt.initTest(s) + err := s.SetPodZoneLabel(tt.args.ctx, tt.args.zoneLabel) + if (err != nil) != tt.wantErr { + t.Errorf("service.SetPodZoneLabel() error = %v, wantErr %v", err, tt.wantErr) + } + }) + } +} From 8011ee1ceebb00c7a9b9e20246245252089ec617 Mon Sep 17 00:00:00 2001 From: "Lau, Luke" Date: Tue, 17 Dec 2024 11:06:26 -0500 Subject: [PATCH 26/40] tidying controller_tests --- service/controller_test.go | 39 ++++++++++++++++++++++++-------------- 1 file changed, 25 insertions(+), 14 deletions(-) diff --git a/service/controller_test.go b/service/controller_test.go index d4d532f4..7ac18b6e 100644 --- a/service/controller_test.go +++ b/service/controller_test.go @@ -1,4 +1,4 @@ -// Copyright © 2019-2024 Dell Inc. or its subsidiaries. All Rights Reserved. +// 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. @@ -31,10 +31,8 @@ func Test_service_getZoneFromZoneLabelKey(t *testing.T) { systems map[string]*sio.System mode string volCache []*siotypes.Volume - volCacheRWL sync.RWMutex volCacheSystemID string snapCache []*siotypes.Volume - snapCacheRWL sync.RWMutex snapCacheSystemID string privDir string storagePoolIDToName map[string]string @@ -42,10 +40,15 @@ func Test_service_getZoneFromZoneLabelKey(t *testing.T) { volumePrefixToSystems map[string][]string connectedSystemNameToID map[string]string } + type args struct { ctx context.Context zoneLabelKey string } + + const validTopologyKey = "topology.kubernetes.io/zone" + const validZone = "zoneA" + tests := []struct { name string fields fields @@ -55,19 +58,21 @@ func Test_service_getZoneFromZoneLabelKey(t *testing.T) { getNodeLabelFunc func(ctx context.Context, s *service) (map[string]string, error) }{ { + // happy path test name: "get good zone label", args: args{ ctx: context.Background(), - zoneLabelKey: "topology.kubernetes.io/zone", + zoneLabelKey: validTopologyKey, }, wantZone: "zoneA", wantErr: false, getNodeLabelFunc: func(ctx context.Context, s *service) (map[string]string, error) { - nodeLabels := map[string]string{"topology.kubernetes.io/zone": "zoneA"} + nodeLabels := map[string]string{validTopologyKey: validZone} return nodeLabels, nil }, }, { + // The key args.zoneLabelKey will not be found in the map returned by getNodeLabelFunc name: "use bad zone label key", args: args{ ctx: context.Background(), @@ -80,6 +85,7 @@ func Test_service_getZoneFromZoneLabelKey(t *testing.T) { }, }, { + // getNodeLabelFunc will return an error, triggering failure to get the labels name: "fail to get node labels", args: args{ ctx: context.Background(), @@ -100,10 +106,10 @@ func Test_service_getZoneFromZoneLabelKey(t *testing.T) { systems: tt.fields.systems, mode: tt.fields.mode, volCache: tt.fields.volCache, - volCacheRWL: tt.fields.volCacheRWL, + volCacheRWL: sync.RWMutex{}, volCacheSystemID: tt.fields.volCacheSystemID, snapCache: tt.fields.snapCache, - snapCacheRWL: tt.fields.snapCacheRWL, + snapCacheRWL: sync.RWMutex{}, snapCacheSystemID: tt.fields.snapCacheSystemID, privDir: tt.fields.privDir, storagePoolIDToName: tt.fields.storagePoolIDToName, @@ -131,10 +137,8 @@ func Test_service_getSystemIDFromZoneLabelKey(t *testing.T) { systems map[string]*sio.System mode string volCache []*siotypes.Volume - volCacheRWL sync.RWMutex volCacheSystemID string snapCache []*siotypes.Volume - snapCacheRWL sync.RWMutex snapCacheSystemID string privDir string storagePoolIDToName map[string]string @@ -142,9 +146,11 @@ func Test_service_getSystemIDFromZoneLabelKey(t *testing.T) { volumePrefixToSystems map[string][]string connectedSystemNameToID map[string]string } + type args struct { req *csi.GetCapacityRequest } + const validSystemID = "valid-id" const validTopologyKey = "topology.kubernetes.io/zone" const validZone = "zoneA" @@ -157,6 +163,7 @@ func Test_service_getSystemIDFromZoneLabelKey(t *testing.T) { wantErr bool }{ { + // happy path test name: "get a valid system ID", wantErr: false, wantSystemID: validSystemID, @@ -171,7 +178,7 @@ func Test_service_getSystemIDFromZoneLabelKey(t *testing.T) { }, fields: fields{ opts: Opts{ - zoneLabelKey: "topology.kubernetes.io/zone", + zoneLabelKey: validTopologyKey, arrays: map[string]*ArrayConnectionData{ "array1": { SystemID: validSystemID, @@ -184,6 +191,8 @@ func Test_service_getSystemIDFromZoneLabelKey(t *testing.T) { }, }, { + // should return an empty string if no topology info is passed + // with the csi request name: "topology not passed with csi request", wantErr: false, wantSystemID: "", @@ -197,11 +206,13 @@ func Test_service_getSystemIDFromZoneLabelKey(t *testing.T) { }, fields: fields{ opts: Opts{ - zoneLabelKey: "topology.kubernetes.io/zone", + zoneLabelKey: validTopologyKey, }, }, }, { + // topology information in the csi request does not match + // any of the arrays in the secret name: "zone name missing in secret", wantErr: true, wantSystemID: "", @@ -216,7 +227,7 @@ func Test_service_getSystemIDFromZoneLabelKey(t *testing.T) { }, fields: fields{ opts: Opts{ - zoneLabelKey: "topology.kubernetes.io/zone", + zoneLabelKey: validTopologyKey, arrays: map[string]*ArrayConnectionData{ "array1": { SystemID: validSystemID, @@ -239,10 +250,10 @@ func Test_service_getSystemIDFromZoneLabelKey(t *testing.T) { systems: tt.fields.systems, mode: tt.fields.mode, volCache: tt.fields.volCache, - volCacheRWL: tt.fields.volCacheRWL, + volCacheRWL: sync.RWMutex{}, volCacheSystemID: tt.fields.volCacheSystemID, snapCache: tt.fields.snapCache, - snapCacheRWL: tt.fields.snapCacheRWL, + snapCacheRWL: sync.RWMutex{}, snapCacheSystemID: tt.fields.snapCacheSystemID, privDir: tt.fields.privDir, storagePoolIDToName: tt.fields.storagePoolIDToName, From 7582c91b55b5cc4e67fa51ea131db68a20645f9e Mon Sep 17 00:00:00 2001 From: "Lau, Luke" Date: Tue, 17 Dec 2024 11:19:26 -0500 Subject: [PATCH 27/40] fix int test --- test/integration/step_defs_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/integration/step_defs_test.go b/test/integration/step_defs_test.go index 869724e6..f718fe33 100644 --- a/test/integration/step_defs_test.go +++ b/test/integration/step_defs_test.go @@ -116,7 +116,7 @@ func (f *feature) getGoscaleioClient() (client *goscaleio.Client, err error) { if err != nil { log.Fatalf("err getting client: %v", err) } - _, err = client.Authenticate(context.Background(), &goscaleio.ConfigConnect{ + _, err = client.Authenticate(&goscaleio.ConfigConnect{ Username: a.Username, Password: a.Password, Endpoint: a.Endpoint, From 5415a15c8b933e999d04224e83f4b722eea9cc31 Mon Sep 17 00:00:00 2001 From: "Lau, Luke" Date: Tue, 17 Dec 2024 11:28:46 -0500 Subject: [PATCH 28/40] linting --- service/controller_test.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/service/controller_test.go b/service/controller_test.go index 7ac18b6e..a6f044a1 100644 --- a/service/controller_test.go +++ b/service/controller_test.go @@ -66,7 +66,7 @@ func Test_service_getZoneFromZoneLabelKey(t *testing.T) { }, wantZone: "zoneA", wantErr: false, - getNodeLabelFunc: func(ctx context.Context, s *service) (map[string]string, error) { + getNodeLabelFunc: func(_ context.Context, _ *service) (map[string]string, error) { nodeLabels := map[string]string{validTopologyKey: validZone} return nodeLabels, nil }, @@ -80,7 +80,7 @@ func Test_service_getZoneFromZoneLabelKey(t *testing.T) { }, wantZone: "", wantErr: true, - getNodeLabelFunc: func(ctx context.Context, s *service) (map[string]string, error) { + getNodeLabelFunc: func(_ context.Context, _ *service) (map[string]string, error) { return nil, nil }, }, @@ -93,7 +93,7 @@ func Test_service_getZoneFromZoneLabelKey(t *testing.T) { }, wantZone: "", wantErr: true, - getNodeLabelFunc: func(ctx context.Context, s *service) (map[string]string, error) { + getNodeLabelFunc: func(_ context.Context, _ *service) (map[string]string, error) { return nil, errors.New("") }, }, From 6671d8d632d675ea0508cf5743d24a87ecab6eed Mon Sep 17 00:00:00 2001 From: "Lau, Luke" Date: Tue, 17 Dec 2024 13:27:50 -0500 Subject: [PATCH 29/40] adding more tests to cover additions --- service/features/service.feature | 12 ++++++++++++ service/step_defs_test.go | 25 +++++++++++++++++++++++++ 2 files changed, 37 insertions(+) diff --git a/service/features/service.feature b/service/features/service.feature index c1a8dce8..7e804472 100644 --- a/service/features/service.feature +++ b/service/features/service.feature @@ -490,6 +490,18 @@ Feature: VxFlex OS CSI interface Given a VxFlexOS service When I call Probe And I call GetCapacity with storage pool "" + + Scenario: Call GetCapacity for a system using Availability zones + Given a VxFlexOS service + And I use config + When I call Probe + And I call GetCapacity with Availability Zone + Then the error contains + + Examples: + | config | zone-key | zone-name | errorMsg | + | "multi_az" | "zone.csi-vxflexos.dellemc.com" | "zoneA" | "none" | + | "multi_az" | "zone.csi-vxflexos.dellemc.com" | "badZone" | "could not find an array assigned to zone 'badZone'" | Scenario: Call GetCapacity with valid Storage Pool Name Given a VxFlexOS service diff --git a/service/step_defs_test.go b/service/step_defs_test.go index 153f0ab0..7084b6a4 100644 --- a/service/step_defs_test.go +++ b/service/step_defs_test.go @@ -2163,6 +2163,30 @@ func (f *feature) iCallGetCapacityWithStoragePool(arg1 string) error { return nil } +func (f *feature) iCallGetCapacityWithAvailabilityZone(zoneLabelKey, zoneName string) error { + ctx := context.Background() + req := new(csi.GetCapacityRequest) + + // need to make sure parameters aren't empty. + // This is a parameter taken from a running driver. + parameters := make(map[string]string) + parameters["csi.storage.k8s.io/fstype"] = "xfs" + req.Parameters = parameters + req.AccessibleTopology = &csi.Topology{ + Segments: map[string]string{ + zoneLabelKey: zoneName, + }, + } + + log.Printf("Calling GetCapacity") + f.getCapacityResponse, f.err = f.service.GetCapacity(ctx, req) + if f.err != nil { + log.Printf("GetCapacity call failed: %s\n", f.err.Error()) + return nil + } + return nil +} + func (f *feature) iCallGetMaximumVolumeSize(arg1 string) { systemid := arg1 f.maxVolSize, f.err = f.service.getMaximumVolumeSize(systemid) @@ -4887,6 +4911,7 @@ func FeatureContext(s *godog.ScenarioContext) { s.Step(`^a valid DeleteVolumeResponse is returned$`, f.aValidDeleteVolumeResponseIsReturned) s.Step(`^the volume is already mapped to an SDC$`, f.theVolumeIsAlreadyMappedToAnSDC) s.Step(`^I call GetCapacity with storage pool "([^"]*)"$`, f.iCallGetCapacityWithStoragePool) + s.Step(`^I call GetCapacity with Availability Zone "([^"]*)" "([^"]*)"$`, f.iCallGetCapacityWithAvailabilityZone) s.Step(`^a valid GetCapacityResponse is returned$`, f.aValidGetCapacityResponseIsReturned) s.Step(`^a valid GetCapacityResponse1 is returned$`, f.aValidGetCapacityResponsewithmaxvolsizeIsReturned) s.Step(`^I call get GetMaximumVolumeSize with systemid "([^"]*)"$`, f.iCallGetMaximumVolumeSize) From 62be51c371742ff284b37bc5d49bc8e718947ef4 Mon Sep 17 00:00:00 2001 From: "Lau, Luke" Date: Tue, 17 Dec 2024 13:30:53 -0500 Subject: [PATCH 30/40] linting --- service/service_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/service/service_test.go b/service/service_test.go index f3fab9ff..33574c82 100644 --- a/service/service_test.go +++ b/service/service_test.go @@ -150,7 +150,7 @@ func Test_service_SetPodZoneLabel(t *testing.T) { KubeNodeName: validNodeName, }, }, - initTest: func(s *service) { + initTest: func(_ *service) { // create a client, but do not create any pods so the request // to list pods fails K8sClientset = fake.NewSimpleClientset() @@ -170,7 +170,7 @@ func Test_service_SetPodZoneLabel(t *testing.T) { KubeNodeName: validNodeName, }, }, - initTest: func(s *service) { + initTest: func(_ *service) { // setup clientset to nil to force creation // Creation should fail because tests are not run in a cluster K8sClientset = nil From bab6660742c0e55465f72decf43c15a69da9bd35 Mon Sep 17 00:00:00 2001 From: "Lau, Luke" Date: Tue, 17 Dec 2024 17:11:09 -0500 Subject: [PATCH 31/40] reverting debug changes --- service/service.go | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/service/service.go b/service/service.go index 44b49742..8095ee26 100644 --- a/service/service.go +++ b/service/service.go @@ -542,9 +542,10 @@ func (s *service) BeforeServe( // Update the ConfigMap with the Interface IPs s.updateConfigMap(s.getIPAddressByInterface, ConfigMapFilePath) - return s.doProbe(ctx) - - // return nil + if _, ok := csictx.LookupEnv(ctx, "X_CSI_VXFLEXOS_NO_PROBE_ON_START"); !ok { + return s.doProbe(ctx) + } + return nil } func (s *service) updateConfigMap(getIPAddressByInterfacefunc GetIPAddressByInterfacefunc, configFilePath string) { @@ -714,7 +715,7 @@ func (s *service) doProbe(ctx context.Context) error { defer px.Unlock() if !strings.EqualFold(s.mode, "node") { - Log.Infof("[doProbe] controllerProbe - zone label: %s", s.opts.zoneLabelKey) + Log.Info("[doProbe] controllerProbe") if err := s.systemProbeAll(ctx, ""); err != nil { return err } @@ -723,7 +724,7 @@ func (s *service) doProbe(ctx context.Context) error { // Do a node probe if !strings.EqualFold(s.mode, "controller") { // Probe all systems managed by driver - Log.Infof("[doProbe] nodeProbe - zone label: %s", s.opts.zoneLabelKey) + Log.Info("[doProbe] nodeProbe") if err := s.systemProbeAll(ctx, s.opts.zoneLabelKey); err != nil { return err } From 095147b26b6d0da6e6bc7bd5898603f5b7657923 Mon Sep 17 00:00:00 2001 From: "Lau, Luke" Date: Tue, 17 Dec 2024 17:13:23 -0500 Subject: [PATCH 32/40] refactoring --- service/controller.go | 22 ++++++------ service/identity.go | 4 +-- service/service.go | 30 ++++++++++++---- service/service_test.go | 76 ++++++++++++++++++++++++++++++++++++++- service/step_defs_test.go | 2 +- 5 files changed, 114 insertions(+), 20 deletions(-) diff --git a/service/controller.go b/service/controller.go index 63104220..d4e8b159 100644 --- a/service/controller.go +++ b/service/controller.go @@ -2575,10 +2575,7 @@ func (s *service) ControllerGetCapabilities( } func (s *service) getZoneFromZoneLabelKey(ctx context.Context, zoneLabelKey string) (zone string, err error) { - if zoneLabelKey == "" { - return "", nil - } - + // get labels for this service, s labels, err := GetNodeLabels(ctx, s) if err != nil { return "", err @@ -2586,6 +2583,7 @@ func (s *service) getZoneFromZoneLabelKey(ctx context.Context, zoneLabelKey stri Log.Infof("Listing labels: %v", labels) + // get the zone name from the labels if val, ok := labels[zoneLabelKey]; ok { Log.Infof("probing zoneLabel %s, zone value: %s", zoneLabelKey, val) return val, nil @@ -2596,23 +2594,27 @@ func (s *service) getZoneFromZoneLabelKey(ctx context.Context, zoneLabelKey stri // systemProbeAll will iterate through all arrays in service.opts.arrays and probe them. If failed, it logs // the failed system name -func (s *service) systemProbeAll(ctx context.Context, zoneLabelKey string) error { +func (s *service) systemProbeAll(ctx context.Context) error { // probe all arrays Log.Infoln("Probing all associated arrays") allArrayFail := true errMap := make(map[string]error) + zoneName := "" + var err error - zoneName, err := s.getZoneFromZoneLabelKey(ctx, zoneLabelKey) - if err != nil { - return err + if s.opts.zoneLabelKey != "" && s.isNodeMode() { + zoneName, err = s.getZoneFromZoneLabelKey(ctx, s.opts.zoneLabelKey) + if err != nil { + return err + } } for _, array := range s.opts.arrays { // If zone information is available, use it to probe the array - if strings.EqualFold(s.mode, "node") && array.AvailabilityZone != nil && array.AvailabilityZone.Name != ZoneName(zoneName) { + if s.isNodeMode() && !array.isInZone(zoneName) { // Driver node containers should not probe arrays that exist outside their assigned zone // Driver controller container should probe all arrays - Log.Warnf("array %s zone %s does not match %s, not pinging this array\n", array.SystemID, array.AvailabilityZone.Name, zoneName) + Log.Infof("array %s zone %s does not match %s, not pinging this array\n", array.SystemID, array.AvailabilityZone.Name, zoneName) continue } diff --git a/service/identity.go b/service/identity.go index 761d0be4..653d3706 100644 --- a/service/identity.go +++ b/service/identity.go @@ -81,7 +81,7 @@ func (s *service) Probe( if !strings.EqualFold(s.mode, "node") { Log.Debug("systemProbe") - if err := s.systemProbeAll(ctx, ""); err != nil { + if err := s.systemProbeAll(ctx); err != nil { Log.Printf("error in systemProbeAll: %s", err.Error()) return nil, err } @@ -107,7 +107,7 @@ func (s *service) ProbeController(ctx context.Context, ) { if !strings.EqualFold(s.mode, "node") { Log.Debug("systemProbe") - if err := s.systemProbeAll(ctx, ""); err != nil { + if err := s.systemProbeAll(ctx); err != nil { Log.Printf("error in systemProbeAll: %s", err.Error()) return nil, err } diff --git a/service/service.go b/service/service.go index 8095ee26..75ca6ed3 100644 --- a/service/service.go +++ b/service/service.go @@ -666,7 +666,7 @@ func (s *service) getIPAddressByInterface(interfaceName string, networkInterface } func (s *service) checkNFS(ctx context.Context, systemID string) (bool, error) { - err := s.systemProbeAll(ctx, s.opts.zoneLabelKey) + err := s.systemProbeAll(ctx) if err != nil { return false, err } @@ -714,18 +714,18 @@ func (s *service) doProbe(ctx context.Context) error { px.Lock() defer px.Unlock() - if !strings.EqualFold(s.mode, "node") { + if !s.isNodeMode() { Log.Info("[doProbe] controllerProbe") - if err := s.systemProbeAll(ctx, ""); err != nil { + if err := s.systemProbeAll(ctx); err != nil { return err } } // Do a node probe - if !strings.EqualFold(s.mode, "controller") { + if !s.isControllerMode() { // Probe all systems managed by driver Log.Info("[doProbe] nodeProbe") - if err := s.systemProbeAll(ctx, s.opts.zoneLabelKey); err != nil { + if err := s.systemProbeAll(ctx); err != nil { return err } @@ -1866,7 +1866,10 @@ func (s *service) SetPodZoneLabel(ctx context.Context, zoneLabel map[string]stri podName := "" for _, pod := range pods.Items { if pod.Spec.NodeName == s.opts.KubeNodeName && pod.Labels["app"] != "" { - podName = pod.Name + // only add labels to node pods. Controller pod is not restricted to a zone + if strings.Contains(pod.Name, "node") { + podName = pod.Name + } } } @@ -1944,3 +1947,18 @@ func getZoneKeyLabelFromSecret(arrays map[string]*ArrayConnectionData) (string, return zoneKeyLabel, nil } + +// isControllerMode returns true if the mode property of service s is set to "node", false otherwise. +func (s *service) isNodeMode() bool { + return strings.EqualFold(s.mode, "node") +} + +// isControllerMode returns true if the mode property of service s is set to "controller", false otherwise. +func (s *service) isControllerMode() bool { + return strings.EqualFold(s.mode, "controller") +} + +// isInZone returns true if the array is configured for use in the provided zoneName, false otherwise. +func (array *ArrayConnectionData) isInZone(zoneName string) bool { + return array.AvailabilityZone != nil && array.AvailabilityZone.Name == ZoneName(zoneName) +} diff --git a/service/service_test.go b/service/service_test.go index 33574c82..38ce5c6c 100644 --- a/service/service_test.go +++ b/service/service_test.go @@ -88,7 +88,7 @@ func Test_service_SetPodZoneLabel(t *testing.T) { const validZoneName = "zoneA" const validZoneLabelKey = "topology.kubernetes.io/zone" - const validAppName = "test-app" + const validAppName = "test-node-pod" const validAppLabelKey = "app" const validNodeName = "kube-node-name" validAppLabels := map[string]string{validAppLabelKey: validAppName} @@ -206,3 +206,77 @@ func Test_service_SetPodZoneLabel(t *testing.T) { }) } } + +func TestArrayConnectionData_isInZone(t *testing.T) { + type fields struct { + SystemID string + Username string + Password string + Endpoint string + SkipCertificateValidation bool + Insecure bool + IsDefault bool + AllSystemNames string + NasName string + AvailabilityZone *AvailabilityZone + } + type args struct { + zoneName string + } + tests := map[string]struct { + fields fields + args args + want bool + }{ + "array is in the zone": { + want: true, + fields: fields{ + AvailabilityZone: &AvailabilityZone{ + LabelKey: "topology.kubernetes.io/zone", + Name: "zoneA", + }, + }, + args: args{ + zoneName: "zoneA", + }, + }, + "availability zone is not used": { + want: false, + fields: fields{}, + args: args{ + zoneName: "zoneA", + }, + }, + "zone names do not match": { + want: false, + fields: fields{ + AvailabilityZone: &AvailabilityZone{ + LabelKey: "topology.kubernetes.io/zone", + Name: "zoneA", + }, + }, + args: args{ + zoneName: "zoneB", + }, + }, + } + for name, tt := range tests { + t.Run(name, func(t *testing.T) { + array := &ArrayConnectionData{ + SystemID: tt.fields.SystemID, + Username: tt.fields.Username, + Password: tt.fields.Password, + Endpoint: tt.fields.Endpoint, + SkipCertificateValidation: tt.fields.SkipCertificateValidation, + Insecure: tt.fields.Insecure, + IsDefault: tt.fields.IsDefault, + AllSystemNames: tt.fields.AllSystemNames, + NasName: tt.fields.NasName, + AvailabilityZone: tt.fields.AvailabilityZone, + } + if got := array.isInZone(tt.args.zoneName); got != tt.want { + t.Errorf("ArrayConnectionData.isInZone() = %v, want %v", got, tt.want) + } + }) + } +} diff --git a/service/step_defs_test.go b/service/step_defs_test.go index 7084b6a4..724f6ec1 100644 --- a/service/step_defs_test.go +++ b/service/step_defs_test.go @@ -4313,7 +4313,7 @@ func (f *feature) iUseConfig(filename string) error { return fmt.Errorf("get zone key label from secret: %s", err.Error()) } fmt.Printf("****************************************************** s.opts.arrays %v\n", f.service.opts.arrays) - f.service.systemProbeAll(context.Background(), "") + f.service.systemProbeAll(context.Background()) f.adminClient = f.service.adminClients[arrayID] if f.adminClient == nil { return fmt.Errorf("adminClient nil") From e636b0633d45061562fb4604743eb4dff09893c8 Mon Sep 17 00:00:00 2001 From: "Lau, Luke" Date: Wed, 18 Dec 2024 10:22:29 -0500 Subject: [PATCH 33/40] update copyright --- service/identity.go | 2 +- service/step_defs_test.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/service/identity.go b/service/identity.go index 653d3706..3facfa81 100644 --- a/service/identity.go +++ b/service/identity.go @@ -1,4 +1,4 @@ -// Copyright © 2019-2022 Dell Inc. or its subsidiaries. All Rights Reserved. +// Copyright © 2019-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. diff --git a/service/step_defs_test.go b/service/step_defs_test.go index 724f6ec1..caa611a2 100644 --- a/service/step_defs_test.go +++ b/service/step_defs_test.go @@ -1,4 +1,4 @@ -// Copyright © 2019-2023 Dell Inc. or its subsidiaries. All Rights Reserved. +// Copyright © 2019-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. From 4b71e12b841e98061c58d13ca85da9ad0bbf37d2 Mon Sep 17 00:00:00 2001 From: "Lau, Luke" Date: Wed, 18 Dec 2024 13:16:24 -0500 Subject: [PATCH 34/40] tidying comments --- service/controller.go | 2 +- service/identity.go | 2 -- 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/service/controller.go b/service/controller.go index d4e8b159..84d2c1b4 100644 --- a/service/controller.go +++ b/service/controller.go @@ -2585,7 +2585,6 @@ func (s *service) getZoneFromZoneLabelKey(ctx context.Context, zoneLabelKey stri // get the zone name from the labels if val, ok := labels[zoneLabelKey]; ok { - Log.Infof("probing zoneLabel %s, zone value: %s", zoneLabelKey, val) return val, nil } @@ -2607,6 +2606,7 @@ func (s *service) systemProbeAll(ctx context.Context) error { if err != nil { return err } + Log.Infof("probing zoneLabel '%s', zone value: '%s'", s.opts.zoneLabelKey, zoneName) } for _, array := range s.opts.arrays { diff --git a/service/identity.go b/service/identity.go index 3facfa81..052a92d8 100644 --- a/service/identity.go +++ b/service/identity.go @@ -77,8 +77,6 @@ func (s *service) Probe( _ *csi.ProbeRequest) ( *csi.ProbeResponse, error, ) { - Log.Infof("[Probe] Probe context: %v", ctx) - if !strings.EqualFold(s.mode, "node") { Log.Debug("systemProbe") if err := s.systemProbeAll(ctx); err != nil { From d82cdfde7d5ef1f56d9bcc98d44492be5e037cce Mon Sep 17 00:00:00 2001 From: "Lau, Luke" Date: Wed, 18 Dec 2024 13:17:19 -0500 Subject: [PATCH 35/40] fix bug after regression tests --- service/controller.go | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/service/controller.go b/service/controller.go index 84d2c1b4..9ab57b49 100644 --- a/service/controller.go +++ b/service/controller.go @@ -2599,9 +2599,10 @@ func (s *service) systemProbeAll(ctx context.Context) error { allArrayFail := true errMap := make(map[string]error) zoneName := "" - var err error + usingZones := s.opts.zoneLabelKey != "" && s.isNodeMode() - if s.opts.zoneLabelKey != "" && s.isNodeMode() { + if usingZones { + var err error zoneName, err = s.getZoneFromZoneLabelKey(ctx, s.opts.zoneLabelKey) if err != nil { return err @@ -2611,7 +2612,7 @@ func (s *service) systemProbeAll(ctx context.Context) error { for _, array := range s.opts.arrays { // If zone information is available, use it to probe the array - if s.isNodeMode() && !array.isInZone(zoneName) { + if usingZones && !array.isInZone(zoneName) { // Driver node containers should not probe arrays that exist outside their assigned zone // Driver controller container should probe all arrays Log.Infof("array %s zone %s does not match %s, not pinging this array\n", array.SystemID, array.AvailabilityZone.Name, zoneName) From 8939c70eda43dd89833481e4caaa73d5bc3cec7d Mon Sep 17 00:00:00 2001 From: "Lau, Luke" Date: Wed, 18 Dec 2024 13:58:14 -0500 Subject: [PATCH 36/40] update goscaleio --- go.mod | 2 +- go.sum | 2 ++ 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/go.mod b/go.mod index 28af9f89..771d8393 100644 --- a/go.mod +++ b/go.mod @@ -15,7 +15,7 @@ require ( github.com/dell/dell-csi-extensions/volumeGroupSnapshot v1.7.0 github.com/dell/gocsi v1.12.0 github.com/dell/gofsutil v1.17.0 - github.com/dell/goscaleio v1.17.2-0.20241213215244-2164caaef4ab + github.com/dell/goscaleio v1.17.2-0.20241218182509-936b677c46d5 github.com/fsnotify/fsnotify v1.5.1 github.com/google/uuid v1.6.0 github.com/gorilla/mux v1.8.0 diff --git a/go.sum b/go.sum index 7b338a59..e53fe730 100644 --- a/go.sum +++ b/go.sum @@ -122,6 +122,8 @@ github.com/dell/goscaleio v1.17.2-0.20241213204026-19006b56eb26 h1:Kg6MSwBmAlmUD github.com/dell/goscaleio v1.17.2-0.20241213204026-19006b56eb26/go.mod h1:7bX3rL8JWMmdifGr/UeD/Ju9wbkHUqvKDrbdu7XyGm8= github.com/dell/goscaleio v1.17.2-0.20241213215244-2164caaef4ab h1:DYWY7fs8v1VbmzF2pAx7peZtKhkppW5NCIyHCJN1fS4= github.com/dell/goscaleio v1.17.2-0.20241213215244-2164caaef4ab/go.mod h1:7bX3rL8JWMmdifGr/UeD/Ju9wbkHUqvKDrbdu7XyGm8= +github.com/dell/goscaleio v1.17.2-0.20241218182509-936b677c46d5 h1:d7DwHvp7/hESR742f4iurtH3nHHSGPvnMadujZA2hsU= +github.com/dell/goscaleio v1.17.2-0.20241218182509-936b677c46d5/go.mod h1:2BsR92dYYnSmbZ34ixYdsucfyoQBDlbhbUUKnv6WalQ= github.com/dgrijalva/jwt-go v3.2.0+incompatible/go.mod h1:E3ru+11k8xSBh+hMPgOLZmtrrCbhqsmaPHjLKYnJCaQ= github.com/dgryski/go-sip13 v0.0.0-20181026042036-e10d5fee7954/go.mod h1:vAd38F8PWV+bWy6jNmig1y/TA+kYO4g3RSRF0IAv0no= github.com/docker/spdystream v0.0.0-20160310174837-449fdfce4d96/go.mod h1:Qh8CwZgvJUkLughtfhJv5dyTYa91l1fOUCrgjqmcifM= From 90867b319ed494553fe6cab193a445445f98185c Mon Sep 17 00:00:00 2001 From: "Lau, Luke" Date: Wed, 18 Dec 2024 15:07:18 -0500 Subject: [PATCH 37/40] PR comments: tdawe --- service/controller.go | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/service/controller.go b/service/controller.go index 9ab57b49..7abffbb0 100644 --- a/service/controller.go +++ b/service/controller.go @@ -2643,19 +2643,19 @@ func (s *service) systemProbe(ctx context.Context, array *ArrayConnectionData) e // Check that we have the details needed to login to the Gateway if array.Endpoint == "" { return status.Error(codes.FailedPrecondition, - "missing VxFlexOS Gateway endpoint") + "missing PowerFlex Gateway endpoint") } if array.Username == "" { return status.Error(codes.FailedPrecondition, - "missing VxFlexOS MDM user") + "missing PowerFlex MDM user") } if array.Password == "" { return status.Error(codes.FailedPrecondition, - "missing VxFlexOS MDM password") + "missing PowerFlex MDM password") } if array.SystemID == "" { return status.Error(codes.FailedPrecondition, - "missing VxFlexOS system name") + "missing PowerFlex system name") } var altSystemNames []string if array.AllSystemNames != "" { @@ -2678,7 +2678,7 @@ func (s *service) systemProbe(ctx context.Context, array *ArrayConnectionData) e } } - Log.Printf("Login to VxFlexOS Gateway, system=%s, endpoint=%s, user=%s\n", systemID, array.Endpoint, array.Username) + Log.Printf("Login to PowerFlex Gateway, system=%s, endpoint=%s, user=%s\n", systemID, array.Endpoint, array.Username) if s.adminClients[systemID].GetToken() == "" { _, err := s.adminClients[systemID].WithContext(ctx).Authenticate(&goscaleio.ConfigConnect{ @@ -2688,7 +2688,7 @@ func (s *service) systemProbe(ctx context.Context, array *ArrayConnectionData) e }) if err != nil { return status.Errorf(codes.FailedPrecondition, - "unable to login to VxFlexOS Gateway: %s", err.Error()) + "unable to login to PowerFlex Gateway: %s", err.Error()) } } @@ -2698,7 +2698,7 @@ func (s *service) systemProbe(ctx context.Context, array *ArrayConnectionData) e array.SystemID, array.SystemID, "") if err != nil { return status.Errorf(codes.FailedPrecondition, - "unable to find matching VxFlexOS system name: %s", + "unable to find matching PowerFlex system name: %s", err.Error()) } s.systems[systemID] = system From 5ea281b8c2e9f777b59f9b693f3d1b0f3966fceb Mon Sep 17 00:00:00 2001 From: "Lau, Luke" Date: Wed, 18 Dec 2024 15:17:24 -0500 Subject: [PATCH 38/40] PR comments: falfaroc --- service/controller_test.go | 32 ++++++++++++-------------------- service/node.go | 5 ++++- service/service.go | 2 -- service/service_test.go | 22 +++++++++------------- 4 files changed, 25 insertions(+), 36 deletions(-) diff --git a/service/controller_test.go b/service/controller_test.go index a6f044a1..3fa6f13d 100644 --- a/service/controller_test.go +++ b/service/controller_test.go @@ -49,17 +49,15 @@ func Test_service_getZoneFromZoneLabelKey(t *testing.T) { const validTopologyKey = "topology.kubernetes.io/zone" const validZone = "zoneA" - tests := []struct { - name string + tests := map[string]struct { fields fields args args wantZone string wantErr bool getNodeLabelFunc func(ctx context.Context, s *service) (map[string]string, error) }{ - { + "success": { // happy path test - name: "get good zone label", args: args{ ctx: context.Background(), zoneLabelKey: validTopologyKey, @@ -71,9 +69,8 @@ func Test_service_getZoneFromZoneLabelKey(t *testing.T) { return nodeLabels, nil }, }, - { + "use bad zone label key": { // The key args.zoneLabelKey will not be found in the map returned by getNodeLabelFunc - name: "use bad zone label key", args: args{ ctx: context.Background(), zoneLabelKey: "badkey", @@ -84,9 +81,8 @@ func Test_service_getZoneFromZoneLabelKey(t *testing.T) { return nil, nil }, }, - { + "fail to get node labels": { // getNodeLabelFunc will return an error, triggering failure to get the labels - name: "fail to get node labels", args: args{ ctx: context.Background(), zoneLabelKey: "unimportant", @@ -98,8 +94,8 @@ func Test_service_getZoneFromZoneLabelKey(t *testing.T) { }, }, } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { + for testName, tt := range tests { + t.Run(testName, func(t *testing.T) { s := &service{ opts: tt.fields.opts, adminClients: tt.fields.adminClients, @@ -155,16 +151,14 @@ func Test_service_getSystemIDFromZoneLabelKey(t *testing.T) { const validTopologyKey = "topology.kubernetes.io/zone" const validZone = "zoneA" - tests := []struct { - name string + tests := map[string]struct { fields fields args args wantSystemID string wantErr bool }{ - { + "success": { // happy path test - name: "get a valid system ID", wantErr: false, wantSystemID: validSystemID, args: args{ @@ -190,10 +184,9 @@ func Test_service_getSystemIDFromZoneLabelKey(t *testing.T) { }, }, }, - { + "topology not passed with csi request": { // should return an empty string if no topology info is passed // with the csi request - name: "topology not passed with csi request", wantErr: false, wantSystemID: "", args: args{ @@ -210,10 +203,9 @@ func Test_service_getSystemIDFromZoneLabelKey(t *testing.T) { }, }, }, - { + "zone name missing in secret": { // topology information in the csi request does not match // any of the arrays in the secret - name: "zone name missing in secret", wantErr: true, wantSystemID: "", args: args{ @@ -242,8 +234,8 @@ func Test_service_getSystemIDFromZoneLabelKey(t *testing.T) { }, }, } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { + for testName, tt := range tests { + t.Run(testName, func(t *testing.T) { s := &service{ opts: tt.fields.opts, adminClients: tt.fields.adminClients, diff --git a/service/node.go b/service/node.go index 298b898d..e3de0874 100644 --- a/service/node.go +++ b/service/node.go @@ -762,7 +762,10 @@ func (s *service) NodeGetInfo( if zone, ok := labels[s.opts.zoneLabelKey]; ok { topology[s.opts.zoneLabelKey] = zone - _ = s.SetPodZoneLabel(ctx, topology) + err = s.SetPodZoneLabel(ctx, topology) + if err != nil { + Log.Warnf("Unable to set availability zone label '%s:%s' for this pod", topology[s.opts.zoneLabelKey], zone) + } } for _, array := range s.opts.arrays { diff --git a/service/service.go b/service/service.go index 75ca6ed3..f0ba9a19 100644 --- a/service/service.go +++ b/service/service.go @@ -1855,8 +1855,6 @@ func (s *service) SetPodZoneLabel(ctx context.Context, zoneLabel map[string]stri K8sClientset = k8sutils.Clientset } - Log.Println("SetPodZoneLabel") - // access the API to fetch node object pods, err := K8sClientset.CoreV1().Pods(DriverNamespace).List(ctx, v1.ListOptions{}) if err != nil { diff --git a/service/service_test.go b/service/service_test.go index 38ce5c6c..1430f86d 100644 --- a/service/service_test.go +++ b/service/service_test.go @@ -93,16 +93,14 @@ func Test_service_SetPodZoneLabel(t *testing.T) { const validNodeName = "kube-node-name" validAppLabels := map[string]string{validAppLabelKey: validAppName} - tests := []struct { - name string + tests := map[string]struct { fields fields args args initTest func(s *service) wantErr bool }{ - { + "successfully add zone labels to a pod": { // happy path test - name: "add zone labels to a pod", wantErr: false, args: args{ ctx: context.Background(), @@ -135,9 +133,8 @@ func Test_service_SetPodZoneLabel(t *testing.T) { } }, }, - { + "when 'list pods' k8s client request fails": { // Attempt to set pod labels when the k8s client cannot get pods - name: "when 'list pods' k8s client request fails", wantErr: true, args: args{ ctx: context.Background(), @@ -156,8 +153,7 @@ func Test_service_SetPodZoneLabel(t *testing.T) { K8sClientset = fake.NewSimpleClientset() }, }, - { - name: "clientset is nil and fails to create one", + "clientset is nil and fails to create one": { wantErr: true, args: args{ ctx: context.Background(), @@ -178,8 +174,8 @@ func Test_service_SetPodZoneLabel(t *testing.T) { }, } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { + for testName, tt := range tests { + t.Run(testName, func(t *testing.T) { s := &service{ opts: tt.fields.opts, adminClients: tt.fields.adminClients, @@ -228,7 +224,7 @@ func TestArrayConnectionData_isInZone(t *testing.T) { args args want bool }{ - "array is in the zone": { + "success": { want: true, fields: fields{ AvailabilityZone: &AvailabilityZone{ @@ -260,8 +256,8 @@ func TestArrayConnectionData_isInZone(t *testing.T) { }, }, } - for name, tt := range tests { - t.Run(name, func(t *testing.T) { + for testName, tt := range tests { + t.Run(testName, func(t *testing.T) { array := &ArrayConnectionData{ SystemID: tt.fields.SystemID, Username: tt.fields.Username, From c6e56f5819a4dd7b3673a9c49a8dc0fab81a8127 Mon Sep 17 00:00:00 2001 From: "Lau, Luke" Date: Wed, 18 Dec 2024 16:05:07 -0500 Subject: [PATCH 39/40] align tests with new error messages from prev commit --- service/features/service.feature | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/service/features/service.feature b/service/features/service.feature index 7e804472..0985a07b 100644 --- a/service/features/service.feature +++ b/service/features/service.feature @@ -140,7 +140,7 @@ Feature: VxFlex OS CSI interface And the Controller has no connection When I invalidate the Probe cache And I call Probe - Then the error contains "unable to login to VxFlexOS Gateway" + Then the error contains "unable to login to PowerFlex Gateway" Scenario Outline: Probe Call with various errors Given a VxFlexOS service @@ -151,11 +151,11 @@ Feature: VxFlex OS CSI interface Examples: | error | msg | - | "NoEndpointError" | "missing VxFlexOS Gateway endpoint" | - | "NoUserError" | "missing VxFlexOS MDM user" | - | "NoPasswordError" | "missing VxFlexOS MDM password" | - | "NoSysNameError" | "missing VxFlexOS system name" | - | "WrongSysNameError" | "unable to find matching VxFlexOS system name" | + | "NoEndpointError" | "missing PowerFlex Gateway endpoint" | + | "NoUserError" | "missing PowerFlex MDM user" | + | "NoPasswordError" | "missing PowerFlex MDM password" | + | "NoSysNameError" | "missing PowerFlex system name" | + | "WrongSysNameError" | "unable to find matching PowerFlex system name" | # This injected error fails on Windows with no SDC but passes on Linux with SDC @@ -1156,7 +1156,7 @@ Feature: VxFlex OS CSI interface Scenario: Call getSystemName, should get error Unable to probe system with ID Given a VxFlexOS service When I call getSystemNameError - Then the error contains "missing VxFlexOS system name" + Then the error contains "missing PowerFlex system name" Scenario: Call getSystemName, should get Found system Name: mocksystem Given a VxFlexOS service @@ -1189,14 +1189,14 @@ Feature: VxFlex OS CSI interface And I do not have a gateway connection And I do not have a valid gateway endpoint When I Call nodeGetAllSystems - Then the error contains "missing VxFlexOS Gateway endpoint" + Then the error contains "missing PowerFlex Gateway endpoint" Scenario: Call Node getAllSystems Given a VxFlexOS service And I do not have a gateway connection And I do not have a valid gateway password When I Call nodeGetAllSystems - Then the error contains "missing VxFlexOS MDM password" + Then the error contains "missing PowerFlex MDM password" Scenario: Call evalsymlinks Given a VxFlexOS service @@ -1262,7 +1262,7 @@ Feature: VxFlex OS CSI interface And I invalidate the Probe cache When I call BeforeServe # Get different error message on Windows vs. Linux - Then the error contains "unable to login to VxFlexOS Gateway" + Then the error contains "unable to login to PowerFlex Gateway" Scenario: Test getArrayConfig with invalid config file Given an invalid config From 5f49dac5ea896f296414844a1777c9af86ded477 Mon Sep 17 00:00:00 2001 From: "Lau, Luke" Date: Wed, 18 Dec 2024 16:05:26 -0500 Subject: [PATCH 40/40] add tests for systemProbeAll --- service/features/service.feature | 10 ++++++++++ service/step_defs_test.go | 31 +++++++++++++++++++++++++++++++ 2 files changed, 41 insertions(+) diff --git a/service/features/service.feature b/service/features/service.feature index 0985a07b..3cecad8b 100644 --- a/service/features/service.feature +++ b/service/features/service.feature @@ -1617,3 +1617,13 @@ Feature: VxFlex OS CSI interface Examples: | name | config | errorMsg | | "volume1" | "multi_az" | "none" | + + Scenario: Probe all systems using availability zones + Given a VxFlexOS service + And I use config + When I call systemProbeAll in mode + Then the error contains + Examples: + | config | mode | errorMsg | + | "multi_az" | "node" | "none" | + | "multi_az" | "controller" | "none" | \ No newline at end of file diff --git a/service/step_defs_test.go b/service/step_defs_test.go index caa611a2..3f8a5e89 100644 --- a/service/step_defs_test.go +++ b/service/step_defs_test.go @@ -4328,6 +4328,36 @@ func (f *feature) iUseConfig(filename string) error { return nil } +func (f *feature) iCallSystemProbeAll(mode string) error { + // set the mode of the service + if mode == "controller" || mode == "node" { + f.service.mode = mode + } else { + return fmt.Errorf("mode '%s' is not a valid service mode. must be 'controller' or 'node'", mode) + } + + // Create a fake node with necessary availability zone labels + f.service.opts.KubeNodeName = "node1" + fakeNode := &v1.Node{ + ObjectMeta: metav1.ObjectMeta{ + Name: f.service.opts.KubeNodeName, + UID: "1aa4c285-d41b-4911-bf3e-621253bfbade", + Labels: map[string]string{ + f.service.opts.zoneLabelKey: string(f.service.opts.arrays[arrayID].AvailabilityZone.Name), + }, + }, + } + thisNode, err := K8sClientset.CoreV1().Nodes().Create(context.Background(), fakeNode, metav1.CreateOptions{}) + if err != nil { + return fmt.Errorf("could not create k8s node for test: err: %s", err.Error()) + } + // delete it when finished, otherwise it will fail to create nodes for subsequent tests + defer K8sClientset.CoreV1().Nodes().Delete(context.Background(), thisNode.Name, *&metav1.DeleteOptions{}) + + f.err = f.service.systemProbeAll(context.Background()) + return nil +} + func (f *feature) iCallGetReplicationCapabilities() error { req := &replication.GetReplicationCapabilityRequest{} ctx := context.Background() @@ -5063,6 +5093,7 @@ func FeatureContext(s *godog.ScenarioContext) { s.Step(`^a valid NodeGetInfo is returned with node topology$`, f.aValidNodeGetInfoIsReturnedWithNodeTopology) s.Step(`^a NodeGetInfo is returned without zone topology$`, f.aNodeGetInfoIsReturnedWithoutZoneTopology) s.Step(`^a NodeGetInfo is returned without zone system topology$`, f.aNodeGetInfoIsReturnedWithoutZoneSystemTopology) + s.Step(`^I call systemProbeAll in mode "([^"]*)"`, f.iCallSystemProbeAll) s.After(func(ctx context.Context, _ *godog.Scenario, _ error) (context.Context, error) { if f.server != nil {