From 70a38b269a5692f76d4ee1bb7bd49733f2652008 Mon Sep 17 00:00:00 2001 From: Hemant Kumar Date: Mon, 7 Oct 2019 10:43:00 -0400 Subject: [PATCH] Verify claimref associated with PVs before resizing --- pkg/controller/controller.go | 5 +++ pkg/controller/controller_test.go | 60 +++++++++++++++++++++++++------ pkg/csi/mock_client.go | 7 ++++ 3 files changed, 61 insertions(+), 11 deletions(-) diff --git a/pkg/controller/controller.go b/pkg/controller/controller.go index eac9f4ba0..934e63271 100644 --- a/pkg/controller/controller.go +++ b/pkg/controller/controller.go @@ -283,6 +283,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 { diff --git a/pkg/controller/controller_test.go b/pkg/controller/controller_test.go index 3b698c996..14b147573 100644 --- a/pkg/controller/controller_test.go +++ b/pkg/controller/controller_test.go @@ -13,6 +13,7 @@ import ( "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" @@ -26,43 +27,62 @@ func TestController(t *testing.T) { CreateObjects bool NodeResize bool + CallCSIExpand bool }{ { - Name: "Invalid key", - PVC: invalidPVC(), + Name: "Invalid key", + PVC: invalidPVC(), + CallCSIExpand: false, }, { - Name: "PVC not found", - PVC: createPVC(1, 1), + 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 doesn't need resize", + Name: "pv claimref does not have pvc UID", PVC: createPVC(2, 1), - PV: createPV(2), - CreateObjects: true, + 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), + PV: createPV(1, "testPVC", "test", "foobar"), CreateObjects: true, + CallCSIExpand: true, }, { Name: "Resize PVC with FS resize", PVC: createPVC(2, 1), - PV: createPV(1), + PV: createPV(1, "testPVC", "test", "foobar"), CreateObjects: true, NodeResize: true, + CallCSIExpand: true, }, } { client := csi.NewMockClient("mock", test.NodeResize, true, true) @@ -104,6 +124,15 @@ func TestController(t *testing.T) { 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) + } } } @@ -128,6 +157,7 @@ func createPVC(requestGB, capacityGB int) *v1.PersistentVolumeClaim { ObjectMeta: metav1.ObjectMeta{ Name: "testPVC", Namespace: "test", + UID: "foobar", }, Spec: v1.PersistentVolumeClaimSpec{ Resources: v1.ResourceRequirements{ @@ -146,10 +176,10 @@ func createPVC(requestGB, capacityGB int) *v1.PersistentVolumeClaim { } } -func createPV(capacityGB int) *v1.PersistentVolume { +func createPV(capacityGB int, pvcName, pvcNamespace string, pvcUID types.UID) *v1.PersistentVolume { capacity := quantityGB(capacityGB) - return &v1.PersistentVolume{ + pv := &v1.PersistentVolume{ ObjectMeta: metav1.ObjectMeta{ Name: "testPV", }, @@ -165,6 +195,14 @@ func createPV(capacityGB int) *v1.PersistentVolume { }, }, } + 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) { diff --git a/pkg/csi/mock_client.go b/pkg/csi/mock_client.go index 231bed78d..85ce03a36 100644 --- a/pkg/csi/mock_client.go +++ b/pkg/csi/mock_client.go @@ -11,6 +11,7 @@ func NewMockClient( name: name, supportsNodeResize: supportsNodeResize, supportsControllerResize: supportsControllerResize, + expandCalled: 0, supportsPluginControllerService: supportsPluginControllerService, } } @@ -20,6 +21,7 @@ type MockClient struct { supportsNodeResize bool supportsControllerResize bool supportsPluginControllerService bool + expandCalled int usedSecrets map[string]string } @@ -45,10 +47,15 @@ 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