Codecov Report

Merging #2580 into master will not change coverage.
The diff coverage is 66.66%.

@@           Coverage Diff           @@
##           master    #2580   +/-   ##
=======================================
  Coverage   44.05%   44.05%           
=======================================
  Files          98       98           
  Lines       10130    10130           
=======================================
  Hits         4463     4463           
  Misses       4945     4945           
  Partials      722      722

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 7cb847b...92243ff. Read the comment docs.

I'm curious about the second fix: returning no error when the file contents are empty. Seems like the caller should be fixing that on error rather than have a function readFileTimestamp hide the underlying problem? I don't have much experience with this particular code, so maybe this is reasonable?

However, I would assume the right solution is to re-write a partially or incorrectly written file w/timestamp when it is discovered to be empty or interrupted during the initial write/create.

However, I would assume the right solution is to re-write a partially or incorrectly written file w/timestamp when it is discovered to be empty or interrupted during the initial write/create.

Ingests should be temporary and we should avoid read side-effects which write. Treating an empty file as an empty timestamp seems reasonable. However I think if this problem is manifesting itself in the first place, we should not be using ioutil.WriteFile and use an atomic implementation which does the write to temp name + rename approach. There is one in continuity which is not exposed (atomicWriteFile) but we could expose one from there (do not use github.com/docker/docker/pkg/ioutils).

@dmcgowan @estesp thanks for your opinions

If i should first create a pr to expose the atomicWriteFile function for github.com/containerd/continuity; then update the containerd/containerd vendor and use the exposed AtomicWriteFile function ?

add a pr in containerd/continuity @dmcgowan PTLA , thanks

Encounter an error when update containerd/continuity package, fs.DiskUsage has changed params: add a context.Context. so all functions used fs.DiskUsage are not worked now.

CI failed: seems to docker.io/library/nginx:latest image changed, i pull this image local also encounter an error:

#pouch pull docker.io/library/nginx:latest
docker.io/library/nginx:latest:                                                resolved       |++++++++++++++++++++++++++++++++++++++|
index-sha256:db196c86553472e3e6a0b2ef1a6220f1c353bf4f92799c0534e6442bf17188fe: done           |++++++++++++++++++++++++++++++++++++++|
elapsed: 5.7 s                                                                 total:   0.0 B (0.0 B/s)
Error: failed to display progress: failed to pull image: manifest sha256:db196c86553472e3e6a0b2ef1a6220f1c353bf4f92799c0534e6442bf17188fe: not found

E0829 14:16:55.615921   10579 remote_image.go:108] PullImage "nginx:latest" from image service failed: rpc error: code = Unknown desc = failed to pull and unpack image "docker.io/library/nginx:latest": failed to unpack image on snapshotter overlayfs: no match for platform in manifest sha256:db196c86553472e3e6a0b2ef1a6220f1c353bf4f92799c0534e6442bf17188fe: not found
Aug 29 14:16:55.615: INFO: Unexpected error occurred: rpc error: code = Unknown desc = failed to pull and unpack image "docker.io/library/nginx:latest": failed to unpack image on snapshotter overlayfs: no match for platform in manifest sha256:db196c86553472e3e6a0b2ef1a6220f1c353bf4f92799c0534e6442bf17188fe: not found

docker-library/official-images#4789

So the CI is happy now, PTAL @estesp @dmcgowan thanks

LGTM