Bug#796588: Bug796588#: adjtimex: Has init script in runlevel S but no matching service file

[Roger Shimizu]

On Mon, Dec 14, 2015 at 11:57 PM, Felipe Sateler  wrote:
> Another option is to simply not enable/start the service, and just
> ship instructions on how to do that in README.Debian.
>
> Running it on postinst looks weird to me. We normally do not run
> possibly long processes synchronously on package installs.

I agree to removing adjtimexconfig from postinst, which takes 70
seconds during install, is a good idea.
But the defconf selection on enable/disable service is another issue.
Yes, without adjtimexconfig running on install implies the service is
disabled by default. But user need an easy way to enable adjtimex
service after adjtimexconfig running.
Do you think it's okay to run debconf to let user decide whether to
start the service after manually running adjtimexconf?

Thank you!

Cheers,
Roger

Bug#796588: Bug796588#: adjtimex: Has init script in runlevel S but no matching service file

[Michael Biebl]

Am 14.12.2015 um 16:19 schrieb Roger Shimizu:
> On Mon, Dec 14, 2015 at 11:57 PM, Felipe Sateler  wrote:
>> Another option is to simply not enable/start the service, and just
>> ship instructions on how to do that in README.Debian.
>>
>> Running it on postinst looks weird to me. We normally do not run
>> possibly long processes synchronously on package installs.
> 
> I agree to removing adjtimexconfig from postinst, which takes 70
> seconds during install, is a good idea.
> But the defconf selection on enable/disable service is another issue.
> Yes, without adjtimexconfig running on install implies the service is
> disabled by default. But user need an easy way to enable adjtimex
> service after adjtimexconfig running.
> Do you think it's okay to run debconf to let user decide whether to
> start the service after manually running adjtimexconf?

You could check for the existence of a config file in your sysv init
script and simply exit if it doesn't exist.
Under systemd the equivalent is a ConditionPathExists=/etc/foobar.conf

Does adjtimexconfig already write a config file? If not, maybe you can
consider making it write a flag file somewhere.

The drop all the debconf machinery and simply add a README.Debian
telling users that want to have adjtimex run during boot, need to
configure the tool once via adjtimexconfig.



-- 
Why is it that all of the instruments seeking intelligent life in the
universe are pointed away from Earth?



signature.asc
Description: OpenPGP digital signature

Bug#796588: Bug796588#: adjtimex: Has init script in runlevel S but no matching service file

[Roger Shimizu]

Dear systemd maintainers,

Thanks for helping for the adjtimex systemd service file last time.
I write again because I find an issue related to the service file.

When installing, adjtimex's postinst script will ask user whether to
start the service by debconf, for the service file currently (enclosed
here), it doesn't respect the debconf if user choose "No".

I find a temporary solution to patch postinst to run "systemctl
disable adjtimex.service" if user choose not to start the service when
in "configure" mode.
I call it "temporary" because it will miss some rare case such as:
- installing adjtimex when in sysvinit and choose not to start the
service, then installing systemd to replace sysvinit, it but this
won't trigger adjtimex's postinst script to disable adjtimex's
service.

I'm wondering whether do you already have a patch to get debconf's
result in service file. I think it's the better solution than the
temporary one above.
Thanks and look forward to your reply!

Cheers,
Roger


adjtimex.service
Description: Binary data

Bug#796588: Bug796588#: adjtimex: Has init script in runlevel S but no matching service file

[Roger Shimizu]

