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

Fix NM unmanaged-devices (changes behavior) #201

Merged
merged 2 commits into from
Jun 14, 2021

Conversation

slyon
Copy link
Collaborator

@slyon slyon commented Mar 23, 2021

Description

  • nm: fix ignoring of multiple match conditions (like interface-name: AND mac:, instead of defaulting to MAC if available)
    • Some interfaces (e.g. bond members) might change their MAC and be picked up ny NM, interfering the tests, cf. test_bond_mac/Networkd

Checklist

  • Runs make check successfully.
  • Retains 100% code coverage (make check-coverage).
  • New/changed keys in YAML format are documented.
  • (Optional) Adds example YAML for new feature.
  • (Optional) Closes an open bug in Launchpad.

@slyon slyon force-pushed the slyon/fix-integration-tests branch from 0b3d5fa to d4e4f39 Compare March 24, 2021 15:39
@slyon slyon changed the title Fix integration test suite Fix NM unmanaged-devices and integration test suite asserts Mar 24, 2021
@slyon slyon marked this pull request as ready for review March 24, 2021 17:07
@slyon slyon requested a review from sil2100 March 24, 2021 17:11
@slyon slyon force-pushed the slyon/fix-integration-tests branch from fd1bf66 to bc188b0 Compare March 25, 2021 09:27
Some interfaces, matched by MAC, could change their MAC address after
assignment (e.g. a bond member), thus NM would not ignore this interface
anymore, even if it is also matched by interface-name.
@slyon slyon force-pushed the slyon/fix-integration-tests branch from bc188b0 to 9e76e80 Compare April 7, 2021 09:40
@slyon slyon changed the title Fix NM unmanaged-devices and integration test suite asserts Fix NM unmanaged-devices Apr 7, 2021
Copy link
Collaborator

@sil2100 sil2100 left a comment

Choose a reason for hiding this comment

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

Ok, this looks sane. I suppose it could potentially change behavior for some people, right? I mean, I think it was a 'bug' that on NM one could only match on one or the other, not both at once, but I guess it would be good to document that in the changelog at least.

@slyon slyon changed the title Fix NM unmanaged-devices Fix NM unmanaged-devices (changes behavior) Jun 14, 2021
@slyon slyon merged commit 44dab84 into master Jun 14, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants