detect network config changes and recreate if needed #12267

ndeloof · 2024-11-05T14:53:55Z

What I did
use a config hash label on networks to detect need to be recreated.

attached services will be stopped so we can recreate network,
then convergence will recreate them as we check the network container is attached to has the expected ID

Related issue
fixes #9203

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

jhrotko · 2024-11-06T15:47:41Z

pkg/compose/hash.go

+	bytes, err := json.Marshal(o)
+	if err != nil {
+		return "", err
+	}


you need to ignore api.VersionLabel and api.ConfigHashLabel similarly to volumes

nope, this is why I introduced CustomLabel in compose-go (same mechanism used for services)
https://github.com/docker/compose/pull/12267/files#diff-1f4d596600c620b3df55dbfbc63e50a6f43b356865fae411c553d0937fbb6d91R122

terev · 2024-11-12T20:50:07Z

Bless you for this change! Was just working on a workaround for our internal compose wrapper but it was proving difficult to be sure it works sanely.

One question, is there a reason we can't do the network recreation more gracefully? Like detach all containers connected to the diverged network then remove the network only. Is this because the disconnected containers won't be reattached in the current reconciliation strategy?

ndeloof · 2024-11-12T21:02:45Z

@terev I also considered this approach, but this indeed require more changes on the convergence side to re-attach + this involves the container will manage network being disconnected/reconnected (IP address will change) and in many cases this is not well supported. Recreating is "safer" from this point of view.

What's your use-case for changing network definition and requirement for app to keep running ?

terev · 2024-11-12T21:17:20Z

@ndeloof Ok understood, that makes sense. It's not a super hard requirement, as my use case only applies to local development where I'd like to gracefully update the default binding address.

I was just weighing network detachment against the possibility of accidentally removing container logs and data. But even if container data is lost it probably should have been kept in a volume in the first place.

ndeloof · 2024-11-13T06:18:50Z

Right. This makes me wonder we should preserve anonymous volumes on recreation, to avoid such a data loss.

terev · 2024-11-13T06:52:42Z

That would be neat if possible, though that sounds like it'd be a totally new mechanism given that anonymous volumes are currently disassociated with a service if its container is recreated. I do think that'd be a neat feature that would make the Dockerfile volume instruction much more useful.

ndeloof · 2024-11-13T07:11:35Z

@terev when a service recreated, anonymous volumes are preserve already. We can reuse this mechanism

terev · 2024-11-13T07:33:36Z

Oh! Right sorry I'm mistaken, that's perfect.

terev · 2024-11-13T17:47:12Z

pkg/compose/convergence.go

