Skip to content

Conversation

glours
Copy link
Contributor

@glours glours commented Jun 27, 2025

What I did
Check error type returned by engine and continue if the container with pre_stop hook is already stopped or removed
Added e2e tests for both pre_stop and post_start hooks

Related issue
fix #12978

(not mandatory) A picture of a cute animal, if possible in relation to what you did
image

@glours glours requested a review from a team as a code owner June 27, 2025 11:55
@glours glours requested a review from ndeloof June 27, 2025 11:55
@glours glours self-assigned this Jun 27, 2025
@@ -52,6 +53,10 @@ func (s composeService) runHook(ctx context.Context, ctr container.Summary, serv
AttachStderr: !detached,
})
if err != nil {
// Ignore errors indicating that some containers were already stopped or removed.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

as runHook is only used in 2 places, and this applies to the stopContainer case, maybe we could check this error in stopContainer caller function and avoid a custom extension?
(this will also cover a race conditions where container is removed after exec was created but before attach/start

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 done

@glours glours force-pushed the pre-stop-already-stopped-service branch from a5cdf1a to ebff485 Compare June 27, 2025 12:17
@glours glours requested a review from ndeloof June 27, 2025 12:18
@glours glours force-pushed the pre-stop-already-stopped-service branch from ebff485 to 5ac821b Compare June 27, 2025 12:19
…moved

Signed-off-by: Guillaume Lours <705411+glours@users.noreply.github.com>
@glours glours force-pushed the pre-stop-already-stopped-service branch from 5ac821b to 7d7c0c6 Compare June 27, 2025 12:30
Copy link
Contributor

@ndeloof ndeloof left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👻

@ndeloof ndeloof enabled auto-merge (rebase) June 27, 2025 12:36
@ndeloof ndeloof merged commit d219aa6 into docker:main Jun 27, 2025
25 of 26 checks passed
@glours glours deleted the pre-stop-already-stopped-service branch June 30, 2025 21:26
tmeijn pushed a commit to tmeijn/dotfiles that referenced this pull request Jul 6, 2025
This MR contains the following updates:

| Package | Update | Change |
|---|---|---|
| [docker/compose](https://github.com/docker/compose) | minor | `v2.37.3` -> `v2.38.1` |

MR created with the help of [el-capitano/tools/renovate-bot](https://gitlab.com/el-capitano/tools/renovate-bot).

**Proposed changes to behavior should be submitted there as MRs.**

---

### Release Notes

<details>
<summary>docker/compose (docker/compose)</summary>

### [`v2.38.1`](https://github.com/docker/compose/releases/tag/v2.38.1)

[Compare Source](docker/compose@v2.38.0...v2.38.1)

#### What's Changed

##### ✨ Improvements

- implement `model_variable` by [@&#8203;ndeloof](https://github.com/ndeloof) in docker/compose#13001

##### ⚙️ Dependencies

- bump compose-go to version v2.7.1 by [@&#8203;glours](https://github.com/glours) in docker/compose#13000

**Full Changelog**: docker/compose@v2.38.0...v2.38.1

### [`v2.38.0`](https://github.com/docker/compose/releases/tag/v2.38.0)

[Compare Source](docker/compose@v2.37.3...v2.38.0)

#### What's Changed

##### ✨ Improvements

- introduce support for models by [@&#8203;ndeloof](https://github.com/ndeloof) in docker/compose#12976
- Add volumes command by [@&#8203;leoperegrino](https://github.com/leoperegrino) in docker/compose#12954
- remove publish limitation on bind mount by [@&#8203;glours](https://github.com/glours) in docker/compose#12997
- mount /var/run/docker.sock for --use-api-socket by [@&#8203;ndeloof](https://github.com/ndeloof) in docker/compose#12995

##### 🐛 Fixes

- only expose API socket to service asking for it by [@&#8203;ndeloof](https://github.com/ndeloof) in docker/compose#12972
- check progress default value instead of empty string to use BUILDKIT\_PROGRESS env variable value by [@&#8203;glours](https://github.com/glours) in docker/compose#12982
- exclude provider services from the list of dependencies that Compose should wait for by [@&#8203;glours](https://github.com/glours) in docker/compose#12983
- don't fail down cmd if services with pre\_stop hook already stopped/removed by [@&#8203;glours](https://github.com/glours) in docker/compose#12986
- Swap to Reader in bake to avoid hangs on output by [@&#8203;nscott](https://github.com/nscott) in docker/compose#12984
- make sure the post\_start hooks fails by [@&#8203;glours](https://github.com/glours) in docker/compose#12996
- remove error message from exec outpout by default by [@&#8203;glours](https://github.com/glours) in docker/compose#12992
- fix: typos by [@&#8203;hezhizhen](https://github.com/hezhizhen) in docker/compose#12963
- pass project.environment to bake by [@&#8203;ndeloof](https://github.com/ndeloof) in docker/compose#12994
- fix provider concurrent environment map accesses by [@&#8203;glours](https://github.com/glours) in docker/compose#12999
- e2e compose run --env by [@&#8203;ndeloof](https://github.com/ndeloof) in docker/compose#12967

##### ⚙️ Dependencies

- build(deps): bump github.com/docker/cli from 28.2.2+incompatible to 28.3.0+incompatible by [@&#8203;dependabot](https://github.com/dependabot) in docker/compose#12974
- build(deps): bump github.com/docker/docker from 28.2.2+incompatible to 28.3.0+incompatible by [@&#8203;dependabot](https://github.com/dependabot) in docker/compose#12975

#### New Contributors

- [@&#8203;nscott](https://github.com/nscott) made their first contribution in docker/compose#12984
- [@&#8203;hezhizhen](https://github.com/hezhizhen) made their first contribution in docker/compose#12963
- [@&#8203;leoperegrino](https://github.com/leoperegrino) made their first contribution in docker/compose#12954

**Full Changelog**: docker/compose@v2.37.3...v2.38.0

</details>

---

### Configuration

📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined).

🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied.

♻ **Rebasing**: Whenever MR becomes conflicted, or you tick the rebase/retry checkbox.

🔕 **Ignore**: Close this MR and you won't be reminded about this update again.

---

 - [ ] <!-- rebase-check -->If you want to rebase/retry this MR, check this box

---

This MR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate).
<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiI0MC42Mi4xIiwidXBkYXRlZEluVmVyIjoiNDAuNjIuMSIsInRhcmdldEJyYW5jaCI6Im1haW4iLCJsYWJlbHMiOlsiUmVub3ZhdGUgQm90Il19-->
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[BUG] Compose down fails if container with pre_stop hooks is stopped
2 participants