Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add volume capability to ControllerExpandVolume CSI call #70

Merged
merged 5 commits into from
Apr 2, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 2 additions & 0 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -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=
Expand Down
42 changes: 33 additions & 9 deletions pkg/controller/controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,14 +21,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",
Expand All @@ -55,36 +58,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())
Expand Down Expand Up @@ -137,6 +149,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)
}
}
}

Expand Down Expand Up @@ -180,14 +202,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,
},
Expand All @@ -197,6 +220,7 @@ func createPV(capacityGB int, pvcName, pvcNamespace string, pvcUID types.UID) *v
VolumeHandle: "foo",
},
},
VolumeMode: volumeMode,
},
}
if len(pvcName) > 0 {
Expand Down
12 changes: 7 additions & 5 deletions pkg/csi/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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 {
Expand Down
15 changes: 13 additions & 2 deletions pkg/csi/mock_client.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,10 @@
package csi

import "context"
import (
"context"

"github.com/container-storage-interface/spec/lib/go/csi"
)

func NewMockClient(
name string,
Expand All @@ -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) {
Expand All @@ -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
Expand Down
69 changes: 68 additions & 1 deletion pkg/resizer/csi_resizer.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What happens if you call this against a driver that doesn't have this new field?

Copy link
Contributor Author

@gnufied gnufied Apr 1, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the server is compiled (i.e the driver) with a version of CSI which does not have this field then the driver will not simply see it.

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) {
gnufied marked this conversation as resolved.
Show resolved Hide resolved
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()
Expand Down
Loading