On Mon, Dec 14, 2015 at 11:31 PM, Felipe Sateler  wrote:
> On 14 December 2015 at 11:20, Roger Shimizu  wrote:
>> Dear systemd maintainers,
>>
>> Thanks for helping for the adjtimex systemd service file last time.
>> I write again because I find an issue related to the service file.
>>
>> When installing, adjtimex's postinst script will ask user whether to
>> start the service by debconf, for the service file currently (enclosed
>> here), it doesn't respect the debconf if user choose "No".
>>
>> I find a temporary solution to patch postinst to run "systemctl
>> disable adjtimex.service" if user choose not to start the service when
>> in "configure" mode.
>> I call it "temporary" because it will miss some rare case such as:
>> - installing adjtimex when in sysvinit and choose not to start the
>> service, then installing systemd to replace sysvinit, it but this
>> won't trigger adjtimex's postinst script to disable adjtimex's
>> service.
>
> I don't follow. What exactly is the current behavior, and how does the
> new systemd service file does not respect it? Please add more context.
>
> I'd even question the need to ask about enabling the service. If I
> don't want the service, why did I install the package? Admins can
> manually disable later if they don't want to run it.

Adjtimex service is for setting up time ticks/frequency in kernel, but
some user don't want to change ticks/frequency, but only want to print
those kernel variables, maybe changed by NTP client.
Please see bug #785208: https://bugs.debian.org/785208

I think this requirement is rare, but reasonable.
If reading debconf's result is wrong, I think another solution is to
split adjtimex into adjtimex-base and adjtimex-service.
But looks overhead because it's a tiny package already.

Cheers,
Roger

Bug#796588: Bug796588#: adjtimex: Has init script in runlevel S but no matching service file

[Felipe Sateler]

On 14 December 2015 at 11:20, Roger Shimizu  wrote:
> Dear systemd maintainers,
>
> Thanks for helping for the adjtimex systemd service file last time.
> I write again because I find an issue related to the service file.
>
> When installing, adjtimex's postinst script will ask user whether to
> start the service by debconf, for the service file currently (enclosed
> here), it doesn't respect the debconf if user choose "No".
>
> I find a temporary solution to patch postinst to run "systemctl
> disable adjtimex.service" if user choose not to start the service when
> in "configure" mode.
> I call it "temporary" because it will miss some rare case such as:
> - installing adjtimex when in sysvinit and choose not to start the
> service, then installing systemd to replace sysvinit, it but this
> won't trigger adjtimex's postinst script to disable adjtimex's
> service.

I don't follow. What exactly is the current behavior, and how does the
new systemd service file does not respect it? Please add more context.

I'd even question the need to ask about enabling the service. If I
don't want the service, why did I install the package? Admins can
manually disable later if they don't want to run it.

>
> I'm wondering whether do you already have a patch to get debconf's
> result in service file. I think it's the better solution than the
> temporary one above.

Don't do this. debconf is not a registry. The results of the debconf
question must be stored in the proper configuration (eg, enablement
state) and then read back to ask the user on the next install run.



-- 

Saludos,
Felipe Sateler

Bug#796588: Bug796588#: adjtimex: Has init script in runlevel S but no matching service file

[Michael Biebl]

Am 14.12.2015 um 15:20 schrieb Roger Shimizu:
> Dear systemd maintainers,
> 
> Thanks for helping for the adjtimex systemd service file last time.
> I write again because I find an issue related to the service file.
> 
> When installing, adjtimex's postinst script will ask user whether to
> start the service by debconf, for the service file currently (enclosed
> here), it doesn't respect the debconf if user choose "No".
> 
> I find a temporary solution to patch postinst to run "systemctl
> disable adjtimex.service" if user choose not to start the service when
> in "configure" mode.
> I call it "temporary" because it will miss some rare case such as:
> - installing adjtimex when in sysvinit and choose not to start the
> service, then installing systemd to replace sysvinit, it but this
> won't trigger adjtimex's postinst script to disable adjtimex's
> service.
> 
> I'm wondering whether do you already have a patch to get debconf's
> result in service file. I think it's the better solution than the
> temporary one above.
> Thanks and look forward to your reply!

My answer is: don't do that. Prompting the user via debconf whether to
start a service is an anti-feature.


-- 
Why is it that all of the instruments seeking intelligent life in the
universe are pointed away from Earth?



signature.asc
Description: OpenPGP digital signature

Bug#796588: Bug796588#: adjtimex: Has init script in runlevel S but no matching service file

