add containerd runtime #1248

tianyax · 2023-02-27T05:09:56Z

Description

add containerd runtime support.

# Install Dapr
>>> dapr init --container-runtime containerd --network mynet
⌛  Making the jump to hyperspace...
ℹ️  Container images will be pulled from Docker Hub
ℹ️  Installing runtime version 1.10.2
↖  Downloading binaries and setting up components...
✅  Downloading binaries and setting up components...
✅  Downloaded binaries and completed components set up.
ℹ️  daprd binary has been installed to /home/x.linux/.dapr/bin.
ℹ️  dapr_placement_mynet container is running.
ℹ️  dapr_redis_mynet container is running.
ℹ️  dapr_zipkin_mynet container is running.
ℹ️  Use `nerdctl ps` to check running containers.
✅  Success! Dapr is up and running. To get started, go here: https://aka.ms/dapr-getting-started

# Show all the containers
>>> nerdctl ps
CONTAINER ID    IMAGE                                 COMMAND                   CREATED               STATUS    PORTS    NAMES
55f8cd7ad1ad    docker.io/daprio/dapr:1.10.2          "./placement"             About a minute ago    Up                 dapr_placement_mynet
6c4a54236073    docker.io/openzipkin/zipkin:latest    "start-zipkin"            About a minute ago    Up                 dapr_zipkin_mynet
b94b3d45d466    docker.io/library/redis:6             "docker-entrypoint.s…"    About a minute ago    Up                 dapr_redis_mynet

# Uninstall dapr
>>> dapr uninstall --container-runtime containerd --network mynet

Issue reference

Please reference the issue this PR will close: #1232

Checklist

Please make sure you've completed the relevant tasks for this PR, out of the following list:

Code compiles correctly
Created/updated tests
Extended the documentation

Signed-off-by: tianya <[email protected]>

cmd/init.go

cmd/uninstall.go

utils/utils.go

Co-authored-by: Pravin Pushkar <[email protected]> Signed-off-by: tianya <[email protected]>

Signed-off-by: tianya <[email protected]>

utils/utils.go

Signed-off-by: tianya <[email protected]>

pravinpushkar · 2023-02-28T15:14:21Z

@tianyax Were you able to verify running dapr init with containerd on Macos?

codecov · 2023-02-28T15:15:08Z

Codecov Report

Merging #1248 (c22aa11) into master (4e3ae81) will increase coverage by 0.01%.
The diff coverage is 0.00%.

@@            Coverage Diff             @@
##           master    #1248      +/-   ##
==========================================
+ Coverage   27.26%   27.28%   +0.01%     
==========================================
  Files          38       38              
  Lines        3715     3713       -2     
==========================================
  Hits         1013     1013              
+ Misses       2635     2633       -2     
  Partials       67       67

Impacted Files	Coverage Δ
pkg/standalone/standalone.go	`4.46% <0.00%> (ø)`
pkg/standalone/uninstall.go	`0.00% <0.00%> (ø)`

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

tianyax · 2023-03-01T07:49:26Z

@tianyax Were you able to verify running dapr init with containerd on Macos?

@pravinpushkar Okay, I'll update the workflows later.

pravinpushkar · 2023-03-01T11:01:48Z

@tianyax Were you able to verify running dapr init with containerd on Macos?

@pravinpushkar Okay, I'll update the workflows later.

I meant locally.

However, should we try updating the workflow for Mac using containerd or have both podman and containerd @mukundansundar

mukundansundar · 2023-03-01T13:02:25Z

@tianyax Were you able to verify running dapr init with containerd on Macos?

@pravinpushkar Okay, I'll update the workflows later.

I meant locally.

However, should we try updating the workflow for Mac using containerd or have both podman and containerd @mukundansundar

If possible both.

tianyax · 2023-03-02T06:23:16Z

@tianyax Were you able to verify running dapr init with containerd on Macos?

@pravinpushkar Okay, I'll update the workflows later.

I meant locally.

However, should we try updating the workflow for Mac using containerd or have both podman and containerd @mukundansundar

@pravinpushkar I have already verified it on my mac, including lima, colima and rancher-desktop.
This is the result after I cleared the environment and re-run with colima as an example.

pravinpushkar · 2023-03-02T06:58:13Z

@tianyax Thanks for verifying locally. Please resolve the conflicts in the PR .

Signed-off-by: tianya <[email protected]>

shubham1172

Can we also add e2e tests with containerd?

README.md

shubham1172 · 2023-03-21T06:21:37Z

README.md

+#### If you want to use podman as a runtime, then please refer to the installation.
+
+* Install [Podman](https://podman-desktop.io/docs/Installation)
+
+#### If you want to use containerd as a runtime, then please refer to the installation.


This can be simpler.

Choosing a container runtime

Docker (recommended for new users)

Docker instructions

Podman

Podman instructions

Containerd

Containerd instructions

Okay, thanks very much!

cmd/uninstall.go

pkg/standalone/standalone.go

utils/utils.go

tianyax · 2023-03-22T08:20:58Z

Can we also add e2e tests with containerd?

@shubham1172 containerd just provides a runtime, the e2e test cases can be equivalent to docker, I think we can add the containerd job to the self-host-e2e workflow to do e2e tests, if that's what you mean, I try to update it.

shubham1172 · 2023-03-22T08:38:11Z

I think we can add the containerd job to the self-host-e2e workflow to do e2e tests, if that's what you mean, I try to update it.

@tianyax, absolutely!

Co-authored-by: Shubham Sharma <[email protected]> Signed-off-by: tianya <[email protected]>

Signed-off-by: tianya <[email protected]>

tianyax · 2023-03-27T14:22:04Z

@shubham1172 @pravinpushkar According to shubham1172's suggestion I modified README and added e2e tests for containerd runtime. But the job timed out and I am looking for the cause, I suspect that the command dapr init ... is invoked in go that does not get the result and make the program waiting. No problem using dapr init ... locally

shubham1172 · 2023-03-27T14:39:49Z

@shubham1172 @pravinpushkar According to shubham1172's suggestion I modified README and added e2e tests for containerd runtime. But the job timed out and I am looking for the cause, I suspect that the command dapr init ... is invoked in go that does not get the result and make the program waiting. No problem using dapr init ... locally

Hi @tianyax, are you able to run the e2e test locally?

pravinpushkar · 2023-03-28T05:28:28Z

Yeah, better to run the test locally to debug it. Also, any reason why enable it only for macos and not windows and linux ?

tianyax · 2023-03-28T08:17:02Z

@shubham1172 @pravinpushkar According to shubham1172's suggestion I modified README and added e2e tests for containerd runtime. But the job timed out and I am looking for the cause, I suspect that the command dapr init ... is invoked in go that does not get the result and make the program waiting. No problem using dapr init ... locally

Hi @tianyax, are you able to run the e2e test locally?

@shubham1172 Yes, I have no problem running e2e tests locally, but my colima environment is not freshly installed, it has been running for a while, I will clean it and do a retest.

tianyax · 2023-03-28T08:38:25Z

Yeah, better to run the test locally to debug it. Also, any reason why enable it only for macos and not windows and linux ?

@pravinpushkar Yes, I will be debug it locally. About why enable it only for macos, I saw that other runtimes just chose a platform to do the test. I think there may be some reasons for this. I am using macos now, so I chose macos to do the test. After solving this problem I will add. 😄️

pravinpushkar · 2023-03-28T09:28:53Z

@pravinpushkar Yes, I will be debug it locally. About why enable it only for macos, I saw that other runtimes just chose a platform to do the test. I think there may be some reasons for this. I am using macos now, so I chose macos to do the test. After solving this problem I will add. 😄️

Basically we recently added Podman support and macos runner has some issues with docker and windows runner has some difficulties with Podman, thats why the current setup looks like this. I was just thinking if containerd has good support for all then we can add for all os and exclude what is not supported.

tianyax · 2023-03-29T07:15:51Z

@shubham1172 @pravinpushkar According to shubham1172's suggestion I modified README and added e2e tests for containerd runtime. But the job timed out and I am looking for the cause, I suspect that the command dapr init ... is invoked in go that does not get the result and make the program waiting. No problem using dapr init ... locally

@shubham1172 @pravinpushkar The problem is that the utils.RunCmdAndWait method causes blocking.

func RunCmdAndWait(name string, args ...string) (string, error) {
    cmd := exec.Command(name, args...)

    stdout, err := cmd.StdoutPipe()
    if err != nil {
        return "", err
    }
    stderr, err := cmd.StderrPipe()
    if err != nil {
        return "", err
    }

    err = cmd.Start()
    if err != nil {
        return "", err
    }

    resp, err := io.ReadAll(stdout)  // Blocking here!
    if err != nil {
        return "", err
    }
    errB, err := io.ReadAll(stderr)
    if err != nil {
        //nolint
        return "", nil
    }

    err = cmd.Wait()
    ...

Command cannot get a response from StdoutPipe, I wrote a simple test and occasionally this happens.

RunCmdAndWait("nerdctl", "run", "--name", "dapr_redis", "-d", "redis:6")

But why no data is written to the standard output I do not know yet.. 😅 If the image already exists there is no problem.

shubham1172 · 2023-03-29T07:44:51Z

@shubham1172 @pravinpushkar According to shubham1172's suggestion I modified README and added e2e tests for containerd runtime. But the job timed out and I am looking for the cause, I suspect that the command dapr init ... is invoked in go that does not get the result and make the program waiting. No problem using dapr init ... locally

@shubham1172 @pravinpushkar The problem is that the utils.RunCmdAndWait method causes blocking.
func RunCmdAndWait(name string, args ...string) (string, error) {
    cmd := exec.Command(name, args...)

    stdout, err := cmd.StdoutPipe()
    if err != nil {
        return "", err
    }
    stderr, err := cmd.StderrPipe()
    if err != nil {
        return "", err
    }

    err = cmd.Start()
    if err != nil {
        return "", err
    }

    resp, err := io.ReadAll(stdout)  // Blocking here!
    if err != nil {
        return "", err
    }
    errB, err := io.ReadAll(stderr)
    if err != nil {
        //nolint
        return "", nil
    }

    err = cmd.Wait()
    ...
Command cannot get a response from StdoutPipe, I wrote a simple test and occasionally this happens.
RunCmdAndWait("nerdctl", "run", "--name", "dapr_redis", "-d", "redis:6")
But why no data is written to the standard output I do not know yet.. 😅 If the image already exists there is no problem.

Is it because the output is not going to stdout? I see a related issue here containerd/nerdctl#1685

shubham1172 · 2023-04-05T04:32:44Z

@tianyax did you get a chance to have a look at the issue?

tianyax · 2023-04-06T04:27:24Z

@tianyax did you get a chance to have a look at the issue?

@shubham1172 I think this problem is caused by lima/colima rather than nerdctl, I opened a discussion lima-vm/lima#1454, the maintainer suggested to use cmd.Stdout = &stdout to solve this problem, and I responded to it.

Signed-off-by: tianya <[email protected]>

mukundansundar · 2023-04-19T16:43:48Z

@tianyax I am seeing that this PR has been moved to draft ...
Do you have an ETA for this feature?

tianyax · 2023-04-21T15:53:46Z

@tianyax I am seeing that this PR has been moved to draft ... Do you have an ETA for this feature?

@mukundansundar There is a new problem here, using nerdctl to quickly create and delete containers will cause errors containerd/nerdctl#2193. this is occasional in e2e tests (with a few retries have completed the full test case), and it may take some time to fix this bug. so I updated the PR to draft, this feature may need more rigorous and comprehensive testing, otherwise I think it needs to be supported with caution at present😅.. I'm trying to find the cause of the nerdctl error and fix it.

dapr-bot · 2023-05-21T15:54:13Z

This pull request has been automatically marked as stale because it has not had activity in the last 30 days. It will be closed in 7 days if no further activity occurs. Please feel free to give a status update now, ping for review, or re-open when it's ready. Thank you for your contributions!

mukundansundar · 2023-06-02T07:26:49Z

@tianyax Do you have any updates on this PR?

tianyax · 2023-06-05T07:08:56Z

@tianyax Do you have any updates on this PR?

@mukundansundar Waiting for containerd/containerd#8550

tianyax added 4 commits February 25, 2023 22:35

support nerdctl+containerd runtime (dapr#1232)

49433fe

Signed-off-by: tianya <[email protected]>

add containerd runtime validity check and fix test errors

33e17ee

Signed-off-by: tianya <[email protected]>

reduce unnecessary container runtime availability checks

cbcb2af

Signed-off-by: tianya <[email protected]>

fixed test failed on macos

9200b3d

Signed-off-by: tianya <[email protected]>

tianyax requested review from a team as code owners February 27, 2023 05:09