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

Update ec2_vpc_net.py to allow setting ipv6_cidr: false in task. #1808

Closed
wants to merge 2 commits into from

Conversation

jscheible
Copy link

Module will fail if ipv6_cidr: false is set in task. This change fixes that bug.

SUMMARY

Module will fail if ipv6_cidr: false is set in task. This change fixes that bug.

ISSUE TYPE
  • Bugfix Pull Request
COMPONENT NAME

ec2_vpc_net

ADDITIONAL INFORMATION

If the option ipv6_cidr: false (which is the default), is used with this module,
the module fails with the following error:

fatal: [localhost]: FAILED! => {"changed": false, "msg": "Failed to wait for IPv6 CIDR association"}

One can comment that line out, but that does not solve the problem of having a set of VMs to create,
some of which will need IPV6 and some of which will not.

Before:

fatal: [localhost]: FAILED! => {"changed": false, "msg": "Failed to wait for IPv6 CIDR association"}

After:

changed: [localhost]

Module will fail if `ipv6_cidr: false` is set in task.  This change fixes that bug.
@softwarefactory-project-zuul
Copy link
Contributor

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

✔️ ansible-galaxy-importer SUCCESS in 5m 21s
✔️ build-ansible-collection SUCCESS in 12m 28s
✔️ ansible-test-splitter SUCCESS in 4m 45s
✔️ integration-amazon.aws-1 SUCCESS in 9m 43s
✔️ integration-community.aws-1 SUCCESS in 44m 35s
✔️ integration-community.aws-2 SUCCESS in 25m 05s
✔️ integration-community.aws-3 SUCCESS in 15m 14s
✔️ integration-community.aws-4 SUCCESS in 8m 17s
✔️ integration-community.aws-5 SUCCESS in 8m 29s
Skipped 38 jobs

@softwarefactory-project-zuul
Copy link
Contributor

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

✔️ ansible-galaxy-importer SUCCESS in 4m 42s
✔️ build-ansible-collection SUCCESS in 12m 36s
✔️ ansible-test-splitter SUCCESS in 5m 12s
✔️ integration-amazon.aws-1 SUCCESS in 10m 26s
✔️ integration-amazon.aws-2 SUCCESS in 9m 09s
✔️ integration-community.aws-1 SUCCESS in 7m 08s
✔️ integration-community.aws-2 SUCCESS in 24m 22s
✔️ integration-community.aws-3 SUCCESS in 48m 48s
✔️ integration-community.aws-4 SUCCESS in 54m 14s
✔️ integration-community.aws-5 SUCCESS in 7m 55s
✔️ integration-community.aws-6 SUCCESS in 26m 13s
✔️ integration-community.aws-7 SUCCESS in 21m 20s
✔️ integration-community.aws-8 SUCCESS in 36m 27s
✔️ integration-community.aws-9 SUCCESS in 25m 28s
✔️ integration-community.aws-10 SUCCESS in 29m 17s
✔️ integration-community.aws-11 SUCCESS in 34m 29s
✔️ integration-community.aws-12 SUCCESS in 56m 43s
✔️ integration-community.aws-13 SUCCESS in 13m 27s
✔️ integration-community.aws-14 SUCCESS in 14m 31s
✔️ integration-community.aws-15 SUCCESS in 28m 51s
✔️ integration-community.aws-16 SUCCESS in 27m 19s
integration-community.aws-17 FAILURE in 49m 53s
✔️ integration-community.aws-18 SUCCESS in 16m 42s
✔️ integration-community.aws-19 SUCCESS in 19m 39s
✔️ integration-community.aws-20 SUCCESS in 6m 34s
Skipped 22 jobs

Copy link
Contributor

@tremble tremble left a comment

Choose a reason for hiding this comment

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

Thanks for taking the time to submit this PR.

If the option ipv6_cidr: false (which is the default), is used with this module

That's not entirely correct and hasn't been since release 3.2.0 (April 2022), the current default is None, ie no change should be made. The documentation now says Default value is C(false) when creating a new VPC. because the default Amazon behaviour is not to associate an IPv6 CIDR. This wording is possibly confusing and can be found on line 64 if you have a better description.

It would be helpful if you could provide more information about the steps you're using to reproduce the issue.

We have integration tests for this module ( tests/integration/targets/ec2_vpc_net/tasks/main.yml ) which don't trigger the issue you're describing. It would be helpful if your PR could include an update to these tests which trigger the issue you're describing. Additionally please include a changelog fragment (https://docs.ansible.com/ansible/latest/community/development_process.html#changelogs-how-to)

@@ -384,7 +384,7 @@ def wait_for_vpc_ipv6_state(module, connection, vpc_id, ipv6_assoc_state):
If ipv6_assoc_state is False, wait for VPC to be dissociated from all Amazon-provided IPv6 CIDR blocks.
"""

if ipv6_assoc_state is None:
if not ipv6_assoc_state:
Copy link
Contributor

Choose a reason for hiding this comment

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

This change would mean that we don't wait for disassociations to complete, which would be the wrong behaviour. I suspect the bad behaviour is somewhere in the logic around line 410

@tremble
Copy link
Contributor

tremble commented Aug 30, 2024

Unable to reproduce, it's possible the issue was fixed between the version you were using when you hit the bug, and the version that was "main" at the time

The description references the default being False which was the case prior to #631 / #722 (3.2.0)

@tremble tremble closed this Aug 30, 2024
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.

2 participants