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

tetragon: Load base sensor via sensor manager #3045

Merged
merged 7 commits into from
Nov 4, 2024
Merged

Conversation

olsajiri
Copy link
Contributor

@olsajiri olsajiri commented Oct 25, 2024

Loading initial sensor via sensor manager so it stores all the
loaded sensors/programs. So far we kept it separated but having
it in one place is handy for metrics requesting this data.

plus related fixes

Copy link

netlify bot commented Oct 25, 2024

Deploy Preview for tetragon ready!

Name Link
🔨 Latest commit de29d49
🔍 Latest deploy log https://app.netlify.com/sites/tetragon/deploys/6728a8e8b9448f0007b1e4b6
😎 Deploy Preview https://deploy-preview-3045--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.

@olsajiri olsajiri added the release-note/minor This PR introduces a minor user-visible change label Oct 25, 2024
@mtardy mtardy self-requested a review October 25, 2024 08:36
@olsajiri olsajiri force-pushed the pr/olsajiri/fixes branch 10 times, most recently from fdbff84 to 979a87a Compare October 31, 2024 07:42
Copy link
Contributor

@kkourt kkourt left a comment

Choose a reason for hiding this comment

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

One comment from a quick look.

pkg/sensors/handler.go Outdated Show resolved Hide resolved
@olsajiri olsajiri force-pushed the pr/olsajiri/fixes branch 2 times, most recently from a963a86 to 4aace87 Compare October 31, 2024 09:56
@olsajiri olsajiri marked this pull request as ready for review October 31, 2024 09:57
@olsajiri olsajiri requested a review from a team as a code owner October 31, 2024 09:57
We already lock access to the collections and we depend on the
queue serialize policy/sensor loads/unloads.

Following change will remove the queue, but we still want to keep
the loads/unloads serialized, so adding lock for that.

Signed-off-by: Jiri Olsa <[email protected]>
Removing go routine from sensor manager and calling the handler
routines directly.

Aslo removing LoadArg/UnloadArg types, which do not seem to
be used anymore.

Signed-off-by: Jiri Olsa <[email protected]>
Removing channel setup from sensor manager functions, because with
go routine removed there's no longer need for that.

Signed-off-by: Jiri Olsa <[email protected]>
Moving policyLister under sensor manager handler to keep
all the handler routines together now that we don't have
the go routine.

Signed-off-by: Jiri Olsa <[email protected]>
Loading initial sensor via sensor manager so it stores all the
loaded sensors/programs. So far we kept it separated but having
it in one place is handy for metrics requesting this data.

Moving the load before the server is started so we don't need to
"wait" for that. This way we can remove the sensorMgWait channel.

Signed-off-by: Jiri Olsa <[email protected]>
We removed the go routine, so it's no longer needed.

Signed-off-by: Jiri Olsa <[email protected]>
Now we removed StopSensorManager call from getTestSensorManager
we no longer need it to have the ctx argument.

Signed-off-by: Jiri Olsa <[email protected]>
Copy link
Contributor

@kkourt kkourt left a comment

Choose a reason for hiding this comment

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

🚀

@olsajiri olsajiri merged commit 4f1e262 into main Nov 4, 2024
43 checks passed
@olsajiri olsajiri deleted the pr/olsajiri/fixes branch November 4, 2024 13:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release-note/minor This PR introduces a minor user-visible change
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants