Add some clarity around the state.json file by duglin · Pull Request #211 · opencontainers/runtime-spec

duglin · 2015-10-02T20:26:12Z

Signed-off-by: Doug Davis dug@us.ibm.com

vbatts · 2015-10-02T20:40:42Z

Maybe we can say Linux/Unix? Thoughts?

On Fri, Oct 02, 2015 at 01:40:43PM -0700, Vincent Batts wrote:
+On Linux based systems the metadata MUST be stored in /run/opencontainer/containers.
Maybe we can say Linux/Unix? Thoughts?
“Systems with POSIX paths”? I'm fine leaving this as “Linux” and
opting in other architectures as we add support for them.
Why don't we require a particular path on Windows?

used Linux/Unix.
If someone from MSFT tells me what dir to use I can add it.

that's where we look forward to the PoC implementations to help define that.
On Fri, Oct 2, 2015 at 4:59 PM, Doug Davis notifications@github.com wrote:
In runtime.md
#211 (comment):
@@ -2,22 +2,26 @@
State
-The runtime state for a container is persisted on disk so that external tools can consume and act on this information.
-The runtime state is stored in a JSON encoded file.
-It is recommended that this file is stored in a temporary filesystem so that it can be removed on a system reboot.
-On Linux based systems the state information should be stored in /run/opencontainer/containers.
-The directory structure for a container is /run/opencontainer/containers/<containerID>/state.json.
-By providing a default location that container state is stored external applications can find all containers running on a system.
-* version (string) Version of the OCI specification used when creating the container.
-* id (string) ID is the container's ID.
-* pid (int) Pid is the ID of the main process within the container.
-* bundlePath (string) BundlePath is the path to the container's bundle directory.
+Metadata for containers are stored on disk so that external tools can consume and act on this information.
+It is recommended that this data be stored in a temporary filesystem so that it can be removed on a system reboot.
+On Linux based systems the metadata MUST be stored in /run/opencontainer/containers.
used Linux/Unix.
If someone from MSFT tells me what dir to use I can add it.
—
Reply to this email directly or view it on GitHub
https://github.com/opencontainers/specs/pull/211/files#r41067051.

wking · 2015-10-02T21:01:20Z

The README for this project suggests (to me) either “OCS” or “OCIS” or “OCI Specification”. I suggest we pick one abbreviation and have the README say:
The Open Container Initiative Specifications (OCIS) …
or whatever. And then use that short name throughout the spec. Until we do that, I'd probably just leave this as “OCI specification”.

hmmm. OCF had been mentioned, but not much defined.
Perhaps just "OpenContainers spec" or similar for now.

wking · 2015-10-02T21:15:54Z

I like this PR a lot, but it's going to conflict with #145, and I think those (minor) changes are useful. I'm fine if you just squash them into this branch, or if I rebase mine on this, or if you rebase yours on mine, …. If I can help manage that conflict in any way, let me know ;).

duglin · 2015-10-02T21:22:20Z

@wking I think I got your edits from the other one

wking · 2015-10-02T21:24:37Z

I'd change to “is the ID of the main process” to avoid doubling up on “process”.

duglin · 2015-10-02T21:27:10Z

I agree that we need to tighten up the format of the ID. I was going to leave that for a different PR so that this is more about just cleaning up the text, but if we can agree quickly (w/o much bike shedding) then I'll add it.

To get the ball rolling, what about IDs must conform to: [0-9a-zA-Z_]+ ?

wking · 2015-10-02T21:27:25Z

Maybe qualify this with “Linux/Unix” somehow? Something like “For example, a Linux/Unix directory /run/opencontainer/containers/173975398351 corresponds to a container with an ID of 173975398351.”

wking · 2015-10-02T21:31:44Z

On Fri, Oct 02, 2015 at 02:22:21PM -0700, Doug Davis wrote:

@wking I think I got your edits from the other one

Looks good, closing #145 in favor of this.

wking · 2015-10-02T21:33:30Z

On Fri, Oct 02, 2015 at 02:27:12PM -0700, Doug Davis wrote:

