Repository: tensorchord/envd (language: python)

Issue:
chore(CLI): Move destroy logic from docker to envd engine.
## Description

Now the func `Destroy(ctx context.Context, name string) (string, error)` is defined in `pkg/docker/docker.go`

We should move it to pkg/envd/engine.go and implement in pkg/envd/docker.go

---
<!-- Issue Author: Don't delete this message to encourage other users to support your issue! -->
**Message from the maintainers**:

Love this enhancement proposal? Give it a 👍. We prioritise the proposals with the most 👍.



Discussion / hints:
/assign 

Fix patch:
diff --git a/pkg/app/destroy.go b/pkg/app/destroy.go
index 20a9efa3c..9b80e07d3 100644
--- a/pkg/app/destroy.go
+++ b/pkg/app/destroy.go
@@ -74,13 +74,21 @@ func destroy(clicontext *cli.Context) error {
 		}
 		ctrName = filepath.Base(buildContext)
 	}
+	context, err := home.GetManager().ContextGetCurrent()
+	if err != nil {
+		return errors.Wrap(err, "failed to get the current context")
+	}
+	opt := envd.Options{Context: context}
+	envdEngine, err := envd.New(clicontext.Context, opt)
+	if err != nil {
+		return errors.Wrap(err, "failed to create envd engine")
+	}
 
