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

Make hashes between up and configure consistent #11010

Merged
merged 2 commits into from
Sep 29, 2023

Conversation

mattwalo32
Copy link
Contributor

What I did
Currently, when docker compose config --hash [service] computes a hash for the service, it deletes all other services before computing the hash. That means if a service depends on another service, the DependsOn field gets deleted, causing the hash to be different than when docker up creates the labels.

I prevented the dependencies from getting deleted before hash computation to fix this.

I also noticed that docker up does some modification to the platform of the image before doing the hash (I.e. if no platform was specified in the yaml file, the default one is set). docker config doesn't do this at all. So I modified docker config to apply these modifications before computing the hash.

It could be argued that config shouldn't call applyPlatform. Take the scenario where a compose file contains services that don't have a platform specified. If that same file is run on two different machines with different default platforms, the hashes will be different. Personally, I think the hashes for the identical files should be different because they'll run on different platforms, but would love to get some feedback on that.

Related issue
Fixes #10907

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

@codecov
Copy link

codecov bot commented Sep 16, 2023

Codecov Report

Attention: 10 lines in your changes are missing coverage. Please review.

Comparison is base (12e0ac8) 57.55% compared to head (c37d032) 57.91%.
Report is 2 commits behind head on main.

❗ Current head c37d032 differs from pull request most recent head b773c54. Consider uploading reports for the commit b773c54 to get more accurate results

Additional details and impacted files
@@            Coverage Diff             @@
##             main   #11010      +/-   ##
==========================================
+ Coverage   57.55%   57.91%   +0.36%     
==========================================
  Files         129      129              
  Lines       11229    11124     -105     
==========================================
- Hits         6463     6443      -20     
+ Misses       4136     4047      -89     
- Partials      630      634       +4     
Files Coverage Δ
cmd/compose/config.go 33.33% <0.00%> (-1.03%) ⬇️

... and 10 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@ndeloof
Copy link
Contributor

ndeloof commented Sep 29, 2023

nice catch! and thanks for investigating this issue

@ndeloof ndeloof enabled auto-merge (rebase) September 29, 2023 04:23
@AkihiroSuda
Copy link

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] config --hash return incorrect hash value for containers with depends_on
3 participants