-
Notifications
You must be signed in to change notification settings - Fork 155
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
network: ensure disconnected interfaces are disabled #2015
Merged
Chris-Peterson444
merged 3 commits into
canonical:main
from
Chris-Peterson444:network-wait-online-lp2063331
Jun 28, 2024
Merged
network: ensure disconnected interfaces are disabled #2015
Chris-Peterson444
merged 3 commits into
canonical:main
from
Chris-Peterson444:network-wait-online-lp2063331
Jun 28, 2024
Conversation
This file contains 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
Chris-Peterson444
force-pushed
the
network-wait-online-lp2063331
branch
2 times, most recently
from
June 17, 2024 22:26
70febe2
to
6ac25d7
Compare
This caused a bug where a user could "disable" both ipv4 and ipv6 but the network device config still contained references to the nameserver configuration, causing it to still show up in the final config written.
Automatically disables interfaces that are not ready by the time we reach the networking screen. Otherwise, the written netplan implies all interfaces are required for start up and will cause systemd-networkd-wait-online.service to timeout on boot (LP: #2063331).
Chris-Peterson444
force-pushed
the
network-wait-online-lp2063331
branch
from
June 17, 2024 23:07
6ac25d7
to
817fc72
Compare
Chris-Peterson444
changed the title
Network wait online lp2063331
network: ensure disconnected interfaces are disabled
Jun 21, 2024
There are some other improvements I think we could make to make the network screen more robust but I'll open a separate PR for those. These changes alone should resolve LP:#2063331. |
ogayot
reviewed
Jun 26, 2024
It's possible that clients do not interact with the network controller - even if the controller is interactive - and simply mark the controller as configured (currently the desktop installer does this). Subiquity's policy is to disable interfaces without a global IP address by the time the network screen is shown. However, other than special handling for autoinstall cases, this logic is only invoked the first time the screen is shown. This makes sure the config written to the target device adheres to policy. This change purposefully does not apply the config in the live environment. It's likely Desktop does not interact with the network controller to avoid interactions between networkd and NetworkManager. Handling of this logic should be for future commits.
Chris-Peterson444
force-pushed
the
network-wait-online-lp2063331
branch
from
June 26, 2024 18:20
9b1e814
to
e22946c
Compare
ogayot
approved these changes
Jun 28, 2024
dbungert
approved these changes
Jun 28, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
According to LP:#2063331, Subiquity policy should be to disable interfaces which are not automatically configured in the installation environment on the target machine. Subiquity already has this logic mostly in place, via a call to
BaseNetworkController.update_initial_configs
in the right places. The subiquitycore network controller calls this function on the first view of the network screen, but the non-core server network controller doesn't. Existing logic and comments in the non-core controller imply this is already the expected behavior in non-core installs as well (see comments in the existing autoinstall logic). This PR adds theupdate_initial_configs
call in two places in the server network controller:GET
request handler. This is the endpoint clients should call to get information to populate the network screen, so we should update the device configurations before sending them to the client.configured
function. It's possible to complete the install by simply marking the network controller as configured without ever interacting with it. However, because the screen was not "shown" (i.e. server received a GET request), the target system will end up with the not updated network config. The Desktop installer specifically does this, likely to avoid changing control of the live session's network from NetworkManager to networkd. Callingupdate_initial_config
here without callingapply_config
will allow the target system to receive the correct config without modifying the live sessions network settings.Lastly, I included a fix for
NetworkDev.ip_networks_for_version
that makes sure static configuration information is really removed from the config when setting ipv4 and ipv6 to either disabled or automatic. Existing behavior would allow static configuration options to be appended to the existing config when resubmitting the form and also persist when presented as "disabled" in the UI.