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.

Sign up for GitHub

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

Jump to bottom

nmcli: conn_reload param and up/down states #8897

Merged
merged 6 commits into from
Oct 3, 2024
Merged

nmcli: conn_reload param and up/down states #8897

merged 6 commits into from
Oct 3, 2024

Conversation

abakanovskii
Copy link
Contributor

@abakanovskii abakanovskii commented Sep 22, 2024 •
edited
Loading

SUMMARY

Add conn_reload option to perform nmcli connection reload if set to true
Add up and down states to perform nmcli up/nmcli down commands

Fixes #3752
Fixes #8704
Fixes #7152

ISSUE TYPE
  • Feature Pull Request
COMPONENT NAME

nmcli

ADDITIONAL INFORMATION

I am not completely sure how to make applying changes correctly as there is no distinct difference between 4 different variants in a related issue (see #3752 (comment)) so if you have some ideas I would be glad to get comments from you :)

# Changing with reloading afterwards
  - name: Change the property of a setting e.g. MTU and reload connection
    community.general.nmcli:
      conn_name: my-eth1
      mtu: 1500
      type: ethernet
      state: present
      conn_reload: true

# Simply disabling connection
  - name: Disable connection
    community.general.nmcli:
      conn_name: my-eth1
      state: down

# Reload and then enable connection
  - name: Reload and enable connection
    community.general.nmcli:
      conn_name: my-eth1
      state: up
      reload: true

@ansibullbot
Copy link
Collaborator

cc @alcamie101
click here for bot help

@ansibullbot ansibullbot added feature This issue/PR relates to a feature request module module plugins plugin (any type) labels Sep 22, 2024
@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 Sep 22, 2024
@felixfontein felixfontein added check-before-release PR will be looked at again shortly before release and merged if possible. backport-9 Automatically create a backport for the stable-9 branch labels Sep 22, 2024
russoz
russoz reviewed Sep 23, 2024
View reviewed changes
Copy link
Collaborator

@russoz russoz left a comment

Choose a reason for hiding this comment

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

Hi @abakanovskii

Thanks again for your contribution! A couple of small details there, other than that it LGTM.

plugins/modules/nmcli.py Outdated
Comment on lines 37 to 38
- Using O(state=present) to create connection will automatically bring connection up.
- Using O(state=up) and O(state=down) will not modify connection with other parameters.
Copy link
Collaborator

Choose a reason for hiding this comment

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

It would be good to mention that those states have been introduced in community.general 9.5.0

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not sure how to add these beacuse version_added is for new params rather then updated ones
Please check last commit if I did it right

Copy link
Collaborator

Choose a reason for hiding this comment

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

Textually, like:

Suggested change
- Using O(state=present) to create connection will automatically bring connection up.
- Using O(state=up) and O(state=down) will not modify connection with other parameters.
- Using O(state=present) to create connection will automatically bring connection up.
- Using O(state=up) and O(state=down) will not modify connection with other parameters. These states have been added in community.general 9.5.0.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

thanks!

plugins/modules/nmcli.py Outdated Show resolved Hide resolved
felixfontein
felixfontein reviewed Sep 27, 2024
View reviewed changes
Copy link
Collaborator

@felixfontein felixfontein left a comment

Choose a reason for hiding this comment

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

If nobody objects, I'll merge this next week.

@felixfontein felixfontein merged commit d4fb6bf into ansible-collections:main Oct 3, 2024
147 checks passed
@patchback Patchback
Copy link

patchback bot commented Oct 3, 2024 •
edited
Loading

Backport to stable-9: 💚 backport PR created

✅ Backport PR branch: patchback/backports/stable-9/d4fb6bf8a65afabe3576b49af906585425571ac3/pr-8897

Backported as #8975

🤖 @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 Oct 3, 2024
patchback bot pushed a commit that referenced this pull request Oct 3, 2024
@abakanovskii @patchback
nmcli: conn_reload param and up/down states (#8897)
8a1b2d0 
* Update nmcli module

* Update nmcli state

* Update test_nmcli

* Add CHANGELOG fragment

* PR Fixes

* Fix DOCUMENTATION block

(cherry picked from commit d4fb6bf)
@patchback patchback bot mentioned this pull request Oct 3, 2024
Merged
@felixfontein
Copy link
Collaborator

@abakanovskii thanks for your contribution!
@russoz thanks for reviewing!

felixfontein pushed a commit that referenced this pull request Oct 3, 2024
@patchback @abakanovskii
[PR #8897/d4fb6bf8 backport][stable-9] nmcli: conn_reload param and u…
9b4decd 
…p/down states (#8975)

nmcli: conn_reload param and up/down states (#8897)

* Update nmcli module

* Update nmcli state

* Update test_nmcli

* Add CHANGELOG fragment

* PR Fixes

* Fix DOCUMENTATION block

(cherry picked from commit d4fb6bf)

Co-authored-by: alexander <79072457+abakanovskii@users.noreply.github.com>
Massl123 pushed a commit to Massl123/community.general that referenced this pull request Feb 7, 2025
@abakanovskii @Massl123
nmcli: conn_reload param and up/down states (ansible-collections#8897)
563e168 
* Update nmcli module

* Update nmcli state

* Update test_nmcli

* Add CHANGELOG fragment

* PR Fixes

* Fix DOCUMENTATION block
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@russoz russoz russoz left review comments

@felixfontein felixfontein felixfontein left review comments

Assignees
No one assigned
Labels
backport-9 Automatically create a backport for the stable-9 branch feature This issue/PR relates to a feature request module module plugins plugin (any type) tests tests unit tests/unit
Projects
None yet
Milestone
No milestone
4 participants
@abakanovskii @ansibullbot @felixfontein @russoz