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
ci: enable DockerPluginSuite (integration-cli) #41559
base: master
Are you sure you want to change the base?
Conversation
ARGH CI is failing, because it looks like some vanity domain went AWOL; https://ci-next.docker.com/public/blue/organizations/jenkins/moby/detail/PR-41559/1/pipeline/207
https://4d63.com/gochecknoglobals/checknoglobals?go-get=1: gives a 404 But https://4d63.com/gochecknoglobals/ exists (so import path should be |
Looks like this broke it golangci/golangci-lint#1422 But also means that (again) we're somehow installing golang-ci-lint master, not the specified version 😞 |
Need something similar to #41151 for golang-ci-lint |
Opened #41560 to fix CI |
d830503
to
b55b2b8
Compare
Rebased to get the fix from #41560 in |
Nice, failing as expected without #41533 |
Ah, great that's what i wanted |
rebased |
b55b2b8
to
6324bd2
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
This may be a genuine failure;
Will have to update that test to see what the actual output is |
6324bd2
to
f20fac5
Compare
@@ -217,7 +217,7 @@ func (ps *DockerPluginSuite) TestPluginInstallImage(c *testing.T) { | |||
|
|||
out, _, err := dockerCmdWithError("plugin", "install", repoName) | |||
assert.ErrorContains(c, err, "") | |||
assert.Assert(c, strings.Contains(out, `Encountered remote "application/vnd.docker.container.image.v1+json"(image) when fetching`)) | |||
assert.Assert(c, strings.Contains(out, `Encountered remote "application/vnd.docker.container.image.v1+json"(image) when fetching`), out) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
cmp.Contains
should take care of this.
docker_cli_plugins_test.go:221: assertion failed: string "Error response from daemon: application/vnd.docker.distribution.manifest.v1+prettyjws not supported\n" does not contain "Encountered remote \"application/vnd.docker.container.image.v1+json\"(image) when fetching"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, yes, I can replace those in this file.
Is the output it received here (application/vnd.docker.distribution.manifest.v1+prettyjws not supported
) the expected output? (So should the test just be updated to match the other error?) or is there a bug at hand?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🤷 This looks like a schema1 manifest?
Seems like the media type changed as is, and then we've also moved this to use containerd distribution instead of docker/distribution.
It probably doesn't make sense to even support schema1 manifests for plugins... but I guess we could tell it to convert for us.
fa78597
to
3ab3d62
Compare
3ab3d62
to
f1b4043
Compare
f1b4043
to
ba011fc
Compare
ba011fc
to
e846383
Compare
Closed for now as this CI-failing test is tracked in another issue and this step was removed from the Jenkinsfile |
e846383
to
f01482d
Compare
f01482d
to
2d657cd
Compare
Interesting; getting some panics on Jenkins
|
This is just a cli panic when |
Ah, thanks for that link! Can be fully ignored then (I thought I'd post it just in case there was something weird) |
2d657cd
to
c8e6159
Compare
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
c8e6159
to
5f1884b
Compare
.
relates to #41533 (comment)
This enables the following tests;