Replace option.Config.{Get,Set,Append}Devices by table lookups
#30578
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
maintainer-s-little-helper
bot
added
the
dont-merge/needs-release-note-label
bimmlerd
added
kind/cleanup
maintainer-s-little-helper
bot
removed
dont-merge/needs-release-note-label
bimmlerd
force-pushed
the
pr/bimmlerd/remove-get-set-devices
branch
6 times, most recently
from
2fd8348
to
782348a
Compare
|
/test |
|
This pull request has been automatically marked as stale because it |
github-actions
bot
added
the
stale
bimmlerd
force-pushed
the
pr/bimmlerd/remove-get-set-devices
branch
from
782348a
to
81564f7
Compare
bimmlerd
removed
the
dont-merge/blocked
bimmlerd
force-pushed
the
pr/bimmlerd/remove-get-set-devices
branch
from
81564f7
to
53e083e
Compare
|
/test |
github-actions
bot
removed
the
stale
bimmlerd
force-pushed
the
pr/bimmlerd/remove-get-set-devices
branch
from
53e083e
to
2f9afc0
Compare
|
/test |
bimmlerd
force-pushed
the
pr/bimmlerd/remove-get-set-devices
branch
from
2f9afc0
to
b568561
Compare
|
/ci-e2e |
bimmlerd
force-pushed
the
pr/bimmlerd/remove-get-set-devices
branch
from
b568561
to
5384d27
Compare
|
/ci-e2e |
joamaki
reviewed
Mar 27, 2024
bimmlerd
force-pushed
the
pr/bimmlerd/remove-get-set-devices
branch
from
5384d27
to
1d80593
Compare
tommyp1ckles
approved these changes
Apr 8, 2024
bimmlerd
force-pushed
the
pr/bimmlerd/remove-get-set-devices
branch
from
a799c4f
to
0e2eb4b
Compare
|
/test |
joamaki
approved these changes
Apr 10, 2024
lmb
approved these changes
Apr 10, 2024
joamaki
force-pushed
the
pr/bimmlerd/remove-get-set-devices
branch
from
0e2eb4b
to
919b2e6
Compare
|
/test |
joamaki
force-pushed
the
pr/bimmlerd/remove-get-set-devices
branch
2 times, most recently
from
6dbcf6f
to
2455787
Compare
|
/test |
Devices (i.e. network interfaces) can change at runtime. The current device API, however, suggest that these devices never change. Indeed, a large portion of the agent is written with that assumption (though workarounds have been added where changes to the devices broke things). As part of an ongoing process to migrate the agent towards a future where a changing set of devices is the norm, deprecate the existing, snapshot-oriented APIs and replace it with table lookups. In subsequent commits, usage of this API is reduced, and replaced with usage of an API which allows for being notified when the set of devices changes. Signed-off-by: David Bimmler <david.bimmler@isovalent.com>
This is handy for future commits in which we increase the number of dependencies we'd have to manually construct when creating a linux NodeHandler. Signed-off-by: David Bimmler <david.bimmler@isovalent.com>
* Refactor out the duplicated code to assert against NeighList * Use proper fake nodemap in TestNodeUpdateIDs Signed-off-by: Jussi Maki <jussi@isovalent.com> Signed-off-by: David Bimmler <david.bimmler@isovalent.com>
While the tests claim they need the full datapath, that is no longer true. Instead, provide a fake datapath (to avoid having to provide all the dependencies). Signed-off-by: David Bimmler <david.bimmler@isovalent.com>
GetDevices returns a snapshot, but devices can change at runtime. The new, table-based API reflects this fact. This patch, however, does not change the semantics - it merely replaces the usage of GetDevices with a single table lookup, to allow us to react to changes in the future (and remove the GetDevices API). Signed-off-by: David Bimmler <david.bimmler@isovalent.com>
GetDevices returns a snapshot, but devices can change at runtime. The new, table-based API reflects this fact. This patch, however, does not change the semantics - it merely replaces the usage of GetDevices with a single table lookup, to allow us to react to changes in the future (and remove the GetDevices API). Signed-off-by: David Bimmler <david.bimmler@isovalent.com>
GetDevices returns a snapshot, but devices can change at runtime. The new, table-based API reflects this fact. This patch, however, does not change the semantics - it merely replaces the usage of GetDevices with a single table lookup, to allow us to react to changes in the future (and remove the GetDevices API). Signed-off-by: David Bimmler <david.bimmler@isovalent.com>
There are no usages, hence it is safe to remove. Signed-off-by: David Bimmler <david.bimmler@isovalent.com>
GetDevices returns a snapshot, but devices can change at runtime. The new, table-based API reflects this fact. This patch, however, does not change the semantics - it merely replaces the usage of GetDevices with a single table lookup, to allow us to react to changes in the future (and remove the GetDevices API). Signed-off-by: David Bimmler <david.bimmler@isovalent.com>
GetDevices returns a snapshot, but devices can change at runtime. The new, table-based API reflects this fact. This patch, however, does not change the semantics - it merely replaces the usage of GetDevices with a single table lookup, to allow us to react to changes in the future (and remove the GetDevices API). Signed-off-by: David Bimmler <david.bimmler@isovalent.com>
As explained in 0670fd2, the WireGuard device must be considered for loading bpf_host in certain conditions. With the removal of the Get/Set/AppendDevices APIs, the global addition of the WG interface to devices is no longer possible, nor was it ever really desirable. Since we don't want to make WG a "selected" device, we move the inclusion of this interface to the two places necessary: when writing static EP data in the config writer, as well as the loading of bpf_host. Signed-off-by: David Bimmler <david.bimmler@isovalent.com>
The agent needs to choose between loading bpf_network or bpf_host onto physical devices. In the long-term, we want to get rid of bpf_network. In the short-term, however, it's still around, so we need to keep the existing semantics intact. Previous code chose by checking the devices configuration (i.e. --devices) as well as the runtime-derived list of devices, to understand whether we were auto-detecting devices. If so, bpf_network would not be loaded in reinitializeIPSec, as bpf_host would be loaded. With the change towards always auto detecting devices, these checks need to be switched for AreDevicesRequired in the short-term, a dedicated flag in the long term to allow clean fall-back to encryptInterface and the removal of bpf_network in the longer term. Co-authored-by: Jussi Maki <jussi@isovalent.com> Signed-off-by: David Bimmler <david.bimmler@isovalent.com>
Since the devices controller is really the owner of this flag, move the configuration into the cell proper, to avoid other things being able to depend on it. Signed-off-by: David Bimmler <david.bimmler@isovalent.com>
The snapshot based APIs are impossible to use correctly, as they do not reflect the fact that the set of devices can change at runtime. Uses of this API have been replaced with lookups into the Table[Device] in the past few commits, hence removal is now easy. Signed-off-by: David Bimmler <david.bimmler@isovalent.com>
joamaki
force-pushed
the
pr/bimmlerd/remove-get-set-devices
branch
from
2455787
to
6e93393
Compare
|
/test |
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 replaces usages of the Get/SetDevices APIs by lookups to the device table, and includes the necessary plumbing of that everywhere.
Scary stuff includes the wireguard interface thing in daemon/loader, not sure my transformation is functionally equivalent. CI will hopefully tell.