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

Jump to bottom

linux: guard launching duplicated fluentd instance with same configuration #617

Merged
merged 4 commits into from
Jun 28, 2024
Merged

linux: guard launching duplicated fluentd instance with same configuration #617

merged 4 commits into from
Jun 28, 2024

Conversation

kenhys
Copy link
Contributor

@kenhys kenhys commented Feb 15, 2024 •
edited
Loading

linux: guard duplicated instance (not adding a new option)

Closes: #611

Before:

We can launch Fluentd by /usr/sbin/fluentd even though fluentd service
is running.

Launching multiple Fluentd with the same config may cause
inconsistency of the buffers or the pos files.

After:

We can't launch Fluentd by /usr/sbin/fluentd with the default config
path if fluentd service is running. If one of the following options is
specified, we can execute fluentd as before.

  • --config (-c)
  • --dry-run
$ sudo /usr/sbin/fluentd 
Error: Can't start duplicate Fluentd instance with the default config.
To start Fluentd, please do one of the following:
(Caution: Please be careful not to start multiple instances with the same config.)
- Stop the Fluentd service 'fluentd'.
- Specify the config path explicitly by '-c' ('--config').

@kenhys
Copy link
Contributor Author

kenhys commented Feb 15, 2024

NOTE: /opt/fluent/bin/fluentd is auto-generated file, so it should be patched.

@daipom daipom self-requested a review February 16, 2024 05:41
@kenhys kenhys force-pushed the guard-duplicated-instance branch 5 times, most recently from 0dcba92 to ab8f2bd Compare February 19, 2024 07:13
@daipom daipom mentioned this pull request Feb 21, 2024
Closed
@kenhys kenhys force-pushed the guard-duplicated-instance branch 2 times, most recently from b1fc5b9 to d341283 Compare February 22, 2024 08:00
@kenhys
Copy link
Contributor Author

kenhys commented Feb 22, 2024

Observed service restarting in short times.
Need to investigating further more.

@kenhys kenhys force-pushed the guard-duplicated-instance branch 2 times, most recently from f860f53 to 3e89a51 Compare February 26, 2024 05:04
@kenhys
Copy link
Contributor Author

kenhys commented Feb 26, 2024

Update comment in this PR.

@kenhys kenhys force-pushed the guard-duplicated-instance branch from 3e89a51 to 2915677 Compare February 26, 2024 05:08
@kenhys
Copy link
Contributor Author

kenhys commented Feb 26, 2024

Unexpectedly, download-artifact fails.

actions/download-artifact@master
Downloading single artifact
Preparing to download the following artifacts:
- packages-rockylinux-8 (ID: 1274111102, Size: 108455247)
Redirecting to blob download url: https://productionresultssa8.blob.core.windows.net/actions-results/b9412b87-4106-46a2-8fb0-86c757be7f85/workflow-job-run-ff7a8386-1a15-5f2b-97b2-c907b43b2f17/artifacts/6d0d09c616a6cee6f67320471aa47616aa04bd135f06b78e56dad6b651114b8f.zip
Starting download of artifact to: /home/runner/work/fluent-package-builder/fluent-package-builder
(node:1729) [DEP0005] DeprecationWarning: Buffer() is deprecated due to security and usability issues. Please use the Buffer.alloc(), Buffer.allocUnsafe(), or Buffer.from() methods instead.
(Use `node --trace-deprecation ...` to show where the warning was created)
Error: Unable to download artifact(s): Unable to download and extract artifact: Artifact download failed after 5 retries.

@kenhys
Copy link
Contributor Author

kenhys commented Feb 26, 2024

It seems that there is no need to stick to download-artifact@master, created PR to use stable download-artifact@v4.
#624

@kenhys
Copy link
Contributor Author

kenhys commented Feb 26, 2024 •
edited
Loading

NOTE:
download warning itself is not related to whether using download-artifact@master.

[bug] Node incompatibility: [DEP0005] DeprecationWarning: Buffer() is deprecated due to security and usability issues
actions/download-artifact#283

