Bug 1121783 - systemd ExecStart handling is confusing
Summary: systemd ExecStart handling is confusing
Keywords:
Status: CLOSED NOTABUG
Alias: None
Product: Red Hat Enterprise Linux 7
Classification: Red Hat
Component: systemd
Version: 7.0
Hardware: Unspecified
OS: Unspecified
unspecified
unspecified
Target Milestone: rc
: ---
Assignee: systemd-maint
QA Contact: qe-baseos-daemons
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2014-07-21 21:11 UTC by Karl Hastings
Modified: 2018-12-06 17:25 UTC (History)
4 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2014-08-25 11:43:59 UTC


Attachments (Terms of Use)

Description Karl Hastings 2014-07-21 21:11:53 UTC
Description of problem:
The way systemd handles duplicate ExecStart lines is confusing.  Instead of throwing an error, later entries should override earlier entries.

I.e.

~~~
root@localhost system# cat docker.service 
.include /usr/lib/systemd/system/docker.service

[Service]
ExecStart=/usr/bin/docker -d --selinux-enabled -H fd:// $DOCKER_OPTS
root@localhost system# systemctl start docker.service
Jul 10 20:23:10 localhost systemd[1]: docker.service has more than one ExecStart setting, which is only allowed for Type=oneshot services. Refusing.
~~~

The workaround is to clear the entry first, and then set it:
~~~
.include /usr/lib/systemd/system/docker.service

[Service]
ExecStart=
ExecStart=/usr/bin/docker -d --selinux-enabled -H fd:// $DOCKER_OPTS
~~~

But this seems unnecessary (and imho somewhat confusing) since it's not valid to have multiple ExecStart entries in this type of unit.

Version-Release number of selected component (if applicable):
systemd-208-11.el7.x86_64

How reproducible:
100%

Steps to Reproduce:
1. Attempt to overwrite an ExecStart line as above

Actual results:
service refuses to run

Expected results:
service runs with overwritten options.

Additional info:

Comment 2 Michal Sekletar 2014-07-22 08:30:00 UTC
I don't think we want to change current behaviour. Altering ExecStart option for service really changes vendor supplied semantics substantially thus in such cases copying the whole thing to /etc is advised. I am tempted to close as NOTABUG.

Comment 3 Jon Stanley 2014-07-22 12:57:25 UTC
While there is an error message that gives some indication of what went wrong, I personally think that since the operation makes no sense whatsoever, the overridden attribute should take precedence.

It breaks the promise of systemd that "configuration in /etc overrides configuration in /usr" to not do this. Of course, I could just not .include the system-provided unit and write my own complete unit, but then you lose the maintainability value that including the system-provided unit gives you.
 
An argument could also possibly be made for what I'll call "non-accretive" values in /etc. Whereas the following is perfectly valid in any unit:

[Service]
Environment=FOO=bar
Environment=BAZ=bar

One could quite reasonably expect that if the first line was in a unit in /usr, and the second in an overriding unit in /etc which included the first unit, then $FOO wouldn't be set in the resultant environment - i.e. the content of the second unit really DOES override the content of the first.

I realize that this results in special handling of .include'd units, possibly adding something like '.includeoverride' or something of the sort so that people can opt-in to this change in behavior.

Comment 4 Jon Stanley 2014-07-22 13:03:30 UTC
(In reply to Jon Stanley from comment #3)

> unit, then $FOO wouldn't be set in the resultant environment - i.e. the
> content of the second unit really DOES override the content of the first.

I really should clarify this and say that "identical keys in the second included unit override the values in the first, and reset them"

Let me know if this is unclear, I realize my first wording of this was about as clear as mud :)

Comment 5 Lukáš Nykrýn 2014-08-25 11:43:59 UTC
Closed for reason mention in Comment 2. Changing ExecStart is substantial change how the unit works. ALso this stanza for example uses variables given by Environment= , this could break easily during update.

Btw using .include is deprecated, you should use drop-in files. For the service foo.service create foo.service.d/SOMETHING.conf.


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