Support specifying StorageClass while creating volumes

Add support for specifying StorageClass when user is creating
volumes via oc set volume command

Author: gnufied

pkg/cmd/cli/cmd/set/volume.go

@@ -29,7 +29,10 @@ import (
 	"github.com/openshift/origin/pkg/cmd/util/clientcmd"
 )
 
-const volumePrefix = "volume-"
+const (
+	volumePrefix    = "volume-"
+	storageAnnClass = "volume.beta.kubernetes.io/storage-class"
+)
 
 var (
 	volumeLong = templates.LongDesc(`
@@ -136,6 +139,7 @@ type AddVolumeOptions struct {
 	ClaimName   string
 	ClaimSize   string
 	ClaimMode   string
+	ClaimClass  string
 
 	TypeChanged bool
 }
@@ -186,6 +190,7 @@ func NewCmdVolume(fullName string, f *clientcmd.Factory, out, errOut io.Writer)
 	cmd.Flags().StringVar(&addOpts.ConfigMapName, "configmap-name", "", "Name of the persisted config map. Must be provided for configmap volume type")
 	cmd.Flags().StringVar(&addOpts.SecretName, "secret-name", "", "Name of the persisted secret. Must be provided for secret volume type")
 	cmd.Flags().StringVar(&addOpts.ClaimName, "claim-name", "", "Persistent volume claim name. Must be provided for persistentVolumeClaim volume type")
+	cmd.Flags().StringVar(&addOpts.ClaimClass, "claim-class", "", "StorageClass to use for provisioning the persistent volume.")
 	cmd.Flags().StringVar(&addOpts.ClaimSize, "claim-size", "", "If specified along with a persistent volume type, create a new claim with the given size in bytes. Accepts SI notation: 10, 10G, 10Gi")
 	cmd.Flags().StringVar(&addOpts.ClaimMode, "claim-mode", "ReadWriteOnce", "Set the access mode of the claim to be created. Valid values are ReadWriteOnce (rwo), ReadWriteMany (rwm), or ReadOnlyMany (rom)")
 	cmd.Flags().StringVar(&addOpts.Source, "source", "", "Details of volume source as json string. This can be used if the required volume type is not supported by --type option. (e.g.: '{\"gitRepo\": {\"repository\": <git-url>, \"revision\": <commit-hash>}}')")
@@ -314,6 +319,15 @@ func (a *AddVolumeOptions) Validate(isAddOp bool) error {
 				return err
 			}
 		}
+		if len(a.ClaimClass) > 0 {
+			selectedLowerType := strings.ToLower(a.Type)
+			if selectedLowerType != "persistentvolumeclaim" && selectedLowerType != "pvc" {
+				return errors.New("must provide --type as persistentVolumeClaim")
+			}
+			if len(a.ClaimSize) == 0 {
+				return errors.New("must provide --claim-size to create new pvc with claim-class")
+			}
+		}
 	} else if len(a.Source) > 0 || len(a.Path) > 0 || len(a.SecretName) > 0 || len(a.ConfigMapName) > 0 || len(a.ClaimName) > 0 || a.Overwrite {
 		return errors.New("--type|--path|--configmap-name|--secret-name|--claim-name|--source|--overwrite are only valid for --add operation")
 	}
@@ -563,7 +577,7 @@ func (v *VolumeOptions) printVolumes(infos []*resource.Info) []error {
 }
 
 func (v *AddVolumeOptions) createClaim() *kapi.PersistentVolumeClaim {
-	return &kapi.PersistentVolumeClaim{
+	pvc := &kapi.PersistentVolumeClaim{
 		ObjectMeta: kapi.ObjectMeta{
 			Name: v.ClaimName,
 		},
@@ -576,6 +590,12 @@ func (v *AddVolumeOptions) createClaim() *kapi.PersistentVolumeClaim {
 			},
 		},
 	}
+	if len(v.ClaimClass) > 0 {
+		pvc.Annotations = map[string]string{
+			storageAnnClass: v.ClaimClass,
+		}
+	}
+	return pvc
 }
 
 func (v *VolumeOptions) setVolumeSource(kv *kapi.Volume) error {

pkg/cmd/cli/cmd/set/volume_test.go

@@ -1,6 +1,7 @@
 package set
 
 import (
+	"errors"
 	"net/http"
 	"testing"
 
@@ -161,3 +162,72 @@ func TestAddVolume(t *testing.T) {
 		t.Error(patchError)
 	}
 }
+
+func TestCreateClaim(t *testing.T) {
+	addOpts := &AddVolumeOptions{
+		Type:       "persistentVolumeClaim",
+		ClaimClass: "foobar",
+		ClaimName:  "foo-vol",
+		ClaimSize:  "5G",
+		MountPath:  "/sandbox",
+	}
+
+	pvc := addOpts.createClaim()
+	if len(pvc.Annotations) == 0 {
+		t.Errorf("Expected storage class annotation")
+	}
+
+	if pvc.Annotations[storageAnnClass] != "foobar" {
+		t.Errorf("Expected storage annotated class to be %s", addOpts.ClaimClass)
+	}
+}
+
+func TestValidateAddOptions(t *testing.T) {
+	tests := []struct {
+		name          string
+		addOpts       *AddVolumeOptions
+		expectedError error
+	}{
+		{
+			"using existing pvc",
+			&AddVolumeOptions{Type: "persistentVolumeClaim"},
+			errors.New("must provide --claim-name or --claim-size (to create a new claim) for --type=pvc"),
+		},
+		{
+			"creating new pvc",
+			&AddVolumeOptions{Type: "persistentVolumeClaim", ClaimName: "sandbox-pvc", ClaimSize: "5G"},
+			nil,
+		},
+		{
+			"error creating pvc with storage class",
+			&AddVolumeOptions{Type: "persistentVolumeClaim", ClaimName: "sandbox-pvc", ClaimClass: "slow"},
+			errors.New("must provide --claim-size to create new pvc with claim-class"),
+		},
+		{
+			"creating pvc with storage class",
+			&AddVolumeOptions{Type: "persistentVolumeClaim", ClaimName: "sandbox-pvc", ClaimClass: "slow", ClaimSize: "5G"},
+			nil,
+		},
+	}
+
+	for _, testCase := range tests {
+		addOpts := testCase.addOpts
+		err := addOpts.Validate(true)
+		if testCase.expectedError == nil && err != nil {
+			t.Errorf("Expected nil error for %s got %s", testCase.name, err)
+			continue
+		}
+
+		if testCase.expectedError != nil {
+			if err == nil {
+				t.Errorf("Expected %s, got nil", testCase.expectedError)
+				continue
+			}
+
+			if testCase.expectedError.Error() != err.Error() {
+				t.Errorf("Expected %s, got %s", testCase.expectedError, err)
+			}
+		}
+
+	}
+}