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

change ip4 type to list of str #3738

Merged

Conversation

haddystuff
Copy link
Contributor

SUMMARY

This PR adds support of multiple ipv4 addresses on one interface which was requested here #1088.

ISSUE TYPE
  • Feature Pull Request
COMPONENT NAME

plugins/modules/net_tools/nmcli.py

ADDITIONAL INFORMATION

As it turns out changing type of parameter is enough to make it work.
Now list can be used in ip4 parameter in playbook, but string is also working, so it's backwards compatible.
I didn't add any additional test or documentation just for now. I think such change need to be discussed first.

@ansibullbot
Copy link
Collaborator

@ansibullbot ansibullbot added feature This issue/PR relates to a feature request module module needs_ci This PR requires CI testing to be performed. Please close and re-open this PR to trigger CI needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR net_tools plugins plugin (any type) and removed needs_ci This PR requires CI testing to be performed. Please close and re-open this PR to trigger CI needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR labels Nov 15, 2021
@felixfontein
Copy link
Collaborator

Thanks for your contribution! Can you please add a changelog fragment? Thanks.

@felixfontein felixfontein added backport-4 check-before-release PR will be looked at again shortly before release and merged if possible. labels Nov 16, 2021
@haddystuff
Copy link
Contributor Author

ok, let me just add some tests and examples to documentation

@ansibullbot ansibullbot added needs_ci This PR requires CI testing to be performed. Please close and re-open this PR to trigger CI needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR tests tests unit tests/unit and removed needs_ci This PR requires CI testing to be performed. Please close and re-open this PR to trigger CI needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR labels Nov 17, 2021
@haddystuff
Copy link
Contributor Author

haddystuff commented Nov 17, 2021

Adding multiple addresses support for ip6 needs more time to implement, so i'll make another pull request later.

Copy link
Collaborator

@Ajpantuso Ajpantuso left a comment

Choose a reason for hiding this comment

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

Other than the changelog fragment this looks good. Pulled this locally for testing and haven't found any issues.

@ansibullbot ansibullbot added the needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR label Nov 18, 2021
….yml

Co-authored-by: Andrew Pantuso <ajpantuso@gmail.com>
@ansibullbot ansibullbot removed the needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR label Nov 18, 2021
Copy link
Collaborator

@Ajpantuso Ajpantuso left a comment

Choose a reason for hiding this comment

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

LGTM

@felixfontein felixfontein merged commit 50c2f3a into ansible-collections:main Nov 19, 2021
@patchback
Copy link

patchback bot commented Nov 19, 2021

Backport to stable-4: 💚 backport PR created

✅ Backport PR branch: patchback/backports/stable-4/50c2f3a97d6cc1b0a1c54c4b0d7fe5d9319f0295/pr-3738

Backported as #3757

🤖 @patchback
I'm built with octomachinery and
my source is open — https://github.com/sanitizers/patchback-github-app.

@felixfontein felixfontein removed the check-before-release PR will be looked at again shortly before release and merged if possible. label Nov 19, 2021
patchback bot pushed a commit that referenced this pull request Nov 19, 2021
* change ip4 type to list of str

* Add several tests and change documentation

* Update changelogs/fragments/1088-nmcli_add_multiple_addresses_support.yml

Co-authored-by: Andrew Pantuso <ajpantuso@gmail.com>

Co-authored-by: Andrew Pantuso <ajpantuso@gmail.com>
(cherry picked from commit 50c2f3a)
@felixfontein
Copy link
Collaborator

@haddystuff thanks for implementing this!
@Ajpantuso thanks for reviewing!

felixfontein pushed a commit that referenced this pull request Nov 19, 2021
* change ip4 type to list of str

* Add several tests and change documentation

* Update changelogs/fragments/1088-nmcli_add_multiple_addresses_support.yml

Co-authored-by: Andrew Pantuso <ajpantuso@gmail.com>

Co-authored-by: Andrew Pantuso <ajpantuso@gmail.com>
(cherry picked from commit 50c2f3a)

Co-authored-by: Alex Groshev <38885591+haddystuff@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature This issue/PR relates to a feature request module module net_tools plugins plugin (any type) tests tests unit tests/unit
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants