Merge pull request #46 from davidz627/feature/breakProvision

BREAKING CHANGE: Refactor volume options to provision options to stop using redundant internal struct fields and ease migration translation

controller/controller.go

@@ -1236,12 +1236,6 @@ func (ctrl *ProvisionController) provisionClaimOperation(claim *v1.PersistentVol
 		return ProvisioningFinished, nil
 	}
 
-	reclaimPolicy := v1.PersistentVolumeReclaimDelete
-	if ctrl.kubeVersion.AtLeast(utilversion.MustParseSemantic("v1.8.0")) {
-		reclaimPolicy = *class.ReclaimPolicy
-	}
-	mountOptions := class.MountOptions
-
 	var selectedNode *v1.Node
 	if ctrl.kubeVersion.AtLeast(utilversion.MustParseSemantic("v1.11.0")) {
 		// Get SelectedNode
@@ -1255,14 +1249,11 @@ func (ctrl *ProvisionController) provisionClaimOperation(claim *v1.PersistentVol
 		}
 	}
 
-	options := VolumeOptions{
-		PersistentVolumeReclaimPolicy: reclaimPolicy,
-		PVName:                        pvName,
-		PVC:                           claim,
-		MountOptions:                  mountOptions,
-		Parameters:                    class.Parameters,
-		SelectedNode:                  selectedNode,
-		AllowedTopologies:             class.AllowedTopologies,
+	options := ProvisionOptions{
+		StorageClass: class,
+		PVName:       pvName,
+		PVC:          claim,
+		SelectedNode: selectedNode,
 	}
 
 	ctrl.eventRecorder.Event(claim, v1.EventTypeNormal, "Provisioning", fmt.Sprintf("External provisioner is provisioning volume for claim %q", claimToClaimKey(claim)))

controller/controller_test.go

@@ -1187,10 +1187,12 @@ func newProvisionedVolumeWithSpecifiedReclaimPolicy(storageClass *storage.Storag
 
 func constructProvisionedVolumeWithoutStorageClassInfo(claim *v1.PersistentVolumeClaim, reclaimPolicy v1.PersistentVolumeReclaimPolicy) *v1.PersistentVolume {
 	// pv.Spec MUST be set to match requirements in claim.Spec, especially access mode and PV size. The provisioned volume size MUST NOT be smaller than size requested in the claim, however it MAY be larger.
-	options := VolumeOptions{
-		PersistentVolumeReclaimPolicy: reclaimPolicy,
-		PVName:                        "pvc-" + string(claim.ObjectMeta.UID),
-		PVC:                           claim,
+	options := ProvisionOptions{
+		StorageClass: &storage.StorageClass{
+			ReclaimPolicy: &reclaimPolicy,
+		},
+		PVName: "pvc-" + string(claim.ObjectMeta.UID),
+		PVC:    claim,
 	}
 	volume, _ := newTestProvisioner().Provision(options)
 
@@ -1262,10 +1264,10 @@ func (p *testBlockProvisioner) SupportsBlock() bool {
 	return p.answer
 }
 
-func (p *testProvisioner) Provision(options VolumeOptions) (*v1.PersistentVolume, error) {
+func (p *testProvisioner) Provision(options ProvisionOptions) (*v1.PersistentVolume, error) {
 	p.provisionCalls <- provisionParams{
 		selectedNode:      options.SelectedNode,
-		allowedTopologies: options.AllowedTopologies,
+		allowedTopologies: options.StorageClass.AllowedTopologies,
 	}
 
 	// Sleep to simulate work done by Provision...for long enough that
@@ -1280,7 +1282,7 @@ func (p *testProvisioner) Provision(options VolumeOptions) (*v1.PersistentVolume
 			Name: options.PVName,
 		},
 		Spec: v1.PersistentVolumeSpec{
-			PersistentVolumeReclaimPolicy: options.PersistentVolumeReclaimPolicy,
+			PersistentVolumeReclaimPolicy: *options.StorageClass.ReclaimPolicy,
 			AccessModes:                   options.PVC.Spec.AccessModes,
 			Capacity: v1.ResourceList{
 				v1.ResourceName(v1.ResourceStorage): options.PVC.Spec.Resources.Requests[v1.ResourceName(v1.ResourceStorage)],
@@ -1311,7 +1313,7 @@ type badTestProvisioner struct {
 
 var _ Provisioner = &badTestProvisioner{}
 
-func (p *badTestProvisioner) Provision(options VolumeOptions) (*v1.PersistentVolume, error) {
+func (p *badTestProvisioner) Provision(options ProvisionOptions) (*v1.PersistentVolume, error) {
 	return nil, errors.New("fake error")
 }
 
@@ -1328,7 +1330,7 @@ type ignoredProvisioner struct {
 
 var _ Provisioner = &ignoredProvisioner{}
 
-func (i *ignoredProvisioner) Provision(options VolumeOptions) (*v1.PersistentVolume, error) {
+func (i *ignoredProvisioner) Provision(options ProvisionOptions) (*v1.PersistentVolume, error) {
 	if options.PVC.Name == "claim-2" {
 		return nil, &IgnoredError{"Ignored"}
 	}
@@ -1359,7 +1361,7 @@ type extProvisioner struct {
 var _ Provisioner = &extProvisioner{}
 var _ ProvisionerExt = &extProvisioner{}
 
-func (m *extProvisioner) Provision(VolumeOptions) (*v1.PersistentVolume, error) {
+func (m *extProvisioner) Provision(ProvisionOptions) (*v1.PersistentVolume, error) {
 	return nil, fmt.Errorf("Not implemented")
 }
 
@@ -1368,7 +1370,7 @@ func (m *extProvisioner) Delete(pv *v1.PersistentVolume) error {
 
 }
 
-func (m *extProvisioner) ProvisionExt(options VolumeOptions) (*v1.PersistentVolume, ProvisioningState, error) {
+func (m *extProvisioner) ProvisionExt(options ProvisionOptions) (*v1.PersistentVolume, ProvisioningState, error) {
 	if m.pvName != options.PVName {
 		m.t.Errorf("Invalid provision call, expected name %q, got %q", m.pvName, options.PVName)
 		return nil, ProvisioningFinished, fmt.Errorf("Invalid provision call, expected name %q, got %q", m.pvName, options.PVName)
@@ -1381,7 +1383,7 @@ func (m *extProvisioner) ProvisionExt(options VolumeOptions) (*v1.PersistentVolu
 				Name: options.PVName,
 			},
 			Spec: v1.PersistentVolumeSpec{
-				PersistentVolumeReclaimPolicy: options.PersistentVolumeReclaimPolicy,
+				PersistentVolumeReclaimPolicy: *options.StorageClass.ReclaimPolicy,
 				AccessModes:                   options.PVC.Spec.AccessModes,
 				Capacity: v1.ResourceList{
 					v1.ResourceName(v1.ResourceStorage): options.PVC.Spec.Resources.Requests[v1.ResourceName(v1.ResourceStorage)],

controller/volume.go

@@ -20,6 +20,7 @@ import (
 	"fmt"
 
 	"k8s.io/api/core/v1"
+	storageapis "k8s.io/api/storage/v1"
 )
 
 // Provisioner is an interface that creates templates for PersistentVolumes
@@ -29,7 +30,7 @@ import (
 type Provisioner interface {
 	// Provision creates a volume i.e. the storage asset and returns a PV object
 	// for the volume
-	Provision(VolumeOptions) (*v1.PersistentVolume, error)
+	Provision(ProvisionOptions) (*v1.PersistentVolume, error)
 	// Delete removes the storage asset that was created by Provision backing the
 	// given PV. Does not delete the PV object itself.
 	//
@@ -73,7 +74,7 @@ type ProvisionerExt interface {
 	// provisioning the volume. The provisioner must return either final error (with
 	// ProvisioningFinished) or success eventually, otherwise the controller will try
 	// forever (unless FailedProvisionThreshold is set).
-	ProvisionExt(options VolumeOptions) (*v1.PersistentVolume, ProvisioningState, error)
+	ProvisionExt(options ProvisionOptions) (*v1.PersistentVolume, ProvisioningState, error)
 }
 
 // ProvisioningState is state of volume provisioning. It tells the controller if
@@ -111,28 +112,22 @@ func (e *IgnoredError) Error() string {
 	return fmt.Sprintf("ignored because %s", e.Reason)
 }
 
-// VolumeOptions contains option information about a volume
-// https://github.com/kubernetes/kubernetes/blob/release-1.4/pkg/volume/plugins.go
-type VolumeOptions struct {
-	// Reclamation policy for a persistent volume
-	PersistentVolumeReclaimPolicy v1.PersistentVolumeReclaimPolicy
+// ProvisionOptions contains all information required to provision a volume
+type ProvisionOptions struct {
+	// StorageClass is a reference to the storage class that is used for
+	// provisioning for this volume
+	StorageClass *storageapis.StorageClass
+
 	// PV.Name of the appropriate PersistentVolume. Used to generate cloud
 	// volume name.
 	PVName string
 
-	// PV mount options. Not validated - mount of the PVs will simply fail if one is invalid.
-	MountOptions []string
-
 	// PVC is reference to the claim that lead to provisioning of a new PV.
 	// Provisioners *must* create a PV that would be matched by this PVC,
 	// i.e. with required capacity, accessMode, labels matching PVC.Selector and
 	// so on.
 	PVC *v1.PersistentVolumeClaim
-	// Volume provisioning parameters from StorageClass
-	Parameters map[string]string
 
 	// Node selected by the scheduler for the volume.
 	SelectedNode *v1.Node
-	// Topology constraint parameter from StorageClass
-	AllowedTopologies []v1.TopologySelectorTerm
 }

examples/hostpath-provisioner/hostpath-provisioner.go

@@ -61,7 +61,7 @@ func NewHostPathProvisioner() controller.Provisioner {
 var _ controller.Provisioner = &hostPathProvisioner{}
 
 // Provision creates a storage asset and returns a PV object representing it.
-func (p *hostPathProvisioner) Provision(options controller.VolumeOptions) (*v1.PersistentVolume, error) {
+func (p *hostPathProvisioner) Provision(options controller.ProvisionOptions) (*v1.PersistentVolume, error) {
 	path := path.Join(p.pvDir, options.PVName)
 
 	if err := os.MkdirAll(path, 0777); err != nil {
@@ -76,7 +76,7 @@ func (p *hostPathProvisioner) Provision(options controller.VolumeOptions) (*v1.P
 			},
 		},
 		Spec: v1.PersistentVolumeSpec{
-			PersistentVolumeReclaimPolicy: options.PersistentVolumeReclaimPolicy,
+			PersistentVolumeReclaimPolicy: *options.StorageClass.ReclaimPolicy,
 			AccessModes:                   options.PVC.Spec.AccessModes,
 			Capacity: v1.ResourceList{
 				v1.ResourceName(v1.ResourceStorage): options.PVC.Spec.Resources.Requests[v1.ResourceName(v1.ResourceStorage)],

gidallocator/gidallocator.go

@@ -63,11 +63,11 @@ func New(client kubernetes.Interface) Allocator {
 	}
 }
 
-// AllocateNext allocates the next available GID for the given VolumeOptions
+// AllocateNext allocates the next available GID for the given ProvisionOptions
 // (claim's options for a volume it wants) from the appropriate GID table.
-func (a *Allocator) AllocateNext(options controller.VolumeOptions) (int, error) {
+func (a *Allocator) AllocateNext(options controller.ProvisionOptions) (int, error) {
 	class := util.GetPersistentVolumeClaimClass(options.PVC)
-	gidMin, gidMax, err := parseClassParameters(options.Parameters)
+	gidMin, gidMax, err := parseClassParameters(options.StorageClass.Parameters)
 	if err != nil {
 		return 0, err
 	}