dapr / cli

Command-line tools for Dapr.
Apache License 2.0
315 stars 200 forks source link

Adding warning for uninstall --all while container runtime not running #1310

Closed mcaupybugs closed 1 year ago

mcaupybugs commented 1 year ago

Description

Added an else if condition to check the case when the dapr uninstall --all command is used but the container runtime is not running. In such a case, this code change will throw a warning stating that it could not remove the supporting containers.

Issue reference

We strive to have all PR being opened based on an issue, where the problem or feature have been discussed prior to implementation.

Please reference the issue this PR will close: #1288

Checklist

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

codecov[bot] commented 1 year ago

Codecov Report

Merging #1310 (93bd112) into master (fa2c99d) will decrease coverage by 0.06%. The diff coverage is 0.00%.

@@            Coverage Diff             @@
##           master    #1310      +/-   ##
==========================================
- Coverage   26.87%   26.82%   -0.06%     
==========================================
  Files          39       39              
  Lines        3873     3881       +8     
==========================================
  Hits         1041     1041              
- Misses       2758     2766       +8     
  Partials       74       74              
Impacted Files Coverage Δ
pkg/standalone/list.go 0.00% <0.00%> (ø)
pkg/standalone/uninstall.go 0.00% <0.00%> (ø)
mcaupybugs commented 1 year ago

Hey @shubham1172, I am not sure what E2E test should be written. Like I started to implement but after looking at the existing E2E tests I realized that each of those requires container runtime to be in the running state and is the first check done but my test case would require non-running container runtime. And if I try to manually give invalid container runtime using a --container-runtime flag then it will be blocked at the container-runtime validation step which is the IsValidContainerRuntime function and it checks if the runtime flag provided is either docker or podman. So not sure if i can even write E2E test for this scenario. please let me know wdyt

shubham1172 commented 1 year ago

Hi @mcaupybugs, you could stop the docker daemon using os/exec. Did you try that? Whilst doing it, we need to make sure that no other test is running in parallel. Although at this point I feel that it's non-trivial, so I am creating a separate issue (https://github.com/dapr/cli/issues/1320) to track it and approving this PR. Thanks again for the PR! :)