Skip to content

K
k3s
Commit 02e0b29b authored by k8s-merge-robot's avatar k8s-merge-robot
Browse files
Options

Merge pull request #23769 from saad-ali/fixVolumeCloudProvider

Automatic merge from submit-queue Ensure object returned by volume getCloudProvider incorporates cloud config This PR addresses https://github.com/kubernetes/kubernetes/issues/23517. **Problem** The existing GCE PD and AWS EBS volume plugin code were fetching cloud provider without specifying a cloud config: `cloudprovider.GetCloudProvider("gce", nil)` This caused the cloud provider to use default auth mechanism, which is not acceptable for the provisioning controller running on GKE master. **Fix** This PR does the following: * Modifies the GCE PD and AWS EBS volume plugin code to use the cloud provider object pre-constructed by the binary with a cloud config. * Enable provisioning E2E test for GKE (to catch future issues). Thanks to @cjcullen for debugging and finding the root cause! 👍 This should be cherry-picked into the v1.2 branch for the next release.
parents fd192309 e7b14e72
Hide whitespace changes
Inline Side-by-side
Showing with 35 additions and 23 deletions
pkg/volume/aws_ebs/aws_util.go
View file @ 02e0b29b
......@@ -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.")
}
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)
}
// The conversion must be safe otherwise bug in GetCloudProvider()
return awsCloudProvider.(*aws.AWSCloud), nil
return awsCloudProvider, nil
}
pkg/volume/gce_pd/gce_util.go
View file @ 02e0b29b
......@@ -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.")
}
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)
}
// The conversion must be safe otherwise bug in GetCloudProvider()
return gceCloudProvider.(*gcecloud.GCECloud), nil
return gceCloudProvider, nil
}
// Calls "udevadm trigger --action=change" for newly created "/dev/sd*" drives (exist only in after set).
......
test/e2e/volume_provisioning.go
View file @ 02e0b29b
......@@ -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() {
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment