Merge pull request #18957 from jsafrane/devel/pv-name-tag

pkg/controller/persistentvolume/persistentvolume_provisioner_controller.go

@@ -172,7 +172,7 @@ func (controller *PersistentVolumeProvisionerController) reconcileClaim(claim *a
 	}
 
 	glog.V(5).Infof("PersistentVolumeClaim[%s] provisioning", claim.Name)
-	provisioner, err := newProvisioner(controller.provisioner, claim)
+	provisioner, err := newProvisioner(controller.provisioner, claim, nil)
 	if err != nil {
 		return fmt.Errorf("Unexpected error getting new provisioner for claim %s: %v\n", claim.Name, err)
 	}
@@ -274,7 +274,7 @@ func provisionVolume(pv *api.PersistentVolume, controller *PersistentVolumeProvi
 	}
 	claim := obj.(*api.PersistentVolumeClaim)
 
-	provisioner, _ := newProvisioner(controller.provisioner, claim)
+	provisioner, _ := newProvisioner(controller.provisioner, claim, pv)
 	err := provisioner.Provision(pv)
 	if err != nil {
 		glog.Errorf("Could not provision %s", pv.Name)
@@ -330,15 +330,21 @@ func (controller *PersistentVolumeProvisionerController) Stop() {
 	}
 }
 
