Update NIC management strategy #402
Merged
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
|
Passes CIT network test: |
drewhli
reviewed
Jul 17, 2024
drewhli
approved these changes
Jul 17, 2024
|
Routing is functional but not correct, there are martian packets coming from the MDS in kernel logs with this build. |
ChaitanyaKulkarni28
approved these changes
Jul 19, 2024
a-crate
changed the title
drewhli
approved these changes
Jul 23, 2024
|
I'll re-request review when I'm ready, want to make some more changes. |
dorileo
reviewed
Jul 24, 2024
dorileo
reviewed
Jul 24, 2024
|
Updated rollback strategy, NetworkManager and Netplan were not applying changes on rollback. Added unit test for FallbacktoDefault. |
a-crate
force-pushed
the
netplan
branch
2 times, most recently
from
f1c2aee to
e8c4b80
Compare
|
Reworked, see commit message. MDS reachability is WAI (only on primary NIC). |
|
/unhold |
drewhli
reviewed
Jul 31, 2024
a-crate
force-pushed
the
netplan
branch
2 times, most recently
from
678686a to
e538357
Compare
drewhli
reviewed
Jul 31, 2024
There was a problem hiding this comment.
Given that it doesn't seem we're using osConfigRule, we can probably remove it, along with the no-op test for TestFindOSRule in manager_test.go.
There was a problem hiding this comment.
Yep, forgot to remove osConfigRule. I think the test is already gone though. PTAL.
Don't delete default netplan config on Debian 12. Write netplan configs to /run/netplan. Don't manage primary NIC unless the user has configured the new manage_primary_nic toggle. Disable use-domains on secondary NICs with netplan. This avoids making the MDS reachable over secondary NICs instead of primary NICs when managing the primary interface. If the MDS is inaccessible during startup, rollback all agent configuration on all interfaces and try again. Apply changes during netplan and network manager rollbacks.
drewhli
approved these changes
Jul 31, 2024
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: a-crate, ChaitanyaKulkarni28, drewhli The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
google-oss-prow
bot
merged commit 3aa08e9
into
GoogleCloudPlatform:main
5 checks passed
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.
See commit messages for details. Short version: bring back OS rules with a configuration option to ignore them.
Stop removing the default configuration on Debian 12, write a config with a higher priority.
Add a network recovery strategy, executed when then MDS is inaccessible on startup. Remove all guest agent configuration and go back to the OS default, exit with a failure if the MDS is still inaccessible with the default configuration.
/hold
/cc @ChaitanyaKulkarni28 @drewhli