Skip to content

Commit

Permalink
Merge pull request #34611 from jsafrane/provision-pvc
Browse files Browse the repository at this point in the history 
Automatic merge from submit-queue

Pass whole PVC to provisioner plugins

Gluster provisioner is interested in namespace of PVCs that are being provisioned and I don't want to add at as a new field in `volume.VolumeOptions` - it would contain almost whole PVC.

Let's rework `VolumeOptions` and pass direct reference to PVC there instead of some "interesting" fields and let the provisioner to pick information it is interested in.

There was lot of refactoring in volume plugins to apply this change (too many plugins), however the logic is simple and it's all the same in all plugins.

@rootfs @humblec
  • Loading branch information
Kubernetes Submit Queue committed Oct 15, 2016
2 parents cfba438 + 101602a commit 4e393fa
Show file tree
Hide file tree
Showing 28 changed files with 135 additions and 133 deletions.
6 changes: 4 additions & 2 deletions pkg/controller/volume/persistentvolume/framework_test.go
View file
Expand Up @@ -1148,15 +1148,17 @@ func (plugin *mockVolumePlugin) Provision() (*api.PersistentVolume, error) {
}
if call.ret == nil {
// Create a fake PV with known GCE volume (to match expected volume)
capacity := plugin.provisionOptions.PVC.Spec.Resources.Requests[api.ResourceName(api.ResourceStorage)]
accessModes := plugin.provisionOptions.PVC.Spec.AccessModes
pv = &api.PersistentVolume{
ObjectMeta: api.ObjectMeta{
Name: plugin.provisionOptions.PVName,
},
Spec: api.PersistentVolumeSpec{
Capacity: api.ResourceList{
api.ResourceName(api.ResourceStorage): plugin.provisionOptions.Capacity,
api.ResourceName(api.ResourceStorage): capacity,
},
AccessModes: plugin.provisionOptions.AccessModes,
AccessModes: accessModes,
PersistentVolumeReclaimPolicy: plugin.provisionOptions.PersistentVolumeReclaimPolicy,
PersistentVolumeSource: api.PersistentVolumeSource{
GCEPersistentDisk: &api.GCEPersistentDiskVolumeSource{},
Expand Down
5 changes: 1 addition & 4 deletions pkg/controller/volume/persistentvolume/pv_controller.go
View file
Expand Up @@ -1260,15 +1260,12 @@ func (ctrl *PersistentVolumeController) provisionClaimOperation(claimObj interfa
tags[cloudVolumeCreatedForVolumeNameTag] = pvName

options := vol.VolumeOptions{
Capacity: claim.Spec.Resources.Requests[api.ResourceName(api.ResourceStorage)],
AccessModes: claim.Spec.AccessModes,
PersistentVolumeReclaimPolicy: api.PersistentVolumeReclaimDelete,
CloudTags: &tags,
ClusterName: ctrl.clusterName,
PVName: pvName,
PVCName: claim.Name,
PVC: claim,
Parameters: storageClass.Parameters,
Selector: claim.Spec.Selector,
}

// Provision the volume
Expand Down
9 changes: 5 additions & 4 deletions pkg/volume/aws_ebs/aws_ebs.go
View file
Expand Up @@ -160,9 +160,6 @@ func (plugin *awsElasticBlockStorePlugin) newDeleterInternal(spec *volume.Spec,
}

func (plugin *awsElasticBlockStorePlugin) NewProvisioner(options volume.VolumeOptions) (volume.Provisioner, error) {
if len(options.AccessModes) == 0 {
options.AccessModes = plugin.GetAccessModes()
}
return plugin.newProvisionerInternal(options, &AWSDiskUtil{})
}

Expand Down Expand Up @@ -429,7 +426,7 @@ func (c *awsElasticBlockStoreProvisioner) Provision() (*api.PersistentVolume, er
},
Spec: api.PersistentVolumeSpec{
PersistentVolumeReclaimPolicy: c.options.PersistentVolumeReclaimPolicy,
AccessModes: c.options.AccessModes,
AccessModes: c.options.PVC.Spec.AccessModes,
Capacity: api.ResourceList{
api.ResourceName(api.ResourceStorage): resource.MustParse(fmt.Sprintf("%dGi", sizeGB)),
},
Expand All @@ -444,6 +441,10 @@ func (c *awsElasticBlockStoreProvisioner) Provision() (*api.PersistentVolume, er
},
}

if len(c.options.PVC.Spec.AccessModes) == 0 {
pv.Spec.AccessModes = c.plugin.GetAccessModes()
}

if len(labels) != 0 {
if pv.Labels == nil {
pv.Labels = make(map[string]string)
Expand Down
9 changes: 2 additions & 7 deletions pkg/volume/aws_ebs/aws_ebs_test.go
View file
Expand Up @@ -23,7 +23,6 @@ import (
"testing"

"k8s.io/kubernetes/pkg/api"
"k8s.io/kubernetes/pkg/api/resource"
"k8s.io/kubernetes/pkg/client/clientset_generated/internalclientset/fake"
"k8s.io/kubernetes/pkg/types"
"k8s.io/kubernetes/pkg/util/mount"
Expand Down Expand Up @@ -180,12 +179,8 @@ func TestPlugin(t *testing.T) {
}

// Test Provisioner
cap := resource.MustParse("100Mi")
options := volume.VolumeOptions{
Capacity: cap,
AccessModes: []api.PersistentVolumeAccessMode{
api.ReadWriteOnce,
},
PVC: volumetest.CreateTestPVC("100Mi", []api.PersistentVolumeAccessMode{api.ReadWriteOnce}),
PersistentVolumeReclaimPolicy: api.PersistentVolumeReclaimDelete,
}
provisioner, err := plug.(*awsElasticBlockStorePlugin).newProvisionerInternal(options, &fakePDManager{})
Expand All @@ -197,7 +192,7 @@ func TestPlugin(t *testing.T) {
if persistentSpec.Spec.PersistentVolumeSource.AWSElasticBlockStore.VolumeID != "test-aws-volume-name" {
t.Errorf("Provision() returned unexpected volume ID: %s", persistentSpec.Spec.PersistentVolumeSource.AWSElasticBlockStore.VolumeID)
}
cap = persistentSpec.Spec.Capacity[api.ResourceStorage]
cap := persistentSpec.Spec.Capacity[api.ResourceStorage]
size := cap.Value()
if size != 100*1024*1024*1024 {
t.Errorf("Provision() returned unexpected volume size: %v", size)
Expand Down
10 changes: 6 additions & 4 deletions pkg/volume/aws_ebs/aws_util.go
View file
Expand Up @@ -23,6 +23,7 @@ import (
"time"

"github.com/golang/glog"
"k8s.io/kubernetes/pkg/api"
"k8s.io/kubernetes/pkg/cloudprovider"
"k8s.io/kubernetes/pkg/cloudprovider/providers/aws"
"k8s.io/kubernetes/pkg/volume"
Expand Down Expand Up @@ -79,13 +80,14 @@ func (util *AWSDiskUtil) CreateVolume(c *awsElasticBlockStoreProvisioner) (strin
}
tags["Name"] = volume.GenerateVolumeName(c.options.ClusterName, c.options.PVName, 255) // AWS tags can have 255 characters

requestBytes := c.options.Capacity.Value()
capacity := c.options.PVC.Spec.Resources.Requests[api.ResourceName(api.ResourceStorage)]
requestBytes := capacity.Value()
// AWS works with gigabytes, convert to GiB with rounding up
requestGB := int(volume.RoundUpSize(requestBytes, 1024*1024*1024))
volumeOptions := &aws.VolumeOptions{
CapacityGB: requestGB,
Tags: tags,
PVCName: c.options.PVCName,
PVCName: c.options.PVC.Name,
}
// Apply Parameters (case-insensitive). We leave validation of
// the values to the cloud provider.
Expand All @@ -112,8 +114,8 @@ func (util *AWSDiskUtil) CreateVolume(c *awsElasticBlockStoreProvisioner) (strin
}
}

// TODO: implement c.options.ProvisionerSelector parsing
if c.options.Selector != nil {
// TODO: implement PVC.Selector parsing
if c.options.PVC.Spec.Selector != nil {
return "", 0, nil, fmt.Errorf("claim.Spec.Selector is not supported for dynamic provisioning on AWS")
}

Expand Down
9 changes: 5 additions & 4 deletions pkg/volume/cinder/cinder.go
View file
Expand Up @@ -161,9 +161,6 @@ func (plugin *cinderPlugin) newDeleterInternal(spec *volume.Spec, manager cdMana
}

func (plugin *cinderPlugin) NewProvisioner(options volume.VolumeOptions) (volume.Provisioner, error) {
if len(options.AccessModes) == 0 {
options.AccessModes = plugin.GetAccessModes()
}
return plugin.newProvisionerInternal(options, &CinderDiskUtil{})
}

Expand Down Expand Up @@ -474,7 +471,7 @@ func (c *cinderVolumeProvisioner) Provision() (*api.PersistentVolume, error) {
},
Spec: api.PersistentVolumeSpec{
PersistentVolumeReclaimPolicy: c.options.PersistentVolumeReclaimPolicy,
AccessModes: c.options.AccessModes,
AccessModes: c.options.PVC.Spec.AccessModes,
Capacity: api.ResourceList{
api.ResourceName(api.ResourceStorage): resource.MustParse(fmt.Sprintf("%dGi", sizeGB)),
},
Expand All @@ -487,6 +484,10 @@ func (c *cinderVolumeProvisioner) Provision() (*api.PersistentVolume, error) {
},
},
}
if len(c.options.PVC.Spec.AccessModes) == 0 {
pv.Spec.AccessModes = c.plugin.GetAccessModes()
}

return pv, nil
}

Expand Down
9 changes: 2 additions & 7 deletions pkg/volume/cinder/cinder_test.go
View file
Expand Up @@ -24,7 +24,6 @@ import (
"time"

"k8s.io/kubernetes/pkg/api"
"k8s.io/kubernetes/pkg/api/resource"
"k8s.io/kubernetes/pkg/types"
"k8s.io/kubernetes/pkg/util/mount"
utiltesting "k8s.io/kubernetes/pkg/util/testing"
Expand Down Expand Up @@ -199,12 +198,8 @@ func TestPlugin(t *testing.T) {
}

// Test Provisioner
cap := resource.MustParse("100Mi")
options := volume.VolumeOptions{
Capacity: cap,
AccessModes: []api.PersistentVolumeAccessMode{
api.ReadWriteOnce,
},
PVC: volumetest.CreateTestPVC("100Mi", []api.PersistentVolumeAccessMode{api.ReadWriteOnce}),
PersistentVolumeReclaimPolicy: api.PersistentVolumeReclaimDelete,
}
provisioner, err := plug.(*cinderPlugin).newProvisionerInternal(options, &fakePDManager{0})
Expand All @@ -216,7 +211,7 @@ func TestPlugin(t *testing.T) {
if persistentSpec.Spec.PersistentVolumeSource.Cinder.VolumeID != "test-volume-name" {
t.Errorf("Provision() returned unexpected volume ID: %s", persistentSpec.Spec.PersistentVolumeSource.Cinder.VolumeID)
}
cap = persistentSpec.Spec.Capacity[api.ResourceStorage]
cap := persistentSpec.Spec.Capacity[api.ResourceStorage]
size := cap.Value()
if size != 1024*1024*1024 {
t.Errorf("Provision() returned unexpected volume size: %v", size)
Expand Down
8 changes: 5 additions & 3 deletions pkg/volume/cinder/cinder_util.go
View file
Expand Up @@ -24,6 +24,7 @@ import (
"time"

"github.com/golang/glog"
"k8s.io/kubernetes/pkg/api"
"k8s.io/kubernetes/pkg/util/exec"
"k8s.io/kubernetes/pkg/volume"
)
Expand Down Expand Up @@ -139,7 +140,8 @@ func (util *CinderDiskUtil) CreateVolume(c *cinderVolumeProvisioner) (volumeID s
return "", 0, err
}

volSizeBytes := c.options.Capacity.Value()
capacity := c.options.PVC.Spec.Resources.Requests[api.ResourceName(api.ResourceStorage)]
volSizeBytes := capacity.Value()
// Cinder works with gigabytes, convert to GiB with rounding up
volSizeGB := int(volume.RoundUpSize(volSizeBytes, 1024*1024*1024))
name := volume.GenerateVolumeName(c.options.ClusterName, c.options.PVName, 255) // Cinder volume name can have up to 255 characters
Expand All @@ -157,8 +159,8 @@ func (util *CinderDiskUtil) CreateVolume(c *cinderVolumeProvisioner) (volumeID s
return "", 0, fmt.Errorf("invalid option %q for volume plugin %s", k, c.plugin.GetPluginName())
}
}
// TODO: implement c.options.ProvisionerSelector parsing
if c.options.Selector != nil {
// TODO: implement PVC.Selector parsing
if c.options.PVC.Spec.Selector != nil {
return "", 0, fmt.Errorf("claim.Spec.Selector is not supported for dynamic provisioning on Cinder")
}

Expand Down
3 changes: 0 additions & 3 deletions pkg/volume/flocker/flocker.go
View file
Expand Up @@ -452,9 +452,6 @@ func (plugin *flockerPlugin) newDeleterInternal(spec *volume.Spec, manager volum
}

func (plugin *flockerPlugin) NewProvisioner(options volume.VolumeOptions) (volume.Provisioner, error) {
if len(options.AccessModes) == 0 {
options.AccessModes = plugin.GetAccessModes()
}
return plugin.newProvisionerInternal(options, &FlockerUtil{})
}

Expand Down
6 changes: 4 additions & 2 deletions pkg/volume/flocker/flocker_util.go
View file
Expand Up @@ -20,6 +20,7 @@ import (
"fmt"
"time"

"k8s.io/kubernetes/pkg/api"
"k8s.io/kubernetes/pkg/util/rand"
"k8s.io/kubernetes/pkg/volume"

Expand Down Expand Up @@ -70,14 +71,15 @@ func (util *FlockerUtil) CreateVolume(c *flockerVolumeProvisioner) (datasetUUID
node := nodes[rand.Intn(len(nodes))]
glog.V(2).Infof("selected flocker node with UUID '%s' to provision dataset", node.UUID)

requestBytes := c.options.Capacity.Value()
capacity := c.options.PVC.Spec.Resources.Requests[api.ResourceName(api.ResourceStorage)]
requestBytes := capacity.Value()
volumeSizeGB = int(volume.RoundUpSize(requestBytes, 1024*1024*1024))

createOptions := &flockerApi.CreateDatasetOptions{
MaximumSize: requestBytes,
Metadata: map[string]string{
"type": "k8s-dynamic-prov",
"pvc": c.options.PVCName,
"pvc": c.options.PVC.Name,
},
Primary: node.UUID,
}
Expand Down
8 changes: 3 additions & 5 deletions pkg/volume/flocker/flocker_util_test.go
View file
Expand Up @@ -21,8 +21,8 @@ import (
"testing"

"k8s.io/kubernetes/pkg/api"
"k8s.io/kubernetes/pkg/api/resource"
"k8s.io/kubernetes/pkg/volume"
volumetest "k8s.io/kubernetes/pkg/volume/testing"

"github.com/stretchr/testify/assert"
)
Expand All @@ -31,11 +31,9 @@ func TestFlockerUtil_CreateVolume(t *testing.T) {
assert := assert.New(t)

// test CreateVolume happy path
pvc := volumetest.CreateTestPVC("3Gi", []api.PersistentVolumeAccessMode{api.ReadWriteOnce})
options := volume.VolumeOptions{
Capacity: resource.MustParse("3Gi"),
AccessModes: []api.PersistentVolumeAccessMode{
api.ReadWriteOnce,
},
PVC: pvc,
PersistentVolumeReclaimPolicy: api.PersistentVolumeReclaimDelete,
}

Expand Down
7 changes: 5 additions & 2 deletions pkg/volume/flocker/flocker_volume.go
View file
Expand Up @@ -58,7 +58,7 @@ func (c *flockerVolumeProvisioner) Provision() (*api.PersistentVolume, error) {
return nil, fmt.Errorf("Provisioning failed: Specified at least one unsupported parameter")
}

if c.options.Selector != nil {
if c.options.PVC.Spec.Selector != nil {
return nil, fmt.Errorf("Provisioning failed: Specified unsupported selector")
}

Expand All @@ -77,7 +77,7 @@ func (c *flockerVolumeProvisioner) Provision() (*api.PersistentVolume, error) {
},
Spec: api.PersistentVolumeSpec{
PersistentVolumeReclaimPolicy: c.options.PersistentVolumeReclaimPolicy,
AccessModes: c.options.AccessModes,
AccessModes: c.options.PVC.Spec.AccessModes,
Capacity: api.ResourceList{
api.ResourceName(api.ResourceStorage): resource.MustParse(fmt.Sprintf("%dGi", sizeGB)),
},
Expand All @@ -88,6 +88,9 @@ func (c *flockerVolumeProvisioner) Provision() (*api.PersistentVolume, error) {
},
},
}
if len(c.options.PVC.Spec.AccessModes) == 0 {
pv.Spec.AccessModes = c.plugin.GetAccessModes()
}

if len(labels) != 0 {
if pv.Labels == nil {
Expand Down
22 changes: 6 additions & 16 deletions pkg/volume/flocker/flocker_volume_test.go
View file
Expand Up @@ -21,7 +21,6 @@ import (
"testing"

"k8s.io/kubernetes/pkg/api"
"k8s.io/kubernetes/pkg/api/resource"
"k8s.io/kubernetes/pkg/api/unversioned"
utiltesting "k8s.io/kubernetes/pkg/util/testing"
"k8s.io/kubernetes/pkg/volume"
Expand All @@ -48,12 +47,9 @@ func newTestableProvisioner(assert *assert.Assertions, options volume.VolumeOpti
func TestProvision(t *testing.T) {
assert := assert.New(t)

cap := resource.MustParse("3Gi")
pvc := volumetest.CreateTestPVC("3Gi", []api.PersistentVolumeAccessMode{api.ReadWriteOnce})
options := volume.VolumeOptions{
Capacity: cap,
AccessModes: []api.PersistentVolumeAccessMode{
api.ReadWriteOnce,
},
PVC: pvc,
PersistentVolumeReclaimPolicy: api.PersistentVolumeReclaimDelete,
}

Expand All @@ -62,7 +58,7 @@ func TestProvision(t *testing.T) {
persistentSpec, err := provisioner.Provision()
assert.NoError(err, "Provision() failed: ", err)

cap = persistentSpec.Spec.Capacity[api.ResourceStorage]
cap := persistentSpec.Spec.Capacity[api.ResourceStorage]

assert.Equal(int64(3*1024*1024*1024), cap.Value())

Expand All @@ -78,10 +74,7 @@ func TestProvision(t *testing.T) {

// parameters are not supported
options = volume.VolumeOptions{
Capacity: cap,
AccessModes: []api.PersistentVolumeAccessMode{
api.ReadWriteOnce,
},
PVC: pvc,
PersistentVolumeReclaimPolicy: api.PersistentVolumeReclaimDelete,
Parameters: map[string]string{
"not-supported-params": "test123",
Expand All @@ -93,13 +86,10 @@ func TestProvision(t *testing.T) {
assert.Error(err, "Provision() did not fail with Parameters specified")

// selectors are not supported
pvc.Spec.Selector = &unversioned.LabelSelector{MatchLabels: map[string]string{"key": "value"}}
options = volume.VolumeOptions{
Capacity: cap,
AccessModes: []api.PersistentVolumeAccessMode{
api.ReadWriteOnce,
},
PVC: pvc,
PersistentVolumeReclaimPolicy: api.PersistentVolumeReclaimDelete,
Selector: &unversioned.LabelSelector{MatchLabels: map[string]string{"key": "value"}},
}

provisioner = newTestableProvisioner(assert, options)
Expand Down

0 comments on commit 4e393fa

Please sign in to comment.