diff --git a/pkg/controller/persistentvolume/persistentvolume_framework_test.go b/pkg/controller/persistentvolume/persistentvolume_framework_test.go index a0115ae5750..fda289b6b33 100644 --- a/pkg/controller/persistentvolume/persistentvolume_framework_test.go +++ b/pkg/controller/persistentvolume/persistentvolume_framework_test.go @@ -41,6 +41,7 @@ import ( "k8s.io/kubernetes/pkg/conversion" "k8s.io/kubernetes/pkg/runtime" "k8s.io/kubernetes/pkg/types" + "k8s.io/kubernetes/pkg/util" "k8s.io/kubernetes/pkg/util/diff" vol "k8s.io/kubernetes/pkg/volume" ) @@ -783,6 +784,7 @@ type mockVolumePlugin struct { deleteCallCounter int recycleCalls []error recycleCallCounter int + provisionOptions vol.VolumeOptions } var _ vol.VolumePlugin = &mockVolumePlugin{} @@ -813,30 +815,49 @@ func (plugin *mockVolumePlugin) NewProvisioner(options vol.VolumeOptions) (vol.P if len(plugin.provisionCalls) > 0 { // mockVolumePlugin directly implements Provisioner interface glog.V(4).Infof("mock plugin NewProvisioner called, returning mock provisioner") + plugin.provisionOptions = options return plugin, nil } else { return nil, fmt.Errorf("Mock plugin error: no provisionCalls configured") } } -func (plugin *mockVolumePlugin) Provision(*api.PersistentVolume) error { - if len(plugin.provisionCalls) <= plugin.provisionCallCounter { - return fmt.Errorf("Mock plugin error: unexpected provisioner call %d", plugin.provisionCallCounter) - } - ret := plugin.provisionCalls[plugin.provisionCallCounter] - plugin.provisionCallCounter++ - glog.V(4).Infof("mock plugin Provision call nr. %d, returning %v", plugin.provisionCallCounter, ret) - return ret -} - -func (plugin *mockVolumePlugin) NewPersistentVolumeTemplate() (*api.PersistentVolume, error) { +func (plugin *mockVolumePlugin) Provision() (*api.PersistentVolume, error) { if len(plugin.provisionCalls) <= plugin.provisionCallCounter { return nil, fmt.Errorf("Mock plugin error: unexpected provisioner call %d", plugin.provisionCallCounter) } - ret := plugin.provisionCalls[plugin.provisionCallCounter] + + var pv *api.PersistentVolume + err := plugin.provisionCalls[plugin.provisionCallCounter] + if err == nil { + // Create a fake PV + fullpath := fmt.Sprintf("/tmp/hostpath_pv/%s", util.NewUUID()) + + pv = &api.PersistentVolume{ + ObjectMeta: api.ObjectMeta{ + Name: plugin.provisionOptions.PVName, + Annotations: map[string]string{ + "kubernetes.io/createdby": "hostpath-dynamic-provisioner", + }, + }, + Spec: api.PersistentVolumeSpec{ + PersistentVolumeReclaimPolicy: plugin.provisionOptions.PersistentVolumeReclaimPolicy, + AccessModes: plugin.provisionOptions.AccessModes, + Capacity: api.ResourceList{ + api.ResourceName(api.ResourceStorage): plugin.provisionOptions.Capacity, + }, + PersistentVolumeSource: api.PersistentVolumeSource{ + HostPath: &api.HostPathVolumeSource{ + Path: fullpath, + }, + }, + }, + } + } + plugin.provisionCallCounter++ - glog.V(4).Infof("mock plugin NewPersistentVolumeTemplate call nr. %d, returning %v", plugin.provisionCallCounter, ret) - return nil, ret + glog.V(4).Infof("mock plugin Provision call nr. %d, returning %v: %v", plugin.provisionCallCounter, pv, err) + return pv, err } // Deleter interfaces diff --git a/pkg/volume/aws_ebs/aws_ebs.go b/pkg/volume/aws_ebs/aws_ebs.go index dfbdae80533..d21bf0cac12 100644 --- a/pkg/volume/aws_ebs/aws_ebs.go +++ b/pkg/volume/aws_ebs/aws_ebs.go @@ -408,14 +408,35 @@ type awsElasticBlockStoreProvisioner struct { var _ volume.Provisioner = &awsElasticBlockStoreProvisioner{} -func (c *awsElasticBlockStoreProvisioner) Provision(pv *api.PersistentVolume) error { +func (c *awsElasticBlockStoreProvisioner) Provision() (*api.PersistentVolume, error) { volumeID, sizeGB, labels, err := c.manager.CreateVolume(c) if err != nil { - return err + return nil, err } - pv.Spec.PersistentVolumeSource.AWSElasticBlockStore.VolumeID = volumeID - pv.Spec.Capacity = api.ResourceList{ - api.ResourceName(api.ResourceStorage): resource.MustParse(fmt.Sprintf("%dGi", sizeGB)), + + pv := &api.PersistentVolume{ + ObjectMeta: api.ObjectMeta{ + Name: c.options.PVName, + Labels: map[string]string{}, + Annotations: map[string]string{ + "kubernetes.io/createdby": "aws-ebs-dynamic-provisioner", + }, + }, + Spec: api.PersistentVolumeSpec{ + PersistentVolumeReclaimPolicy: c.options.PersistentVolumeReclaimPolicy, + AccessModes: c.options.AccessModes, + Capacity: api.ResourceList{ + api.ResourceName(api.ResourceStorage): resource.MustParse(fmt.Sprintf("%dGi", sizeGB)), + }, + PersistentVolumeSource: api.PersistentVolumeSource{ + AWSElasticBlockStore: &api.AWSElasticBlockStoreVolumeSource{ + VolumeID: volumeID, + FSType: "ext4", + Partition: 0, + ReadOnly: false, + }, + }, + }, } if len(labels) != 0 { @@ -427,34 +448,5 @@ func (c *awsElasticBlockStoreProvisioner) Provision(pv *api.PersistentVolume) er } } - return nil -} - -func (c *awsElasticBlockStoreProvisioner) NewPersistentVolumeTemplate() (*api.PersistentVolume, error) { - // Provide dummy api.PersistentVolume.Spec, it will be filled in - // awsElasticBlockStoreProvisioner.Provision() - return &api.PersistentVolume{ - ObjectMeta: api.ObjectMeta{ - GenerateName: "pv-aws-", - Labels: map[string]string{}, - Annotations: map[string]string{ - "kubernetes.io/createdby": "aws-ebs-dynamic-provisioner", - }, - }, - Spec: api.PersistentVolumeSpec{ - PersistentVolumeReclaimPolicy: c.options.PersistentVolumeReclaimPolicy, - AccessModes: c.options.AccessModes, - Capacity: api.ResourceList{ - api.ResourceName(api.ResourceStorage): c.options.Capacity, - }, - PersistentVolumeSource: api.PersistentVolumeSource{ - AWSElasticBlockStore: &api.AWSElasticBlockStoreVolumeSource{ - VolumeID: volume.ProvisionedVolumeName, - FSType: "ext4", - Partition: 0, - ReadOnly: false, - }, - }, - }, - }, nil + return pv, nil } diff --git a/pkg/volume/aws_ebs/aws_ebs_test.go b/pkg/volume/aws_ebs/aws_ebs_test.go index c0193021f00..02e2d098e0a 100644 --- a/pkg/volume/aws_ebs/aws_ebs_test.go +++ b/pkg/volume/aws_ebs/aws_ebs_test.go @@ -220,14 +220,7 @@ func TestPlugin(t *testing.T) { PersistentVolumeReclaimPolicy: api.PersistentVolumeReclaimDelete, } provisioner, err := plug.(*awsElasticBlockStorePlugin).newProvisionerInternal(options, &fakePDManager{}) - persistentSpec, err := provisioner.NewPersistentVolumeTemplate() - if err != nil { - t.Errorf("NewPersistentVolumeTemplate() failed: %v", err) - } - - // get 2nd Provisioner - persistent volume controller will do the same - provisioner, err = plug.(*awsElasticBlockStorePlugin).newProvisionerInternal(options, &fakePDManager{}) - err = provisioner.Provision(persistentSpec) + persistentSpec, err := provisioner.Provision() if err != nil { t.Errorf("Provision() failed: %v", err) } diff --git a/pkg/volume/cinder/cinder.go b/pkg/volume/cinder/cinder.go index b4dcaedb8a5..e680528c217 100644 --- a/pkg/volume/cinder/cinder.go +++ b/pkg/volume/cinder/cinder.go @@ -427,25 +427,16 @@ type cinderVolumeProvisioner struct { var _ volume.Provisioner = &cinderVolumeProvisioner{} -func (c *cinderVolumeProvisioner) Provision(pv *api.PersistentVolume) error { +func (c *cinderVolumeProvisioner) Provision() (*api.PersistentVolume, error) { volumeID, sizeGB, err := c.manager.CreateVolume(c) if err != nil { - return err + return nil, err } - pv.Spec.PersistentVolumeSource.Cinder.VolumeID = volumeID - pv.Spec.Capacity = api.ResourceList{ - api.ResourceName(api.ResourceStorage): resource.MustParse(fmt.Sprintf("%dGi", sizeGB)), - } - return nil -} -func (c *cinderVolumeProvisioner) NewPersistentVolumeTemplate() (*api.PersistentVolume, error) { - // Provide dummy api.PersistentVolume.Spec, it will be filled in - // cinderVolumeProvisioner.Provision() - return &api.PersistentVolume{ + pv := &api.PersistentVolume{ ObjectMeta: api.ObjectMeta{ - GenerateName: "pv-cinder-", - Labels: map[string]string{}, + Name: c.options.PVName, + Labels: map[string]string{}, Annotations: map[string]string{ "kubernetes.io/createdby": "cinder-dynamic-provisioner", }, @@ -454,16 +445,16 @@ func (c *cinderVolumeProvisioner) NewPersistentVolumeTemplate() (*api.Persistent PersistentVolumeReclaimPolicy: c.options.PersistentVolumeReclaimPolicy, AccessModes: c.options.AccessModes, Capacity: api.ResourceList{ - api.ResourceName(api.ResourceStorage): c.options.Capacity, + api.ResourceName(api.ResourceStorage): resource.MustParse(fmt.Sprintf("%dGi", sizeGB)), }, PersistentVolumeSource: api.PersistentVolumeSource{ Cinder: &api.CinderVolumeSource{ - VolumeID: volume.ProvisionedVolumeName, + VolumeID: volumeID, FSType: "ext4", ReadOnly: false, }, }, }, - }, nil - + } + return pv, nil } diff --git a/pkg/volume/cinder/cinder_test.go b/pkg/volume/cinder/cinder_test.go index 6f1a53eceba..facbbc8f47c 100644 --- a/pkg/volume/cinder/cinder_test.go +++ b/pkg/volume/cinder/cinder_test.go @@ -212,14 +212,7 @@ func TestPlugin(t *testing.T) { PersistentVolumeReclaimPolicy: api.PersistentVolumeReclaimDelete, } provisioner, err := plug.(*cinderPlugin).newProvisionerInternal(options, &fakePDManager{0}) - persistentSpec, err := provisioner.NewPersistentVolumeTemplate() - if err != nil { - t.Errorf("NewPersistentVolumeTemplate() failed: %v", err) - } - - // get 2nd Provisioner - persistent volume controller will do the same - provisioner, err = plug.(*cinderPlugin).newProvisionerInternal(options, &fakePDManager{0}) - err = provisioner.Provision(persistentSpec) + persistentSpec, err := provisioner.Provision() if err != nil { t.Errorf("Provision() failed: %v", err) } diff --git a/pkg/volume/gce_pd/gce_pd.go b/pkg/volume/gce_pd/gce_pd.go index 006ed573496..32ea1043420 100644 --- a/pkg/volume/gce_pd/gce_pd.go +++ b/pkg/volume/gce_pd/gce_pd.go @@ -370,14 +370,35 @@ type gcePersistentDiskProvisioner struct { var _ volume.Provisioner = &gcePersistentDiskProvisioner{} -func (c *gcePersistentDiskProvisioner) Provision(pv *api.PersistentVolume) error { +func (c *gcePersistentDiskProvisioner) Provision() (*api.PersistentVolume, error) { volumeID, sizeGB, labels, err := c.manager.CreateVolume(c) if err != nil { - return err + return nil, err } - pv.Spec.PersistentVolumeSource.GCEPersistentDisk.PDName = volumeID - pv.Spec.Capacity = api.ResourceList{ - api.ResourceName(api.ResourceStorage): resource.MustParse(fmt.Sprintf("%dGi", sizeGB)), + + pv := &api.PersistentVolume{ + ObjectMeta: api.ObjectMeta{ + Name: c.options.PVName, + Labels: map[string]string{}, + Annotations: map[string]string{ + "kubernetes.io/createdby": "gce-pd-dynamic-provisioner", + }, + }, + Spec: api.PersistentVolumeSpec{ + PersistentVolumeReclaimPolicy: c.options.PersistentVolumeReclaimPolicy, + AccessModes: c.options.AccessModes, + Capacity: api.ResourceList{ + api.ResourceName(api.ResourceStorage): resource.MustParse(fmt.Sprintf("%dGi", sizeGB)), + }, + PersistentVolumeSource: api.PersistentVolumeSource{ + GCEPersistentDisk: &api.GCEPersistentDiskVolumeSource{ + PDName: volumeID, + FSType: "ext4", + Partition: 0, + ReadOnly: false, + }, + }, + }, } if len(labels) != 0 { @@ -389,34 +410,5 @@ func (c *gcePersistentDiskProvisioner) Provision(pv *api.PersistentVolume) error } } - return nil -} - -func (c *gcePersistentDiskProvisioner) NewPersistentVolumeTemplate() (*api.PersistentVolume, error) { - // Provide dummy api.PersistentVolume.Spec, it will be filled in - // gcePersistentDiskProvisioner.Provision() - return &api.PersistentVolume{ - ObjectMeta: api.ObjectMeta{ - GenerateName: "pv-gce-", - Labels: map[string]string{}, - Annotations: map[string]string{ - "kubernetes.io/createdby": "gce-pd-dynamic-provisioner", - }, - }, - Spec: api.PersistentVolumeSpec{ - PersistentVolumeReclaimPolicy: c.options.PersistentVolumeReclaimPolicy, - AccessModes: c.options.AccessModes, - Capacity: api.ResourceList{ - api.ResourceName(api.ResourceStorage): c.options.Capacity, - }, - PersistentVolumeSource: api.PersistentVolumeSource{ - GCEPersistentDisk: &api.GCEPersistentDiskVolumeSource{ - PDName: volume.ProvisionedVolumeName, - FSType: "ext4", - Partition: 0, - ReadOnly: false, - }, - }, - }, - }, nil + return pv, nil } diff --git a/pkg/volume/gce_pd/gce_pd_test.go b/pkg/volume/gce_pd/gce_pd_test.go index 4987f2bd665..9c2bf7e5ffe 100644 --- a/pkg/volume/gce_pd/gce_pd_test.go +++ b/pkg/volume/gce_pd/gce_pd_test.go @@ -216,14 +216,7 @@ func TestPlugin(t *testing.T) { PersistentVolumeReclaimPolicy: api.PersistentVolumeReclaimDelete, } provisioner, err := plug.(*gcePersistentDiskPlugin).newProvisionerInternal(options, &fakePDManager{}) - persistentSpec, err := provisioner.NewPersistentVolumeTemplate() - if err != nil { - t.Errorf("NewPersistentVolumeTemplate() failed: %v", err) - } - - // get 2nd Provisioner - persistent volume controller will do the same - provisioner, err = plug.(*gcePersistentDiskPlugin).newProvisionerInternal(options, &fakePDManager{}) - err = provisioner.Provision(persistentSpec) + persistentSpec, err := provisioner.Provision() if err != nil { t.Errorf("Provision() failed: %v", err) } diff --git a/pkg/volume/host_path/host_path.go b/pkg/volume/host_path/host_path.go index b0ddb0aa020..ff2d9c667ff 100644 --- a/pkg/volume/host_path/host_path.go +++ b/pkg/volume/host_path/host_path.go @@ -252,18 +252,12 @@ type hostPathProvisioner struct { // Create for hostPath simply creates a local /tmp/hostpath_pv/%s directory as a new PersistentVolume. // This Provisioner is meant for development and testing only and WILL NOT WORK in a multi-node cluster. -func (r *hostPathProvisioner) Provision(pv *api.PersistentVolume) error { - if pv.Spec.HostPath == nil { - return fmt.Errorf("pv.Spec.HostPath cannot be nil") - } - return os.MkdirAll(pv.Spec.HostPath.Path, 0750) -} - -func (r *hostPathProvisioner) NewPersistentVolumeTemplate() (*api.PersistentVolume, error) { +func (r *hostPathProvisioner) Provision() (*api.PersistentVolume, error) { fullpath := fmt.Sprintf("/tmp/hostpath_pv/%s", util.NewUUID()) - return &api.PersistentVolume{ + + pv := &api.PersistentVolume{ ObjectMeta: api.ObjectMeta{ - GenerateName: "pv-hostpath-", + Name: r.options.PVName, Annotations: map[string]string{ "kubernetes.io/createdby": "hostpath-dynamic-provisioner", }, @@ -280,7 +274,9 @@ func (r *hostPathProvisioner) NewPersistentVolumeTemplate() (*api.PersistentVolu }, }, }, - }, nil + } + + return pv, os.MkdirAll(pv.Spec.HostPath.Path, 0750) } // hostPathDeleter deletes a hostPath PV from the cluster. diff --git a/pkg/volume/host_path/host_path_test.go b/pkg/volume/host_path/host_path_test.go index 440f4cfdeee..6a6e9e2f3bb 100644 --- a/pkg/volume/host_path/host_path_test.go +++ b/pkg/volume/host_path/host_path_test.go @@ -163,7 +163,7 @@ func TestProvisioner(t *testing.T) { if err != nil { t.Errorf("Failed to make a new Provisioner: %v", err) } - pv, err := creater.NewPersistentVolumeTemplate() + pv, err := creater.Provision() if err != nil { t.Errorf("Unexpected error creating volume: %v", err) } diff --git a/pkg/volume/testing/testing.go b/pkg/volume/testing/testing.go index e7ecb96f351..425c148550c 100644 --- a/pkg/volume/testing/testing.go +++ b/pkg/volume/testing/testing.go @@ -340,11 +340,12 @@ type FakeProvisioner struct { Host VolumeHost } -func (fc *FakeProvisioner) NewPersistentVolumeTemplate() (*api.PersistentVolume, error) { +func (fc *FakeProvisioner) Provision() (*api.PersistentVolume, error) { fullpath := fmt.Sprintf("/tmp/hostpath_pv/%s", util.NewUUID()) - return &api.PersistentVolume{ + + pv := &api.PersistentVolume{ ObjectMeta: api.ObjectMeta{ - GenerateName: "pv-fakeplugin-", + Name: fc.Options.PVName, Annotations: map[string]string{ "kubernetes.io/createdby": "fakeplugin-provisioner", }, @@ -361,11 +362,9 @@ func (fc *FakeProvisioner) NewPersistentVolumeTemplate() (*api.PersistentVolume, }, }, }, - }, nil -} + } -func (fc *FakeProvisioner) Provision(pv *api.PersistentVolume) error { - return nil + return pv, nil } // FindEmptyDirectoryUsageOnTmpfs finds the expected usage of an empty directory existing on diff --git a/pkg/volume/volume.go b/pkg/volume/volume.go index 88a4e6d871e..9a609e18bb0 100644 --- a/pkg/volume/volume.go +++ b/pkg/volume/volume.go @@ -111,13 +111,10 @@ type Recycler interface { // Provisioner is an interface that creates templates for PersistentVolumes and can create the volume // as a new resource in the infrastructure provider. type Provisioner interface { - // Provision creates the resource by allocating the underlying volume in a storage system. - // This method should block until completion. - Provision(*api.PersistentVolume) error - // NewPersistentVolumeTemplate creates a new PersistentVolume to be used as a template before saving. - // The provisioner will want to tweak its properties, assign correct annotations, etc. - // This func should *NOT* persist the PV in the API. That is left to the caller. - NewPersistentVolumeTemplate() (*api.PersistentVolume, error) + // Provision creates the resource by allocating the underlying volume in a + // storage system. This method should block until completion and returns + // PersistentVolume representing the created storage resource. + Provision() (*api.PersistentVolume, error) } // Deleter removes the resource from the underlying storage provider. Calls to this method should block until