From e7b14e721a6046a695220b58d31903febcefff91 Mon Sep 17 00:00:00 2001 From: saadali Date: Fri, 1 Apr 2016 16:40:36 -0700 Subject: [PATCH] Ensure volume GetCloudProvider code uses cloud config --- pkg/volume/aws_ebs/aws_util.go | 28 +++++++++++++++++----------- pkg/volume/gce_pd/gce_util.go | 28 +++++++++++++++++----------- test/e2e/volume_provisioning.go | 2 +- 3 files changed, 35 insertions(+), 23 deletions(-) diff --git a/pkg/volume/aws_ebs/aws_util.go b/pkg/volume/aws_ebs/aws_util.go index f2fc90c52d9..1c7727eafeb 100644 --- a/pkg/volume/aws_ebs/aws_util.go +++ b/pkg/volume/aws_ebs/aws_util.go @@ -23,7 +23,6 @@ import ( "time" "github.com/golang/glog" - "k8s.io/kubernetes/pkg/cloudprovider" "k8s.io/kubernetes/pkg/cloudprovider/providers/aws" "k8s.io/kubernetes/pkg/util/keymutex" "k8s.io/kubernetes/pkg/util/runtime" @@ -108,7 +107,7 @@ func (util *AWSDiskUtil) DetachDisk(c *awsElasticBlockStoreUnmounter) error { } func (util *AWSDiskUtil) DeleteVolume(d *awsElasticBlockStoreDeleter) error { - cloud, err := getCloudProvider() + cloud, err := getCloudProvider(d.awsElasticBlockStore.plugin) if err != nil { return err } @@ -129,7 +128,7 @@ func (util *AWSDiskUtil) DeleteVolume(d *awsElasticBlockStoreDeleter) error { // CreateVolume creates an AWS EBS volume. // Returns: volumeID, volumeSizeGB, labels, error func (util *AWSDiskUtil) CreateVolume(c *awsElasticBlockStoreProvisioner) (string, int, map[string]string, error) { - cloud, err := getCloudProvider() + cloud, err := getCloudProvider(c.awsElasticBlockStore.plugin) if err != nil { return "", 0, nil, err } @@ -175,7 +174,7 @@ func attachDiskAndVerify(b *awsElasticBlockStoreMounter, xvdBeforeSet sets.Strin for numRetries := 0; numRetries < maxRetries; numRetries++ { var err error if awsCloud == nil { - awsCloud, err = getCloudProvider() + awsCloud, err = getCloudProvider(b.awsElasticBlockStore.plugin) if err != nil || awsCloud == nil { // Retry on error. See issue #11321 glog.Errorf("Error getting AWSCloudProvider while detaching PD %q: %v", b.volumeID, err) @@ -250,7 +249,7 @@ func detachDiskAndVerify(c *awsElasticBlockStoreUnmounter) { for numRetries := 0; numRetries < maxRetries; numRetries++ { var err error if awsCloud == nil { - awsCloud, err = getCloudProvider() + awsCloud, err = getCloudProvider(c.awsElasticBlockStore.plugin) if err != nil || awsCloud == nil { // Retry on error. See issue #11321 glog.Errorf("Error getting AWSCloudProvider while detaching PD %q: %v", c.volumeID, err) @@ -348,12 +347,19 @@ func pathExists(path string) (bool, error) { } // Return cloud provider -func getCloudProvider() (*aws.AWSCloud, error) { - awsCloudProvider, err := cloudprovider.GetCloudProvider("aws", nil) - if err != nil || awsCloudProvider == nil { - return nil, err +func getCloudProvider(plugin *awsElasticBlockStorePlugin) (*aws.AWSCloud, error) { + if plugin == nil { + return nil, fmt.Errorf("Failed to get AWS Cloud Provider. plugin object is nil.") + } + if plugin.host == nil { + return nil, fmt.Errorf("Failed to get AWS Cloud Provider. plugin.host object is nil.") } - // The conversion must be safe otherwise bug in GetCloudProvider() - return awsCloudProvider.(*aws.AWSCloud), nil + cloudProvider := plugin.host.GetCloudProvider() + awsCloudProvider, ok := cloudProvider.(*aws.AWSCloud) + if !ok || awsCloudProvider == nil { + return nil, fmt.Errorf("Failed to get AWS Cloud Provider. plugin.host.GetCloudProvider returned %v instead", cloudProvider) + } + + return awsCloudProvider, nil } diff --git a/pkg/volume/gce_pd/gce_util.go b/pkg/volume/gce_pd/gce_util.go index a55b204db44..4b7c982f8f1 100644 --- a/pkg/volume/gce_pd/gce_util.go +++ b/pkg/volume/gce_pd/gce_util.go @@ -25,7 +25,6 @@ import ( "time" "github.com/golang/glog" - "k8s.io/kubernetes/pkg/cloudprovider" gcecloud "k8s.io/kubernetes/pkg/cloudprovider/providers/gce" "k8s.io/kubernetes/pkg/util/exec" "k8s.io/kubernetes/pkg/util/keymutex" @@ -114,7 +113,7 @@ func (util *GCEDiskUtil) DetachDisk(c *gcePersistentDiskUnmounter) error { } func (util *GCEDiskUtil) DeleteVolume(d *gcePersistentDiskDeleter) error { - cloud, err := getCloudProvider() + cloud, err := getCloudProvider(d.gcePersistentDisk.plugin) if err != nil { return err } @@ -130,7 +129,7 @@ func (util *GCEDiskUtil) DeleteVolume(d *gcePersistentDiskDeleter) error { // CreateVolume creates a GCE PD. // Returns: volumeID, volumeSizeGB, labels, error func (gceutil *GCEDiskUtil) CreateVolume(c *gcePersistentDiskProvisioner) (string, int, map[string]string, error) { - cloud, err := getCloudProvider() + cloud, err := getCloudProvider(c.gcePersistentDisk.plugin) if err != nil { return "", 0, nil, err } @@ -171,7 +170,7 @@ func attachDiskAndVerify(b *gcePersistentDiskMounter, sdBeforeSet sets.String) ( for numRetries := 0; numRetries < maxRetries; numRetries++ { var err error if gceCloud == nil { - gceCloud, err = getCloudProvider() + gceCloud, err = getCloudProvider(b.gcePersistentDisk.plugin) if err != nil || gceCloud == nil { // Retry on error. See issue #11321 glog.Errorf("Error getting GCECloudProvider while detaching PD %q: %v", b.pdName, err) @@ -245,7 +244,7 @@ func detachDiskAndVerify(c *gcePersistentDiskUnmounter) { for numRetries := 0; numRetries < maxRetries; numRetries++ { var err error if gceCloud == nil { - gceCloud, err = getCloudProvider() + gceCloud, err = getCloudProvider(c.gcePersistentDisk.plugin) if err != nil || gceCloud == nil { // Retry on error. See issue #11321 glog.Errorf("Error getting GCECloudProvider while detaching PD %q: %v", c.pdName, err) @@ -342,14 +341,21 @@ func pathExists(path string) (bool, error) { } // Return cloud provider -func getCloudProvider() (*gcecloud.GCECloud, error) { - gceCloudProvider, err := cloudprovider.GetCloudProvider("gce", nil) - if err != nil || gceCloudProvider == nil { - return nil, err +func getCloudProvider(plugin *gcePersistentDiskPlugin) (*gcecloud.GCECloud, error) { + if plugin == nil { + return nil, fmt.Errorf("Failed to get GCE Cloud Provider. plugin object is nil.") + } + if plugin.host == nil { + return nil, fmt.Errorf("Failed to get GCE Cloud Provider. plugin.host object is nil.") } - // The conversion must be safe otherwise bug in GetCloudProvider() - return gceCloudProvider.(*gcecloud.GCECloud), nil + cloudProvider := plugin.host.GetCloudProvider() + gceCloudProvider, ok := cloudProvider.(*gcecloud.GCECloud) + if !ok || gceCloudProvider == nil { + return nil, fmt.Errorf("Failed to get GCE Cloud Provider. plugin.host.GetCloudProvider returned %v instead", cloudProvider) + } + + return gceCloudProvider, nil } // Calls "udevadm trigger --action=change" for newly created "/dev/sd*" drives (exist only in after set). diff --git a/test/e2e/volume_provisioning.go b/test/e2e/volume_provisioning.go index 38246be0075..f6258ed906d 100644 --- a/test/e2e/volume_provisioning.go +++ b/test/e2e/volume_provisioning.go @@ -50,7 +50,7 @@ var _ = KubeDescribe("Dynamic provisioning", func() { KubeDescribe("DynamicProvisioner", func() { It("should create and delete persistent volumes", func() { - SkipUnlessProviderIs("openstack", "gce", "aws") + SkipUnlessProviderIs("openstack", "gce", "aws", "gke") By("creating a claim with a dynamic provisioning annotation") claim := createClaim(ns) defer func() {