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.

Sign up for GitHub

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

Allow to launch fluentd from C:\Program Files\ #2920

Merged
merged 6 commits into from
Apr 10, 2020
+64 −14
Merged

Allow to launch fluentd from C:\Program Files\ #2920

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
eba73d9
Allow to launch fluentd from `C:\Program Files\`
ashie Mar 30, 2020
3f1008e
Add a test for #2915
ashie Mar 31, 2020
8ec8f3e
Fix typo
ashie Mar 31, 2020
6dc3955
test: Fix the assertion message of assert_log_match
ashie Apr 6, 2020
2dd1d37
Fix an omitted test "-E option is set to RUBYOPT" on Windows
ashie Apr 6, 2020
5c129dd
test: Show the correct reason for an omitted test
ashie Apr 6, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 7 additions & 4 deletions lib/fluent/supervisor.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -882,7 +882,11 @@ def build_system_config(conf)
RUBY_ENCODING_OPTIONS_REGEX = %r{\A(-E|--encoding=|--internal-encoding=|--external-encoding=)}.freeze

def build_spawn_command
fluentd_spawn_cmd = [ServerEngine.ruby_bin_path]
if ENV['TEST_RUBY_PATH']
fluentd_spawn_cmd = [ENV['TEST_RUBY_PATH']]
else
fluentd_spawn_cmd = [ServerEngine.ruby_bin_path]
end

rubyopt = ENV['RUBYOPT']
if rubyopt
Expand All @@ -897,10 +901,9 @@ def build_spawn_command

# Adding `-h` so that it can avoid ruby's command blocking
# e.g. `ruby -Eascii-8bit:ascii-8bit` will block. but `ruby -Eascii-8bit:ascii-8bit -h` won't.
cmd = fluentd_spawn_cmd.join(' ')
_, e, s = Open3.capture3("#{cmd} -h")
_, e, s = Open3.capture3(*fluentd_spawn_cmd, "-h")
if s.exitstatus != 0
$log.error('Invalid option is passed to RUBYOPT', command: cmd, error: e)
$log.error('Invalid option is passed to RUBYOPT', command: fluentd_spawn_cmd, error: e)
exit s.exitstatus
end

Expand Down
67 changes: 57 additions & 10 deletions test/command/test_fluentd.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ class TestFluentdCommand < ::Test::Unit::TestCase
FileUtils.mkdir_p(TMP_DIR)
@supervisor_pid = nil
@worker_pids = []
ENV["TEST_RUBY_PATH"] = nil
end

def process_exist?(pid)
Expand Down Expand Up @@ -98,7 +99,8 @@ def eager_read(io)

def assert_log_matches(cmdline, *pattern_list, patterns_not_match: [], timeout: 10, env: {})
matched = false
assert_error_msg = "matched correctly"
matched_wrongly = false
assert_error_msg = ""
stdio_buf = ""
begin
execute_command(cmdline, TMP_DIR, env) do |pid, stdout|
Expand Down Expand Up @@ -128,23 +130,36 @@ def assert_log_matches(cmdline, *pattern_list, patterns_not_match: [], timeout:
end
end
rescue Timeout::Error
assert_error_msg = "execution timeout with command out:\n" + stdio_buf
assert_error_msg = "execution timeout"
rescue => e
assert_error_msg = "unexpected error in launching fluentd: #{e.inspect}\n" + stdio_buf
assert_error_msg = "unexpected error in launching fluentd: #{e.inspect}"
else
assert_error_msg = "log doesn't match" unless matched
end
assert matched, assert_error_msg

unless patterns_not_match.empty?
if patterns_not_match.empty?
assert_error_msg = build_message(assert_error_msg,
"<?>\nwas expected to include:\n<?>",
stdio_buf, pattern_list)
else
lines = stdio_buf.split("\n")
patterns_not_match.each do |ptn|
matched_wrongly = if ptn.is_a? Regexp
lines.any?{|line| ptn.match(line) }
else
lines.any?{|line| line.include?(ptn) }
end
assert_false matched_wrongly, "pattern exists in logs wrongly:\n" + stdio_buf
if matched_wrongly
assert_error_msg << "\n" unless assert_error_msg.empty?
assert_error_msg << "pattern exists in logs wrongly: #{ptn}"
end
end
assert_error_msg = build_message(assert_error_msg,
"<?>\nwas expected to include:\n<?>\nand not include:\n<?>",
stdio_buf, pattern_list, patterns_not_match)
end

assert matched && !matched_wrongly, assert_error_msg
end

def assert_fluentd_fails_to_start(cmdline, *pattern_list, timeout: 10)
Expand Down Expand Up @@ -842,8 +857,7 @@ def multi_workers_ready?
'-external-encoding' => '--external-encoding=utf-8',
'-internal-encoding' => '--internal-encoding=utf-8',
)
test "-E option is set to RUBYOPT3" do |opt|
omit "hard to run correctly on Windows. Need to debug." if Fluent.windows?
test "-E option is set to RUBYOPT" do |opt|
conf = <<CONF
<source>
@type dummy
Expand All @@ -854,6 +868,7 @@ def multi_workers_ready?
</match>
CONF
conf_path = create_conf_file('rubyopt_test.conf', conf)
opt << " #{ENV['RUBYOPT']}" if ENV['RUBYOPT']
assert_log_matches(
create_cmdline(conf_path),
*opt.split(' '),
Expand All @@ -862,7 +877,7 @@ def multi_workers_ready?
)
end

test "without RUBYOPT" do
test "without RUBYOPT" do
conf = <<CONF
<source>
@type dummy
Expand All @@ -877,7 +892,7 @@ def multi_workers_ready?
end

test 'invalid values are set to RUBYOPT' do
omit "hard to run correctly on Windows. Need to debug." if Fluent.windows?
omit "hard to run correctly because RUBYOPT=-r/path/to/bundler/setup is required on Windows while this test set invalid RUBYOPT" if Fluent.windows?
Copy link
Member

Choose a reason for hiding this comment

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

Doesn't it work when RUBYOPT is -r/path/to/bundler/setup a?

Copy link
Member Author

Choose a reason for hiding this comment

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

It doesn't work since the invalid option a is also passed:

can't find gem bundler (>= 0.a) with executable bundle (Gem::GemNotFoundException)
	from C:/tools/ruby25/lib/ruby/2.5.0/rubygems.rb:308:in `activate_bin_path'
	from C:/tools/ruby25/bin/bundle:23:in `<main>'

Copy link
Member

Choose a reason for hiding this comment

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

Understood.

Copy link
Member Author

Choose a reason for hiding this comment

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

Sorry, the error log was wrong. The correct one is:

C:/Users/aho/Projects/fluentd/test/command/test_fluentd.rb:162:in `assert_log_matches'
Failure: test: invalid values are set to RUBYOPT(TestFluentdCommand::configured to run 2 workers):
  log doesn't match.
  <"C:\\tools\\ruby25\\bin\\ruby.exe: invalid switch in RUBYOPT: -a (RuntimeError)\r\n">
  was expected to include:
  <["Invalid option is passed to RUBYOPT"]>

