Skip to content
This repository has been archived by the owner on Feb 24, 2020. It is now read-only.

stage1/init: only consider running from unit file when session leader #1694

Merged
merged 1 commit into from Nov 2, 2015
Merged

stage1/init: only consider running from unit file when session leader #1694

merged 1 commit into from Nov 2, 2015

Conversation

vcaputo
Copy link
Contributor

@vcaputo vcaputo commented Oct 29, 2015

This augments the heuristics used to err on the side of non-unit-file,
but will still consider as run from a unit file when executed directly
via systemd.service Exec* directives.

With this change, interactive invocations of rkt on CoreOS cease passing
--keep-unit to nspawn.

Fixes #1664

@iaguis
Copy link
Member

iaguis commented Oct 29, 2015

This means that if a systemd unit file has a ExecStart=/bin/sh -c "rkt run ..." rkt won't detect it's running from a unit file and won't pass --keep-unit, right?

I guess it's a reasonable compromise.

@vcaputo
Copy link
Contributor Author

vcaputo commented Oct 29, 2015

Maybe there should also be a way to explicitly set --keep-unit, for when the heuristics fail?

// int
// am_session_leader()
// {
// return (getsid(0) == getpid());
Copy link
Member

Choose a reason for hiding this comment

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

We need to include <unistd.h>

Copy link
Contributor Author

Choose a reason for hiding this comment

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

added, good catch

@iaguis
Copy link
Member

iaguis commented Oct 30, 2015

I think this is reasonable and I don't think we need a flag for --keep-unit unless we find a common use case where the heuristic fails.

@alban?

This augments the heuristics used to err on the side of non-unit-file,
but will still consider as run from a unit file when executed directly
via systemd.service Exec* directives.

With this change, interactive invocations of rkt on CoreOS cease passing
--keep-unit to nspawn.

Fixes #1664
@alban
Copy link
Member

alban commented Nov 1, 2015

I think this is good as it is.

Do you think it is worth documenting in Advanced Unit File? Something like:

The following pattern should be avoided

ExecStart=/bin/sh -c "foo ; rkt run ..."

Instead, the following should be ok:

ExecStart=/bin/sh -c "foo ; exec rkt run ..."

or:

ExecStartPre=foo
ExecStart=rkt run ...

@iaguis
Copy link
Member

iaguis commented Nov 2, 2015

Do you think it is worth documenting in Advanced Unit File?

can't hurt

iaguis added a commit that referenced this pull request Nov 2, 2015
…der_too

stage1/init: only consider running from unit file when session leader
@iaguis iaguis merged commit cec6a7c into rkt:master Nov 2, 2015
iaguis added a commit to iaguis/rkt that referenced this pull request Nov 2, 2015
After rkt#1694, rkt needs to be the group
leader in a systemd unit file to make unit file detection work.

Add some documentation on the topic.
@iaguis
Copy link
Member

iaguis commented Nov 2, 2015

#1704

iaguis added a commit to iaguis/rkt that referenced this pull request Nov 2, 2015
After rkt#1694, rkt needs to be the group
leader in a systemd unit file to make unit file detection work.

Add some documentation on the topic.
iaguis added a commit to iaguis/rkt that referenced this pull request Nov 2, 2015
After rkt#1694, rkt needs to be the group
leader in a systemd unit file to make unit file detection work.

Add some documentation on the topic.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants