[Do Not Merge]: Track runc missing options and commands #1613

c3d · 2023-03-03T15:56:09Z

DO NOT MERGE - FOR REFERENCE / CHERRY-PICKING ONLY

Add the missing command-line options and subcommands in liboci-cli, as documented for runc, including:

Missing options for checkpoint
Missing no_pivot option for create
Missing options for exec
Missing options for run
Missing options for update
Missing options for spec

Also add the missing undocumented features subcommand from runc

(This does not mean that they are necessarily implemented)

@adrianreber In case you need to sync the options for this command.

Context: The ociplex tool needs the additional options to be able to pass them down to some other runtime.

c3d · 2023-03-03T15:57:26Z

@adrianreber For context, this is part of the work I am doing on ociplex, and attempt to restore podman compatibility for Kata Containers using Youki as an in-between, see https://github.com/c3d/youki/tree/ociplex.

c3d · 2023-03-03T15:58:05Z

crates/liboci-cli/src/checkpoint.rs

+    pub lazy_pages: bool,
+    /// Pass a file descriptor fd to criu
+    #[clap(long)]
+    pub status_fd: Option<u32>, // TODO: Is u32 the right type?


For what it's worth, I am not sure how file descriptors should be passed around in that context.

adrianreber · 2023-03-03T17:46:33Z

Not sure this is a good idea. Podman tries to figure out certain features of crun and runc by parsing the help output. The expectation is, that if it is listed in the help output it is supported.

In the case of youki it is not really important as checkpoint is still hidden behind checkpointt.

c3d · 2023-03-03T21:51:05Z

Not sure this is a good idea. Podman tries to figure out certain features of crun and runc by parsing the help output. The expectation is, that if it is listed in the help output it is supported.

In the case of youki it is not really important as checkpoint is still hidden behind checkpointt.

I was not aware of this behaviour of podman. That's weird 😄 . Well, I need these changes on my branch, feel free to pick them up if/when you implement that. I'll also figure out a way to have this be different for ociplex and youki.

adrianreber · 2023-03-04T09:42:11Z

Not sure this is a good idea. Podman tries to figure out certain features of crun and runc by parsing the help output. The expectation is, that if it is listed in the help output it is supported.
In the case of youki it is not really important as checkpoint is still hidden behind checkpointt.

I was not aware of this behaviour of podman. That's weird smile . Well, I need these changes on my branch, feel free to pick them up if/when you implement that. I'll also figure out a way to have this be different for ociplex and youki.

It is only during restore: https://github.com/containers/podman/blob/main/pkg/checkpoint/crutils/checkpoint_restore_utils.go#L229

c3d · 2023-03-06T10:12:22Z

@adrianreber OK, I understand your constraints. In any case, the changes in this PR are not for immediate consumption. It's more "If you need to implement this or that option, feel free to grab it from this branch". I will try to maintain two parallel branches:

The oci-changes branch, where I'll try to keep only the OCI command-line interface changes
The ociplex branch where we are developing an OCI multiplexer that leverages youki's command-line parsing, the primary goal being to build a bridge between podman and shimv2-only runtimes like Kata Containers v2 and later.

I'll try to reorder and cleanup the commits in these branches to rebase ociplex on oci-changes. In case there is no negative impact of adding the missing options. It's really your call if and when you want to do that.

codecov-commenter · 2023-03-07T02:35:05Z

Codecov Report

Merging #1613 (a191eb7) into main (74a499f) will decrease coverage by 0.10%.
The diff coverage is 0.00%.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1613      +/-   ##
==========================================
- Coverage   68.83%   68.74%   -0.10%     
==========================================
  Files         120      122       +2     
  Lines       13109    13197      +88     
==========================================
+ Hits         9024     9072      +48     
- Misses       4085     4125      +40

utam0k · 2023-03-07T02:35:57Z

Not sure this is a good idea. Podman tries to figure out certain features of crun and runc by parsing the help output. The expectation is, that if it is listed in the help output it is supported.