+				}
+			}
+			if !found {
+				// config is up-t-date but container is not connected to network - maybe recreated ?


Fwiw this makes a lot of sense to me. Would even say it makes sense to recreate if it is connected to undesired networks.

terev · 2024-11-13T19:55:23Z

pkg/compose/convergence.go

+		return true, nil
+	}
+
+	if c.networks != nil {


Should we recreate if networks is nil but expected/actual has networks?

networks is nil only if caller didn't created convergence with actual network state - this is the case running compose run command, need to investigate why we don't manage networks in this command, will do in a follow-up PR.

glours

Sounds good to me

pkg/compose/convergence.go

Signed-off-by: Nicolas De Loof <[email protected]>

Co-authored-by: Guillaume Lours <[email protected]> Signed-off-by: Nicolas De loof <[email protected]>

This MR contains the following updates: | Package | Update | Change | |---|---|---| | [docker/compose](https://github.com/docker/compose) | minor | `v2.30.3` -> `v2.31.0` | 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.31.0`](https://github.com/docker/compose/releases/tag/v2.31.0) [Compare Source](docker/compose@v2.30.3...v2.31.0) #### What's Changed ##### ✨ Improvements - Delegate build to buildx bake by [@ndeloof](https://github.com/ndeloof) [(12300)](docker/compose#12300) - Add commit command by [@jarqvi](https://github.com/jarqvi) [(12268)](docker/compose#12268) ##### 🐛 Fixes - Fix(config): Print service names with --no-interpolate by [@idsulik](https://github.com/idsulik) [(12282)](docker/compose#12282) - Remove obsolete containers first on scale down by [@ndeloof](https://github.com/ndeloof) [(12272)](docker/compose#12272) - Fix compose images that return a different image with the same ID by [@koooge](https://github.com/koooge) [(12278)](docker/compose#12278) - Emit events for building images by [@felixfontein](https://github.com/felixfontein) [(11498)](docker/compose#11498) - Fix support for --remove-orphans on `docker compose run` by [@ndeloof](https://github.com/ndeloof) [(12288)](docker/compose#12288) - Push empty descriptor layer when using OCI version 1.1 for Compose artifact by [@glours](https://github.com/glours) [(12289)](docker/compose#12289) - Detect network config changes and recreate if needed by [@ndeloof](https://github.com/ndeloof) [(12267)](docker/compose#12267) - Update wait-timeout flag usage to include the unit by [@terev](https://github.com/terev) [(12316)](docker/compose#12316) - Use service.stop to stop dependent containers by [@ndeloof](https://github.com/ndeloof) [(12322)](docker/compose#12322) - Only check attached networks on running containers by [@ndeloof](https://github.com/ndeloof) [(12327)](docker/compose#12327) - Only stop dependent containers ... if there's some by [@ndeloof](https://github.com/ndeloof) [(12328)](docker/compose#12328) ##### 🔧 Internal - Pass stale bot inactivity limit from 6 to 3 months by [@glours](https://github.com/glours) [(12284)](docker/compose#12284) - Ci: enable testifylint linter by [@mmorel-35](https://github.com/mmorel-35) [(11761)](docker/compose#11761) - Remove ddev e2e tests by [@glours](https://github.com/glours) [(12291)](docker/compose#12291) - Gha: test against docker engine v27.4.0 by [@thaJeztah](https://github.com/thaJeztah) [(12299)](docker/compose#12299) - Run build tests against bake by [@ndeloof](https://github.com/ndeloof) [(12325)](docker/compose#12325) ##### ⚙️ Dependencies - Build(deps): bump golang.org/x/sync from `0.8.0` to `0.9.0` by [@dependabot](https://github.com/dependabot) [(12277)](docker/compose#12277) - Build(deps): bump golang.org/x/sys from `0.26.0` to `0.27.0` by [@dependabot](https://github.com/dependabot) [(12276)](docker/compose#12276) - Build(deps): bump github.com/moby/buildkit `v0.17.1`, github.com/docker/buildx `v0.18.0` by [@thaJeztah](https://github.com/thaJeztah) [(12298)](docker/compose#12298) - Build(deps): bump docker/docker `v27.4.0-rc.2`, docker/cli `v27.4.0-rc.2` by [@thaJeztah](https://github.com/thaJeztah) [(12306)](docker/compose#12306) - Build(deps): bump github.com/stretchr/testify from `1.9.0` to `1.10.0` by [@dependabot](https://github.com/dependabot) [(12319)](docker/compose#12319) - Build(deps): bump github.com/compose-spec/compose-go/v2 from `2.4.5-0.20241111154218-9d02caaf8465` to `2.4.5` by [@dependabot](https://github.com/dependabot) [(12324)](docker/compose#12324) - Build(deps): bump github.com/moby/buildkit from `0.17.1` to `0.17.2` by [@dependabot](https://github.com/dependabot) [(12320)](docker/compose#12320) - Bump google.golang.org/grpc to v1.68.0 and containerd to `v1.7.24` by [@glours](https://github.com/glours) [(12329)](docker/compose#12329) #### New Contributors - [@terev](https://github.com/terev) made their first contribution in docker/compose#12316 **Full Changelog**: docker/compose@v2.30.3...v2.31.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. --- - [ ] If you want to rebase/retry this MR, check this box --- This MR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate).

ndeloof marked this pull request as draft November 6, 2024 11:48

ndeloof force-pushed the network_diverged branch from 73b8ac3 to 600ead2 Compare November 6, 2024 11:59

ndeloof marked this pull request as ready for review November 6, 2024 12:23

ndeloof mentioned this pull request Nov 6, 2024

Recreate on volume change #12223

Closed

jhrotko reviewed Nov 6, 2024

View reviewed changes

ndeloof requested review from a team and glours and removed request for a team November 8, 2024 11:16

ndeloof force-pushed the network_diverged branch from 600ead2 to 8282f2c Compare November 12, 2024 08:46

ndeloof force-pushed the network_diverged branch from 8282f2c to c664a73 Compare November 13, 2024 15:00

terev reviewed Nov 13, 2024

View reviewed changes

ndeloof force-pushed the network_diverged branch from c664a73 to ef63c2e Compare November 13, 2024 19:23

terev reviewed Nov 13, 2024

View reviewed changes

ndeloof force-pushed the network_diverged branch 2 times, most recently from 1730ede to 23e5f2a Compare November 14, 2024 17:02

glours approved these changes Nov 15, 2024

View reviewed changes

pkg/compose/convergence.go Outdated Show resolved Hide resolved

ndeloof and others added 2 commits November 15, 2024 15:00

detect network config changes and recreate if needed

ff4bd9c

Signed-off-by: Nicolas De Loof <[email protected]>

Update pkg/compose/convergence.go

a0c9692

Co-authored-by: Guillaume Lours <[email protected]> Signed-off-by: Nicolas De loof <[email protected]>

ndeloof force-pushed the network_diverged branch from a1fd1c7 to a0c9692 Compare November 15, 2024 14:00

ndeloof merged commit d4fa63f into docker:main Nov 15, 2024
28 checks passed

ndeloof deleted the network_diverged branch November 15, 2024 14:37

BrewTestBot mentioned this pull request Nov 28, 2024

docker-compose 2.31.0 Homebrew/homebrew-core#199374

Merged

EugenMayer mentioned this pull request Jan 24, 2025

[BUG] since 2.31.0 neworks are removed on up without reasons and fail since containers still exist #12495

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

detect network config changes and recreate if needed #12267

detect network config changes and recreate if needed #12267

ndeloof commented Nov 5, 2024 •

edited

Loading

jhrotko Nov 6, 2024

ndeloof Nov 7, 2024

terev commented Nov 12, 2024

ndeloof commented Nov 12, 2024

terev commented Nov 12, 2024

ndeloof commented Nov 13, 2024

terev commented Nov 13, 2024 •

edited

Loading

ndeloof commented Nov 13, 2024

terev commented Nov 13, 2024

terev Nov 13, 2024

terev Nov 13, 2024 •

edited

Loading

ndeloof Nov 14, 2024

glours left a comment

detect network config changes and recreate if needed #12267

detect network config changes and recreate if needed #12267

Conversation

ndeloof commented Nov 5, 2024 • edited Loading

jhrotko Nov 6, 2024

Choose a reason for hiding this comment

ndeloof Nov 7, 2024

Choose a reason for hiding this comment

terev commented Nov 12, 2024

ndeloof commented Nov 12, 2024

terev commented Nov 12, 2024

ndeloof commented Nov 13, 2024

terev commented Nov 13, 2024 • edited Loading

ndeloof commented Nov 13, 2024

terev commented Nov 13, 2024

terev Nov 13, 2024

Choose a reason for hiding this comment

terev Nov 13, 2024 • edited Loading

Choose a reason for hiding this comment

ndeloof Nov 14, 2024

Choose a reason for hiding this comment

glours left a comment

Choose a reason for hiding this comment

ndeloof commented Nov 5, 2024 •

edited

Loading

terev commented Nov 13, 2024 •

edited

Loading

terev Nov 13, 2024 •

edited

Loading