Modularize ipcache BPF listener #29194
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
giorio94
added
kind/cleanup
|
/test |
giorio94
force-pushed
the
mio/ipcache-listener-modularization
branch
from
ad19eae
to
7c663f3
Compare
They are leftovers from 84901e4 ("bpf, ipcache: unconditionally assume LPM trie delete/dump support") and not used anymore. Hence, let's just drop them as a preparatory step to convert the listener into a cell. Signed-off-by: Marco Iorio <marco.iorio@isovalent.com>
Break the daemon dependency, as a preparation for the subsequent conversion of the listener into a cell. Signed-off-by: Marco Iorio <marco.iorio@isovalent.com>
Crafting a dedicated unit test revealed that this logic was broken, because we considered an entry for deletion if and only if it the given prefix was not found in the ipcache struct, and the source of the corresponding identity was either kvstore or local. But when the prefix is not found, we retrieve the default value of the identity type, which will never match the source filter. Now, considering that: * the ipcache map gets recreated from scratch at every agent restart, * the garbage collection logic was triggered only when either Cilium was configured in kvstore mode or was connected to at least one remote cluster, * the etcd watcher already implements a dedicated logic to replay possibly missed deletion events both after relist and in case of complete reconnection, * fixing this logic would likely expose us to even more headaches, given that we may remove entries when the ipcache is not yet fully synchronized from the other sources, causing the removal of valid entries and the disruption of existing connections, let's just remove it. Signed-off-by: Marco Iorio <marco.iorio@isovalent.com>
Following the removal of the only non-empty implementation of OnIPIdentityCacheGC, let's simplify the IPIdentityMappingListener interface by dropping it altogether. Let's also remove the ForEachListener ipcache method, which is no longer used. Signed-off-by: Marco Iorio <marco.iorio@isovalent.com>
This makes explicit which methods are actually used and allows swapping implementations, e.g., for mocking purposes. Signed-off-by: Marco Iorio <marco.iorio@isovalent.com>
Convert the ipcache bpf listener into a cell, to increase modularity and remove its initialization from the main daemon start-up logic. The main difference being that now we register it into the ipcache module using AddListener, instead of SetListeners. While this doesn't cause any differences at the moment (given that we are anticipating the initialization and the ipcache is still empty at that point), it prevents possible future ordering issues if any other invoke function also calls AddListener, as SetListeners would blindly override any registered listener. Signed-off-by: Marco Iorio <marco.iorio@isovalent.com>
giorio94
force-pushed
the
mio/ipcache-listener-modularization
branch
from
7c663f3
to
92dc5bc
Compare
|
Rebased onto main, let's see if that makes CI happier. |
|
/test |
|
@brb @jrajahalme Ping |
jrajahalme
approved these changes
Nov 27, 2023
maintainer-s-little-helper
bot
added
the
ready-to-merge
github-merge-queue
bot
removed this pull request from the merge queue due to failed status checks
github-merge-queue
bot
removed this pull request from the merge queue due to failed status checks
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.
Convert the ipcache BPF listener module into a cell. Additionally drop the existing ipcache map garbage collection logic, which turned out to be buggy and not really effective, as well as not necessary. Please review commit by commit, and refer to the individual commit messages for additional details.
Related: #28004