Merge pull request #76 from lpabon/pick49

Bugfix: PV should use capacity bytes returned by plugin, not PVC bytes

Author: lpabon

pkg/controller/controller.go

@@ -23,6 +23,9 @@ import (
 	"strings"
 	"time"
 
+	"k8s.io/apimachinery/pkg/api/resource"
+	_ "k8s.io/apimachinery/pkg/util/json"
+
 	"github.com/golang/glog"
 
 	"github.com/kubernetes-incubator/external-storage/lib/controller"
@@ -289,7 +292,6 @@ func (p *csiProvisioner) Provision(options controller.VolumeOptions) (*v1.Persis
 		},
 		CapacityRange: &csi.CapacityRange{
 			RequiredBytes: int64(volSizeBytes),
-			LimitBytes:    int64(volSizeBytes),
 		},
 	}
 	rep := &csi.CreateVolumeResponse{}
@@ -336,6 +338,28 @@ func (p *csiProvisioner) Provision(options controller.VolumeOptions) (*v1.Persis
 	for k, v := range rep.Volume.Attributes {
 		volumeAttributes[k] = v
 	}
+	respCap := rep.GetVolume().GetCapacityBytes()
+	if respCap < volSizeBytes {
+		capErr := fmt.Errorf("created volume capacity %v less than requested capacity %v", respCap, volSizeBytes)
+		delReq := &csi.DeleteVolumeRequest{
+			VolumeId: rep.GetVolume().GetId(),
+		}
+		if options.Parameters != nil {
+			credentials, err := getCredentialsFromParameters(p.client, options.Parameters)
+			if err != nil {
+				return nil, err
+			}
+			delReq.ControllerDeleteSecrets = credentials
+		}
+		ctx, cancel := context.WithTimeout(context.Background(), p.timeout)
+		defer cancel()
+		_, err := p.csiClient.DeleteVolume(ctx, delReq)
+		if err != nil {
+			capErr = fmt.Errorf("%v. Cleanup of volume %s failed, volume is orphaned: %v", capErr, share, err)
+		}
+		return nil, capErr
+	}
+	repBytesString := fmt.Sprintf("%v", respCap)
 	pv := &v1.PersistentVolume{
 		ObjectMeta: metav1.ObjectMeta{
 			Name: share,
@@ -344,7 +368,7 @@ func (p *csiProvisioner) Provision(options controller.VolumeOptions) (*v1.Persis
 			PersistentVolumeReclaimPolicy: options.PersistentVolumeReclaimPolicy,
 			AccessModes:                   options.PVC.Spec.AccessModes,
 			Capacity: v1.ResourceList{
-				v1.ResourceName(v1.ResourceStorage): options.PVC.Spec.Resources.Requests[v1.ResourceName(v1.ResourceStorage)],
+				v1.ResourceName(v1.ResourceStorage): resource.MustParse(repBytesString),
 			},
 			// TODO wait for CSI VolumeSource API
 			PersistentVolumeSource: v1.PersistentVolumeSource{

pkg/controller/controller_test.go

@@ -18,13 +18,18 @@ package controller
 
 import (
 	"fmt"
+	"strconv"
 	"testing"
 	"time"
 
 	csi "github.com/container-storage-interface/spec/lib/go/csi/v0"
 	"github.com/golang/mock/gomock"
 	"github.com/kubernetes-csi/csi-test/driver"
+	"github.com/kubernetes-incubator/external-storage/lib/controller"
 	"google.golang.org/grpc"
+	"k8s.io/api/core/v1"
+	"k8s.io/apimachinery/pkg/api/resource"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 )
 
 const (
@@ -384,3 +389,94 @@ func TestGetDriverName(t *testing.T) {
 		}
 	}
 }
+
+func TestCreateDriverReturnsInvalidCapacityDuringProvision(t *testing.T) {
+	// Set up mocks
+	var requestedBytes int64 = 100
+	mockController, driver, identityServer, controllerServer, csiConn, err := createMockServer(t)
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer mockController.Finish()
+	defer driver.Stop()
+
+	csiProvisioner := NewCSIProvisioner(nil, driver.Address(), 5*time.Second, "test-provisioner", "test", 5, csiConn.conn)
+
+	// Requested PVC with requestedBytes storage
+	opts := controller.VolumeOptions{
+		PersistentVolumeReclaimPolicy: v1.PersistentVolumeReclaimDelete,
+		PVName:     "test-name",
+		PVC:        createFakePVC(requestedBytes),
+		Parameters: map[string]string{},
+	}
+
+	// Drivers CreateVolume response with lower capacity bytes than request
+	out := &csi.CreateVolumeResponse{
+		Volume: &csi.Volume{
+			CapacityBytes: requestedBytes - 1,
+			Id:            "test-volume-id",
+		},
+	}
+
+	// Set up Mocks
+	provisionMockServerSetupExpectations(identityServer, controllerServer)
+	controllerServer.EXPECT().CreateVolume(gomock.Any(), gomock.Any()).Return(out, nil).Times(1)
+	// Since capacity returned by driver is invalid, we expect the provision call to clean up the volume
+	controllerServer.EXPECT().DeleteVolume(gomock.Any(), &csi.DeleteVolumeRequest{
+		VolumeId: "test-volume-id",
+	}).Return(&csi.DeleteVolumeResponse{}, nil).Times(1)
+
+	// Call provision
+	_, err = csiProvisioner.Provision(opts)
+	if err == nil {
+		t.Errorf("Provision did not cause an error when one was expected")
+		return
+	}
+	t.Logf("Provision encountered an error: %v, expected: create volume capacity less than requested capacity", err)
+}
+
+func provisionMockServerSetupExpectations(identityServer *driver.MockIdentityServer, controllerServer *driver.MockControllerServer) {
+	identityServer.EXPECT().GetPluginCapabilities(gomock.Any(), gomock.Any()).Return(&csi.GetPluginCapabilitiesResponse{
+		Capabilities: []*csi.PluginCapability{
+			&csi.PluginCapability{
+				Type: &csi.PluginCapability_Service_{
+					Service: &csi.PluginCapability_Service{
+						Type: csi.PluginCapability_Service_CONTROLLER_SERVICE,
+					},
+				},
+			},
+		},
+	}, nil).Times(1)
+	controllerServer.EXPECT().ControllerGetCapabilities(gomock.Any(), gomock.Any()).Return(&csi.ControllerGetCapabilitiesResponse{
+		Capabilities: []*csi.ControllerServiceCapability{
+			&csi.ControllerServiceCapability{
+				Type: &csi.ControllerServiceCapability_Rpc{
+					Rpc: &csi.ControllerServiceCapability_RPC{
+						Type: csi.ControllerServiceCapability_RPC_CREATE_DELETE_VOLUME,
+					},
+				},
+			},
+		},
+	}, nil).Times(1)
+	identityServer.EXPECT().GetPluginInfo(gomock.Any(), gomock.Any()).Return(&csi.GetPluginInfoResponse{
+		Name:          "test-driver",
+		VendorVersion: "test-vendor",
+	}, nil).Times(1)
+}
+
+// Minimal PVC required for tests to function
+func createFakePVC(requestBytes int64) *v1.PersistentVolumeClaim {
+	return &v1.PersistentVolumeClaim{
+		ObjectMeta: metav1.ObjectMeta{
+			UID: "testid",
+		},
+		Spec: v1.PersistentVolumeClaimSpec{
+			Selector: nil, // Provisioner doesn't support selector
+			Resources: v1.ResourceRequirements{
+				Requests: v1.ResourceList{
+					v1.ResourceName(v1.ResourceStorage): resource.MustParse(strconv.FormatInt(requestBytes, 10)),
+				},
+			},
+		},
+	}
+}