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.

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

apt yum: use fluentd.service by default #461

Merged
merged 8 commits into from
May 16, 2023

Conversation

kenhys
Copy link
Contributor

@kenhys kenhys commented May 11, 2023

Basically, fluentd.service should be used by default, but with considering
migration from old td-agent, added alias (Alias=td-agent.service)
in fluentd.service.

To distinguish service name and compatible service name,
it use SERVICE_NAME (fluentd) and COMPAT_SERVICE_NAME (td-agent) respectively.

When upgrading from v4 package, td-agent service will be removed by
default, but instead of it, fluentd.service launches.

This limitation is caused by execution order of old package's
maintainer scriptlet behavior (%postun).

If you want to use td-agent as well, you must re-enable service via
systemctl explicitly.

deb:
sudo systemctl unmask td-agent
sudo systemctl enable fluentd
(td-agent.service is masked, so need to unmask first)

rpm:
sudo systemctl enable fluentd
(td-agent.service is inactive, so need to enable first)

NOTE: service environment file is moved to /etc/default/fluentd or
/etc/sysconfig/fluentd for consistency.

@kenhys kenhys force-pushed the use-fluentd-service branch 3 times, most recently from 6de8435 to 882e893 Compare May 11, 2023 07:48
@kenhys
Copy link
Contributor Author

kenhys commented May 11, 2023

Chekcking diff outcome.

diff deb
--- fluent5deb.orig     2023-05-11 16:51:26.598363453 +0900
+++ fluent5deb.rev      2023-05-11 16:51:26.878363842 +0900
@@ -7,7 +7,7 @@
 ./lib/
 ./lib/systemd/
 ./lib/systemd/system/
-./lib/systemd/system/td-agent.service
+./lib/systemd/system/fluentd.service
 ./opt/
 ./opt/td-agent/
 ./opt/td-agent/LICENSES/
diff rpm
--- fluent5rpm.orig     2023-05-11 16:51:26.930363913 +0900
+++ fluent5rpm.rev      2023-05-11 16:51:26.978363979 +0900
@@ -9362,7 +9362,7 @@
 /opt/td-agent/share/td-agent.conf.tmpl
 /tmp/td-agent
 /usr/bin/td
-/usr/lib/systemd/system/td-agent.service
+/usr/lib/systemd/system/fluentd.service
 /usr/lib/tmpfiles.d/td-agent.conf
 /usr/sbin/td-agent
 /usr/sbin/td-agent-gem

@kenhys kenhys marked this pull request as ready for review May 11, 2023 08:02
@ashie
Copy link
Member

ashie commented May 11, 2023

