cli icon indicating copy to clipboard operation
cli copied to clipboard

add containerd runtime

Open tianyax opened this issue 2 years ago • 26 comments

Description

For #1232

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:

  • [x] Code compiles correctly
  • [x] Created/updated tests
  • [x] Extended the documentation

tianyax avatar Feb 27 '23 05:02 tianyax

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

pravinpushkar avatar Feb 28 '23 15:02 pravinpushkar

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.

codecov[bot] avatar Feb 28 '23 15:02 codecov[bot]

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

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

tianyax avatar Mar 01 '23 07:03 tianyax

@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 avatar Mar 01 '23 11:03 pravinpushkar

@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.

mukundansundar avatar Mar 01 '23 13:03 mukundansundar

@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. image

tianyax avatar Mar 02 '23 06:03 tianyax

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

pravinpushkar avatar Mar 02 '23 06:03 pravinpushkar

@tianyax There is small merge conflict that needs to be resolved.

pravinpushkar avatar Mar 17 '23 06:03 pravinpushkar

@tianyax There is small merge conflict that needs to be resolved.

@pravinpushkar resolved.

tianyax avatar Mar 17 '23 07:03 tianyax

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.

tianyax avatar Mar 22 '23 08:03 tianyax

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!

shubham1172 avatar Mar 22 '23 08:03 shubham1172

@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

tianyax avatar Mar 27 '23 14:03 tianyax

@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 avatar Mar 27 '23 14:03 shubham1172

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 avatar Mar 28 '23 05:03 pravinpushkar

@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. image

tianyax avatar Mar 28 '23 08:03 tianyax

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. 😄️

tianyax avatar Mar 28 '23 08:03 tianyax

@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.

pravinpushkar avatar Mar 28 '23 09:03 pravinpushkar

@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.

tianyax avatar Mar 29 '23 07:03 tianyax

@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 https://github.com/containerd/nerdctl/issues/1685

shubham1172 avatar Mar 29 '23 07:03 shubham1172

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

shubham1172 avatar Apr 05 '23 04:04 shubham1172

@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 https://github.com/lima-vm/lima/discussions/1454, the maintainer suggested to use cmd.Stdout = &stdout to solve this problem, and I responded to it.

tianyax avatar Apr 06 '23 04:04 tianyax

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

mukundansundar avatar Apr 19 '23 16:04 mukundansundar

@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 https://github.com/containerd/nerdctl/discussions/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.

tianyax avatar Apr 21 '23 15:04 tianyax

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!

dapr-bot avatar May 21 '23 15:05 dapr-bot

@tianyax Do you have any updates on this PR?

mukundansundar avatar Jun 02 '23 07:06 mukundansundar

@tianyax Do you have any updates on this PR?

@mukundansundar Waiting for https://github.com/containerd/containerd/pull/8550

tianyax avatar Jun 05 '23 07:06 tianyax