-	if ctrName, err := dockerClient.Destroy(clicontext.Context, ctrName); err != nil {
+	if ctrName, err := envdEngine.Destroy(clicontext.Context, ctrName); err != nil {
 		return errors.Wrapf(err, "failed to destroy the environment: %s", ctrName)
 	} else if ctrName != "" {
 		logrus.Infof("container(%s) is destroyed", ctrName)
 	}
-
 	tags, err := getContainerTag(clicontext, ctrName)
 	if err != nil {
 		return err
diff --git a/pkg/docker/docker.go b/pkg/docker/docker.go
index d15e6780c..c3dec8cbd 100644
--- a/pkg/docker/docker.go
+++ b/pkg/docker/docker.go
@@ -43,7 +43,6 @@ type Client interface {
 	StartBuildkitd(ctx context.Context, tag, name, mirror string) (string, error)
 
 	Exec(ctx context.Context, cname string, cmd []string) error
-	Destroy(ctx context.Context, name string) (string, error)
 
 	GetImageWithCacheHashLabel(ctx context.Context, image string, hash string) (types.ImageSummary, error)
 	RemoveImage(ctx context.Context, image string) error
@@ -179,30 +178,6 @@ func (c generalClient) ResumeContainer(ctx context.Context, name string) (string
 	return name, nil
 }
 
-func (c generalClient) Destroy(ctx context.Context, name string) (string, error) {
-	logger := logrus.WithField("container", name)
-	// Refer to https://docs.docker.com/engine/reference/commandline/container_kill/
-	if err := c.ContainerKill(ctx, name, "KILL"); err != nil {
-		errCause := errors.UnwrapAll(err).Error()
-		switch {
-		case strings.Contains(errCause, "is not running"):
-			// If the container is not running, there is no need to kill it.
-			logger.Debug("container is not running, there is no need to kill it")
-		case strings.Contains(errCause, "No such container"):
-			// If the container is not found, it is already destroyed or the name is wrong.
-			logger.Infof("cannot find container %s, maybe it's already destroyed or the name is wrong", name)
-			return "", nil
-		default:
-			return "", errors.Wrap(err, "failed to kill the container")
-		}
-	}
-
-	if err := c.ContainerRemove(ctx, name, types.ContainerRemoveOptions{}); err != nil {
-		return "", errors.Wrap(err, "failed to remove the container")
-	}
-	return name, nil
-}
-
 func (c generalClient) StartBuildkitd(ctx context.Context, tag, name, mirror string) (string, error) {
 	logger := logrus.WithFields(logrus.Fields{
 		"tag":       tag,
diff --git a/pkg/envd/docker.go b/pkg/envd/docker.go
index e16f2c807..8d66bbccd 100644
--- a/pkg/envd/docker.go
+++ b/pkg/envd/docker.go
@@ -447,6 +447,30 @@ func (e dockerEngine) StartEnvd(ctx context.Context, so StartOptions) (*StartRes
 	return result, nil
 }
 
+func (e dockerEngine) Destroy(ctx context.Context, name string) (string, error) {
+	logger := logrus.WithField("container", name)
+	// Refer to https://docs.docker.com/engine/reference/commandline/container_kill/
+	if err := e.ContainerKill(ctx, name, "KILL"); err != nil {
+		errCause := errors.UnwrapAll(err).Error()
+		switch {
+		case strings.Contains(errCause, "is not running"):
+			// If the container is not running, there is no need to kill it.
+			logger.Debug("container is not running, there is no need to kill it")
+		case strings.Contains(errCause, "No such container"):
+			// If the container is not found, it is already destroyed or the name is wrong.
+			logger.Infof("cannot find container %s, maybe it's already destroyed or the name is wrong", name)
+			return "", nil
+		default:
+			return "", errors.Wrap(err, "failed to kill the container")
+		}
+	}
+
+	if err := e.ContainerRemove(ctx, name, dockertypes.ContainerRemoveOptions{}); err != nil {
+		return "", errors.Wrap(err, "failed to remove the container")
+	}
+	return name, nil
+}
+
 func (e dockerEngine) WaitUntilRunning(ctx context.Context,
 	name string, timeout time.Duration) error {
 	logger := logrus.WithField("container", name)
diff --git a/pkg/envd/engine.go b/pkg/envd/engine.go
index b19164a1e..5128a5cd2 100644
--- a/pkg/envd/engine.go
+++ b/pkg/envd/engine.go
@@ -37,6 +37,7 @@ type EnvironmentClient interface {
 	IsRunning(ctx context.Context, name string) (bool, error)
 	Exists(ctx context.Context, name string) (bool, error)
 	WaitUntilRunning(ctx context.Context, name string, timeout time.Duration) error
+	Destroy(ctx context.Context, name string) (string, error)
 }
 
 type ImageClient interface {
diff --git a/pkg/envd/envdserver.go b/pkg/envd/envdserver.go
index 23cad8f6e..12e1cb5cb 100644
--- a/pkg/envd/envdserver.go
+++ b/pkg/envd/envdserver.go
@@ -37,6 +37,10 @@ func (e *envdServerEngine) ListImage(ctx context.Context) ([]types.EnvdImage, er
 	return nil, errors.New("not implemented")
 }
 
+func (e envdServerEngine) Destroy(ctx context.Context, name string) (string, error) {
+	return "", errors.New("not implemented")
+}
+
 func (e *envdServerEngine) ListImageDependency(ctx context.Context, image string) (*types.Dependency, error) {
 	return nil, errors.New("not implemented")
 }


Test patch:
diff --git a/e2e/cli/build_test.go b/e2e/cli/build_test.go
index 56c13b3d6..f70caf0e0 100644
--- a/e2e/cli/build_test.go
+++ b/e2e/cli/build_test.go
@@ -23,7 +23,9 @@ import (
 	"github.com/tensorchord/envd/e2e"
 	"github.com/tensorchord/envd/pkg/app"
 	"github.com/tensorchord/envd/pkg/docker"
+	"github.com/tensorchord/envd/pkg/envd"
 	"github.com/tensorchord/envd/pkg/home"
+	"github.com/tensorchord/envd/pkg/types"
 )
 
 var _ = Describe("build command", Ordered, func() {
@@ -35,11 +37,14 @@ var _ = Describe("build command", Ordered, func() {
 			e2e.ResetEnvdApp()
 			err := envdApp.Run([]string{"envd.test", "--debug", "bootstrap"})
 			Expect(err).NotTo(HaveOccurred())
-			cli, err := docker.NewClient(context.TODO())
+			_, err = docker.NewClient(context.TODO())
 			Expect(err).NotTo(HaveOccurred())
-			_, err = cli.Destroy(context.TODO(), buildTestName)
+			c := types.Context{Runner: types.RunnerTypeDocker}
+			opt := envd.Options{Context: &c}
+			envdEngine, err := envd.New(context.TODO(), opt)
+			Expect(err).NotTo(HaveOccurred())
+			_, err = envdEngine.Destroy(context.TODO(), buildTestName)
 			Expect(err).NotTo(HaveOccurred())
-
 			envdApp = app.New()
 			args := []string{
 				"envd.test", "--debug", "build", "--path", buildTestName,
@@ -54,11 +59,14 @@ var _ = Describe("build command", Ordered, func() {
 			e2e.ResetEnvdApp()
 			err := envdApp.Run([]string{"envd.test", "--debug", "bootstrap"})
 			Expect(err).NotTo(HaveOccurred())
-			cli, err := docker.NewClient(context.TODO())
+			_, err = docker.NewClient(context.TODO())
 			Expect(err).NotTo(HaveOccurred())
-			_, err = cli.Destroy(context.TODO(), customImageTestName)
+			c := types.Context{Runner: types.RunnerTypeDocker}
+			opt := envd.Options{Context: &c}
+			envdEngine, err := envd.New(context.TODO(), opt)
+			Expect(err).NotTo(HaveOccurred())
+			_, err = envdEngine.Destroy(context.TODO(), buildTestName)
 			Expect(err).NotTo(HaveOccurred())
-
 			envdApp = app.New()
 			args := []string{
 				"envd.test", "--debug", "build", "--path", customImageTestName,
@@ -73,11 +81,13 @@ var _ = Describe("build command", Ordered, func() {
 		e2e.ResetEnvdApp()
 		err := envdApp.Run([]string{"envd.test", "--debug", "bootstrap"})
 		Expect(err).NotTo(HaveOccurred())
-		cli, err := docker.NewClient(context.TODO())
+		_, err = docker.NewClient(context.TODO())
 		Expect(err).NotTo(HaveOccurred())
-		_, err = cli.Destroy(context.TODO(), buildTestName)
+		c := types.Context{Runner: types.RunnerTypeDocker}
+		opt := envd.Options{Context: &c}
+		envdEngine, err := envd.New(context.TODO(), opt)
 		Expect(err).NotTo(HaveOccurred())
-		_, err = cli.Destroy(context.TODO(), customImageTestName)
+		_, err = envdEngine.Destroy(context.TODO(), buildTestName)
 		Expect(err).NotTo(HaveOccurred())
 		// Init DefaultGraph.
 		// ir.DefaultGraph = ir.NewGraph()
diff --git a/e2e/cli/up_test.go b/e2e/cli/up_test.go
index 1115c4827..79212971c 100644
--- a/e2e/cli/up_test.go
+++ b/e2e/cli/up_test.go
@@ -23,7 +23,9 @@ import (
 	"github.com/tensorchord/envd/e2e"
 	"github.com/tensorchord/envd/pkg/app"
 	"github.com/tensorchord/envd/pkg/docker"
+	"github.com/tensorchord/envd/pkg/envd"
 	"github.com/tensorchord/envd/pkg/home"
+	"github.com/tensorchord/envd/pkg/types"
 )
 
 var _ = Describe("up command", Ordered, func() {
@@ -37,10 +39,15 @@ var _ = Describe("up command", Ordered, func() {
 		envdApp := app.New()
 		err := envdApp.Run(append(baseArgs, "bootstrap"))
 		Expect(err).NotTo(HaveOccurred())
-		cli, err := docker.NewClient(context.TODO())
+		_, err = docker.NewClient(context.TODO())
 		Expect(err).NotTo(HaveOccurred())
-		_, err = cli.Destroy(context.TODO(), env)
+		c := types.Context{Runner: types.RunnerTypeDocker}
+		opt := envd.Options{Context: &c}
+		envdEngine, err := envd.New(context.TODO(), opt)
 		Expect(err).NotTo(HaveOccurred())
+		_, err = envdEngine.Destroy(context.TODO(), env)
+		Expect(err).NotTo(HaveOccurred())
+
 	})
 	When("given the right arguments", func() {
 		It("should up and destroy successfully", func() {