conf = <<CONF
<source>
@type dummy
Expand All @@ -895,6 +910,38 @@ def multi_workers_ready?
)
end

# https://github.com/fluent/fluentd/issues/2915
test "ruby path contains spaces" do
conf = <<CONF
<source>
@type dummy
tag dummy
</source>
<match>
@type null
</match>
CONF
ruby_path = ServerEngine.ruby_bin_path
tmp_ruby_path = File.join(TMP_DIR, "ruby with spaces")
ashie marked this conversation as resolved.
Show resolved Hide resolved
if Fluent.windows?
tmp_ruby_path << ".bat"
File.open(tmp_ruby_path, "w") do |file|
file.write "#{ruby_path} %*"
end
else
FileUtils.ln_sf(ruby_path, tmp_ruby_path)
end
ENV["TEST_RUBY_PATH"] = tmp_ruby_path
cmd_path = File.expand_path(File.dirname(__FILE__) + "../../../bin/fluentd")
conf_path = create_conf_file('space_mixed_ruby_path_test.conf', conf)
args = ["bundle", "exec", tmp_ruby_path, cmd_path, "-c", conf_path]
assert_log_matches(
args,
'spawn command to main:',
'-Eascii-8bit:ascii-8bit'
)
end

test 'success to start workers when file buffer is configured in non-workers way only for specific worker' do
conf = <<CONF
<system>
Expand Down