Netplan status --diff refactoring#444
Merged
slyon merged 7 commits intocanonical:mainfrom Feb 29, 2024
Merged
Conversation
daniloegea
force-pushed
the
netplan_status_diff_fixes2
branch
2 times, most recently
from
41b8a47 to
48280b0
Compare
daniloegea
force-pushed
the
netplan_status_diff_fixes2
branch
from
77acb2b to
f5fb016
Compare
Contributor
Author
|
Hi @rkratky, may I ask you to review the last commit of this PR, please? It's updating the |
utils.route_table_lookup: - use the new path of the configuration files. It was changed recently in iproute2. It will fallback to the old path if the iproute2 version in the system is older. - change the dictionary to map names to numbers as well - if it fails to open the file, return the standard content found in rt_tables. - mock this method all over the tests so they will not try to open the real file. state_diff._default_route_tables_name_to_number: - use utils.route_table_lookup instead of its own mapping
If the MAC address in Netplan is not a valid MAC (it can also be an option now, such as 'random' and 'permanent'), don't try to diff it against the system's MAC address.
The low level API has two new functions that will return a boolean representing the internal state of the property. In the netdef, this property is stored in an internal struct that contains two gboolean variables. To avoid having to expose this struct I decided to separate it in two functions. In the Python code, the new propserty 'link_local' will return a list containing ipv4 or ipv6 or an empty list to mimic the link-local property in the YAML.
Instead of ignoring link local IPs and routes, check the netdef 'link-local' property to see if the they should be part of the diff.
It implemented a PoC of the tabular user interface that will not be used anymore.
daniloegea
force-pushed
the
netplan_status_diff_fixes2
branch
from
f5fb016 to
9c5220b
Compare
slyon
suggested changes
Feb 28, 2024
slyon
approved these changes
Feb 28, 2024
Contributor
slyon
left a comment
slyon
left a comment
There was a problem hiding this comment.
LGTM with the configuration error resolved.
Update the manpage and autocomplete files.
daniloegea
force-pushed
the
netplan_status_diff_fixes2
branch
from
9c5220b to
7312392
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
A few improvements for
netplan status --diff:/etc/iproute2/were moved to/usr/sharein a recent release of iproute2. Fix unit tests to not open the real file.randomis used in the YAML.link-localproperty via a new API.link-localproperty. Note that currently it will not check if the system interface should have a link local address (in other words, it will not show a diff iflink-localis enabled but the address is not present in the system). It will only check if the address is present and check thelink-localproperty to see if the address is expected to be there.netplan-statusmanpage and autocomplete files.Example of link-local changes:
In this case, link-local has the default configuration, which enables it only for IPv6, but the IPv4 link local address is also present so it will show up as a diff.
Description
Checklist
make checksuccessfully.make check-coverage).