Fetch region from node info (#78)

* Changes for region label

* Changes in deployment file

* Changes in deployment file

* ibm-csi-common upgrade

deploy/kubernetes/driver/kubernetes/manifests/controller-server.yaml

@@ -110,6 +110,10 @@ spec:
               valueFrom:
                 fieldRef:
                   fieldPath: metadata.namespace
+            - name: KUBE_NODE_NAME
+              valueFrom:
+                fieldRef:
+                  fieldPath: spec.nodeName
           resources:
             limits:
               cpu: 500m

go.mod

@@ -3,7 +3,7 @@ module github.com/kubernetes-sigs/ibm-vpc-block-csi-driver
 go 1.16
 
 require (
-	github.com/IBM/ibm-csi-common v1.0.0-beta15
+	github.com/IBM/ibm-csi-common v1.0.0-beta16
 	github.com/IBM/ibmcloud-volume-interface v1.0.0-beta10
 	github.com/IBM/ibmcloud-volume-vpc v1.0.0-beta15
 	github.com/container-storage-interface/spec v1.5.0

go.sum

@@ -42,8 +42,8 @@ github.com/BurntSushi/xgb v0.0.0-20160522181843-27f122750802/go.mod h1:IVnqGOEym
 github.com/GoogleCloudPlatform/k8s-cloud-provider v0.0.0-20200415212048-7901bc822317/go.mod h1:DF8FZRxMHMGv/vP2lQP6h+dYzzjpuRn24VeRiYn3qjQ=
 github.com/IBM-Cloud/ibm-cloud-cli-sdk v0.6.7 h1:eHgfQl6IeSmzWUyiSi13CvoFYsovoyqWlpHX0pa9J54=
 github.com/IBM-Cloud/ibm-cloud-cli-sdk v0.6.7/go.mod h1:RiUvKuHKTBmBApDMUQzBL14pQUGKcx/IioKQPIcRQjs=
-github.com/IBM/ibm-csi-common v1.0.0-beta15 h1:1U571Xf1RfpI5hIkuz6oIaahuUZpEy6CeRzDIfH/UwY=
-github.com/IBM/ibm-csi-common v1.0.0-beta15/go.mod h1:PeSXn0la1H7hKalfIwgeldFQ7I5d9h6j6pPId3qxlb8=
+github.com/IBM/ibm-csi-common v1.0.0-beta16 h1:nOrRJl3Nw/1zHKRspZ2BJcZiYl4VZ59MK6rCFTGZSZ4=
+github.com/IBM/ibm-csi-common v1.0.0-beta16/go.mod h1:PeSXn0la1H7hKalfIwgeldFQ7I5d9h6j6pPId3qxlb8=
 github.com/IBM/ibmcloud-volume-interface v1.0.0-beta10 h1:m3OO2Xd7NDWovxwa8KLsdUsP95NkKpMJ8YWBS1pa0Ew=
 github.com/IBM/ibmcloud-volume-interface v1.0.0-beta10/go.mod h1:BWj2o0UmOSI1rGcObFYmpQgX0zHuhm51W5o+Js8BHcE=
 github.com/IBM/ibmcloud-volume-vpc v1.0.0-beta15 h1:NdosO0H9xyKv0jWpWaSOiYcUSaqik0PYUItF+CMxt+w=

pkg/ibmcsidriver/controller.go

@@ -120,7 +120,7 @@ func (csiCS *CSIControllerServer) CreateVolume(ctx context.Context, req *csi.Cre
 	if existingVol != nil && err == nil {
 		ctxLogger.Info("Volume already exists", zap.Reflect("ExistingVolume", existingVol))
 		if existingVol.Capacity != nil && requestedVolume.Capacity != nil && *existingVol.Capacity == *requestedVolume.Capacity {
-			return createCSIVolumeResponse(*existingVol, int64(*(existingVol.Capacity)*utils.GB), nil, csiCS.CSIProvider.GetClusterInfo().ClusterID), nil
+			return createCSIVolumeResponse(*existingVol, int64(*(existingVol.Capacity)*utils.GB), nil, csiCS.CSIProvider.GetClusterInfo().ClusterID, csiCS.Driver.region), nil
 		}
 		return nil, commonError.GetCSIError(ctxLogger, commonError.VolumeAlreadyExists, requestID, err, name, *requestedVolume.Capacity)
 	}
@@ -132,7 +132,7 @@ func (csiCS *CSIControllerServer) CreateVolume(ctx context.Context, req *csi.Cre
 	}
 
 	// return csi volume object
-	return createCSIVolumeResponse(*volumeObj, int64(*(requestedVolume.Capacity)*utils.GB), nil, csiCS.CSIProvider.GetClusterInfo().ClusterID), nil
+	return createCSIVolumeResponse(*volumeObj, int64(*(requestedVolume.Capacity)*utils.GB), nil, csiCS.CSIProvider.GetClusterInfo().ClusterID, csiCS.Driver.region), nil
 }
 
 // DeleteVolume ...

pkg/ibmcsidriver/controller_helper.go

@@ -251,25 +251,15 @@ func getVolumeParameters(logger *zap.Logger, req *csi.CreateVolumeRequest, confi
 		volume.Iops = nil
 	}
 
-	//if zone is not given in SC parameters, but region is given, error out
-	if len(strings.TrimSpace(volume.Az)) == 0 && len(strings.TrimSpace(volume.Region)) != 0 {
-		err = fmt.Errorf("zone parameter is empty in storage class for region %s", strings.TrimSpace(volume.Region))
-		return volume, err
-	}
-
-	//If both zone and region not provided in storage class parameters then we pick from the Topology
-	//if zone is provided but region is not provided, fetch region for specified zone
-	if len(strings.TrimSpace(volume.Region)) == 0 {
+	//If  zone not provided in storage class parameters then we pick from the Topology
+	if len(strings.TrimSpace(volume.Az)) == 0 {
 		zones, err := pickTargetTopologyParams(req.GetAccessibilityRequirements())
 		if err != nil {
 			err = fmt.Errorf("unable to fetch zone information from topology: '%v'", err)
 			logger.Error("getVolumeParameters", zap.NamedError("InvalidParameter", err))
 			return volume, err
 		}
-		volume.Region = zones[utils.NodeRegionLabel]
-		if len(strings.TrimSpace(volume.Az)) == 0 {
-			volume.Az = zones[utils.NodeZoneLabel]
-		}
+		volume.Az = zones[utils.NodeZoneLabel]
 
 	}
 
@@ -423,7 +413,7 @@ func checkIfVolumeExists(session provider.Session, vol provider.Volume, ctxLogge
 }
 
 // createCSIVolumeResponse ...
-func createCSIVolumeResponse(vol provider.Volume, capBytes int64, zones []string, clusterID string) *csi.CreateVolumeResponse {
+func createCSIVolumeResponse(vol provider.Volume, capBytes int64, zones []string, clusterID string, region string) *csi.CreateVolumeResponse {
 	labels := map[string]string{}
 
 	// Update labels for PV objects
@@ -434,7 +424,12 @@ func createCSIVolumeResponse(vol provider.Volume, capBytes int64, zones []string
 	if vol.Iops != nil && len(*vol.Iops) > 0 {
 		labels[IOPSLabel] = *vol.Iops
 	}
-	labels[utils.NodeRegionLabel] = vol.Region
+
+	if vol.Region != "" {
+		labels[utils.NodeRegionLabel] = vol.Region
+	} else {
+		labels[utils.NodeRegionLabel] = region
+	}
 	labels[utils.NodeZoneLabel] = vol.Az
 
 	topology := &csi.Topology{
@@ -455,7 +450,6 @@ func createCSIVolumeResponse(vol provider.Volume, capBytes int64, zones []string
 	}
 	return volResp
 }
-
 func createControllerPublishVolumeResponse(volumeAttachmentResponse provider.VolumeAttachmentResponse, extraPublishInfo map[string]string) *csi.ControllerPublishVolumeResponse {
 	publishContext := map[string]string{
 		PublishInfoVolumeID:   volumeAttachmentResponse.VolumeID,

pkg/ibmcsidriver/controller_helper_test.go

@@ -300,36 +300,6 @@ func TestGetVolumeParameters(t *testing.T) {
 			expectedStatus: true,
 			expectedError:  fmt.Errorf("volume capabilities are empty"),
 		},
-		{
-			testCaseName: "Region present but zone not given as parameter",
-			request: &csi.CreateVolumeRequest{Name: volumeName, CapacityRange: &csi.CapacityRange{RequiredBytes: 11811160064, LimitBytes: utils.MinimumVolumeSizeInBytes + utils.MinimumVolumeSizeInBytes},
-				VolumeCapabilities: []*csi.VolumeCapability{{AccessMode: &csi.VolumeCapability_AccessMode{Mode: csi.VolumeCapability_AccessMode_SINGLE_NODE_WRITER}}},
-				Parameters: map[string]string{Profile: "general-purpose",
-					Region:        "us-south-test",
-					Tag:           "test-tag",
-					ResourceGroup: "myresourcegroups",
-					Encrypted:     "false",
-					EncryptionKey: "key",
-					ClassVersion:  "",
-					Generation:    "generation",
-					IOPS:          noIops,
-				},
-			},
-			expectedVolume: &provider.Volume{Name: &volumeName,
-				Capacity: &volumeSize,
-				VPCVolume: provider.VPCVolume{VPCBlockVolume: provider.VPCBlockVolume{
-					Tags: []string{createdByIBM},
-				},
-					Profile:       &provider.Profile{Name: "general-purpose"},
-					ResourceGroup: &provider.ResourceGroup{ID: "myresourcegroups"},
-				},
-				Region: "us-south-test",
-				Iops:   &noIops,
-				Az:     "testzone",
-			},
-			expectedStatus: true,
-			expectedError:  fmt.Errorf("zone parameter is empty in storage class for region us-south-test"),
-		},
 		{
 			testCaseName: "Region and Zone not given as parameter from SC",
 			request: &csi.CreateVolumeRequest{Name: volumeName, CapacityRange: &csi.CapacityRange{RequiredBytes: 11811160064, LimitBytes: utils.MinimumVolumeSizeInBytes + utils.MinimumVolumeSizeInBytes},
@@ -359,9 +329,8 @@ func TestGetVolumeParameters(t *testing.T) {
 					Profile:       &provider.Profile{Name: "general-purpose"},
 					ResourceGroup: &provider.ResourceGroup{ID: "myresourcegroups"},
 				},
-				Region: "myregion",
-				Iops:   &noIops,
-				Az:     "myzone",
+				Iops: &noIops,
+				Az:   "myzone",
 			},
 			expectedStatus: true,
 			expectedError:  nil,
@@ -677,7 +646,8 @@ func isCSIResponseSame(expectedVolume *csi.CreateVolumeResponse, actualCSIVolume
 	if expectedVolume == nil || actualCSIVolume == nil {
 		return false
 	}
-
+	fmt.Println(expectedVolume.Volume)
+	fmt.Println(actualCSIVolume.Volume)
 	return expectedVolume.Volume.VolumeId == actualCSIVolume.Volume.VolumeId &&
 		expectedVolume.Volume.CapacityBytes == actualCSIVolume.Volume.CapacityBytes &&
 		expectedVolume.Volume.GetAccessibleTopology()[0].GetSegments()[utils.NodeRegionLabel] == actualCSIVolume.Volume.GetAccessibleTopology()[0].GetSegments()[utils.NodeRegionLabel] &&
@@ -731,11 +701,47 @@ func TestCreateCSIVolumeResponse(t *testing.T) {
 			},
 			expectedStatus: true,
 		},
+		{
+			testCaseName: "Valid volume response with region in vol request empty",
+			requestVol: provider.Volume{VolumeID: volumeID,
+				VPCVolume: provider.VPCVolume{VPCBlockVolume: provider.VPCBlockVolume{
+					Tags: []string{createdByIBM},
+				},
+					Profile:       &provider.Profile{Name: "general-purpose"},
+					ResourceGroup: &provider.ResourceGroup{ID: "myresourcegroups"},
+				},
+				Iops: &threeIops,
+				Az:   "testzone",
+			},
+			requestCap:   20,
+			clusterID:    "1234",
+			requestZones: []string{"", ""},
+			expectedVolume: &csi.CreateVolumeResponse{
+				Volume: &csi.Volume{
+					CapacityBytes: 20,
+					VolumeId:      volumeID,
+					VolumeContext: map[string]string{VolumeIDLabel: volumeID, IOPSLabel: threeIops, utils.NodeRegionLabel: "my-region", utils.NodeZoneLabel: "testzone"},
+					AccessibleTopology: []*csi.Topology{{
+						Segments: map[string]string{
+							utils.NodeRegionLabel: "testregion",
+							utils.NodeZoneLabel:   "testzone",
+						},
+					},
+					},
+				},
+			},
+			expectedStatus: true,
+		},
+	}
+	// Setup test driver
+	icDriver := initIBMCSIDriver(t)
+	if icDriver == nil {
+		t.Fatalf("Failed to setup IBM CSI Driver")
 	}
 
 	for _, testcase := range testCases {
 		t.Run(testcase.testCaseName, func(t *testing.T) {
-			actualCSIVolume := createCSIVolumeResponse(testcase.requestVol, testcase.requestCap, testcase.requestZones, testcase.clusterID)
+			actualCSIVolume := createCSIVolumeResponse(testcase.requestVol, testcase.requestCap, testcase.requestZones, testcase.clusterID, icDriver.region)
 			assert.Equal(t, testcase.expectedStatus, isCSIResponseSame(testcase.expectedVolume, actualCSIVolume))
 		})
 	}

pkg/ibmcsidriver/controller_test.go

@@ -216,10 +216,14 @@ func TestCreateVolumeArguments(t *testing.T) {
 				},
 			},
 			expVol: &csi.Volume{
-				CapacityBytes:      20 * 1024 * 1024 * 1024, // In byte
-				VolumeId:           "testVolumeId",
-				VolumeContext:      map[string]string{utils.NodeRegionLabel: "myregion", utils.NodeZoneLabel: "myzone", VolumeIDLabel: "testVolumeId", Tag: "", VolumeCRNLabel: "", ClusterIDLabel: ""},
-				AccessibleTopology: stdTopology,
+				CapacityBytes: 20 * 1024 * 1024 * 1024, // In byte
+				VolumeId:      "testVolumeId",
+				VolumeContext: map[string]string{utils.NodeRegionLabel: "testregion", utils.NodeZoneLabel: "myzone", VolumeIDLabel: "testVolumeId", Tag: "", VolumeCRNLabel: "", ClusterIDLabel: ""},
+				AccessibleTopology: []*csi.Topology{
+					{
+						Segments: map[string]string{utils.NodeZoneLabel: "myzone", utils.NodeRegionLabel: "testregion"},
+					},
+				},
 			},
 			libVolumeResponse: &provider.Volume{Capacity: &cap, Name: &volName, VolumeID: "testVolumeId", Iops: &iopsStr, Az: "myzone", Region: "myregion"},
 			expErrCode:        codes.OK,
@@ -245,10 +249,14 @@ func TestCreateVolumeArguments(t *testing.T) {
 				},
 			},
 			expVol: &csi.Volume{
-				CapacityBytes:      20 * 1024 * 1024 * 1024, // In byte
-				VolumeId:           "testVolumeId",
-				VolumeContext:      map[string]string{utils.NodeRegionLabel: "myregion", utils.NodeZoneLabel: "myzone", VolumeIDLabel: "testVolumeId", Tag: "", VolumeCRNLabel: "", ClusterIDLabel: ""},
-				AccessibleTopology: stdTopology,
+				CapacityBytes: 20 * 1024 * 1024 * 1024, // In byte
+				VolumeId:      "testVolumeId",
+				VolumeContext: map[string]string{utils.NodeRegionLabel: "testregion", utils.NodeZoneLabel: "myzone", VolumeIDLabel: "testVolumeId", Tag: "", VolumeCRNLabel: "", ClusterIDLabel: ""},
+				AccessibleTopology: []*csi.Topology{
+					{
+						Segments: map[string]string{utils.NodeZoneLabel: "myzone", utils.NodeRegionLabel: "testregion"},
+					},
+				},
 			},
 			libVolumeResponse: &provider.Volume{Capacity: &cap, Name: &volName, VolumeID: "testVolumeId", Iops: &iopsStr, Az: "myzone", Region: "myregion"},
 			expErrCode:        codes.OK,

pkg/ibmcsidriver/ibm_csi_driver.go

@@ -19,21 +19,22 @@ package ibmcsidriver
 
 import (
 	"fmt"
-
 	cloudProvider "github.com/IBM/ibm-csi-common/pkg/ibmcloudprovider"
 	commonError "github.com/IBM/ibm-csi-common/pkg/messages"
 	nodeMetadata "github.com/IBM/ibm-csi-common/pkg/metadata"
 	mountManager "github.com/IBM/ibm-csi-common/pkg/mountmanager"
 	"github.com/IBM/ibm-csi-common/pkg/utils"
 	csi "github.com/container-storage-interface/spec/lib/go/csi"
 	"go.uber.org/zap"
+	"os"
 )
 
 // IBMCSIDriver ...
 type IBMCSIDriver struct {
 	name          string
 	vendorVersion string
 	logger        *zap.Logger
+	region        string
 
 	ids *CSIIdentityServer
 	ns  *CSINodeServer
@@ -44,6 +45,8 @@ type IBMCSIDriver struct {
 	nscap []*csi.NodeServiceCapability
 }
 
+var nodeMeta = nodeMetadata.NewNodeMetadata
+
 // GetIBMCSIDriver ...
 func GetIBMCSIDriver() *IBMCSIDriver {
 	return &IBMCSIDriver{}
@@ -105,6 +108,13 @@ func (icDriver *IBMCSIDriver) SetupIBMCSIDriver(provider cloudProvider.CloudProv
 
 	icDriver.logger.Info("Successfully setup IBM CSI driver")
 
+	// Set up Region
+	regionMetadata, err := nodeMeta(os.Getenv("KUBE_NODE_NAME"), lgr)
+	if err != nil {
+		return fmt.Errorf("Controller_Helper: Failed to initialize node metadata")
+	}
+	icDriver.region = regionMetadata.GetRegion()
+
 	return nil
 }
 

pkg/ibmcsidriver/ibm_csi_driver_test.go

@@ -29,12 +29,11 @@ import (
 func initIBMCSIDriver(t *testing.T) *IBMCSIDriver {
 	vendorVersion := "test-vendor-version-1.1.2"
 	driver := "mydriver"
-
 	// Creating test logger
 	logger, teardown := cloudProvider.GetTestLogger(t)
 	defer teardown()
 	icDriver := GetIBMCSIDriver()
-
+	nodeMeta = nodeMetadata.InitMetadata
 	// Create fake provider and mounter
 	provider, _ := cloudProvider.NewFakeIBMCloudStorageProvider("", logger)
 	mounter := mountManager.NewFakeNodeMounter()