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

Version hash is dependent on repository source folder name #5501

Closed
dadadom opened this issue Dec 1, 2023 · 5 comments · Fixed by #5508 or #5735
Closed

Version hash is dependent on repository source folder name #5501

dadadom opened this issue Dec 1, 2023 · 5 comments · Fixed by #5508 or #5735
Assignees

Comments

@dadadom
Copy link
Contributor

dadadom commented Dec 1, 2023

Bug

I have the suspicion that ever since https://github.com/garden-io/garden/releases/tag/0.12.61 and the introduction of the fix for #4661 the version depends on the path of the source repository.

Current Behavior

The version hash generated to determine if an action needs to be executed again seems to depend on the source folder name of the checked out remote repository.

Expected behavior

The version hash should only depend on the contents of the repository, not on the folder name.

Reproducible example

$> cat umbrella/project.garden.yml

apiVersion: garden.io/v1
kind: Project
name: umbrella

defaultEnvironment: local

environments:
  - name: local
    defaultNamespace: garden-local

providers:
  - name: local-kubernetes
    environments:
      - local
  - name: jib
 
sources:
  - name: app
    repositoryUrl: https://github.com/dadadom/app-testing#master

$> garden version
garden version: 0.13.21

$>  git clone git@github.com:dadadom/app-testing.git app1
...
$>  git clone git@github.com:dadadom/app-testing.git app2
...

$> garden --root umbrella link source app app1

$> garden --root umbrella build --log-level=debug
...
Execing ... -Djib.container.args=GARDEN_MODULE_VERSION=v-9c6dacb57c ...
...

$> garden --root umbrella unlink source app
$> garden --root umbrella link source app app2
$> garden --root umbrella build --log-level=debug
...
Execing ... -Djib.container.args=GARDEN_MODULE_VERSION=v-a469a92ed9 ...
...

Workaround

Suggested solution(s)

As far as I understand the system, the "source" of the files should be irrelevant as long as the contents and locations of the files within a repository don't change.

Additional context

For us the issue is critical as we re-execute all gradle jib builds for every production deployment although the exact same image was already built for the staging environment.
In our CI, the repository where garden checks out the sources into is different on every run (not sure how the folder names are calculated).

Looking back a few months I saw that this did not happen with Garden 0.12.56 and only happened when we (directly) upgraded to Garden 0.12.61, hence my suspicion about the mentioned release/issue.

Your environment

Happens on our CI (linux/amd64), but the example above was executed on an M1 MacBook.

@stefreak
Copy link
Member

stefreak commented Dec 1, 2023

Hello, thank you for the report! Indeed, the absolute path to the git repository should not matter for version calculation. We'll look into this.

@vvagaytsev
Copy link
Collaborator

The fix will be released in 0.13.22.

@dadadom
Copy link
Contributor Author

dadadom commented Feb 8, 2024

Good day,

could this ticket be reopened? I still have the same issue, here is a full reproducer:

$> minikube start
$> garden version # returns 0.13.24
$> cd /tmp
$> mkdir -p garden-testing/umbrella && cd garden-testing/umbrella
$> git init
$> cat >project.garden.yml <<EOL
apiVersion: garden.io/v1
kind: Project
name: umbrella

defaultEnvironment: local

environments:
  - name: local
    defaultNamespace: garden-local

providers:
  - name: local-kubernetes
    environments:
      - local
  - name: jib

sources:
  - name: app
    repositoryUrl: https://github.com/dadadom/app-testing#master
EOL
$> cd ..
$> git clone git@github.com:dadadom/app-testing.git app1
$> git clone git@github.com:dadadom/app-testing.git app2

$> garden --root umbrella link source app app1
$> garden --root umbrella build --log-level=debug # ... -Djib.to.image=app-builder:v-1c7f44ec67 ...
$> garden --root umbrella unlink source app
$> garden --root umbrella link source app app2
$> garden --root umbrella build --log-level=debug # ... -Djib.to.image=app-builder:v-5d90f256ba ...

From my understanding of the logic, there seem (cave: my personal hypothesis) to be two issues.

On the one hand, the calculated hash is still different although the contents of umbrella/.garden/build/app-builder are identical between the two different sources.

On the other hand, at least according to the logs, the hash calculation happens before all files are synced to the build/app-builder directory. If I understand it correctly, the build/* directories are used to determine the version of the sources and the exceptions from .gardenignore` are taken into account here.

❯ /opt/homebrew/bin/garden --root umbrella build --log-level=debug
[verbose] garden version: 0.13.24
Build 🔨
...
ℹ graph                     → Resolving actions and modules...
ℹ git                       → [debug] Scanning Build action app-builder root at /Users/myuser/development/tmp/garden/app2
  → Includes: (none)
  → Excludes: **/.garden/**/*
ℹ git                       → [debug] Found 9 files in Build action app-builder root /Users/myuser/development/tmp/garden/app2
ℹ graph                     → [debug] Found 9 files in module path, filtering by 1 include and 0 exclude globs
ℹ graph                     → [debug] Found 9 files in module path after glob matching
✔ graph                     → Finished resolving graph (took 0 sec)
ℹ garden                    → Finished initializing Garden (took 1.5 sec)

─
The following actions are linked to a local path:
  Build type=jib-container name=app-builder linked to path /Users/myuser/development/tmp/garden/app2
─
ℹ build.app-builder         → [debug] resolve Build type=jib-container name=app-builder is ready.
ℹ build.app-builder         → Getting status for Build app-builder (type jib-container)...
ℹ build.app-builder         → [debug] Spawning '/Users/myuser/.garden/tools/docker/1cbaf5c26b78806e/docker/docker images app-builder:v-6447dfc5dc -q' in /Users/myuser/development/tmp/garden/umbrella
ℹ build.app-builder         → Status is not ready, app-builder will be built
ℹ build.app-builder         → Building app-builder (type jib-container) at version v-aa663d09d4...
ℹ build.app-builder         → [verbose] Syncing sources (8 files)...
ℹ build.app-builder         → [debug] Syncing 8 files from ../app2 to .garden/build/app-builder (and removing any extraneous files)
ℹ build.app-builder         → [verbose] Done syncing sources (took 0.5 sec)
...
ℹ build.app-builder         → [debug] Execing /Users/myuser/development/tmp/garden/app2/gradlew jibDockerBuild -Djib.to.image=app-builder:v-6447dfc5dc -Djib.container.args=GARDEN_MODULE_VERSION=v-6447dfc5dc,GARDEN_ACTION_VERSION=v-6447dfc5dc -Dstyle.color=always -Djansi.passthrough=true -Djib.console=plain
...
Done! ✔️

Please let me know if I am misunderstanding something and the hashes don't actually matter, but at least the execution of the jib build happens way too often ... ;-)

@stefreak stefreak reopened this Feb 8, 2024
@stefreak
Copy link
Member

stefreak commented Feb 8, 2024

@dadadom Sorry for closing this despite the issue still being present! We'll look into this and post updates here.

@stefreak stefreak changed the title Version hash is dependent on repository source folder name (?) Version hash is dependent on repository source folder name Feb 8, 2024
@thsig thsig self-assigned this Feb 14, 2024
@dadadom
Copy link
Contributor Author

dadadom commented Feb 14, 2024

Just validated that this issue still exists on 0.13.25.

github-merge-queue bot pushed a commit that referenced this issue Feb 15, 2024
<!--  Thanks for sending a pull request! Here are some tips for you:

1. If this is your first pull request, please read our contributor
guidelines in the
https://github.com/garden-io/garden/blob/main/CONTRIBUTING.md file.
2. Please label this pull request according to what type of issue you
are addressing (see "What type of PR is this?" below)
3. Ensure you have added or run the appropriate tests for your PR.
4. If the PR is unfinished, add `WIP:` at the beginning of the title or
use the GitHub Draft PR feature.
5. Please add at least two reviewers to the PR. Currently active
maintainers are: @edvald, @thsig, @eysi09, @shumailxyz, @stefreak,
@TimBeyer, @mkhq, and @vvagaytsev.
-->

**What this PR does / why we need it**:

Before this fix, we were including the relative path to a file from
project root in the string to be hashed when calculating versions.

This was problematic when linking remote sources, since the chosen name
and path of the locally linked repo would be factored into the version
(resulting in unwanted cache misses).

This was fixed by using the relative path from the directory containing
the file's parent config. For example, an action's directory should be
able to be moved around or renamed without it affecting the version of
that action.

**Which issue(s) this PR fixes**:

Fixes #5501.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants