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

cloudwatch_metric_alarm: fix idempotency for alarm without dimensions #1865

Conversation

jmisset-cb
Copy link
Contributor

SUMMARY

A metric alarm in Cloudwatch can optionally have Dimensions. When a metric alarm in Cloudwatch does not have any dimensions, it returns: "Dimensions": [] when queried via boto3.

When configuring a metric alarm without Dimensions in Cloudwatch using the cloudwatch_metric_alarm plugin, Dimensions must be absent from the parameters.

Because "Dimensions": [] does not match Dimensions: None, the result is always Changed.

This Pull Request fixes this by setting Dimensions from the returned alarm parameters to None when the field is empty.

Fixes #1750

ISSUE TYPE
  • Bugfix Pull Request
COMPONENT NAME

cloudwatch_metric_alarm

ADDITIONAL INFORMATION

Copy link
Contributor

Build succeeded.
https://ansible.softwarefactory-project.io/zuul/buildset/11de4804995c49bb8a2e7851073d2fcc

✔️ ansible-galaxy-importer SUCCESS in 3m 53s
✔️ build-ansible-collection SUCCESS in 14m 37s
✔️ ansible-test-splitter SUCCESS in 5m 32s
✔️ integration-amazon.aws-1 SUCCESS in 9m 54s
Skipped 43 jobs

@jmisset-cb
Copy link
Contributor Author

@gravesm Is there anything I can do to get someone to review this PR + the related issue it solves? Thanks in advance.

Copy link
Contributor

Build failed.
https://ansible.softwarefactory-project.io/zuul/buildset/975ec4cffb004ddeb7729fc193fcd3ed

✔️ ansible-galaxy-importer SUCCESS in 5m 17s
✔️ build-ansible-collection SUCCESS in 15m 14s
✔️ ansible-test-splitter SUCCESS in 5m 27s
integration-amazon.aws-1 FAILURE in 5m 20s
Skipped 43 jobs

Copy link
Contributor

Build succeeded.
https://ansible.softwarefactory-project.io/zuul/buildset/0e0287eed6ca4bf4923bdf4764a5af0f

✔️ ansible-galaxy-importer SUCCESS in 3m 45s
✔️ build-ansible-collection SUCCESS in 14m 01s
✔️ ansible-test-splitter SUCCESS in 5m 53s
✔️ integration-amazon.aws-1 SUCCESS in 6m 46s
Skipped 43 jobs

@GomathiselviS
Copy link
Collaborator

Hi @jmisset-cb Thank you for initiating this pull request. We are currently in the process of reviewing it. Kindly incorporate a changelog entry and perform a rebase of the PR.

Copy link
Contributor

Build succeeded.
https://ansible.softwarefactory-project.io/zuul/buildset/064c77a449524406866b2bbd3b1e72e2

✔️ ansible-galaxy-importer SUCCESS in 4m 25s
✔️ build-ansible-collection SUCCESS in 14m 23s
✔️ ansible-test-splitter SUCCESS in 5m 37s
✔️ integration-amazon.aws-1 SUCCESS in 8m 45s
Skipped 43 jobs

@jmisset-cb
Copy link
Contributor Author

Hi @GomathiselviS thank you for taking the time to review my PR.
I've added the assertion task and merge the latest main commits into the branch.
Could you please assist me with the changelog? In which file/directory should I place this?
Thanks in advance!

Copy link
Contributor

Build succeeded.
https://ansible.softwarefactory-project.io/zuul/buildset/a8fec7de4f0449959f9d0daa41727def

✔️ ansible-galaxy-importer SUCCESS in 5m 15s
✔️ build-ansible-collection SUCCESS in 15m 06s
✔️ ansible-test-splitter SUCCESS in 5m 31s
✔️ integration-amazon.aws-1 SUCCESS in 10m 43s
Skipped 43 jobs

@GomathiselviS
Copy link
Collaborator

Hi @GomathiselviS thank you for taking the time to review my PR. I've added the assertion task and merge the latest main commits into the branch. Could you please assist me with the changelog? In which file/directory should I place this? Thanks in advance!

This an example of a changelog fragment. You can create one for your fix. The category of your fix will be one of these

@jmisset-cb
Copy link
Contributor Author

Hi @GomathiselviS thank you for taking the time to review my PR. I've added the assertion task and merge the latest main commits into the branch. Could you please assist me with the changelog? In which file/directory should I place this? Thanks in advance!

This an example of a changelog fragment. You can create one for your fix. The category of your fix will be one of these

I've added the changelog entry! Thanks again.

Copy link
Contributor

Build succeeded.
https://ansible.softwarefactory-project.io/zuul/buildset/04ca17bc52b54c4ab1b23a4c22d3e422

✔️ ansible-galaxy-importer SUCCESS in 4m 21s
✔️ build-ansible-collection SUCCESS in 14m 16s
✔️ ansible-test-splitter SUCCESS in 5m 23s
✔️ integration-amazon.aws-1 SUCCESS in 9m 49s
Skipped 43 jobs

@jmisset-cb jmisset-cb requested a review from GomathiselviS May 10, 2024 12:10
@alinabuzachis alinabuzachis added this to the 8.2.0 milestone Aug 22, 2024
Copy link
Collaborator

@alinabuzachis alinabuzachis left a comment

Choose a reason for hiding this comment

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

@jmisset-cb Thank you for taking time to work on this bugfix and for patience. Can you please rebase your Pull Request?

@alinabuzachis alinabuzachis added the backport-8 PR should be backported to the stable-8 branch label Aug 22, 2024
@jmisset-cb
Copy link
Contributor Author

@alinabuzachis No worries, thank you for looking into this. I have rebased the PR.

Copy link
Contributor

Build succeeded.
https://ansible.softwarefactory-project.io/zuul/buildset/a4f566d1af78483d913f19c55f6e76a6

✔️ ansible-galaxy-importer SUCCESS in 5m 21s
✔️ build-ansible-collection SUCCESS in 10m 42s
✔️ ansible-test-splitter SUCCESS in 4m 22s
✔️ integration-amazon.aws-1 SUCCESS in 8m 03s
Skipped 43 jobs

tremble pushed a commit to jmisset-cb/amazon.aws that referenced this pull request Aug 28, 2024
@tremble tremble force-pushed the metric_alarm_no_dimensions branch from 229b1f2 to ebcfba4 Compare August 28, 2024 07:54
@tremble tremble dismissed GomathiselviS’s stale review August 28, 2024 07:55

Suggestion doesn't apply in this case.

tremble pushed a commit to jmisset-cb/amazon.aws that referenced this pull request Aug 28, 2024
@tremble tremble force-pushed the metric_alarm_no_dimensions branch from ebcfba4 to 5757308 Compare August 28, 2024 07:56
@tremble tremble added the mergeit Merge the PR (SoftwareFactory) label Aug 28, 2024
Copy link
Contributor

Build succeeded (gate pipeline).
https://ansible.softwarefactory-project.io/zuul/buildset/82b2eb7e0f0d49fa978473a9d7c9ed99

✔️ ansible-galaxy-importer SUCCESS in 4m 32s
✔️ build-ansible-collection SUCCESS in 10m 51s
✔️ ansible-test-splitter SUCCESS in 4m 27s
✔️ integration-amazon.aws-1 SUCCESS in 9m 30s
Skipped 43 jobs

Copy link
Contributor

Pull request merge failed: Required status check "ansible/gate" is expected.

@tremble tremble force-pushed the metric_alarm_no_dimensions branch from 5757308 to db059ad Compare August 28, 2024 08:24
@tremble tremble added mergeit Merge the PR (SoftwareFactory) and removed mergeit Merge the PR (SoftwareFactory) labels Aug 28, 2024
Copy link
Contributor

Build succeeded (gate pipeline).
https://ansible.softwarefactory-project.io/zuul/buildset/95835b33362c4d72b3981611da2da181

✔️ ansible-galaxy-importer SUCCESS in 4m 31s
✔️ build-ansible-collection SUCCESS in 10m 52s
✔️ ansible-test-splitter SUCCESS in 4m 26s
✔️ integration-amazon.aws-1 SUCCESS in 8m 39s
Skipped 43 jobs

@softwarefactory-project-zuul softwarefactory-project-zuul bot merged commit dc574ca into ansible-collections:main Aug 28, 2024
21 of 38 checks passed
Copy link

patchback bot commented Aug 28, 2024

Backport to stable-8: 💚 backport PR created

✅ Backport PR branch: patchback/backports/stable-8/dc574ca4de4f61dddb7bec39b45122fe0a2d091c/pr-1865

Backported as #2256

🤖 @patchback
I'm built with octomachinery and
my source is open — https://github.com/sanitizers/patchback-github-app.

patchback bot pushed a commit that referenced this pull request Aug 28, 2024
…#1865)

SUMMARY
A metric alarm in Cloudwatch can optionally have Dimensions. When a metric alarm in Cloudwatch does not have any dimensions, it returns: "Dimensions": [] when queried via boto3.
When configuring a metric alarm without Dimensions in Cloudwatch using the cloudwatch_metric_alarm plugin, Dimensions must be absent from the parameters.
Because "Dimensions": [] does not match Dimensions: None, the result is always Changed.
This Pull Request fixes this by setting Dimensions from the returned alarm parameters to None when the field is empty.
Fixes #1750
ISSUE TYPE

Bugfix Pull Request

COMPONENT NAME
cloudwatch_metric_alarm
ADDITIONAL INFORMATION

Reviewed-by: GomathiselviS
Reviewed-by: Jasper Misset
Reviewed-by: Alina Buzachis
Reviewed-by: Mark Chappell
(cherry picked from commit dc574ca)
softwarefactory-project-zuul bot pushed a commit that referenced this pull request Aug 28, 2024
…#1865) (#2256)

This is a backport of PR #1865 as merged into main (dc574ca).
SUMMARY
A metric alarm in Cloudwatch can optionally have Dimensions. When a metric alarm in Cloudwatch does not have any dimensions, it returns: "Dimensions": [] when queried via boto3.
When configuring a metric alarm without Dimensions in Cloudwatch using the cloudwatch_metric_alarm plugin, Dimensions must be absent from the parameters.
Because "Dimensions": [] does not match Dimensions: None, the result is always Changed.
This Pull Request fixes this by setting Dimensions from the returned alarm parameters to None when the field is empty.
Fixes #1750
ISSUE TYPE

Bugfix Pull Request

COMPONENT NAME
cloudwatch_metric_alarm
ADDITIONAL INFORMATION

Reviewed-by: Mark Chappell
braydencw1 pushed a commit to braydencw1/amazon.aws that referenced this pull request Aug 29, 2024
…ansible-collections#1865)

SUMMARY
A metric alarm in Cloudwatch can optionally have Dimensions. When a metric alarm in Cloudwatch does not have any dimensions, it returns: "Dimensions": [] when queried via boto3.
When configuring a metric alarm without Dimensions in Cloudwatch using the cloudwatch_metric_alarm plugin, Dimensions must be absent from the parameters.
Because "Dimensions": [] does not match Dimensions: None, the result is always Changed.
This Pull Request fixes this by setting Dimensions from the returned alarm parameters to None when the field is empty.
Fixes ansible-collections#1750
ISSUE TYPE

Bugfix Pull Request

COMPONENT NAME
cloudwatch_metric_alarm
ADDITIONAL INFORMATION

Reviewed-by: GomathiselviS
Reviewed-by: Jasper Misset
Reviewed-by: Alina Buzachis
Reviewed-by: Mark Chappell
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport-8 PR should be backported to the stable-8 branch mergeit Merge the PR (SoftwareFactory)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

cloudwatch_metric_alarm - Changes to alarms always report 'changed' due to dimensions
4 participants