-func newProvisioner(plugin volume.ProvisionableVolumePlugin, claim *api.PersistentVolumeClaim) (volume.Provisioner, error) {
+func newProvisioner(plugin volume.ProvisionableVolumePlugin, claim *api.PersistentVolumeClaim, pv *api.PersistentVolume) (volume.Provisioner, error) {
+	tags := make(map[string]string)
+	tags[cloudVolumeCreatedForClaimNamespaceTag] = claim.Namespace
+	tags[cloudVolumeCreatedForClaimNameTag] = claim.Name
+
+	// pv can be nil when the provisioner has not created the PV yet
+	if pv != nil {
+		tags[cloudVolumeCreatedForVolumeNameTag] = pv.Name
+	}
+
 	volumeOptions := volume.VolumeOptions{
 		Capacity:                      claim.Spec.Resources.Requests[api.ResourceName(api.ResourceStorage)],
 		AccessModes:                   claim.Spec.AccessModes,
 		PersistentVolumeReclaimPolicy: api.PersistentVolumeReclaimDelete,
-		CloudTags: &map[string]string{
-			cloudVolumeCreatedForNamespaceTag: claim.Namespace,
-			cloudVolumeCreatedForNameTag:      claim.Name,
-		},
+		CloudTags:                     &tags,
 	}
 
 	provisioner, err := plugin.NewProvisioner(volumeOptions)

pkg/controller/persistentvolume/persistentvolume_provisioner_controller_test.go

@@ -33,7 +33,7 @@ import (
 )
 
 func TestProvisionerRunStop(t *testing.T) {
-	controller, _ := makeTestController()
+	controller, _, _ := makeTestController()
 
 	if len(controller.stopChannels) != 0 {
 		t.Errorf("Non-running provisioner should not have any stopChannels.  Got %v", len(controller.stopChannels))
@@ -92,15 +92,15 @@ func makeTestClaim() *api.PersistentVolumeClaim {
 	}
 }
 
-func makeTestController() (*PersistentVolumeProvisionerController, *mockControllerClient) {
+func makeTestController() (*PersistentVolumeProvisionerController, *mockControllerClient, *volume.FakeVolumePlugin) {
 	mockClient := &mockControllerClient{}
 	mockVolumePlugin := &volume.FakeVolumePlugin{}
 	controller, _ := NewPersistentVolumeProvisionerController(mockClient, 1*time.Second, nil, mockVolumePlugin, &fake_cloud.FakeCloud{})
-	return controller, mockClient
+	return controller, mockClient, mockVolumePlugin
 }
 
 func TestReconcileClaim(t *testing.T) {
-	controller, mockClient := makeTestController()
+	controller, mockClient, _ := makeTestController()
 	pvc := makeTestClaim()
 
 	// watch would have added the claim to the store
@@ -132,9 +132,16 @@ func TestReconcileClaim(t *testing.T) {
 	}
 }
 
+func checkTagValue(t *testing.T, tags map[string]string, tag string, expectedValue string) {
+	value, found := tags[tag]
+	if !found || value != expectedValue {
+		t.Errorf("Expected tag value %s = %s but value %s found", tag, expectedValue, value)
+	}
+}
+
 func TestReconcileVolume(t *testing.T) {
 
-	controller, mockClient := makeTestController()
+	controller, mockClient, mockVolumePlugin := makeTestController()
 	pv := makeTestVolume()
 	pvc := makeTestClaim()
 
@@ -163,6 +170,13 @@ func TestReconcileVolume(t *testing.T) {
 	if !isAnnotationMatch(pvProvisioningRequiredAnnotationKey, pvProvisioningCompletedAnnotationValue, mockClient.volume.Annotations) {
 		t.Errorf("Expected %s but got %s", pvProvisioningRequiredAnnotationKey, mockClient.volume.Annotations[pvProvisioningRequiredAnnotationKey])
 	}
+
+	// Check that the volume plugin was called with correct tags
+	tags := *mockVolumePlugin.LastProvisionerOptions.CloudTags
+	checkTagValue(t, tags, cloudVolumeCreatedForClaimNamespaceTag, pvc.Namespace)
+	checkTagValue(t, tags, cloudVolumeCreatedForClaimNameTag, pvc.Name)
+	checkTagValue(t, tags, cloudVolumeCreatedForVolumeNameTag, pv.Name)
+
 }
 
 var _ controllerClient = &mockControllerClient{}

pkg/controller/persistentvolume/types.go

@@ -32,10 +32,13 @@ const (
 	qosProvisioningKey = "volume.alpha.kubernetes.io/storage-class"
 	// Name of a tag attached to a real volume in cloud (e.g. AWS EBS or GCE PD)
 	// with namespace of a persistent volume claim used to create this volume.
-	cloudVolumeCreatedForNamespaceTag = "kubernetes.io/created-for/pvc/namespace"
+	cloudVolumeCreatedForClaimNamespaceTag = "kubernetes.io/created-for/pvc/namespace"
 	// Name of a tag attached to a real volume in cloud (e.g. AWS EBS or GCE PD)
 	// with name of a persistent volume claim used to create this volume.
-	cloudVolumeCreatedForNameTag = "kubernetes.io/created-for/pvc/name"
+	cloudVolumeCreatedForClaimNameTag = "kubernetes.io/created-for/pvc/name"
+	// Name of a tag attached to a real volume in cloud (e.g. AWS EBS or GCE PD)
+	// with name of appropriate Kubernetes persistent volume .
+	cloudVolumeCreatedForVolumeNameTag = "kubernetes.io/created-for/pv/name"
 )
 
 // persistentVolumeOrderedIndex is a cache.Store that keeps persistent volumes indexed by AccessModes and ordered by storage capacity.

pkg/volume/testing.go

@@ -120,9 +120,10 @@ func ProbeVolumePlugins(config VolumeConfig) []VolumePlugin {
 // Use as:
 //   volume.RegisterPlugin(&FakePlugin{"fake-name"})
 type FakeVolumePlugin struct {
-	PluginName string
-	Host       VolumeHost
-	Config     VolumeConfig
+	PluginName             string
+	Host                   VolumeHost
+	Config                 VolumeConfig
+	LastProvisionerOptions VolumeOptions
 }
 
 var _ VolumePlugin = &FakeVolumePlugin{}
@@ -161,6 +162,7 @@ func (plugin *FakeVolumePlugin) NewDeleter(spec *Spec) (Deleter, error) {
 }
 
 func (plugin *FakeVolumePlugin) NewProvisioner(options VolumeOptions) (Provisioner, error) {
+	plugin.LastProvisionerOptions = options
 	return &FakeProvisioner{options, plugin.Host}, nil
 }
 

test/integration/persistent_volumes_test.go

@@ -50,7 +50,7 @@ func TestPersistentVolumeRecycler(t *testing.T) {
 	testClient := client.NewOrDie(&client.Config{Host: s.URL, GroupVersion: testapi.Default.GroupVersion()})
 	host := volume.NewFakeVolumeHost("/tmp/fake", nil, nil)
 
-	plugins := []volume.VolumePlugin{&volume.FakeVolumePlugin{"plugin-name", host, volume.VolumeConfig{}}}
+	plugins := []volume.VolumePlugin{&volume.FakeVolumePlugin{"plugin-name", host, volume.VolumeConfig{}, volume.VolumeOptions{}}}
 	cloud := &fake_cloud.FakeCloud{}
 
 	binder := persistentvolumecontroller.NewPersistentVolumeClaimBinder(binderClient, 10*time.Second)