Skip to content

Conversation

@mtrmac
Copy link
Contributor

@mtrmac mtrmac commented Nov 5, 2019

See #760 (comment) below.

Copy link
Member

@vrothberg vrothberg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are we ready to go? Before merging, the "x" commit could need a commit message.

@mtrmac
Copy link
Contributor Author

mtrmac commented Nov 21, 2019

For this PR to work, both the tests in here and containers/image#744 need to succeed, which is not currently the case.

This allows using the vendored dependencies instead of
searching for them in $GOPATH and elsewhere.

This does not necessarily matter for skopeo itself, but
the test-skopeo Makefile target in containers/image uses
(go mod edit -replace) to replace the vendored c/image with
a locally-edited copy; skopeo's (make check) then runs tests in
a container which does not have access to this locally-edited
copy, and since Go 1.13 this causes (go {list,test,vet})
to fail if -mod=vendor is not used.

Signed-off-by: Miloslav Trmač <[email protected]>
(go list) removes the vendor subdirectory automatically since
Go 1.9.

Signed-off-by: Miloslav Trmač <[email protected]>
The nested podman tries to write to it.  This primarily only
removes noise from logs, it does not seem to significantly change
behavior.

Signed-off-by: Miloslav Trmač <[email protected]>
Experimentally, this seems to help with localhost access inside that
container (but I have no idea what's the reason for that).

Signed-off-by: Miloslav Trmač <[email protected]>
Apparently we run into the 10-minute timeout now in Travis.

Signed-off-by: Miloslav Trmač <[email protected]>
… testing

This image is about 100 MB instead of about 2 GB for the Server Core,
decreasing disk requirements and hopefully significantly speeeding up
integration tests.

Signed-off-by: Miloslav Trmač <[email protected]>
@mtrmac mtrmac changed the title DO NOT MERGE: Experimenting with c/image test failures Support running tests from c/image Nov 21, 2019
@mtrmac
Copy link
Contributor Author

mtrmac commented Nov 21, 2019

Yay, success!

This primarily modifies the test suite so that it can again succeed when run from c/image’s make test-skopeo. See individual commit messages for details.

The two major elements are:

  • Use -mod=vendor everywhere, so that a vendored-in copy of c/image under test is visible to Go 1.13.
  • Remove --net=host from the container running make test-system; it nowadays breaks localhost accesses (e.g. ping 127.0.0.1 fails). I have no idea why it breaks networking, but arguably --net=host was never necessary and having separate namespaces is cleaner anyway, and we don’t quite have the luxury to stop c/image development just to diagnose this right now.

containers/image#744 , with

SKOPEO_REPO = mtrmac/skopeo
SKOPEO_BRANCH = test-failures

(which will be momentarily removed) succeeds.

@vrothberg PTAL.

@rhatdan
Copy link
Member

rhatdan commented Nov 22, 2019

LGTM

@rhatdan rhatdan merged commit 39540db into containers:master Nov 22, 2019
@mtrmac mtrmac deleted the test-failures branch November 22, 2019 13:51
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Oct 1, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants