Gentoo Websites Logo
Go to: Gentoo Home Documentation Forums Lists Bugs Planet Store Wiki Get Gentoo!
Bug 482690 - net-misc/isatapd - add systemd support
Summary: net-misc/isatapd - add systemd support
Status: RESOLVED FIXED
Alias: None
Product: Gentoo Linux
Classification: Unclassified
Component: Current packages (show other bugs)
Hardware: All Linux
: Normal normal (vote)
Assignee: Michael Weber (RETIRED)
URL: https://github.com/shlusiak/isatapd/i...
Whiteboard:
Keywords:
Depends on:
Blocks: install-systemd-unit
  Show dependency tree
 
Reported: 2013-08-27 16:25 UTC by Yichao Zhou
Modified: 2014-03-30 10:09 UTC (History)
2 users (show)

See Also:
Package list:
Runtime testing required: ---


Attachments
/etc/systemd/system/isatapd.service.d/00gentoo (00gentoo.conf,1.00 KB, text/plain)
2014-02-22 12:01 UTC, Yichao Zhou
Details
/usr/lib64/systemd/system/isatapd.service (isatapd.service,336 bytes, text/plain)
2014-02-22 12:02 UTC, Yichao Zhou
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Yichao Zhou 2013-08-27 16:25:10 UTC
Add systemd support to net-misc/isatapd.

I'm now using a self-write script:
[Unit]
Description=ISATAP Client for Linux
After=network.target nss-lookup.target

[Service]
EnvironmentFile=/etc/conf.d/isatapd
ExecStart=/usr/sbin/isatapd ${DAEMON_OPTS} ${ISATAP_ROUTERS}

[Install]
WantedBy=multi-user.target


It works, but obviously lacks some configuration.
Comment 1 Yichao Zhou 2013-08-27 16:26:13 UTC
upstream bug report:
https://github.com/shlusiak/isatapd/issues/2
Comment 2 Pacho Ramos gentoo-dev 2013-08-27 17:16:49 UTC
regarding EnvironmentFile, it's preferred to use another file inside /etc/systemd instead of conf.d as explained at:
https://wiki.gentoo.org/wiki/Systemd/Ebuild_policy

If you can update patch to apply to that, would be nice :D
Comment 3 Yichao Zhou 2013-08-28 08:53:23 UTC
I use conf.d because I'm using ntpd's unit file as template.  It is using conf.d.  Is that meaning ntpd should also change its service file?

I found it hard to port OpenRC script to s systemd because I do not have a shell environment in unit file.  What is the suitable location to store a sh script to configure something such as command-line options? Or I should just patch the target executable file?
Comment 4 Pacho Ramos gentoo-dev 2013-08-28 09:12:15 UTC
For this case, I would have a gentoo.conf file under /etc/systemd similar to current conf.d file. Service file would then simply have a ExecStart with that --interval and other options inheritting them from $ISATAP_ variables. 

Reading current conf.d file I would try to minimize it to support what people should really set and, then, if you doubt about how to replace:
        [ -n "${ISATAP_INTERVAL}" ] && DAEMON_OPTS="${DAEMON_OPTS} --interval ${ISATAP_INTERVAL} "

from init.d file I would simply change config file to *set* vars to the default values (instead of getting defaults when unset)

For example:
# Interval in seconds to check for DNS changes. Set to 0 to disable.
# Default: 3600
ISATAP_CHECK_DNS="3600" -> notice the line is uncommented 

and, then, ExecStart command would be called with that ISATAP_CHECK_DNS value (that would be default one in this case)
Comment 5 Yichao Zhou 2013-08-28 09:50:24 UTC
(In reply to Pacho Ramos from comment #4)
> For this case, I would have a gentoo.conf file under /etc/systemd similar to
> current conf.d file. Service file would then simply have a ExecStart with
> that --interval and other options inheritting them from $ISATAP_ variables. 
> 
> Reading current conf.d file I would try to minimize it to support what
> people should really set and, then, if you doubt about how to replace:
>         [ -n "${ISATAP_INTERVAL}" ] && DAEMON_OPTS="${DAEMON_OPTS}
> --interval ${ISATAP_INTERVAL} "
> 
> from init.d file I would simply change config file to *set* vars to the
> default values (instead of getting defaults when unset)
> 
> For example:
> # Interval in seconds to check for DNS changes. Set to 0 to disable.
> # Default: 3600
> ISATAP_CHECK_DNS="3600" -> notice the line is uncommented 
> 
> and, then, ExecStart command would be called with that ISATAP_CHECK_DNS
> value (that would be default one in this case)

I tried this way last night.  Unfortunately, I did not success.  One reason is that some option has default parameter "auto", you cannot set it explicitly.  "--link auto" will give you an error message.  A script is necessary without doing patch.

Of course, I can simply delete these options.  I just wonder what is the standard way to "add a script".  But it seems that it hasn't.

Another is that unit file in "net-misc/ntp" is using "conf.d".  Should I submit a bug report?
Comment 6 Pacho Ramos gentoo-dev 2013-08-28 10:03:19 UTC
(In reply to Yichao Zhou from comment #5)
[...]
> I tried this way last night.  Unfortunately, I did not success.  One reason
> is that some option has default parameter "auto", you cannot set it
> explicitly.  "--link auto" will give you an error message.  A script is
> necessary without doing patch.
> 

If that is the only failing option, I would simply drop it

> Of course, I can simply delete these options.  I just wonder what is the
> standard way to "add a script".  But it seems that it hasn't.
> 

There is a way that would be to add a helper (like x11-misc/virtualgl), but it's over complicated for this case and I wouldn't use it.

> Another is that unit file in "net-misc/ntp" is using "conf.d".  Should I
> submit a bug report?

We are aware of the problem and, since we are in a transition period, I think it's not needed :/ (we don't have time and resources to do all at the same time -> add new unit files and migrate available ones to new locations. Will anyway need to review that one as it has a problem with ntp-client unit file). On the other hand, if that kind of conf.d -> /etc/systemd bugs comes with patches, they are always welcome of course :)
Comment 7 Pacho Ramos gentoo-dev 2013-08-31 07:42:28 UTC
Is any other option apart of --link affected by that? If not, could you provide a service file with comment #4 approach? Thanks
Comment 8 Yichao Zhou 2013-08-31 08:02:57 UTC
See https://github.com/shlusiak/isatapd/issues/2.

