Skip to content

Ignore EOF from ReadAt in content.ReadBlob#3318

Merged
estesp merged 1 commit intocontainerd:masterfrom
hinshun:ignore-readat-eof
Jun 4, 2019
Merged

Ignore EOF from ReadAt in content.ReadBlob#3318
estesp merged 1 commit intocontainerd:masterfrom
hinshun:ignore-readat-eof

Conversation

@hinshun
Copy link
Copy Markdown
Contributor

@hinshun hinshun commented May 31, 2019

Fixes #3317

@hinshun hinshun force-pushed the ignore-readat-eof branch 2 times, most recently from caedd3c to fb18344 Compare May 31, 2019 22:26
@theopenlab-ci
Copy link
Copy Markdown

theopenlab-ci Bot commented May 31, 2019

Build succeeded.

@AkihiroSuda
Copy link
Copy Markdown
Member

AkihiroSuda commented Jun 1, 2019

commit needs to be signed

@hinshun hinshun force-pushed the ignore-readat-eof branch from fb18344 to b532e06 Compare June 1, 2019 21:31
@theopenlab-ci
Copy link
Copy Markdown

theopenlab-ci Bot commented Jun 1, 2019

Build succeeded.

@fuweid
Copy link
Copy Markdown
Member

fuweid commented Jun 2, 2019

@hinshun could you mind to repush it again? the failed CI is flaky one. Sorry for that.

@hinshun hinshun force-pushed the ignore-readat-eof branch from b532e06 to 76a3181 Compare June 2, 2019 21:52
@theopenlab-ci
Copy link
Copy Markdown

theopenlab-ci Bot commented Jun 2, 2019

Build succeeded.

@hinshun hinshun force-pushed the ignore-readat-eof branch from 76a3181 to 3ff0d9b Compare June 2, 2019 21:57
@theopenlab-ci
Copy link
Copy Markdown

theopenlab-ci Bot commented Jun 2, 2019

Build succeeded.

@hinshun
Copy link
Copy Markdown
Contributor Author

hinshun commented Jun 2, 2019 •
edited
Loading

Appveyor having troubles downloading golang on windows:

rd C:\Go /s /q
appveyor DownloadFile https://storage.googleapis.com/golang/go%GO_VERSION%.windows-amd64.zip
Downloading go1.12.5.windows-amd64.zip (141,300,257 bytes)...1%Error downloading remote file: One or more errors occurred.
Inner Exception: Unable to read data from the transport connection: The connection was closed.

I was able to download it locally:

❯ curl -fLO https://storage.googleapis.com/golang/go1.12.5.windows-amd64.zip
// ...
 --:--:-- 15.4M

@hinshun hinshun force-pushed the ignore-readat-eof branch from 3ff0d9b to faf925b Compare June 4, 2019 17:18
@theopenlab-ci
Copy link
Copy Markdown

theopenlab-ci Bot commented Jun 4, 2019

Build succeeded.

@codecov-io
Copy link
Copy Markdown

codecov-io commented Jun 4, 2019

Codecov Report

Merging #3318 into master will decrease coverage by 0.02%.
The diff coverage is 0%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #3318      +/-   ##
==========================================
- Coverage   44.83%   44.81%   -0.03%     
==========================================
  Files         112      112              
  Lines       12311    12317       +6     
==========================================
  Hits         5520     5520              
- Misses       5943     5949       +6     
  Partials      848      848
Flag Coverage Δ
#linux 48.73% <0%> (-0.03%) ⬇️
#windows 40.18% <0%> (-0.03%) ⬇️
Impacted Files Coverage Δ
content/helpers.go 21.42% <0%> (-1.22%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update faa5f55...faf925b. Read the comment docs.

estesp
estesp approved these changes Jun 4, 2019
View reviewed changes
Copy link
Copy Markdown
Member

@estesp estesp left a comment

Choose a reason for hiding this comment

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

LGTM

estesp
estesp approved these changes Jun 4, 2019
View reviewed changes
Copy link
Copy Markdown
Member

@estesp estesp left a comment

Choose a reason for hiding this comment

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

LGTM

@estesp estesp merged commit 15ae6b7 into containerd:master Jun 4, 2019
@thaJeztah
Copy link
Copy Markdown
Member

thaJeztah commented Jun 4, 2019

does this one need a backport? (not sure if the issue was theoretical, or if there were actual examples)

@estesp
Copy link
Copy Markdown
Member

estesp commented Jun 4, 2019

I assume it is to fix for proper usage via API, and not necessarily a bug in containerd's use of the API? @hinshun can you shed any light? Thanks!

@hinshun
Copy link
Copy Markdown
Contributor Author

hinshun commented Jun 4, 2019

@thaJeztah I ran into this while experimenting with my peer-to-peer content plugin, in which my (provider.ReaderAt).ReadAt returned io.EOF, causing helper functions like images.Manifest to bail on io.EOF. I have not seen containerd with the default plugins have problems related to this, so perhaps a backport isn't required.

@thaJeztah
Copy link
Copy Markdown
Member

thaJeztah commented Jun 4, 2019

Gotcha, thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@estesp estesp estesp approved these changes

@AkihiroSuda AkihiroSuda AkihiroSuda approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

content.ReadBlob should ignore io.EOF

6 participants

@hinshun @AkihiroSuda @fuweid @codecov-io @thaJeztah @estesp