@kenhys kenhys force-pushed the guard-duplicated-instance branch 3 times, most recently from f0960bd to 96c47f9 Compare February 28, 2024 03:08
@kenhys
Copy link
Contributor Author

kenhys commented Feb 28, 2024

got it, it may be missing runtime package dependency.

@kenhys kenhys force-pushed the guard-duplicated-instance branch from db39aca to 528ba69 Compare February 28, 2024 05:13
@kenhys
Copy link
Contributor Author

kenhys commented Feb 28, 2024

checking with timeout.

@kenhys kenhys force-pushed the guard-duplicated-instance branch 5 times, most recently from 27fd5fd to 5320ed8 Compare February 28, 2024 08:37
@kenhys
Copy link
Contributor Author

kenhys commented Feb 28, 2024

As v5.0.3 is not released yet, test case in fresh install v5, lts should be skipped.

@kenhys kenhys force-pushed the guard-duplicated-instance branch 11 times, most recently from a716237 to 31e0415 Compare June 28, 2024 04:25
@kenhys kenhys marked this pull request as ready for review June 28, 2024 04:32
@kenhys
Copy link
Contributor Author

kenhys commented Jun 28, 2024

waiting CI to pass...

@kenhys kenhys force-pushed the guard-duplicated-instance branch from 31e0415 to 1231fba Compare June 28, 2024 04:36
@kenhys kenhys mentioned this pull request Jun 28, 2024
Merged
@kenhys kenhys force-pushed the guard-duplicated-instance branch 2 times, most recently from 36cbf6b to feba576 Compare June 28, 2024 05:59
@kenhys
linux: guard duplicated instance (not adding a new option)
881cbdf 
Closes: fluent#611

Before:

We can launch Fluentd by /usr/sbin/fluentd even though fluentd service
is running.

Launching multiple Fluentd with the same config may cause
inconsistency of the buffers or the pos files.

After:

We can't launch Fluentd by /usr/sbin/fluentd with the default config
path if fluentd service is running.  If one of the following options is
specified, we can execute fluentd as before.

* --config (-c)
* --dry-run

Signed-off-by: Kentaro Hayashi <[email protected]>
@kenhys kenhys force-pushed the guard-duplicated-instance branch from feba576 to 881cbdf Compare June 28, 2024 06:10
kenhys and others added 2 commits June 28, 2024 15:33
@kenhys @daipom
Simplify logic to detect fluentd
feebdbe 
Co-authored-by: Daijiro Fukuda <[email protected]>
Signed-off-by: Kentaro Hayashi <[email protected]>
@kenhys @daipom
Cosmetic change
3d96430 
Co-authored-by: Daijiro Fukuda <[email protected]>
Signed-off-by: Kentaro Hayashi <[email protected]>
@kenhys kenhys force-pushed the guard-duplicated-instance branch from d6aaab0 to 3d96430 Compare June 28, 2024 06:34
daipom
daipom approved these changes Jun 28, 2024
View reviewed changes
Copy link
Contributor

@daipom daipom left a comment

Choose a reason for hiding this comment

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

I have commented on one minor point, but this basically looks good to me.
Thanks for this!
I have confirmed this script on my local AlmaLinux 9, and it works well!

fluent-package/templates/usr/sbin/fluentd.erb Outdated Show resolved Hide resolved
@kenhys @daipom
Update fluent-package/templates/usr/sbin/fluentd.erb
aee1135 
Co-authored-by: Daijiro Fukuda <[email protected]>
Signed-off-by: Kentaro Hayashi <[email protected]>
@kenhys kenhys force-pushed the guard-duplicated-instance branch from 070680f to aee1135 Compare June 28, 2024 06:50
@kenhys kenhys merged commit 17fd9cd into fluent:master Jun 28, 2024
65 checks passed
@kenhys kenhys deleted the guard-duplicated-instance branch June 28, 2024 08:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@daipom daipom daipom approved these changes

Assignees
No one assigned
Labels
enhancement New feature or request
Projects
None yet
Milestone
5.0.4
Development

Successfully merging this pull request may close these issues.

Prevent accidental duplicate launching
2 participants
@kenhys @daipom