Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: ignore agentpool label when looking for similar node groups with Azure provider #2171

Merged
merged 3 commits into from
Jul 23, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,8 @@ package alicloud

import (
"fmt"
"strings"

"os"
"strings"

apiv1 "k8s.io/api/core/v1"
"k8s.io/apimachinery/pkg/api/resource"
Expand All @@ -32,9 +31,6 @@ import (
)

const (
// ProviderName is the cloud provider name for alicloud
ProviderName = "alicloud"

// GPULabel is the label added to nodes with GPU resource.
GPULabel = "aliyun.accelerator/nvidia_name"
)
Expand Down Expand Up @@ -100,7 +96,7 @@ func (ali *aliCloudProvider) addAsg(asg *Asg) {
}

func (ali *aliCloudProvider) Name() string {
return ProviderName
return cloudprovider.AlicloudProviderName
}

// GPULabel returns the label added to nodes with GPU resource.
Expand Down
5 changes: 1 addition & 4 deletions cluster-autoscaler/cloudprovider/aws/aws_cloud_provider.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,9 +33,6 @@ import (
)

const (
// ProviderName is the cloud provider name for AWS
ProviderName = "aws"

// GPULabel is the label added to nodes with GPU resource.
GPULabel = "k8s.amazonaws.com/accelerator"
)
Expand Down Expand Up @@ -71,7 +68,7 @@ func (aws *awsCloudProvider) Cleanup() error {

// Name returns name of the cloud provider.
func (aws *awsCloudProvider) Name() string {
return ProviderName
return cloudprovider.AwsProviderName
}

// GPULabel returns the label added to nodes with GPU resource.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,7 @@ func TestBuildAwsCloudProvider(t *testing.T) {

func TestName(t *testing.T) {
provider := testProvider(t, testAwsManager)
assert.Equal(t, provider.Name(), ProviderName)
assert.Equal(t, provider.Name(), cloudprovider.AwsProviderName)
}

func TestNodeGroups(t *testing.T) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,19 +20,15 @@ import (
"io"
"os"

"k8s.io/klog"

apiv1 "k8s.io/api/core/v1"
"k8s.io/apimachinery/pkg/api/resource"
"k8s.io/autoscaler/cluster-autoscaler/cloudprovider"
"k8s.io/autoscaler/cluster-autoscaler/config"
"k8s.io/autoscaler/cluster-autoscaler/utils/errors"
"k8s.io/klog"
)

const (
// ProviderName is the cloud provider name for Azure
ProviderName = "azure"

// GPULabel is the label added to nodes with GPU resource.
GPULabel = "cloud.google.com/gke-accelerator"
)
Expand Down
3 changes: 2 additions & 1 deletion cluster-autoscaler/cloudprovider/azure/azure_util.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,11 +34,12 @@ import (
azStorage "github.com/Azure/azure-sdk-for-go/storage"
"github.com/Azure/go-autorest/autorest"
"github.com/Azure/go-autorest/autorest/to"

"golang.org/x/crypto/pkcs12"
"k8s.io/klog"

"k8s.io/autoscaler/cluster-autoscaler/cloudprovider"
"k8s.io/client-go/pkg/version"
"k8s.io/klog"
)

const (
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,9 +33,6 @@ import (
)

const (
// ProviderName is the cloud provider name for baiducloud
ProviderName = "baiducloud"

// GPULabel is the label added to nodes with GPU resource.
GPULabel = "cloud.google.com/gke-accelerator"
)
Expand Down Expand Up @@ -147,7 +144,7 @@ func (baiducloud *baiducloudCloudProvider) addAsg(asg *Asg) {

// Name returns name of the cloud provider.
func (baiducloud *baiducloudCloudProvider) Name() string {
return ProviderName
return cloudprovider.BaiducloudProviderName
}

// NodeGroups returns all node groups configured for this cloud provider.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ func TestBuildAwsCloudProvider(t *testing.T) {

func TestName(t *testing.T) {
provider := testProvider(t, testBaiducloudManager)
assert.Equal(t, provider.Name(), ProviderName)
assert.Equal(t, provider.Name(), cloudprovider.BaiducloudProviderName)
}

func TestNodeGroups(t *testing.T) {
Expand Down
6 changes: 3 additions & 3 deletions cluster-autoscaler/cloudprovider/builder/builder_alicloud.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,15 +26,15 @@ import (

// AvailableCloudProviders supported by the cloud provider builder.
var AvailableCloudProviders = []string{
alicloud.ProviderName,
cloudprovider.AlicloudProviderName,
}

// DefaultCloudProvider for alicloud-only build is alicloud.
const DefaultCloudProvider = alicloud.ProviderName
const DefaultCloudProvider = cloudprovider.AlicloudProviderName

func buildCloudProvider(opts config.AutoscalingOptions, do cloudprovider.NodeGroupDiscoveryOptions, rl *cloudprovider.ResourceLimiter) cloudprovider.CloudProvider {
switch opts.CloudProviderName {
case alicloud.ProviderName:
case cloudprovider.AlicloudProviderName:
return alicloud.BuildAlicloud(opts, do, rl)
}

Expand Down
26 changes: 13 additions & 13 deletions cluster-autoscaler/cloudprovider/builder/builder_all.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,30 +31,30 @@ import (

// AvailableCloudProviders supported by the cloud provider builder.
var AvailableCloudProviders = []string{
aws.ProviderName,
azure.ProviderName,
gce.ProviderNameGCE,
alicloud.ProviderName,
baiducloud.ProviderName,
magnum.ProviderName,
cloudprovider.AwsProviderName,
cloudprovider.AzureProviderName,
cloudprovider.GceProviderName,
cloudprovider.AlicloudProviderName,
cloudprovider.BaiducloudProviderName,
cloudprovider.MagnumProviderName,
}

// DefaultCloudProvider is GCE.
const DefaultCloudProvider = gce.ProviderNameGCE
const DefaultCloudProvider = cloudprovider.GceProviderName

func buildCloudProvider(opts config.AutoscalingOptions, do cloudprovider.NodeGroupDiscoveryOptions, rl *cloudprovider.ResourceLimiter) cloudprovider.CloudProvider {
switch opts.CloudProviderName {
case gce.ProviderNameGCE:
case cloudprovider.GceProviderName:
return gce.BuildGCE(opts, do, rl)
case aws.ProviderName:
case cloudprovider.AwsProviderName:
return aws.BuildAWS(opts, do, rl)
case azure.ProviderName:
case cloudprovider.AzureProviderName:
return azure.BuildAzure(opts, do, rl)
case alicloud.ProviderName:
case cloudprovider.AlicloudProviderName:
return alicloud.BuildAlicloud(opts, do, rl)
case baiducloud.ProviderName:
case cloudprovider.BaiducloudProviderName:
return baiducloud.BuildBaiducloud(opts, do, rl)
case magnum.ProviderName:
case cloudprovider.MagnumProviderName:
return magnum.BuildMagnum(opts, do, rl)
}
return nil
Expand Down
6 changes: 3 additions & 3 deletions cluster-autoscaler/cloudprovider/builder/builder_aws.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,15 +26,15 @@ import (

// AvailableCloudProviders supported by the cloud provider builder.
var AvailableCloudProviders = []string{
aws.ProviderName,
cloudprovider.AwsProviderName,
}

// DefaultCloudProvider for AWS-only build is AWS.
const DefaultCloudProvider = aws.ProviderName
const DefaultCloudProvider = cloudprovider.AwsProviderName

func buildCloudProvider(opts config.AutoscalingOptions, do cloudprovider.NodeGroupDiscoveryOptions, rl *cloudprovider.ResourceLimiter) cloudprovider.CloudProvider {
switch opts.CloudProviderName {
case aws.ProviderName:
case cloudprovider.AwsProviderName:
return aws.BuildAWS(opts, do, rl)
}

Expand Down
6 changes: 3 additions & 3 deletions cluster-autoscaler/cloudprovider/builder/builder_azure.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,15 +26,15 @@ import (

// AvailableCloudProviders supported by the cloud provider builder.
var AvailableCloudProviders = []string{
azure.ProviderName,
cloudprovider.AzureProviderName,
}

// DefaultCloudProvider on Azure-only build is Azure.
const DefaultCloudProvider = azure.ProviderName
const DefaultCloudProvider = cloudprovider.AzureProviderName

func buildCloudProvider(opts config.AutoscalingOptions, do cloudprovider.NodeGroupDiscoveryOptions, rl *cloudprovider.ResourceLimiter) cloudprovider.CloudProvider {
switch opts.CloudProviderName {
case azure.ProviderName:
case cloudprovider.AzureProviderName:
return azure.BuildAzure(opts, do, rl)
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,22 +20,21 @@ package builder

import (
"k8s.io/autoscaler/cluster-autoscaler/cloudprovider"
"k8s.io/autoscaler/cluster-autoscaler/cloudprovider/aws"
"k8s.io/autoscaler/cluster-autoscaler/cloudprovider/baiducloud"
"k8s.io/autoscaler/cluster-autoscaler/config"
)

// AvailableCloudProviders supported by the cloud provider builder.
var AvailableCloudProviders = []string{
baiducloud.ProviderName,
cloudprovider.BaiducloudProviderName,
}

// DefaultCloudProvider for baiducloud-only build is baiducloud.
const DefaultCloudProvider = baiducloud.ProviderName
const DefaultCloudProvider = cloudprovider.BaiducloudProviderName

func buildCloudProvider(opts config.AutoscalingOptions, do cloudprovider.NodeGroupDiscoveryOptions, rl *cloudprovider.ResourceLimiter) cloudprovider.CloudProvider {
switch opts.CloudProviderName {
case baiducloud.ProviderName:
case cloudprovider.BaiducloudProviderName:
return baiducloud.BuildBaiducloud(opts, do, rl)
}

Expand Down
6 changes: 3 additions & 3 deletions cluster-autoscaler/cloudprovider/builder/builder_gce.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,15 +26,15 @@ import (

// AvailableCloudProviders supported by the cloud provider builder.
var AvailableCloudProviders = []string{
gce.ProviderNameGCE,
cloudprovider.GceProviderName,
}

// DefaultCloudProvider is GCE.
const DefaultCloudProvider = gce.ProviderNameGCE
const DefaultCloudProvider = cloudprovider.GceProviderName

func buildCloudProvider(opts config.AutoscalingOptions, do cloudprovider.NodeGroupDiscoveryOptions, rl *cloudprovider.ResourceLimiter) cloudprovider.CloudProvider {
switch opts.CloudProviderName {
case gce.ProviderNameGCE:
case cloudprovider.GceProviderName:
return gce.BuildGCE(opts, do, rl)
}

Expand Down
6 changes: 3 additions & 3 deletions cluster-autoscaler/cloudprovider/builder/builder_kubemark.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,15 +26,15 @@ import (

// AvailableCloudProviders supported by the cloud provider builder.
var AvailableCloudProviders = []string{
kubemark.ProviderName,
cloudprovider.KubemarkProviderName,
}

// DefaultCloudProvider for Kubemark-only build is Kubemark.
const DefaultCloudProvider = kubemark.ProviderName
const DefaultCloudProvider = cloudprovider.KubemarkProviderName

func buildCloudProvider(opts config.AutoscalingOptions, do cloudprovider.NodeGroupDiscoveryOptions, rl *cloudprovider.ResourceLimiter) cloudprovider.CloudProvider {
switch opts.CloudProviderName {
case kubemark.ProviderName:
case cloudprovider.KubemarkProviderName:
return kubemark.BuildKubemark(opts, do, rl)
}

Expand Down
6 changes: 3 additions & 3 deletions cluster-autoscaler/cloudprovider/builder/builder_magnum.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,15 +26,15 @@ import (

// AvailableCloudProviders supported by the cloud provider builder.
var AvailableCloudProviders = []string{
magnum.ProviderName,
cloudprovider.MagnumProviderName,
}

// DefaultCloudProvider for OpenStack-only build is OpenStack.
const DefaultCloudProvider = magnum.ProviderName
const DefaultCloudProvider = cloudprovider.MagnumProviderName

func buildCloudProvider(opts config.AutoscalingOptions, do cloudprovider.NodeGroupDiscoveryOptions, rl *cloudprovider.ResourceLimiter) cloudprovider.CloudProvider {
switch opts.CloudProviderName {
case magnum.ProviderName:
case cloudprovider.MagnumProviderName:
return magnum.BuildMagnum(opts, do, rl)
}

Expand Down
17 changes: 17 additions & 0 deletions cluster-autoscaler/cloudprovider/cloud_provider.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,23 @@ import (
schedulernodeinfo "k8s.io/kubernetes/pkg/scheduler/nodeinfo"
)

const (
// AzureProviderName gets the provider name of azure
AzureProviderName = "azure"
// AlicloudProviderName gets the provider name of alicloud
AlicloudProviderName = "alicloud"
// AwsProviderName gets the provider name of aws
AwsProviderName = "aws"
// BaiducloudProviderName gets the provider name of baiducloud
BaiducloudProviderName = "baiducloud"
// GceProviderName gets the provider name of gce
GceProviderName = "gce"
// MagnumProviderName gets the provider name of magnum
MagnumProviderName = "magnum"
// KubemarkProviderName gets the provider name of kubemark
KubemarkProviderName = "kubemark"
)

// CloudProvider contains configuration info and functions for interacting with
// cloud provider (GCE, AWS, etc).
type CloudProvider interface {
Expand Down
5 changes: 1 addition & 4 deletions cluster-autoscaler/cloudprovider/gce/gce_cloud_provider.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,9 +32,6 @@ import (
)

const (
// ProviderNameGCE is the name of GCE cloud provider.
ProviderNameGCE = "gce"

// GPULabel is the label added to nodes with GPU resource.
GPULabel = "cloud.google.com/gke-accelerator"
)
Expand Down Expand Up @@ -69,7 +66,7 @@ func (gce *GceCloudProvider) Cleanup() error {

// Name returns name of the cloud provider.
func (gce *GceCloudProvider) Name() string {
return ProviderNameGCE
return cloudprovider.GceProviderName
}

// GPULabel returns the label added to nodes with GPU resource.
Expand Down
3 changes: 0 additions & 3 deletions cluster-autoscaler/cloudprovider/kubemark/kubemark_other.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,9 +30,6 @@ import (
)

const (
// ProviderName is the cloud provider name for kubemark
ProviderName = "kubemark"

// GPULabel is the label added to nodes with GPU resource.
GPULabel = "cloud.google.com/gke-accelerator"
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,6 @@ import (
)

const (
// ProviderName is the cloud provider name for Magnum
ProviderName = "magnum"
// GPULabel is the label added to nodes with GPU resource.
GPULabel = "cloud.google.com/gke-accelerator"
)
Expand Down Expand Up @@ -63,7 +61,7 @@ func buildMagnumCloudProvider(magnumManager magnumManager, resourceLimiter *clou

// Name returns the name of the cloud provider.
func (mcp *magnumCloudProvider) Name() string {
return ProviderName
return cloudprovider.MagnumProviderName
}

// GPULabel returns the label added to nodes with GPU resource.
Expand Down
6 changes: 4 additions & 2 deletions cluster-autoscaler/core/static_autoscaler.go
Original file line number Diff line number Diff line change
Expand Up @@ -165,10 +165,12 @@ func (a *StaticAutoscaler) cleanUpIfRequired() {
if readyNodes, err := a.ReadyNodeLister().List(); err != nil {
klog.Errorf("Failed to list ready nodes, not cleaning up taints: %v", err)
} else {
deletetaint.CleanAllToBeDeleted(readyNodes, a.AutoscalingContext.ClientSet, a.Recorder)
deletetaint.CleanAllToBeDeleted(readyNodes,
a.AutoscalingContext.ClientSet, a.Recorder)
if a.AutoscalingContext.AutoscalingOptions.MaxBulkSoftTaintCount == 0 {
// Clean old taints if soft taints handling is disabled
deletetaint.CleanAllDeletionCandidates(readyNodes, a.AutoscalingContext.ClientSet, a.Recorder)
deletetaint.CleanAllDeletionCandidates(readyNodes,
a.AutoscalingContext.ClientSet, a.Recorder)
}
}
a.initialized = true
Expand Down
Loading