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

lifecycle: Don't require /run/opencontainer/<runtime>/containers #269

Merged
merged 1 commit into from
Dec 15, 2015

Conversation

wking
Copy link
Contributor

@wking wking commented Dec 11, 2015

As discussed in #231. We already require it for
Linux/Unix-based systems
, so we don't have to repeat it here. And
other systems will use different paths, which we haven't specified
yet. When I asked why we didn't specify a path for Windows,
Vincent said we were waiting on help from PoC implementations.
So this commit punts the location to the “State” section, and lets the
“Lifecycle” section just focus on when the write-to-filesystem
happens.

There's also discussion about removing the filesystem state registry
completely
, in which case we'd want to remove the whole line from
the lifecycle. If we expect to switch to something like --state PATH in the near future this PR is just polishing dead
documentation.

@mrunalp
Copy link
Contributor

mrunalp commented Dec 14, 2015

LGTM

@duglin
Copy link
Contributor

duglin commented Dec 14, 2015

I'd prefer to NOT do this because we're replacing a concrete, and interoperable, solution with something that is not. If we don't like the current proposal then I think it would be better to only remove it when there's a viable alternative proposed - replace it with just "write it to the filesystem" doesn't help interop.

We already require it for Linux/Unix-based systems [1], so we don't
have to repeat it here.  And other systems will use different paths,
which we haven't specified yet.  When I asked why we didn't specify a
path for Windows [2], Vincent said we were waiting on help from PoC
implementations [3].  So this commit punts the location to the "State"
section, and lets the "Lifecycle" section just focus on when the
write-to-filesystem happens.

There's also discussion about removing the filesystem state registry
completely [4], in which case we'd want to remove the whole line from
the lifecycle.

[1]: opencontainers@7713efc#diff-b84a8d65d8ed53f4794cd2db7e8ea731L7
[2]: opencontainers#211 (comment)
[3]: opencontainers#211 (comment)
[4]: https://groups.google.com/a/opencontainers.org/forum/#!topic/dev/q6TYqVZOcX8
     Subject: removal of /run/opencontainer/containers
     Date: Wed, 25 Nov 2015 14:29:35 +0000
     Message-ID: <CAD2oYtNipt3i_C6=J4Bc-jwauo5YAvKXUqTROnPNP3vZ9+C5Vw@mail.gmail.com>

Signed-off-by: W. Trevor King <wking@tremily.us>
@wking
Copy link
Contributor Author

wking commented Dec 14, 2015

On Mon, Dec 14, 2015 at 09:51:11AM -0800, Doug Davis wrote:

I'd prefer to NOT do this because we're replacing a concrete, and
interoperable, solution with something that is not…

This PR doesn't change the semantics for Linux/Unix-based systems,
which still require runtimes to write the state to
/run/opencontainer//containers based on wording in the
“State” section 1. It just removes the unqualified reference from
the “Lifecycle” section [2], which was redundant for Linux/Unix-based
systems and wrong for other systems.

@wking
Copy link
Contributor Author

wking commented Dec 14, 2015 via email

@duglin
Copy link
Contributor

duglin commented Dec 14, 2015

@wking ah, doi - sorry missed that. I take back my concerns :-)

@hqhq
Copy link
Contributor

hqhq commented Dec 15, 2015

LGTM

hqhq added a commit that referenced this pull request Dec 15, 2015
lifecycle: Don't require /run/opencontainer/<runtime>/containers
@hqhq hqhq merged commit 7c17452 into opencontainers:master Dec 15, 2015
@wking wking deleted the lifecycle-state-path branch December 21, 2015 21:19
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.

4 participants