Ensure capacity does not exceed ASG MaxSize

[nitrocode]

Closes #34

@yob

[yob]

Thanks @nitrocode, we'll give this a test run and get back to you 🙏

[nitrocode]

after this is tested and merged, could we have a new release v1.0.2 tag of this scaler and have it be used in the master aws-stack.yml ?

We're eager to get past this issue. For now to be safe, we've raised all of our ASG MaxSize=100 and/or reset our ScaleOutFactor=1.

[yob]

Thanks for this @nitrocode.

It'd be great to avoid the extra API call to fetch the ASG state, and I just noticed that we have existing code for capping the scale out to MaxSize:

buildkite-agent-scaler/scaler/scaler.go

Lines 118 to 124 in 12d9dfb

	if desired > current.MaxSize {
	log.Printf("⚠️ Desired count exceed MaxSize, capping at %d", current.MaxSize)
	desired = current.MaxSize
	} else if desired < current.MinSize {
	log.Printf("⚠️ Desired count is less than MinSize, capping at %d", current.MinSize)
	desired = current.MinSize
	}

Unfortunately, that capping happens before the scale factors are applied.

I haven't explored this idea fully or tested it, but what do you think about moving the application of the scale factors (in and out) to happen before the capping process? Maybe as an extra unexported function like this:

diff --git a/scaler/scaler.go b/scaler/scaler.go
index b6a4334..6f5bcef 100644
--- a/scaler/scaler.go
+++ b/scaler/scaler.go
@@ -115,6 +115,12 @@ func (s *Scaler) Run() (time.Duration, error) {
                return metrics.PollDuration, err
        }
 
+       if desired > current {
+               desired = s.applyScaleOutFactor(desired)
+       } else if desired < current {
+               desired = s.applyScaleInFactor(desired)
+       }
+
        if desired > current.MaxSize {
                log.Printf("⚠️  Desired count exceed MaxSize, capping at %d", current.MaxSize)
                desired = current.MaxSize

[nitrocode]

Sure, sounds better than making another api call I suppose. As long as it works, I'm happy.

[nitrocode]

I reverted my change and added yours. Are there any unit or integration tests that can be run on this? @yob

[yob]

Thanks @nitrocode 🙏

We ended up extending this PR with a slightly different solution to the one I suggested, and added some unit tests in #37.

[nitrocode]

Perfect! Thanks @yob

Could we get this released as part of a 5.0 beta 2 for the elastic ci stack?

[yob]

yup, that's the plan 👍