From 694dc51fe572090f9d4ca8c27362cdecd103fd75 Mon Sep 17 00:00:00 2001 From: Benjamin Pineau Date: Thu, 8 Apr 2021 14:49:56 +0200 Subject: [PATCH] NodeInfo Processor for exclusive usage of template infos (with warnings) This is a third attempt at #1021 (might also provides an alternate solution for #2892 and #3608 amd #3609). Some uses cases includes: balance Similar when uspscaling from zero, edited/updated ASGs/MIGs taints and labels, updated instance type. Per #1021 discussion, a flag might be acceptable, if defaulting to false and describing the limitations (not all cloud providers) and the risk of using it (loss of accuracy, risk of upscaling unusable nodes or leaving pending pods). Per #3609 discussion, using a NodeInfo processor is prefered. --- .../config/autoscaling_options.go | 4 + cluster-autoscaler/core/static_autoscaler.go | 2 +- cluster-autoscaler/main.go | 7 ++ .../nodeinfos/node_info_processor.go | 6 +- .../template_only_node_info_processor.go | 101 ++++++++++++++++++ .../template_only_node_info_processor_test.go | 63 +++++++++++ 6 files changed, 180 insertions(+), 3 deletions(-) create mode 100644 cluster-autoscaler/processors/nodeinfos/template_only_node_info_processor.go create mode 100644 cluster-autoscaler/processors/nodeinfos/template_only_node_info_processor_test.go diff --git a/cluster-autoscaler/config/autoscaling_options.go b/cluster-autoscaler/config/autoscaling_options.go index 9a45089579ab..7a51b586e34a 100644 --- a/cluster-autoscaler/config/autoscaling_options.go +++ b/cluster-autoscaler/config/autoscaling_options.go @@ -147,4 +147,8 @@ type AutoscalingOptions struct { // ClusterAPICloudConfigAuthoritative tells the Cluster API provider to treat the CloudConfig option as authoritative and // not use KubeConfigPath as a fallback when it is not provided. ClusterAPICloudConfigAuthoritative bool + // ScaleUpTemplateFromCloudProvider tells cluster-autoscaler to always use cloud-providers node groups (ASG, MIG, VMSS...) + // templates rather than templates built from real-world nodes. Warning: this isn't supported by all providers, gives less + // accurate informations than real-world nodes, and can lead to wrong upscale decisions. + ScaleUpTemplateFromCloudProvider bool } diff --git a/cluster-autoscaler/core/static_autoscaler.go b/cluster-autoscaler/core/static_autoscaler.go index 0d249a41968f..f97af94fe51f 100644 --- a/cluster-autoscaler/core/static_autoscaler.go +++ b/cluster-autoscaler/core/static_autoscaler.go @@ -278,7 +278,7 @@ func (a *StaticAutoscaler) RunOnce(currentTime time.Time) errors.AutoscalerError return autoscalerError.AddPrefix("failed to build node infos for node groups: ") } - nodeInfosForGroups, err = a.processors.NodeInfoProcessor.Process(autoscalingContext, nodeInfosForGroups) + nodeInfosForGroups, err = a.processors.NodeInfoProcessor.Process(autoscalingContext, nodeInfosForGroups, daemonsets, a.ignoredTaints) if err != nil { klog.Errorf("Failed to process nodeInfos: %v", err) return errors.ToAutoscalerError(errors.InternalError, err) diff --git a/cluster-autoscaler/main.go b/cluster-autoscaler/main.go index e00ed45c669f..154fcada0c8b 100644 --- a/cluster-autoscaler/main.go +++ b/cluster-autoscaler/main.go @@ -43,6 +43,7 @@ import ( "k8s.io/autoscaler/cluster-autoscaler/metrics" ca_processors "k8s.io/autoscaler/cluster-autoscaler/processors" "k8s.io/autoscaler/cluster-autoscaler/processors/nodegroupset" + "k8s.io/autoscaler/cluster-autoscaler/processors/nodeinfos" "k8s.io/autoscaler/cluster-autoscaler/simulator" "k8s.io/autoscaler/cluster-autoscaler/utils/errors" kube_util "k8s.io/autoscaler/cluster-autoscaler/utils/kubernetes" @@ -175,6 +176,7 @@ var ( concurrentGceRefreshes = flag.Int("gce-concurrent-refreshes", 1, "Maximum number of concurrent refreshes per cloud object type.") enableProfiling = flag.Bool("profiling", false, "Is debug/pprof endpoint enabled") clusterAPICloudConfigAuthoritative = flag.Bool("clusterapi-cloud-config-authoritative", false, "Treat the cloud-config flag authoritatively (do not fallback to using kubeconfig flag). ClusterAPI only") + scaleUpTemplateFromCloudProvider = flag.Bool("scale-up-from-cloud-provider-template", false, "Build nodes templates from cloud providers node groups rather than real-world nodes. WARNING: this isn't supported by all cloud providers, and can lead to wrong autoscaling decisions (erroneous capacity evaluations causing infinite upscales, or pods left pending).") ) func createAutoscalingOptions() config.AutoscalingOptions { @@ -245,6 +247,7 @@ func createAutoscalingOptions() config.AutoscalingOptions { AWSUseStaticInstanceList: *awsUseStaticInstanceList, ConcurrentGceRefreshes: *concurrentGceRefreshes, ClusterAPICloudConfigAuthoritative: *clusterAPICloudConfigAuthoritative, + ScaleUpTemplateFromCloudProvider: *scaleUpTemplateFromCloudProvider, } } @@ -319,6 +322,10 @@ func buildAutoscaler() (core.Autoscaler, error) { Comparator: nodeInfoComparatorBuilder(autoscalingOptions.BalancingExtraIgnoredLabels), } + if autoscalingOptions.ScaleUpTemplateFromCloudProvider { + opts.Processors.NodeInfoProcessor = nodeinfos.NewTemplateOnlyNodeInfoProcessor() + } + // These metrics should be published only once. metrics.UpdateNapEnabled(autoscalingOptions.NodeAutoprovisioningEnabled) metrics.UpdateMaxNodesCount(autoscalingOptions.MaxNodesTotal) diff --git a/cluster-autoscaler/processors/nodeinfos/node_info_processor.go b/cluster-autoscaler/processors/nodeinfos/node_info_processor.go index 9f9c4b2b883e..91ef99e77e55 100644 --- a/cluster-autoscaler/processors/nodeinfos/node_info_processor.go +++ b/cluster-autoscaler/processors/nodeinfos/node_info_processor.go @@ -17,14 +17,16 @@ limitations under the License. package nodeinfos import ( + appsv1 "k8s.io/api/apps/v1" "k8s.io/autoscaler/cluster-autoscaler/context" + "k8s.io/autoscaler/cluster-autoscaler/utils/taints" schedulerframework "k8s.io/kubernetes/pkg/scheduler/framework/v1alpha1" ) // NodeInfoProcessor processes nodeInfos after they're created. type NodeInfoProcessor interface { // Process processes a map of nodeInfos for node groups. - Process(ctx *context.AutoscalingContext, nodeInfosForNodeGroups map[string]*schedulerframework.NodeInfo) (map[string]*schedulerframework.NodeInfo, error) + Process(ctx *context.AutoscalingContext, nodeInfosForNodeGroups map[string]*schedulerframework.NodeInfo, daemonsets []*appsv1.DaemonSet, ignoredTaints taints.TaintKeySet) (map[string]*schedulerframework.NodeInfo, error) // CleanUp cleans up processor's internal structures. CleanUp() } @@ -34,7 +36,7 @@ type NoOpNodeInfoProcessor struct { } // Process returns unchanged nodeInfos. -func (p *NoOpNodeInfoProcessor) Process(ctx *context.AutoscalingContext, nodeInfosForNodeGroups map[string]*schedulerframework.NodeInfo) (map[string]*schedulerframework.NodeInfo, error) { +func (p *NoOpNodeInfoProcessor) Process(ctx *context.AutoscalingContext, nodeInfosForNodeGroups map[string]*schedulerframework.NodeInfo, daemonsets []*appsv1.DaemonSet, ignoredTaints taints.TaintKeySet) (map[string]*schedulerframework.NodeInfo, error) { return nodeInfosForNodeGroups, nil } diff --git a/cluster-autoscaler/processors/nodeinfos/template_only_node_info_processor.go b/cluster-autoscaler/processors/nodeinfos/template_only_node_info_processor.go new file mode 100644 index 000000000000..e431f3ead4fc --- /dev/null +++ b/cluster-autoscaler/processors/nodeinfos/template_only_node_info_processor.go @@ -0,0 +1,101 @@ +/* +Copyright 2021 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 nodeinfos + +import ( + "math/rand" + "time" + + appsv1 "k8s.io/api/apps/v1" + "k8s.io/autoscaler/cluster-autoscaler/cloudprovider" + "k8s.io/autoscaler/cluster-autoscaler/context" + "k8s.io/autoscaler/cluster-autoscaler/core/utils" + "k8s.io/autoscaler/cluster-autoscaler/utils/errors" + "k8s.io/autoscaler/cluster-autoscaler/utils/taints" + klog "k8s.io/klog/v2" + schedulerframework "k8s.io/kubernetes/pkg/scheduler/framework/v1alpha1" +) + +const nodeInfoRefreshInterval = 15 * time.Minute + +type nodeInfoCacheEntry struct { + nodeInfo *schedulerframework.NodeInfo + lastRefresh time.Time +} + +// TemplateOnlyNodeInfoProcessor return NodeInfos built from node group templates. +type TemplateOnlyNodeInfoProcessor struct { + nodeInfoCache map[string]*nodeInfoCacheEntry +} + +// Process returns nodeInfos built from node groups templates. +func (p *TemplateOnlyNodeInfoProcessor) Process(ctx *context.AutoscalingContext, nodeInfosForNodeGroups map[string]*schedulerframework.NodeInfo, daemonsets []*appsv1.DaemonSet, ignoredTaints taints.TaintKeySet) (map[string]*schedulerframework.NodeInfo, error) { + result := make(map[string]*schedulerframework.NodeInfo) + seenGroups := make(map[string]bool) + + for _, nodeGroup := range ctx.CloudProvider.NodeGroups() { + id := nodeGroup.Id() + seenGroups[id] = true + + splay := rand.New(rand.NewSource(time.Now().UnixNano())).Intn(int(nodeInfoRefreshInterval.Seconds() + 1)) + lastRefresh := time.Now().Add(-time.Second * time.Duration(splay)) + if ng, ok := p.nodeInfoCache[id]; ok { + if ng.lastRefresh.Add(nodeInfoRefreshInterval).After(time.Now()) { + result[id] = ng.nodeInfo + continue + } + lastRefresh = time.Now() + } + + nodeInfo, err := utils.GetNodeInfoFromTemplate(nodeGroup, daemonsets, ctx.PredicateChecker, ignoredTaints) + if err != nil { + if err == cloudprovider.ErrNotImplemented { + klog.Warningf("Running in template only mode, but template isn't implemented for group %s", id) + continue + } else { + klog.Errorf("Unable to build proper template node for %s: %v", id, err) + return map[string]*schedulerframework.NodeInfo{}, + errors.ToAutoscalerError(errors.CloudProviderError, err) + } + } + + p.nodeInfoCache[id] = &nodeInfoCacheEntry{ + nodeInfo: nodeInfo, + lastRefresh: lastRefresh, + } + result[id] = nodeInfo + } + + for id := range p.nodeInfoCache { + if _, ok := seenGroups[id]; !ok { + delete(p.nodeInfoCache, id) + } + } + + return result, nil +} + +// CleanUp cleans up processor's internal structures. +func (p *TemplateOnlyNodeInfoProcessor) CleanUp() { +} + +// NewTemplateOnlyNodeInfoProcessor returns a NodeInfoProcessor generating NodeInfos from node group templates. +func NewTemplateOnlyNodeInfoProcessor() *TemplateOnlyNodeInfoProcessor { + return &TemplateOnlyNodeInfoProcessor{ + nodeInfoCache: make(map[string]*nodeInfoCacheEntry), + } +} diff --git a/cluster-autoscaler/processors/nodeinfos/template_only_node_info_processor_test.go b/cluster-autoscaler/processors/nodeinfos/template_only_node_info_processor_test.go new file mode 100644 index 000000000000..181efc4406df --- /dev/null +++ b/cluster-autoscaler/processors/nodeinfos/template_only_node_info_processor_test.go @@ -0,0 +1,63 @@ +/* +Copyright 2021 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 nodeinfos + +import ( + "testing" + + "github.com/stretchr/testify/assert" + testprovider "k8s.io/autoscaler/cluster-autoscaler/cloudprovider/test" + "k8s.io/autoscaler/cluster-autoscaler/context" + "k8s.io/autoscaler/cluster-autoscaler/simulator" + + . "k8s.io/autoscaler/cluster-autoscaler/utils/test" + + schedulerframework "k8s.io/kubernetes/pkg/scheduler/framework/v1alpha1" +) + +func TestTemplateOnlyNodeInfoProcessorProcess(t *testing.T) { + predicateChecker, err := simulator.NewTestPredicateChecker() + assert.NoError(t, err) + + tni := schedulerframework.NewNodeInfo() + tni.SetNode(BuildTestNode("tn", 100, 100)) + + provider1 := testprovider.NewTestAutoprovisioningCloudProvider( + nil, nil, nil, nil, nil, + map[string]*schedulerframework.NodeInfo{"ng1": tni, "ng2": tni}) + provider1.AddNodeGroup("ng1", 1, 10, 1) + provider1.AddNodeGroup("ng2", 2, 20, 2) + + ctx := &context.AutoscalingContext{ + CloudProvider: provider1, + PredicateChecker: predicateChecker, + } + + processor := NewTemplateOnlyNodeInfoProcessor() + res, err := processor.Process(ctx, nil, nil, nil) + + // nodegroups providing templates + assert.NoError(t, err) + assert.Equal(t, 2, len(res)) + assert.Contains(t, res, "ng1") + assert.Contains(t, res, "ng2") + + // nodegroup not providing templates + provider1.AddNodeGroup("ng3", 0, 1000, 0) + _, err = processor.Process(ctx, nil, nil, nil) + assert.Error(t, err) +}