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

Add missing digest when setting helm image tag #2624

Merged

Conversation

krakowski
Copy link
Contributor

@krakowski krakowski commented Aug 9, 2019

Description

The helm deployer did not append the digest during deployment,
which caused file sync and log output (and maybe other features)
to stop working.

Explanation

goland-debug

  • I removed our repository's name from the screenshot

As you can see in the screenshot, dockerRef.Tag (which does not contain the digest) is used for setting the image tag (line 192 and 195) if the imageStrategy is set to helm. If the imageStrategy is not set to helm the deployer uses v.Tag which contains the registry location, the repository name and the tag (including the digest).

Issues

Fixes #1379

Related

The helm deployer did not append the digest during deployment,
which caused file sync and log output (and maybe other features)
to stop working. (GoogleContainerTools#1379)
@krakowski krakowski force-pushed the hotfix/1379/helm-tag branch from 701e3ab to b02733b Compare August 9, 2019 16:08
@codecov
Copy link

codecov bot commented Aug 9, 2019

Codecov Report

Merging #2624 into master will increase coverage by 0.08%.
The diff coverage is 100%.

Impacted Files Coverage Δ
pkg/skaffold/deploy/helm.go 67.53% <100%> (+2.63%) ⬆️

@tejal29 tejal29 added the kokoro:run runs the kokoro jobs on a PR label Aug 9, 2019
@kokoro-team kokoro-team removed the kokoro:run runs the kokoro jobs on a PR label Aug 9, 2019
@tejal29
Copy link
Contributor

tejal29 commented Aug 9, 2019

@krakowski Thank you for your PR. I restarted the travis build. test failures looked like some flakes.

Thanks
Tejal

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Skaffold failing to sync with helm
4 participants