Remove implementation details (CleanUp) from the interface.

The CleanUp method is instead called directly from the implementation,
when required.
Test updated in a quick way since the mock we're using does not support
AtLeast(1) - thus Times(2).
This commit is contained in:
Karol Gołąb 2018-05-07 14:48:56 +02:00
parent 0b94471727
commit 854fcc1ff8
6 changed files with 17 additions and 18 deletions

View File

@ -46,8 +46,6 @@ type AutoscalerOptions struct {
type Autoscaler interface {
// RunOnce represents an iteration in the control-loop of CA
RunOnce(currentTime time.Time) errors.AutoscalerError
// CleanUp represents a clean-up required before the first invocation of RunOnce
CleanUp()
// CloudProvider returns the cloud provider associated to this autoscaler
CloudProvider() cloudprovider.CloudProvider
// ExitCleanUp is a clean-up performed just before process termination.

View File

@ -47,11 +47,6 @@ func NewDynamicAutoscaler(autoscalerBuilder AutoscalerBuilder, configFetcher dyn
}, nil
}
// CleanUp does the work required before all the iterations of a dynamic autoscaler run
func (a *DynamicAutoscaler) CleanUp() {
a.autoscaler.CleanUp()
}
// CloudProvider returns the cloud provider associated to this autoscaler
func (a *DynamicAutoscaler) CloudProvider() cloudprovider.CloudProvider {
return a.autoscaler.CloudProvider()

View File

@ -34,10 +34,6 @@ func (m *AutoscalerMock) RunOnce(currentTime time.Time) errors.AutoscalerError {
return nil
}
func (m *AutoscalerMock) CleanUp() {
m.Called()
}
func (m *AutoscalerMock) CloudProvider() cloudprovider.CloudProvider {
args := m.Called()
return args.Get(0).(cloudprovider.CloudProvider)

View File

@ -59,6 +59,7 @@ type StaticAutoscaler struct {
lastScaleDownFailTime time.Time
scaleDown *ScaleDown
podListProcessor pods.PodListProcessor
initialized bool
}
// NewStaticAutoscaler creates an instance of Autoscaler filled with provided parameters
@ -91,12 +92,18 @@ func NewStaticAutoscaler(opts context.AutoscalingOptions, predicateChecker *simu
}, nil
}
// CleanUp cleans up ToBeDeleted taints added by the previously run and then failed CA
func (a *StaticAutoscaler) CleanUp() {
// cleanUpIfRequired removes ToBeDeleted taints added by a previous run of CA
// the taints are removed only once per runtime
func (a *StaticAutoscaler) cleanUpIfRequired() {
if a.initialized {
return
}
// CA can die at any time. Removing taints that might have been left from the previous run.
if readyNodes, err := a.ReadyNodeLister().List(); err != nil {
cleanToBeDeleted(readyNodes, a.AutoscalingContext.ClientSet, a.Recorder)
}
a.initialized = true
}
// CloudProvider returns the cloud provider associated to this autoscaler
@ -106,6 +113,8 @@ func (a *StaticAutoscaler) CloudProvider() cloudprovider.CloudProvider {
// RunOnce iterates over node groups and scales them up/down if necessary
func (a *StaticAutoscaler) RunOnce(currentTime time.Time) errors.AutoscalerError {
a.cleanUpIfRequired()
readyNodeLister := a.ReadyNodeLister()
allNodeLister := a.AllNodeLister()
unschedulablePodLister := a.UnschedulablePodLister()

View File

@ -200,7 +200,8 @@ func TestStaticAutoscalerRunOnce(t *testing.T) {
lastScaleUpTime: time.Now(),
lastScaleDownFailTime: time.Now(),
scaleDown: sd,
podListProcessor: pods.NewDefaultPodListProcessor()}
podListProcessor: pods.NewDefaultPodListProcessor(),
initialized: true}
// MaxNodesTotal reached.
readyNodeListerMock.On("List").Return([]*apiv1.Node{n1}, nil).Once()
@ -379,7 +380,8 @@ func TestStaticAutoscalerRunOnceWithAutoprovisionedEnabled(t *testing.T) {
lastScaleUpTime: time.Now(),
lastScaleDownFailTime: time.Now(),
scaleDown: sd,
podListProcessor: pods.NewDefaultPodListProcessor()}
podListProcessor: pods.NewDefaultPodListProcessor(),
initialized: true}
// Scale up.
readyNodeListerMock.On("List").Return([]*apiv1.Node{n1}, nil).Once()
@ -519,7 +521,7 @@ func TestStaticAutoscalerRunOnceWithALongUnregisteredNode(t *testing.T) {
podListProcessor: pods.NewDefaultPodListProcessor()}
// Scale up.
readyNodeListerMock.On("List").Return([]*apiv1.Node{n1}, nil).Once()
readyNodeListerMock.On("List").Return([]*apiv1.Node{n1}, nil).Times(2) // due to initialized=false
allNodeListerMock.On("List").Return([]*apiv1.Node{n1}, nil).Once()
scheduledPodMock.On("List").Return([]*apiv1.Pod{p1}, nil).Once()
unschedulablePodMock.On("List").Return([]*apiv1.Pod{p2}, nil).Once()
@ -656,7 +658,7 @@ func TestStaticAutoscalerRunOncePodsWithPriorities(t *testing.T) {
podListProcessor: pods.NewDefaultPodListProcessor()}
// Scale up
readyNodeListerMock.On("List").Return([]*apiv1.Node{n1, n2, n3}, nil).Once()
readyNodeListerMock.On("List").Return([]*apiv1.Node{n1, n2, n3}, nil).Times(2) // due to initialized=false
allNodeListerMock.On("List").Return([]*apiv1.Node{n1, n2, n3}, nil).Once()
scheduledPodMock.On("List").Return([]*apiv1.Pod{p1, p2, p3}, nil).Once()
unschedulablePodMock.On("List").Return([]*apiv1.Pod{p4, p5, p6}, nil).Once()

View File

@ -261,7 +261,6 @@ func run(healthCheck *metrics.HealthCheck) {
if err != nil {
glog.Fatalf("Failed to create autoscaler: %v", err)
}
autoscaler.CleanUp()
registerSignalHandlers(autoscaler)
healthCheck.StartMonitoring()