In the case of youki it is not really important as checkpoint is still hidden behind checkpointt.

BTW, podman should use the feature command to verify the futures of the oci runtimes
opencontainers/runtime-spec#1130

c3d · 2023-03-07T09:57:00Z

On 7 Mar 2023, at 03:36, Toru Komatsu ***@***.***> wrote: Not sure this is a good idea. Podman tries to figure out certain features of crun and runc by parsing the help output. The expectation is, that if it is listed in the help output it is supported. In the case of youki it is not really important as checkpoint is still hidden behind checkpointt. BTW, podman should use the feature command to verify the futures of the oci runtimes opencontainers/runtime-spec#1130

Interesting. I was not aware of this command, and it’s not implemented in Youki yet, but it's tracked. #815

Add the missing command-line options as documented for runc, and also reorder the options to match the documentation: https://github.com/opencontainers/runc/blob/main/man/runc-checkpoint.8.md (This does not mean that they are necessarily implemented) Signed-off-by: Christophe de Dinechin <dinechin@redhat.com>

The --no-pivot option is documented in https://github.com/opencontainers/runc/blob/main/man/runc-create.8.md Also change the options order in order to match the doc, this makes the code a bit easier to maintain. Signed-off-by: Christophe de Dinechin <dinechin@redhat.com>

utam0k · 2023-03-07T23:02:55Z

crates/liboci-cli/src/features.rs

@@ -0,0 +1,7 @@
+use clap::Parser;


May I ask you to update the README for liboci-cli?
https://github.com/containers/youki/tree/main/crates/liboci-cli

utam0k

I apologize for the inconvenience, but for commands that are not yet implemented, you should make it clear in the prefix in the comment section of each option.
Then they should appear in the help command.

c3d · 2023-03-08T08:53:04Z

@utam0k Not an inconvenience at all. Thanks for pointing that out, I'll work on it.

Add the missing command-line options for the exec subcommand. Reference: https://github.com/opencontainers/runc/blob/main/man/runc-exec.8.md Signed-off-by: Christophe de Dinechin <dinechin@redhat.com>

Also change the order to match the documentation in https://github.com/opencontainers/runc/blob/main/man/runc-run.8.md Signed-off-by: Christophe de Dinechin <dinechin@redhat.com>

Add command-line options as documented in https://github.com/opencontainers/runc/blob/main/man/runc-update.8.md Signed-off-by: Christophe de Dinechin <dinechin@redhat.com>

Add the missing bundle option, as documented in https://github.com/opencontainers/runc/blob/main/man/runc-spec.8.md Signed-off-by: Christophe de Dinechin <dinechin@redhat.com>

The 'features' subcommand is not publicly documented yet, but it was introduced in `runc` in opencontainers/runc#3296. Signed-off-by: Christophe de Dinechin <dinechin@redhat.com>

The `features` subcommand is implemented in `runc`, but not documented. See opencontainers/runc#3296 Signed-off-by: Christophe de Dinechin <dinechin@redhat.com> Suggested-by: Toru Komatsu <k0ma@utam0k.jp>

Add the command-line options documented in https://github.com/opencontainers/runc/blob/main/man/runc-list.8.md Signed-off-by: Christophe de Dinechin <dinechin@redhat.com>

yihuaf · 2024-05-20T04:42:17Z

Given this PR is stale for more than a year, I will close this. Please re-open when needed.

c3d commented Mar 3, 2023

View reviewed changes

c3d force-pushed the oci-changes branch from 8202ba8 to 43bbdfb Compare March 3, 2023 22:08

c3d force-pushed the oci-changes branch from 43bbdfb to 312700d Compare March 7, 2023 17:50

c3d force-pushed the oci-changes branch 3 times, most recently from d51aeaf to 173b011 Compare March 7, 2023 18:44

utam0k reviewed Mar 7, 2023

View reviewed changes

utam0k requested changes Mar 7, 2023

