refactor persistent volume labeler admission controller to use cloudprovider.PVLabler

plugin/pkg/admission/storage/persistentvolume/label/BUILD

@@ -15,13 +15,12 @@ go_library(
     importpath = "k8s.io/kubernetes/plugin/pkg/admission/storage/persistentvolume/label",
     deps = [
         "//pkg/apis/core:go_default_library",
-        "//pkg/cloudprovider/providers/aws:go_default_library",
-        "//pkg/cloudprovider/providers/azure:go_default_library",
-        "//pkg/cloudprovider/providers/gce:go_default_library",
+        "//pkg/apis/core/v1:go_default_library",
         "//pkg/kubeapiserver/admission:go_default_library",
         "//pkg/kubelet/apis:go_default_library",
         "//pkg/volume:go_default_library",
         "//pkg/volume/util:go_default_library",
+        "//staging/src/k8s.io/api/core/v1:go_default_library",
         "//staging/src/k8s.io/apiserver/pkg/admission:go_default_library",
         "//staging/src/k8s.io/cloud-provider:go_default_library",
         "//vendor/k8s.io/klog:go_default_library",
@@ -34,13 +33,12 @@ go_test(
     embed = [":go_default_library"],
     deps = [
         "//pkg/apis/core:go_default_library",
-        "//pkg/cloudprovider/providers/aws:go_default_library",
         "//pkg/kubelet/apis:go_default_library",
         "//pkg/volume/util:go_default_library",
-        "//staging/src/k8s.io/apimachinery/pkg/api/resource:go_default_library",
+        "//staging/src/k8s.io/api/core/v1:go_default_library",
         "//staging/src/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library",
-        "//staging/src/k8s.io/apimachinery/pkg/types:go_default_library",
         "//staging/src/k8s.io/apiserver/pkg/admission:go_default_library",
+        "//staging/src/k8s.io/cloud-provider:go_default_library",
     ],
 )
 

plugin/pkg/admission/storage/persistentvolume/label/admission.go

@@ -18,17 +18,18 @@ package label
 
 import (
 	"bytes"
+	"context"
+	"errors"
 	"fmt"
 	"io"
 	"sync"
 
+	v1 "k8s.io/api/core/v1"
 	"k8s.io/apiserver/pkg/admission"
 	cloudprovider "k8s.io/cloud-provider"
 	"k8s.io/klog"
 	api "k8s.io/kubernetes/pkg/apis/core"
-	"k8s.io/kubernetes/pkg/cloudprovider/providers/aws"
-	"k8s.io/kubernetes/pkg/cloudprovider/providers/azure"
-	"k8s.io/kubernetes/pkg/cloudprovider/providers/gce"
+	k8s_api_v1 "k8s.io/kubernetes/pkg/apis/core/v1"
 	kubeapiserveradmission "k8s.io/kubernetes/pkg/kubeapiserver/admission"
 	kubeletapis "k8s.io/kubernetes/pkg/kubelet/apis"
 	vol "k8s.io/kubernetes/pkg/volume"
@@ -53,11 +54,12 @@ var _ = admission.Interface(&persistentVolumeLabel{})
 type persistentVolumeLabel struct {
 	*admission.Handler
 
-	mutex            sync.Mutex
-	ebsVolumes       aws.Volumes
-	cloudConfig      []byte
-	gceCloudProvider *gce.Cloud
-	azureProvider    *azure.Cloud
+	mutex              sync.Mutex
+	cloudConfig        []byte
+	awsPVLabeler       cloudprovider.PVLabeler
+	gcePVLabeler       cloudprovider.PVLabeler
+	azurePVLabeler     cloudprovider.PVLabeler
+	openStackPVLabeler cloudprovider.PVLabeler
 }
 
 var _ admission.MutationInterface = &persistentVolumeLabel{}
@@ -186,47 +188,47 @@ func (l *persistentVolumeLabel) findAWSEBSLabels(volume *api.PersistentVolume) (
 	if volume.Spec.AWSElasticBlockStore.VolumeID == vol.ProvisionedVolumeName {
 		return nil, nil
 	}
-	ebsVolumes, err := l.getEBSVolumes()
+	pvlabler, err := l.getAWSPVLabeler()
 	if err != nil {
 		return nil, err
 	}
-	if ebsVolumes == nil {
+	if pvlabler == nil {
 		return nil, fmt.Errorf("unable to build AWS cloud provider for EBS")
 	}
 
-	// TODO: GetVolumeLabels is actually a method on the Volumes interface
-	// If that gets standardized we can refactor to reduce code duplication
-	spec := aws.KubernetesVolumeID(volume.Spec.AWSElasticBlockStore.VolumeID)
-	labels, err := ebsVolumes.GetVolumeLabels(spec)
+	pv := &v1.PersistentVolume{}
+	err = k8s_api_v1.Convert_core_PersistentVolume_To_v1_PersistentVolume(volume, pv, nil)
 	if err != nil {
-		return nil, err
+		return nil, fmt.Errorf("failed to convert PersistentVolume to core/v1: %q", err)
 	}
 
-	return labels, nil
+	return pvlabler.GetLabelsForVolume(context.TODO(), pv)
 }
 
-// getEBSVolumes returns the AWS Volumes interface for ebs
-func (l *persistentVolumeLabel) getEBSVolumes() (aws.Volumes, error) {
+// getAWSPVLabeler returns the AWS implementation of PVLabeler
+func (l *persistentVolumeLabel) getAWSPVLabeler() (cloudprovider.PVLabeler, error) {
 	l.mutex.Lock()
 	defer l.mutex.Unlock()
 
-	if l.ebsVolumes == nil {
+	if l.awsPVLabeler == nil {
 		var cloudConfigReader io.Reader
 		if len(l.cloudConfig) > 0 {
 			cloudConfigReader = bytes.NewReader(l.cloudConfig)
 		}
+
 		cloudProvider, err := cloudprovider.GetCloudProvider("aws", cloudConfigReader)
 		if err != nil || cloudProvider == nil {
 			return nil, err
 		}
-		awsCloudProvider, ok := cloudProvider.(*aws.Cloud)
+
+		awsPVLabeler, ok := cloudProvider.(cloudprovider.PVLabeler)
 		if !ok {
-			// GetCloudProvider has gone very wrong
-			return nil, fmt.Errorf("error retrieving AWS cloud provider")
+			return nil, errors.New("AWS cloud provider does not implement PV labeling")
 		}
-		l.ebsVolumes = awsCloudProvider
+
+		l.awsPVLabeler = awsPVLabeler
 	}
-	return l.ebsVolumes, nil
+	return l.awsPVLabeler, nil
 }
 
 func (l *persistentVolumeLabel) findGCEPDLabels(volume *api.PersistentVolume) (map[string]string, error) {
@@ -235,72 +237,73 @@ func (l *persistentVolumeLabel) findGCEPDLabels(volume *api.PersistentVolume) (m
 		return nil, nil
 	}
 
-	provider, err := l.getGCECloudProvider()
+	pvlabler, err := l.getGCEPVLabeler()
 	if err != nil {
 		return nil, err
 	}
-	if provider == nil {
+	if pvlabler == nil {
 		return nil, fmt.Errorf("unable to build GCE cloud provider for PD")
 	}
 
-	// If the zone is already labeled, honor the hint
-	zone := volume.Labels[kubeletapis.LabelZoneFailureDomain]
-
-	labels, err := provider.GetAutoLabelsForPD(volume.Spec.GCEPersistentDisk.PDName, zone)
+	pv := &v1.PersistentVolume{}
+	err = k8s_api_v1.Convert_core_PersistentVolume_To_v1_PersistentVolume(volume, pv, nil)
 	if err != nil {
-		return nil, err
+		return nil, fmt.Errorf("failed to convert PersistentVolume to core/v1: %q", err)
 	}
-
-	return labels, nil
+	return pvlabler.GetLabelsForVolume(context.TODO(), pv)
 }
 
-// getGCECloudProvider returns the GCE cloud provider, for use for querying volume labels
-func (l *persistentVolumeLabel) getGCECloudProvider() (*gce.Cloud, error) {
+// getGCEPVLabeler returns the GCE implementation of PVLabeler
+func (l *persistentVolumeLabel) getGCEPVLabeler() (cloudprovider.PVLabeler, error) {
 	l.mutex.Lock()
 	defer l.mutex.Unlock()
 
-	if l.gceCloudProvider == nil {
+	if l.gcePVLabeler == nil {
 		var cloudConfigReader io.Reader
 		if len(l.cloudConfig) > 0 {
 			cloudConfigReader = bytes.NewReader(l.cloudConfig)
 		}
+
 		cloudProvider, err := cloudprovider.GetCloudProvider("gce", cloudConfigReader)
 		if err != nil || cloudProvider == nil {
 			return nil, err
 		}
-		gceCloudProvider, ok := cloudProvider.(*gce.Cloud)
+
+		gcePVLabeler, ok := cloudProvider.(cloudprovider.PVLabeler)
 		if !ok {
-			// GetCloudProvider has gone very wrong
-			return nil, fmt.Errorf("error retrieving GCE cloud provider")
+			return nil, errors.New("GCE cloud provider does not implement PV labeling")
 		}
-		l.gceCloudProvider = gceCloudProvider
+
+		l.gcePVLabeler = gcePVLabeler
+
 	}
-	return l.gceCloudProvider, nil
+	return l.gcePVLabeler, nil
 }
 
-// getAzureCloudProvider returns the Azure cloud provider, for use for querying volume labels
-func (l *persistentVolumeLabel) getAzureCloudProvider() (*azure.Cloud, error) {
+// getAzurePVLabeler returns the Azure implementation of PVLabeler
+func (l *persistentVolumeLabel) getAzurePVLabeler() (cloudprovider.PVLabeler, error) {
 	l.mutex.Lock()
 	defer l.mutex.Unlock()
 
-	if l.azureProvider == nil {
+	if l.azurePVLabeler == nil {
 		var cloudConfigReader io.Reader
 		if len(l.cloudConfig) > 0 {
 			cloudConfigReader = bytes.NewReader(l.cloudConfig)
 		}
+
 		cloudProvider, err := cloudprovider.GetCloudProvider("azure", cloudConfigReader)
 		if err != nil || cloudProvider == nil {
 			return nil, err
 		}
-		azureProvider, ok := cloudProvider.(*azure.Cloud)
+
+		azurePVLabeler, ok := cloudProvider.(cloudprovider.PVLabeler)
 		if !ok {
-			// GetCloudProvider has gone very wrong
-			return nil, fmt.Errorf("error retrieving Azure cloud provider")
+			return nil, errors.New("Azure cloud provider does not implement PV labeling")
 		}
-		l.azureProvider = azureProvider
+		l.azurePVLabeler = azurePVLabeler
 	}
 
-	return l.azureProvider, nil
+	return l.azurePVLabeler, nil
 }
 
 func (l *persistentVolumeLabel) findAzureDiskLabels(volume *api.PersistentVolume) (map[string]string, error) {
@@ -309,13 +312,18 @@ func (l *persistentVolumeLabel) findAzureDiskLabels(volume *api.PersistentVolume
 		return nil, nil
 	}
 
-	provider, err := l.getAzureCloudProvider()
+	pvlabler, err := l.getAzurePVLabeler()
 	if err != nil {
 		return nil, err
 	}
-	if provider == nil {
+	if pvlabler == nil {
 		return nil, fmt.Errorf("unable to build Azure cloud provider for AzureDisk")
 	}
 
-	return provider.GetAzureDiskLabels(volume.Spec.AzureDisk.DataDiskURI)
+	pv := &v1.PersistentVolume{}
+	err = k8s_api_v1.Convert_core_PersistentVolume_To_v1_PersistentVolume(volume, pv, nil)
+	if err != nil {
+		return nil, fmt.Errorf("failed to convert PersistentVolume to core/v1: %q", err)
+	}
+	return pvlabler.GetLabelsForVolume(context.TODO(), pv)
 }

plugin/pkg/admission/storage/persistentvolume/label/admission_test.go

@@ -17,18 +17,17 @@ limitations under the License.
 package label
 
 import (
-	"testing"
-
+	"context"
 	"fmt"
 	"reflect"
 	"sort"
+	"testing"
 
-	"k8s.io/apimachinery/pkg/api/resource"
+	"k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
-	"k8s.io/apimachinery/pkg/types"
 	"k8s.io/apiserver/pkg/admission"
+	cloudprovider "k8s.io/cloud-provider"
 	api "k8s.io/kubernetes/pkg/apis/core"
-	"k8s.io/kubernetes/pkg/cloudprovider/providers/aws"
 	kubeletapis "k8s.io/kubernetes/pkg/kubelet/apis"
 	volumeutil "k8s.io/kubernetes/pkg/volume/util"
 )
@@ -38,47 +37,12 @@ type mockVolumes struct {
 	volumeLabelsError error
 }
 
-var _ aws.Volumes = &mockVolumes{}
+var _ cloudprovider.PVLabeler = &mockVolumes{}
 
-func (v *mockVolumes) AttachDisk(diskName aws.KubernetesVolumeID, nodeName types.NodeName) (string, error) {
-	return "", fmt.Errorf("not implemented")
-}
-
-func (v *mockVolumes) DetachDisk(diskName aws.KubernetesVolumeID, nodeName types.NodeName) (string, error) {
-	return "", fmt.Errorf("not implemented")
-}
-
-func (v *mockVolumes) CreateDisk(volumeOptions *aws.VolumeOptions) (volumeName aws.KubernetesVolumeID, err error) {
-	return "", fmt.Errorf("not implemented")
-}
-
-func (v *mockVolumes) DeleteDisk(volumeName aws.KubernetesVolumeID) (bool, error) {
-	return false, fmt.Errorf("not implemented")
-}
-
-func (v *mockVolumes) GetVolumeLabels(volumeName aws.KubernetesVolumeID) (map[string]string, error) {
+func (v *mockVolumes) GetLabelsForVolume(ctx context.Context, pv *v1.PersistentVolume) (map[string]string, error) {
 	return v.volumeLabels, v.volumeLabelsError
 }
 
-func (v *mockVolumes) GetDiskPath(volumeName aws.KubernetesVolumeID) (string, error) {
-	return "", fmt.Errorf("not implemented")
-}
-
-func (v *mockVolumes) DiskIsAttached(volumeName aws.KubernetesVolumeID, nodeName types.NodeName) (bool, error) {
-	return false, fmt.Errorf("not implemented")
-}
-
-func (v *mockVolumes) DisksAreAttached(nodeDisks map[types.NodeName][]aws.KubernetesVolumeID) (map[types.NodeName]map[aws.KubernetesVolumeID]bool, error) {
-	return nil, fmt.Errorf("not implemented")
-}
-
-func (v *mockVolumes) ResizeDisk(
-	diskName aws.KubernetesVolumeID,
-	oldSize resource.Quantity,
-	newSize resource.Quantity) (resource.Quantity, error) {
-	return oldSize, nil
-}
-
 func mockVolumeFailure(err error) *mockVolumes {
 	return &mockVolumes{volumeLabelsError: err}
 }
@@ -135,7 +99,7 @@ func TestAdmission(t *testing.T) {
 	}
 
 	// Errors from the cloudprovider block creation of the volume
-	pvHandler.ebsVolumes = mockVolumeFailure(fmt.Errorf("invalid volume"))
+	pvHandler.awsPVLabeler = mockVolumeFailure(fmt.Errorf("invalid volume"))
 	err = handler.Admit(admission.NewAttributesRecord(&awsPV, nil, api.Kind("PersistentVolume").WithVersion("version"), awsPV.Namespace, awsPV.Name, api.Resource("persistentvolumes").WithVersion("version"), "", admission.Create, false, nil))
 	if err == nil {
 		t.Errorf("Expected error when aws pv info fails")
@@ -143,7 +107,7 @@ func TestAdmission(t *testing.T) {
 
 	// Don't add labels if the cloudprovider doesn't return any
 	labels := make(map[string]string)
-	pvHandler.ebsVolumes = mockVolumeLabels(labels)
+	pvHandler.awsPVLabeler = mockVolumeLabels(labels)
 	err = handler.Admit(admission.NewAttributesRecord(&awsPV, nil, api.Kind("PersistentVolume").WithVersion("version"), awsPV.Namespace, awsPV.Name, api.Resource("persistentvolumes").WithVersion("version"), "", admission.Create, false, nil))
 	if err != nil {
 		t.Errorf("Expected no error when creating aws pv")
@@ -156,7 +120,7 @@ func TestAdmission(t *testing.T) {
 	}
 
 	// Don't panic if the cloudprovider returns nil, nil
-	pvHandler.ebsVolumes = mockVolumeFailure(nil)
+	pvHandler.awsPVLabeler = mockVolumeFailure(nil)
 	err = handler.Admit(admission.NewAttributesRecord(&awsPV, nil, api.Kind("PersistentVolume").WithVersion("version"), awsPV.Namespace, awsPV.Name, api.Resource("persistentvolumes").WithVersion("version"), "", admission.Create, false, nil))
 	if err != nil {
 		t.Errorf("Expected no error when cloud provider returns empty labels")
@@ -168,7 +132,7 @@ func TestAdmission(t *testing.T) {
 	labels["b"] = "2"
 	zones, _ := volumeutil.ZonesToSet("1,2,3")
 	labels[kubeletapis.LabelZoneFailureDomain] = volumeutil.ZonesSetToLabelValue(zones)
-	pvHandler.ebsVolumes = mockVolumeLabels(labels)
+	pvHandler.awsPVLabeler = mockVolumeLabels(labels)
 	err = handler.Admit(admission.NewAttributesRecord(&awsPV, nil, api.Kind("PersistentVolume").WithVersion("version"), awsPV.Namespace, awsPV.Name, api.Resource("persistentvolumes").WithVersion("version"), "", admission.Create, false, nil))
 	if err != nil {
 		t.Errorf("Expected no error when creating aws pv")
@@ -223,7 +187,7 @@ func TestAdmission(t *testing.T) {
 	labels["a"] = "1"
 	labels["b"] = "2"
 	labels["c"] = "3"
-	pvHandler.ebsVolumes = mockVolumeLabels(labels)
+	pvHandler.awsPVLabeler = mockVolumeLabels(labels)
 	err = handler.Admit(admission.NewAttributesRecord(&awsPV, nil, api.Kind("PersistentVolume").WithVersion("version"), awsPV.Namespace, awsPV.Name, api.Resource("persistentvolumes").WithVersion("version"), "", admission.Create, false, nil))
 	if err != nil {
 		t.Errorf("Expected no error when creating aws pv")
@@ -244,7 +208,7 @@ func TestAdmission(t *testing.T) {
 	labels["e"] = "1"
 	labels["f"] = "2"
 	labels["g"] = "3"
-	pvHandler.ebsVolumes = mockVolumeLabels(labels)
+	pvHandler.awsPVLabeler = mockVolumeLabels(labels)
 	err = handler.Admit(admission.NewAttributesRecord(&awsPV, nil, api.Kind("PersistentVolume").WithVersion("version"), awsPV.Namespace, awsPV.Name, api.Resource("persistentvolumes").WithVersion("version"), "", admission.Create, false, nil))
 	if err != nil {
 		t.Errorf("Expected no error when creating aws pv")