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

fix(userspace/falco): when counting -M timeout, do not account for async events #3505

Merged
merged 1 commit into from
Feb 26, 2025

Conversation

FedeDP
Copy link
Contributor

@FedeDP FedeDP commented Feb 26, 2025

What type of PR is this?

/kind bug

Any specific area of the project related to this PR?

/area engine

What this PR does / why we need it:

When using -M flag with eg: a plugin (like the container plugin) that generates asyncevents during the startup phase, could set first duration_start to a value that would be lower than first event retrieved from scap.
Bug spotted by the container plugin CI and the Falco testing framework: https://github.com/FedeDP/container_plugin/actions/runs/13546519679?pr=21

TestFalco_Miscs_PrometheusMetricsNoDriver
{"deadline":300000000000,"level":"info","msg":"running falco with runner","time":"2025-02-26T14:50:59Z"}
{"cmd":"/usr/bin/falco -c /etc/falco/falco.yaml -o append_output.suggested_output=false -o metrics.enabled=true -o metrics.output_rule=true -o metrics.interval=2s -o webserver.enabled=true -o webserver.prometheus_metrics_enabled=true -r single_rule.yaml -M 5 -o engine.kind=nodriver -o log_level=debug -o log_stderr=true -o log_syslog=false -o stdout_output.enabled=true","level":"debug","msg":"executing command","time":"2025-02-26T14:50:59Z"}
    miscs_test.go:137: 
        	Error Trace:	/__w/_actions/falcosecurity/testing/main/miscs_test.go:137
        	Error:      	Received unexpected error:
        	            	Get "http://127.0.0.1:8765/metrics": dial tcp 127.0.0.1:8765: connect: connection refused
        	Test:       	TestFalco_Miscs_PrometheusMetricsNoDriver

Basically, with the container plugin loaded and -M passed, Falco would leave immediately instead of waiting 5seconds (in nodriver mode) if there were containers running on the system.

Which issue(s) this PR fixes:

Fixes #

Special notes for your reviewer:

Does this PR introduce a user-facing change?:

NONE

@FedeDP
Copy link
Contributor Author

FedeDP commented Feb 26, 2025

/milestone 0.41.0

@FedeDP
Copy link
Contributor Author

FedeDP commented Feb 26, 2025

/hold

… diff is > 0.

Signed-off-by: Federico Di Pierro <nierro92@gmail.com>
@FedeDP FedeDP force-pushed the fix/do_not_account_for_asyncevents branch from 4349307 to d0f18bd Compare February 26, 2025 16:26
@FedeDP
Copy link
Contributor Author

FedeDP commented Feb 26, 2025

/unhold

Copy link
Member

@leogr leogr left a comment

Choose a reason for hiding this comment

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

good catch!

Copy link
Member

@Andreagit97 Andreagit97 left a comment

Choose a reason for hiding this comment

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

/approve

@poiana
Copy link
Contributor

poiana commented Feb 26, 2025

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: Andreagit97, FedeDP

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@poiana poiana merged commit 9cbfdda into master Feb 26, 2025
32 checks passed
@poiana poiana deleted the fix/do_not_account_for_asyncevents branch February 26, 2025 21:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

4 participants