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

Don't ignore IPv6 addresses when a single network interface is specified #1979

Merged
merged 2 commits into from
May 24, 2024

Conversation

maggu
Copy link
Contributor

@maggu maggu commented Feb 20, 2024

SUMMARY

When "ipv6_adresses" are used with "network" without specifying "interfaces", the parameter is ignored and no IPv6 adresses get set. I don't believe this is the intended or correct behaviour.

ISSUE TYPE
  • Bugfix Pull Request
COMPONENT NAME

ec2_instance

ADDITIONAL INFORMATION

Copy link
Contributor

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

✔️ ansible-galaxy-importer SUCCESS in 5m 51s
✔️ build-ansible-collection SUCCESS in 15m 15s
✔️ ansible-test-splitter SUCCESS in 5m 40s
✔️ integration-amazon.aws-1 SUCCESS in 22m 52s
✔️ integration-amazon.aws-2 SUCCESS in 17m 07s
✔️ integration-amazon.aws-3 SUCCESS in 13m 39s
✔️ integration-amazon.aws-4 SUCCESS in 11m 13s
✔️ integration-amazon.aws-5 SUCCESS in 8m 13s
✔️ integration-amazon.aws-6 SUCCESS in 6m 02s
✔️ integration-amazon.aws-7 SUCCESS in 7m 47s
✔️ integration-amazon.aws-8 SUCCESS in 13m 18s
✔️ integration-amazon.aws-9 SUCCESS in 11m 14s
✔️ integration-amazon.aws-10 SUCCESS in 11m 43s
✔️ integration-amazon.aws-11 SUCCESS in 7m 44s
✔️ integration-amazon.aws-12 SUCCESS in 11m 37s
✔️ integration-amazon.aws-13 SUCCESS in 12m 15s
✔️ integration-amazon.aws-14 SUCCESS in 10m 59s
✔️ integration-amazon.aws-15 SUCCESS in 7m 13s
✔️ integration-amazon.aws-16 SUCCESS in 7m 57s
✔️ integration-amazon.aws-17 SUCCESS in 6m 33s
✔️ integration-amazon.aws-18 SUCCESS in 9m 24s
✔️ integration-amazon.aws-19 SUCCESS in 8m 10s
Skipped 25 jobs

Copy link
Contributor

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

✔️ ansible-galaxy-importer SUCCESS in 3m 59s
✔️ build-ansible-collection SUCCESS in 14m 25s
✔️ ansible-test-splitter SUCCESS in 5m 22s
✔️ integration-amazon.aws-1 SUCCESS in 25m 44s
✔️ integration-amazon.aws-2 SUCCESS in 11m 54s
✔️ integration-amazon.aws-3 SUCCESS in 12m 40s
✔️ integration-amazon.aws-4 SUCCESS in 12m 40s
✔️ integration-amazon.aws-5 SUCCESS in 7m 21s
✔️ integration-amazon.aws-6 SUCCESS in 7m 13s
✔️ integration-amazon.aws-7 SUCCESS in 6m 43s
✔️ integration-amazon.aws-8 SUCCESS in 13m 31s
✔️ integration-amazon.aws-9 SUCCESS in 11m 19s
✔️ integration-amazon.aws-10 SUCCESS in 8m 10s
✔️ integration-amazon.aws-11 SUCCESS in 8m 45s
✔️ integration-amazon.aws-12 SUCCESS in 12m 09s
✔️ integration-amazon.aws-13 SUCCESS in 11m 02s
✔️ integration-amazon.aws-14 SUCCESS in 9m 53s
✔️ integration-amazon.aws-15 SUCCESS in 8m 48s
✔️ integration-amazon.aws-16 SUCCESS in 9m 05s
✔️ integration-amazon.aws-17 SUCCESS in 6m 51s
✔️ integration-amazon.aws-18 SUCCESS in 8m 53s
✔️ integration-amazon.aws-19 SUCCESS in 9m 14s
Skipped 25 jobs

@alinabuzachis
Copy link
Contributor

@maggu Thanks for taking the time to submit this PR. Can you please add a changelog fragment https://docs.ansible.com/ansible/latest/community/development_process.html#changelogs-how-to?
To help avoid regressions, please cover this change with integration tests.

