Bug 913585 - Provide native systemd unit file for ncid services
Summary: Provide native systemd unit file for ncid services
Keywords:
Status: CLOSED WONTFIX
Alias: None
Product: Fedora
Classification: Fedora
Component: ncid
Version: 19
Hardware: Unspecified
OS: Unspecified
unspecified
unspecified
Target Milestone: ---
Assignee: Eric Sandeen
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks: 913251
TreeView+ depends on / blocked
 
Reported: 2013-02-21 14:24 UTC by Jóhann B. Guðmundsson
Modified: 2016-04-15 19:01 UTC (History)
1 user (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2014-05-26 09:58:58 UTC
Type: Bug
Embargoed:


Attachments (Terms of Use)
Native systemd service file for ncid2ncid (244 bytes, text/plain)
2013-02-21 14:25 UTC, Jóhann B. Guðmundsson
no flags Details
Native systemd service file for ncidd (215 bytes, text/plain)
2013-02-21 14:26 UTC, Jóhann B. Guðmundsson
no flags Details
Native systemd service file for ncidsip (222 bytes, text/plain)
2013-02-21 14:26 UTC, Jóhann B. Guðmundsson
no flags Details
Native systemd service file for sip2ncid (232 bytes, application/octet-stream)
2013-02-21 14:27 UTC, Jóhann B. Guðmundsson
no flags Details
Native systemd service file for yac2ncid (144 bytes, text/plain)
2013-02-21 14:27 UTC, Jóhann B. Guðmundsson
no flags Details
Native systemd service file for ncid-initmodem (212 bytes, text/plain)
2013-02-21 16:10 UTC, Jóhann B. Guðmundsson
no flags Details
Native systemd service file for ncid-mythtv.service (207 bytes, text/plain)
2013-02-21 16:21 UTC, Jóhann B. Guðmundsson
no flags Details
Native systemd service file for ncid page (200 bytes, text/plain)
2013-02-21 16:42 UTC, Jóhann B. Guðmundsson
no flags Details
Native systemd service file for ncid-yac (194 bytes, text/plain)
2013-02-21 16:48 UTC, Jóhann B. Guðmundsson
no flags Details
Native systemd service file for ncid-samba (203 bytes, text/plain)
2013-02-21 16:52 UTC, Jóhann B. Guðmundsson
no flags Details
Native systemd service file for ncid-speak (204 bytes, text/plain)
2013-02-21 16:57 UTC, Jóhann B. Guðmundsson
no flags Details

Description Jóhann B. Guðmundsson 2013-02-21 14:24:46 UTC
Description of problem:

https://fedoraproject.org/wiki/Features/SysVtoSystemd

Version-Release number of selected component (if applicable):


How reproducible:


Steps to Reproduce:
1.
2.
3.
  
Actual results:


Expected results:


Additional info:

Comment 1 Jóhann B. Guðmundsson 2013-02-21 14:25:27 UTC
Created attachment 700563 [details]
Native systemd service file for ncid2ncid

Comment 2 Jóhann B. Guðmundsson 2013-02-21 14:26:00 UTC
Created attachment 700564 [details]
Native systemd service file for ncidd

Comment 3 Jóhann B. Guðmundsson 2013-02-21 14:26:33 UTC
Created attachment 700565 [details]
Native systemd service file for ncidsip

Comment 4 Jóhann B. Guðmundsson 2013-02-21 14:27:10 UTC
Created attachment 700569 [details]
Native systemd service file for sip2ncid

Comment 5 Jóhann B. Guðmundsson 2013-02-21 14:27:44 UTC
Created attachment 700577 [details]
Native systemd service file for yac2ncid

Comment 6 Jóhann B. Guðmundsson 2013-02-21 16:10:43 UTC
Created attachment 700638 [details]
Native systemd service file for ncid-initmodem

Comment 7 Jóhann B. Guðmundsson 2013-02-21 16:21:49 UTC
Created attachment 700645 [details]
Native systemd service file for ncid-mythtv.service

Comment 8 Jóhann B. Guðmundsson 2013-02-21 16:42:41 UTC
Created attachment 700663 [details]
Native systemd service file for ncid page

Comment 9 Jóhann B. Guðmundsson 2013-02-21 16:48:09 UTC
Created attachment 700665 [details]
Native systemd service file for ncid-yac

Comment 10 Jóhann B. Guðmundsson 2013-02-21 16:52:37 UTC
Created attachment 700666 [details]
Native systemd service file for ncid-samba

Comment 11 Jóhann B. Guðmundsson 2013-02-21 16:57:02 UTC
Created attachment 700670 [details]
Native systemd service file for ncid-speak

Comment 12 Jóhann B. Guðmundsson 2013-02-21 16:58:31 UTC
Im wondering if we should not create a spesific ncid target for this, given the share amount of units we are dealing with.

How many of those units are installed and enabled on a typical setup?

Comment 13 Eric Sandeen 2013-02-22 15:43:29 UTC
Depends on the user I guess; in most cases most of them are probably *not* used.

I can't test this very well, unfortunately - I have ncid running on a RHEL6 box, but not on Fedora anywhere, TBH.

Comment 14 Fedora End Of Life 2013-04-03 14:51:48 UTC
This bug appears to have been reported against 'rawhide' during the Fedora 19 development cycle.
Changing version to '19'.

(As we did not run this process for some time, it could affect also pre-Fedora 19 development
cycle bugs. We are very sorry. It will help us with cleanup during Fedora 19 End Of Life. Thank you.)

More information and reason for this action is here:
https://fedoraproject.org/wiki/BugZappers/HouseKeeping/Fedora19

Comment 15 Jóhann B. Guðmundsson 2014-05-26 09:58:58 UTC
Given that I have left the project and a new individual may or may not continue with systemd integration in the project by submitting new feature following whatever demands FPC and FESCo might have and thus new units in the process I'm closing this and all remaining bugs I had submitted for this particular feature as WONTFIX

Comment 16 Eric Sandeen 2016-04-13 16:22:28 UTC
It may please you to know that ncid has proper unit files upstream now, and I have built version 1.3 into rawhide which includes them.  :)

