Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Dev command leaves containers running when process exits with an error #677

Merged
merged 9 commits into from Aug 19, 2022

Conversation

TylerAldrich
Copy link
Contributor

@TylerAldrich TylerAldrich commented Aug 18, 2022

When we hit this code:

try {
    await compose_process;
} catch (ex) {
    if (!is_exiting) {
        throw ex;
    }
}

it was possible for the compose_process to end (with an error that we throw) - but the process finishing with an error doesn't guarantee that containers get cleaned up. One container may hard-fail, the process ends, and other containers remain running.

To fix that, we now run:

    try {
      await this.compose_process;
    } catch (ex) {
      if (!this.is_exiting) {
        // Always call `docker compose stop` here - the process died so there's nothing to kill,
        // need to call `docker compose stop` to clean up any remaining running containers.
        console.error(chalk.red(ex));
        console.log(chalk.red('\nError detected - Stopping running containers...'));
        await this.stop();
      }
    }

to call docker compose stop after the compose up process ends to ensure that containers are stopped.

I also moved all the code that manages the docker compose up process into it's own class to make the logic in this file more easily understandable.

  • The Dev command class is still handling most of the work
  • The UpProcessManager now starts/registers the process interrupts/stops the long-running docker compose up process and when it's finished running, the Dev.run can continue cleanup.

Tested on

  • Mac
  • WSL2
  • (Windows) Powershell
  • (Windows) Cmd

src/commands/dev.ts Outdated Show resolved Hide resolved
@tjhiggins tjhiggins merged commit b8e5165 into rc Aug 19, 2022
@tjhiggins tjhiggins deleted the dev-command-bad-volume branch August 19, 2022 20:03
github-actions bot pushed a commit that referenced this pull request Aug 19, 2022
# [1.24.0-rc.6](v1.24.0-rc.5...v1.24.0-rc.6) (2022-08-19)

### Bug Fixes

* **dev:** Dev command leaves containers running when process exits with an error ([#677](#677)) ([b8e5165](b8e5165))
github-actions bot pushed a commit that referenced this pull request Sep 1, 2022
# [1.25.0-rc.1](v1.24.0...v1.25.0-rc.1) (2022-09-01)

### Bug Fixes

* **cli:** convert deployCommand auto-approve ([#685](#685)) ([78f3209](78f3209))
* **cli:** Docker verify improved ([#679](#679)) ([552cd77](552cd77))
* **dev:** Dev command leaves containers running when process exits with an error ([#677](#677)) ([b8e5165](b8e5165))
* **dev:** Fixed host regex that shouldnt have a global match that caused regexp.exec to not work as desired ([#689](#689)) ([1a6428d](1a6428d))
* **dev:** fixing healthcheck liveness probe protocol for port/path ([#678](#678)) ([81e69f0](81e69f0))
* **dev:** Handle edge cases when starting components with pre-existing containers ([9cffb28](9cffb28))
* **exec:** Handle case where older version of compose is used and ConfigFiles doesnt exist ([#681](#681)) ([c0112d2](c0112d2))
* **exec:** Handle terminal resize events in exec ([#675](#675)) ([6ff95a5](6ff95a5))
* **exec:** Improve error message for Windows PS users ([#680](#680)) ([c214870](c214870))
* **subdomain:** 488 prevent nested subdomain ([#674](#674)) ([0445430](0445430))

### Features

* **cli:** 496 consistent boolean flags ([#683](#683)) ([4203c74](4203c74))
* **dev:** Use local SSL for  ([#676](#676)) ([536b38e](536b38e))
* **exec:** 482 pass replica name ([#663](#663)) ([84ce8c5](84ce8c5))
* **register:** add register multiple components and test cases ([#657](#657)) ([2bd1fd2](2bd1fd2))
github-actions bot pushed a commit that referenced this pull request Sep 1, 2022
# [1.25.0](v1.24.0...v1.25.0) (2022-09-01)

### Bug Fixes

* **cli:** convert deployCommand auto-approve ([#685](#685)) ([78f3209](78f3209))
* **cli:** Docker verify improved ([#679](#679)) ([552cd77](552cd77))
* **dev:** Dev command leaves containers running when process exits with an error ([#677](#677)) ([b8e5165](b8e5165))
* **dev:** Fixed host regex that shouldnt have a global match that caused regexp.exec to not work as desired ([#689](#689)) ([1a6428d](1a6428d))
* **dev:** fixing healthcheck liveness probe protocol for port/path ([#678](#678)) ([81e69f0](81e69f0))
* **dev:** Handle edge cases when starting components with pre-existing containers ([9cffb28](9cffb28))
* **exec:** Handle case where older version of compose is used and ConfigFiles doesnt exist ([#681](#681)) ([c0112d2](c0112d2))
* **exec:** Handle terminal resize events in exec ([#675](#675)) ([6ff95a5](6ff95a5))
* **exec:** Improve error message for Windows PS users ([#680](#680)) ([c214870](c214870))
* **subdomain:** 488 prevent nested subdomain ([#674](#674)) ([0445430](0445430))

### Features

* **cli:** 496 consistent boolean flags ([#683](#683)) ([4203c74](4203c74))
* **dev:** Use local SSL for  ([#676](#676)) ([536b38e](536b38e))
* **exec:** 482 pass replica name ([#663](#663)) ([84ce8c5](84ce8c5))
* **register:** add register multiple components and test cases ([#657](#657)) ([2bd1fd2](2bd1fd2))
@github-actions
Copy link

github-actions bot commented Sep 1, 2022

🎉 This PR is included in version 1.25.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants