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: Fix bond option xmit_hash_policy #6527

Merged
merged 4 commits into from
May 21, 2023
Merged

nmcli: Fix bond option xmit_hash_policy #6527

merged 4 commits into from
May 21, 2023

Conversation

psvmcc
Copy link
Contributor

@psvmcc psvmcc commented May 16, 2023

SUMMARY

There is no alias option for configure xmit_hash_policy param for bond interface, we need to set it via +bond.option key.

Fixes: #5861

ISSUE TYPE
  • Bugfix Pull Request
COMPONENT NAME

nmcli

@ansibullbot
Copy link
Collaborator

cc @alcamie101
click here for bot help

@ansibullbot ansibullbot added bug This issue/PR relates to a bug module module plugins plugin (any type) small_patch Hopefully easy to review labels May 16, 2023
@ansibullbot ansibullbot removed the small_patch Hopefully easy to review label May 16, 2023
felixfontein
felixfontein reviewed May 17, 2023
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.

Is a similar special case check needed for idempotency?

changelogs/fragments/6527-nmcli-bond-fix-xmit_hash_policy.yml Outdated Show resolved Hide resolved
@felixfontein felixfontein added check-before-release PR will be looked at again shortly before release and merged if possible. backport-6 labels May 17, 2023
@psvmcc @felixfontein
Update changelogs/fragments/6527-nmcli-bond-fix-xmit_hash_policy.yml
247ecf5 
Co-authored-by: Felix Fontein <felix@fontein.de>
@psvmcc
Copy link
Contributor Author

psvmcc commented May 17, 2023 •
edited
Loading

Is a similar special case check needed for idempotency?

Well I'm not sure, but I checked it locally in my environment, and everything is good with idempotency.
We can check it only with integration testing only I think. Should I add such tests for this case, or maybe you can suggest other way?
UPD: my mistake, we can just use unit tests, but I'm not sure should I add it here?

@psvmcc
Copy link
Contributor Author

psvmcc commented May 18, 2023

@felixfontein I just extended current tests, can you please check?

@ansibullbot ansibullbot added tests tests unit tests/unit labels May 18, 2023
@felixfontein
Copy link
Collaborator

Looks good to me (without knowing nmcli well). I'll merge this by the end of the weekend if nobody objects.

@felixfontein felixfontein merged commit cb1e637 into ansible-collections:main May 21, 2023
@patchback
Copy link

patchback bot commented May 21, 2023 •
edited
Loading

Backport to stable-6: 💚 backport PR created

✅ Backport PR branch: patchback/backports/stable-6/cb1e6376db322f68a44f3681f6c3c3cdcc78019a/pr-6527

Backported as #6555

🤖 @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 May 21, 2023
patchback bot pushed a commit that referenced this pull request May 21, 2023
@psvmcc @patchback
nmcli: Fix bond option xmit_hash_policy (#6527)
e062a78 
* nmcli_bond_xmit_fix

* Create 6527-nmcli-bond-fix-xmit_hash_policy.yml

add changelog

* Update changelogs/fragments/6527-nmcli-bond-fix-xmit_hash_policy.yml

Co-authored-by: Felix Fontein <felix@fontein.de>

* unit tests extend

---------

Co-authored-by: Felix Fontein <felix@fontein.de>
(cherry picked from commit cb1e637)
@patchback patchback bot mentioned this pull request May 21, 2023
Merged
@patchback
Copy link

patchback bot commented May 21, 2023 •
edited
Loading

Backport to stable-7: 💚 backport PR created

✅ Backport PR branch: patchback/backports/stable-7/cb1e6376db322f68a44f3681f6c3c3cdcc78019a/pr-6527

Backported as #6556

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

@felixfontein
Copy link
Collaborator

@psvmcc thanks a lot for fixing this!

patchback bot pushed a commit that referenced this pull request May 21, 2023
@psvmcc @patchback
nmcli: Fix bond option xmit_hash_policy (#6527)
db7a271 
* nmcli_bond_xmit_fix

* Create 6527-nmcli-bond-fix-xmit_hash_policy.yml

add changelog

* Update changelogs/fragments/6527-nmcli-bond-fix-xmit_hash_policy.yml

Co-authored-by: Felix Fontein <felix@fontein.de>

* unit tests extend

---------

Co-authored-by: Felix Fontein <felix@fontein.de>
(cherry picked from commit cb1e637)
@patchback patchback bot mentioned this pull request May 21, 2023
Merged
felixfontein pushed a commit that referenced this pull request May 21, 2023
@patchback @psvmcc
[PR #6527/cb1e6376 backport][stable-6] nmcli: Fix bond option xmit_ha…
dfb61b2 
…sh_policy (#6555)

nmcli: Fix bond option xmit_hash_policy (#6527)

* nmcli_bond_xmit_fix

* Create 6527-nmcli-bond-fix-xmit_hash_policy.yml

add changelog

* Update changelogs/fragments/6527-nmcli-bond-fix-xmit_hash_policy.yml

Co-authored-by: Felix Fontein <felix@fontein.de>

* unit tests extend

---------

Co-authored-by: Felix Fontein <felix@fontein.de>
(cherry picked from commit cb1e637)

Co-authored-by: Sergey Putko <mail@psvlan.com>
felixfontein pushed a commit that referenced this pull request May 21, 2023
@patchback @psvmcc
[PR #6527/cb1e6376 backport][stable-7] nmcli: Fix bond option xmit_ha…
443d5a2 
…sh_policy (#6556)

nmcli: Fix bond option xmit_hash_policy (#6527)

* nmcli_bond_xmit_fix

* Create 6527-nmcli-bond-fix-xmit_hash_policy.yml

add changelog

* Update changelogs/fragments/6527-nmcli-bond-fix-xmit_hash_policy.yml

Co-authored-by: Felix Fontein <felix@fontein.de>

* unit tests extend

---------

Co-authored-by: Felix Fontein <felix@fontein.de>
(cherry picked from commit cb1e637)

Co-authored-by: Sergey Putko <mail@psvlan.com>
@psvmcc psvmcc deleted the psvmcc-patch-1 branch May 22, 2023 07:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@felixfontein felixfontein felixfontein left review comments

Assignees
No one assigned
Labels
bug This issue/PR relates to a bug module module plugins plugin (any type) tests tests unit tests/unit
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

xmit_hash_policy with bond interfaces
3 participants
@psvmcc @ansibullbot @felixfontein