Report estimated expiry timers for connection-based FQDN entries #32013
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
e8e42b0 to
84c0846
Compare
d1135ed to
15772a8
Compare
15772a8 to
09fac0c
Compare
84c0846 to
cc4df44
Compare
cc4df44 to
32dc26f
Compare
|
/test |
pippolo84
reviewed
Apr 30, 2024
32dc26f to
4280d05
Compare
|
@pippolo84 done, thanks for the review. |
|
/test |
pippolo84
approved these changes
May 1, 2024
markpash
approved these changes
May 2, 2024
aditighag
approved these changes
May 2, 2024
There was a problem hiding this comment.
The changes along with comments make sense, LGTM!
Just a related q: the connection expiry times are not static, are they? The connection entry would be kept alive as long as there is traffic seen, regardless of when the next GC runs. I wonder if this warrants a documentation note as the expiration times are interpreted differently than DNS TTL.
4280d05 to
4aedd54
Compare
|
/test |
Correct. I've taken a stab at describing the behaviour here: #32350 . Strictly speaking this behaviour was already the case, the only difference with this PR is that the user-facing CLI output will reflect this more closely. |
This benchmark was broken (triggered segfault) and was based on the older go.check framework. Switch it to standard testing and fix it up. Signed-off-by: Joe Stringer <joe@cilium.io>
Modernize this code by using netip over net for filtering IPs. Signed-off-by: Joe Stringer <joe@cilium.io>
Split API request parsing from the code to form the model, so we can move the model output generation logic under pkg/fqdn in an upcoming patch. Signed-off-by: Joe Stringer <joe@cilium.io>
This will allow us to converge usage of this function by the FQDN dump routines to either directly look up one specific endpoint or to dump all endpoints via this function in an upcoming patch. Signed-off-by: Joe Stringer <joe@cilium.io>
Moving this logic into pkg/fqdn allows it to be more self-contained, and makes it easier to make changes solely within the pkg/fqdn package for future extensions of the filtering and formatting logic. Two changes are made to the main function being moved here: - At the start, use `NameManager.config.GetEndpointsDNSInfo()` to fetch the endpoints rather than pulling them from the EndpointManager directly. - Switch from pulling the `endpoint.ID` field to `endpoint.ID64`. Signed-off-by: Joe Stringer <joe@cilium.io>
Report the expiry time for entries kept alive based on connection state as the next time that connection tracking garbage collection runs. This way, the expiration time should be closer to the actual expiry time rather than simply reporting the zero timestamp. Signed-off-by: Joe Stringer <joe@cilium.io>
4aedd54 to
795d80b
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 app
2D80
lied.
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.
Based on #32012
Report the expiry time for entries kept alive based on connection state
as the next time that connection tracking garbage collection will run.
This way, the expiration time should be closer to the actual expiry time
rather than simply reporting the zero timestamp.
Commits 1-5 do some refactoring and cleanups in preparation for the target
feature in this PR. The final commit has the functional changes.
Related: #24246
Fixes: #21540