[Felipe Sateler]

On 14 December 2015 at 11:45, Roger Shimizu  wrote:
> On Mon, Dec 14, 2015 at 11:31 PM, Felipe Sateler  wrote:
>> On 14 December 2015 at 11:20, Roger Shimizu  wrote:
>>> Dear systemd maintainers,
>>>
>>> Thanks for helping for the adjtimex systemd service file last time.
>>> I write again because I find an issue related to the service file.
>>>
>>> When installing, adjtimex's postinst script will ask user whether to
>>> start the service by debconf, for the service file currently (enclosed
>>> here), it doesn't respect the debconf if user choose "No".
>>>
>>> I find a temporary solution to patch postinst to run "systemctl
>>> disable adjtimex.service" if user choose not to start the service when
>>> in "configure" mode.
>>> I call it "temporary" because it will miss some rare case such as:
>>> - installing adjtimex when in sysvinit and choose not to start the
>>> service, then installing systemd to replace sysvinit, it but this
>>> won't trigger adjtimex's postinst script to disable adjtimex's
>>> service.
>>
>> I don't follow. What exactly is the current behavior, and how does the
>> new systemd service file does not respect it? Please add more context.
>>
>> I'd even question the need to ask about enabling the service. If I
>> don't want the service, why did I install the package? Admins can
>> manually disable later if they don't want to run it.
>
> Adjtimex service is for setting up time ticks/frequency in kernel, but
> some user don't want to change ticks/frequency, but only want to print
> those kernel variables, maybe changed by NTP client.
> Please see bug #785208: https://bugs.debian.org/785208
>
> I think this requirement is rare, but reasonable.
> If reading debconf's result is wrong, I think another solution is to
> split adjtimex into adjtimex-base and adjtimex-service.
> But looks overhead because it's a tiny package already.

Another option is to simply not enable/start the service, and just
ship instructions on how to do that in README.Debian.

Running it on postinst looks weird to me. We normally do not run
possibly long processes synchronously on package installs.

-- 

Saludos,
Felipe Sateler

Bug#796588: Bug796588#: adjtimex: Has init script in runlevel S but no matching service file

[Roger Shimizu]

Dear Felipe,

Thanks for your feedback!

> Note that this is not the only place that this description appears. It
> also shows in the output of "systemctl status adjtimex.service", or
> "systemctl list-units". This should really be a full descriptive
> sentence. Maybe, "Adjust kernel time variables".

I think you might misunderstand me.
The description you proposed would generate the following log:
[DATE TIME] [HOSTNAME] systemd[1]: Starting set the kernel time variables...
[DATE TIME] [HOSTNAME] systemd[1]: Started set the kernel time variables.
or
[DATE TIME] [HOSTNAME] systemd[1]: Starting Adjust kernel time variables...
[DATE TIME] [HOSTNAME] systemd[1]: Started Adjust kernel time variables.

What I proposed was:
[DATE TIME] [HOSTNAME] systemd[1]: Starting the kernel time
variables setting...
[DATE TIME] [HOSTNAME] systemd[1]: Started the kernel time
variables setting.

IMHO, it looks more natural.

Yes, I found other existing systemd service also use the way you
propose, like "systemd-tmpfiles-setup.service":

Nov 30 23:43:32 sid systemd[1]: Starting Create Volatile Files and
Directories...
Nov 30 23:43:32 sid systemd[1]: Started Create Volatile Files and
Directories.

I consider it's minor bug to say "starting verb" or "started verb",
which is better to fix in the future.

>> Environment="TICK=1 FREQ=0"
> I don't think this works. There should be no quotes there, or systemd
> might treat TICK variable as containing "1 FREQ=0".

Thanks for pointing this out.
Yes, it's totally not working.
I confirmed by commenting out "EnvironmentFile" line, and found
"ExecStart" command failed because of no FREQ is set.

changing from
Environment="TICK=1 FREQ=0"
to
Environment="TICK=10005" "FREQ=0"
will fix it.