Comment 17 Jóhann B. Guðmundsson 2016-04-14 11:17:13 UTC
(In reply to Eric Sandeen from comment #16)
> It may please you to know that ncid has proper unit files upstream now, and
> I have built version 1.3 into rawhide which includes them.  :)

Environment entries that get later used in the same type unit are a no no ( type unit files are not shell script ) 

You dont do this
Environment=prog=ncid2ncid
Then this
ExecStart=/usr/bin/ncid2ncid --pidfile /var/run/${prog}.pid <-- 

Today ( unlike when I submitted the above type units ) the PID path should be /run/$DAEMON.pid not /var/run/$DAEMON.pid ( think automounting + udev )

So a properly written type unit would contain 

Type=forking
PIDFile=/var/run/ncid2ncid.pid
ExecStart=/usr/bin/ncid2ncid --pidfile /run/ncid2ncid.pid

This is so poorly written I find the sudden urge to poor acid in my eyes and crop them out with a fork. 

[Service]
Type=simple
Environment=module=ncid-notify
PIDFile=/var/run/ncid-notify.pid
ExecStart=/usr/bin/ncid --no-gui -p /var/run/${module}.pid -P $module
ExecStop=/bin/kill $MAINPID ; /bin/rm -f /var/run/${module}.pid

So if by proper you mean they magically somehow work ( for now at least ) but from "how to properly write type unit files" these type units are not the most graceful ones out there...

Comment 18 Jóhann B. Guðmundsson 2016-04-14 11:19:02 UTC
(In reply to Jóhann B. Guðmundsson from comment #17)
Forgot to fix the PIDFILE path as well there.

Type=forking
PIDFile=/run/ncid2ncid.pid
ExecStart=/usr/bin/ncid2ncid --pidfile /run/ncid2ncid.pid

Comment 19 Eric Sandeen 2016-04-14 16:18:14 UTC
*shrug* I'm no systemd wizard by any means, which is why this bug went unfixed for so long.  IMHO that's always been the problem w/ systemd, is a lot of non-obvious and unexpected behavior & rules.

If you'd like to help me get them fixed up, I'd certainly appreciate it.

Comment 20 Eric Sandeen 2016-04-14 16:21:02 UTC
Sorry, forgot you had attached unit files.  So are they acceptable, just needing a pidfile path change?

Comment 21 Eric Sandeen 2016-04-14 21:40:07 UTC
Ok, I think I got them fixed up per your suggestions, thanks.

Comment 22 Jóhann B. Guðmundsson 2016-04-15 14:18:01 UTC
(In reply to Eric Sandeen from comment #21)
> Ok, I think I got them fixed up per your suggestions, thanks.

Afaikt they still need fixing.

You dont need to specific "Type=simple" it's the default so for type unit simplicity ( which resides in fewer lines, a well written, well behaving daemon/service is made up of ca <12 lines total. anything more than that the daemon usually needs to be patched/fixed to behave correctly ) they are usually skipped but it's a matter of personal preference, some people prefer always declaring the Type ( for clarification purposes ). 

PIDFile= are not used in conjunction with Type=simple since it is expected that the process configured with ExecStart= is the main process of the service.

Dont add any "ExecStop=/bin/kill" lines unless they are absolutely necessary systemd tries to do the right thing here, on it's own. First it tries to ask the main process to stop ( sigterm ) and then smacks it with a hammer if does not obey ( sigkill ) :) . 

Now this seems to be some weird hack that you need to check with upstream wtf they are doing but anyway don't do this since it gives out the false notion that type units are shell scripts ( ignore what you see on the internet since the internet is good at spreading ignorance you never ever escape to bash in type units ) .

"; /bin/rm -f /run/$daemon.pid" 

But do this instead ( if this is necessary o_O )
ExecStopPost=/bin/rm -f /run/$daemon.pid

Comment 23 Eric Sandeen 2016-04-15 18:15:52 UTC
Ok, I can look into that.  Thing is, I can't test all of these daemons, because I don't have the hardware.

You know what would be super cool is a utility that straces a daemon start, fires signals at it, figure out how it forks, etc, and suggests a proper unit file template...

Comment 24 Jóhann B. Guðmundsson 2016-04-15 19:01:05 UTC
(In reply to Eric Sandeen from comment #23)
> Ok, I can look into that.  Thing is, I can't test all of these daemons,
> because I don't have the hardware.

Then let user with that hardware test them for you ( which could have been done in 2013 if this had been implemented when I submitted those units ), worst case scenario it's just a (couple) of line patch anyway should there be any problem ;) 

You probably will receive RFE implementing restart behaviour once this hits the wild to any extent, which I would have taken care of in the cleanup proposal ( amongst other thins like deprecation of EnvironmentFiles etc. ) in the systemd integration for all daemons and services but things went as they went. 

> You know what would be super cool is a utility that straces a daemon start,
> fires signals at it, figure out how it forks, etc, and suggests a proper
> unit file template...

So would be a nuclear free world without religion,borders and financial systems which ran entirely on green energy but here we are hence we have to make do with what we have...


Note You need to log in before you can comment on or make changes to this bug.