Upgrade go-containerregistry third-party library #957

antechrestos · 2020-01-08T10:35:46Z

Fixes #808

Description

In this change I updated go-containerregistry dependency to a more recent version. It was needed to get the fix brought by the following pull request on the third party project
By doing so, it forced me to upgrade to newer dependency and/or versions.

One of them is moby/buildkit. This late one is developed in go 1.11. Starting version 1.13, versioning does not allow invalid pseudo-version. The invalid pseudo versions used by moby/buildkit (or dependency using itself an invalid pseudo version) are the one that are declared in the replacesection.

Submitter Checklist

These are the criteria that every PR should meet, please check them off as you
review them:

Includes Not needed
Adds integration tests if needed Not needed

Reviewer Notes

The code flow looks good.
Unit tests and or integration tests added.

Release Notes

Describe any changes here so maintainer can include it in the release notes, or delete this block.

Fix issues when pushing images on some registry built from a base image hosted on a registry different than docker hub and target registry

cvgw · 2020-01-10T17:59:19Z

go.mod

@@ -2,114 +2,73 @@ module github.com/GoogleContainerTools/kaniko

 go 1.13

+replace (
+	github.com/containerd/containerd v1.4.0-0.20191014053712-acdcf13d5eaf => github.com/containerd/containerd v0.0.0-20191014053712-acdcf13d5eaf


@antechrestos do you have a link to some info or would you mind giving me a quick explanation on why this is the correct fix for the problem? AFAIK this is correct, but I can't really find much info about this problem.

@cvgw I crawled around the web to find the way to fix this problem. I though that using a simple commit id would be better, yet go tidy replaced it with worse version.
Aside from that I don't understand why go 1.13 does not allow the minus zero, as minus zero means do nothing.

Truly I am very confused with that: it allows using github.com/docker/docker v1.14.0-0.20190319215453-e7b5f7dbe98c in require section, but if one of the dependency needs it with the very same version, it cries.

I would have preferred something like removing every -0.from go.mod file :

@@ -4,7 +4,7 @@ go 1.13 replace ( github.com/containerd/containerd v1.4.0-0.20191014053712-acdcf13d5eaf => github.com/containerd/containerd v0.0.0-20191014053712-acdcf13d5eaf - github.com/docker/docker v1.14.0-0.20190319215453-e7b5f7dbe98c => github.com/docker/docker v0.0.0-20190319215453-e7b5f7dbe98c + github.com/docker/docker v1.14.0-0.20190319215453-e7b5f7dbe98c => github.com/docker/docker v1.14.0-20190319215453-e7b5f7dbe98c github.com/tonistiigi/fsutil v0.0.0-20190819224149-3d2716dd0a4d => github.com/tonistiigi/fsutil v0.0.0-20191018213012-0f039a052ca1 ) @@ -12,14 +12,14 @@ require ( cloud.google.com/go v0.26.0 github.com/Azure/azure-pipeline-go v0.2.2 // indirect github.com/Azure/azure-storage-blob-go v0.8.0 - github.com/Microsoft/go-winio v0.4.15-0.20190919025122-fc70bd9a86b5 // indirect + github.com/Microsoft/go-winio v0.4.15.20190919025122-fc70bd9a86b5 // indirect github.com/alcortesm/tgz v0.0.0-20161220082320-9c5fe88206d7 // indirect github.com/anmitsu/go-shlex v0.0.0-20161002113705-648efa622239 // indirect github.com/aws/aws-sdk-go v1.25.19 github.com/beorn7/perks v0.0.0-20180321164747-3a771d992973 // indirect - github.com/docker/docker v1.14.0-0.20190319215453-e7b5f7dbe98c + github.com/docker/docker v1.14.0.20190319215453-e7b5f7dbe98c github.com/docker/go-metrics v0.0.0-20180209012529-399ea8c73916 // indirect - github.com/docker/swarmkit v1.12.1-0.20180726190244-7567d47988d8 // indirect + github.com/docker/swarmkit v1.12.1.20180726190244-7567d47988d8 // indirect github.com/emirpasic/gods v1.9.0 // indirect github.com/flynn/go-shlex v0.0.0-20150515145356-3f9db97f8568 // indirect github.com/genuinetools/amicontained v0.4.3

Yet it does not find dependencies after that.:

$> go mod vendor fix/scopes_asked_to_remote_registry ✖ ✱ go: finding github.com/Microsoft/go-winio v0.4.15.20190919025122-fc70bd9a86b5 go: finding github.com/Microsoft/go-winio v0.4.15.20190919025122-fc70bd9a86b5 go: finding github.com/docker/docker v1.14.0.20190319215453-e7b5f7dbe98c go: finding github.com/docker/docker v1.14.0.20190319215453-e7b5f7dbe98c go: finding github.com/docker/swarmkit v1.12.1.20180726190244-7567d47988d8 go: finding github.com/docker/swarmkit v1.12.1.20180726190244-7567d47988d8 go: errors parsing go.mod: kaniko/go.mod:15: require github.com/Microsoft/go-winio: version "v0.4.15.20190919025122-fc70bd9a86b5" invalid: unknown revision v0.4.15.20190919025122-fc70bd9a86b5 kaniko/go.mod:20: require github.com/docker/docker: version "v1.14.0.20190319215453-e7b5f7dbe98c" invalid: unknown revision v1.14.0.20190319215453-e7b5f7dbe98c kaniko/go.mod:22: require github.com/docker/swarmkit: version "v1.12.1.20180726190244-7567d47988d8" invalid: unknown revision v1.12.1.20180726190244-7567d47988d8

I am opened to any suggestion on this point as I am not an expert on go modules.

Thanks for that additional information @antechrestos

I found this documentation which I think is particularly relevant https://golang.org/cmd/go/#hdr-Pseudo_versions

This section caught my eye

Pseudo-versions never need to be typed by hand: the go command will accept the plain commit hash and translate it into a pseudo-version (or a tagged version if available) automatically. This conversion is an example of a module query.

I think that explains why go tidy updated the replace statements when you used the commit id.

I know there was some additional pseudo version validation added in golang 1.13 which probably has something to do with this.

@antechrestos did you use go mod edit -replace for these?

@cvgw
I did used the go mod edit -replacecommand at first but did not like the result as I think it is less nice.

I've just used it and here is the diff, please let me know if you prefer this way:

go 1.13 -replace ( - github.com/containerd/containerd v1.4.0-0.20191014053712-acdcf13d5eaf => github.com/containerd/containerd v0.0.0-20191014053712-acdcf13d5eaf - github.com/docker/docker v1.14.0-0.20190319215453-e7b5f7dbe98c => github.com/docker/docker v0.0.0-20190319215453-e7b5f7dbe98c - github.com/tonistiigi/fsutil v0.0.0-20190819224149-3d2716dd0a4d => github.com/tonistiigi/fsutil v0.0.0-20191018213012-0f039a052ca1 -) - require ( cloud.google.com/go v0.26.0 github.com/Azure/azure-pipeline-go v0.2.2 // indirect @@ -72,3 +66,9 @@ require ( gopkg.in/warnings.v0 v0.1.2 // indirect k8s.io/kubernetes v1.13.0 // indirect ) + +replace github.com/docker/docker v1.14.0-0.20190319215453-e7b5f7dbe98c => github.com/docker/docker v1.4.2-0.20190319215453-e7b5f7dbe98c + +replace github.com/containerd/containerd v1.4.0-0.20191014053712-acdcf13d5eaf => github.com/containerd/containerd v1.3.1-0.20191014053712-acdcf13d5eaf + +replace github.com/tonistiigi/fsutil v0.0.0-20190819224149-3d2716dd0a4d => github.com/tonistiigi/fsutil v0.0.0-20191018213012-0f039a052ca1

Shall I group the replace in a single replace section?

BTW, would you share the output of kaniko tests?

I launched them from master branch and I have random results.

First launch on master

--- FAIL: TestRun (0.00s) --- FAIL: TestRun/test_Dockerfile_test_cmd (32.40s) --- FAIL: TestRun/test_Dockerfile_test_metadata (32.41s) --- FAIL: TestRun/test_Dockerfile_test_volume_4 (50.78s) --- FAIL: TestLayers (0.00s) --- FAIL: TestLayers/test_layer_Dockerfile_test_arg_multi (4.25s) --- FAIL: TestLayers/test_layer_Dockerfile_test_arg_multi_with_quotes (2.53s) --- FAIL: TestLayers/test_layer_Dockerfile_test_meta_arg (13.72s) --- FAIL: TestLayers/test_layer_Dockerfile_test_copy_same_file_many_times (17.26s) FAIL FAIL github.com/GoogleContainerTools/kaniko/integration 1020.141s

I cleaned images, containers, and gcloud and retried (still on master):

--- FAIL: TestRun (0.00s) --- FAIL: TestRun/test_Dockerfile_test_volume_4 (26.22s) --- FAIL: TestRun/test_Dockerfile_test_cmd (22.37s) --- FAIL: TestRun/test_Dockerfile_test_metadata (29.66s) --- FAIL: TestRun/test_Dockerfile_test_onbuild (40.64s) --- FAIL: TestRun/test_Dockerfile_test_hardlink (83.07s) --- FAIL: TestLayers (0.00s) --- FAIL: TestLayers/test_layer_Dockerfile_test_onbuild (10.27s) --- FAIL: TestLayers/test_layer_Dockerfile_test_cache_copy (11.03s) --- FAIL: TestLayers/test_layer_Dockerfile_test_hardlink (31.00s) --- FAIL: TestLayers/test_layer_Dockerfile_test_volume_4 (15.86s) --- FAIL: TestCache (1.33s) --- FAIL: TestCache/test_cache_Dockerfile_test_cache_perm (27.73s) --- FAIL: TestCache/test_cache_Dockerfile_test_cache_install (70.29s) --- FAIL: TestCache/test_cache_Dockerfile_test_cache (71.23s) --- FAIL: TestCache/test_cache_Dockerfile_test_cache_copy (73.13s) FAIL FAIL github.com/GoogleContainerTools/kaniko/integration 610.926s

Do you also observe random results on migration tests? 🤔

I also launched on my branch:

--- FAIL: TestRun (0.00s) --- FAIL: TestRun/test_Dockerfile_test_cmd (21.51s) --- FAIL: TestRun/test_Dockerfile_test_add_dest_symlink_dir (17.46s) --- FAIL: TestRun/test_Dockerfile_test_volume_4 (60.68s) --- FAIL: TestRun/test_Dockerfile_test_scratch (13.83s) --- FAIL: TestRun/test_Dockerfile_test_replaced_symlinks (28.89s) --- FAIL: TestRun/test_Dockerfile_test_metadata (19.60s) --- FAIL: TestRun/test_Dockerfile_test_hardlink (94.27s) --- FAIL: TestLayers (0.00s) --- FAIL: TestLayers/test_layer_Dockerfile_test_arg_multi (4.12s) --- FAIL: TestLayers/test_layer_Dockerfile_test_arg_multi_with_quotes (2.43s) --- FAIL: TestLayers/test_layer_Dockerfile_test_scratch (5.34s) --- FAIL: TestLayers/test_layer_Dockerfile_test_meta_arg (18.65s) --- FAIL: TestLayers/test_layer_Dockerfile_test_replaced_symlinks (6.44s) --- FAIL: TestLayers/test_layer_Dockerfile_test_add_dest_symlink_dir (10.40s) --- FAIL: TestLayers/test_layer_Dockerfile_test_arg_blank_with_quotes (2.07s) --- FAIL: TestLayers/test_layer_Dockerfile_test_copy_same_file_many_times (18.39s) --- FAIL: TestLayers/test_layer_Dockerfile_test_hardlink (10.63s) FAIL FAIL github.com/GoogleContainerTools/kaniko/integration 740.007s

@antechrestos I don't have a strong preference between go mod edit version and yours. I would say keep what you have.

The following tests failed

FAIL: TestRun/test_Dockerfile_test_hardlink (31.88s) error building image: file with no instructions. FAIL: TestRun/test_Dockerfile_test_replaced_symlinks (8.67s) INFO[0000] Resolved base name tenstartups/alpine@sha256:31dc8b12e0f73a1de899146c3663644b7668f8fd198cfe9b266886c9abfa944b to tenstartups/alpine@sha256:31dc8b12e0f73a1de899146c3663644b7668f8fd198cfe9b266886c9abfa944b INFO[0000] Using dockerignore file: /workspace/.dockerignore INFO[0000] Resolved base name tenstartups/alpine@sha256:31dc8b12e0f73a1de899146c3663644b7668f8fd198cfe9b266886c9abfa944b to tenstartups/alpine@sha256:31dc8b12e0f73a1de899146c3663644b7668f8fd198cfe9b266886c9abfa944b INFO[0000] Image tenstartups/alpine@sha256:31dc8b12e0f73a1de899146c3663644b7668f8fd198cfe9b266886c9abfa944b not found in cache INFO[0000] Retrieving image manifest tenstartups/alpine@sha256:31dc8b12e0f73a1de899146c3663644b7668f8fd198cfe9b266886c9abfa944b INFO[0000] Built cross stage deps: map[] INFO[0000] Image tenstartups/alpine@sha256:31dc8b12e0f73a1de899146c3663644b7668f8fd198cfe9b266886c9abfa944b not found in cache INFO[0000] Retrieving image manifest tenstartups/alpine@sha256:31dc8b12e0f73a1de899146c3663644b7668f8fd198cfe9b266886c9abfa944b error building image: file with no instructions. FAIL: TestRun/test_Dockerfile_test_scratch (7.11s) INFO[0000] Resolved base name ${image} to scratch INFO[0000] Using dockerignore file: /workspace/.dockerignore INFO[0000] Resolved base name ${image} to scratch INFO[0000] Built cross stage deps: map[] ERRO[0000] Error while retrieving image from cache: could not parse reference: INFO[0000] Image not found in cache INFO[0000] Retrieving image manifest FAIL: TestRun/test_Dockerfile_test_add_dest_symlink_dir (10.78s) INFO[0000] Resolved base name phusion/baseimage:0.11 to phusion/baseimage:0.11 INFO[0000] Using dockerignore file: /workspace/.dockerignore INFO[0000] Resolved base name phusion/baseimage:0.11 to phusion/baseimage:0.11 INFO[0000] Retrieving image manifest phusion/baseimage:0.11 INFO[0000] Image phusion/baseimage:0.11 not found in cache INFO[0000] Retrieving image manifest phusion/baseimage:0.11 INFO[0000] Built cross stage deps: map[] INFO[0000] Retrieving image manifest phusion/baseimage:0.11 INFO[0000] Image phusion/baseimage:0.11 not found in cache INFO[0000] Retrieving image manifest phusion/baseimage:0.11 error building image: file with no instructions.

Three of them seem to be the same error error building image: file with no instructions which seems to be coming from buildkit https://github.com/moby/buildkit/blob/f53dcc830b03ccc1719fb9472852dcb960131e24/frontend/dockerfile/parser/parser.go#L280

But it certainly doesn't look like those dockerfiles are empty https://github.com/GoogleContainerTools/kaniko/blob/master/integration/dockerfiles/Dockerfile_test_add_dest_symlink_dir

Looking at the integration code I don't see any obvious reasons for the failure. Need to investigate more

@cvgw Thanks for the info. I will take a look.

@cvgw I think I solved one of the two problems.

The first one is linked to evolution in parser. This method is called by pkg/executor/build.go:resolveOnBuild with an empty array as config.OnBuild. It used to be accepted by buildkit but no longer. No more Mr nice guy they may have though.
I changed the if config.OnBuild == nil to if config.OnBuild == nil || len(config.OnBuild) == 0 in pkg/executor/build.go:resolveOnBuild and it solved tests with Dockerfile_test_replaced_symlinks, Dockerfile_test_add_dest_symlink_dir and Dockerfile_test_hardlink.

The second issue is linked to Dockerfile_test_scratch. It has an ARG named image. Don't know why but, it also has a meta argument (don't know what it is...yet) also named imageand valued to empty string; this late one overrides the build arg and the image resolution made by pkg/util/image_util.go:RetrieveSourceImage results in empty value. (ERRO[0000] Error while retrieving image from cache: could not parse reference: )

I will go deeper for this problem.

I found it: now in moby BuildEnvs function they override value while before they kept the previous one. I will try to solve it by putting meta args first and then build args (which is logic)

tejal29 · 2020-01-22T17:56:45Z

@antechrestos We now run integration tests on Travis. If you rebase on master, you will pick up the change.
Thanks
Tejal

antechrestos · 2020-01-22T20:12:34Z

@tejal29 thank you I will take a look tomorrow.

By doing so it will fix issues met when mixing source/remote registry Close #808

antechrestos · 2020-01-26T18:44:11Z

@cvgw @tejal29 unit tests and integration tests are passing.

antechrestos · 2020-01-27T17:10:43Z

BTW I am stuck with need rebase label despite the rebase 😏

antechrestos · 2020-01-28T09:10:47Z

@cvgw hurray kokoro succeeded here 😊

cvgw · 2020-01-29T00:00:43Z

Awesome, thanks for making this one happen @antechrestos 👍

googlebot added the cla: yes CLA signed by all commit authors label Jan 8, 2020

antechrestos mentioned this pull request Jan 8, 2020

Use cache instead of vendor #959

Closed

4 tasks

cvgw reviewed Jan 10, 2020

View reviewed changes

cvgw added the kokoro:run label Jan 11, 2020

kokoro-team removed the kokoro:run label Jan 11, 2020

tejal29 added the needs-rebase label Jan 17, 2020

cvgw self-assigned this Jan 17, 2020

antechrestos closed this Jan 18, 2020

antechrestos reopened this Jan 18, 2020

Upgrade dependency to go-containerregistry third-party library

03b8922

By doing so it will fix issues met when mixing source/remote registry Close #808

cvgw added kokoro:run and removed needs-rebase labels Jan 27, 2020

kokoro-team removed the kokoro:run label Jan 27, 2020

cvgw approved these changes Jan 29, 2020

View reviewed changes

cvgw merged commit 65cd912 into GoogleContainerTools:master Jan 29, 2020

tejal29 mentioned this pull request Jan 29, 2020

Fix#977 upgrade go mod dependencies #978

Closed

4 tasks

antechrestos deleted the fix/scopes_asked_to_remote_registry branch January 29, 2020 08:02

antechrestos mentioned this pull request Feb 5, 2020

Connection refused for non-TLS registry URL #977

Closed

Forst mentioned this pull request May 5, 2020

Pushing images to dockerhub stopped working #1209

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Upgrade go-containerregistry third-party library #957

Upgrade go-containerregistry third-party library #957

antechrestos commented Jan 8, 2020

cvgw Jan 10, 2020

antechrestos Jan 10, 2020

cvgw Jan 11, 2020

cvgw Jan 11, 2020

antechrestos Jan 11, 2020

cvgw Jan 17, 2020

antechrestos Jan 18, 2020

antechrestos Jan 26, 2020 •

edited

Loading

antechrestos Jan 26, 2020

antechrestos Jan 26, 2020

tejal29 commented Jan 22, 2020

antechrestos commented Jan 22, 2020

antechrestos commented Jan 26, 2020

antechrestos commented Jan 27, 2020

antechrestos commented Jan 28, 2020

cvgw commented Jan 29, 2020

Upgrade go-containerregistry third-party library #957

Upgrade go-containerregistry third-party library #957

Conversation

antechrestos commented Jan 8, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

antechrestos Jan 26, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tejal29 commented Jan 22, 2020

antechrestos commented Jan 22, 2020

antechrestos commented Jan 26, 2020

antechrestos commented Jan 27, 2020

antechrestos commented Jan 28, 2020

cvgw commented Jan 29, 2020

antechrestos Jan 26, 2020 •

edited

Loading