Modularise MTU discovery #28964
Merged
Modularise MTU discovery #28964
Conversation
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
e6c8835 to
b5c11d1
Compare
f58d61f to
f067651
Compare
|
/test |
343e7c6 to
3e30edb
Compare
|
/test |
eeadf33 to
c3fc92f
Compare
|
/test |
c3fc92f to
afcd983
Compare
|
/test |
afcd983 to
f934622
Compare
|
/test |
CODEOWNERs took that as WG, IPsec and DP in general, though 😁 Going to pull in @pchaigno for IPsec in that case, I'm under the impression he has patches he wants to get in which may conflict. |
rgo3
approved these changes
Nov 23, 2023
pchaigno
requested changes
Nov 24, 2023
There was a problem hiding this comment.
Thanks for the ping David!
The second commit seems the most likely to conflict with other ongoing IPsec work. Could we split it into smaller, easier-to-handle commits? Or alternatively, skip it if that seems easier (IIUC, it's not needed by the third commit)?
Introduces the IPsec key custodian which encompasses the small part of IPsec initialization logic which was in daemon, with the goal of preparing MTU modularization. MTU depends on knowing the IPsec key size (though, notably, it seemingly assumes the size doesn't change), hence we start here with the modularisation. This doesn't modularise a significant part of the IPsec subsystem, but it's a incremental step in the direction. It would be conceivable to also pull the key file watcher into the custodian, but I've opted for the smallest diff here. In terms of initialization ordering, this puts the IPsec key setup _before_ the daemon startup, but after the local node store. As a result, the SPI is guaranteed to be set earlier than before this patch. Signed-off-by: David Bimmler <david.bimmler@isovalent.com>
In preparation of changing the key file watcher functionality into hive jobs, weaken the explicit dependency on NodeDiscovery and replace it with the (newly introduced) NodeUpdater interface, as this is the only functionality the KeyfileWatcher needs from NodeDiscovery. Signed-off-by: David Bimmler <david.bimmler@isovalent.com>
With the introduction of this new IPsecKeyCustodian cell, it makes sense to move the responsibility of starting the background workers to the cell's interface. This move additionally prepares the code for conversion to the hive/job framework. Ideally, this would not still live in cmd/daemon, but in the start hook of the key custodian, but I'm unsure of the ordering dependencies that exist, and don't want to rock the boat too badly at this stage. Signed-off-by: David Bimmler <david.bimmler@isovalent.com>
Instead of launching raw goroutines, we can employ the hive job mechanism. As is, this doesn't give an immediate benefit (except for aligning with project direction), but enables health reporting in a follow-up commit. The (minor) functional difference is that instead of waiting for a minute in between the stale key reclamation, we now start the job every minute, as long as it completes within that minute. Signed-off-by: David Bimmler <david.bimmler@isovalent.com>
Use the job health reporting API to add visibility to the health of the keyfile watcher. Signed-off-by: David Bimmler <david.bimmler@isovalent.com>
Previous refactoring removed the dependency on the daemon context, hence we can drop it from the interface and implementations. Signed-off-by: David Bimmler <david.bimmler@isovalent.com>
Create a minimal MTU discovery cell which exposes an MTU interface capable of fulfilling the MTU queries present in current-day code. This is a pre-requisite to modularise the loader, and itself depends on the initial modularisation of the IPsec key setup steps. Signed-off-by: David Bimmler <david.bimmler@isovalent.com>
f934622 to
c768e89
Compare
pchaigno
approved these changes
Nov 29, 2023
|
/test |
|
/ci-runtime (failed with |
|
/ci-external-workloads (failed with #29312 - |
joamaki
approved these changes
Nov 30, 2023
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.
This series is a preparation for loader modularisation, by pulling a little bit of IPsec key setup and MTU into hive, so that the future loader cell can consume it.