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

Anchor tags #707

Merged
merged 3 commits into from
Mar 3, 2017
Merged

Anchor tags #707

merged 3 commits into from
Mar 3, 2017

Conversation

mrunalp
Copy link
Contributor

@mrunalp mrunalp commented Mar 2, 2017

This PR adds anchor tags to remaining files.
Closes #634

Signed-off-by: Mrunal Patel <mrunalp@gmail.com>
Signed-off-by: Mrunal Patel <mrunalp@gmail.com>
Signed-off-by: Mrunal Patel <mrunalp@gmail.com>
@mrunalp
Copy link
Contributor Author

mrunalp commented Mar 2, 2017

@crosbymichael @hqhq @dqminh PTAL

@hqhq
Copy link
Contributor

hqhq commented Mar 3, 2017

LGTM

Approved with PullApprove

@crosbymichael crosbymichael added this to the 1.0.0 milestone Mar 3, 2017
@crosbymichael
Copy link
Member

crosbymichael commented Mar 3, 2017

LGTM

Approved with PullApprove

@crosbymichael crosbymichael merged commit 41839d7 into opencontainers:master Mar 3, 2017
wking added a commit to wking/opencontainer-runtime-spec that referenced this pull request Mar 3, 2017
I expect the lifecycle information was removed accidentally in
be59415 (Split create and start, 2016-04-01, opencontainers#384), because for a
time it seemed like that PR would also be removing hooks.  Putting the
lifecycle information back in, I made some tweaks to adjust to the new
environment, for example:

* Put the pre-start hooks after the 'start' call, but before the meat
  of the start call (the container-process exec trigger).  Folks who
  want a post-create hook can add one with that name.  I'd like to
  have renamed poststop to post-delete to avoid confusion like [1].
  But the motivation for keeping hooks was backwards compatibility [2]
  so I've left the name alone.

* Put each "...command is invoked..." lifecycle entry in its own list
  entry, to match the 'create' list entry.

* Move the rules about what happens on hook failure into the
  lifecycle.  This matches pre-split entries like:

    If any prestart hook fails, then the container MUST be stopped and
    the lifecycle continues at step 7.

  and avoids respecifying that information in a second location
  (config.md).

* I added the warning section to try and follow post-split's generic
  "generates an error" approach while respecting the pre-split desire
  to see what failed (we had "then an error including the exit code
  and the stderr is returned to the caller" and "then an error is
  logged").

* I left the state 'id' context out, since Michael didn't want it [3].

* Make runtime.md references to "generate an error" and "log a
  warning" links, so readers have an easier time finding more detail
  on that wording.

Where I reference a section, I'm still using the auto-generated anchor
for that header and not the anchors which were added in 41839d7 (Merge
pull request opencontainers#707 from mrunalp/anchor_tags, 2017-03-03) and similar.
Mrunal suggested that the manually-added anchors were mainly intended
for the validation tooling [4].

[1]: opencontainers#395
     Subject: Run post-stop hooks before the container sandbox is deleted.
[2]: opencontainers#483 (comment)
     Subject: *: Remove hooks
[3]: opencontainers#532 (comment)
     Subject: Restore hook language removed by create/start split
[4]: http://ircbot.wl.linuxfoundation.org/eavesdrop/%23opencontainers/%23opencontainers.2017-03-03.log.html#t2017-03-03T18:02:12

Signed-off-by: W. Trevor King <wking@tremily.us>
@vbatts vbatts mentioned this pull request Mar 6, 2017
@wking wking mentioned this pull request Jul 27, 2017
11 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants