From 5c3ee307604c806fe10fb5ac7b8042616f49ad7b Mon Sep 17 00:00:00 2001 From: Nikolay Edigaryev Date: Fri, 25 Sep 2020 15:29:16 +0300 Subject: [PATCH] Option to keep containers after execution for debugging (#109) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Differentiate additional container logging * Option to keep containers after execution for debugging Resolves #8. * Debugf → Infof Co-authored-by: Fedor Korotkov * Debugf → Infof Co-authored-by: Fedor Korotkov * Debugf → Infof Co-authored-by: Fedor Korotkov * Debugf → Infof Co-authored-by: Fedor Korotkov Co-authored-by: Fedor Korotkov --- internal/commands/run.go | 16 ++++-- internal/commands/run_test.go | 56 +++++++++++++++++++ .../testdata/run-no-cleanup/.cirrus.yml | 10 ++++ internal/executor/instance/container.go | 7 +++ internal/executor/instance/instance.go | 51 ++++++++++++----- internal/executor/instance/pipe.go | 7 +++ internal/executor/options/docker.go | 1 + 7 files changed, 128 insertions(+), 20 deletions(-) create mode 100644 internal/commands/testdata/run-no-cleanup/.cirrus.yml diff --git a/internal/commands/run.go b/internal/commands/run.go index 5d24a1a6..80279680 100644 --- a/internal/commands/run.go +++ b/internal/commands/run.go @@ -34,6 +34,9 @@ var verbose bool // Docker-related flags. var dockerNoPull bool +// Flags useful for debugging. +var debugNoCleanup bool + // Experimental features flags. var experimentalParser bool @@ -218,11 +221,10 @@ func run(cmd *cobra.Command, args []string) error { } // Docker-related options - if dockerNoPull { - executorOpts = append(executorOpts, executor.WithDockerOptions(options.DockerOptions{ - NoPull: dockerNoPull, - })) - } + executorOpts = append(executorOpts, executor.WithDockerOptions(options.DockerOptions{ + NoPull: dockerNoPull, + NoCleanup: debugNoCleanup, + })) // Environment executorOpts = append(executorOpts, @@ -264,6 +266,10 @@ func newRunCmd() *cobra.Command { cmd.PersistentFlags().BoolVar(&dockerNoPull, "docker-no-pull", false, "don't attempt to pull the images before starting containers") + // Flags useful for debugging + cmd.PersistentFlags().BoolVar(&debugNoCleanup, "debug-no-cleanup", false, + "don't remove containers and volumes after execution") + // Experimental features flags cmd.PersistentFlags().BoolVar(&experimentalParser, "experimental-parser", false, "use local configuration parser instead of sending parse request to Cirrus Cloud") diff --git a/internal/commands/run_test.go b/internal/commands/run_test.go index a16dd02c..25e5e9a7 100644 --- a/internal/commands/run_test.go +++ b/internal/commands/run_test.go @@ -2,8 +2,11 @@ package commands_test import ( "bytes" + "context" "github.com/cirruslabs/cirrus-cli/internal/commands" "github.com/cirruslabs/cirrus-cli/internal/testutil" + "github.com/docker/docker/api/types" + "github.com/docker/docker/client" "github.com/go-git/go-git/v5" "github.com/go-git/go-git/v5/plumbing/object" "github.com/stretchr/testify/assert" @@ -11,6 +14,8 @@ import ( "io" "io/ioutil" "os" + "regexp" + "strings" "testing" ) @@ -239,3 +244,54 @@ func TestRunTaskFilteringByLabel(t *testing.T) { assert.Contains(t, buf.String(), "VERSION:1.14") assert.NotContains(t, buf.String(), "VERSION:1.15") } + +// TestRunNoCleanup ensures that containers and volumes are kept intact +// after execution ends and --debug-no-cleanup is used. +func TestRunNoCleanup(t *testing.T) { + testutil.TempChdirPopulatedWith(t, "testdata/run-no-cleanup") + + // Create os.Stderr writer that duplicates it's output to buf + buf := bytes.NewBufferString("") + writer := io.MultiWriter(os.Stderr, buf) + + command := commands.NewRootCmd() + command.SetArgs([]string{"run", "-v", "-o simple", "--debug-no-cleanup"}) + command.SetOut(writer) + command.SetErr(writer) + err := command.Execute() + + require.NoError(t, err) + assert.Contains(t, buf.String(), "not cleaning up container") + assert.Contains(t, buf.String(), "not cleaning up additional container") + assert.Contains(t, buf.String(), "not cleaning up working volume") + + // The fun ends here since now we have to cleanup containers and volumes ourselves + cli, err := client.NewClientWithOpts(client.FromEnv, client.WithAPIVersionNegotiation()) + if err != nil { + t.Fatal(err) + } + + containerRegex := regexp.MustCompile("not cleaning up (?:container|additional container) (?P[^,]+)") + volumeRegex := regexp.MustCompile("not cleaning up working volume (?P[^,]+)") + + for _, line := range strings.Split(buf.String(), "\n") { + matches := containerRegex.FindStringSubmatch(line) + if matches != nil { + containerID := matches[containerRegex.SubexpIndex("container_id")] + if err := cli.ContainerRemove(context.Background(), containerID, types.ContainerRemoveOptions{ + RemoveVolumes: true, + Force: true, + }); err != nil { + t.Fatal(err) + } + } + + matches = volumeRegex.FindStringSubmatch(line) + if matches != nil { + volumeID := matches[volumeRegex.SubexpIndex("volume_id")] + if err := cli.VolumeRemove(context.Background(), volumeID, false); err != nil { + t.Fatal(err) + } + } + } +} diff --git a/internal/commands/testdata/run-no-cleanup/.cirrus.yml b/internal/commands/testdata/run-no-cleanup/.cirrus.yml new file mode 100644 index 00000000..695eff72 --- /dev/null +++ b/internal/commands/testdata/run-no-cleanup/.cirrus.yml @@ -0,0 +1,10 @@ +task: + container: + image: debian:latest + + additional_containers: + - name: mysql + image: mysql:latest + port: 3306 + + script: true diff --git a/internal/executor/instance/container.go b/internal/executor/instance/container.go index 08f9bd89..9d3e9816 100644 --- a/internal/executor/instance/container.go +++ b/internal/executor/instance/container.go @@ -18,6 +18,13 @@ func (inst *ContainerInstance) Run(ctx context.Context, config *RunConfig) (err return err } defer func() { + if config.DockerOptions.NoCleanup { + config.Logger.Infof("not cleaning up working volume %s, don't forget to remove it with \"docker volume rm %s\"", + workingVolume.Name(), workingVolume.Name()) + + return + } + cleanupErr := workingVolume.Close() if err == nil { err = cleanupErr diff --git a/internal/executor/instance/instance.go b/internal/executor/instance/instance.go index 8fff2a2f..5643606a 100644 --- a/internal/executor/instance/instance.go +++ b/internal/executor/instance/instance.go @@ -205,13 +205,19 @@ func RunDockerizedAgent(ctx context.Context, config *RunConfig, params *Params) // Schedule all containers for removal defer func() { - logger.Debugf("cleaning up container %s", cont.ID) - err := cli.ContainerRemove(context.Background(), cont.ID, types.ContainerRemoveOptions{ - RemoveVolumes: true, - Force: true, - }) - if err != nil { - logger.Warnf("error while removing container: %v", err) + if config.DockerOptions.NoCleanup { + logger.Infof("not cleaning up container %s, don't forget to remove it with \"docker rm -v %s\"", + cont.ID, cont.ID) + } else { + logger.Debugf("cleaning up container %s", cont.ID) + + err := cli.ContainerRemove(context.Background(), cont.ID, types.ContainerRemoveOptions{ + RemoveVolumes: true, + Force: true, + }) + if err != nil { + logger.Warnf("error while removing container: %v", err) + } } additionalContainersCancel() @@ -225,7 +231,14 @@ func RunDockerizedAgent(ctx context.Context, config *RunConfig, params *Params) additionalContainersWG.Add(1) go func() { - if err := runAdditionalContainer(additionalContainersCtx, logger, additionalContainer, cli, cont.ID); err != nil { + if err := runAdditionalContainer( + additionalContainersCtx, + logger, + additionalContainer, + cli, + cont.ID, + config.DockerOptions, + ); err != nil { additionalContainersErrChan <- err } additionalContainersWG.Done() @@ -257,8 +270,9 @@ func runAdditionalContainer( additionalContainer *api.AdditionalContainer, cli *client.Client, connectToContainer string, + dockerOptions options.DockerOptions, ) error { - logger.Debugf("pulling image %s", additionalContainer.Image) + logger.Debugf("pulling additional container image %s", additionalContainer.Image) progress, err := cli.ImagePull(ctx, additionalContainer.Image, types.ImagePullOptions{}) if err != nil { return fmt.Errorf("%w: %v", ErrAdditionalContainerFailed, err) @@ -268,7 +282,7 @@ func runAdditionalContainer( return fmt.Errorf("%w: %v", ErrAdditionalContainerFailed, err) } - logger.Debugf("creating container") + logger.Debugf("creating additional container") containerConfig := container.Config{ Image: additionalContainer.Image, Cmd: additionalContainer.Command, @@ -287,13 +301,20 @@ func runAdditionalContainer( } defer func() { - logger.Debugf("cleaning up container %s", cont.ID) + if dockerOptions.NoCleanup { + logger.Infof("not cleaning up additional container %s, don't forget to remove it with \"docker rm -v %s\"", + cont.ID, cont.ID) + + return + } + + logger.Debugf("cleaning up additional container %s", cont.ID) err := cli.ContainerRemove(context.Background(), cont.ID, types.ContainerRemoveOptions{ RemoveVolumes: true, Force: true, }) if err != nil { - logger.Warnf("Error while removing container: %v", err) + logger.Warnf("Error while removing additional container: %v", err) } }() @@ -306,16 +327,16 @@ func runAdditionalContainer( "running in the additional container '%s' to use a different port", additionalContainer.Name) } - logger.Debugf("starting container %s", cont.ID) + logger.Debugf("starting additional container %s", cont.ID) if err := cli.ContainerStart(ctx, cont.ID, types.ContainerStartOptions{}); err != nil { return fmt.Errorf("%w: %v", ErrAdditionalContainerFailed, err) } - logger.Debugf("waiting for container %s to finish", cont.ID) + logger.Debugf("waiting for additional container %s to finish", cont.ID) waitChan, errChan := cli.ContainerWait(ctx, cont.ID, container.WaitConditionNotRunning) select { case res := <-waitChan: - logger.Debugf("container exited with %v error and exit code %d", res.Error, res.StatusCode) + logger.Debugf("additional container exited with %v error and exit code %d", res.Error, res.StatusCode) case err := <-errChan: return fmt.Errorf("%w: %v", ErrAdditionalContainerFailed, err) } diff --git a/internal/executor/instance/pipe.go b/internal/executor/instance/pipe.go index a66284df..1c6a4518 100644 --- a/internal/executor/instance/pipe.go +++ b/internal/executor/instance/pipe.go @@ -55,6 +55,13 @@ func (pi *PipeInstance) Run(ctx context.Context, config *RunConfig) (err error) return err } defer func() { + if config.DockerOptions.NoCleanup { + config.Logger.Infof("not cleaning up working volume %s, don't forget to remove it with \"docker volume rm %s\"", + workingVolume.Name(), workingVolume.Name()) + + return + } + cleanupErr := workingVolume.Close() if err == nil { err = cleanupErr diff --git a/internal/executor/options/docker.go b/internal/executor/options/docker.go index 255bf8f4..47245cc6 100644 --- a/internal/executor/options/docker.go +++ b/internal/executor/options/docker.go @@ -3,6 +3,7 @@ package options type DockerOptions struct { NoPull bool NoPullImages []string + NoCleanup bool } func (do DockerOptions) ShouldPullImage(image string) bool {