View reviewed changes

c3d changed the title ~~checkpoint: Add the missing options~~ DNM: Track runc missing options and commands Mar 8, 2023

c3d added 5 commits March 8, 2023 10:15

exec: Add missing command-line options

6d21344

Add the missing command-line options for the exec subcommand. Reference: https://github.com/opencontainers/runc/blob/main/man/runc-exec.8.md Signed-off-by: Christophe de Dinechin <dinechin@redhat.com>

run: Add missing options to run subcommand

126d3d9

Also change the order to match the documentation in https://github.com/opencontainers/runc/blob/main/man/runc-run.8.md Signed-off-by: Christophe de Dinechin <dinechin@redhat.com>

update: Add missing options to the update subcommand

a4ab99b

Add command-line options as documented in https://github.com/opencontainers/runc/blob/main/man/runc-update.8.md Signed-off-by: Christophe de Dinechin <dinechin@redhat.com>

spec: Add options for the spec subcommand

684667c

Add the missing bundle option, as documented in https://github.com/opencontainers/runc/blob/main/man/runc-spec.8.md Signed-off-by: Christophe de Dinechin <dinechin@redhat.com>

features: Add features subcommand

4a0546c

The 'features' subcommand is not publicly documented yet, but it was introduced in `runc` in opencontainers/runc#3296. Signed-off-by: Christophe de Dinechin <dinechin@redhat.com>

c3d force-pushed the oci-changes branch from 173b011 to 4a0546c Compare March 8, 2023 09:15

c3d added 2 commits March 8, 2023 10:29

liboci-cli: Update README with information about features subcommand

a191eb7

The `features` subcommand is implemented in `runc`, but not documented. See opencontainers/runc#3296 Signed-off-by: Christophe de Dinechin <dinechin@redhat.com> Suggested-by: Toru Komatsu <k0ma@utam0k.jp>

list: Add missing command-line options

3d91cdb

Add the command-line options documented in https://github.com/opencontainers/runc/blob/main/man/runc-list.8.md Signed-off-by: Christophe de Dinechin <dinechin@redhat.com>

utam0k marked this pull request as draft March 10, 2023 06:47

yihuaf changed the title ~~DNM: Track runc missing options and commands~~ [Do Not Merge]: Track runc missing options and commands Jun 10, 2023

yihuaf closed this May 20, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Do Not Merge]: Track runc missing options and commands #1613

[Do Not Merge]: Track runc missing options and commands #1613

c3d commented Mar 3, 2023 •

edited

c3d commented Mar 3, 2023

c3d Mar 3, 2023

adrianreber commented Mar 3, 2023

c3d commented Mar 3, 2023

adrianreber commented Mar 4, 2023

c3d commented Mar 6, 2023

codecov-commenter commented Mar 7, 2023 •

edited

utam0k commented Mar 7, 2023

c3d commented Mar 7, 2023 via email

utam0k Mar 7, 2023

utam0k left a comment

c3d commented Mar 8, 2023

yihuaf commented May 20, 2024

[Do Not Merge]: Track runc missing options and commands #1613

[Do Not Merge]: Track runc missing options and commands #1613

Conversation

c3d commented Mar 3, 2023 • edited

c3d commented Mar 3, 2023

c3d Mar 3, 2023

Choose a reason for hiding this comment

adrianreber commented Mar 3, 2023

c3d commented Mar 3, 2023

adrianreber commented Mar 4, 2023

c3d commented Mar 6, 2023

codecov-commenter commented Mar 7, 2023 • edited

Codecov Report

utam0k commented Mar 7, 2023

c3d commented Mar 7, 2023 via email

utam0k Mar 7, 2023

Choose a reason for hiding this comment

utam0k left a comment

Choose a reason for hiding this comment

c3d commented Mar 8, 2023

yihuaf commented May 20, 2024

c3d commented Mar 3, 2023 •

edited

codecov-commenter commented Mar 7, 2023 •

edited