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

[Plugin elasticache_info] includes the description of each replication group #646

Conversation

MKCG
Copy link
Contributor

@MKCG MKCG commented Jul 16, 2021

SUMMARY

This pull request add the description of the corresponding Elasticache replication group into each described cache cluster.

ISSUE TYPE
  • Feature Pull Request
COMPONENT NAME

elasticache_info

ADDITIONAL INFORMATION

Actually the only way to describe Elasticache replication groups with Ansible is to use the dynamic inventory.

However, exposing the informations about each replication group could be useful in other contexts.

In my case, I am generating a few .env files and I need to include some Elasticache replication group primary endpoint and reader endpoint.

Since the elasticache_info plugin already exist and already retrieves the appropriate replication_group_id, I believe that this plugin should also retrieve the replication group description when it exists.

OTHER REMARKS CONCERNING THIS PULL REQUEST

Each provided description is copy/pasted from the official boto3 documentation which is distributed under an Apache 2.0 licence. I did it for consistency reason between and also because I am not a native speaker.

Also the provided description is incomplete : I only documented the fields that I was able to retrieve from my current AWS clusters.

Boto3 documentation of the "describe_replication_groups" function : https://boto3.amazonaws.com/v1/documentation/api/latest/reference/services/elasticache.html#ElastiCache.Client.describe_replication_groups

Moreover this pull request might violates the "integration tests" rule : I didn't find any integration test for the elasticache_info plugin and I don't know why no integration test have been made.

@ansibullbot
Copy link

@ansibullbot ansibullbot added community_review feature This issue/PR relates to a feature request module module needs_triage new_contributor Help guide this first time contributor plugins plugin (any type) labels Jul 16, 2021
@MKCG
Copy link
Contributor Author

MKCG commented Jul 17, 2021

Should I also includes my name as one of the author of this module ?
I don't think that this contribution is "big enough" since I merely included a function but I am not really familiar with this project policies.

@markuman
Copy link
Member

Sanity is failing

ERROR: Found 4 validate-modules issue(s) which need to be resolved:
ERROR: plugins/modules/elasticache_info.py:0:0: return-syntax-error: RETURN.elasticache_clusters.contains.replication_group.contains.node_groups.contains.reader_endpoint.address: extra keys not allowed @ data['elasticache_clusters']['contains']['replication_group']['contains']['node_groups']['contains']['reader_endpoint']['address']. Got {'description': 'The DNS hostname of the cache node', 'returned': 'always', 'type': 'str'}
ERROR: plugins/modules/elasticache_info.py:0:0: return-syntax-error: RETURN.elasticache_clusters.contains.replication_group.contains.node_groups.contains.reader_endpoint.description: required key not provided @ data['elasticache_clusters']['contains']['replication_group']['contains']['node_groups']['contains']['reader_endpoint']['description']. Got None
ERROR: plugins/modules/elasticache_info.py:0:0: return-syntax-error: RETURN.elasticache_clusters.contains.replication_group.contains.node_groups.contains.reader_endpoint.port: extra keys not allowed @ data['elasticache_clusters']['contains']['replication_group']['contains']['node_groups']['contains']['reader_endpoint']['port']. Got {'description': 'The port number that the cache engine is listening on', 'returned': 'always', 'type': 'int', 'sample': 6379}
ERROR: plugins/modules/elasticache_info.py:0:0: return-syntax-error: RETURN.elasticache_clusters.contains.replication_group.contains.node_groups.contains.reader_endpoint.type: required key not provided @ data['elasticache_clusters']['contains']['replication_group']['contains']['node_groups']['contains']['reader_endpoint']['type']. Got None
See documentation for help: https://docs.ansible.com/ansible/devel/dev_guide/testing/sanity/validate-modules.html

You can run sanity checks easy locally before pushing, see https://docs.ansible.com/ansible/latest/dev_guide/testing_sanity.html#testing-sanity

Furthermore it would be good when you verify the extended output in the integration test.

Should I also includes my name as one of the author of this module ?
I don't think that this contribution is "big enough"

Feel free, A contribution is a contribution, and every contribution is valueable!

alinabuzachis pushed a commit to alinabuzachis/community.aws that referenced this pull request May 25, 2022
…e-collections#646)

Handle ResourceNotFoundException while iterating certificates

SUMMARY

The module/utils/acm.py was not correctly handling deletion of certificates. While iterating over a list of certificates, the get_certificate function was making API calls to obtain more information about the certificates, but some certificates may be deleted while iterating.
Fixes ansible-collections#622

ISSUE TYPE


Bugfix Pull Request

COMPONENT NAME

acm.py
ADDITIONAL INFORMATION