@maggu
Copy link
Contributor Author

maggu commented Mar 28, 2024

@alinabuzachis Thanks for the reply and feedback. Regarding integration tests, there appear to currently be no tests at all for checking ec2_instance network settings.

What do you think would be the most appropriate approach for this? Adding it to ec2_instance_instance_*? Creating a new ec2_instance_network to test network settings? Something else, like ipv6_tests to test ivp6 settings across multiple modules?

Copy link
Contributor

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

✔️ ansible-galaxy-importer SUCCESS in 4m 29s
✔️ build-ansible-collection SUCCESS in 14m 26s
✔️ ansible-test-splitter SUCCESS in 5m 42s
✔️ integration-amazon.aws-1 SUCCESS in 25m 22s
✔️ integration-amazon.aws-2 SUCCESS in 20m 52s
✔️ integration-amazon.aws-3 SUCCESS in 13m 07s
✔️ integration-amazon.aws-4 SUCCESS in 14m 25s
✔️ integration-amazon.aws-5 SUCCESS in 9m 03s
✔️ integration-amazon.aws-6 SUCCESS in 6m 28s
✔️ integration-amazon.aws-7 SUCCESS in 12m 46s
✔️ integration-amazon.aws-8 SUCCESS in 14m 05s
✔️ integration-amazon.aws-9 SUCCESS in 8m 20s
✔️ integration-amazon.aws-10 SUCCESS in 11m 43s
✔️ integration-amazon.aws-11 SUCCESS in 12m 23s
✔️ integration-amazon.aws-12 SUCCESS in 16m 52s
✔️ integration-amazon.aws-13 SUCCESS in 12m 47s
✔️ integration-amazon.aws-14 SUCCESS in 11m 44s
✔️ integration-amazon.aws-15 SUCCESS in 8m 36s
✔️ integration-amazon.aws-16 SUCCESS in 14m 17s
✔️ integration-amazon.aws-17 SUCCESS in 12m 03s
✔️ integration-amazon.aws-18 SUCCESS in 9m 16s
✔️ integration-amazon.aws-19 SUCCESS in 12m 19s
Skipped 25 jobs

@maggu
Copy link
Contributor Author

maggu commented Apr 6, 2024

@alinabuzachis Changelog fragment added, and rebased to current main.

I believe that integration tests might need a wider scope than this PR, considering that currently similar functionality isn't tested and there's no obvious place to add it. Such tests would obviously be beneficial though.

Copy link
Contributor

@hakbailey hakbailey left a comment

Choose a reason for hiding this comment

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

I agree there is more work to do on this part of the module (including adding tests for the network interface options) and have added an issue for that, but for now this seems like a good bugfix.

Copy link
Contributor

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

✔️ ansible-galaxy-importer SUCCESS in 4m 48s
✔️ build-ansible-collection SUCCESS in 14m 46s
✔️ ansible-test-splitter SUCCESS in 6m 17s
✔️ integration-amazon.aws-1 SUCCESS in 28m 28s
✔️ integration-amazon.aws-2 SUCCESS in 17m 13s
✔️ integration-amazon.aws-3 SUCCESS in 14m 01s
✔️ integration-amazon.aws-4 SUCCESS in 11m 30s
✔️ integration-amazon.aws-5 SUCCESS in 12m 30s
✔️ integration-amazon.aws-6 SUCCESS in 10m 15s
✔️ integration-amazon.aws-7 SUCCESS in 13m 39s
✔️ integration-amazon.aws-8 SUCCESS in 13m 53s
✔️ integration-amazon.aws-9 SUCCESS in 16m 27s
✔️ integration-amazon.aws-10 SUCCESS in 8m 40s
✔️ integration-amazon.aws-11 SUCCESS in 12m 37s
✔️ integration-amazon.aws-12 SUCCESS in 17m 14s
✔️ integration-amazon.aws-13 SUCCESS in 15m 46s
✔️ integration-amazon.aws-14 SUCCESS in 11m 59s
✔️ integration-amazon.aws-15 SUCCESS in 11m 57s
✔️ integration-amazon.aws-16 SUCCESS in 12m 41s
✔️ integration-amazon.aws-17 SUCCESS in 8m 28s
✔️ integration-amazon.aws-18 SUCCESS in 11m 42s
✔️ integration-amazon.aws-19 SUCCESS in 14m 47s
✔️ integration-community.aws-1 SUCCESS in 22m 57s
✔️ integration-community.aws-2 SUCCESS in 10m 10s
Skipped 23 jobs

@alinabuzachis alinabuzachis added backport-7 PR should be backported to the stable-7 branch backport-8 PR should be backported to the stable-8 branch labels May 22, 2024
@GomathiselviS
Copy link
Contributor

@maggu If you can rebase this PR, we can merge it.

@abikouo abikouo added the mergeit Merge the PR (SoftwareFactory) label May 24, 2024
Copy link
Contributor

Build failed (gate pipeline). For information on how to proceed, see
http://docs.openstack.org/infra/manual/developers.html#automated-testing

https://ansible.softwarefactory-project.io/zuul/buildset/43b69c4e135e429c9f7a8cbd689dc787

✔️ ansible-galaxy-importer SUCCESS in 6m 00s
✔️ build-ansible-collection SUCCESS in 19m 05s
✔️ ansible-test-splitter SUCCESS in 6m 32s
✔️ integration-amazon.aws-1 SUCCESS in 19m 07s
✔️ integration-amazon.aws-2 SUCCESS in 13m 57s
✔️ integration-amazon.aws-3 SUCCESS in 9m 00s
✔️ integration-amazon.aws-4 SUCCESS in 8m 20s
✔️ integration-amazon.aws-5 SUCCESS in 8m 20s
✔️ integration-amazon.aws-6 SUCCESS in 7m 07s
integration-amazon.aws-7 FAILURE in 9m 43s
✔️ integration-amazon.aws-8 SUCCESS in 9m 10s
✔️ integration-amazon.aws-9 SUCCESS in 8m 05s
✔️ integration-amazon.aws-10 SUCCESS in 12m 27s
✔️ integration-amazon.aws-11 SUCCESS in 16m 54s
✔️ integration-amazon.aws-12 SUCCESS in 17m 54s
✔️ integration-amazon.aws-13 SUCCESS in 11m 39s
✔️ integration-amazon.aws-14 SUCCESS in 12m 36s
✔️ integration-amazon.aws-15 SUCCESS in 8m 49s
✔️ integration-amazon.aws-16 SUCCESS in 10m 02s
✔️ integration-amazon.aws-17 SUCCESS in 11m 42s
✔️ integration-amazon.aws-18 SUCCESS in 7m 17s
✔️ integration-amazon.aws-19 SUCCESS in 8m 28s
✔️ integration-community.aws-1 SUCCESS in 46m 02s
Skipped 24 jobs

@abikouo
Copy link
Contributor

abikouo commented May 24, 2024

regate

Copy link
Contributor

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

✔️ ansible-galaxy-importer SUCCESS in 4m 52s
✔️ build-ansible-collection SUCCESS in 14m 53s
✔️ ansible-test-splitter SUCCESS in 5m 46s
✔️ integration-amazon.aws-1 SUCCESS in 29m 59s
✔️ integration-amazon.aws-2 SUCCESS in 19m 39s
✔️ integration-amazon.aws-3 SUCCESS in 13m 16s
✔️ integration-amazon.aws-4 SUCCESS in 12m 02s
✔️ integration-amazon.aws-5 SUCCESS in 11m 51s
✔️ integration-amazon.aws-6 SUCCESS in 10m 19s
✔️ integration-amazon.aws-7 SUCCESS in 10m 48s
✔️ integration-amazon.aws-8 SUCCESS in 17m 45s
✔️ integration-amazon.aws-9 SUCCESS in 15m 04s
✔️ integration-amazon.aws-10 SUCCESS in 11m 20s
✔️ integration-amazon.aws-11 SUCCESS in 7m 08s
✔️ integration-amazon.aws-12 SUCCESS in 10m 55s
✔️ integration-amazon.aws-13 SUCCESS in 10m 45s
✔️ integration-amazon.aws-14 SUCCESS in 7m 24s
✔️ integration-amazon.aws-15 SUCCESS in 7m 26s
✔️ integration-amazon.aws-16 SUCCESS in 16m 24s
✔️ integration-amazon.aws-17 SUCCESS in 8m 57s
✔️ integration-amazon.aws-18 SUCCESS in 11m 25s
✔️ integration-amazon.aws-19 SUCCESS in 8m 26s
✔️ integration-community.aws-1 SUCCESS in 56m 53s
Skipped 24 jobs

