Skip to content

vendor: update tar-split#37771

Merged
thaJeztah merged 2 commits intomoby:masterfrom
kolyshkin:fix-8gb
Sep 6, 2018
Merged

vendor: update tar-split#37771
thaJeztah merged 2 commits intomoby:masterfrom
kolyshkin:fix-8gb

Conversation

@kolyshkin
Copy link
Copy Markdown
Contributor

@kolyshkin kolyshkin commented Sep 6, 2018 •
edited
Loading

@codecov
Copy link
Copy Markdown

codecov bot commented Sep 6, 2018 •
edited
Loading

Codecov Report

❗ No coverage uploaded for pull request base (master@7129beb). Click here to learn what that means.
The diff coverage is n/a.

@@            Coverage Diff            @@
##             master   #37771   +/-   ##
=========================================
  Coverage          ?   36.05%           
=========================================
  Files             ?      609           
  Lines             ?    45072           
  Branches          ?        0           
=========================================
  Hits              ?    16252           
  Misses            ?    26591           
  Partials          ?     2229

@kolyshkin
integration/build: add TestBuildHugeFile
b3165f5 
Add a test case for creating a 8GB file inside a container.
Due to a bug in tar-split this was failing in Docker 18.06.

The file being created is sparse, so there's not much I/O
happening or disk space being used -- meaning the test is
fast and does not require a lot of disk space.

Signed-off-by: Kir Kolyshkin <[email protected]>
This was referenced Sep 6, 2018
Closed
Merged
Merged
@kolyshkin
Copy link
Copy Markdown
Contributor Author

kolyshkin commented Sep 6, 2018

Yes, the test case (that was failing in #37772) is now passing, meaning the fix is working. This is in addition to some manual testing I've done.

@kolyshkin
Copy link
Copy Markdown
Contributor Author

kolyshkin commented Sep 6, 2018

TODO: merge vbatts/tar-split#48; do a proper revendor (w/o my fork).

@kolyshkin
Copy link
Copy Markdown
Contributor Author

kolyshkin commented Sep 6, 2018

ppc failure is unrelated (#32673)

01:32:13.399 FAIL: docker_api_swarm_test.go:296: DockerSwarmSuite.TestAPISwarmLeaderElection

vdemeester
vdemeester approved these changes Sep 6, 2018
View reviewed changes
Copy link
Copy Markdown
Member

@vdemeester vdemeester left a comment

Choose a reason for hiding this comment

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

SGTM 🌮
cc @dgageot

@dgageot
Copy link
Copy Markdown
Contributor

dgageot commented Sep 6, 2018

Thank you! SGTM too

@vbatts
Copy link
Copy Markdown
Contributor

vbatts commented Sep 6, 2018

cc self

@vbatts
Copy link
Copy Markdown
Contributor

vbatts commented Sep 6, 2018

I've merged the PR and tagged v0.11.0
Big thanks @kolyshkin for the fix. Thanks @dgageot for the report reproducer.
And thanks to @vdemeester for being another cool Vincent ;-)

@kolyshkin
vendor: update tar-split
92e7543 
To include vbatts/tar-split#48 which
fixes the issue of creating an image with >8GB file in it.

Signed-off-by: Kir Kolyshkin <[email protected]>
@kolyshkin kolyshkin changed the title [DO NOT MERGE] vendor: revendor tar-split vendor: update tar-split Sep 6, 2018
@kolyshkin
Copy link
Copy Markdown
Contributor Author

kolyshkin commented Sep 6, 2018

I've merged the PR and tagged v0.11.0

Rebased to use it, added a test case. This one is ready to be merged.

thaJeztah
thaJeztah approved these changes Sep 6, 2018
View reviewed changes
Copy link
Copy Markdown
Member

@thaJeztah thaJeztah left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

@kolyshkin
Copy link
Copy Markdown
Contributor Author

kolyshkin commented Sep 6, 2018

exp CI failure is def unrelated (see #34988):

01:09:47.681 FAIL: check_test.go:347: DockerSwarmSuite.TearDownTest
...
01:09:47.686 PANIC: docker_api_swarm_test.go:359: DockerSwarmSuite.TestAPISwarmRaftQuorum

@thaJeztah thaJeztah merged commit 54b62e6 into moby:master Sep 6, 2018
This was referenced Sep 7, 2018
Closed
Merged
nashasha1 pushed a commit to nashasha1/moby that referenced this pull request Sep 7, 2018
@thaJeztah @nashasha1
Merge pull request moby#37771 from kolyshkin/fix-8gb
c2f0672 
vendor: update tar-split
Signed-off-by: nashasha1 <[email protected]>
nashasha1 pushed a commit to nashasha1/moby that referenced this pull request Sep 7, 2018
@thaJeztah @nashasha1
Merge pull request moby#37771 from kolyshkin/fix-8gb
6ef9fec 
vendor: update tar-split
Signed-off-by: nashasha1 <[email protected]>
andrewhsu added a commit to docker-archive/engine that referenced this pull request Sep 13, 2018
@andrewhsu
Merge pull request #46 from kolyshkin/18.09-backport-pr37771
e988001 
[18.09] backport moby#37771 "vendor: update tar-split"
@TBBle TBBle mentioned this pull request Jun 19, 2019
Closed
@shilpikm
Copy link
Copy Markdown

shilpikm commented Feb 4, 2020

Hi, We are using Docker 18.09.6 and we are seeing this error randomly but quite often with image sizes much smaller than 8GB.

@thaJeztah
Copy link
Copy Markdown
Member

thaJeztah commented Feb 4, 2020

please open a new ticket with details and steps to reproduce. Does it also reproduce on docker 19.03? (18.09 reached eol)

@TBBle TBBle mentioned this pull request Jul 27, 2020
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@thaJeztah thaJeztah thaJeztah approved these changes

+1 more reviewer

@vdemeester vdemeester vdemeester approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

status/4-merge

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Error processing tar file(exit status 1): unexpected EOF adding an 8G file (8G - 1 byte is ok)

7 participants

@kolyshkin @dgageot @vbatts @shilpikm @thaJeztah @vdemeester @GordonTheTurtle