Skip to content

Commit

Permalink
chumask: fix issue where it only works with standalone worker (#4836)
Browse files Browse the repository at this point in the history
**Which issue(s) this PR fixes**: 
* Fixes #3671

**What this PR does / why we need it**: 
Fix #3671 (88a7260)

After #3671, the value of `chumask` passed to ServerEngine was always
`nil`.
The `chumask` value was being used only when standalone. It appears to
be unintentional.

This fixes it.

* By default, `0` as `chumask` is passed to ServerEngine.
* If specifying `--umask` option, that value is passed.
* This `chumask` value is applied when using `daemonize`.
  * When not using `daemonize`, the value is not used.
  * This is the specification of ServerEngine.

Specification change:
(Just undo the specification that #3671 unintentionally changed.)

* This changes the default `umask` when using `daemonize`.
  * fluent-package (RPM/DEB), for example.
  * Before: system default (`nil` for ServerEngine)
  * After: `0`

**Docs Changes**:
Not needed.
(There is not enough information on
https://docs.fluentd.org/deployment/command-line-option. It would be
good to add more information.)

**Release Note**: 
* Fixed a bug where the default `umask` was not set to `0` when using
`--daemon` (td-agent, fluent-package) since v1.14.6.
* `--umask` command line option: Fixed so that it is applied when
Fluentd runs with `--daemon` (fluent-package) as well as when Fluentd
runs with `--no-supervisor`.

Signed-off-by: Daijiro Fukuda <fukuda@clear-code.com>
  • Loading branch information
daipom authored Feb 26, 2025
1 parent 3227e05 commit 69bdc91
Show file tree
Hide file tree
Showing 3 changed files with 86 additions and 44 deletions.
3 changes: 2 additions & 1 deletion lib/fluent/supervisor.rb
Original file line number Diff line number Diff line change
Expand Up @@ -587,7 +587,7 @@ def self.serverengine_config(params = {})
log_level: params['log_level'],
chuser: params['chuser'],
chgroup: params['chgroup'],
chumask: params['chumask'],
chumask: params['chumask'].is_a?(Integer) ? params['chumask'] : params['chumask']&.to_i(8),
daemonize: daemonize,
rpc_endpoint: params['rpc_endpoint'],
counter_server: params['counter_server'],
Expand Down Expand Up @@ -924,6 +924,7 @@ def supervise
'inline_config' => @inline_config,
'chuser' => @chuser,
'chgroup' => @chgroup,
'chumask' => @chumask,
'fluentd_conf_path' => @config_path,
'fluentd_conf' => @conf.to_s,
'use_v1_config' => @use_v1_config,
Expand Down
8 changes: 8 additions & 0 deletions test/command/test_fluentd.rb
Original file line number Diff line number Diff line change
Expand Up @@ -1336,6 +1336,14 @@ def multi_workers_ready?; true; end
end
end

test "--umask should be recognized as command line" do
conf_path = create_conf_file("empty.conf", "")
assert File.exist?(conf_path)
assert_log_matches(create_cmdline(conf_path, "--umask", "222"),
"spawn command to main:", '"--umask", "222"',
patterns_not_match: ["[error]"])
end

sub_test_case "--with-source-only" do
setup do
omit "Not supported on Windows" if Fluent.windows?
Expand Down
119 changes: 76 additions & 43 deletions test/test_supervisor.rb
Original file line number Diff line number Diff line change
Expand Up @@ -610,51 +610,84 @@ def server.config
assert_equal('{"ok":true}', response)
end

def test_serverengine_config
params = {}
params['workers'] = 1
params['fluentd_conf_path'] = "fluentd.conf"
params['use_v1_config'] = true
params['conf_encoding'] = 'utf-8'
params['log_level'] = Fluent::Log::LEVEL_INFO
load_config_proc = Proc.new { Fluent::Supervisor.serverengine_config(params) }

se_config = load_config_proc.call
assert_equal Fluent::Log::LEVEL_INFO, se_config[:log_level]
assert_equal 'spawn', se_config[:worker_type]
assert_equal 1, se_config[:workers]
assert_equal false, se_config[:log_stdin]
assert_equal false, se_config[:log_stdout]
assert_equal false, se_config[:log_stderr]
assert_equal true, se_config[:enable_heartbeat]
assert_equal false, se_config[:auto_heartbeat]
assert_equal "fluentd.conf", se_config[:config_path]
assert_equal false, se_config[:daemonize]
assert_nil se_config[:pid_path]
sub_test_case "serverengine_config" do
def test_normal
params = {}
params['workers'] = 1
params['fluentd_conf_path'] = "fluentd.conf"
params['use_v1_config'] = true
params['conf_encoding'] = 'utf-8'
params['log_level'] = Fluent::Log::LEVEL_INFO
load_config_proc = Proc.new { Fluent::Supervisor.serverengine_config(params) }

se_config = load_config_proc.call
assert_equal Fluent::Log::LEVEL_INFO, se_config[:log_level]
assert_equal 'spawn', se_config[:worker_type]
assert_equal 1, se_config[:workers]
assert_equal false, se_config[:log_stdin]
assert_equal false, se_config[:log_stdout]
assert_equal false, se_config[:log_stderr]
assert_equal true, se_config[:enable_heartbeat]
assert_equal false, se_config[:auto_heartbeat]
assert_equal "fluentd.conf", se_config[:config_path]
assert_equal false, se_config[:daemonize]
assert_nil se_config[:pid_path]
end

def test_daemonize
params = {}
params['workers'] = 1
params['fluentd_conf_path'] = "fluentd.conf"
params['use_v1_config'] = true
params['conf_encoding'] = 'utf-8'
params['log_level'] = Fluent::Log::LEVEL_INFO
params['daemonize'] = './fluentd.pid'
load_config_proc = Proc.new { Fluent::Supervisor.serverengine_config(params) }

se_config = load_config_proc.call
assert_equal Fluent::Log::LEVEL_INFO, se_config[:log_level]
assert_equal 'spawn', se_config[:worker_type]
assert_equal 1, se_config[:workers]
assert_equal false, se_config[:log_stdin]
assert_equal false, se_config[:log_stdout]
assert_equal false, se_config[:log_stderr]
assert_equal true, se_config[:enable_heartbeat]
assert_equal false, se_config[:auto_heartbeat]
assert_equal "fluentd.conf", se_config[:config_path]
assert_equal true, se_config[:daemonize]
assert_equal './fluentd.pid', se_config[:pid_path]
end

data("nil", [nil, nil])
data("default", ["0", 0])
data("000", ["000", 0])
data("0000", ["0000", 0])
data("2", ["2", 2])
data("222", ["222", 146])
data("0222", ["0222", 146])
data("0 as integer", [0, 0])
def test_chumask((chumask, expected))
params = { "chumask" => chumask }
load_config_proc = Proc.new { Fluent::Supervisor.serverengine_config(params) }

se_config = load_config_proc.call

assert_equal expected, se_config[:chumask]
end
end

def test_serverengine_config_for_daemonize
params = {}
params['workers'] = 1
params['fluentd_conf_path'] = "fluentd.conf"
params['use_v1_config'] = true
params['conf_encoding'] = 'utf-8'
params['log_level'] = Fluent::Log::LEVEL_INFO
params['daemonize'] = './fluentd.pid'
load_config_proc = Proc.new { Fluent::Supervisor.serverengine_config(params) }

se_config = load_config_proc.call
assert_equal Fluent::Log::LEVEL_INFO, se_config[:log_level]
assert_equal 'spawn', se_config[:worker_type]
assert_equal 1, se_config[:workers]
assert_equal false, se_config[:log_stdin]
assert_equal false, se_config[:log_stdout]
assert_equal false, se_config[:log_stderr]
assert_equal true, se_config[:enable_heartbeat]
assert_equal false, se_config[:auto_heartbeat]
assert_equal "fluentd.conf", se_config[:config_path]
assert_equal true, se_config[:daemonize]
assert_equal './fluentd.pid', se_config[:pid_path]
data("default", [{}, "0"])
data("222", [{chumask: "222"}, "222"])
def test_chumask_should_be_passed_to_ServerEngine((cl_opt, expected_chumask_value))
proxy.mock(Fluent::Supervisor).serverengine_config(hash_including("chumask" => expected_chumask_value))
any_instance_of(ServerEngine::Daemon) { |daemon| mock(daemon).run.once }

supervisor = Fluent::Supervisor.new(cl_opt)
stub(Fluent::Config).build { config_element('ROOT') }
stub(supervisor).build_spawn_command { "dummy command line" }
supervisor.configure(supervisor: true)

supervisor.run_supervisor
end

sub_test_case "init logger" do
Expand Down

0 comments on commit 69bdc91

Please sign in to comment.