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.

Sign up for GitHub

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

Split create and start #384

Merged
merged 1 commit into from
May 26, 2016
Merged

Split create and start #384

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
77 changes: 43 additions & 34 deletions runtime.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,6 @@ The state of a container MUST include, at least, the following properties:
* **`id`**: (string) is the container's ID.
This MUST be unique across all containers on this host.
There is no requirement that it be unique across hosts.
The ID is provided in the state because hooks will be executed with the state as the payload.
This allows the hooks to perform cleanup and teardown logic after the runtime destroys its own state.
* **`pid`**: (int) is the ID of the main process within the container, as seen by the host.
* **`bundlePath`**: (string) is the absolute path to the container's bundle directory.
This is provided so that consumers can find the container's configuration and root filesystem on the host.
Expand All @@ -34,22 +32,19 @@ See [Query State](#query-state) for information on retrieving the state of a con

## Lifecycle
The lifecycle describes the timeline of events that happen from when a container is created to when it ceases to exist.

1. OCI compliant runtime is invoked with a reference to the location of the bundle.
How this reference is passed to the runtime is an implementation detail.
1. OCI compliant runtime's `create` command is invoked with a reference to the location of the bundle and a unique identifier.
How these references are passed to the runtime is an implementation detail.
2. The container's runtime environment MUST be created according to the configuration in [`config.json`](config.md).
Any updates to `config.json` after container is running MUST not affect the container.
3. The prestart hooks MUST be invoked by the runtime.
If any prestart hook fails, then the container MUST be stopped and the lifecycle continues at step 7.
4. The user specified process MUST be executed in the container.
5. The poststart hooks MUST be invoked by the runtime.
If any poststart hook fails, then the container MUST be stopped and the lifecycle continues at step 7.
6. Additional actions such as pausing the container, resuming the container or signaling the container MAY be performed using the runtime interface.
The container MAY also error out, exit or crash.
7. The container MUST be destroyed by undoing the steps performed during create phase (step 2).
8. The poststop hooks MUST be invoked by the runtime and errors, if any, SHOULD be logged.

Note: The lifecycle is a WIP and it will evolve as we have more use cases and more information on the viability of a separate create phase.
While the resources requested in the [`config.json`](config.md) MUST be created, the user-specified code (from [`process`](config.md#process-configuration) MUST NOT be run at this time.
Any updates to `config.json` after this step MUST NOT affect the container.
3. Once the container is created additional actions MAY be performed based on the features the runtime chooses to support.
However, some actions might only be available based on the current state of the container (e.g. only available while it is started).
Copy link
Contributor

Choose a reason for hiding this comment

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

Needs one more space for prefix.

4. Runtime's `start` command is invoked with the unique identifier of the container.
The runtime MUST run the user-specified code, as specified by [`process`](config.md#process-configuration).
5. The container's process is stopped.
This MAY happen due to them erroring out, exiting, crashing or the runtime's `kill` operation being invoked.
6. Runtime's `delete` command is invoked with the unique identifier of the container.
The container MUST be destroyed by undoing the steps performed during create phase (step 2).
Copy link
Member

Choose a reason for hiding this comment

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

You need a delete because when a container exits on its own there is no way for the runtimes to remove state or anything else.


## Errors

Expand All @@ -67,36 +62,50 @@ Note: these operations are not specifying any command-line APIs, and the paramen
`state <container-id>`

This operation MUST generate an error if it is not provided the ID of a container.
Attempting to query a container that does not exist MUST generate an error.
This operation MUST return the state of a container as specified in the [State](#state) section.

### Start
### Create

`start <container-id> <path-to-bundle>`
`create <container-id> <path-to-bundle>`

This operation MUST generate an error if it is not provided a path to the bundle and the container ID to associate with the container.
If the ID provided is not unique across all containers within the scope of the runtime, or is not valid in any other way, the implementation MUST generate an error.
Using the data in `config.json`, that are in the bundle's directory, this operation MUST create a new container.
This includes creating the relevant namespaces, resource limits, etc and configuring the appropriate capabilities for the container.
A new process within the scope of the container MUST be created as specified by the `config.json` file otherwise an error MUST be generated.
If the ID provided is not unique across all containers within the scope of the runtime, or is not valid in any other way, the implementation MUST generate an error and a new container MUST not be created.
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there a schema for the ID? If yes, we should link it here.

Using the data in [`config.json`](config.md), this operation MUST create a new container.
This means that all of the resources associated with the container MUST be created, however, the user-specified process MUST NOT be run at this time.
Copy link
Contributor

@wking wking May 26, 2016 •
edited
Loading

Choose a reason for hiding this comment

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

I think we should echo the lifecycle wording here and say “the user-specified code, as specified by process, MUST NOT be run at this time”. Especially with the idling-process reference removed, saying anything here about not running a process (vs. not running code) seems like it could be confusing.


The runtime MAY validate `config.json` against this spec, either generically or with respect to the local system capabilities, before creating the container ([step 2](#lifecycle)).
If the runtime does not perform initial validation and triggers an error due to an invalid or incompatible configuration, it MUST generate an error and jump to cleanup ([step 7](#lifecycle)).
Runtime callers who are interested in pre-start validation can run [bundle-validation tools](implementations.md#testing--tools) before invoking the start operation.
Runtime callers who are interested in pre-create validation can run [bundle-validation tools](implementations.md#testing--tools) before invoking the create operation.

Any changes made to the [`config.json`](config.md) file after this operation will not have an effect on the container.

### Start
Copy link
Contributor

Choose a reason for hiding this comment

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

This is from #418 and should not be dropped (it's orthogonal to the create/start split).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

weird - I dropped it because I thought it was moved to someplace else in this doc, but now I can't find it. I'll add it back.

Copy link
Contributor

Choose a reason for hiding this comment

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

I still don't think we want to drop this line (and it should be shifted to the Create section).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I added it back but I think its unnecessary since people can error check whenever they want.

Copy link
Contributor

Choose a reason for hiding this comment

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

On Thu, May 26, 2016 at 12:47:28PM -0700, Doug Davis wrote:

-The runtime MAY validate config.json against this spec, either generically or with respect to the local system capabilities, before creating the container (step 2).

I added it back but I think its unnecessary since people can error
check whenever they want.

Without this line, how would you know that? #418 was about making it
explicitly optional so everyone is on the same page.

`start <container-id>`

Attempting to start an already running container MUST have no effect on the container and MUST generate an error.
This operation MUST generate an error if it is not provided the container ID.
Attempting to start a container that does not exist MUST generate an error.
Copy link
Contributor

Choose a reason for hiding this comment

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

If we're listing this for every operation (it seems obvious to me, but I don't mind saying it explicitly), we'll want a similar entry for the “Query State” section.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

Attempting to start an already started container MUST have no effect on the container and MUST generate an error.
This operation MUST run the user-specified code as specified by [`process`](config.md#process-configuration).
Copy link
Contributor

@wking wking May 26, 2016 •
edited
Loading

Choose a reason for hiding this comment

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

If we're documenting the current opencontainers/runc#827, we want to clarify that this is process (as loaded by create when it was invoked).

If the runtime fails to run the code as specified, an error MUST be generated.
Copy link
Contributor

Choose a reason for hiding this comment

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

I still think this line is not supportable with the current opencontainers/runc#827 (which just throws a signal over the wall).


### Kill
`kill <container-id> <signal>`
Copy link
Contributor

Choose a reason for hiding this comment

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

Windows doesn't support interprocess signals, so I think we need to decrease the flexibility and have explicit soft/hard terminate operations.


### Stop
This operation MUST generate an error if it is not provided the container ID.
Attempting to send a signal to a container that is not running MUST have no effect on the container and MUST generate an error.
Copy link
Contributor

Choose a reason for hiding this comment

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

Signals may not be cross platform.

Copy link
Member

Choose a reason for hiding this comment

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

"signal" in the general sense ;)

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah, that makes sense.

Copy link
Contributor

Choose a reason for hiding this comment

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

On Thu, May 26, 2016 at 02:08:30PM -0700, Michael Crosby wrote:

-### Stop
+This operation MUST generate an error if it is not provided the container ID.
+Attempting to send a signal to a container that is not running MUST have no effect on the container and MUST generate an error.

"signal" in the general sense ;)

Do you have a list of “in the general sense” signals? I have no idea
how far the hcsshim functionality extends beyond the soft
ShutdownComputeSystem 1.

This operation MUST send the specified signal to the process in the container.

`stop <container-id>`
### Delete
Copy link
Contributor

Choose a reason for hiding this comment

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

Shifting from stop to delete sounds like #356. Do we need to pull that into this PR? It seems like we can just focus on the setup side of things here.

Copy link
Contributor

Choose a reason for hiding this comment

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

How do we handle graceful deletions? As of now the general norm is that of posting a SIGTERM followed by a SIGKILL.

Copy link
Contributor

Choose a reason for hiding this comment

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

On Thu, Apr 14, 2016 at 01:58:20PM -0700, Vish Kannan wrote:

+### Delete

How do we handle graceful deletions? As of now the general norm is
that of posting a SIGTERM followed by a SIGKILL.

Can we keep the delete discussion in [1,2]? It seems orthogonal to a
create/start split.

 Subject: Clarify distinction between ‘stop’ and ‘delete’
 Date: Wed, 23 Mar 2016 11:49:35 -0700
 Message-ID: <20160323184935.GB23066@odin.tremily.us>

`delete <container-id>`

This operation MUST generate an error if it is not provided the container ID.
Copy link
Contributor

Choose a reason for hiding this comment

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

What happened to poststop hooks?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We shouldn't need hooks if we do this, right?

Copy link
Contributor

Choose a reason for hiding this comment

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

What about post stop? If we don't pin the namespaces, I cannot extract files from the mount namespaces after the init process dies, for example.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It seems to me then we should split stop/delete at the same time, no?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah. That was what I was suggesting during the weekly meetings.

On Fri, Apr 22, 2016 at 11:10 AM, Doug Davis notifications@github.com
wrote:

In runtime.md
#384 (comment)
:

@@ -122,8 +137,3 @@ This specification does not mandate the name of this JSON file.
See the specification of the config.json file for the definition of these fields.
The stopping, or exiting, of these secondary process MUST have no effect on the state of the container.

In other words, a container (and its PID 1 process) MUST NOT be stopped due to the exiting of a secondary process.

-## Hooks

-Many of the operations specified in this specification have "hooks" that allow for additional actions to be taken before or after each operation.

It seems to me then we should split stop/delete at the same time, no?

—
You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub
https://github.com/opencontainers/runtime-spec/pull/384/files/5813d96a74b5e753851fdad11ce4731ac7be0576#r60780090

Copy link
Contributor

Choose a reason for hiding this comment

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

On Fri, Apr 22, 2016 at 10:59:59AM -0700, Vish Kannan wrote:
“What about post stop? If we don't pin the namespaces, I cannot
extract files from the mount namespaces after the init process dies,
for example.”

I'm with @julz 1 and @duglin 2 on this. Sometimes folks will want
to pin namespaces, and they can do that via sub-containers 1, or
bind-mounts, etc. as they see fit. But splitting stop/delete is
requiring the runtime to pin namespaces, and that seems overly
complicated to me.

Copy link
Contributor

Choose a reason for hiding this comment

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

Right, sorry, my point is this is orthogonal. You don't need a post stop hook to stream out the mount namespace after a process does, you need a sub container (which, at least to my mind works nicely with our existing discussions about moving away from exec and towards sub-containers) or a bind mount done after create, as @wking says. I agree we can lose hooks.

Copy link
Contributor

@vishh vishh Apr 22, 2016 •
edited
Loading

Choose a reason for hiding this comment

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

@duglin @julz
Let me try to summarize the plan for supporting post-stop hooks:

  1. Create a container A
  2. Create another container B that will be reuse the namespaces (excepting pid?) from A
  3. Start, then eventually stop/delete A.
  4. Since B is expected to be still active, once can exec into B and run post stop hooks.

Some thoughts & question on the proposed approach.

  1. What parts of A's environment is sharable with B?
  2. When the init pid in A dies what parts of the sandbox is allowed to be cleaned up? For example, can B use As cgroups and will cgroups be preserved?
  3. Can B be created without a pid namespace? If we rely on pid namespace, there is a chance that the init process we launch as part of Create might die..

Overall, although the proposed approach sounds somewhat plausible, it is very cumbersome. The API and hence the clients that use the API, will be much cleaner if we were able to preserve (parts) the sanbox even after the init process dies.

Copy link
Contributor

@wking wking Apr 22, 2016 •
edited
Loading

Choose a reason for hiding this comment

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

On Fri, Apr 22, 2016 at 01:25:23PM -0700, Vish Kannan wrote:
“Let me try to summarize the plan for supporting post-stop hooks:”

The current spec pretty clear that post-stop hooks (step 8) happen
after container teardown (step 7) 1. So the “namespace-pinning and
post-stop hooks” discussion seems to be orthogonal to this PR.

For your sub-container example, I don't think the order/parent-ness
matters much. You have the pinning container joining the working
container, but you could also have the working container join the
pinning container. In the latter case, you're basically doing work
via exec, and the sub-container looks like #391.

“1. What parts of A's environment is sharable with B?”

There are many reasonable answers to this question. If I was writing
this up, I'd probably have the parent only create the sandbox it
wanted to pin. THe child would join, create the un-pinned sandbox,
execute, and die. Then the child sandbox gets reaped, you pull your
data out of the pinned sandbox, and kill the parent.

Copy link
Contributor

Choose a reason for hiding this comment

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

Posted #395 for post-stop semantics

This operation MUST stop and delete a running container.
Stopping a container MUST stop all of the processes running within the scope of the container.
Deleting a container MUST delete the associated namespaces and resources associated with the container.
Once a container is deleted, its `id` MAY be used by subsequent containers.
Attempting to stop a container that is not running MUST have no effect on the container and MUST generate an error.
Attempting to delete a container that does not exist MUST generate an error.
Attempting to delete a container whose process is still running MUST generate an error.
Deleting a container MUST delete the resources that were created during the `create` step.
Note that resources associated with the container, but not created by this container, MUST NOT be deleted.
Once a container is deleted its ID MAY be used by a subsequent container.

## Hooks

## Hooks
Many of the operations specified in this specification have "hooks" that allow for additional actions to be taken before or after each operation.
See [runtime configuration for hooks](./config.md#hooks) for more information.