Skip to content

test_fluentd: read until EOF in assert_fluentd_fails_to_start#5447

Open
Watson1978 wants to merge 1 commit into
fluent:masterfrom
Watson1978:fix-flaky-assert-fluentd-fails-to-start
Open

test_fluentd: read until EOF in assert_fluentd_fails_to_start#5447
Watson1978 wants to merge 1 commit into
fluent:masterfrom
Watson1978:fix-flaky-assert-fluentd-fails-to-start

Conversation

@Watson1978

@Watson1978 Watson1978 commented Jul 19, 2026

Copy link
Copy Markdown
Contributor

Which issue(s) this PR fixes:
Fixes #

What this PR does / why we need it:
Gating the read loop on process_exist?(pid) is racy: fluentd can write its final error line and exit right after eager_read drains the pipe, so the loop exits (process gone) before that last line is read. The error message is then missing from stdio_buf and the assertion fails even though fluentd did fail to start as expected.

This showed up as a flaky failure of "fails to launch fluentd if specified root path is invalid path for directory": the captured output ended at "bundler: failed to load command:" without the following "non directory entry exists:" line.

Loop until the pipe reaches EOF (the write-end is closed when the process exits) instead, so the remaining buffered output is always drained. The success case still breaks early once the worker is running, and hangs are still caught by the outer waiting(timeout). This mirrors the existing EOF handling in assert_log_matches.

This PR will fix following error:

4) Failure: test: fails to launch fluentd if specified root path is invalid path for directory(TestFluentdCommand::with system configuration about root directory):
  fluentd failed to start, without specified regular expressions:
  2026-07-19 09:09:44 +0000 [info]: init supervisor logger path=nil rotate_age=nil rotate_size=nil
  2026-07-19 09:09:44 +0000 [info]: parsing config file is succeeded path="/Users/runner/work/fluentd/fluentd/test/command/../tmp/commandfluentd/ce6c61934634f7909ab7/existing_root_dir.conf"
  2026-07-19 09:09:44 +0000 [info]: inaccessible include directory was specified path="/etc/fluent/conf.d"
  2026-07-19 09:09:44 +0000 [info]: gem 'fluentd' version '1.19.0'
  bundler: failed to load command: /Users/runner/work/fluentd/fluentd/bin/fluentd (/Users/runner/work/fluentd/fluentd/bin/fluentd)
  .
  <false> is not true.
/Users/runner/work/fluentd/fluentd/test/command/test_fluentd.rb:271:in `assert_fluentd_fails_to_start'
/Users/runner/work/fluentd/fluentd/test/command/test_fluentd.rb:378:in `block (2 levels) in <class:TestFluentdCommand>'
     375:       end
     376:       conf_path = create_conf_file('existing_root_dir.conf', @conf)
     377: 
  => 378:       assert_fluentd_fails_to_start(
     379:         create_cmdline(conf_path),
     380:         "non directory entry exists:#{@root_path}",
     381:       )

Ref. https://github.com/fluent/fluentd/actions/runs/29680404414/job/88175417915?pr=5446#step:6:5364

Docs Changes:
N/A

Release Note:
N/A

Gating the read loop on `process_exist?(pid)` is racy: fluentd can write
its final error line and exit right after `eager_read` drains the pipe,
so the loop exits (process gone) before that last line is read. The
error message is then missing from `stdio_buf` and the assertion fails
even though fluentd did fail to start as expected.

This showed up as a flaky failure of "fails to launch fluentd if
specified root path is invalid path for directory": the captured output
ended at "bundler: failed to load command:" without the following
"non directory entry exists:" line.

Loop until the pipe reaches EOF (the write-end is closed when the process
exits) instead, so the remaining buffered output is always drained. The
success case still breaks early once the worker is running, and hangs are
still caught by the outer `waiting(timeout)`. This mirrors the existing
EOF handling in `assert_log_matches`.

Signed-off-by: Shizuo Fujita <fujita@clear-code.com>
@Watson1978 Watson1978 added CI Test/CI issues backport to v1.19 We will backport this fix to the LTS branch labels Jul 19, 2026
@Watson1978 Watson1978 added this to the v1.20.0 milestone Jul 19, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backport to v1.19 We will backport this fix to the LTS branch CI Test/CI issues

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant