tests: runtime: plugins: Enable to build and run runtime testcases on windows - #12092
Conversation
Signed-off-by: Hiroshi Hatake <hiroshi@chronosphere.io>
Signed-off-by: Hiroshi Hatake <hiroshi@chronosphere.io>
Signed-off-by: Hiroshi Hatake <hiroshi@chronosphere.io>
Signed-off-by: Hiroshi Hatake <hiroshi@chronosphere.io>
Signed-off-by: Hiroshi Hatake <hiroshi@chronosphere.io>
Signed-off-by: Hiroshi Hatake <hiroshi@chronosphere.io>
Signed-off-by: Hiroshi Hatake <hiroshi@chronosphere.io>
Signed-off-by: Hiroshi Hatake <hiroshi@chronosphere.io>
Signed-off-by: Hiroshi Hatake <hiroshi@chronosphere.io>
Signed-off-by: Hiroshi Hatake <hiroshi@chronosphere.io>
Signed-off-by: Hiroshi Hatake <hiroshi@chronosphere.io>
Signed-off-by: Hiroshi Hatake <hiroshi@chronosphere.io>
Signed-off-by: Hiroshi Hatake <hiroshi@chronosphere.io>
Signed-off-by: Hiroshi Hatake <hiroshi@chronosphere.io>
Signed-off-by: Hiroshi Hatake <hiroshi@chronosphere.io>
Signed-off-by: Hiroshi Hatake <hiroshi@chronosphere.io>
Signed-off-by: Hiroshi Hatake <hiroshi@chronosphere.io>
Signed-off-by: Hiroshi Hatake <hiroshi@chronosphere.io>
Signed-off-by: Hiroshi Hatake <hiroshi@chronosphere.io>
Signed-off-by: Hiroshi Hatake <hiroshi@chronosphere.io>
Signed-off-by: Hiroshi Hatake <hiroshi@chronosphere.io>
Signed-off-by: Hiroshi Hatake <hiroshi@chronosphere.io>
Signed-off-by: Hiroshi Hatake <hiroshi@chronosphere.io>
Signed-off-by: Hiroshi Hatake <hiroshi@chronosphere.io>
Signed-off-by: Hiroshi Hatake <hiroshi@chronosphere.io>
Signed-off-by: Hiroshi Hatake <hiroshi@chronosphere.io>
Signed-off-by: Hiroshi Hatake <hiroshi@chronosphere.io>
Signed-off-by: Hiroshi Hatake <hiroshi@chronosphere.io>
Signed-off-by: Hiroshi Hatake <hiroshi@chronosphere.io>
Signed-off-by: Hiroshi Hatake <hiroshi@chronosphere.io>
Signed-off-by: Hiroshi Hatake <hiroshi@chronosphere.io>
Signed-off-by: Hiroshi Hatake <hiroshi@chronosphere.io>
Signed-off-by: Hiroshi Hatake <hiroshi@chronosphere.io>
Signed-off-by: Hiroshi Hatake <hiroshi@chronosphere.io>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 911c18a5bf
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (2)
tests/runtime_shell/custom_calyptia.ps1 (1)
24-28: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winReplace fixed 30-second sleep with bounded polling.
Start-Sleep -Seconds 30always waits the full duration even if YAML files appear immediately, and delays failure detection when Fluent Bit crashes early. The PR objectives explicitly call out "replacing fixed test delays with bounded polling or completion synchronization," andcommon.ps1already providesWait-ForFilefor this pattern. A similar polling loop checking for YAML files in$env:CALYPTIA_FLEET_DIRwould return sooner on success and detect early exit faster.♻️ Proposed bounded-polling replacement for lines 24-28
$process = Start-FluentBit $config -Start-Sleep -Seconds 30 -if ($process.HasExited -and $process.ExitCode -ne 0) { - throw "Fluent Bit exited with code $($process.ExitCode)" -} + +$deadline = [DateTime]::UtcNow.AddSeconds(30) +while ([DateTime]::UtcNow -lt $deadline) { + $yamlFiles = @(Get-ChildItem -LiteralPath $env:CALYPTIA_FLEET_DIR ` + -Filter "*.yaml" -File -Recurse -ErrorAction SilentlyContinue) + if ($yamlFiles.Count -gt 0) { + break + } + if ($process.HasExited) { + throw "Fluent Bit exited before generating YAML files (code $($process.ExitCode))" + } + Start-Sleep -Milliseconds 100 +} $yamlFiles = @(Get-ChildItem -LiteralPath $env:CALYPTIA_FLEET_DIR ` -Filter "*.yaml" -File -Recurse -ErrorAction SilentlyContinue)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/runtime_shell/custom_calyptia.ps1` around lines 24 - 28, Replace the fixed Start-Sleep call in the Start-FluentBit flow with bounded polling for YAML files under $env:CALYPTIA_FLEET_DIR, reusing Wait-ForFile from common.ps1 where applicable. Poll until the expected files appear or the timeout expires, checking $process.HasExited during polling to fail promptly with its exit code, while preserving the existing failure handling.tests/runtime/out_s3.c (1)
108-144: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd timeout diagnostics to S3 polling helpers for consistency.
wait_for_s3_call_countandwait_for_file_countsilently return on timeout without any log, unlikewait_with_timeoutinfilter_kubernetes.cwhich emitsflb_warn("[timeout] elapsed_time: %ld", ...). A timeout warning would help distinguish "condition never met" from "logical assertion failure" during test debugging.♻️ Proposed timeout warning for both helpers
static void wait_for_s3_call_count(const char *api, int expected) { uint64_t elapsed_ms; struct flb_time start_time; struct flb_time end_time; struct flb_time diff_time; elapsed_ms = 0; flb_time_get(&start_time); while (get_s3_call_count(api) < expected && elapsed_ms < S3_TEST_WAIT_TIMEOUT_MS) { flb_time_msleep(S3_TEST_WAIT_STEP_MS); flb_time_get(&end_time); flb_time_diff(&end_time, &start_time, &diff_time); elapsed_ms = flb_time_to_nanosec(&diff_time) / 1000000; } + if (elapsed_ms >= S3_TEST_WAIT_TIMEOUT_MS) { + flb_warn("[timeout] wait_for_s3_call_count(%s, %d) elapsed: %llu ms", + api, expected, (unsigned long long) elapsed_ms); + } } static void wait_for_file_count(const char *path, int expected) { uint64_t elapsed_ms; struct flb_time start_time; struct flb_time end_time; struct flb_time diff_time; elapsed_ms = 0; flb_time_get(&start_time); while (count_files_recursive(path) < expected && elapsed_ms < S3_TEST_WAIT_TIMEOUT_MS) { flb_time_msleep(S3_TEST_WAIT_STEP_MS); flb_time_get(&end_time); flb_time_diff(&end_time, &start_time, &diff_time); elapsed_ms = flb_time_to_nanosec(&diff_time) / 1000000; } + if (elapsed_ms >= S3_TEST_WAIT_TIMEOUT_MS) { + flb_warn("[timeout] wait_for_file_count(%s, %d) elapsed: %llu ms", + path, expected, (unsigned long long) elapsed_ms); + } }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/runtime/out_s3.c` around lines 108 - 144, Add timeout warnings to both wait_for_s3_call_count and wait_for_file_count after their polling loops when the expected condition remains unmet, including the elapsed timeout duration in the log. Keep the existing polling and success behavior unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@plugins/filter_throttle/throttle.c`:
- Around line 95-97: Update the Windows sleep path in the ticker loop around
pthread_testcancel() to divide long sleeps into shorter intervals, invoking
pthread_testcancel() between each interval and preserving the total configured
delay. Keep cb_throttle_exit shutdown behavior unchanged while ensuring
cancellation is observed promptly during the sleep.
In `@tests/runtime_shell/common.ps1`:
- Around line 63-70: Update Stop-FluentBit to tolerate the process exiting
between the HasExited check and Stop-Process invocation. Handle the expected
Stop-Process failure from concurrent exit locally so the global Stop error
policy cannot mask the test result, while preserving the existing cleanup and
wait behavior.
In `@tests/runtime_shell/processor_conditional.ps1`:
- Around line 13-17: Replace fixed sleeps with bounded polling in all three
sites: in tests/runtime_shell/processor_conditional.ps1 lines 13-17, poll
captured output for the expected records before stopping Fluent Bit; in
tests/runtime_shell/processor_conditional.sh lines 20-21, poll for the GET
marker while verifying the POST marker remains absent; and in
tests/runtime_shell/processor_conditional.sh lines 56-57, poll until both
accepted endpoints appear or the timeout/process exit condition occurs.
In `@tests/runtime_shell/processor_conditional.sh`:
- Around line 23-36: Update every Fluent Bit termination path in the test,
including the missing-output branch and the cleanup around both cases, to wait
for the process after sending SIGTERM. Preserve the existing tolerant kill
behavior, then invoke the appropriate wait on FLB_PID before proceeding,
exiting, or starting the next case so the process has fully terminated.
---
Nitpick comments:
In `@tests/runtime_shell/custom_calyptia.ps1`:
- Around line 24-28: Replace the fixed Start-Sleep call in the Start-FluentBit
flow with bounded polling for YAML files under $env:CALYPTIA_FLEET_DIR, reusing
Wait-ForFile from common.ps1 where applicable. Poll until the expected files
appear or the timeout expires, checking $process.HasExited during polling to
fail promptly with its exit code, while preserving the existing failure
handling.
In `@tests/runtime/out_s3.c`:
- Around line 108-144: Add timeout warnings to both wait_for_s3_call_count and
wait_for_file_count after their polling loops when the expected condition
remains unmet, including the elapsed timeout duration in the log. Keep the
existing polling and success behavior unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 06b5fc6d-f0e5-4c0f-90d2-66cf5fcee9a8
📒 Files selected for processing (68)
CMakeLists.txtcmake/windows-setup.cmakeplugins/filter_kubernetes/kube_meta.cplugins/filter_throttle/throttle.cplugins/in_event_test/event_test.cplugins/in_kubernetes_events/kubernetes_events.cplugins/in_tail/tail_scan_win32.cplugins/out_loki/loki.cplugins/out_s3/s3.cplugins/out_syslog/syslog.ctests/include/aws_client_mock.htests/runtime/CMakeLists.txttests/runtime/core-timeout.ctests/runtime/core_accept_timeout.ctests/runtime/core_chunk_trace.ctests/runtime/core_engine.ctests/runtime/counter_parity_e2e.ctests/runtime/custom_calyptia_input_test.ctests/runtime/filter_kubernetes.ctests/runtime/filter_parser.ctests/runtime/filter_throttle_size.ctests/runtime/flb_tests_runtime.h.intests/runtime/group_counter_semantics.ctests/runtime/http_client_chunked.ctests/runtime/in_calyptia_fleet_test.ctests/runtime/in_dummy.ctests/runtime/in_event_test.ctests/runtime/in_forward.ctests/runtime/in_http.ctests/runtime/in_kubernetes_events.ctests/runtime/in_opentelemetry_routing.ctests/runtime/in_prometheus_textfile.ctests/runtime/in_random.ctests/runtime/in_simple_systems.ctests/runtime/in_syslog.ctests/runtime/in_tail.ctests/runtime/in_tcp.ctests/runtime/in_udp.ctests/runtime/out_counter.ctests/runtime/out_firehose.ctests/runtime/out_http.ctests/runtime/out_logdna.ctests/runtime/out_s3.ctests/runtime/out_s3_otlp_json.ctests/runtime/out_stackdriver.ctests/runtime/out_syslog.ctests/runtime/processor_cumulative_to_delta.ctests/runtime_shell/CMakeLists.txttests/runtime_shell/common.ps1tests/runtime_shell/conf/dry_run_invalid_property.yamltests/runtime_shell/conf/in_tail_expect.conftests/runtime_shell/conf/processor_conditional.yamltests/runtime_shell/conf/processor_conditional_grep.yamltests/runtime_shell/conf/processor_invalid.yamltests/runtime_shell/custom_calyptia.ps1tests/runtime_shell/dry_run_invalid_property.ps1tests/runtime_shell/dry_run_invalid_property.shtests/runtime_shell/in_dummy_expect.ps1tests/runtime_shell/in_http_tls_expect.ps1tests/runtime_shell/in_syslog_tcp_plaintext_expect.ps1tests/runtime_shell/in_syslog_tcp_tls_expect.ps1tests/runtime_shell/in_syslog_udp_plaintext_expect.ps1tests/runtime_shell/in_tail_expect.ps1tests/runtime_shell/in_tail_expect.shtests/runtime_shell/processor_conditional.ps1tests/runtime_shell/processor_conditional.shtests/runtime_shell/processor_invalid.ps1tests/runtime_shell/processor_invalid.sh
| /* Windows sleep is not a pthread cancellation point. */ | ||
| sleep(ctx->ticker_data.seconds); | ||
| pthread_testcancel(); |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
Shutdown may block for the full sleep duration on Windows.
pthread_testcancel() after sleep() correctly enables cancellation, but pthread_join() in cb_throttle_exit (Line 285) will block until the sleep completes. If ctx->ticker_data.seconds is large, shutdown could be delayed significantly. Consider breaking the sleep into smaller increments with pthread_testcancel() between them for faster cancellation response.
♻️ Suggested improvement
/* Windows sleep is not a pthread cancellation point. */
- sleep(ctx->ticker_data.seconds);
- pthread_testcancel();
+ {
+ int remaining = ctx->ticker_data.seconds;
+ while (remaining > 0) {
+ int chunk = remaining > 5 ? 5 : remaining;
+ sleep(chunk);
+ pthread_testcancel();
+ remaining -= chunk;
+ }
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| /* Windows sleep is not a pthread cancellation point. */ | |
| sleep(ctx->ticker_data.seconds); | |
| pthread_testcancel(); | |
| /* Windows sleep is not a pthread cancellation point. */ | |
| { | |
| int remaining = ctx->ticker_data.seconds; | |
| while (remaining > 0) { | |
| int chunk = remaining > 5 ? 5 : remaining; | |
| sleep(chunk); | |
| pthread_testcancel(); | |
| remaining -= chunk; | |
| } | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@plugins/filter_throttle/throttle.c` around lines 95 - 97, Update the Windows
sleep path in the ticker loop around pthread_testcancel() to divide long sleeps
into shorter intervals, invoking pthread_testcancel() between each interval and
preserving the total configured delay. Keep cb_throttle_exit shutdown behavior
unchanged while ensuring cancellation is observed promptly during the sleep.
| function Stop-FluentBit | ||
| { | ||
| param([System.Diagnostics.Process] $Process) | ||
|
|
||
| if ($null -ne $Process -and -not $Process.HasExited) { | ||
| Stop-Process -Id $Process.Id -Force | ||
| [void] $Process.WaitForExit(5000) | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
Make process cleanup tolerant of concurrent exit.
The process can exit after HasExited is checked, causing Stop-Process to throw under the global Stop error policy and mask the real test result.
Proposed fix
if ($null -ne $Process -and -not $Process.HasExited) {
- Stop-Process -Id $Process.Id -Force
+ Stop-Process -Id $Process.Id -Force -ErrorAction SilentlyContinue
[void] $Process.WaitForExit(5000)
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| function Stop-FluentBit | |
| { | |
| param([System.Diagnostics.Process] $Process) | |
| if ($null -ne $Process -and -not $Process.HasExited) { | |
| Stop-Process -Id $Process.Id -Force | |
| [void] $Process.WaitForExit(5000) | |
| } | |
| function Stop-FluentBit | |
| { | |
| param([System.Diagnostics.Process] $Process) | |
| if ($null -ne $Process -and -not $Process.HasExited) { | |
| Stop-Process -Id $Process.Id -Force -ErrorAction SilentlyContinue | |
| [void] $Process.WaitForExit(5000) | |
| } |
🧰 Tools
🪛 PSScriptAnalyzer (1.25.0)
[warning] 63-63: Function 'Stop-FluentBit' has verb that could change system state. Therefore, the function has to support 'ShouldProcess'.
(PSUseShouldProcessForStateChangingFunctions)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tests/runtime_shell/common.ps1` around lines 63 - 70, Update Stop-FluentBit
to tolerate the process exiting between the HasExited check and Stop-Process
invocation. Handle the expected Stop-Process failure from concurrent exit
locally so the global Stop error policy cannot mask the test result, while
preserving the existing cleanup and wait behavior.
| $process = Start-FluentBit $ConfigPath @("-o", "stdout") $OutputPath $ErrorPath | ||
| Start-Sleep -Seconds 5 | ||
| if ($process.HasExited -and $process.ExitCode -ne 0) { | ||
| throw "Fluent Bit exited with code $($process.ExitCode)" | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
Replace fixed delays with bounded completion polling across both implementations.
tests/runtime_shell/processor_conditional.ps1#L13-L17: poll captured output for the expected records before stopping Fluent Bit.tests/runtime_shell/processor_conditional.sh#L20-L21: poll for the GET marker and ensure the POST marker remains absent.tests/runtime_shell/processor_conditional.sh#L56-L57: poll until both accepted endpoints are present or timeout/process exit occurs.
📍 Affects 2 files
tests/runtime_shell/processor_conditional.ps1#L13-L17(this comment)tests/runtime_shell/processor_conditional.sh#L20-L21tests/runtime_shell/processor_conditional.sh#L56-L57
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tests/runtime_shell/processor_conditional.ps1` around lines 13 - 17, Replace
fixed sleeps with bounded polling in all three sites: in
tests/runtime_shell/processor_conditional.ps1 lines 13-17, poll captured output
for the expected records before stopping Fluent Bit; in
tests/runtime_shell/processor_conditional.sh lines 20-21, poll for the GET
marker while verifying the POST marker remains absent; and in
tests/runtime_shell/processor_conditional.sh lines 56-57, poll until both
accepted endpoints appear or the timeout/process exit condition occurs.
| if [ ! -f "$OUTPUT_FILE" ]; then | ||
| echo "Output file not found" | ||
| kill -15 $FLB_PID || true | ||
| kill -15 "$FLB_PID" || true | ||
| exit 1 | ||
| fi | ||
|
|
||
| echo "Output file content:" | ||
| cat $OUTPUT_FILE | ||
| cat "$OUTPUT_FILE" | ||
|
|
||
| # Verify that the GET condition was applied but not the POST condition | ||
| GET_FIELD=$(grep -c "modified_if_get" $OUTPUT_FILE) | ||
| POST_FIELD=$(grep -c "modified_if_post" $OUTPUT_FILE) | ||
| GET_FIELD=$(grep -c "modified_if_get" "$OUTPUT_FILE") | ||
| POST_FIELD=$(grep -c "modified_if_post" "$OUTPUT_FILE") | ||
|
|
||
| # Clean up | ||
| echo "Cleaning up..." | ||
| kill -15 $FLB_PID || true | ||
| rm -f /tmp/processor_conditional.yaml | ||
| rm -f $OUTPUT_FILE | ||
| kill -15 "$FLB_PID" || true | ||
| rm -f "$OUTPUT_FILE" |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Wait for Fluent Bit after sending SIGTERM.
kill only requests termination. Without wait, the test can proceed or exit while Fluent Bit remains alive, and the first process may overlap the second case.
Proposed cleanup pattern
kill -15 "$FLB_PID" || true
+wait "$FLB_PID" 2>/dev/null || true
rm -f "$OUTPUT_FILE"Apply the same sequence to every termination path.
Also applies to: 59-73
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tests/runtime_shell/processor_conditional.sh` around lines 23 - 36, Update
every Fluent Bit termination path in the test, including the missing-output
branch and the cleanup around both cases, to wait for the process after sending
SIGTERM. Preserve the existing tolerant kill behavior, then invoke the
appropriate wait on FLB_PID before proceeding, exiting, or starting the next
case so the process has fully terminated.
Verification
Runtime testing is enabled on Windows.
The complete runtime test suite was executed, rather than only the tests related
to the initial changes. This broader verification exposed that the
runtime_shellcases depended on Bash scripts and therefore could not runnatively on Windows.
The affected shell test runners were converted to PowerShell. The Bash and
PowerShell runners use the same external configuration files; Fluent Bit
configuration content is not embedded in the PowerShell scripts.
Final results:
flb-rt-in_event_test: passed, including five consecutive executions.out_exit: built and tested with its Windows default enabled.git diff --check: passed.the Windows test path.
Log
Enter
[N/A]in the box, if an item is not applicable to your change.Testing
Before we can approve your change; please submit the following in a comment:
If this is a change to packaging of containers or native binaries then please confirm it works for all targets.
ok-package-testlabel to test for all targets (requires maintainer to do).Documentation
Backporting
Fluent Bit is licensed under Apache 2.0, by submitting this pull request I understand that this code will be released under the terms of that license.
Summary by CodeRabbit
Bug Fixes
Tests