From 400ebf87a18648497a693e898e0922ec71e55c1d Mon Sep 17 00:00:00 2001 From: Jan Safranek Date: Mon, 4 Jan 2016 12:28:02 +0100 Subject: [PATCH 1/2] Add PV.Name to volume tags. We add claim.Name and claim.Namespace as tags to AWS EBS / GCE PD / OpenStack Cinder volumes created by Kubernetes. To easily match Kubernetes volumes and cloud volumes, let's add also PV.Name. --- ...persistentvolume_provisioner_controller.go | 20 ++++++++++++------- pkg/controller/persistentvolume/types.go | 7 +++++-- 2 files changed, 18 insertions(+), 9 deletions(-) diff --git a/pkg/controller/persistentvolume/persistentvolume_provisioner_controller.go b/pkg/controller/persistentvolume/persistentvolume_provisioner_controller.go index 846bc090ee9..8fc8e3fce8e 100644 --- a/pkg/controller/persistentvolume/persistentvolume_provisioner_controller.go +++ b/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) diff --git a/pkg/controller/persistentvolume/types.go b/pkg/controller/persistentvolume/types.go index 58cfade69b0..df9a0bb011f 100644 --- a/pkg/controller/persistentvolume/types.go +++ b/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. From 8c48250a558130b003da587255dd8a6ae22b4399 Mon Sep 17 00:00:00 2001 From: Jan Safranek Date: Fri, 8 Jan 2016 19:09:29 +0100 Subject: [PATCH 2/2] Add an integration test for volume tags. --- ...stentvolume_provisioner_controller_test.go | 24 +++++++++++++++---- pkg/volume/testing.go | 8 ++++--- test/integration/persistent_volumes_test.go | 2 +- 3 files changed, 25 insertions(+), 9 deletions(-) diff --git a/pkg/controller/persistentvolume/persistentvolume_provisioner_controller_test.go b/pkg/controller/persistentvolume/persistentvolume_provisioner_controller_test.go index d05fa38b860..e86daf7ae77 100644 --- a/pkg/controller/persistentvolume/persistentvolume_provisioner_controller_test.go +++ b/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{} diff --git a/pkg/volume/testing.go b/pkg/volume/testing.go index 02f8fba72fc..68b3d4e670e 100644 --- a/pkg/volume/testing.go +++ b/pkg/volume/testing.go @@ -119,9 +119,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{} @@ -160,6 +161,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 } diff --git a/test/integration/persistent_volumes_test.go b/test/integration/persistent_volumes_test.go index 8884bf875c4..539041e0bdc 100644 --- a/test/integration/persistent_volumes_test.go +++ b/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)