Wow, it seems many tests are very flaky. I'm attempting to fix an issue in ACM, but problems occur elsewhere. Not to mention I raised this PR to fix ansible-collections#622, which was discovered while working on ansible-collections#870. And I discovered other issues as well, and so it looks like it's not possible to make any progress without going down a tree of bug fixes.
TypeError: 'NoneType' object is not subscriptable
fatal: [testhost]: FAILED! => {
    "changed": false,
    "module_stderr": "Traceback (most recent call last):\n  File \"<stdin>\", line 121, in <module>\n  File \"<stdin>\", line 113, in _ansiballz_main\n  File \"<stdin>\", line 61, in invoke_module\n  File \"/usr/lib64/python3.8/runpy.py\", line 207, in run_module\n    return _run_module_code(code, init_globals, run_name, mod_spec)\n  File \"/usr/lib64/python3.8/runpy.py\", line 97, in _run_module_code\n    _run_code(code, mod_globals, init_globals,\n  File \"/usr/lib64/python3.8/runpy.py\", line 87, in _run_code\n    exec(code, run_globals)\n  File \"/tmp/ansible_ec2_vpc_igw_payload_g4o1kl_r/ansible_ec2_vpc_igw_payload.zip/ansible_collections/amazon/aws/plugins/modules/ec2_vpc_igw.py\", line 248, in <module>\n  File \"/tmp/ansible_ec2_vpc_igw_payload_g4o1kl_r/ansible_ec2_vpc_igw_payload.zip/ansible_collections/amazon/aws/plugins/modules/ec2_vpc_igw.py\", line 242, in main\n  File \"/tmp/ansible_ec2_vpc_igw_payload_g4o1kl_r/ansible_ec2_vpc_igw_payload.zip/ansible_collections/amazon/aws/plugins/modules/ec2_vpc_igw.py\", line 132, in process\n  File \"/tmp/ansible_ec2_vpc_igw_payload_g4o1kl_r/ansible_ec2_vpc_igw_payload.zip/ansible_collections/amazon/aws/plugins/modules/ec2_vpc_igw.py\", line 220, in ensure_igw_present\n  File \"/tmp/ansible_ec2_vpc_igw_payload_g4o1kl_r/ansible_ec2_vpc_igw_payload.zip/ansible_collections/amazon/aws/plugins/modules/ec2_vpc_igw.py\", line 158, in get_igw_info\nTypeError: 'NoneType' object is not subscriptable\n",
    "module_stdout": "",
    "msg": "MODULE FAILURE\nSee stdout/stderr for the exact error",
    "rc": 1
}

Reviewed-by: Alina Buzachis <None>
Reviewed-by: Jill R <None>
@tremble tremble force-pushed the elasticache_info_fetch_replication_group_info branch from 2bc526b to a2af0b8 Compare July 1, 2022 09:44
@ansibullbot
Copy link

@ansibullbot ansibullbot added integration tests/integration tests tests labels Jul 1, 2022
@tremble tremble force-pushed the elasticache_info_fetch_replication_group_info branch from 437d96f to 5aaa42d Compare July 1, 2022 09:49
@tremble
Copy link
Contributor

tremble commented Jul 1, 2022

I've rebased and added some minimal integration tests. We don't currently support managing the replication so testing's pretty minimal, but is at least there.

@tremble tremble force-pushed the elasticache_info_fetch_replication_group_info branch from 5aaa42d to c1aa1c2 Compare July 1, 2022 09:53
@tremble tremble added the backport-4 PR should be backported to the stable-4 branch label Jul 1, 2022
@github-actions
Copy link

github-actions bot commented Jul 1, 2022

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.

✔️ ansible-galaxy-importer SUCCESS in 4m 17s (non-voting)
✔️ build-ansible-collection SUCCESS in 5m 15s
✔️ ansible-test-sanity-docker-devel SUCCESS in 9m 36s (non-voting)
✔️ ansible-test-sanity-docker-milestone SUCCESS in 9m 22s
✔️ ansible-test-sanity-docker-stable-2.12 SUCCESS in 10m 02s
✔️ ansible-test-sanity-docker-stable-2.13 SUCCESS in 10m 35s
✔️ ansible-test-units-community-aws-python38 SUCCESS in 6m 21s
✔️ ansible-test-units-community-aws-python39 SUCCESS in 6m 21s
✔️ ansible-test-splitter SUCCESS in 2m 37s
✔️ integration-community.aws-1 SUCCESS in 6m 36s
⚠️ integration-community.aws-2 SKIPPED
⚠️ integration-community.aws-3 SKIPPED
⚠️ integration-community.aws-4 SKIPPED
⚠️ integration-community.aws-5 SKIPPED
⚠️ integration-community.aws-6 SKIPPED
⚠️ integration-community.aws-7 SKIPPED
⚠️ integration-community.aws-8 SKIPPED
⚠️ integration-community.aws-9 SKIPPED
⚠️ integration-community.aws-10 SKIPPED
⚠️ integration-community.aws-11 SKIPPED
⚠️ integration-community.aws-12 SKIPPED
⚠️ integration-community.aws-13 SKIPPED

@tremble tremble added the mergeit Merge the PR (SoftwareFactory) label Jul 1, 2022
@softwarefactory-project-zuul
Copy link
Contributor

Build succeeded (gate pipeline).

✔️ ansible-galaxy-importer SUCCESS in 4m 01s (non-voting)
✔️ build-ansible-collection SUCCESS in 5m 59s
✔️ ansible-test-sanity-docker-devel SUCCESS in 9m 05s (non-voting)
✔️ ansible-test-sanity-docker-milestone SUCCESS in 9m 35s
✔️ ansible-test-sanity-docker-stable-2.12 SUCCESS in 9m 19s
✔️ ansible-test-sanity-docker-stable-2.13 SUCCESS in 11m 54s
✔️ ansible-test-units-community-aws-python38 SUCCESS in 6m 51s
✔️ ansible-test-units-community-aws-python39 SUCCESS in 6m 23s
✔️ ansible-test-splitter SUCCESS in 2m 35s
✔️ integration-community.aws-1 SUCCESS in 5m 25s
⚠️ integration-community.aws-2 SKIPPED
⚠️ integration-community.aws-3 SKIPPED
⚠️ integration-community.aws-4 SKIPPED
⚠️ integration-community.aws-5 SKIPPED
⚠️ integration-community.aws-6 SKIPPED
⚠️ integration-community.aws-7 SKIPPED
⚠️ integration-community.aws-8 SKIPPED
⚠️ integration-community.aws-9 SKIPPED
⚠️ integration-community.aws-10 SKIPPED
⚠️ integration-community.aws-11 SKIPPED
⚠️ integration-community.aws-12 SKIPPED
⚠️ integration-community.aws-13 SKIPPED

@softwarefactory-project-zuul softwarefactory-project-zuul bot merged commit d7e3617 into ansible-collections:main Jul 1, 2022
@patchback
Copy link

patchback bot commented Jul 1, 2022

Backport to stable-4: 💚 backport PR created

✅ Backport PR branch: patchback/backports/stable-4/d7e36178c674dc902af4925726a38483d7293f0a/pr-646

Backported as #1292

🤖 @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 1, 2022
…n group (#646)

[Plugin elasticache_info] includes the description of each replication group

SUMMARY
This pull request add the description of the corresponding Elasticache replication group into each described cache cluster.
ISSUE TYPE

Feature Pull Request

COMPONENT NAME
elasticache_info
ADDITIONAL INFORMATION
Actually the only way to describe Elasticache replication groups with Ansible is to use the dynamic inventory.
However, exposing the informations about each replication group could be useful in other contexts.
In my case, I am generating a few .env files and I need to include some Elasticache replication group primary endpoint and reader endpoint.
Since the elasticache_info plugin already exist and already retrieves the appropriate replication_group_id, I believe that this plugin should also retrieve the replication group description when it exists.
OTHER REMARKS CONCERNING THIS PULL REQUEST
Each provided description is copy/pasted from the official boto3 documentation which is distributed under an Apache 2.0 licence. I did it for consistency reason between and also because I am not a native speaker.
Also the provided description is incomplete : I only documented the fields that I was able to retrieve from my current AWS clusters.
Boto3 documentation of the "describe_replication_groups" function : https://boto3.amazonaws.com/v1/documentation/api/latest/reference/services/elasticache.html#ElastiCache.Client.describe_replication_groups
Moreover this pull request might violates the "integration tests" rule : I didn't find any integration test for the elasticache_info plugin and I don't know why no integration test have been made.

Reviewed-by: Mark Chappell <None>
(cherry picked from commit d7e3617)
@tremble
Copy link
Contributor

tremble commented Jul 1, 2022

Thanks for your submission, I'm sorry it's taken so long to get this merged.

softwarefactory-project-zuul bot pushed a commit that referenced this pull request Jul 1, 2022
…n group (#646) (#1292)

[PR #646/d7e36178 backport][stable-4] [Plugin elasticache_info] includes the description of each replication group

This is a backport of PR #646 as merged into main (d7e3617).
SUMMARY
This pull request add the description of the corresponding Elasticache replication group into each described cache cluster.
ISSUE TYPE

Feature Pull Request

COMPONENT NAME
elasticache_info
ADDITIONAL INFORMATION
Actually the only way to describe Elasticache replication groups with Ansible is to use the dynamic inventory.
However, exposing the informations about each replication group could be useful in other contexts.
In my case, I am generating a few .env files and I need to include some Elasticache replication group primary endpoint and reader endpoint.
Since the elasticache_info plugin already exist and already retrieves the appropriate replication_group_id, I believe that this plugin should also retrieve the replication group description when it exists.
OTHER REMARKS CONCERNING THIS PULL REQUEST
Each provided description is copy/pasted from the official boto3 documentation which is distributed under an Apache 2.0 licence. I did it for consistency reason between and also because I am not a native speaker.
Also the provided description is incomplete : I only documented the fields that I was able to retrieve from my current AWS clusters.
Boto3 documentation of the "describe_replication_groups" function : https://boto3.amazonaws.com/v1/documentation/api/latest/reference/services/elasticache.html#ElastiCache.Client.describe_replication_groups
Moreover this pull request might violates the "integration tests" rule : I didn't find any integration test for the elasticache_info plugin and I don't know why no integration test have been made.

Reviewed-by: Mark Chappell <None>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport-4 PR should be backported to the stable-4 branch community_review feature This issue/PR relates to a feature request integration tests/integration mergeit Merge the PR (SoftwareFactory) module module new_contributor Help guide this first time contributor plugins plugin (any type) tests tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants