vSphere: Return useful errors from parameter validation

Returning "VolumeOptions verification failed" when StorageClass parameter
validation failed is not really useful. It does not tell which parameter is
wrong and what is a VolumeOption at all.

Trying to return something more useful here.

staging/src/k8s.io/legacy-cloud-providers/vsphere/vclib/custom_errors.go

@@ -20,20 +20,18 @@ import "errors"
 
 // Error Messages
 const (
-	FileAlreadyExistErrMsg     = "File requested already exist"
+	FileAlreadyExistErrMsg = "File requested already exist"
-	NoDiskUUIDFoundErrMsg      = "No disk UUID found"
+	NoDiskUUIDFoundErrMsg  = "No disk UUID found"
-	NoDevicesFoundErrMsg       = "No devices found"
+	NoDevicesFoundErrMsg   = "No devices found"
-	DiskNotFoundErrMsg         = "No vSphere disk ID found"
+	DiskNotFoundErrMsg     = "No vSphere disk ID found"
-	InvalidVolumeOptionsErrMsg = "VolumeOptions verification failed"
+	NoVMFoundErrMsg        = "No VM found"
-	NoVMFoundErrMsg            = "No VM found"
 )
 
 // Error constants
 var (
-	ErrFileAlreadyExist     = errors.New(FileAlreadyExistErrMsg)
+	ErrFileAlreadyExist = errors.New(FileAlreadyExistErrMsg)
-	ErrNoDiskUUIDFound      = errors.New(NoDiskUUIDFoundErrMsg)
+	ErrNoDiskUUIDFound  = errors.New(NoDiskUUIDFoundErrMsg)
-	ErrNoDevicesFound       = errors.New(NoDevicesFoundErrMsg)
+	ErrNoDevicesFound   = errors.New(NoDevicesFoundErrMsg)
-	ErrNoDiskIDFound        = errors.New(DiskNotFoundErrMsg)
+	ErrNoDiskIDFound    = errors.New(DiskNotFoundErrMsg)
-	ErrInvalidVolumeOptions = errors.New(InvalidVolumeOptionsErrMsg)
+	ErrNoVMFound        = errors.New(NoVMFoundErrMsg)
-	ErrNoVMFound            = errors.New(NoVMFoundErrMsg)
 )

staging/src/k8s.io/legacy-cloud-providers/vsphere/vclib/diskmanagers/virtualdisk.go

@@ -64,9 +64,9 @@ func (virtualDisk *VirtualDisk) Create(ctx context.Context, datastore *vclib.Dat
 	if virtualDisk.VolumeOptions.DiskFormat == "" {
 		virtualDisk.VolumeOptions.DiskFormat = vclib.ThinDiskType
 	}
-	if !virtualDisk.VolumeOptions.VerifyVolumeOptions() {
+	if err := virtualDisk.VolumeOptions.VerifyVolumeOptions(); err != nil {
-		klog.Error("VolumeOptions verification failed. volumeOptions: ", virtualDisk.VolumeOptions)
+		klog.Errorf("VolumeOptions verification failed: %s (options: %+v)", err, virtualDisk.VolumeOptions)
-		return "", vclib.ErrInvalidVolumeOptions
+		return "", fmt.Errorf("validation of parameters failed: %s", err)
 	}
 	if virtualDisk.VolumeOptions.StoragePolicyID != "" && virtualDisk.VolumeOptions.StoragePolicyName != "" {
 		return "", fmt.Errorf("Storage Policy ID and Storage Policy Name both set, Please set only one parameter")

staging/src/k8s.io/legacy-cloud-providers/vsphere/vclib/volumeoptions.go

@@ -17,6 +17,7 @@ limitations under the License.
 package vclib
 
 import (
+	"fmt"
 	"strings"
 
 	"k8s.io/api/core/v1"
@@ -90,21 +91,21 @@ func CheckControllerSupported(ctrlType string) bool {
 }
 
 // VerifyVolumeOptions checks if volumeOptions.SCIControllerType is valid controller type
-func (volumeOptions VolumeOptions) VerifyVolumeOptions() bool {
+func (volumeOptions VolumeOptions) VerifyVolumeOptions() error {
 	// Validate only if SCSIControllerType is set by user.
 	// Default value is set later in virtualDiskManager.Create and vmDiskManager.Create
 	if volumeOptions.SCSIControllerType != "" {
 		isValid := CheckControllerSupported(volumeOptions.SCSIControllerType)
 		if !isValid {
-			return false
+			return fmt.Errorf("invalid scsiControllerType: %s", volumeOptions.SCSIControllerType)
 		}
 	}
 	// ThinDiskType is the default, so skip the validation.
 	if volumeOptions.DiskFormat != ThinDiskType {
 		isValid := CheckDiskFormatSupported(volumeOptions.DiskFormat)
 		if !isValid {
-			return false
+			return fmt.Errorf("invalid diskFormat: %s", volumeOptions.DiskFormat)
 		}
 	}
-	return true
+	return nil
 }