>> EnvironmentFile=-/etc/default/adjtimex
>> ExecStart=/sbin/adjtimex -tick "$TICK" -frequency "$FREQ"
>
> I think the quotes are superfluous. If you want to preserve quoting,
> systemd does it by using ${TICK} and ${FREQ} syntax.

It seems fine as it was. However, following spec is always a good
thing, so I changed to what you proposed:
ExecStart=/sbin/adjtimex -tick ${TICK} -frequency ${FREQ}

Thanks again for your review!

Cheers,
Roger


adjtimex.service
Description: Binary data

Bug#796588: Bug796588#: adjtimex: Has init script in runlevel S but no matching service file

[Roger Shimizu]

Dear Felipe and Andreas,

Your detailed feedback is really helpful.
I really appreciate your kindness help!

Enclosed my updated adjtimex.service file.
Here's the explanation why I changed a bit.

>> [Unit]
>> Description=adjtimex service in early boot
> I think the description of the init script is better: "set the kernel
> time variables".

I changed to "Description=the kernel time variables setting"
Because it's more proper in the log with context:

Nov 29 22:36:01 sid systemd[1]: Starting the kernel time variables setting...
Nov 29 22:36:01 sid systemd[1]: Started the kernel time variables setting.

>> #RemainAfterExit=yes
> I have seen that Type=oneshot services that do not have
> RemainAfterExit=yes can be executed multiple times during boot. This
> may or may not be a problem here.
> The RemainAfterExit directive controls wether systemd considers the
> unit "active" after all the running processes in the unit exited. This
> can trigger multiple runs during boot, if at some point this unit is
> wanted, but it already ran and exited. My sense is that
> RemainAfterExit should be =yes for most Type=oneshot service.
> Unfortunately, that means that to manually re-run the unit, one needs
> to do a "restart" instead of a "start" to make systemd run the program
> again.

Considering adjtimex is simply set time ticking related kernel
variables on boot, it won't look after those kernel variables, which
may be changed by other processes, such as NTP client, I think it's
better to indicate the user that adjtimex service is NOT "active"
after exiting. So I removed the line completely.

If you're comfortable with this version of adjtimex.service, together
with another minor fix, I'll build a package to upload to mentors, and
ask for sponsor in mentors list.

Thanks again!

Cheers,
Roger


adjtimex.service
Description: Binary data

Bug#796588: Bug796588#: adjtimex: Has init script in runlevel S but no matching service file

[Felipe Sateler]

On 29 November 2015 at 11:04, Roger Shimizu  wrote:
> Dear Felipe and Andreas,
>
> Your detailed feedback is really helpful.
> I really appreciate your kindness help!
>
> Enclosed my updated adjtimex.service file.
> Here's the explanation why I changed a bit.
>
>>> [Unit]
>>> Description=adjtimex service in early boot
>> I think the description of the init script is better: "set the kernel
>> time variables".
>
> I changed to "Description=the kernel time variables setting"
> Because it's more proper in the log with context:
>
> Nov 29 22:36:01 sid systemd[1]: Starting the kernel time variables setting...
> Nov 29 22:36:01 sid systemd[1]: Started the kernel time variables setting.

Note that this is not the only place that this description appears. It
also shows in the output of "systemctl status adjtimex.service", or
"systemctl list-units". This should really be a full descriptive
sentence. Maybe, "Adjust kernel time variables".


>
>>> #RemainAfterExit=yes
>> I have seen that Type=oneshot services that do not have
>> RemainAfterExit=yes can be executed multiple times during boot. This
>> may or may not be a problem here.
>> The RemainAfterExit directive controls wether systemd considers the
>> unit "active" after all the running processes in the unit exited. This
>> can trigger multiple runs during boot, if at some point this unit is
>> wanted, but it already ran and exited. My sense is that
>> RemainAfterExit should be =yes for most Type=oneshot service.
>> Unfortunately, that means that to manually re-run the unit, one needs
>> to do a "restart" instead of a "start" to make systemd run the program
>> again.
>
> Considering adjtimex is simply set time ticking related kernel
> variables on boot, it won't look after those kernel variables, which
> may be changed by other processes, such as NTP client, I think it's
> better to indicate the user that adjtimex service is NOT "active"
> after exiting. So I removed the line completely.
>
> If you're comfortable with this version of adjtimex.service, together
> with another minor fix, I'll build a package to upload to mentors, and
> ask for sponsor in mentors list.

> Environment="TICK=1 FREQ=0"

I don't think this works. There should be no quotes there, or systemd
might treat TICK variable as containing "1 FREQ=0".

> EnvironmentFile=-/etc/default/adjtimex
> ExecStart=/sbin/adjtimex -tick "$TICK" -frequency "$FREQ"

I think the quotes are superfluous. If you want to preserve quoting,
systemd does it by using ${TICK} and ${FREQ} syntax.

There is more information on using variables here:
http://www.freedesktop.org/software/systemd/man/systemd.service.html#Command%20lines

-- 

Saludos,
Felipe Sateler

Bug#796588: Bug796588#: adjtimex: Has init script in runlevel S but no matching service file

[Roger Shimizu]

Dear systemd maintainers,

I'm in the process of ITA adjtimex package, which contains a bug you
reported that need to support rcS service. I'm trying to fix this
before send out my 1st release to close ITA.

> Package: adjtimex
> Severity: important
> User: pkg-systemd-maintain...@lists.alioth.debian.org
> Usertags: init-rcs-service
> Your package adjtimex has an initscript that is enabled in runlevel
> S, but it does not provide a corresponding systemd service unit.

As you may already know, adjtimex is a tool to set up kernel time
variables in boot time. Correct time and time ticking is important to
many services, including time-sync service, so in SysV world adjtimex
is run in rcS level, which is very early.

Enclosed the "adjtimex.service" file I wrote and confirmed working
well on my box.
Since this is the first time I write service file, it would be helpful
if you can help to review it.

I also have one doubt whether to have "RemainAfterExit=yes", which is
commented out now.
After setting the kernel time variables, adjtimex simply exits and
don't need to remain as daemon. I guess it should be okay to be "no".

Looking forward to your reply. Thank you!

Cheers,
Roger


adjtimex.service
Description: Binary data

Bug#796588: Bug796588#: adjtimex: Has init script in runlevel S but no matching service file

[Andreas Henriksson]

Hello Roger Shimizu.

On Sat, Nov 28, 2015 at 11:12:38PM +0900, Roger Shimizu wrote:
> Dear systemd maintainers,
[...]

I'm just a random bystander, but hope I can come up with a few
useful suggestion.

> Enclosed the "adjtimex.service" file I wrote and confirmed working
> well on my box.
> Since this is the first time I write service file, it would be helpful
> if you can help to review it.

Thanks for your interest in participating in resolving the rcS situation.

> 
> I also have one doubt whether to have "RemainAfterExit=yes", which is
> commented out now.
> After setting the kernel time variables, adjtimex simply exits and
> don't need to remain as daemon. I guess it should be okay to be "no".

I'd suggest you remove the line entirely.  (The default is
RemainAfterExit=no.)
You have no "stop" method, thus there's no point in using this
directive to mark the service as still running so that it can
later be "stopped".

For more information:
http://www.freedesktop.org/software/systemd/man/systemd.service.html#RemainAfterExit=


> 
> Looking forward to your reply. Thank you!

I also suggest you look into the possibility to not running
the init script from the service file.

Should be possible by making the following changes:

Remove:
ConditionFileIsExecutable=/etc/init.d/adjtimex

Add:
Environment="TICK=1 FREQ=0"
EnvironmentFile=-/etc/default/adjtimex

See:
http://www.freedesktop.org/software/systemd/man/systemd.exec.html#Environment=
EnvironmentFile= should "win" over default settings in Environment=,
while the dash (-) prefix says it's ok if the file does not exist.

Replace:
ExecStart=/etc/init.d/adjtimex start

with:
ExecStart=/sbin/adjtimex -tick "$TICK" -frequency "$FREQ"


In case these changes sounds useful to you, then please confirm the
above to make sure my understanding of the situation is correct.

Regards,
Andreas Henriksson

Bug#796588: Bug796588#: adjtimex: Has init script in runlevel S but no matching service file

[Felipe Sateler]

Hi Roger,

On 28 November 2015 at 11:12, Roger Shimizu  wrote:
> Dear systemd maintainers,
>
> I'm in the process of ITA adjtimex package, which contains a bug you
> reported that need to support rcS service. I'm trying to fix this
> before send out my 1st release to close ITA.

Excellent!

>
>> Package: adjtimex
>> Severity: important
>> User: pkg-systemd-maintain...@lists.alioth.debian.org
>> Usertags: init-rcs-service
>> Your package adjtimex has an initscript that is enabled in runlevel
>> S, but it does not provide a corresponding systemd service unit.
>
> As you may already know, adjtimex is a tool to set up kernel time
> variables in boot time. Correct time and time ticking is important to
> many services, including time-sync service, so in SysV world adjtimex
> is run in rcS level, which is very early.
>
> Enclosed the "adjtimex.service" file I wrote and confirmed working
> well on my box.
> Since this is the first time I write service file, it would be helpful
> if you can help to review it.

I have some comments:

> [Unit]
> Description=adjtimex service in early boot

I think the description of the init script is better: "set the kernel
time variables".

> ConditionFileIsExecutable=/etc/init.d/adjtimex

I thin this is superflous.

> DefaultDependencies=no
> After=local-fs.target
> Before=time-sync.target sysinit.target shutdown.target
> Conflicts=shutdown.target

I'm not sure about time-sync.target. `man systemd.special` says that
time-sync.target is for synchronizing against a remote source, which
AFAICT adjtimex doesn't do. Note that if what you wanted to do is to
run before other time-synchronization daemons, this will not do it.

sysinit.target should be ok, though.
I don't think the conflict/before with shutdown.target is relevant, as
this unit does nothing on stop, and thus does not need to do anything
on shutdown.

>
> [Service]
> Type=oneshot
> ExecStart=/etc/init.d/adjtimex start

I think you should call the program directly and bypass the init
script. You can do as follows:

# default values
Environment=TICK=1
Environment=FREQ=0
# override as in the init script
EnvironmentFile=-/etc/default/adjtimex
ExecStart=/sbin/adjtimex -tick $TICK -frequency $FREQ

> TimeoutSec=0

This is almost certainly wrong. I think you should skip this entry and
leave it at the default. At some point the system needs to continue
booting, even if the time is wrong, no?

> StandardOutput=tty

Do not set this, as this overrides the administrator default in
/etc/systemd/system.conf. Let standard output go to the system-wide
default.

> #RemainAfterExit=yes

I have seen that Type=oneshot services that do not have
RemainAfterExit=yes can be executed multiple times during boot. This
may or may not be a problem here.

>
> [Install]
> WantedBy=sysinit.target

>
> I also have one doubt whether to have "RemainAfterExit=yes", which is
> commented out now.
> After setting the kernel time variables, adjtimex simply exits and
> don't need to remain as daemon. I guess it should be okay to be "no".

The RemainAfterExit directive controls wether systemd considers the
unit "active" after all the running processes in the unit exited. This
can trigger multiple runs during boot, if at some point this unit is
wanted, but it already ran and exited. My sense is that
RemainAfterExit should be =yes for most Type=oneshot service.
Unfortunately, that means that to manually re-run the unit, one needs
to do a "restart" instead of a "start" to make systemd run the program
again.

>
> Looking forward to your reply. Thank you!

Thank you for your contribution!


-- 

Saludos,
Felipe Sateler