I agree that we need to tighten up the format of the ID. I was going
to leave that for a different PR so that this is more about just
cleaning up the text, but if we can agree quickly (w/o much bike
shedding) then I'll add it.

Fair. So far, it's just been silence, so I don't think bikeshedding
will be an issue ;).

To get the ball rolling, what about ID must conform to:
[0-9a-zA-Z_]+ ?

I think you want ‘-’ too, so you can use UUIDs (links in my thread).

vbatts · 2015-10-05T14:09:17Z

To get the ball rolling, what about ID must conform to:
[0-9a-zA-Z_]+ ?
I think you want ‘-’ too, so you can use UUIDs (links in my thread).

While there may be the need for rigidity, I'm not sure that needs to be the case right now. UUID needs to be allowed. But possibly colons too.

duglin · 2015-10-05T14:17:35Z

Let's save the ID format for another PR.

vbatts · 2015-10-05T14:30:47Z

sounds good

On Mon, Oct 5, 2015 at 10:17 AM, Doug Davis notifications@github.com
wrote:

Let's let the ID format for another PR.
—
Reply to this email directly or view it on GitHub
#211 (comment).

mrunalp · 2015-10-05T16:42:20Z

s/in/under ?

changed to "under"

mrunalp · 2015-10-05T16:43:39Z

LGTM

Signed-off-by: Doug Davis <dug@us.ibm.com>

crosbymichael · 2015-10-05T20:08:33Z

LGTM

Add some clarity around the state.json file

vbatts reviewed Oct 2, 2015
View reviewed changes

duglin force-pushed the tweakRuntime branch from 421d087 to 32af7bbCompare October 2, 2015 21:00

wking reviewed Oct 2, 2015
View reviewed changes

duglin force-pushed the tweakRuntime branch 2 times, most recently from 4665ce0 to 6a3e10bCompare October 2, 2015 21:21

wking reviewed Oct 2, 2015
View reviewed changes

Comment threadruntime.md Outdated
wkingOct 2, 2015
Copy link
Copy Markdown
Contributor
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd change to “is the ID of the main process” to avoid doubling up on “process”.

wking reviewed Oct 2, 2015
View reviewed changes

duglin force-pushed the tweakRuntime branch 2 times, most recently from ee6dc1c to 7d2f0ccCompare October 2, 2015 21:29

wking mentioned this pull request Oct 2, 2015
runtime: Deduplicate phrasing in state attribute definitions #145
Closed

mrunalp reviewed Oct 5, 2015
View reviewed changes

Add some clarity around the state.json file
44d9ba6
Signed-off-by: Doug Davis <dug@us.ibm.com>

duglin force-pushed the tweakRuntime branch from 7d2f0cc to 44d9ba6Compare October 5, 2015 17:29

crosbymichael added a commit that referenced this pull request Oct 5, 2015
Merge pull request#211from duglin/tweakRuntime
fe09674
Add some clarity around the state.json file

crosbymichael merged commit fe09674 into opencontainers:masterOct 5, 2015

duglin deleted the tweakRuntime branch October 13, 2015 13:35

Uh oh!

Conversation

duglin commented Oct 2, 2015

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

State

-By providing a default location that container state is stored external applications can find all containers running on a system.

-* bundlePath (string) BundlePath is the path to the container's bundle directory.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

wking commented Oct 2, 2015 via email

Uh oh!

duglin commented Oct 2, 2015

Uh oh!

Choose a reason for hiding this comment

Uh oh!

duglin commented Oct 2, 2015

Uh oh!

Choose a reason for hiding this comment

Uh oh!

wking commented Oct 2, 2015

Uh oh!

wking commented Oct 2, 2015

Uh oh!

vbatts commented Oct 5, 2015

Uh oh!

duglin commented Oct 5, 2015

Uh oh!

vbatts commented Oct 5, 2015

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mrunalp commented Oct 5, 2015

Uh oh!

crosbymichael commented Oct 5, 2015

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

-* `bundlePath` (string) BundlePath is the path to the container's bundle directory.