Prepare ec2_vpc_vgw_* modules for promotion #2171

mandar242 · 2024-10-10T17:59:47Z

SUMMARY

Depends-On: ansible-collections/amazon.aws#2331
Prepare ec2_vpc_vgw_* modules for promotion

ISSUE TYPE

Bugfix Pull Request
Docs Pull Request
Feature Pull Request
New Module Pull Request

COMPONENT NAME

ec2_vpc_vgw
ec2_vpc_vgw_info

ADDITIONAL INFORMATION

github-actions · 2024-10-10T18:02:37Z

Docs Build 📝

Thank you for contribution!✨

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

plugins/modules/ec2_vpc_vgw.py

plugins/modules/ec2_vpc_vgw_info.py

plugins/modules/ec2_vpc_vgw.py

plugins/modules/ec2_vpc_vgw_info.py

abikouo

Please update the documentation of the ec2_vpc_vgw module, the state argument has values C(present) and C(absent), should be V(present) and V(absent) instead.
You also need to update the ec2_vpc_vgw integration test case: add uppercase to tasks names and use FQCN for module.

plugins/modules/ec2_vpc_vgw.py

plugins/modules/ec2_vpc_vgw_info.py

plugins/modules/ec2_vpc_vgw.py

alinabuzachis

Can you also check why some tasks are commented out in the integration tests https://github.com/ansible-collections/community.aws/blob/main/tests/integration/targets/ec2_vpc_vgw/tasks/tags.yml and make sure that the module is fully tested, adding more tests if necessary?

plugins/modules/ec2_vpc_vgw.py

plugins/modules/ec2_vpc_vgw_info.py

mandar242 · 2024-10-17T05:10:46Z

Can you also check why some tasks are commented out in the integration tests https://github.com/ansible-collections/community.aws/blob/main/tests/integration/targets/ec2_vpc_vgw/tasks/tags.yml and make sure that the module is fully tested, adding more tests if necessary?

ec2_vpc_vgw did not had check_mode support. Added check_mode support and enabled tests, added more tests.

softwarefactory-project-zuul · 2024-10-17T20:17:41Z

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

✔️ ansible-galaxy-importer SUCCESS in 3m 18s (non-voting)
✔️ build-ansible-collection SUCCESS in 10m 38s
✔️ ansible-test-splitter SUCCESS in 4m 06s
❌ integration-community.aws-1 FAILURE in 5m 40s
Skipped 21 jobs

plugins/modules/ec2_vpc_vgw.py

tests/integration/targets/ec2_vpc_vgw/tasks/tags.yml

plugins/modules/ec2_vpc_vgw.py

alinabuzachis · 2024-10-18T11:26:48Z

plugins/modules/ec2_vpc_vgw.py

+        if desired_status == "attached":
+            wait_for_resource_state(
+                client,
+                module,
+                "vpn_gateway_attached",
+                VpnGatewayIds=vpn_gateway_id,
+                delay=polling_increment_secs,
+                max_attempts=max_retries,
+            )
+        elif desired_status == "detached":
+            wait_for_resource_state(
+                client,
+                module,
+                "vpn_gateway_detached",
+                VpnGatewayIds=vpn_gateway_id,
+                delay=polling_increment_secs,
+                max_attempts=max_retries,
+            )


Suggested change

if desired_status == "attached":

wait_for_resource_state(

client,

module,

"vpn_gateway_attached",

VpnGatewayIds=vpn_gateway_id,

delay=polling_increment_secs,

max_attempts=max_retries,

)

elif desired_status == "detached":

wait_for_resource_state(

client,

module,

"vpn_gateway_detached",

VpnGatewayIds=vpn_gateway_id,

delay=polling_increment_secs,

max_attempts=max_retries,

)

if desired_status not in ("attached", "detached"):

module.fail_json(msg=f"Unsupported status: {desired_status}")

wait_for_resource_state(

client,

module,

f"vpn_gateway_{desired_status}",

VpnGatewayIds=[vpn_gateway_id],

delay=polling_increment_secs,

max_attempts=max_retries,

)

changing VpnGatewayIds=vpn_gateway_id to VpnGatewayIds=[vpn_gateway_id] causes failure with

"msg": "An exception happened while trying to wait for 'vpn_gateway_attached': Parameter validation failed:\nInvalid type for parameter VpnGatewayIds[0], value: ['vgw-0c3df60baf0acc49f'], type: <class 'list'>, valid types: <class 'str'>", "resource_actions": [ "ec2:DescribeVpnGateways", "ec2:CreateVpnGateway", "ec2:AttachVpnGateway" ]

even though API states otherwise it to be a list of str, weird.

Same as below.

Hi @alinabuzachis , fixed. It was indeed being passed as List[Str] at the API call.
Only thing was that it was handled to be passed as List[Str] earlier in the function call tree.
Updated the code to ensure it is passed as List[Str] before calling module_utils/ec2 functions.
27b07e0

alinabuzachis · 2024-10-18T11:28:47Z

plugins/modules/ec2_vpc_vgw.py


    result = response
    return result


-def find_vgw(client, module, vpn_gateway_id=None):
+def find_vgw(
+    client, module: AnsibleAWSModule, vpn_gateway_id: Optional[Union[str, List[str]]] = None


Suggested change

client, module: AnsibleAWSModule, vpn_gateway_id: Optional[Union[str, List[str]]] = None

client, module: AnsibleAWSModule, vpn_gateway_id: Optional[str] = None

We should also update the way the VpcGatewayIds attribute of the describe_vpn_gateways()
See suggestion below

Hi @alinabuzachis @abikouo , fixed. It was indeed being passed as List[Str] at the API call.
Only thing was that it was handled to be passed as List[Str] earlier in the function call tree.
Updated the code to ensure it is passed as List[Str] before calling module_utils/ec2 functions.
27b07e0

alinabuzachis · 2024-10-18T11:29:02Z

plugins/modules/ec2_vpc_vgw.py

-def find_vgw(client, module, vpn_gateway_id=None):
+def find_vgw(
+    client, module: AnsibleAWSModule, vpn_gateway_id: Optional[Union[str, List[str]]] = None
+) -> List[Dict[str, Any]]:
    params = dict()
    if vpn_gateway_id:
        params["VpnGatewayIds"] = vpn_gateway_id


Suggested change

params["VpnGatewayIds"] = vpn_gateway_id

params["VpnGatewayIds"] = [vpn_gateway_id]

not sure what happens with the API but it fails with

An exception occurred during task execution. To see the full traceback, use -vvv. The error was: Invalid type for parameter VpnGatewayIds[0], value: ['vgw-0849b49ec813d18ee'], type: <class 'list'>, valid types: <class 'str'>

Same as below.

Hi @alinabuzachis , fixed. It was indeed being passed as List[Str] at the API call.
Only thing was that it was handled to be passed as List[Str] earlier in the function call tree.
Updated the code to ensure it is passed as List[Str] before calling module_utils/ec2 functions.
27b07e0

plugins/modules/ec2_vpc_vgw.py

softwarefactory-project-zuul · 2024-10-19T01:29:01Z

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

❌ ansible-galaxy-importer FAILURE in 3m 46s (non-voting)
✔️ build-ansible-collection SUCCESS in 10m 05s
✔️ ansible-test-splitter SUCCESS in 3m 51s
❌ integration-community.aws-1 FAILURE in 7m 13s
Skipped 21 jobs

softwarefactory-project-zuul · 2024-10-19T01:49:35Z

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

❌ ansible-galaxy-importer FAILURE in 3m 16s (non-voting)
✔️ build-ansible-collection SUCCESS in 10m 07s
✔️ ansible-test-splitter SUCCESS in 3m 52s
❌ integration-community.aws-1 FAILURE in 4m 42s
Skipped 21 jobs

softwarefactory-project-zuul · 2024-10-19T02:11:51Z

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

❌ ansible-galaxy-importer FAILURE in 5m 09s (non-voting)
✔️ build-ansible-collection SUCCESS in 10m 34s
✔️ ansible-test-splitter SUCCESS in 3m 53s
❌ integration-community.aws-1 FAILURE in 6m 12s
Skipped 21 jobs

…n call

softwarefactory-project-zuul · 2024-10-21T21:12:50Z

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

❌ ansible-galaxy-importer FAILURE in 4m 38s (non-voting)
✔️ build-ansible-collection SUCCESS in 11m 11s
✔️ ansible-test-splitter SUCCESS in 3m 54s
❌ integration-community.aws-1 FAILURE in 7m 21s
Skipped 21 jobs

alinabuzachis · 2024-10-22T10:47:12Z

recheck

softwarefactory-project-zuul · 2024-10-22T13:05:26Z

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

❌ ansible-galaxy-importer FAILURE in 4m 55s (non-voting)
✔️ build-ansible-collection SUCCESS in 10m 48s
✔️ ansible-test-splitter SUCCESS in 4m 19s
❌ integration-community.aws-1 FAILURE in 40m 04s
✔️ integration-community.aws-2 SUCCESS in 22m 59s
✔️ integration-community.aws-3 SUCCESS in 15m 27s
✔️ integration-community.aws-4 SUCCESS in 10m 02s
✔️ integration-community.aws-5 SUCCESS in 8m 10s
✔️ integration-community.aws-6 SUCCESS in 8m 08s
✔️ integration-community.aws-7 SUCCESS in 21m 47s
✔️ integration-community.aws-8 SUCCESS in 11m 42s
✔️ integration-community.aws-9 SUCCESS in 7m 57s
Skipped 13 jobs

alinabuzachis · 2024-10-22T16:40:44Z

recheck

alinabuzachis

Can you please ensure that the other linter test are green (don't worry about the import errors for the moment)?

ERROR: plugins/modules/ec2_vpc_vgw.py:99:30: invalid-examples: EXAMPLES is not valid YAML
ERROR: plugins/modules/ec2_vpc_vgw_info.py:0:0: invalid-documentation-markup: Directive "RV(resource_tags)" contains a non-existing return value "resource_tags"
ERROR: plugins/modules/ec2_vpc_vgw_info.py:0:0: invalid-documentation-markup: Directive "RV(tags)" contains a non-existing return value "tags"

softwarefactory-project-zuul · 2024-10-22T17:49:00Z

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

✔️ ansible-galaxy-importer SUCCESS in 3m 12s (non-voting)
✔️ build-ansible-collection SUCCESS in 11m 04s
✔️ ansible-test-splitter SUCCESS in 4m 19s
✔️ integration-community.aws-1 SUCCESS in 37m 37s
✔️ integration-community.aws-2 SUCCESS in 31m 01s
❌ integration-community.aws-3 FAILURE in 14m 52s
✔️ integration-community.aws-4 SUCCESS in 6m 53s
✔️ integration-community.aws-5 SUCCESS in 9m 45s
✔️ integration-community.aws-6 SUCCESS in 6m 41s
✔️ integration-community.aws-7 SUCCESS in 24m 42s
✔️ integration-community.aws-8 SUCCESS in 9m 30s
❌ integration-community.aws-9 FAILURE in 21m 25s
Skipped 13 jobs

alinabuzachis · 2024-10-22T18:58:02Z

recheck

alinabuzachis · 2024-10-22T19:02:49Z

@mandar242 there are still other linting issues.

softwarefactory-project-zuul · 2024-10-22T20:41:11Z

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

❌ ansible-galaxy-importer FAILURE in 4m 29s (non-voting)
✔️ build-ansible-collection SUCCESS in 10m 22s
✔️ ansible-test-splitter SUCCESS in 4m 01s
✔️ integration-community.aws-1 SUCCESS in 9m 31s
Skipped 21 jobs

When state == absent, the expectation and the current behavior is to delete the vgw.

softwarefactory-project-zuul · 2024-10-23T13:49:28Z

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

❌ ansible-galaxy-importer FAILURE in 3m 46s (non-voting)
✔️ build-ansible-collection SUCCESS in 10m 06s
✔️ ansible-test-splitter SUCCESS in 4m 09s
✔️ integration-community.aws-1 SUCCESS in 9m 04s
Skipped 21 jobs

SUMMARY Depends-On: ansible-collections/amazon.aws#2331 Prepare ec2_vpc_vgw_* modules for promotion ISSUE TYPE Bugfix Pull Request Docs Pull Request Feature Pull Request New Module Pull Request COMPONENT NAME ec2_vpc_vgw ec2_vpc_vgw_info ADDITIONAL INFORMATION Reviewed-by: Alina Buzachis Reviewed-by: Mandar Kulkarni <[email protected]> Reviewed-by: Bikouo Aubin Reviewed-by: GomathiselviS <[email protected]>

mandar242 mentioned this pull request Oct 10, 2024

Add utils for ec2_vpc_vgw_* ansible-collections/amazon.aws#2331

Merged

This comment was marked as outdated.

Sign in to view

alinabuzachis reviewed Oct 14, 2024

View reviewed changes

mandar242 force-pushed the prepare_ec2_vpc_vgw_ branch from a7ef6b2 to a7550db Compare October 15, 2024 21:20

This comment was marked as outdated.

Sign in to view

mandar242 requested a review from alinabuzachis October 16, 2024 01:04

This comment was marked as outdated.

Sign in to view

abikouo requested changes Oct 16, 2024

View reviewed changes

alinabuzachis reviewed Oct 16, 2024

View reviewed changes

plugins/modules/ec2_vpc_vgw.py Outdated Show resolved Hide resolved

alinabuzachis reviewed Oct 16, 2024

View reviewed changes

GomathiselviS requested changes Oct 16, 2024

View reviewed changes

plugins/modules/ec2_vpc_vgw.py Outdated Show resolved Hide resolved

plugins/modules/ec2_vpc_vgw.py Outdated Show resolved Hide resolved

plugins/modules/ec2_vpc_vgw_info.py Show resolved Hide resolved

This comment was marked as outdated.

Sign in to view

mandar242 force-pushed the prepare_ec2_vpc_vgw_ branch from b8497ac to 29639e8 Compare October 17, 2024 02:58

This comment was marked as outdated.

Sign in to view

mandar242 requested review from alinabuzachis, GomathiselviS and abikouo October 17, 2024 05:11

This comment was marked as outdated.

Sign in to view

alinabuzachis reviewed Oct 18, 2024

View reviewed changes

plugins/modules/ec2_vpc_vgw.py Outdated Show resolved Hide resolved

plugins/modules/ec2_vpc_vgw.py Outdated Show resolved Hide resolved

plugins/modules/ec2_vpc_vgw.py Show resolved Hide resolved

plugins/modules/ec2_vpc_vgw.py Outdated Show resolved Hide resolved

alinabuzachis reviewed Oct 18, 2024

View reviewed changes

tests/integration/targets/ec2_vpc_vgw/tasks/tags.yml Outdated Show resolved Hide resolved

alinabuzachis reviewed Oct 18, 2024

View reviewed changes

plugins/modules/ec2_vpc_vgw.py Outdated Show resolved Hide resolved

alinabuzachis reviewed Oct 18, 2024

View reviewed changes

plugins/modules/ec2_vpc_vgw.py Outdated Show resolved Hide resolved

alinabuzachis reviewed Oct 18, 2024

View reviewed changes

plugins/modules/ec2_vpc_vgw.py Outdated Show resolved Hide resolved

plugins/modules/ec2_vpc_vgw.py Outdated Show resolved Hide resolved

alinabuzachis reviewed Oct 18, 2024

View reviewed changes

plugins/modules/ec2_vpc_vgw.py Outdated Show resolved Hide resolved

feedback modifications round 3

d35a5fd

feedback modifications round 4

4f07764

mandar242 requested review from alinabuzachis, abikouo and GomathiselviS October 19, 2024 01:55

ensure vpn_gateway_id is passed as list[str] before the final functio…

27b07e0

…n call

alinabuzachis reviewed Oct 22, 2024

View reviewed changes

minor fixes

938dc1c

mandar242 requested a review from alinabuzachis October 22, 2024 16:57

linter fix

600d783

alinabuzachis approved these changes Oct 23, 2024

View reviewed changes

GomathiselviS approved these changes Oct 23, 2024

View reviewed changes

GomathiselviS added the mergeit Merge the PR (SoftwareFactory) label Oct 23, 2024

softwarefactory-project-zuul bot merged commit b78cba1 into ansible-collections:main Oct 23, 2024
66 of 67 checks passed

	client, module: AnsibleAWSModule, vpn_gateway_id: Optional[Union[str, List[str]]] = None
	client, module: AnsibleAWSModule, vpn_gateway_id: Optional[str] = None

	params["VpnGatewayIds"] = vpn_gateway_id
	params["VpnGatewayIds"] = [vpn_gateway_id]

Prepare ec2_vpc_vgw_* modules for promotion #2171

Prepare ec2_vpc_vgw_* modules for promotion #2171

Conversation

mandar242 commented Oct 10, 2024 • edited by alinabuzachis Loading

SUMMARY

ISSUE TYPE

COMPONENT NAME

ADDITIONAL INFORMATION

github-actions bot commented Oct 10, 2024 • edited Loading

Docs Build 📝

This comment was marked as outdated.

This comment was marked as outdated.

This comment was marked as outdated.

This comment was marked as outdated.

abikouo left a comment

Choose a reason for hiding this comment

alinabuzachis left a comment

Choose a reason for hiding this comment

This comment was marked as outdated.

This comment was marked as outdated.

mandar242 commented Oct 17, 2024

This comment was marked as outdated.

softwarefactory-project-zuul bot commented Oct 17, 2024

alinabuzachis Oct 18, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

softwarefactory-project-zuul bot commented Oct 19, 2024

softwarefactory-project-zuul bot commented Oct 19, 2024

softwarefactory-project-zuul bot commented Oct 19, 2024

softwarefactory-project-zuul bot commented Oct 21, 2024

alinabuzachis commented Oct 22, 2024

softwarefactory-project-zuul bot commented Oct 22, 2024

alinabuzachis commented Oct 22, 2024

alinabuzachis left a comment

Choose a reason for hiding this comment

softwarefactory-project-zuul bot commented Oct 22, 2024

alinabuzachis commented Oct 22, 2024

alinabuzachis commented Oct 22, 2024

softwarefactory-project-zuul bot commented Oct 22, 2024

softwarefactory-project-zuul bot commented Oct 23, 2024

mandar242 commented Oct 10, 2024 •

edited by alinabuzachis

Loading

github-actions bot commented Oct 10, 2024 •

edited

Loading

alinabuzachis Oct 18, 2024 •

edited

Loading