From 74b540fdab468eae98a2974b9d98cf17d49eb8c7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Karol=20Go=C5=82=C4=85b?= Date: Mon, 14 May 2018 20:22:42 +0200 Subject: [PATCH] Remove DynamicAutoscaler since it's unused (#851) * Remove DynamicAutoscaler since it's unused * Remove configmap flag with its unused-elsewhere dependecies * gofmt --- cluster-autoscaler/config/dynamic/config.go | 36 ------ .../config/dynamic/config_fetcher.go | 79 ------------- cluster-autoscaler/core/autoscaler.go | 6 - cluster-autoscaler/core/autoscaler_test.go | 46 -------- cluster-autoscaler/core/dynamic_autoscaler.go | 85 -------------- .../core/dynamic_autoscaler_test.go | 108 ------------------ cluster-autoscaler/main.go | 22 +--- 7 files changed, 6 insertions(+), 376 deletions(-) delete mode 100644 cluster-autoscaler/config/dynamic/config_fetcher.go delete mode 100644 cluster-autoscaler/core/dynamic_autoscaler.go delete mode 100644 cluster-autoscaler/core/dynamic_autoscaler_test.go diff --git a/cluster-autoscaler/config/dynamic/config.go b/cluster-autoscaler/config/dynamic/config.go index abf6ed4d64..7626b83f44 100644 --- a/cluster-autoscaler/config/dynamic/config.go +++ b/cluster-autoscaler/config/dynamic/config.go @@ -17,11 +17,7 @@ limitations under the License. package dynamic import ( - "encoding/json" "fmt" - - "github.com/golang/glog" - "k8s.io/api/core/v1" ) // Config which represents not static but dynamic configuration of cluster-autoscaler which would be updated periodically at runtime @@ -45,38 +41,6 @@ func NewDefaultConfig() Config { } } -// ConfigFromConfigMap returns the configuration read from a configmap -func ConfigFromConfigMap(configmap *v1.ConfigMap) (*Config, error) { - settingsInJson := configmap.Data["settings"] - - if settingsInJson == "" { - return nil, fmt.Errorf(`invalid format of configmap: missing the key named "nodeGroups" in config = %v`, settingsInJson) - } - - settings := Settings{} - if err := json.Unmarshal([]byte(settingsInJson), &settings); err != nil { - return nil, fmt.Errorf(`failed to parse configmap data: %v`, err) - } - - config := &Config{ - Settings: settings, - resourceVersion: configmap.ResourceVersion, - } - - glog.V(5).Infof("json=%v settings=%v config=%v", settingsInJson, settings, config) - - if err := config.validate(); err != nil { - return nil, fmt.Errorf("invalid config : %v", err) - } - - return config, nil -} - -// VersionMismatchesAgainst returns true if versions between two configs don't match i.e. the config should be updated. -func (c Config) VersionMismatchesAgainst(other Config) bool { - return c.resourceVersion != other.resourceVersion -} - // NodeGroupSpecStrings returns node group specs represented in the form of `::` to be passed to cloudprovider. func (c Config) NodeGroupSpecStrings() []string { return c.nodeGroupSpecStrings() diff --git a/cluster-autoscaler/config/dynamic/config_fetcher.go b/cluster-autoscaler/config/dynamic/config_fetcher.go deleted file mode 100644 index 1fd4b5a59b..0000000000 --- a/cluster-autoscaler/config/dynamic/config_fetcher.go +++ /dev/null @@ -1,79 +0,0 @@ -/* -Copyright 2016 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 dynamic - -import ( - "fmt" - apiv1 "k8s.io/api/core/v1" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - kube_client "k8s.io/client-go/kubernetes" - kube_record "k8s.io/client-go/tools/record" -) - -// ConfigFetcher fetches the up-to-date dynamic configuration from the apiserver -type ConfigFetcher interface { - FetchConfigIfUpdated() (*Config, error) -} - -type configFetcherImpl struct { - configMapName string - namespace string - kubeClient kube_client.Interface - lastConfig Config - // Recorder for recording events. - recorder kube_record.EventRecorder -} - -// ConfigFetcherOptions contains the various options to customize ConfigFetcher -type ConfigFetcherOptions struct { - ConfigMapName string - Namespace string -} - -// NewConfigFetcher builds a config fetcher from the parameters and dependencies -func NewConfigFetcher(options ConfigFetcherOptions, kubeClient kube_client.Interface, recorder kube_record.EventRecorder) *configFetcherImpl { - return &configFetcherImpl{ - configMapName: options.ConfigMapName, - namespace: options.Namespace, - kubeClient: kubeClient, - lastConfig: NewDefaultConfig(), - recorder: recorder, - } -} - -// Returns the config if it has changed since the last sync. Returns nil if it has not changed. -func (c *configFetcherImpl) FetchConfigIfUpdated() (*Config, error) { - opts := metav1.GetOptions{} - cm, err := c.kubeClient.CoreV1().ConfigMaps(c.namespace).Get(c.configMapName, opts) - if err != nil { - return nil, fmt.Errorf("failed to fetch config map named %s in namespace %s. please confirm if the configmap name and the namespace are correctly spelled and you've already created the configmap: %v", c.configMapName, c.namespace, err) - } - - configFromServer, err := ConfigFromConfigMap(cm) - if err != nil { - c.recorder.Eventf(cm, apiv1.EventTypeNormal, "FailedToBeLoaded", - "cluster-autoscaler tried to load this configmap but failed: %v", err) - return nil, fmt.Errorf("failed to load dynamic config: %v", err) - } - - if c.lastConfig.VersionMismatchesAgainst(*configFromServer) { - c.lastConfig = *configFromServer - return configFromServer, nil - } - - return nil, nil -} diff --git a/cluster-autoscaler/core/autoscaler.go b/cluster-autoscaler/core/autoscaler.go index 8d8d92ab9e..db052deced 100644 --- a/cluster-autoscaler/core/autoscaler.go +++ b/cluster-autoscaler/core/autoscaler.go @@ -19,7 +19,6 @@ package core import ( "time" - "k8s.io/autoscaler/cluster-autoscaler/config/dynamic" "k8s.io/autoscaler/cluster-autoscaler/context" "k8s.io/autoscaler/cluster-autoscaler/simulator" "k8s.io/autoscaler/cluster-autoscaler/utils/errors" @@ -32,7 +31,6 @@ import ( // AutoscalerOptions is the whole set of options for configuring an autoscaler type AutoscalerOptions struct { context.AutoscalingOptions - dynamic.ConfigFetcherOptions KubeClient kube_client.Interface KubeEventRecorder kube_record.EventRecorder PredicateChecker *simulator.PredicateChecker @@ -63,9 +61,5 @@ func NewAutoscaler(opts AutoscalerOptions) (Autoscaler, errors.AutoscalerError) return nil, errors.ToAutoscalerError(errors.InternalError, err) } autoscalerBuilder := NewAutoscalerBuilder(opts.AutoscalingOptions, opts.PredicateChecker, opts.KubeClient, opts.KubeEventRecorder, opts.ListerRegistry, opts.PodListProcessor) - if opts.ConfigMapName != "" { - configFetcher := dynamic.NewConfigFetcher(opts.ConfigFetcherOptions, opts.KubeClient, opts.KubeEventRecorder) - return NewDynamicAutoscaler(autoscalerBuilder, configFetcher) - } return autoscalerBuilder.Build() } diff --git a/cluster-autoscaler/core/autoscaler_test.go b/cluster-autoscaler/core/autoscaler_test.go index a026fd8123..03956f1e54 100644 --- a/cluster-autoscaler/core/autoscaler_test.go +++ b/cluster-autoscaler/core/autoscaler_test.go @@ -21,8 +21,6 @@ import ( "testing" "time" - "k8s.io/autoscaler/cluster-autoscaler/config/dynamic" - "github.com/stretchr/testify/assert" apiv1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/errors" @@ -63,9 +61,6 @@ func TestNewAutoscalerStatic(t *testing.T) { predicateChecker := simulator.NewTestPredicateChecker() listerRegistry := kube_util.NewListerRegistry(nil, nil, nil, nil, nil, nil) opts := AutoscalerOptions{ - ConfigFetcherOptions: dynamic.ConfigFetcherOptions{ - ConfigMapName: "", - }, PredicateChecker: predicateChecker, KubeClient: fakeClient, KubeEventRecorder: kubeEventRecorder, @@ -74,44 +69,3 @@ func TestNewAutoscalerStatic(t *testing.T) { a, _ := NewAutoscaler(opts) assert.IsType(t, &StaticAutoscaler{}, a) } - -func TestNewAutoscalerDynamic(t *testing.T) { - fakeClient := &fake.Clientset{} - n1 := BuildTestNode("n1", 1000, 1000) - SetNodeReadyState(n1, false, time.Now().Add(-3*time.Minute)) - n2 := BuildTestNode("n2", 1000, 1000) - SetNodeReadyState(n2, true, time.Time{}) - p2 := BuildTestPod("p2", 800, 0) - p2.Spec.NodeName = "n2" - - fakeClient.Fake.AddReactor("list", "pods", func(action core.Action) (bool, runtime.Object, error) { - return true, &apiv1.PodList{Items: []apiv1.Pod{*p2}}, nil - }) - fakeClient.Fake.AddReactor("get", "pods", func(action core.Action) (bool, runtime.Object, error) { - return true, nil, errors.NewNotFound(apiv1.Resource("pod"), "whatever") - }) - fakeClient.Fake.AddReactor("get", "nodes", func(action core.Action) (bool, runtime.Object, error) { - getAction := action.(core.GetAction) - switch getAction.GetName() { - case n1.Name: - return true, n1, nil - case n2.Name: - return true, n2, nil - } - return true, nil, fmt.Errorf("Wrong node: %v", getAction.GetName()) - }) - kubeEventRecorder := kube_util.CreateEventRecorder(fakeClient) - predicateChecker := simulator.NewTestPredicateChecker() - listerRegistry := kube_util.NewListerRegistry(nil, nil, nil, nil, nil, nil) - opts := AutoscalerOptions{ - ConfigFetcherOptions: dynamic.ConfigFetcherOptions{ - ConfigMapName: "testconfigmap", - }, - PredicateChecker: predicateChecker, - KubeClient: fakeClient, - KubeEventRecorder: kubeEventRecorder, - ListerRegistry: listerRegistry, - } - a, _ := NewAutoscaler(opts) - assert.IsType(t, &DynamicAutoscaler{}, a) -} diff --git a/cluster-autoscaler/core/dynamic_autoscaler.go b/cluster-autoscaler/core/dynamic_autoscaler.go deleted file mode 100644 index 85d08b4471..0000000000 --- a/cluster-autoscaler/core/dynamic_autoscaler.go +++ /dev/null @@ -1,85 +0,0 @@ -/* -Copyright 2016 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 core - -import ( - "fmt" - "time" - - "github.com/golang/glog" - "k8s.io/autoscaler/cluster-autoscaler/config/dynamic" - "k8s.io/autoscaler/cluster-autoscaler/metrics" - "k8s.io/autoscaler/cluster-autoscaler/utils/errors" -) - -// DynamicAutoscaler is a variant of autoscaler which supports dynamic reconfiguration at runtime -type DynamicAutoscaler struct { - autoscaler Autoscaler - autoscalerBuilder AutoscalerBuilder - configFetcher dynamic.ConfigFetcher -} - -// NewDynamicAutoscaler builds a DynamicAutoscaler from required parameters -func NewDynamicAutoscaler(autoscalerBuilder AutoscalerBuilder, configFetcher dynamic.ConfigFetcher) (*DynamicAutoscaler, errors.AutoscalerError) { - autoscaler, err := autoscalerBuilder.Build() - if err != nil { - return nil, err - } - return &DynamicAutoscaler{ - autoscaler: autoscaler, - autoscalerBuilder: autoscalerBuilder, - configFetcher: configFetcher, - }, nil -} - -// ExitCleanUp cleans-up after autoscaler, so no mess remains after process termination. -func (a *DynamicAutoscaler) ExitCleanUp() { - a.autoscaler.ExitCleanUp() -} - -// RunOnce represents a single iteration of a dynamic autoscaler inside the CA's control-loop -func (a *DynamicAutoscaler) RunOnce(currentTime time.Time) errors.AutoscalerError { - reconfigureStart := time.Now() - metrics.UpdateLastTime(metrics.Reconfigure, reconfigureStart) - if err := a.Reconfigure(); err != nil { - glog.Errorf("Failed to reconfigure : %v", err) - } - metrics.UpdateDurationFromStart(metrics.Reconfigure, reconfigureStart) - return a.autoscaler.RunOnce(currentTime) -} - -// Reconfigure this dynamic autoscaler if the configmap is updated -func (a *DynamicAutoscaler) Reconfigure() error { - var updatedConfig *dynamic.Config - var err error - - if updatedConfig, err = a.configFetcher.FetchConfigIfUpdated(); err != nil { - return fmt.Errorf("failed to fetch updated config: %v", err) - } - - if updatedConfig != nil { - // For safety, any config change should stop and recreate all the stuff running in CA hence recreating all the Autoscaler instance here - // See https://github.com/kubernetes/contrib/pull/2226#discussion_r94126064 - a.autoscaler, err = a.autoscalerBuilder.SetDynamicConfig(*updatedConfig).Build() - if err != nil { - return err - } - glog.V(4).Infof("Dynamic reconfiguration finished: updatedConfig=%v", updatedConfig) - } - - return nil -} diff --git a/cluster-autoscaler/core/dynamic_autoscaler_test.go b/cluster-autoscaler/core/dynamic_autoscaler_test.go deleted file mode 100644 index 7d0aea4fea..0000000000 --- a/cluster-autoscaler/core/dynamic_autoscaler_test.go +++ /dev/null @@ -1,108 +0,0 @@ -/* -Copyright 2016 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 core - -import ( - "github.com/stretchr/testify/mock" - "k8s.io/autoscaler/cluster-autoscaler/config/dynamic" - "k8s.io/autoscaler/cluster-autoscaler/utils/errors" - "testing" - "time" -) - -type AutoscalerMock struct { - mock.Mock -} - -func (m *AutoscalerMock) RunOnce(currentTime time.Time) errors.AutoscalerError { - m.Called(currentTime) - return nil -} - -func (m *AutoscalerMock) ExitCleanUp() { - m.Called() -} - -type ConfigFetcherMock struct { - mock.Mock -} - -func (m *ConfigFetcherMock) FetchConfigIfUpdated() (*dynamic.Config, error) { - args := m.Called() - return args.Get(0).(*dynamic.Config), args.Error(1) -} - -type AutoscalerBuilderMock struct { - mock.Mock -} - -func (m *AutoscalerBuilderMock) SetDynamicConfig(config dynamic.Config) AutoscalerBuilder { - args := m.Called(config) - return args.Get(0).(AutoscalerBuilder) -} - -func (m *AutoscalerBuilderMock) Build() (Autoscaler, errors.AutoscalerError) { - args := m.Called() - return args.Get(0).(Autoscaler), nil -} - -func TestRunOnceWhenNoUpdate(t *testing.T) { - currentTime := time.Now() - - autoscaler := &AutoscalerMock{} - autoscaler.On("RunOnce", currentTime).Once() - - configFetcher := &ConfigFetcherMock{} - configFetcher.On("FetchConfigIfUpdated").Return((*dynamic.Config)(nil), nil).Once() - - builder := &AutoscalerBuilderMock{} - builder.On("Build").Return(autoscaler).Once() - - a, _ := NewDynamicAutoscaler(builder, configFetcher) - a.RunOnce(currentTime) - - autoscaler.AssertExpectations(t) - configFetcher.AssertExpectations(t) - builder.AssertExpectations(t) -} - -func TestRunOnceWhenUpdated(t *testing.T) { - currentTime := time.Now() - - newConfig := dynamic.NewDefaultConfig() - - initialAutoscaler := &AutoscalerMock{} - - newAutoscaler := &AutoscalerMock{} - newAutoscaler.On("RunOnce", currentTime).Once() - - configFetcher := &ConfigFetcherMock{} - configFetcher.On("FetchConfigIfUpdated").Return(&newConfig, nil).Once() - - builder := &AutoscalerBuilderMock{} - builder.On("Build").Return(initialAutoscaler).Once() - builder.On("SetDynamicConfig", newConfig).Return(builder).Once() - builder.On("Build").Return(newAutoscaler).Once() - - a, _ := NewDynamicAutoscaler(builder, configFetcher) - a.RunOnce(currentTime) - - initialAutoscaler.AssertNotCalled(t, "RunOnce", mock.AnythingOfType("time.Time")) - newAutoscaler.AssertExpectations(t) - configFetcher.AssertExpectations(t) - builder.AssertExpectations(t) -} diff --git a/cluster-autoscaler/main.go b/cluster-autoscaler/main.go index fc2d54e849..eecaba9878 100644 --- a/cluster-autoscaler/main.go +++ b/cluster-autoscaler/main.go @@ -32,7 +32,6 @@ import ( kube_flag "k8s.io/apiserver/pkg/util/flag" cloudBuilder "k8s.io/autoscaler/cluster-autoscaler/cloudprovider/builder" "k8s.io/autoscaler/cluster-autoscaler/config" - "k8s.io/autoscaler/cluster-autoscaler/config/dynamic" "k8s.io/autoscaler/cluster-autoscaler/context" "k8s.io/autoscaler/cluster-autoscaler/core" "k8s.io/autoscaler/cluster-autoscaler/estimator" @@ -75,8 +74,7 @@ var ( kubernetes = flag.String("kubernetes", "", "Kubernetes master location. Leave blank for default") kubeConfigFile = flag.String("kubeconfig", "", "Path to kubeconfig file with authorization and master location information.") cloudConfig = flag.String("cloud-config", "", "The path to the cloud provider configuration file. Empty string for no configuration file.") - configMapName = flag.String("configmap", "", "The name of the ConfigMap containing settings used for dynamic reconfiguration. Empty string for no ConfigMap.") - namespace = flag.String("namespace", "kube-system", "Namespace in which cluster-autoscaler run. If a --configmap flag is also provided, ensure that the configmap exists in this namespace before CA runs.") + namespace = flag.String("namespace", "kube-system", "Namespace in which cluster-autoscaler run.") scaleDownEnabled = flag.Bool("scale-down-enabled", true, "Should CA scale down the cluster") scaleDownDelayAfterAdd = flag.Duration("scale-down-delay-after-add", 10*time.Minute, "How long after scale up that scale down evaluation resumes") @@ -186,13 +184,6 @@ func createAutoscalingOptions() context.AutoscalingOptions { } } -func createConfigFetcherOptions() dynamic.ConfigFetcherOptions { - return dynamic.ConfigFetcherOptions{ - ConfigMapName: *configMapName, - Namespace: *namespace, - } -} - func createKubeClient() kube_client.Interface { if *kubeConfigFile != "" { glog.V(1).Infof("Using kubeconfig file: %s", *kubeConfigFile) @@ -250,12 +241,11 @@ func run(healthCheck *metrics.HealthCheck) { listerRegistry := kube_util.NewListerRegistryWithDefaultListers(kubeClient, listerRegistryStopChannel) opts := core.AutoscalerOptions{ - AutoscalingOptions: autoscalingOptions, - ConfigFetcherOptions: createConfigFetcherOptions(), - PredicateChecker: predicateChecker, - KubeClient: kubeClient, - KubeEventRecorder: kubeEventRecorder, - ListerRegistry: listerRegistry, + AutoscalingOptions: autoscalingOptions, + PredicateChecker: predicateChecker, + KubeClient: kubeClient, + KubeEventRecorder: kubeEventRecorder, + ListerRegistry: listerRegistry, } autoscaler, err := core.NewAutoscaler(opts) if err != nil {