If you use RPM, Alias=td-agent.service is set to
fluentd.service, but it is effective for fresh install only.
Upgraded from old package, you must systemctl enable fluentd
explicitly.(as old package's %postun removed td-agent.service)

Oops, it's not desired. Should we swap entity & alias, or add something other to resolve it?

@kenhys kenhys force-pushed the use-fluentd-service branch 4 times, most recently from 75984de to 265c2ef Compare May 12, 2023 05:39
@kenhys
Copy link
Contributor Author

kenhys commented May 12, 2023

 ./pkgdiff.sh fluent
diff deb
--- fluent5deb.orig     2023-05-12 14:39:17.331699815 +0900
+++ fluent5deb.rev      2023-05-12 14:39:17.627699196 +0900
@@ -7,7 +7,7 @@
 ./lib/
 ./lib/systemd/
 ./lib/systemd/system/
-./lib/systemd/system/td-agent.service
+./lib/systemd/system/fluentd.service
 ./opt/
 ./opt/td-agent/
 ./opt/td-agent/LICENSES/
diff rpm
--- fluent5rpm.orig     2023-05-12 14:39:17.675699095 +0900
+++ fluent5rpm.rev      2023-05-12 14:39:17.723698993 +0900
@@ -1,5 +1,5 @@
 /etc/logrotate.d/td-agent
-/etc/sysconfig/td-agent
+/etc/sysconfig/fluentd
 /etc/td-agent
 /etc/td-agent/plugin
 /etc/td-agent/td-agent.conf
@@ -9362,7 +9362,7 @@
 /opt/td-agent/share/td-agent.conf.tmpl
 /tmp/td-agent
 /usr/bin/td
-/usr/lib/systemd/system/td-agent.service
+/usr/lib/systemd/system/fluentd.service
 /usr/lib/tmpfiles.d/td-agent.conf
 /usr/sbin/td-agent
 /usr/sbin/td-agent-gem

@kenhys
Copy link
Contributor Author

kenhys commented May 12, 2023

@kenhys kenhys force-pushed the use-fluentd-service branch from 265c2ef to 4b0c94a Compare May 12, 2023 07:29
@kenhys
Copy link
Contributor Author

kenhys commented May 12, 2023

Fixed to use modern macro.

@kenhys
Copy link
Contributor Author

kenhys commented May 12, 2023

waiting CI results.

Copy link
Member

@ashie ashie left a comment

Choose a reason for hiding this comment

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

This limitation is caused by execution order of old package's
%postun maintainer script behavior. (no workaround)

There is a hacky work around to resolve it.
%posttrans of the new package will be run after %postun of the old one.
So that running systemctl enable fluentd at %posttrans will resolve it.

But I think we don't have to go that far to enable it.
We don't need to add such hacky way in this PR.
If it's really needed, we'll do it in another PR.

@ashie
Copy link
Member

ashie commented May 12, 2023

I'll merge this after I confirm this on my environment.

Copy link
Member

@ashie ashie left a comment

Choose a reason for hiding this comment

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

Hmm, I can't migrate it properly on Ubuntu.
We might need more work.

@kenhys
Copy link
Contributor Author

kenhys commented May 15, 2023

We might need more work.

Sure, I'll fix it.

@kenhys
Copy link
Contributor Author

kenhys commented May 15, 2023

On ubuntu side, dh_installsystemd assumes service_name instead of compat_service_name.

@ashie
Copy link
Member

ashie commented May 15, 2023

Thanks, I've tested it again on Ubuntu.
Now fluentd service is successfully launched after upgrade!
Although td-agent service is still masked, it's acceptable for me.
On the contrary, it has the advantage of letting users know that the service name has changed.
(Of course we should describe in the document how to upgrade.)

@kenhys
Copy link
Contributor Author

kenhys commented May 15, 2023

checking whether unexpected breaking change exists.

@ashie
Copy link
Member

ashie commented May 15, 2023

me too

@kenhys kenhys force-pushed the use-fluentd-service branch from b2e0353 to f42fd56 Compare May 15, 2023 09:30
kenhys added 4 commits May 16, 2023 10:35
Basically, fluentd.service should be used by default, but with considering
migration from old td-agent, added alias (Alias=td-agent.service)
in fluentd.service.

To distinguish service name and compatible service name,
it use SERVICE_NAME (fluentd) and COMPAT_SERVICE_NAME (td-agent) respectively.

When upgrading from v4 package, td-agent service will be removed by
default, but instead of it, fluentd.service launches.

This limitation is caused by execution order of old package's
 maintainer scriptlet behavior (%postun).

If you want to use td-agent as well, you must re-enable service via
systemctl explicitly.

deb:
  sudo systemctl unmask td-agent
  sudo systemctl enable fluentd
(td-agent.service is masked, so need to unmask first)

rpm:
  sudo systemctl enable fluentd
(td-agent.service is inactive, so need to enable first)

NOTE: service environment file is moved to /etc/default/fluentd or
/etc/sysconfig/fluentd for consistency.

Signed-off-by: Kentaro Hayashi <[email protected]>
bullseye or later can use debhelper compatibility 12 or later.
so at the minimum requirement, use level 12.

Newer debhelper compatibility level enables more strict
checking, so newer is better as much as possible.

Signed-off-by: Kentaro Hayashi <[email protected]>
kenhys added 4 commits May 16, 2023 10:38
Signed-off-by: Kentaro Hayashi <[email protected]>
Signed-off-by: Kentaro Hayashi <[email protected]>
When upgrading from v4 is treated like a install
process ($1 -eq 1).
If td-agent service is running before upgrading process,
explicitly restart fluentd.service.

Signed-off-by: Kentaro Hayashi <[email protected]>
@kenhys kenhys force-pushed the use-fluentd-service branch from f42fd56 to f2d3bec Compare May 16, 2023 01:57
@kenhys
Copy link
Contributor Author

kenhys commented May 16, 2023

Updated commit message. f531fa5

Copy link
Member

@ashie ashie left a comment

Choose a reason for hiding this comment

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

LGTM

Waiting for CI...

@ashie ashie merged commit 5cd2c8b into fluent:master May 16, 2023
@ashie
Copy link
Member

ashie commented May 16, 2023

Thanks a lot!

@kenhys kenhys deleted the use-fluentd-service branch May 16, 2023 03:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants