Skip to content

Minimum runc shim implementation #24

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

Merged
merged 4 commits into from
Feb 21, 2022
Merged

Conversation

Burning1020
Copy link
Member

Implement shim cli commands and some basic task APIs to run a runc container

Co-authored-by: Feng Shaobao fshb1988@gmail.com
Co-authored-by: Zhang Tianyang burning9699@gmail.com
Co-authored-by: Yu Qitao yuqitao1024@qq.com

Signed-off-by: Zhang Tianyang burning9699@gmail.com

@Burning1020
Copy link
Member Author

Hi, all! I'm from Huawei Cloud Company. My team has implemented a whole runc shim that can truly run a runc container and it works well with containerd and even kubernetes for mounths in production .

Now that having noticed this "rust-extensions" project, we'd like to open our work and provide LTS as well. Actually, this commit is only a part just including some basic tasks(eg. create/start/exec/wait/kill/delete/shutdown...), the other tasks will be brought up if this commit can be accepted. Hope you can take our implementation into consinderations.

You can build it and replace to where the containerd-shim-runc-v2 (go verison) is for your own, use ctr or crictl to run pod/containers then I'm sure you'll get a wonderful experience!

@Burning1020
Copy link
Member Author

This commit has many features as you see, no limited to this:

  1. The completed shim commands(start/delete/default) imps based on your current work
  2. Impls for basic tasks, with runc cli underlay.
  3. A signal handler wording with a static subscribe/nortify monitor and a subreaper
  4. The abstract of ContainerFactory, Container, Process, CommandRuntime and related RuncFactory, RuncContainer, InitProcess/ExitProcess, Runc and so on. (we'll continuous optimize this part to make it more clear)
  5. A checker for process exits and so on

Honestly, this commit may still have defects or something like that:

  1. We've defined a crate level error in error.rs, which is different from your mod level error. The reason is that, it is much complicated to return each mod error layer by layer and there would left many duplicated errors. BTW, our solation is not perfect either, we can discuss this point lately.
  2. Exec process with tty works well in rust 1.48 but in 1.52, the terminal display may be a little unfriendly but that doesn't affect function. I believe there are something wrong in std::io::copy(), I will figure out and push again at the end.
  3. Not enouth UT and comments in our code.

There is still some work needed to be done, perhaps we can develop together!

@Burning1020
Copy link
Member Author

I'll fix the CI checks and continue to add changes lately, don't worry~

@AkihiroSuda
Copy link
Member

Seems a duplicate of #21 and #22

@AkihiroSuda AkihiroSuda added the T-duplicate This issue or pull request already exists label Feb 12, 2022
@fuweid
Copy link
Member

fuweid commented Feb 14, 2022

@AkihiroSuda I don't think it is duplicate 😄 I Checkout this patch and it provides more helpers like go-runc-shim, for instance,fifo scheme support and pure runC commandline. I would like to say it can work with #21 or #27 together. WDYT?

Cc @mxpv

@fuweid fuweid removed the T-duplicate This issue or pull request already exists label Feb 14, 2022
@Burning1020 Burning1020 force-pushed the mini-runc branch 3 times, most recently from 4e93126 to 2794e1f Compare February 15, 2022 17:35
@Burning1020
Copy link
Member Author

I just pushed again and it passed CI finally.
cc plz. @AkihiroSuda @mxpv
@jiangliu plz give review it to avoid same impls within we two

@Burning1020
Copy link
Member Author

When handling container IO, we take a method that pass the stdio from request task to runc directly, which is quite different from pipeIO. Cause we think our solution has higher efficiency than CopyIO in shim layer.

}

impl Container for RuncContainer {
#[cfg(not(feature = "async"))]
Copy link
Member Author

Choose a reason for hiding this comment

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

@mxpv async feature defined in Cargo.toml used here

Implement shim cli commands and some basic task APIs to run a runc container

Co-authored-by: Feng Shaobao fshb1988@gmail.com
Co-authored-by: Zhang Tianyang burning9699@gmail.com
Co-authored-by: Yu Qitao yuqitao1024@qq.com

Signed-off-by: Zhang Tianyang <burning9699@gmail.com>
Signed-off-by: Zhang Tianyang <burning9699@gmail.com>
Signed-off-by: Zhang Tianyang <burning9699@gmail.com>
Signed-off-by: Zhang Tianyang <burning9699@gmail.com>
@Burning1020
Copy link
Member Author

update done. ping @mxpv

Copy link
Member

@mxpv mxpv left a comment

Choose a reason for hiding this comment

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

LGTM

@mxpv
Copy link
Member

mxpv commented Feb 17, 2022

@jiangliu @kzys PTAL?

@mxpv
Copy link
Member

mxpv commented Feb 17, 2022

Should we move runc dependent bits from shim crate to runc-shim ? (container, task).
Not a blocker for this PR, I can follow up on this.

Copy link
Member

@fuweid fuweid left a comment

Choose a reason for hiding this comment

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

LGTM

@Burning1020
Copy link
Member Author

ping @mxpv

@mxpv mxpv merged commit a4e923d into containerd:main Feb 21, 2022
@Burning1020
Copy link
Member Author

thanks for reviewing! @mxpv @fuweid @kzys

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.

6 participants