Skip to content
This repository has been archived by the owner on Dec 24, 2019. It is now read-only.

Add scale-down-step functionality #45

Merged
merged 5 commits into from
Dec 16, 2017

Conversation

ichekrygin
Copy link
Contributor

Overview

Adding scale down functionality as per enhancement request #31.

Scale down step is expressed in:

  • fixed: instance count - defaults to and the minimum value is 1
  • percentage: expressed by value [0.0, 1.0) and defaults to 0.0

Logic

These values are used in slow_down_downscale with following logic:

There are two new_desired_size values are computed: new_desired_size_fixed and new_desired_size_percentage

  • new_desired_size_fixed is computed as previously defined by subtracting scale_down_step_fixed from nodes_count
  • new_desired_size_percentage is computed as a rounded up percentage value of the node_count
  • new_desired_size is set to a smallest value from the two above

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.5%) to 96.622% when pulling ba63068 on ichekrygin:scale-down-step into 04d9a29 on hjacobs:master.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.2%) to 97.333% when pulling 66776ae on ichekrygin:scale-down-step into 04d9a29 on hjacobs:master.

@hjacobs
Copy link
Owner

hjacobs commented Dec 13, 2017

@ichekrygin thanks a lot, actually I wanted to add the same anyway for our test clusters (where can scale down more than one instance at a time).

@hjacobs hjacobs self-requested a review December 13, 2017 23:30
for resource in RESOURCES:
parser.add_argument('--buffer-{}-percentage'.format(resource), type=float,
help='{} buffer %%'.format(resource.capitalize()),
default=os.getenv('BUFFER_{}_PERCENTAGE'.format(resource.upper()), DEFAULT_BUFFER_PERCENTAGE[resource]))
parser.add_argument('--buffer-{}-fixed'.format(resource), type=str,
help='{} buffer (fixed amount)'.format(resource.capitalize()),
default=os.getenv('BUFFER_{}_FIXED'.format(resource.upper()), DEFAULT_BUFFER_FIXED[resource]))

parser.add_argument('--scale-down-step-fixed',
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would add some os.getenv default (similar to the --buffer-* options) as we are configuring our downscaler via a config map (envFrom).

@coveralls
Copy link

Coverage Status

Coverage increased (+0.2%) to 97.333% when pulling 3acf364 on ichekrygin:scale-down-step into 04d9a29 on hjacobs:master.

help='Scale down strategy expressed in terms of instances count, defaults to 1. Note: value must be >= 1',
type=int, default=os.getenv('SCALE_DOWN_STEP_FIXED', 1))
parser.add_argument('--scale-down-step-percentage',
help='Scale down strategy expressed in terms of instances count, defaults to 0.01, i.e. 1%.',
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the default is 0.0, but the help says 0.01 😏

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ah... nice catch.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.2%) to 97.333% when pulling 3c803c2 on ichekrygin:scale-down-step into 04d9a29 on hjacobs:master.

@hjacobs hjacobs merged commit 4ded12a into hjacobs:master Dec 16, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants