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

cloudwatchevent_rule should return false when there is no change done to the rule #1589

Merged

Conversation

taehopark32
Copy link
Contributor

@taehopark32 taehopark32 commented Jun 1, 2023

SUMMARY

Fixes #1080

ISSUE TYPE
  • Bugfix Pull Request
COMPONENT NAME

plugins/modules/cloudwatchevent_rule.py

ADDITIONAL INFORMATION

@taehopark32 taehopark32 changed the title Branch new Fixed cloudwatchevent_rule always reports Changed = True Jun 1, 2023
@@ -433,6 +433,14 @@ def _rule_matches_aws(self):
def _targets_to_put(self):
"""Returns a list of targets that need to be updated or added remotely"""
remote_targets = self.rule.list_targets()

# The self.targets have extra parameters, so scrub is used to
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
# The self.targets have extra parameters, so scrub is used to
# keys with none values must be scrubbed off of self.targets

@softwarefactory-project-zuul
Copy link
Contributor

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

✔️ ansible-galaxy-importer SUCCESS in 3m 38s
✔️ build-ansible-collection SUCCESS in 12m 52s
✔️ ansible-test-splitter SUCCESS in 4m 49s
✔️ integration-amazon.aws-1 SUCCESS in 7m 09s
Skipped 43 jobs

@GomathiselviS GomathiselviS changed the title Fixed cloudwatchevent_rule always reports Changed = True cloudwatchevent_rule should return false when there is no change done to the rule Jun 1, 2023
@@ -51,6 +51,27 @@
assert:
that:
- event_rule_input_transformer_output.changed

- name: Create cloudwatch event rule with input transformer
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
- name: Create cloudwatch event rule with input transformer
- name: Create cloudwatch event rule with input transformer (idempotent)

input_template: "<instance> is in state <state>"
register: event_rule_input_transformer_output

- name: Assert that input transformer event rule was created
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
- name: Assert that input transformer event rule was created
- name: Assert that no changes were made to the rule

@softwarefactory-project-zuul
Copy link
Contributor

Build failed.
https://ansible.softwarefactory-project.io/zuul/buildset/16653b47d9234200a5843596f7b0d40e

✔️ ansible-galaxy-importer SUCCESS in 4m 27s
✔️ build-ansible-collection SUCCESS in 12m 41s
✔️ ansible-test-splitter SUCCESS in 4m 56s
integration-amazon.aws-1 FAILURE in 8m 12s
Skipped 43 jobs

@softwarefactory-project-zuul
Copy link
Contributor

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

✔️ ansible-galaxy-importer SUCCESS in 3m 58s
✔️ build-ansible-collection SUCCESS in 13m 36s
✔️ ansible-test-splitter SUCCESS in 4m 44s
integration-amazon.aws-1 FAILURE in 9m 13s
Skipped 43 jobs

@softwarefactory-project-zuul
Copy link
Contributor

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

✔️ ansible-galaxy-importer SUCCESS in 3m 44s
✔️ build-ansible-collection SUCCESS in 12m 38s
✔️ ansible-test-splitter SUCCESS in 4m 54s
integration-amazon.aws-1 FAILURE in 6m 12s
Skipped 43 jobs

@softwarefactory-project-zuul
Copy link
Contributor

Build failed.
https://ansible.softwarefactory-project.io/zuul/buildset/7e7d19b7775f4ac59036c277a103e05f

✔️ ansible-galaxy-importer SUCCESS in 4m 03s
✔️ build-ansible-collection SUCCESS in 12m 55s
✔️ ansible-test-splitter SUCCESS in 4m 53s
integration-amazon.aws-1 FAILURE in 5m 56s
Skipped 43 jobs

@github-actions
Copy link

github-actions bot commented Jun 2, 2023

Docs Build 📝

Thank you for contribution!✨

This PR has been merged and your docs changes will be incorporated when they are next published.

@softwarefactory-project-zuul
Copy link
Contributor

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

✔️ ansible-galaxy-importer SUCCESS in 5m 10s
✔️ build-ansible-collection SUCCESS in 13m 04s
✔️ ansible-test-splitter SUCCESS in 5m 00s
✔️ integration-amazon.aws-1 SUCCESS in 5m 52s
Skipped 43 jobs

- name: Assert that no changes were made to the rule
assert:
that:
- event_rule_input_transformer_output.changed
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
- event_rule_input_transformer_output.changed
- not event_rule_input_transformer_output.changed

Copy link
Contributor

Choose a reason for hiding this comment

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

changed should be false in this testcase.

Copy link
Contributor

Choose a reason for hiding this comment

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

Alternatively

Suggested change
- event_rule_input_transformer_output.changed
- event_rule_input_transformer_output is not changed

I believe in theory this is the 'preferred' way to reference changed as it's more natural.

@softwarefactory-project-zuul
Copy link
Contributor

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

✔️ ansible-galaxy-importer SUCCESS in 4m 17s
✔️ build-ansible-collection SUCCESS in 12m 28s
✔️ ansible-test-splitter SUCCESS in 4m 52s
integration-amazon.aws-1 FAILURE in 7m 35s
Skipped 43 jobs

@softwarefactory-project-zuul
Copy link
Contributor

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

✔️ ansible-galaxy-importer SUCCESS in 4m 32s
✔️ build-ansible-collection SUCCESS in 13m 38s
✔️ ansible-test-splitter SUCCESS in 4m 41s
✔️ integration-amazon.aws-1 SUCCESS in 5m 48s
Skipped 43 jobs

@softwarefactory-project-zuul
Copy link
Contributor

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

✔️ ansible-galaxy-importer SUCCESS in 3m 41s
✔️ build-ansible-collection SUCCESS in 12m 31s
✔️ ansible-test-splitter SUCCESS in 4m 39s
✔️ integration-amazon.aws-1 SUCCESS in 5m 18s
Skipped 43 jobs

@softwarefactory-project-zuul
Copy link
Contributor

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

✔️ ansible-galaxy-importer SUCCESS in 4m 07s
✔️ build-ansible-collection SUCCESS in 12m 23s
✔️ ansible-test-splitter SUCCESS in 5m 12s
✔️ integration-amazon.aws-1 SUCCESS in 5m 38s
Skipped 43 jobs

@softwarefactory-project-zuul
Copy link
Contributor

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

✔️ ansible-galaxy-importer SUCCESS in 4m 10s
✔️ build-ansible-collection SUCCESS in 13m 37s
✔️ ansible-test-splitter SUCCESS in 4m 49s
✔️ integration-amazon.aws-1 SUCCESS in 7m 43s
Skipped 43 jobs

@@ -0,0 +1,3 @@
---
bugfixes:
- cloudwatchevent_rule - add scrub none inside the _targets_to_put method (https://github.com/ansible-collections/amazon.aws/pull/1589)."
Copy link
Collaborator

Choose a reason for hiding this comment

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

The changelog should describe what the end user needs to know, functionaly, about the change, rather than the implementation details of the code. Something like
"Fixes changed status to report False when no change has been made. The module had incorrectly always reported a change."
But make sure it describes whatever scenario the wrong change status would have occurred for, if it wasn't always.

@softwarefactory-project-zuul
Copy link
Contributor

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

✔️ ansible-galaxy-importer SUCCESS in 4m 47s
✔️ build-ansible-collection SUCCESS in 13m 20s
✔️ ansible-test-splitter SUCCESS in 5m 08s
✔️ integration-amazon.aws-1 SUCCESS in 8m 17s
Skipped 43 jobs

@alinabuzachis alinabuzachis added backport-6 PR should be backported to the stable-6 branch backport-5 PR should be backported to the stable-5 branch labels Jun 12, 2023
@alinabuzachis alinabuzachis added the mergeit Merge the PR (SoftwareFactory) label Jul 4, 2023
@softwarefactory-project-zuul
Copy link
Contributor

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

✔️ ansible-galaxy-importer SUCCESS in 3m 57s
✔️ build-ansible-collection SUCCESS in 12m 36s
✔️ ansible-test-splitter SUCCESS in 5m 02s
✔️ integration-amazon.aws-1 SUCCESS in 5m 56s
Skipped 43 jobs

@softwarefactory-project-zuul softwarefactory-project-zuul bot merged commit 614b792 into ansible-collections:main Jul 4, 2023
72 checks passed
@patchback
Copy link

patchback bot commented Jul 4, 2023

Backport to stable-5: 💚 backport PR created

✅ Backport PR branch: patchback/backports/stable-5/614b792be81a8a64c8c955093c853733ca1f57c3/pr-1589

Backported as #1639

🤖 @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 Jul 4, 2023
… to the rule (#1589)

cloudwatchevent_rule should return false when there is no change done to the rule

SUMMARY

Fixes #1080
ISSUE TYPE

Bugfix Pull Request

COMPONENT NAME

plugins/modules/cloudwatchevent_rule.py
ADDITIONAL INFORMATION

Reviewed-by: GomathiselviS
Reviewed-by: Mark Chappell
Reviewed-by: Jill R
Reviewed-by: Alina Buzachis
(cherry picked from commit 614b792)
@patchback
Copy link

patchback bot commented Jul 4, 2023

Backport to stable-6: 💚 backport PR created

✅ Backport PR branch: patchback/backports/stable-6/614b792be81a8a64c8c955093c853733ca1f57c3/pr-1589

Backported as #1640

🤖 @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 Jul 4, 2023
… to the rule (#1589)

cloudwatchevent_rule should return false when there is no change done to the rule

SUMMARY

Fixes #1080
ISSUE TYPE

Bugfix Pull Request

COMPONENT NAME

plugins/modules/cloudwatchevent_rule.py
ADDITIONAL INFORMATION

Reviewed-by: GomathiselviS
Reviewed-by: Mark Chappell
Reviewed-by: Jill R
Reviewed-by: Alina Buzachis
(cherry picked from commit 614b792)
softwarefactory-project-zuul bot pushed a commit that referenced this pull request Jul 4, 2023
… to the rule (#1589) (#1640)

[PR #1589/614b792b backport][stable-6] cloudwatchevent_rule should return false when there is no change done to the rule

This is a backport of PR #1589 as merged into main (614b792).
SUMMARY


Fixes #1080
ISSUE TYPE


Bugfix Pull Request

COMPONENT NAME

plugins/modules/cloudwatchevent_rule.py
ADDITIONAL INFORMATION

Reviewed-by: Alina Buzachis
softwarefactory-project-zuul bot pushed a commit that referenced this pull request Jul 4, 2023
… to the rule (#1589) (#1639)

[PR #1589/614b792b backport][stable-5] cloudwatchevent_rule should return false when there is no change done to the rule

This is a backport of PR #1589 as merged into main (614b792).
SUMMARY


Fixes #1080
ISSUE TYPE


Bugfix Pull Request

COMPONENT NAME

plugins/modules/cloudwatchevent_rule.py
ADDITIONAL INFORMATION

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

Successfully merging this pull request may close these issues.

cloudwatchevent_rule always reports Changed = True
5 participants