fix(hash): recreate container on project config content change

[idsulik]

What I did Fixed hash.ServiceHash() to support config content change

Related issue https://github.com/docker/compose/issues/11900

[ndeloof]

While I understand the intent, I don't like we get the config content added into the service hash. This also only makes sense as the config content is inlined. I wonder we could rely on a label to track the config state by the time it was created : com.docker.compose.config.name=<config hash>.

[idsulik]

I don't like we get the config content added into the service hash

why?

I wonder we could rely on a label to track the config state by the time it was created : com.docker.compose.config.name=<config hash>.

I don't get the idea, time it was created - do you mean config file? or docker-compose.yaml? but docker-compose can refer to external config file

[ndeloof]

time container was created, so we can check it needs to be recreated if current config doesn't match

[ndeloof]

Some poins:

1. At some point we will have to do the same for secrets (same constraints) and networks/volumes (which can get labeled with a has). Solution we adopt here should be extensible to allow this in the future
2. Configs set by file are implemented by a bind mount but this may change in near future (see https://github.com/docker/compose/pull/11871) so we also need to consider those
3. There's no technical reason docker engine can't offer secrets/configs natively, just this is guarded by Swarm mode, but AFAIK some discussion happened to get them also available in standalone mode. I can't tell if/when this would take place, but preferably the logic here should consider this may happen
4. Last but not least, I'd prefer we don't mix service config hash with resources it depends on, so my suggestion to introduce an additional label com.docker.compose.config.xx=hash that we can use to track this relation, and need to recreate container, without the need to change the service hash computation (which as impact on existing installations)

[idsulik]

@ndeloof thanks for the details. pushed changes:

1. reverted old changes
2. added new func ServiceDependenciesHash and label

// ConfigHashDependenciesLabel stores configuration hash for a compose service dependencies
ConfigHashDependenciesLabel = "com.docker.compose.config-hash-dependencies"

Let me know if you have better idea for the label name, because I'm not fully satisfied with my name)

[codecov[bot]]

Codecov Report

Attention: Patch coverage is 59.77011% with 70 lines in your changes missing coverage. Please review.

Project coverage is 51.69%. Comparing base (49575ef) to head (7c3c8d2). Report is 14 commits behind head on main.

Files with missing lines	Patch %	Lines
pkg/utils/tar.go	58.22%	22 Missing and 11 partials :warning:
pkg/compose/convergence.go	32.00%	10 Missing and 7 partials :warning:
pkg/compose/hash.go	71.42%	11 Missing and 3 partials :warning:
pkg/compose/create.go	68.42%	4 Missing and 2 partials :warning:

Additional details and impacted files

@@            Coverage Diff             @@
##             main   #11931      +/-   ##
==========================================
- Coverage   51.78%   51.69%   -0.10%     
==========================================
  Files         156      157       +1     
  Lines       15697    15800     +103     
==========================================
+ Hits         8129     8168      +39     
- Misses       6763     6807      +44     
- Partials      805      825      +20     

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

[stale[bot]]

This issue has been automatically marked as not stale anymore due to the recent activity.

[apollo13]

This would be a really great addition, anything I can help out with?

[idsulik]

This would be a really great addition, anything I can help out with?

no, thank you, waiting for @ndeloof response

[apollo13]

@idsulik Maybe it might make sense to fix the failing linting checks before another round of reviews?

[idsulik]

@ndeloof could you please check it?

[idsulik]

@glours hi! Maybe you can check it? otherwise, it can be stuck here for a long time

[idsulik]

@ndeloof @glours hi! Rebased the main branch and resolved conflicts, is there anything else I should do ?

[apollo13]

@ndeloof Anything I can do to help move this forward?

[idsulik]

I have to resolve conflicts several times, could you please check it? @ndeloof

[ianhinder]

Would it be possible to get this in? I just ran into the problem it would fix.