From 32b69193c63ff54fc5b77225d57616963b31932e Mon Sep 17 00:00:00 2001 From: Hemant Kumar Date: Mon, 4 Jun 2018 14:16:58 -0400 Subject: [PATCH] Fix panic caused by no cloudprovider in test We should not panic when no cloudprovider is present --- pkg/kubelet/kubelet_node_status_test.go | 50 +++++++++++++++++-------- pkg/volume/aws_ebs/aws_ebs.go | 14 +++++-- pkg/volume/azure_dd/azure_dd.go | 16 ++++++-- pkg/volume/gce_pd/gce_pd.go | 14 +++++-- 4 files changed, 70 insertions(+), 24 deletions(-) diff --git a/pkg/kubelet/kubelet_node_status_test.go b/pkg/kubelet/kubelet_node_status_test.go index d711576428..04f22b1b18 100644 --- a/pkg/kubelet/kubelet_node_status_test.go +++ b/pkg/kubelet/kubelet_node_status_test.go @@ -1648,6 +1648,12 @@ func TestSetVolumeLimits(t *testing.T) { expectedVolumeKey: util.AzureVolumeLimitKey, expectedLimit: 16, }, + { + name: "when no cloudprovider is present", + cloudProviderName: "", + expectedVolumeKey: util.AzureVolumeLimitKey, + expectedLimit: -1, + }, } for _, test := range testcases { node := &v1.Node{ @@ -1655,28 +1661,42 @@ func TestSetVolumeLimits(t *testing.T) { Spec: v1.NodeSpec{}, } - fakeCloud := &fakecloud.FakeCloud{ - Provider: test.cloudProviderName, - Err: nil, + if test.cloudProviderName != "" { + fakeCloud := &fakecloud.FakeCloud{ + Provider: test.cloudProviderName, + Err: nil, + } + kubelet.cloud = fakeCloud + kubelet.cloudproviderRequestParallelism = make(chan int, 1) + kubelet.cloudproviderRequestSync = make(chan int) + kubelet.cloudproviderRequestTimeout = 10 * time.Second + } else { + kubelet.cloud = nil } - kubelet.cloud = fakeCloud - kubelet.cloudproviderRequestParallelism = make(chan int, 1) - kubelet.cloudproviderRequestSync = make(chan int) - kubelet.cloudproviderRequestTimeout = 10 * time.Second + kubelet.setVolumeLimits(node) nodeLimits := []v1.ResourceList{} nodeLimits = append(nodeLimits, node.Status.Allocatable) nodeLimits = append(nodeLimits, node.Status.Capacity) for _, volumeLimits := range nodeLimits { - fl, ok := volumeLimits[v1.ResourceName(test.expectedVolumeKey)] - if !ok { - t.Errorf("Expected to found volume limit for %s found none", test.expectedVolumeKey) - } - foundLimit, _ := fl.AsInt64() - expectedValue := resource.NewQuantity(test.expectedLimit, resource.DecimalSI) - if expectedValue.Cmp(fl) != 0 { - t.Errorf("Expected volume limit for %s to be %v found %v", test.expectedVolumeKey, test.expectedLimit, foundLimit) + if test.expectedLimit == -1 { + _, ok := volumeLimits[v1.ResourceName(test.expectedVolumeKey)] + if ok { + t.Errorf("Expected no volume limit found for %s", test.expectedVolumeKey) + } + } else { + fl, ok := volumeLimits[v1.ResourceName(test.expectedVolumeKey)] + + if !ok { + t.Errorf("Expected to found volume limit for %s found none", test.expectedVolumeKey) + } + foundLimit, _ := fl.AsInt64() + expectedValue := resource.NewQuantity(test.expectedLimit, resource.DecimalSI) + if expectedValue.Cmp(fl) != 0 { + t.Errorf("Expected volume limit for %s to be %v found %v", test.expectedVolumeKey, test.expectedLimit, foundLimit) + } } + } } diff --git a/pkg/volume/aws_ebs/aws_ebs.go b/pkg/volume/aws_ebs/aws_ebs.go index d91ace3041..1afa28e291 100644 --- a/pkg/volume/aws_ebs/aws_ebs.go +++ b/pkg/volume/aws_ebs/aws_ebs.go @@ -98,15 +98,23 @@ func (plugin *awsElasticBlockStorePlugin) SupportsBulkVolumeVerification() bool } func (plugin *awsElasticBlockStorePlugin) GetVolumeLimits() (map[string]int64, error) { + volumeLimits := map[string]int64{ + util.EBSVolumeLimitKey: 39, + } cloud := plugin.host.GetCloudProvider() + // if we can't fetch cloudprovider we return an error + // hoping external CCM or admin can set it. Returning + // default values from here will mean, no one can + // override them. + if cloud == nil { + return nil, fmt.Errorf("No cloudprovider present") + } + if cloud.ProviderName() != aws.ProviderName { return nil, fmt.Errorf("Expected aws cloud, found %s", cloud.ProviderName()) } - volumeLimits := map[string]int64{ - util.EBSVolumeLimitKey: 39, - } instances, ok := cloud.Instances() if !ok { glog.V(3).Infof("Failed to get instances from cloud provider") diff --git a/pkg/volume/azure_dd/azure_dd.go b/pkg/volume/azure_dd/azure_dd.go index 11aba03a13..412ca3bba1 100644 --- a/pkg/volume/azure_dd/azure_dd.go +++ b/pkg/volume/azure_dd/azure_dd.go @@ -112,14 +112,24 @@ func (plugin *azureDataDiskPlugin) SupportsBulkVolumeVerification() bool { } func (plugin *azureDataDiskPlugin) GetVolumeLimits() (map[string]int64, error) { + volumeLimits := map[string]int64{ + util.AzureVolumeLimitKey: 16, + } + cloud := plugin.host.GetCloudProvider() + + // if we can't fetch cloudprovider we return an error + // hoping external CCM or admin can set it. Returning + // default values from here will mean, no one can + // override them. + if cloud == nil { + return nil, fmt.Errorf("No cloudprovider present") + } + if cloud.ProviderName() != azure.CloudProviderName { return nil, fmt.Errorf("Expected Azure cloudprovider, got %s", cloud.ProviderName()) } - volumeLimits := map[string]int64{ - util.AzureVolumeLimitKey: 16, - } return volumeLimits, nil } diff --git a/pkg/volume/gce_pd/gce_pd.go b/pkg/volume/gce_pd/gce_pd.go index 01b34628d6..d4f5878479 100644 --- a/pkg/volume/gce_pd/gce_pd.go +++ b/pkg/volume/gce_pd/gce_pd.go @@ -103,15 +103,23 @@ func (plugin *gcePersistentDiskPlugin) GetAccessModes() []v1.PersistentVolumeAcc } func (plugin *gcePersistentDiskPlugin) GetVolumeLimits() (map[string]int64, error) { + volumeLimits := map[string]int64{ + util.GCEVolumeLimitKey: 16, + } cloud := plugin.host.GetCloudProvider() + // if we can't fetch cloudprovider we return an error + // hoping external CCM or admin can set it. Returning + // default values from here will mean, no one can + // override them. + if cloud == nil { + return nil, fmt.Errorf("No cloudprovider present") + } + if cloud.ProviderName() != gcecloud.ProviderName { return nil, fmt.Errorf("Expected gce cloud got %s", cloud.ProviderName()) } - volumeLimits := map[string]int64{ - util.GCEVolumeLimitKey: 16, - } return volumeLimits, nil }