Conversation
|
@mtrmac PTAL |
|
I think it's time to setup the schema 1 registry in tests :) I'll do it |
runcom
force-pushed
the
fix-copy-test-manlist
branch
2 times, most recently
from
58b5ced to
c7b972b
Compare
|
If it’s all the same to you, I’d prefer running the tests against a v2 registry with s2; s1 images’ signatures eventually should break with fixed s1 upload (#88), so converting most of the tests from Docker Hub estesp (s1+s2+list) against OpenShift to s2 against v2 registry would be nicer. |
wait, v2s2 broke something in openshift here, I'll revert some changes and you can see |
runcom
force-pushed
the
fix-copy-test-manlist
branch
from
c7b972b to
682236a
Compare
|
@mtrmac do you know why the current test with |
|
Atomic Registry by default refuses to accept s2. https://github.com/mtrmac/skopeo/tree/docker-push-to-tag enables that, but then we are running tests in a non-default configuration, not a great idea. That’s why I was suggesting a docker/distribution registry + s2, instead of Atomic. (And why this is so much work.) |
|
To summarize, you want that test to push the image to an in-test docker/distribution V2 registry, instead of the atomic registry, right? |
|
At least |
|
Right, we can find an s1 image to still test something for the atomic registry, fine. I'll finish this tomorrow morning. The v2s2 registry is already setup in the test infra so it shouldn't be that hard I hope. |
|
Yes, at least |
runcom
force-pushed
the
fix-copy-test-manlist
branch
3 times, most recently
from
0518f8f to
5036ee2
Compare
|
@mtrmac tests fixed, PTAL |
mtrmac
left a comment
mtrmac
left a comment
There was a problem hiding this comment.
Well, this does work, and most importantly it does fix the test breakage.
Please consider using CopySuite.registry instead of embedding SkopeoSuite; but if you want to merge the code as is, that’s OK as well.
integration/copy_test.go
Outdated
| } | ||
|
|
||
| func (s *CopySuite) SetUpTest(c *check.C) { | ||
| s.ss.SetUpTest(c) |
There was a problem hiding this comment.
SkopeoSuite is setting up a fresh set of five registries for every single test. That seems a fair bit of unnecessary overhead, especially when CopySuite.SetUpSuite is already setting up a v2 registry.
| // FIXME: It would be nice to use one of the local Docker registries instead of neeeding an Internet connection. | ||
| // "pull": docker: → dir: | ||
| assertSkopeoSucceeds(c, "", "copy", "docker://estesp/busybox:latest", "dir:"+dir1) | ||
| assertSkopeoSucceeds(c, "", "copy", "docker://estesp/busybox:amd64", "dir:"+dir1) |
runcom
force-pushed
the
fix-copy-test-manlist
branch
from
5036ee2 to
d6669d6
Compare
|
Should be good to go if tests agree @mtrmac |
| // "pull": docker: → dir: | ||
| assertSkopeoSucceeds(c, "", "copy", "docker://busybox", "dir:"+dir1) | ||
| // "push": dir: → docker(v2s2): | ||
| assertSkopeoSucceeds(c, "", "--tls-verify=false", "--debug", "copy", "dir:"+dir1, ourRegistry+"busybox:unsigned") |
There was a problem hiding this comment.
This addresses the FIXME: Also check pushing to docker:// at line 152, which can now be removed.
mtrmac
left a comment
mtrmac
left a comment
There was a problem hiding this comment.
Thanks! Please merge if tests pass.
Signed-off-by: Antonio Murdaca <runcom@redhat.com>
runcom
force-pushed
the
fix-copy-test-manlist
branch
from
d6669d6 to
dc1e560
Compare
2 participants
specifically containers/image#115 and also add an integration test
Signed-off-by: Antonio Murdaca runcom@redhat.com