From 1ad497ca255c411649df97a387509de9f15d810e Mon Sep 17 00:00:00 2001 From: Alvaro Aleman Date: Mon, 3 Jun 2019 17:34:50 +0200 Subject: [PATCH] Fixes for kube 1.11 --- .../cloudprovider/azure/azure_scale_set.go | 2 +- .../cloudprovider/builder/builder_all.go | 65 ------------------- .../builder/builder_clusterapi.go | 42 ------------ .../builder/cloud_provider_builder.go | 17 ++--- .../clusterapi/clusterapi_nodegroup.go | 20 ++---- .../clusterapi/clusterapi_nodegroup_test.go | 8 +-- .../clusterapi/clusterapi_provider.go | 9 ++- .../clusterapi/clusterapi_provider_test.go | 7 -- .../context/autoscaling_context.go | 5 +- 9 files changed, 27 insertions(+), 148 deletions(-) delete mode 100644 cluster-autoscaler/cloudprovider/builder/builder_all.go delete mode 100644 cluster-autoscaler/cloudprovider/builder/builder_clusterapi.go diff --git a/cluster-autoscaler/cloudprovider/azure/azure_scale_set.go b/cluster-autoscaler/cloudprovider/azure/azure_scale_set.go index 419592dfb83a..f56fb1fb9156 100644 --- a/cluster-autoscaler/cloudprovider/azure/azure_scale_set.go +++ b/cluster-autoscaler/cloudprovider/azure/azure_scale_set.go @@ -269,7 +269,7 @@ func (scaleSet *ScaleSet) DeleteInstances(instances []*azureRef) error { return nil } - glog.V(3).Infof("Deleting vmss instances %q", instances) + glog.V(3).Infof("Deleting vmss instances %v", instances) commonAsg, err := scaleSet.manager.GetAsgForInstance(instances[0]) if err != nil { diff --git a/cluster-autoscaler/cloudprovider/builder/builder_all.go b/cluster-autoscaler/cloudprovider/builder/builder_all.go deleted file mode 100644 index 7b1bb8aba399..000000000000 --- a/cluster-autoscaler/cloudprovider/builder/builder_all.go +++ /dev/null @@ -1,65 +0,0 @@ -// +build !gce,!aws,!azure,!kubemark,!alicloud,!clusterapi - -/* -Copyright 2018 The Kubernetes Authors. - -Licensed under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. -You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. -*/ - -package builder - -import ( - "k8s.io/autoscaler/cluster-autoscaler/cloudprovider" - "k8s.io/autoscaler/cluster-autoscaler/cloudprovider/alicloud" - "k8s.io/autoscaler/cluster-autoscaler/cloudprovider/aws" - "k8s.io/autoscaler/cluster-autoscaler/cloudprovider/azure" - "k8s.io/autoscaler/cluster-autoscaler/cloudprovider/baiducloud" - "k8s.io/autoscaler/cluster-autoscaler/cloudprovider/clusterapi" - "k8s.io/autoscaler/cluster-autoscaler/cloudprovider/gce" - "k8s.io/autoscaler/cluster-autoscaler/cloudprovider/gke" - "k8s.io/autoscaler/cluster-autoscaler/config" -) - -// AvailableCloudProviders supported by the cloud provider builder. -var AvailableCloudProviders = []string{ - aws.ProviderName, - azure.ProviderName, - gce.ProviderNameGCE, - gke.ProviderNameGKE, - alicloud.ProviderName, - baiducloud.ProviderName, - clusterapi.ProviderName, -} - -// DefaultCloudProvider is GCE. -const DefaultCloudProvider = gce.ProviderNameGCE - -func buildCloudProvider(opts config.AutoscalingOptions, do cloudprovider.NodeGroupDiscoveryOptions, rl *cloudprovider.ResourceLimiter) cloudprovider.CloudProvider { - switch opts.CloudProviderName { - case gce.ProviderNameGCE: - return gce.BuildGCE(opts, do, rl) - case gke.ProviderNameGKE: - return gke.BuildGKE(opts, do, rl) - case aws.ProviderName: - return aws.BuildAWS(opts, do, rl) - case azure.ProviderName: - return azure.BuildAzure(opts, do, rl) - case alicloud.ProviderName: - return alicloud.BuildAlicloud(opts, do, rl) - case baiducloud.ProviderName: - return baiducloud.BuildBaiducloud(opts, do, rl) - case clusterapi.ProviderName: - return clusterapi.BuildClusterAPI(opts, do, rl) - } - return nil -} diff --git a/cluster-autoscaler/cloudprovider/builder/builder_clusterapi.go b/cluster-autoscaler/cloudprovider/builder/builder_clusterapi.go deleted file mode 100644 index d7e47829e70e..000000000000 --- a/cluster-autoscaler/cloudprovider/builder/builder_clusterapi.go +++ /dev/null @@ -1,42 +0,0 @@ -// +build clusterapi - -/* -Copyright 2019 The Kubernetes Authors. - -Licensed under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. -You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. -*/ - -package builder - -import ( - "k8s.io/autoscaler/cluster-autoscaler/cloudprovider" - "k8s.io/autoscaler/cluster-autoscaler/cloudprovider/clusterapi" - "k8s.io/autoscaler/cluster-autoscaler/config" -) - -// AvailableCloudProviders supported by the cloud provider builder. -var AvailableCloudProviders = []string{ - clusterapi.ProviderName, -} - -// DefaultCloudProvider for machineapi-only build. -const DefaultCloudProvider = clusterapi.ProviderName - -func buildCloudProvider(opts config.AutoscalingOptions, do cloudprovider.NodeGroupDiscoveryOptions, rl *cloudprovider.ResourceLimiter) cloudprovider.CloudProvider { - switch opts.CloudProviderName { - case clusterapi.ProviderName: - return clusterapi.BuildClusterAPI(opts, do, rl) - } - - return nil -} diff --git a/cluster-autoscaler/cloudprovider/builder/cloud_provider_builder.go b/cluster-autoscaler/cloudprovider/builder/cloud_provider_builder.go index 528b7e846ff1..a132af226fe8 100644 --- a/cluster-autoscaler/cloudprovider/builder/cloud_provider_builder.go +++ b/cluster-autoscaler/cloudprovider/builder/cloud_provider_builder.go @@ -23,6 +23,7 @@ import ( "k8s.io/autoscaler/cluster-autoscaler/cloudprovider" "k8s.io/autoscaler/cluster-autoscaler/cloudprovider/aws" "k8s.io/autoscaler/cluster-autoscaler/cloudprovider/azure" + "k8s.io/autoscaler/cluster-autoscaler/cloudprovider/clusterapi" "k8s.io/autoscaler/cluster-autoscaler/cloudprovider/gce" "k8s.io/autoscaler/cluster-autoscaler/cloudprovider/kubemark" "k8s.io/client-go/informers" @@ -36,15 +37,11 @@ import ( // AvailableCloudProviders supported by the cloud provider builder. var AvailableCloudProviders = []string{ - aws.ProviderName, - azure.ProviderName, - gce.ProviderNameGCE, - gce.ProviderNameGKE, - kubemark.ProviderName, + clusterapi.ProviderName, } -// DefaultCloudProvider is GCE. -const DefaultCloudProvider = gce.ProviderNameGCE +// DefaultCloudProvider for machineapi-only build. +const DefaultCloudProvider = clusterapi.ProviderName // CloudProviderBuilder builds a cloud provider from all the necessary parameters including the name of a cloud provider e.g. aws, gce // and the path to a config file @@ -54,16 +51,18 @@ type CloudProviderBuilder struct { clusterName string autoprovisioningEnabled bool regional bool + kubeconfigPath string } // NewCloudProviderBuilder builds a new builder from static settings -func NewCloudProviderBuilder(cloudProviderFlag, cloudConfig, clusterName string, autoprovisioningEnabled, regional bool) CloudProviderBuilder { +func NewCloudProviderBuilder(kubeconfigPath, cloudProviderFlag, cloudConfig, clusterName string, autoprovisioningEnabled, regional bool) CloudProviderBuilder { return CloudProviderBuilder{ cloudProviderFlag: cloudProviderFlag, cloudConfig: cloudConfig, clusterName: clusterName, autoprovisioningEnabled: autoprovisioningEnabled, regional: regional, + kubeconfigPath: kubeconfigPath, } } @@ -87,6 +86,8 @@ func (b CloudProviderBuilder) Build(discoveryOpts cloudprovider.NodeGroupDiscove return b.buildAzure(discoveryOpts, resourceLimiter) case kubemark.ProviderName: return b.buildKubemark(discoveryOpts, resourceLimiter) + case clusterapi.ProviderName: + return clusterapi.BuildClusterAPI(b.kubeconfigPath, discoveryOpts, resourceLimiter) case "": // Ideally this would be an error, but several unit tests of the // StaticAutoscaler depend on this behaviour. diff --git a/cluster-autoscaler/cloudprovider/clusterapi/clusterapi_nodegroup.go b/cluster-autoscaler/cloudprovider/clusterapi/clusterapi_nodegroup.go index 669aa7c79df9..e6870e14476b 100644 --- a/cluster-autoscaler/cloudprovider/clusterapi/clusterapi_nodegroup.go +++ b/cluster-autoscaler/cloudprovider/clusterapi/clusterapi_nodegroup.go @@ -22,7 +22,7 @@ import ( corev1 "k8s.io/api/core/v1" "k8s.io/autoscaler/cluster-autoscaler/cloudprovider" - schedulernodeinfo "k8s.io/kubernetes/pkg/scheduler/nodeinfo" + schedulercache "k8s.io/kubernetes/pkg/scheduler/cache" "sigs.k8s.io/cluster-api/pkg/apis/cluster/v1alpha1" clusterv1alpha1 "sigs.k8s.io/cluster-api/pkg/client/clientset_generated/clientset/typed/cluster/v1alpha1" ) @@ -168,20 +168,12 @@ func (ng *nodegroup) Debug() string { } // Nodes returns a list of all nodes that belong to this node group. -func (ng *nodegroup) Nodes() ([]cloudprovider.Instance, error) { +func (ng *nodegroup) Nodes() ([]string, error) { nodes, err := ng.scalableResource.Nodes() if err != nil { return nil, err } - - instances := make([]cloudprovider.Instance, len(nodes)) - for i := range nodes { - instances[i] = cloudprovider.Instance{ - Id: nodes[i], - } - } - - return instances, nil + return nodes, nil } // TemplateNodeInfo returns a schedulercache.NodeInfo structure of an @@ -192,7 +184,7 @@ func (ng *nodegroup) Nodes() ([]cloudprovider.Instance, error) { // allocatable information as well as all pods that are started on the // node by default, using manifest (most likely only kube-proxy). // Implementation optional. -func (ng *nodegroup) TemplateNodeInfo() (*schedulernodeinfo.NodeInfo, error) { +func (ng *nodegroup) TemplateNodeInfo() (*schedulercache.NodeInfo, error) { return nil, cloudprovider.ErrNotImplemented } @@ -205,8 +197,8 @@ func (ng *nodegroup) Exist() bool { // Create creates the node group on the cloud nodegroup side. // Implementation optional. -func (ng *nodegroup) Create() (cloudprovider.NodeGroup, error) { - return nil, cloudprovider.ErrAlreadyExist +func (ng *nodegroup) Create() error { + return cloudprovider.ErrAlreadyExist } // Delete deletes the node group on the cloud nodegroup side. This will diff --git a/cluster-autoscaler/cloudprovider/clusterapi/clusterapi_nodegroup_test.go b/cluster-autoscaler/cloudprovider/clusterapi/clusterapi_nodegroup_test.go index b14d582e9b4c..3e281ead0667 100644 --- a/cluster-autoscaler/cloudprovider/clusterapi/clusterapi_nodegroup_test.go +++ b/cluster-autoscaler/cloudprovider/clusterapi/clusterapi_nodegroup_test.go @@ -175,7 +175,7 @@ func TestNodeGroupNewNodeGroupConstructor(t *testing.T) { t.Errorf("expected %t, got %t", true, exists) } - if _, err := ng.Create(); err != cloudprovider.ErrAlreadyExist { + if err := ng.Create(); err != cloudprovider.ErrAlreadyExist { t.Error("expected error") } @@ -623,12 +623,12 @@ func TestNodeGroupDeleteNodes(t *testing.T) { } sort.SliceStable(nodeNames, func(i, j int) bool { - return nodeNames[i].Id < nodeNames[j].Id + return nodeNames[i] < nodeNames[j] }) for i := 0; i < len(nodeNames); i++ { - if nodeNames[i].Id != testConfig.nodes[i].Spec.ProviderID { - t.Fatalf("expected %q, got %q", testConfig.nodes[i].Spec.ProviderID, nodeNames[i].Id) + if nodeNames[i] != testConfig.nodes[i].Spec.ProviderID { + t.Fatalf("expected %q, got %q", testConfig.nodes[i].Spec.ProviderID, nodeNames[i]) } } diff --git a/cluster-autoscaler/cloudprovider/clusterapi/clusterapi_provider.go b/cluster-autoscaler/cloudprovider/clusterapi/clusterapi_provider.go index d7db037c6862..1b5dd8a77f35 100644 --- a/cluster-autoscaler/cloudprovider/clusterapi/clusterapi_provider.go +++ b/cluster-autoscaler/cloudprovider/clusterapi/clusterapi_provider.go @@ -19,16 +19,15 @@ package clusterapi import ( "reflect" + "github.com/golang/glog" corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/resource" + clusterclientset "k8s.io/autoscaler/cluster-autoscaler/client/clusterapi/clientset/versioned" "k8s.io/autoscaler/cluster-autoscaler/cloudprovider" - "k8s.io/autoscaler/cluster-autoscaler/config" "k8s.io/autoscaler/cluster-autoscaler/utils/errors" "k8s.io/client-go/kubernetes" "k8s.io/client-go/rest" "k8s.io/client-go/tools/clientcmd" - "k8s.io/klog" - clusterclientset "sigs.k8s.io/cluster-api/pkg/client/clientset_generated/clientset" ) const ( @@ -132,11 +131,11 @@ func newProvider( } // BuildClusterAPI builds CloudProvider implementation for machine api. -func BuildClusterAPI(opts config.AutoscalingOptions, do cloudprovider.NodeGroupDiscoveryOptions, rl *cloudprovider.ResourceLimiter) cloudprovider.CloudProvider { +func BuildClusterAPI(kubeconfigPath string, do cloudprovider.NodeGroupDiscoveryOptions, rl *cloudprovider.ResourceLimiter) cloudprovider.CloudProvider { var err error var config *rest.Config - config, err = clientcmd.BuildConfigFromFlags("", opts.KubeConfigPath) + config, err = clientcmd.BuildConfigFromFlags("", kubeconfigPath) if err != nil { klog.Fatalf("cannot build config: %v", err) } diff --git a/cluster-autoscaler/cloudprovider/clusterapi/clusterapi_provider_test.go b/cluster-autoscaler/cloudprovider/clusterapi/clusterapi_provider_test.go index 507d5d27f4f3..6da48c21f581 100644 --- a/cluster-autoscaler/cloudprovider/clusterapi/clusterapi_provider_test.go +++ b/cluster-autoscaler/cloudprovider/clusterapi/clusterapi_provider_test.go @@ -96,11 +96,4 @@ func TestProviderConstructorProperties(t *testing.T) { t.Fatalf("unexpected nodegroup: %v", ng.Id()) } - if got := provider.GPULabel(); got != GPULabel { - t.Fatalf("expected %q, got %q", GPULabel, got) - } - - if got := len(provider.GetAvailableGPUTypes()); got != 0 { - t.Fatalf("expected 0 GPU types, got %d", got) - } } diff --git a/cluster-autoscaler/context/autoscaling_context.go b/cluster-autoscaler/context/autoscaling_context.go index 0a92ecf676b2..4caf88364fc0 100644 --- a/cluster-autoscaler/context/autoscaling_context.go +++ b/cluster-autoscaler/context/autoscaling_context.go @@ -143,7 +143,8 @@ type AutoscalingOptions struct { // Pods with null priority (PodPriority disabled) are non-expendable. ExpendablePodsPriorityCutoff int // Regional tells whether the cluster is regional. - Regional bool + Regional bool + KubeConfigPath string } // NewResourceLimiterFromAutoscalingOptions creates new instance of cloudprovider.ResourceLimiter @@ -170,7 +171,7 @@ func NewAutoscalingContext(options AutoscalingOptions, predicateChecker *simulat kubeClient kube_client.Interface, kubeEventRecorder kube_record.EventRecorder, logEventRecorder *utils.LogEventRecorder, listerRegistry kube_util.ListerRegistry) (*AutoscalingContext, errors.AutoscalerError) { - cloudProviderBuilder := builder.NewCloudProviderBuilder(options.CloudProviderName, options.CloudConfig, options.ClusterName, options.NodeAutoprovisioningEnabled, options.Regional) + cloudProviderBuilder := builder.NewCloudProviderBuilder(options.KubeConfigPath, options.CloudProviderName, options.CloudConfig, options.ClusterName, options.NodeAutoprovisioningEnabled, options.Regional) cloudProvider := cloudProviderBuilder.Build(cloudprovider.NodeGroupDiscoveryOptions{ NodeGroupSpecs: options.NodeGroups, NodeGroupAutoDiscoverySpecs: options.NodeGroupAutoDiscovery},