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

config: StopSignal support #698

Closed
cyphar opened this issue Feb 27, 2017 · 6 comments
Closed

config: StopSignal support #698

cyphar opened this issue Feb 27, 2017 · 6 comments

Comments

@cyphar
Copy link
Member

cyphar commented Feb 27, 2017

Recently in the image-spec (opencontainers/image-spec#544) support for StopSignal was added. As part of opencontainers/image-spec#492 I've been working on trying to make the conversion from Image -> runtime configuration lossless (in one direction at least).

However, currently (as far as I can tell) there isn't a proper way of specifying what signal the runtime should send to the init process. While we define a kill operation, we should probably provide some default which is stored in the config.json to be able to implement this aspect of the conversion?

@runcom
Copy link
Member

runcom commented Feb 27, 2017

My understanding was that each runtime (for any valid reason) has a default signal which varies on platform for instance so it's in charge of defining so and the image config alone (which is platform specific) is the only one to specify the kill signal. (did I miss something?)

@cyphar
Copy link
Member Author

cyphar commented Feb 27, 2017

@runcom While that is mostly right (though the kill operation is defined to also have a <signal> argument which is provided by the user), the point of this is that conversion from the Image configuration (which is generic) to config.json (which is platform-specific) is not possible for StopSignal currently because the runtime specification doesn't provide the necessary options.

I mean, this could just be implemented as an annotation but then it's of less use for users (the system then has to follow the annotation which is the same as adding a field to process).

@dqminh
Copy link
Contributor

dqminh commented Feb 27, 2017

I think being able to set stop-signal as part of process config makes sense. Not every application is intended to be stop with the default sigterm signal.

@wking
Copy link
Contributor

wking commented Feb 27, 2017 via email

@crosbymichael
Copy link
Member

I think it makes sense for the image spec but i'm unsure about having it in the runtime spec. The runtime is mostly imperative with little defaults. It just does what you tell it to do. We only have a default kill signal as a usability thing but if you don't want to use the default then tell it exactly what you want.

Any kill operations inside the runtime always uses SIGKILL and SIGTERM is only the default on the cli command.

Currently the runtime spec has fields for how to create the container and process and nothing about how to manage it after the creation. Adding this inside the spec changes what and I think it would be best to just leave this to the caller and keep the spec simple and focused on the create of the container and process.

@cyphar
Copy link
Member Author

cyphar commented Feb 28, 2017

Alright. I'll make it map to an annotation then.

@cyphar cyphar closed this as completed Feb 28, 2017
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

No branches or pull requests

5 participants