fix: Optimize err when jupyter port is already allocated (#224)

* fix: Optimize err

Signed-off-by: Ce Gao <cegao@tensorchord.ai>

* fix: Use switch

Signed-off-by: Ce Gao <cegao@tensorchord.ai>

* fix: Fix paths

Signed-off-by: Ce Gao <cegao@tensorchord.ai>

.github/workflows/CI.yml

@@ -6,15 +6,13 @@ on:
       - main
     paths:
       - '.github/workflows/**'
-      - 'cmd/**'
-      - 'pkg/**'
+      - '**.go'
       - 'Makefile'
   pull_request:
     types: [review_requested, ready_for_review]
     paths:
       - '.github/workflows/**'
-      - 'cmd/**'
-      - 'pkg/**'
+      - '**.go'
       - 'Makefile'
     branches:
       - main

cmd/envd/build_test.go

@@ -33,7 +33,8 @@ var _ = Describe("build command", func() {
 		Expect(home.Initialize()).NotTo(HaveOccurred())
 		cli, err := docker.NewClient(context.TODO())
 		Expect(err).NotTo(HaveOccurred())
-		_ = cli.Destroy(context.TODO(), buildContext)
+		_, err = cli.Destroy(context.TODO(), buildContext)
+		Expect(err).NotTo(HaveOccurred())
 	})
 	When("given the right arguments", func() {
 		It("should build successfully", func() {

cmd/envd/destroy.go

@@ -55,14 +55,15 @@ func destroy(clicontext *cli.Context) error {
 
 	ctr := fileutil.Base(buildContext)
 
-	if err := dockerClient.Destroy(clicontext.Context, ctr); err != nil {
+	if name, err := dockerClient.Destroy(clicontext.Context, ctr); err != nil {
 		return errors.Wrapf(err, "failed to destroy the environment: %s", ctr)
+	} else if name != "" {
+		logrus.Infof("%s is destroyed", name)
 	}
 
 	if err = sshconfig.RemoveEntry(ctr); err != nil {
 		logrus.Infof("failed to remove entry %s from your SSH config file: %s", ctr, err)
 		return errors.Wrap(err, "failed to remove entry from your SSH config file")
 	}
-	logrus.Info("envd environment destroyed")
 	return nil
 }

cmd/envd/up_test.go

@@ -33,7 +33,8 @@ var _ = Describe("up command", func() {
 		Expect(home.Initialize()).NotTo(HaveOccurred())
 		cli, err := docker.NewClient(context.TODO())
 		Expect(err).NotTo(HaveOccurred())
-		_ = cli.Destroy(context.TODO(), buildContext)
+		_, err = cli.Destroy(context.TODO(), buildContext)
+		Expect(err).NotTo(HaveOccurred())
 	})
 	When("given the right arguments", func() {
 		It("should up and destroy successfully", func() {

pkg/docker/docker.go

@@ -53,7 +53,7 @@ type Client interface {
 	IsCreated(ctx context.Context, name string) (bool, error)
 	WaitUntilRunning(ctx context.Context, name string, timeout time.Duration) error
 	Exec(ctx context.Context, cname string, cmd []string) error
-	Destroy(ctx context.Context, name string) error
+	Destroy(ctx context.Context, name string) (string, error)
 	List(ctx context.Context) ([]types.Container, error)
 	// GPUEnabled returns true if nvidia container runtime exists in docker daemon.
 	GPUEnabled(ctx context.Context) (bool, error)
@@ -124,15 +124,28 @@ func (c generalClient) List(ctx context.Context) ([]types.Container, error) {
 	return ctrs, nil
 }
 
-func (c generalClient) Destroy(ctx context.Context, name string) error {
+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 {
-		return errors.Wrap(err, "failed to kill the container")
+		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.
+			logger.Debug("container is not found, there is no need to destroy it")
+			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 "", errors.Wrap(err, "failed to remove the container")
 	}
-	return nil
+	return name, nil
 }
 
 func (g generalClient) StartBuildkitd(ctx context.Context,
@@ -272,20 +285,26 @@ func (c generalClient) StartEnvd(ctx context.Context, tag, name, buildContext st
 
 	resp, err := c.ContainerCreate(ctx, config, hostConfig, nil, nil, name)
 	if err != nil {
-		return "", "", err
+		return "", "", errors.Wrap(err, "failed to create the container")
 	}
 
 	for _, w := range resp.Warnings {
 		logger.Warnf("run with warnings: %s", w)
 	}
 
 	if err := c.ContainerStart(ctx, resp.ID, types.ContainerStartOptions{}); err != nil {
-		return "", "", err
+		errCause := errors.UnwrapAll(err)
+		// Hack to check if the port is already allocated.
+		if strings.Contains(errCause.Error(), "port is already allocated") {
+			logrus.Debugf("failed to allocate the port: %s", err)
+			return "", "", errors.New("jupyter port is already allocated in the host")
+		}
+		return "", "", errors.Wrap(err, "failed to run the container")
 	}
 
 	container, err := c.ContainerInspect(ctx, resp.ID)
 	if err != nil {
-		return "", "", err
+		return "", "", errors.Wrap(err, "failed to inpsect the container")
 	}
 
 	if err := c.WaitUntilRunning(