Merge pull request #6 from akshata-s-banoshi/sprint-78-bruce-8184

Fix volume creation for Dell server

api/v1/volume_types.go

@@ -24,9 +24,9 @@ import (
 // VolumeSpec defines the desired state of Volume
 // +kubebuilder:validation:XPreserveUnknownFields
 type VolumeSpec struct {
-	StorageControllerID string `json:"storageControllerID,omitempty"`
-	RAIDType            string `json:"RAIDType,omitempty"`
-	Drives              []int  `json:"drives,omitempty"`
+	StorageControllerID string   `json:"storageControllerID,omitempty"`
+	RAIDType            string   `json:"RAIDType,omitempty"`
+	Drives              []string `json:"drives,omitempty"`
 }
 
 type Identifier struct {
@@ -41,7 +41,7 @@ type VolumeStatus struct {
 	VolumeName          string     `json:"volumeName"`
 	RAIDType            string     `json:"RAIDType"`
 	StorageControllerID string     `json:"storageControllerID"`
-	Drives              []int      `json:"drives"`
+	Drives              []string   `json:"drives"`
 	CapacityBytes       string     `json:"capacityBytes"`
 	Identifiers         Identifier `json:"Identifiers"`
 }

config/crd/bases/infra.io.odimra_volumes.yaml

@@ -52,7 +52,7 @@ spec:
                 type: string
               drives:
                 items:
-                  type: integer
+                  type: string
                 type: array
               storageControllerID:
                 type: string
@@ -77,7 +77,7 @@ spec:
                 type: string
               drives:
                 items:
-                  type: integer
+                  type: string
                 type: array
               storageControllerID:
                 type: string

controllers/pollData/mockUtils_test.go

@@ -189,7 +189,7 @@ func (m *MockCommonRec) GetVolumeObjectByVolumeID(ctx context.Context, volumeID,
 		volObj := &infraiov1.Volume{ObjectMeta: metav1.ObjectMeta{Name: "10.10.10.10.Volume6"},
 			Spec: infraiov1.VolumeSpec{
 				RAIDType: "RAID0",
-				Drives:   []int{6},
+				Drives:   []string{"6"},
 			}}
 		return volObj
 	}

controllers/pollData/reconcileVolume.go

@@ -18,13 +18,11 @@ import (
 	"context"
 	"fmt"
 	"net/http"
-	"reflect"
-	"sort"
-	"strconv"
 	"strings"
 
 	infraiov1 "github.com/ODIM-Project/BMCOperator/api/v1"
 	common "github.com/ODIM-Project/BMCOperator/controllers/common"
+	utils "github.com/ODIM-Project/BMCOperator/controllers/utils"
 	volume "github.com/ODIM-Project/BMCOperator/controllers/volume"
 	l "github.com/ODIM-Project/BMCOperator/logs"
 	"k8s.io/apimachinery/pkg/types"
@@ -235,7 +233,7 @@ func (pr PollingReconciler) getVolumeDetailsAndCreateVolumeObject(bmc *infraiov1
 		newVolume.ObjectMeta.Annotations = map[string]string{}
 		newVolume.ObjectMeta.Annotations["odata.id"] = volumeURL
 		controllerutil.AddFinalizer(&newVolume, volume.VolFinalizer)
-		var drives = []int{}
+		var drives = []string{}
 		var identifier infraiov1.Identifier
 		l.LogWithFields(pr.ctx).Info(fmt.Sprintf("Creating volume object %s...", newVolume.ObjectMeta.Name))
 		err = pr.Client.Create(pr.ctx, &newVolume)
@@ -248,10 +246,8 @@ func (pr PollingReconciler) getVolumeDetailsAndCreateVolumeObject(bmc *infraiov1
 		driveLinks := newVolumeDetails["Links"].(map[string]interface{})["Drives"].([]interface{})
 		for _, dl := range driveLinks {
 			drive := dl.(map[string]interface{})["@odata.id"].(string)
-			driveID, err := strconv.Atoi(drive[len(drive)-1:])
-			if err != nil {
-				l.Log.Info("Could not convert drive id to Integer")
-			}
+			driveURI := strings.Split(drive, "/")
+			driveID := driveURI[len(driveURI)-1]
 			drives = append(drives, driveID)
 		}
 		newVolume.Status.Drives = drives
@@ -355,19 +351,15 @@ func (pr PollingReconciler) getNewVolumeIDForObject(bmcObj *infraiov1.Bmc, stora
 			return "", nil
 		} else {
 			//getting all drives in the volume
-			drives := []int{}
+			drives := []string{}
 			driveLinks := getEachVolResp["Links"].(map[string]interface{})["Drives"].([]interface{})
 			for _, dl := range driveLinks {
 				drive := dl.(map[string]interface{})["@odata.id"].(string)
-				driveID, err := strconv.Atoi(drive[len(drive)-1:])
-				if err != nil {
-					l.Log.Info("Could not convert drive id to Integer")
-				}
+				driveURI := strings.Split(drive, "/")
+				driveID := driveURI[len(driveURI)-1]
 				drives = append(drives, driveID)
 			}
-			sort.Ints(drives)
-			sort.Ints(volObj.Status.Drives)
-			if getEachVolResp["Name"].(string) == volObj.Status.VolumeName && getEachVolResp["RAIDType"] == volObj.Status.RAIDType && reflect.DeepEqual(drives, volObj.Status.Drives) {
+			if getEachVolResp["RAIDType"] == volObj.Status.RAIDType && utils.CompareArray(drives, volObj.Status.Drives) {
 				return getEachVolResp["Id"].(string), nil
 			}
 		}

controllers/volume/volumeUtils.go

@@ -50,7 +50,7 @@ type requestPayload struct {
 	RAIDType    string `json:"RAIDType"`
 	Links       link   `json:"Links"`
 	DisplayName string `json:"DisplayName"`
-	ApplyTime   string `json:"@Redfish.OperationApplyTime"`
+	ApplyTime   string `json:"@Redfish.OperationApplyTime,omitempty"`
 }
 
 type link struct {

controllers/volume/volume_controller.go

@@ -21,8 +21,6 @@ import (
 	"net/http"
 	"os"
 	"reflect"
-	"sort"
-	"strconv"
 	"strings"
 	"time"
 
@@ -223,19 +221,18 @@ func (vu *volumeUtils) UpdateVolumeStatusAndClearSpec(bmcObj *infraiov1.Bmc, dis
 		} else {
 			//getting all drives in the volume
 			var found = false
-			drives := []int{}
+			drives := []string{}
 			driveLinks := getEachVolResp["Links"].(map[string]interface{})["Drives"].([]interface{})
 			for _, dl := range driveLinks {
 				drive := dl.(map[string]interface{})["@odata.id"].(string)
-				driveID, err := strconv.Atoi(drive[len(drive)-1:])
+				driveURI := strings.Split(drive, "/")
+				driveID := driveURI[len(driveURI)-1]
 				if err != nil {
 					l.Log.Info("Could not convert drive id to Integer")
 				}
 				drives = append(drives, driveID)
 			}
-			sort.Ints(drives)
-			sort.Ints(vu.volObj.Spec.Drives)
-			if getEachVolResp["Name"].(string) == dispName && getEachVolResp["RAIDType"] == vu.volObj.Spec.RAIDType && reflect.DeepEqual(drives, vu.volObj.Spec.Drives) {
+			if getEachVolResp["RAIDType"] == vu.volObj.Spec.RAIDType && utils.CompareArray(drives, vu.volObj.Spec.Drives) {
 				vu.volObj.ObjectMeta.Annotations["odata.id"] = getEachVolResp["@odata.id"].(string)
 				err = vu.commonRec.GetCommonReconcilerClient().Update(vu.ctx, vu.volObj)
 				if err != nil {
@@ -247,7 +244,7 @@ func (vu *volumeUtils) UpdateVolumeStatusAndClearSpec(bmcObj *infraiov1.Bmc, dis
 					durableName = id.(map[string]interface{})["DurableName"].(string)
 					durableNameFormat = id.(map[string]interface{})["DurableNameFormat"].(string)
 				}
-				vu.commonRec.UpdateVolumeStatus(vu.ctx, vu.volObj, getEachVolResp["Id"].(string), dispName, fmt.Sprintf("%f", getEachVolResp["CapacityBytes"].(float64)), durableName, durableNameFormat)
+				vu.commonRec.UpdateVolumeStatus(vu.ctx, vu.volObj, getEachVolResp["Id"].(string), getEachVolResp["Name"].(string), fmt.Sprintf("%f", getEachVolResp["CapacityBytes"].(float64)), durableName, durableNameFormat)
 				vu.volObj.Spec = infraiov1.VolumeSpec{}
 				err := vu.commonRec.GetCommonReconcilerClient().Update(vu.ctx, vu.volObj)
 				if err != nil {
@@ -274,17 +271,17 @@ func (vu *volumeUtils) GetVolumeRequestPayload(systemID, dispName string) []byte
 	if vu.volObj.Spec.RAIDType == "" && vu.volObj.Spec.Drives == nil && vu.volObj.Spec.StorageControllerID == "" {
 		for _, driveId := range vu.volObj.Status.Drives {
 			// TODO : check if the drive is present and then proceed
-			driveDet := map[string]string{"@odata.id": fmt.Sprintf("/redfish/v1/Systems/%s/Storage/%s/Drives/%s", systemID, vu.volObj.Status.StorageControllerID, strconv.Itoa(driveId))}
+			driveDet := map[string]string{"@odata.id": fmt.Sprintf("/redfish/v1/Systems/%s/Storage/%s/Drives/%s", systemID, vu.volObj.Status.StorageControllerID, driveId)}
 			linkBody.Drives = append(linkBody.Drives, driveDet)
 		}
 		reqBody = requestPayload{RAIDType: vu.volObj.Status.RAIDType, Links: linkBody, DisplayName: dispName, ApplyTime: constants.ApplyTime}
 	} else { // when volume is created via operator directly, it will pass through this case
 		for _, driveId := range vu.volObj.Spec.Drives {
 			// TODO : check if the drive is present and then proceed
-			driveDet := map[string]string{"@odata.id": fmt.Sprintf("/redfish/v1/Systems/%s/Storage/%s/Drives/%s", systemID, vu.volObj.Spec.StorageControllerID, strconv.Itoa(driveId))}
+			driveDet := map[string]string{"@odata.id": fmt.Sprintf("/redfish/v1/Systems/%s/Storage/%s/Drives/%s", systemID, vu.volObj.Spec.StorageControllerID, driveId)}
 			linkBody.Drives = append(linkBody.Drives, driveDet)
 		}
-		reqBody = requestPayload{RAIDType: vu.volObj.Spec.RAIDType, Links: linkBody, DisplayName: dispName, ApplyTime: constants.ApplyTime}
+		reqBody = requestPayload{RAIDType: vu.volObj.Spec.RAIDType, Links: linkBody, DisplayName: dispName}
 	}
 	marshalBody, err := json.Marshal(reqBody)
 	if err != nil {