Maybe we need to wait sometime?
Comment 9 Pacho Ramos gentoo-dev 2013-08-31 09:10:08 UTC
(In reply to Yichao Zhou from comment #8)
> See https://github.com/shlusiak/isatapd/issues/2.
> 
> Maybe we need to wait sometime?

Please don't get me wrong, I was simply reviewing (and committing when possible) all blockers of the tracker bug and came across this one again ;)
Comment 10 Yichao Zhou 2013-08-31 13:24:37 UTC
(In reply to Pacho Ramos from comment #9)
> (In reply to Yichao Zhou from comment #8)
> > See https://github.com/shlusiak/isatapd/issues/2.
> > 
> > Maybe we need to wait sometime?
> 
> Please don't get me wrong, I was simply reviewing (and committing when
> possible) all blockers of the tracker bug and came across this one again ;)

...... Sorry for my bad English.  I think what I want to say is "wait some time".  I have just realized that "wait sometime" has another meaning.  Sorry for that.
Comment 11 Yichao Zhou 2013-09-04 17:36:25 UTC
Which environment file should I use?

I read wiki and I cannot have such directory/file like /etc/systemd/system/foo.service.d/gentoo.conf.

$ grep EnvironmentFile -R
ntpd.service:EnvironmentFile=/etc/conf.d/ntpd
saslauthd.service:EnvironmentFile=/etc/conf.d/saslauthd
isatapd.service:EnvironmentFile=/etc/conf.d/isatapd
smartd.service:EnvironmentFile=-/etc/sysconfig/smartmontools
sntp.service:EnvironmentFile=/etc/conf.d/sntp
ntp-client.service:EnvironmentFile=/etc/conf.d/ntp-client

It contains no good example.
Comment 12 Pacho Ramos gentoo-dev 2013-09-04 21:06:59 UTC
(In reply to Yichao Zhou from comment #11)
> Which environment file should I use?
> 
> I read wiki and I cannot have such directory/file like
> /etc/systemd/system/foo.service.d/gentoo.conf.

It should be:
/etc/systemd/system/isatapd.service.d/gentoo.conf

And that should contain the vars you are going to use in unit file
Comment 13 Yichao Zhou 2013-09-05 05:20:03 UTC
(In reply to Pacho Ramos from comment #12)
> (In reply to Yichao Zhou from comment #11)
> > Which environment file should I use?
> > 
> > I read wiki and I cannot have such directory/file like
> > /etc/systemd/system/foo.service.d/gentoo.conf.
> 
> It should be:
> /etc/systemd/system/isatapd.service.d/gentoo.conf
> 
> And that should contain the vars you are going to use in unit file

Emmm...  Are there some packages which actually uses this style that I can refer to?  On my system, there does not exists such directory like xxx.services.d.
Comment 14 Pacho Ramos gentoo-dev 2013-09-05 06:39:21 UTC
$ grep -r EnvironmentFile *

-> Looks like there isn't any yet :/, I only see the old style conf.d way and some of them using a general config file in /etc (like courier-imap)
Comment 15 Yichao Zhou 2013-09-05 18:45:37 UTC
(In reply to Pacho Ramos from comment #14)
> $ grep -r EnvironmentFile *
> 
> -> Looks like there isn't any yet :/, I only see the old style conf.d way
> and some of them using a general config file in /etc (like courier-imap)

I just do not understand why this is so strange..

/etc/systemd/system/isatapd.service.d/gentoo.conf

Why it is called gentoo.conf?   Why we need a whole directory for this single file?  Is this file the default EnvironmentFile?
Comment 16 Pacho Ramos gentoo-dev 2013-09-06 18:25:23 UTC
(In reply to Yichao Zhou from comment #15)
> (In reply to Pacho Ramos from comment #14)
> > $ grep -r EnvironmentFile *
> > 
> > -> Looks like there isn't any yet :/, I only see the old style conf.d way
> > and some of them using a general config file in /etc (like courier-imap)
> 
> I just do not understand why this is so strange..
> 
> /etc/systemd/system/isatapd.service.d/gentoo.conf
> 
> Why it is called gentoo.conf? 

It's per our policy, to use "gentoo.conf" for our settings and let any other names for users wanting to play with config under that location

>  Why we need a whole directory for this
> single file?  

Because upstream wants to use this dir :/

> Is this file the default EnvironmentFile?

I think we still need to specify it explictely in unit file
Comment 17 Yichao Zhou 2013-09-09 08:08:52 UTC
> Because upstream wants to use this dir :/

Are you sure? Could you give a reference?  Fedora suggests put environment files at /etc/sysconfig:
http://fedoraproject.org/wiki/Packaging:Systemd#EnvironmentFiles_and_support_for_.2Fetc.2Fsysconfig_files

If fedora even not follows that, I suspect if such rule exists.

I'm afraid that we misunderstood something.
See https://wiki.archlinux.org/index.php/Systemd, section  "Editing provided unit files" for some information.  But I have not found a official explanation about xxx.service.d under systemd.

> I think we still need to specify it explictely in unit file

This is not very intuitive.  We we have some .d directories, such as profile.d, env.d, all the files in that directory will be sourced.  But for /etc/systemd/system/isatapd.service.d/gentoo.conf, only gentoo.conf will be used.  This is very confusing for me.

I think /etc/conf.d is much more reasonable since it provide a uniform way to configure both systemd and openrc.  Using /etc/systemd/system/isatapd.service.d/gentoo.conf is just too strange to me and may be incorrect.  And there is even no ebuilds follows this rule.  

Anyway, I hope this policy could be changed.
Comment 18 Michał Górny archtester Gentoo Infrastructure gentoo-dev Security 2013-09-09 08:16:15 UTC
(In reply to Yichao Zhou from comment #15)
> (In reply to Pacho Ramos from comment #14)
> > $ grep -r EnvironmentFile *
> > 
> > -> Looks like there isn't any yet :/, I only see the old style conf.d way
> > and some of them using a general config file in /etc (like courier-imap)
> 
> Why it is called gentoo.conf?

Because it's Gentoo-specific.

> Why we need a whole directory for this single file?

That's the design of '.d' directories. Directory with possibly multiple files inside.

> Is this file the default EnvironmentFile?

It's not an EnvironmentFile. It's unit override file.
Comment 19 Yichao Zhou 2013-09-09 08:32:22 UTC
(In reply to Michał Górny from comment #18)
> (In reply to Yichao Zhou from comment #15)
> > (In reply to Pacho Ramos from comment #14)
> > > $ grep -r EnvironmentFile *
> > > 
> > > -> Looks like there isn't any yet :/, I only see the old style conf.d way
> > > and some of them using a general config file in /etc (like courier-imap)
> > 
> > Why it is called gentoo.conf?
> 
> Because it's Gentoo-specific.
> 
> > Why we need a whole directory for this single file?
> 
> That's the design of '.d' directories. Directory with possibly multiple
> files inside.
> 
> > Is this file the default EnvironmentFile?
> 
> It's not an EnvironmentFile. It's unit override file.

Then where should I place the EnvironmentFile?
Comment 20 Michał Górny archtester Gentoo Infrastructure gentoo-dev Security 2013-09-09 08:35:55 UTC
You don't use that thing. Simple as that.
Comment 21 Yichao Zhou 2013-09-10 13:06:38 UTC
> You don't use that thing. Simple as that.

Why I don't need a environment file?  Then how do I write configuration.
Comment 22 Michał Górny archtester Gentoo Infrastructure gentoo-dev Security 2013-09-10 16:40:42 UTC
(In reply to Yichao Zhou from comment #21)
> > You don't use that thing. Simple as that.
> 
> Why I don't need a environment file?  Then how do I write configuration.

Environment files are *not* for configuration. As the name says, they set *environment* variables for the program. That is, everything that's in EnvironmentFile= gets *exported*.

That said, you *don't* write configuration. You can only provide an example snippet of what user may want to override via 00gentoo.conf file. Please wait till we commit the relevant stuff and then we'll prepare a few examples.
Comment 23 Pacho Ramos gentoo-dev 2013-09-14 12:38:05 UTC
The idea will be to install a /etc/systemd/systemd/foo.service.d/00gentoo.conf with something like:
Environment=FOO=
Comment 24 Pacho Ramos gentoo-dev 2013-09-18 21:28:23 UTC
Latest ntp ebuild will show you how to deal with conf files and units
Comment 25 Yichao Zhou 2013-09-26 07:33:51 UTC
version bump?
Comment 26 Pacho Ramos gentoo-dev 2013-09-28 11:18:17 UTC
+*isatapd-0.9.7 (28 Sep 2013)
+
+  28 Sep 2013; Pacho Ramos <pacho@gentoo.org> +files/isatapd.service,
+  +isatapd-0.9.7.ebuild:
+  Version bump, add unit file (#482690 by Yichao Zhou)
+
Comment 27 Yichao Zhou 2014-02-22 12:00:12 UTC
Today I upgrade my isatapd and found the systemd file which gentoo ships doesn't work.

I update the systemd configuration files so that it should pass the QA now.
Comment 28 Yichao Zhou 2014-02-22 12:01:24 UTC
Created attachment 371020 [details]
/etc/systemd/system/isatapd.service.d/00gentoo
Comment 29 Yichao Zhou 2014-02-22 12:02:58 UTC
Created attachment 371022 [details]
/usr/lib64/systemd/system/isatapd.service
Comment 30 Pacho Ramos gentoo-dev 2014-02-22 12:08:48 UTC
This looks to me more like an "enhancement" over our current unit file (that is pretty similar to Arch one and works ok for them without needing to play with all the options). I would open a new bug report to discuss this new suggested changes
Comment 31 Yichao Zhou 2014-02-22 12:14:48 UTC
No.  The systemd file that gentoo ships now doesn't work at all.

In the original bug, I propose to add
    EnvironmentFile=/etc/conf.d/isatapd
    ExecStart=/usr/sbin/isatapd ${DAEMON_OPTS} ${ISATAP_ROUTERS}


However, in the portage version, "EnvironmentFile" and " ${DAEMON_OPTS} ${ISATAP_ROUTERS}" is removed for unknown reason, which makes it doesn't work:  isatapd need to know which router it uses.
Comment 32 Yichao Zhou 2014-03-19 08:10:46 UTC
Any updates?  The old systemd file for isatapd really does not work at all.
Comment 33 Pacho Ramos gentoo-dev 2014-03-19 21:14:41 UTC
As explained in:
https://wiki.gentoo.org/wiki/Systemd/Ebuild_policy#Unit_file_guidelines

In general we don't want to share conf.d files with openRC neither allow people to pass random options to the executables. I would add a /etc/systemd/system/foo.service.d/gentoo.conf to let people set ISATAP_ROUTERS :/
Comment 34 Yichao Zhou 2014-03-22 13:30:49 UTC
(In reply to Pacho Ramos from comment #33)
> As explained in:
> https://wiki.gentoo.org/wiki/Systemd/Ebuild_policy#Unit_file_guidelines
> 
> In general we don't want to share conf.d files with openRC neither allow
> people to pass random options to the executables. I would add a
> /etc/systemd/system/foo.service.d/gentoo.conf to let people set
> ISATAP_ROUTERS :/

Hi Pacho Ramos,

I have added two new files in my previous comments, which I think implemented what you want and work on my computer.  Would you please do a check for them?   Thanks.
Comment 35 Pacho Ramos gentoo-dev 2014-03-30 10:09:25 UTC
+*isatapd-0.9.7-r1 (30 Mar 2014)
+
+  30 Mar 2014; Pacho Ramos <pacho@gentoo.org> +files/isatapd.service-r1,
+  +files/isatapd.service.conf, +isatapd-0.9.7-r1.ebuild:
+  Fix systemd support (#482690 by Yichao Zhou)
+