diff --git a/.github/workflows/CI.yml b/.github/workflows/CI.yml index ba0aa3efd..a2f982d93 100644 --- a/.github/workflows/CI.yml +++ b/.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 diff --git a/cmd/envd/build_test.go b/cmd/envd/build_test.go index d1e75aa5c..cc5b4f051 100644 --- a/cmd/envd/build_test.go +++ b/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() { diff --git a/cmd/envd/destroy.go b/cmd/envd/destroy.go index 288ea6cbb..34e8d30a3 100644 --- a/cmd/envd/destroy.go +++ b/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 } diff --git a/cmd/envd/up_test.go b/cmd/envd/up_test.go index 97af44f7b..2385e4c67 100644 --- a/cmd/envd/up_test.go +++ b/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() { diff --git a/pkg/docker/docker.go b/pkg/docker/docker.go index ceca9ee94..ab3b94b32 100644 --- a/pkg/docker/docker.go +++ b/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,7 +285,7 @@ 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 { @@ -280,12 +293,18 @@ func (c generalClient) StartEnvd(ctx context.Context, tag, name, buildContext st } 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(