Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

do not require custom build of tini #28454

Merged
merged 1 commit into from
Jan 20, 2017
Merged

do not require custom build of tini #28454

merged 1 commit into from
Jan 20, 2017

Conversation

glensc
Copy link
Contributor

@glensc glensc commented Nov 15, 2016

- What I did

the original implementation #28037 by @crosbymichael required custom build of tini that does not have option parser (requested in krallin/tini#55)
but it can be accomplished without need for such custom build by just using -- to stop argument parsing.

- How I did it

adds -- argument when calling init reaper allowing flags from CMD not to interfere with init own commandline arguments

krallin/tini#55 (comment)
krallin/tini#55 (comment)
#28037

- How to verify it

# docker run --init --rm -it ubuntu echo 123

- Description for the changelog

Do not require custom build of tini.

- A picture of a cute animal (not mandatory but encouraged)

14721688_1137226096331677_3024758630935413975_n

@GordonTheTurtle GordonTheTurtle added dco/no Automatically set by a bot when one of the commits lacks proper signature status/0-triage labels Nov 15, 2016
@GordonTheTurtle GordonTheTurtle removed the dco/no Automatically set by a bot when one of the commits lacks proper signature label Nov 15, 2016
glensc added a commit to pld-linux/docker that referenced this pull request Nov 15, 2016
@crosbymichael
Copy link
Contributor

The only problem I have with this change is that to have the init swappable, not all inits have flags like tini did and if they don't, they will accept -- as an arg and pass it to exec. We made the change so that we have a contract with all inits, that you passthrough all arguments and there is nothing to parse or interpret.

@glensc
Copy link
Contributor Author

glensc commented Nov 17, 2016

@crosbymichael well. it's the same way the opposite: you can't use init that parses options, like tiny was initially.

@crosbymichael
Copy link
Contributor

Yes, and that is why we changed tini and added the flag, so it wouldn't

@glensc
Copy link
Contributor Author

glensc commented Nov 17, 2016

btw, what init daemons you had in mind? i'm pretty sure sysv init, systemd init and upstart init don't pass argv to command to execute and follow them. there must be used init process specifically written for the docker purpose.

@cpuguy83
Copy link
Member

What's the status here?

@AkihiroSuda
Copy link
Member

@crosbymichael WDYT about @glensc 's question?

@justincormack
Copy link
Contributor

justincormack commented Jan 19, 2017 via email

@crosbymichael
Copy link
Contributor

LGTM

1 similar comment
@LK4D4
Copy link
Contributor

LK4D4 commented Jan 20, 2017

LGTM

@LK4D4 LK4D4 merged commit 50a72c7 into moby:master Jan 20, 2017
@GordonTheTurtle GordonTheTurtle added this to the 1.14.0 milestone Jan 20, 2017
tianon added a commit to tianon/docker-overlay that referenced this pull request Jan 24, 2017
@vieux vieux modified the milestones: 1.13.1, 1.14.0 Jan 25, 2017
@glensc glensc deleted the init-args branch March 28, 2017 10:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants