Verify claimref associated with PVs before resizing

.prow.sh

@@ -0,0 +1,4 @@
+#! /bin/bash
+
+. release-tools/prow.sh
+main

CHANGELOG-0.1.0.md → CHANGELOG-0.1.md

@@ -1,10 +1,12 @@
-# v0.1.0
-
 ## Support Status
 
 Alpha
 
-## Initial release Changelog
+## v0.1.1
+
+* [#61](https://github.com/kubernetes-csi/external-resizer/pull/61) Verify claimref associated with PV before doing volume expansion.
+
+## v0.1.0
 
 * [#1](https://github.com/kubernetes-csi/external-resizer/pull/1) Add a external resize controller which monitors Persistent volume claims and performs CSI `ControllerExpandVolume` as needed.
 * [#26](https://github.com/kubernetes-csi/external-resizer/pull/26) If plugin does not implement `ControllerExpandVolume` it performs a no-op expansion and updates PV object.

pkg/controller/controller.go

@@ -254,6 +254,11 @@ func (ctrl *resizeController) pvNeedResize(pvc *v1.PersistentVolumeClaim, pv *v1
 		return false
 	}
 
+	if (pv.Spec.ClaimRef == nil) || (pvc.Namespace != pv.Spec.ClaimRef.Namespace) || (pvc.UID != pv.Spec.ClaimRef.UID) {
+		klog.V(4).Infof("persistent volume is not bound to PVC being updated: %s", util.PVCKey(pvc))
+		return false
+	}
+
 	pvSize := pv.Spec.Capacity[v1.ResourceStorage]
 	requestSize := pvc.Spec.Resources.Requests[v1.ResourceStorage]
 	if pvSize.Cmp(requestSize) >= 0 {

pkg/controller/controller_test.go

@@ -0,0 +1,212 @@
+package controller
+
+import (
+	"context"
+	"fmt"
+	"testing"
+	"time"
+
+	"github.com/kubernetes-csi/external-resizer/pkg/csi"
+	"github.com/kubernetes-csi/external-resizer/pkg/resizer"
+
+	"k8s.io/api/core/v1"
+	"k8s.io/apimachinery/pkg/api/resource"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/runtime"
+	"k8s.io/apimachinery/pkg/types"
+	"k8s.io/client-go/informers"
+	"k8s.io/client-go/kubernetes"
+	"k8s.io/client-go/kubernetes/fake"
+)
+
+func TestController(t *testing.T) {
+	for _, test := range []struct {
+		Name string
+		PVC  *v1.PersistentVolumeClaim
+		PV   *v1.PersistentVolume
+
+		CreateObjects bool
+		NodeResize    bool
+		CallCSIExpand bool
+	}{
+		{
+			Name:          "Invalid key",
+			PVC:           invalidPVC(),
+			CallCSIExpand: false,
+		},
+		{
+			Name:          "PVC not found",
+			PVC:           createPVC(1, 1),
+			CallCSIExpand: false,
+		},
+		{
+			Name:          "PVC doesn't need resize",
+			PVC:           createPVC(1, 1),
+			CreateObjects: true,
+			CallCSIExpand: false,
+		},
+		{
+			Name:          "PV not found",
+			PVC:           createPVC(2, 1),
+			CreateObjects: true,
+			CallCSIExpand: false,
+		},
+		{
+			Name:          "pv claimref does not have pvc UID",
+			PVC:           createPVC(2, 1),
+			PV:            createPV(1, "testPVC" /*pvcName*/, "test" /*pvcNamespace*/, "foobaz" /*pvcUID*/),
+			CallCSIExpand: false,
+		},
+		{
+			Name:          "pv claimref does not have PVC namespace",
+			PVC:           createPVC(2, 1),
+			PV:            createPV(1, "testPVC" /*pvcName*/, "test1" /*pvcNamespace*/, "foobar" /*pvcUID*/),
+			CallCSIExpand: false,
+		},
+		{
+			Name:          "pv claimref is nil",
+			PVC:           createPVC(2, 1),
+			PV:            createPV(1, "" /*pvcName*/, "test1" /*pvcNamespace*/, "foobar" /*pvcUID*/),
+			CallCSIExpand: false,
+		},
+		{
+			Name:          "Resize PVC, no FS resize",
+			PVC:           createPVC(2, 1),
+			PV:            createPV(1, "testPVC", "test", "foobar"),
+			CreateObjects: true,
+			CallCSIExpand: true,
+		},
+		{
+			Name:          "Resize PVC with FS resize",
+			PVC:           createPVC(2, 1),
+			PV:            createPV(1, "testPVC", "test", "foobar"),
+			CreateObjects: true,
+			NodeResize:    true,
+			CallCSIExpand: true,
+		},
+	} {
+		client := csi.NewMockClient(test.NodeResize, true, true)
+		driverName, _ := client.GetDriverName(context.TODO())
+
+		initialObjects := []runtime.Object{}
+		if test.CreateObjects {
+			if test.PVC != nil {
+				initialObjects = append(initialObjects, test.PVC)
+			}
+			if test.PV != nil {
+				test.PV.Spec.PersistentVolumeSource.CSI.Driver = driverName
+				initialObjects = append(initialObjects, test.PV)
+			}
+		}
+
+		kubeClient, informerFactory := fakeK8s(initialObjects)
+		pvInformer := informerFactory.Core().V1().PersistentVolumes()
+		pvcInformer := informerFactory.Core().V1().PersistentVolumeClaims()
+
+		for _, obj := range initialObjects {
+			switch obj.(type) {
+			case *v1.PersistentVolume:
+				pvInformer.Informer().GetStore().Add(obj)
+			case *v1.PersistentVolumeClaim:
+				pvcInformer.Informer().GetStore().Add(obj)
+			default:
+				t.Fatalf("Test %s: Unknown initalObject type: %+v", test.Name, obj)
+			}
+		}
+
+		csiResizer, err := resizer.NewResizerFromClient(client, 15*time.Second, kubeClient, informerFactory)
+		if err != nil {
+			t.Fatalf("Test %s: Unable to create resizer: %v", test.Name, err)
+		}
+
+		controller := NewResizeController(driverName, csiResizer, kubeClient, time.Second, informerFactory)
+		err = controller.(*resizeController).syncPVC(fmt.Sprintf("%s/%s", test.PVC.Namespace, test.PVC.Name))
+		if err != nil {
+			t.Fatalf("Test %s: Unexpected error: %v", test.Name, err)
+		}
+
+		expandCallCount := client.GetExpandCount()
+		if test.CallCSIExpand && expandCallCount == 0 {
+			t.Fatalf("for %s: expected csi expand call, no csi expand call was made", test.Name)
+		}
+
+		if !test.CallCSIExpand && expandCallCount > 0 {
+			t.Fatalf("for %s: expected no csi expand call, received csi expansion request", test.Name)
+		}
+	}
+}
+
+func invalidPVC() *v1.PersistentVolumeClaim {
+	pvc := createPVC(1, 1)
+	pvc.ObjectMeta.Name = ""
+	pvc.ObjectMeta.Namespace = ""
+
+	return pvc
+}
+
+func quantityGB(i int) resource.Quantity {
+	q := resource.NewQuantity(int64(i*1024*1024), resource.BinarySI)
+	return *q
+}
+
+func createPVC(requestGB, capacityGB int) *v1.PersistentVolumeClaim {
+	request := quantityGB(requestGB)
+	capacity := quantityGB(capacityGB)
+
+	return &v1.PersistentVolumeClaim{
+		ObjectMeta: metav1.ObjectMeta{
+			Name:      "testPVC",
+			Namespace: "test",
+			UID:       "foobar",
+		},
+		Spec: v1.PersistentVolumeClaimSpec{
+			Resources: v1.ResourceRequirements{
+				Requests: map[v1.ResourceName]resource.Quantity{
+					v1.ResourceStorage: request,
+				},
+			},
+			VolumeName: "testPV",
+		},
+		Status: v1.PersistentVolumeClaimStatus{
+			Phase: v1.ClaimBound,
+			Capacity: map[v1.ResourceName]resource.Quantity{
+				v1.ResourceStorage: capacity,
+			},
+		},
+	}
+}
+
+func createPV(capacityGB int, pvcName, pvcNamespace string, pvcUID types.UID) *v1.PersistentVolume {
+	capacity := quantityGB(capacityGB)
+
+	pv := &v1.PersistentVolume{
+		ObjectMeta: metav1.ObjectMeta{
+			Name: "testPV",
+		},
+		Spec: v1.PersistentVolumeSpec{
+			Capacity: map[v1.ResourceName]resource.Quantity{
+				v1.ResourceStorage: capacity,
+			},
+			PersistentVolumeSource: v1.PersistentVolumeSource{
+				CSI: &v1.CSIPersistentVolumeSource{
+					Driver:       "foo",
+					VolumeHandle: "foo",
+				},
+			},
+		},
+	}
+	if len(pvcName) > 0 {
+		pv.Spec.ClaimRef = &v1.ObjectReference{
+			Namespace: pvcNamespace,
+			Name:      pvcName,
+			UID:       pvcUID,
+		}
+	}
+	return pv
+}
+
+func fakeK8s(objs []runtime.Object) (kubernetes.Interface, informers.SharedInformerFactory) {
+	client := fake.NewSimpleClientset(objs...)
+	informerFactory := informers.NewSharedInformerFactory(client, 0)
+	return client, informerFactory
+}

pkg/csi/mock_client.go

@@ -10,6 +10,7 @@ func NewMockClient(
 		name:                            "mock",
 		supportsNodeResize:              supportsNodeResize,
 		supportsControllerResize:        supportsControllerResize,
+		expandCalled:                    0,
 		supportsPluginControllerService: supportsPluginControllerService,
 	}
 }
@@ -19,6 +20,8 @@ type MockClient struct {
 	supportsNodeResize              bool
 	supportsControllerResize        bool
 	supportsPluginControllerService bool
+	expandCalled                    int
+	usedSecrets                     map[string]string
 }
 
 func (c *MockClient) GetDriverName(context.Context) (string, error) {
@@ -43,5 +46,16 @@ func (c *MockClient) Expand(
 	requestBytes int64,
 	secrets map[string]string) (int64, bool, error) {
 	// TODO: Determine whether the operation succeeds or fails by parameters.
+	c.expandCalled++
+	c.usedSecrets = secrets
 	return requestBytes, c.supportsNodeResize, nil
 }
+
+func (c *MockClient) GetExpandCount() int {
+	return c.expandCalled
+}
+
+// GetSecrets returns secrets used for volume expansion
+func (c *MockClient) GetSecrets() map[string]string {
+	return c.usedSecrets
+}

pkg/resizer/csi_resizer.go

@@ -56,10 +56,10 @@ func NewResizer(
 	if err != nil {
 		return nil, err
 	}
-	return newResizer(csiClient, timeout, k8sClient, informerFactory)
+	return NewResizerFromClient(csiClient, timeout, k8sClient, informerFactory)
 }
 
-func newResizer(
+func NewResizerFromClient(
 	csiClient csi.Client,
 	timeout time.Duration,
 	k8sClient kubernetes.Interface,
@@ -101,7 +101,6 @@ func newResizer(
 		timeout: timeout,
 
 		k8sClient: k8sClient,
-		scLister:  informerFactory.Storage().V1().StorageClasses().Lister(),
 	}, nil
 }
 

pkg/resizer/csi_resizer_test.go

@@ -53,7 +53,7 @@ func TestNewResizer(t *testing.T) {
 	} {
 		client := csi.NewMockClient(c.SupportsNodeResize, c.SupportsControllerResize, c.SupportsPluginControllerService)
 		k8sClient, informerFactory := fakeK8s()
-		resizer, err := newResizer(client, 0, k8sClient, informerFactory)
+		resizer, err := NewResizerFromClient(client, 0, k8sClient, informerFactory)
 		if err != c.Error {
 			t.Errorf("Case %d: Unexpected error: wanted %v, got %v", i, c.Error, err)
 		}