Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

podinfo: add a deleted pod cache #2930

Merged
merged 6 commits into from
Sep 19, 2024
Merged

Conversation

kkourt
Copy link
Contributor

@kkourt kkourt commented Sep 17, 2024

Add a deleted pod cache for pod association. See commits.

Copy link

netlify bot commented Sep 17, 2024

Deploy Preview for tetragon ready!

Name Link
🔨 Latest commit 4294881
🔍 Latest deploy log https://app.netlify.com/sites/tetragon/deploys/66ebd7ef6c4b510008f96f8f
😎 Deploy Preview https://deploy-preview-2930--tetragon.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@kkourt kkourt added the release-note/misc This PR makes changes that have no direct user impact. label Sep 17, 2024
@kkourt kkourt force-pushed the pr/kkourt/podinfo-deleted-pod-cache branch from 492abf0 to 94af537 Compare September 17, 2024 14:37
@kkourt kkourt marked this pull request as ready for review September 17, 2024 14:43
@kkourt kkourt requested review from mtardy and a team as code owners September 17, 2024 14:43
Copy link
Member

@mtardy mtardy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's cool! :) lgtm

Especially this cache is small and static for now (128) so it should not be a memory issue.

pkg/process/podinfo_test.go Outdated Show resolved Hide resolved
pkg/watcher/watcher_test.go Outdated Show resolved Hide resolved
pkg/watcher/watcher_test.go Show resolved Hide resolved
This commit refactors the watcher code. It adds an error value (unused
for now), and creates a new internal newK8sWatcher function. These two
changes are going to be used by subsequent patches.

Signed-off-by: Kornilios Kourtis <kornilios@isovalent.com>
Add a containerIDKey function for getting the container key from the
full container id. It is intended for a subsequent patch.

Signed-off-by: Kornilios Kourtis <kornilios@isovalent.com>
This does not change the semantics of the function. It is intended for a
subsequent patch.

Signed-off-by: Kornilios Kourtis <kornilios@isovalent.com>
This commit ads a test where a lookup for a pod via its container id happens
after the pod was deleted. This case, might happen if a pod is started and
deleted from the cache before user-space gets a chance to associate a pod with
an exec event.

Example:
go test ./pkg/watcher -test.run TestFastK8s -test.v
=== RUN   TestFastK8s
    watcher_test.go:32: time="2024-09-17T13:38:51+02:00" level=info msg="Initialized informer cache" count=0 informer=pod

    watcher_test.go:95: adding pod
    watcher_test.go:103: deleting pod
    watcher_test.go:108:
                Error Trace:    /home/kkourt/src/tetragon/pkg/watcher/watcher_test.go:108
                Error:          Should be true
                Test:           TestFastK8s
                Messages:       deleted pod should be found
--- FAIL: TestFastK8s (0.10s)
FAIL
FAIL    github.com/cilium/tetragon/pkg/watcher  0.118s
FAIL

Signed-off-by: Kornilios Kourtis <kornilios@isovalent.com>
This commit adds an LRU cache that holds container ids for deleted pods.
It is used to do pod association for pods that are deleted, and are no
longer available in the pod cache.

TestFastK8s now succeeds.

Signed-off-by: Kornilios Kourtis <kornilios@isovalent.com>
Signed-off-by: Kornilios Kourtis <kornilios@isovalent.com>
@kkourt kkourt force-pushed the pr/kkourt/podinfo-deleted-pod-cache branch from 94af537 to 4294881 Compare September 19, 2024 07:51
@kkourt
Copy link
Contributor Author

kkourt commented Sep 19, 2024

Thanks for the review. Pushed a new version based on the feedback. Merging.

@kkourt kkourt merged commit 3cefc44 into main Sep 19, 2024
47 checks passed
@kkourt kkourt deleted the pr/kkourt/podinfo-deleted-pod-cache branch September 19, 2024 09:15
@kkourt kkourt added the needs-backport/1.2 This PR needs backporting to 1.2 label Sep 27, 2024
@kkourt kkourt mentioned this pull request Sep 27, 2024
@kkourt kkourt added backport-pending/1.2 The backport for this PR is in progress. and removed needs-backport/1.2 This PR needs backporting to 1.2 labels Sep 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport-pending/1.2 The backport for this PR is in progress. release-note/misc This PR makes changes that have no direct user impact.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants