Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add ignoreNullValues for AWS CloudWatch Scaler #5635

Open
wants to merge 28 commits into
base: main
Choose a base branch
from

Conversation

robpickerill
Copy link

@robpickerill robpickerill commented Mar 28, 2024

This PR adds support for erroring the AWS Cloudwatch Scaler when the GetMetricData call returns an empty set of metrics, resulting in no scale-changing behaviour when no custom metrics are available during the Cloudwatch query window.

Checklist

Fixes #5352

Relates to:

@robpickerill robpickerill requested a review from a team as a code owner March 28, 2024 15:03
Signed-off-by: Rob Pickerill <r.pickerill@gmail.com>
@robpickerill robpickerill force-pushed the cloudwatch-error-when-empty-metrics branch from 13c0fec to 53ea3fd Compare March 28, 2024 18:07
Signed-off-by: Rob Pickerill <r.pickerill@gmail.com>
Signed-off-by: Rob Pickerill <r.pickerill@gmail.com>
@robpickerill robpickerill force-pushed the cloudwatch-error-when-empty-metrics branch 3 times, most recently from 495c201 to 8e41311 Compare March 29, 2024 18:38
Signed-off-by: Rob Pickerill <r.pickerill@gmail.com>
@robpickerill robpickerill force-pushed the cloudwatch-error-when-empty-metrics branch from 584eba6 to b0d4e84 Compare March 29, 2024 19:01
Signed-off-by: Rob Pickerill <r.pickerill@gmail.com>
Signed-off-by: Rob Pickerill <r.pickerill@gmail.com>
Signed-off-by: Rob Pickerill <r.pickerill@gmail.com>
@robpickerill robpickerill changed the title add errorWhenMetricValueEmpty add errorWhenNullValues Mar 31, 2024
@robpickerill robpickerill changed the title add errorWhenNullValues add errorWhenNullValues for AWS CloudWatch Scaler Apr 1, 2024
@robpickerill
Copy link
Author

I added end to end tests here for both minMetricValue and errorWhenNullValues to cover off both scenarios. I need to tidy them up a little, which ill do over the next few days.

@robpickerill robpickerill force-pushed the cloudwatch-error-when-empty-metrics branch 2 times, most recently from 53fac56 to 0ef98c5 Compare April 4, 2024 17:26
Signed-off-by: Rob Pickerill <r.pickerill@gmail.com>
@robpickerill robpickerill force-pushed the cloudwatch-error-when-empty-metrics branch from 0ef98c5 to 0b61f97 Compare April 4, 2024 17:28
Signed-off-by: Rob Pickerill <r.pickerill@gmail.com>
Signed-off-by: Rob Pickerill <r.pickerill@gmail.com>
@robpickerill robpickerill force-pushed the cloudwatch-error-when-empty-metrics branch from 68a1c71 to 0dc9b77 Compare April 4, 2024 17:44
Signed-off-by: Rob Pickerill <r.pickerill@gmail.com>
@robpickerill robpickerill changed the title add errorWhenNullValues for AWS CloudWatch Scaler add ignoreNullValues for AWS CloudWatch Scaler Apr 11, 2024
Signed-off-by: Rob Pickerill <r.pickerill@gmail.com>
robpickerill and others added 7 commits April 26, 2024 18:14
Co-authored-by: Jorge Turrado Ferrero <Jorge_turrado@hotmail.es>
Signed-off-by: Rob Pickerill <r.pickerill@gmail.com>
…cloudwatch_ignore_null_values_false_test.go

Co-authored-by: Jorge Turrado Ferrero <Jorge_turrado@hotmail.es>
Signed-off-by: Rob Pickerill <r.pickerill@gmail.com>
Co-authored-by: Jorge Turrado Ferrero <Jorge_turrado@hotmail.es>
Signed-off-by: Rob Pickerill <r.pickerill@gmail.com>
Signed-off-by: Rob Pickerill <r.pickerill@gmail.com>
Signed-off-by: Rob Pickerill <r.pickerill@gmail.com>
Signed-off-by: Rob Pickerill <r.pickerill@gmail.com>
Signed-off-by: Rob Pickerill <r.pickerill@gmail.com>
…tricValue

Signed-off-by: robpickerill <r.pickerill@gmail.com>
@zroubalik
Copy link
Member

@robpickerill any updates here please?

Signed-off-by: Rob Pickerill <r.pickerill@gmail.com>
@robpickerill
Copy link
Author

@robpickerill any updates here please?

Good for review, all comments are addressed

@robpickerill robpickerill force-pushed the cloudwatch-error-when-empty-metrics branch from a67d1c0 to 410c10f Compare May 21, 2024 06:08
Signed-off-by: robpickerill <r.pickerill@gmail.com>
@robpickerill robpickerill force-pushed the cloudwatch-error-when-empty-metrics branch from 410c10f to 9374c26 Compare May 21, 2024 06:25
@JorTurFer
Copy link
Member

JorTurFer commented May 21, 2024

/run-e2e aws
Update: You can check the progress here

…ch_min_metric_value_test.go


fix invalid check

Co-authored-by: Jorge Turrado Ferrero <Jorge_turrado@hotmail.es>
Signed-off-by: Rob Pickerill <r.pickerill@gmail.com>
@JorTurFer
Copy link
Member

JorTurFer commented May 26, 2024

/run-e2e aws
Update: You can check the progress here

@JorTurFer
Copy link
Member

JorTurFer commented May 28, 2024

/run-e2e aws
Update: You can check the progress here

@zroubalik
Copy link
Member

@robpickerill do we have any update on this, please?

@robpickerill
Copy link
Author

I was discussing with Jorge on slack to get the e2e tests re-ran, as the failures were upstream issues: https://kubernetes.slack.com/archives/C01JGDP8MB8/p1717129850310929?thread_ts=1715943304.582119&cid=C01JGDP8MB8

However, since then the changes for the validation have gone in, so let me fix up the conflicts here over the weekend, and then let's get e2e passing again on this branch

@zroubalik
Copy link
Member

Thanks!

Signed-off-by: robpickerill <r.pickerill@gmail.com>
@robpickerill robpickerill force-pushed the cloudwatch-error-when-empty-metrics branch from 731d45c to 2e76a9b Compare June 8, 2024 15:37
@robpickerill
Copy link
Author

Fixed conflicts, this needs another e2e test run

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

AWS Cloudwatch Scaler, do nothing on empty metric data received
3 participants