Merge pull request #70 from gnufied/add-extra-fields

Add volume capability to ControllerExpandVolume CSI call

Author: k8s-ci-robot

README.md

@@ -16,7 +16,8 @@ This information reflects the head of this branch.
 
 | Compatible with CSI Version                                                                | Container Image                | Recommended K8s Version |
 | ------------------------------------------------------------------------------------------ | -------------------------------| --------------- |
-| [CSI Spec v1.1.0](https://github.com/container-storage-interface/spec/releases/tag/v1.1.0) | quay.io/k8scsi/csi-resizer | 1.16            |
+| [CSI Spec v1.2.0](https://github.com/container-storage-interface/spec/releases/tag/v1.2.0) | quay.io/k8scsi/csi-resizer | 1.16            |
+
 
 
 ## Feature status

go.mod

@@ -3,7 +3,7 @@ module github.com/kubernetes-csi/external-resizer
 go 1.12
 
 require (
-	github.com/container-storage-interface/spec v1.1.0
+	github.com/container-storage-interface/spec v1.2.0
 	github.com/imdario/mergo v0.3.7 // indirect
 	github.com/kubernetes-csi/csi-lib-utils v0.7.0
 	google.golang.org/grpc v1.26.0

go.sum

@@ -24,6 +24,8 @@ github.com/census-instrumentation/opencensus-proto v0.2.1/go.mod h1:f6KPmirojxKA
 github.com/client9/misspell v0.3.4/go.mod h1:qj6jICC3Q7zFZvVWo7KLAzC3yx5G7kyvSDkc90ppPyw=
 github.com/container-storage-interface/spec v1.1.0 h1:qPsTqtR1VUPvMPeK0UnCZMtXaKGyyLPG8gj/wG6VqMs=
 github.com/container-storage-interface/spec v1.1.0/go.mod h1:6URME8mwIBbpVyZV93Ce5St17xBiQJQY67NDsuohiy4=
+github.com/container-storage-interface/spec v1.2.0 h1:bD9KIVgaVKKkQ/UbVUY9kCaH/CJbhNxe0eeB4JeJV2s=
+github.com/container-storage-interface/spec v1.2.0/go.mod h1:6URME8mwIBbpVyZV93Ce5St17xBiQJQY67NDsuohiy4=
 github.com/davecgh/go-spew v0.0.0-20151105211317-5215b55f46b2/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
 github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
 github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c=

pkg/controller/controller_test.go

@@ -22,14 +22,17 @@ import (
 )
 
 func TestController(t *testing.T) {
+	blockVolumeMode := v1.PersistentVolumeBlock
+	fsVolumeMode := v1.PersistentVolumeFilesystem
 	for _, test := range []struct {
 		Name string
 		PVC  *v1.PersistentVolumeClaim
 		PV   *v1.PersistentVolume
 
-		CreateObjects bool
-		NodeResize    bool
-		CallCSIExpand bool
+		CreateObjects     bool
+		NodeResize        bool
+		CallCSIExpand     bool
+		expectBlockVolume bool
 	}{
 		{
 			Name:          "Invalid key",
@@ -56,36 +59,45 @@ func TestController(t *testing.T) {
 		{
 			Name:          "pv claimref does not have pvc UID",
 			PVC:           createPVC(2, 1),
-			PV:            createPV(1, "testPVC" /*pvcName*/, "test" /*pvcNamespace*/, "foobaz" /*pvcUID*/),
+			PV:            createPV(1, "testPVC" /*pvcName*/, "test" /*pvcNamespace*/, "foobaz" /*pvcUID*/, &fsVolumeMode),
 			CallCSIExpand: false,
 		},
 		{
 			Name:          "pv claimref does not have PVC namespace",
 			PVC:           createPVC(2, 1),
-			PV:            createPV(1, "testPVC" /*pvcName*/, "test1" /*pvcNamespace*/, "foobar" /*pvcUID*/),
+			PV:            createPV(1, "testPVC" /*pvcName*/, "test1" /*pvcNamespace*/, "foobar" /*pvcUID*/, &fsVolumeMode),
 			CallCSIExpand: false,
 		},
 		{
 			Name:          "pv claimref is nil",
 			PVC:           createPVC(2, 1),
-			PV:            createPV(1, "" /*pvcName*/, "test1" /*pvcNamespace*/, "foobar" /*pvcUID*/),
+			PV:            createPV(1, "" /*pvcName*/, "test1" /*pvcNamespace*/, "foobar" /*pvcUID*/, &fsVolumeMode),
 			CallCSIExpand: false,
 		},
 		{
 			Name:          "Resize PVC, no FS resize",
 			PVC:           createPVC(2, 1),
-			PV:            createPV(1, "testPVC", "test", "foobar"),
+			PV:            createPV(1, "testPVC", "test", "foobar", &fsVolumeMode),
 			CreateObjects: true,
 			CallCSIExpand: true,
 		},
 		{
 			Name:          "Resize PVC with FS resize",
 			PVC:           createPVC(2, 1),
-			PV:            createPV(1, "testPVC", "test", "foobar"),
+			PV:            createPV(1, "testPVC", "test", "foobar", &fsVolumeMode),
 			CreateObjects: true,
 			NodeResize:    true,
 			CallCSIExpand: true,
 		},
+		{
+			Name:              "Block Resize PVC with FS resize",
+			PVC:               createPVC(2, 1),
+			PV:                createPV(1, "testPVC", "test", "foobar", &blockVolumeMode),
+			CreateObjects:     true,
+			NodeResize:        true,
+			CallCSIExpand:     true,
+			expectBlockVolume: true,
+		},
 	} {
 		client := csi.NewMockClient("mock", test.NodeResize, true, true)
 		driverName, _ := client.GetDriverName(context.TODO())
@@ -138,6 +150,16 @@ func TestController(t *testing.T) {
 		if !test.CallCSIExpand && expandCallCount > 0 {
 			t.Fatalf("for %s: expected no csi expand call, received csi expansion request", test.Name)
 		}
+
+		usedCapability := client.GetCapability()
+
+		if test.CallCSIExpand && test.expectBlockVolume && usedCapability.GetBlock() == nil {
+			t.Errorf("For %s: expected block accesstype got: %v", test.Name, usedCapability)
+		}
+
+		if test.CallCSIExpand && !test.expectBlockVolume && usedCapability.GetMount() == nil {
+			t.Errorf("For %s: expected mount accesstype got: %v", test.Name, usedCapability)
+		}
 	}
 }
 
@@ -181,14 +203,15 @@ func createPVC(requestGB, capacityGB int) *v1.PersistentVolumeClaim {
 	}
 }
 
-func createPV(capacityGB int, pvcName, pvcNamespace string, pvcUID types.UID) *v1.PersistentVolume {
+func createPV(capacityGB int, pvcName, pvcNamespace string, pvcUID types.UID, volumeMode *v1.PersistentVolumeMode) *v1.PersistentVolume {
 	capacity := quantityGB(capacityGB)
 
 	pv := &v1.PersistentVolume{
 		ObjectMeta: metav1.ObjectMeta{
 			Name: "testPV",
 		},
 		Spec: v1.PersistentVolumeSpec{
+			AccessModes: []v1.PersistentVolumeAccessMode{v1.ReadWriteOnce},
 			Capacity: map[v1.ResourceName]resource.Quantity{
 				v1.ResourceStorage: capacity,
 			},
@@ -198,6 +221,7 @@ func createPV(capacityGB int, pvcName, pvcNamespace string, pvcUID types.UID) *v
 					VolumeHandle: "foo",
 				},
 			},
+			VolumeMode: volumeMode,
 		},
 	}
 	if len(pvcName) > 0 {

pkg/csi/client.go

@@ -48,7 +48,7 @@ type Client interface {
 
 	// Expand expands the volume to a new size at least as big as requestBytes.
 	// It returns the new size and whether the volume need expand operation on the node.
-	Expand(ctx context.Context, volumeID string, requestBytes int64, secrets map[string]string) (int64, bool, error)
+	Expand(ctx context.Context, volumeID string, requestBytes int64, secrets map[string]string, capability *csi.VolumeCapability) (int64, bool, error)
 }
 
 // New creates a new CSI client.
@@ -120,11 +120,13 @@ func (c *client) Expand(
 	ctx context.Context,
 	volumeID string,
 	requestBytes int64,
-	secrets map[string]string) (int64, bool, error) {
+	secrets map[string]string,
+	capability *csi.VolumeCapability) (int64, bool, error) {
 	req := &csi.ControllerExpandVolumeRequest{
-		Secrets:       secrets,
-		VolumeId:      volumeID,
-		CapacityRange: &csi.CapacityRange{RequiredBytes: requestBytes},
+		Secrets:          secrets,
+		VolumeId:         volumeID,
+		CapacityRange:    &csi.CapacityRange{RequiredBytes: requestBytes},
+		VolumeCapability: capability,
 	}
 	resp, err := c.ctrlClient.ControllerExpandVolume(ctx, req)
 	if err != nil {

pkg/csi/mock_client.go

@@ -1,6 +1,10 @@
 package csi
 
-import "context"
+import (
+	"context"
+
+	"github.com/container-storage-interface/spec/lib/go/csi"
+)
 
 func NewMockClient(
 	name string,
@@ -23,6 +27,7 @@ type MockClient struct {
 	supportsPluginControllerService bool
 	expandCalled                    int
 	usedSecrets                     map[string]string
+	usedCapability                  *csi.VolumeCapability
 }
 
 func (c *MockClient) GetDriverName(context.Context) (string, error) {
@@ -45,17 +50,23 @@ func (c *MockClient) Expand(
 	ctx context.Context,
 	volumeID string,
 	requestBytes int64,
-	secrets map[string]string) (int64, bool, error) {
+	secrets map[string]string,
+	capability *csi.VolumeCapability) (int64, bool, error) {
 	// TODO: Determine whether the operation succeeds or fails by parameters.
 	c.expandCalled++
 	c.usedSecrets = secrets
+	c.usedCapability = capability
 	return requestBytes, c.supportsNodeResize, nil
 }
 
 func (c *MockClient) GetExpandCount() int {
 	return c.expandCalled
 }
 
+func (c *MockClient) GetCapability() *csi.VolumeCapability {
+	return c.usedCapability
+}
+
 // GetSecrets returns secrets used for volume expansion
 func (c *MockClient) GetSecrets() map[string]string {
 	return c.usedSecrets

pkg/resizer/csi_resizer.go

@@ -22,6 +22,7 @@ import (
 	"fmt"
 	"time"
 
+	csilib "github.com/container-storage-interface/spec/lib/go/csi"
 	"github.com/kubernetes-csi/csi-lib-utils/metrics"
 	"github.com/kubernetes-csi/external-resizer/pkg/csi"
 	"github.com/kubernetes-csi/external-resizer/pkg/util"
@@ -157,10 +158,12 @@ func (r *csiResizer) Resize(pv *v1.PersistentVolume, requestSize resource.Quanti
 
 	var volumeID string
 	var source *v1.CSIPersistentVolumeSource
+	var pvSpec v1.PersistentVolumeSpec
 	if pv.Spec.CSI != nil {
 		// handle CSI volume
 		source = pv.Spec.CSI
 		volumeID = source.VolumeHandle
+		pvSpec = pv.Spec
 	} else {
 		if csitranslationlib.IsMigratedCSIDriverByName(r.name) {
 			// handle migrated in-tree volume
@@ -169,6 +172,7 @@ func (r *csiResizer) Resize(pv *v1.PersistentVolume, requestSize resource.Quanti
 				return oldSize, false, fmt.Errorf("failed to translate persistent volume: %v", err)
 			}
 			source = csiPV.Spec.CSI
+			pvSpec = csiPV.Spec
 			volumeID = source.VolumeHandle
 		} else {
 			// non-migrated in-tree volume
@@ -190,16 +194,79 @@ func (r *csiResizer) Resize(pv *v1.PersistentVolume, requestSize resource.Quanti
 		}
 	}
 
+	capability, err := GetVolumeCapabilities(pvSpec)
+	if err != nil {
+		return oldSize, false, fmt.Errorf("failed to get capabilities of volume %s with %v", pv.Name, err)
+	}
+
 	ctx, cancel := timeoutCtx(r.timeout)
 	defer cancel()
-	newSizeBytes, nodeResizeRequired, err := r.client.Expand(ctx, volumeID, requestSize.Value(), secrets)
+	newSizeBytes, nodeResizeRequired, err := r.client.Expand(ctx, volumeID, requestSize.Value(), secrets, capability)
 	if err != nil {
 		return oldSize, nodeResizeRequired, err
 	}
 
 	return *resource.NewQuantity(newSizeBytes, resource.BinarySI), nodeResizeRequired, err
 }
 
+// GetVolumeCapabilities returns volumecapability from PV spec
+func GetVolumeCapabilities(pvSpec v1.PersistentVolumeSpec) (*csilib.VolumeCapability, error) {
+	m := map[v1.PersistentVolumeAccessMode]bool{}
+	for _, mode := range pvSpec.AccessModes {
+		m[mode] = true
+	}
+
+	if pvSpec.CSI == nil {
+		return nil, errors.New("CSI volume source was nil")
+	}
+
+	var cap *csilib.VolumeCapability
+	if pvSpec.VolumeMode != nil && *pvSpec.VolumeMode == v1.PersistentVolumeBlock {
+		cap = &csilib.VolumeCapability{
+			AccessType: &csilib.VolumeCapability_Block{
+				Block: &csilib.VolumeCapability_BlockVolume{},
+			},
+			AccessMode: &csilib.VolumeCapability_AccessMode{},
+		}
+
+	} else {
+		fsType := pvSpec.CSI.FSType
+
+		cap = &csilib.VolumeCapability{
+			AccessType: &csilib.VolumeCapability_Mount{
+				Mount: &csilib.VolumeCapability_MountVolume{
+					FsType:     fsType,
+					MountFlags: pvSpec.MountOptions,
+				},
+			},
+			AccessMode: &csilib.VolumeCapability_AccessMode{},
+		}
+	}
+
+	// Translate array of modes into single VolumeCapability
+	switch {
+	case m[v1.ReadWriteMany]:
+		// ReadWriteMany trumps everything, regardless what other modes are set
+		cap.AccessMode.Mode = csilib.VolumeCapability_AccessMode_MULTI_NODE_MULTI_WRITER
+
+	case m[v1.ReadOnlyMany] && m[v1.ReadWriteOnce]:
+		// This is no way how to translate this to CSI...
+		return nil, fmt.Errorf("CSI does not support ReadOnlyMany and ReadWriteOnce on the same PersistentVolume")
+
+	case m[v1.ReadOnlyMany]:
+		// There is only ReadOnlyMany set
+		cap.AccessMode.Mode = csilib.VolumeCapability_AccessMode_MULTI_NODE_READER_ONLY
+
+	case m[v1.ReadWriteOnce]:
+		// There is only ReadWriteOnce set
+		cap.AccessMode.Mode = csilib.VolumeCapability_AccessMode_SINGLE_NODE_WRITER
+
+	default:
+		return nil, fmt.Errorf("unsupported AccessMode combination: %+v", pvSpec.AccessModes)
+	}
+	return cap, nil
+}
+
 func getDriverName(client csi.Client, timeout time.Duration) (string, error) {
 	ctx, cancel := timeoutCtx(timeout)
 	defer cancel()