Merge pull request #830 from mkimuram/blockcheckopt1

[v2] Add block volume support check to external provisioners

iscsi/targetd/provisioner/iscsi-provisioner.go

@@ -475,3 +475,7 @@ func (p *iscsiProvisioner) getConnection() (*jsonrpc2.Client, error) {
 	log.Debugln("targetd client created")
 	return client, nil
 }
+
+func (p *iscsiProvisioner) SupportsBlock() bool {
+	return true
+}

lib/controller/controller.go

@@ -30,6 +30,7 @@ import (
 	"github.com/kubernetes-incubator/external-storage/lib/controller/metrics"
 	"github.com/kubernetes-incubator/external-storage/lib/leaderelection"
 	rl "github.com/kubernetes-incubator/external-storage/lib/leaderelection/resourcelock"
+	"github.com/kubernetes-incubator/external-storage/lib/util"
 	"github.com/prometheus/client_golang/prometheus"
 	"github.com/prometheus/client_golang/prometheus/promhttp"
 	"golang.org/x/time/rate"
@@ -896,6 +897,16 @@ func (ctrl *ProvisionController) shouldDelete(volume *v1.PersistentVolume) bool
 	return true
 }
 
+// canProvision returns error if provisioner can't provision claim.
+func (ctrl *ProvisionController) canProvision(claim *v1.PersistentVolumeClaim) error {
+	// Check if this provisioner supports Block volume
+	if util.CheckPersistentVolumeClaimModeBlock(claim) && !ctrl.supportsBlock() {
+		return fmt.Errorf("%s does not support block volume provisioning", ctrl.provisionerName)
+	}
+
+	return nil
+}
+
 // lockProvisionClaimOperation wraps provisionClaimOperation. In case other
 // controllers are serving the same claims, to prevent them all from creating
 // volumes for a claim & racing to submit their PV, each controller creates a
@@ -1021,6 +1032,14 @@ func (ctrl *ProvisionController) provisionClaimOperation(claim *v1.PersistentVol
 		return nil
 	}
 
+	// Check if this provisioner can provision this claim.
+	if err := ctrl.canProvision(claim); err != nil {
+		ctrl.eventRecorder.Event(claim, v1.EventTypeWarning, "ProvisioningFailed", err.Error())
+		glog.Errorf("Failed to provision volume for claim %q with StorageClass %q: %v",
+			claimToClaimKey(claim), claimClass, err)
+		return nil
+	}
+
 	reclaimPolicy := v1.PersistentVolumeReclaimDelete
 	if ctrl.kubeVersion.AtLeast(utilversion.MustParseSemantic("v1.8.0")) {
 		reclaimPolicy, err = ctrl.fetchReclaimPolicy(claimClass)
@@ -1361,3 +1380,13 @@ func (ctrl *ProvisionController) fetchReclaimPolicy(storageClassName string) (v1
 
 	return v1.PersistentVolumeReclaimDelete, fmt.Errorf("Cannot convert object to StorageClass: %+v", classObj)
 }
+
+// supportsBlock returns whether a provisioner supports block volume.
+// Provisioners that implement BlockProvisioner interface and return true to SupportsBlock
+// will be regarded as supported for block volume.
+func (ctrl *ProvisionController) supportsBlock() bool {
+	if blockProvisioner, ok := ctrl.provisioner.(BlockProvisioner); ok {
+		return blockProvisioner.SupportsBlock()
+	}
+	return false
+}

lib/controller/controller_test.go

@@ -495,6 +495,93 @@ func TestShouldDelete(t *testing.T) {
 	}
 }
 
+func TestCanProvision(t *testing.T) {
+	const (
+		provisionerName = "foo.bar/baz"
+		blockErrFormat  = "%s does not support block volume provisioning"
+	)
+
+	tests := []struct {
+		name             string
+		provisioner      Provisioner
+		claim            *v1.PersistentVolumeClaim
+		serverGitVersion string
+		expectedCan      error
+	}{
+		// volumeMode tests for provisioner w/o BlockProvisoner I/F
+		{
+			name:        "Undefined volumeMode PV request to provisioner w/o BlockProvisoner I/F",
+			provisioner: newTestProvisioner(),
+			claim:       newClaim("claim-1", "1-1", "class-1", provisionerName, "", nil),
+			expectedCan: nil,
+		},
+		{
+			name:        "FileSystem volumeMode PV request to provisioner w/o BlockProvisoner I/F",
+			provisioner: newTestProvisioner(),
+			claim:       newClaimWithVolumeMode("claim-1", "1-1", "class-1", provisionerName, "", nil, v1.PersistentVolumeFilesystem),
+			expectedCan: nil,
+		},
+		{
+			name:        "Block volumeMode PV request to provisioner w/o BlockProvisoner I/F",
+			provisioner: newTestProvisioner(),
+			claim:       newClaimWithVolumeMode("claim-1", "1-1", "class-1", provisionerName, "", nil, v1.PersistentVolumeBlock),
+			expectedCan: fmt.Errorf(blockErrFormat, provisionerName),
+		},
+		// volumeMode tests for BlockProvisioner that returns false
+		{
+			name:        "Undefined volumeMode PV request to BlockProvisoner that returns false",
+			provisioner: newTestBlockProvisioner(false),
+			claim:       newClaim("claim-1", "1-1", "class-1", provisionerName, "", nil),
+			expectedCan: nil,
+		},
+		{
+			name:        "FileSystem volumeMode PV request to BlockProvisoner that returns false",
+			provisioner: newTestBlockProvisioner(false),
+			claim:       newClaimWithVolumeMode("claim-1", "1-1", "class-1", provisionerName, "", nil, v1.PersistentVolumeFilesystem),
+			expectedCan: nil,
+		},
+		{
+			name:        "Block volumeMode PV request to BlockProvisoner that returns false",
+			provisioner: newTestBlockProvisioner(false),
+			claim:       newClaimWithVolumeMode("claim-1", "1-1", "class-1", provisionerName, "", nil, v1.PersistentVolumeBlock),
+			expectedCan: fmt.Errorf(blockErrFormat, provisionerName),
+		},
+		// volumeMode tests for BlockProvisioner that returns true
+		{
+			name:        "Undefined volumeMode PV request to BlockProvisoner that returns true",
+			provisioner: newTestBlockProvisioner(true),
+			claim:       newClaim("claim-1", "1-1", "class-1", provisionerName, "", nil),
+			expectedCan: nil,
+		},
+		{
+			name:        "FileSystem volumeMode PV request to BlockProvisoner that returns true",
+			provisioner: newTestBlockProvisioner(true),
+			claim:       newClaimWithVolumeMode("claim-1", "1-1", "class-1", provisionerName, "", nil, v1.PersistentVolumeFilesystem),
+			expectedCan: nil,
+		},
+		{
+			name:        "Block volumeMode PV request to BlockProvisioner that returns true",
+			provisioner: newTestBlockProvisioner(true),
+			claim:       newClaimWithVolumeMode("claim-1", "1-1", "class-1", provisionerName, "", nil, v1.PersistentVolumeBlock),
+			expectedCan: nil,
+		},
+	}
+	for _, test := range tests {
+		client := fake.NewSimpleClientset(test.claim)
+		serverVersion := defaultServerVersion
+		if test.serverGitVersion != "" {
+			serverVersion = test.serverGitVersion
+		}
+		ctrl := newTestProvisionController(client, provisionerName, test.provisioner, serverVersion)
+
+		can := ctrl.canProvision(test.claim)
+		if !reflect.DeepEqual(test.expectedCan, can) {
+			t.Logf("test case: %s", test.name)
+			t.Errorf("expected can provision %v but got %v\n", test.expectedCan, can)
+		}
+	}
+}
+
 func TestControllerSharedInformers(t *testing.T) {
 	tests := []struct {
 		name            string
@@ -684,6 +771,12 @@ func newClaim(name, claimUID, class, provisioner, volumeName string, annotations
 	return claim
 }
 
+func newClaimWithVolumeMode(name, claimUID, class, provisioner, volumeName string, annotations map[string]string, volumeMode v1.PersistentVolumeMode) *v1.PersistentVolumeClaim {
+	claim := newClaim(name, claimUID, class, provisioner, volumeName, annotations)
+	claim.Spec.VolumeMode = &volumeMode
+	return claim
+}
+
 func newVolume(name string, phase v1.PersistentVolumePhase, policy v1.PersistentVolumeReclaimPolicy, annotations map[string]string) *v1.PersistentVolume {
 	pv := &v1.PersistentVolume{
 		ObjectMeta: metav1.ObjectMeta{
@@ -786,6 +879,22 @@ func (p *testQualifiedProvisioner) ShouldProvision(claim *v1.PersistentVolumeCla
 	return p.answer
 }
 
+func newTestBlockProvisioner(answer bool) *testBlockProvisioner {
+	return &testBlockProvisioner{newTestProvisioner(), answer}
+}
+
+type testBlockProvisioner struct {
+	*testProvisioner
+	answer bool
+}
+
+var _ Provisioner = &testBlockProvisioner{}
+var _ BlockProvisioner = &testBlockProvisioner{}
+
+func (p *testBlockProvisioner) SupportsBlock() bool {
+	return p.answer
+}
+
 func (p *testProvisioner) Provision(options VolumeOptions) (*v1.PersistentVolume, error) {
 	p.provisionCalls <- true
 

lib/controller/volume.go

@@ -47,6 +47,14 @@ type Qualifier interface {
 	ShouldProvision(*v1.PersistentVolumeClaim) bool
 }
 
+// BlockProvisioner is an optional interface implemented by provisioners to determine
+// whether it supports block volume.
+type BlockProvisioner interface {
+	Provisioner
+	// SupportsBlock returns whether provisioner supports block volume.
+	SupportsBlock() bool
+}
+
 // IgnoredError is the value for Delete to return to indicate that the call has
 // been ignored and no action taken. In case multiple provisioners are serving
 // the same storage class, provisioners may ignore PVs they are not responsible

lib/util/util.go

@@ -59,3 +59,9 @@ func AccessModesContainedInAll(indexedModes []v1.PersistentVolumeAccessMode, req
 	}
 	return true
 }
+
+// CheckPersistentVolumeClaimModeBlock checks VolumeMode.
+// If the mode is Block, return true otherwise return false.
+func CheckPersistentVolumeClaimModeBlock(pvc *v1.PersistentVolumeClaim) bool {
+	return pvc.Spec.VolumeMode != nil && *pvc.Spec.VolumeMode == v1.PersistentVolumeBlock
+}