@softwarefactory-project-zuul softwarefactory-project-zuul bot merged commit 0d1a604 into ansible-collections:main May 24, 2024
36 of 37 checks passed
Copy link

patchback bot commented May 24, 2024

Backport to stable-7: 💚 backport PR created

✅ Backport PR branch: patchback/backports/stable-7/0d1a604b9aaa1c4e35274eaf3ebfe97fdd5fd00c/pr-1979

Backported as #2112

🤖 @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 May 24, 2024
…ied (#1979)

Don't ignore IPv6 addresses when a single network interface is specified

SUMMARY

When "ipv6_adresses" are used with "network" without specifying "interfaces", the parameter is ignored and no IPv6 adresses get set. I don't believe this is the intended or correct behaviour.

ISSUE TYPE

Bugfix Pull Request

COMPONENT NAME

ec2_instance
ADDITIONAL INFORMATION

Reviewed-by: Helen Bailey <[email protected]>
Reviewed-by: Bikouo Aubin
Reviewed-by: Alina Buzachis
(cherry picked from commit 0d1a604)
Copy link

patchback bot commented May 24, 2024

Backport to stable-8: 💚 backport PR created

✅ Backport PR branch: patchback/backports/stable-8/0d1a604b9aaa1c4e35274eaf3ebfe97fdd5fd00c/pr-1979

Backported as #2113

🤖 @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 May 24, 2024
…ied (#1979)

Don't ignore IPv6 addresses when a single network interface is specified

SUMMARY

When "ipv6_adresses" are used with "network" without specifying "interfaces", the parameter is ignored and no IPv6 adresses get set. I don't believe this is the intended or correct behaviour.

ISSUE TYPE

Bugfix Pull Request

COMPONENT NAME

ec2_instance
ADDITIONAL INFORMATION

Reviewed-by: Helen Bailey <[email protected]>
Reviewed-by: Bikouo Aubin
Reviewed-by: Alina Buzachis
(cherry picked from commit 0d1a604)
gravesm pushed a commit that referenced this pull request Jun 4, 2024
…ied (#1979) (#2113)

Don't ignore IPv6 addresses when a single network interface is specified

SUMMARY

When "ipv6_adresses" are used with "network" without specifying "interfaces", the parameter is ignored and no IPv6 adresses get set. I don't believe this is the intended or correct behaviour.

ISSUE TYPE

Bugfix Pull Request

COMPONENT NAME

ec2_instance
ADDITIONAL INFORMATION

Reviewed-by: Helen Bailey <[email protected]>
Reviewed-by: Bikouo Aubin
Reviewed-by: Alina Buzachis
(cherry picked from commit 0d1a604)

Co-authored-by: C C Magnus Gustavsson <[email protected]>
gravesm pushed a commit that referenced this pull request Jun 4, 2024
…ied (#1979) (#2112)

Don't ignore IPv6 addresses when a single network interface is specified

SUMMARY

When "ipv6_adresses" are used with "network" without specifying "interfaces", the parameter is ignored and no IPv6 adresses get set. I don't believe this is the intended or correct behaviour.

ISSUE TYPE

Bugfix Pull Request

COMPONENT NAME

ec2_instance
ADDITIONAL INFORMATION

Reviewed-by: Helen Bailey <[email protected]>
Reviewed-by: Bikouo Aubin
Reviewed-by: Alina Buzachis
(cherry picked from commit 0d1a604)

Co-authored-by: C C Magnus Gustavsson <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport-7 PR should be backported to the stable-7 branch 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.

None yet

5 participants