Prevent accidental duplicate launching

[daipom]

fluentd or td-agent command launches Fluentd with the default config path. This makes it easy to start Fluentd, but it also causes accidental duplicate launching.

For example, it launches Fluentd duplicated while Fluentd is running as a systemd unit.

Processing the same files, such as buffer files, pos files, and output destination files, causes unassumed errors. At worst, it may cause file corruption or data loss.

The current specification makes it too easy to start Fluentd, so I'm concerned about mishandling. For example, trying to check the version and launching Fluentd by mistake, etc.

The current specification

The default config path is set in env variables.

https://github.com/fluent/fluent-package-builder/blob/c75754e6619dab73920fdb386c0de67b77c83e22/fluent-package/templates/usr/sbin/fluentd.erb#L4

So, we can start Fluentd without any command arguments.

fluent-package:

$ fluentd

td-agent:

$ td-agent

What I want to do

I want to add some prevention. For example,

• Add confirmation that you are really trying to start Fluentd

It would change the specifications, so we need to be careful about compatibility.

[kenhys]

It may be better to implement such a guard feature in fluentd not fluent-package-builder.

[kenhys]

https://github.com/fluent/fluentd/blob/master/lib/fluent/command/fluentd.rb#L343 It is enough to guard before starting service appropriately. :thinking:

[daipom]

It may be better to implement such a guard feature in fluentd not fluent-package-builder.

I too think it may end up that way. However, since the specification of easy duplicate launching is only a problem of the package, it is necessary to consider how to take appropriate countermeasures on either side.

[kenhys]

Use case on Windows:

• Already started fluentd as a service (fluentd)
• Launch Fluent Package Prompt with Administrator privilege
• Execute another instance via fluentd -v

If fluentdopt is not changed, usually c:/opt/fluent/etc/fluent/fluentd.conf will be used in both cases. Thus, blocking from c:/opt/fluent/fluentd.bat (msi/assets/fluentd.bat) is effective if it will be fixed in fluent-package side.

c:\opt\fluent>where fluentd
c:\opt\fluent\fluentd.bat
c:\opt\fluent\bin\fluentd
c:\opt\fluent\bin\fluentd.bat

Use case in Linux:

• Already started fluentd as a service (fluentdwinsvc)
• Execute another instance via fluentd -v

If fluentd.service is not changed, usually /etc/fluent/fluentd.conf (FLUENT_CONF) will be used. The default path of fluentd itself is /etc/fluent/fluent.conf, Thus same configuration is not used by default.

If configuration path is specified explicitly with -C /etc/fluent/fluentd.conf, need to guard it.

/opt/fluent/bin/fluentd is autogenerated file, so lib/fluent/comman/fluentd may be better to block it.

NOTE: both of cases, finally call lib/fluent/comand/fluentd (/opt/fluent/bin/fluentd => gems.../bin/fluentd => gems../lib/fluent/command/fluentd)

[kenhys]

PoC PR for this issue.

https://github.com/fluent/fluentd/pull/4399

Not stable yet because the guard condition fires unexpectedly.

[kenhys]

Reconsider, it is not appropriate to introduce package specific fix to fluentd internals. 🤔

[kenhys]

• Linux: guard in template fluent-package/templates/usr/sbin/fluentd.erb
• Windows: guard in assets fluent-package/msi/assets/fluentd.bat

[daipom]

Thanks!

Use case in Linux: ... If fluentd.service is not changed, usually /etc/fluent/fluentd.conf (FLUENT_CONF) will be used. The default path of fluentd itself is /etc/fluent/fluent.conf, Thus same configuration is not used by default.

The default path of fluentd itself is /etc/fluent/fluent.conf

This?

https://github.com/fluent/fluentd/blob/2b4ca5d2927b706c3bdc98ffd0a0b66232bc0b65/lib/fluent/env.rb#L21

DEFAULT_CONFIG_PATH = ENV['FLUENT_CONF'] || '/etc/fluent/fluent.conf'

However, /usr/sbin/fluentd sets the FLUENT_CONF environmental var. So, the same configuration is used by default.

https://github.com/fluent/fluent-package-builder/blob/88e84849773def85e52aa63ae0e93b30a2b5db50/fluent-package/templates/usr/sbin/fluentd.erb#L4

[daipom]

• #617

is this for Linux?

[kenhys]

However, /usr/sbin/fluentd sets the FLUENT_CONF environmental var. So, the same configuration is used by default.

Yes, so fixed via #622.

[kenhys]

both of PR was ready for review.

https://github.com/fluent/fluent-package-builder/pull/617 https://github.com/fluent/fluent-package-builder/pull/622

[daipom]

Thanks! Sorry for keeping you waiting. I will see it tomorrow or this weekend.

[kenhys]

msi: #648 has been merged into master.

TODO: when #617 has been merged, we can close this issue.