docker/save: stable timestamp for blobs/digest dir by SamStudio8 · Pull Request #51365 · moby/moby

SamStudio8 · 2025-10-31T13:31:20Z

- What I did

Fixes #51364: files generated by docker save with the overlay2 storage driver are now consistent across time.

- How I did it

This change moves the call to system.Chtimes to set the mtime of the blobs/digest directory to 0 after writing the OCI manifest file.

- How to verify it

Generate a tar with docker save after applying the patch:

$ docker pull ubuntu:24.04
$ docker save ubuntu:24.04 > save.tar
$ tar tvf save.tar
  drwxr-xr-x 0/0               0 2025-10-01 13:01 blobs/
> drwxr-xr-x 0/0               0 1970-01-01 00:00 blobs/sha256/
  -rw-r--r-- 0/0        80634368 2025-10-01 13:01 blobs/sha256/073ec47a8c22dcaa4d6e5758799ccefe2f9bde943685830b1bf6fd2395f5eabc
  -rw-r--r-- 0/0            1288 2025-10-01 13:01 blobs/sha256/8346eb021ab33bee3b3a4a2022f8274dd4581aec2b5dad83a156b0a272907030
  -rw-r--r-- 0/0            2297 2025-10-01 13:01 blobs/sha256/97bed23a34971024aa8d254abbe67b7168772340d1f494034773bc464e8dd5b6
  -rw-r--r-- 0/0             402 1970-01-01 00:00 blobs/sha256/a80ae87142a0ae1f9e02e3561844d7004a28886aaca46db5392dd556df7326f2
  -rw-r--r-- 0/0             360 1970-01-01 00:00 index.json
  -rw-r--r-- 0/0             457 1970-01-01 00:00 manifest.json
  -rw-r--r-- 0/0              31 1970-01-01 00:00 oci-layout
  -rw-r--r-- 0/0              88 1970-01-01 00:00 repositories

- Human readable description for the release notes

`docker save`: Fixed inconsistent tar member timestamps when exporting images with the overlay2 storage driver.

- A picture of a cute animal (not mandatory but encouraged)

Please enjoy this picture of Daisy Doodle, our toy poodle:

vvoland · 2025-10-31T13:35:00Z

Thanks! ❤️

This test will also need adjustment:

moby/integration/image/save_test.go

Line 87 in 560dfb1

    
           assert.Check(t, is.Equal(info.ModTime().Format(time.RFC3339), created.Format(time.RFC3339)))

SamStudio8 · 2025-10-31T13:48:21Z

Thanks! ❤️

This test will also need adjustment:

moby/integration/image/save_test.go

Line 87 in 560dfb1

assert.Check(t, is.Equal(info.ModTime().Format(time.RFC3339), created.Format(time.RFC3339)))

Oops thanks. Will address!

thaJeztah · 2025-10-31T14:08:19Z

This change moves the call to system.Chtimes to set the mtime of the blobs/digest directory to 0 after writing the OCI manifest file.

Nice catch! I saw the PR (and ticket) and thought "didn't we fix that?", but that makes sense!

thaJeztah

LGTM, thanks!

@vvoland LGTY?

SamStudio8 · 2025-10-31T17:00:32Z

Hey @thaJeztah, I'm just coming back to update that test to check all the members of the tar aren't newer than the img.Created (or 0, in the containerd case).

vvoland · 2025-10-31T17:01:29Z

I think we might be missing something - the test didn't fail and it should I think?

SamStudio8 · 2025-10-31T17:02:54Z

@vvoland The test doesn't fail as it only checks that the OCI manifest has a timestamp of 0 (which it did previously). None of the other members of the tar are actually checked.

vvoland · 2025-10-31T17:05:03Z

moby/integration/image/save_test.go

Line 77 in 560dfb1

info, err := fs.Stat(tarfs, ls[0].Config)

Doesn't it check the image Config?

In the test, we should just remove the branching and make sure that in both cases the ModTime is 0:

moby/integration/image/save_test.go

Lines 83 to 88 in 560dfb1

    
           if testEnv.UsingSnapshotter() { 
        
           	// containerd archive export sets the mod time to zero. 
        
           	assert.Check(t, is.Equal(info.ModTime(), time.Unix(0, 0))) 
        
           } else { 
        
           	assert.Check(t, is.Equal(info.ModTime().Format(time.RFC3339), created.Format(time.RFC3339))) 
        
           }

SamStudio8 · 2025-10-31T17:09:52Z

Sorry @vvoland, I don't know the terminology very well, does ls[0].Config refer to the OCI manifest written here or something else? If the former, you're right that something is up - as that test should already be failing if testEnv.UsingSnapshotter() is not true, as it has always been set to mtime 0... Perhaps it is not run under both conditions?

vvoland · 2025-10-31T17:15:50Z

No, it's the image config https://github.com/opencontainers/image-spec/blob/main/config.md referenced by the legacy manifest.json file (backwards compatibility with the old Docker image).

It's written in:

moby/daemon/internal/image/tarexport/save.go

Line 490 in 3287abc

if err := os.WriteFile(configFile, img.RawJSON(), 0o644); err != nil {

vvoland · 2025-10-31T17:16:20Z

But, we should probably just change that test so it verifies that all files inside the tar have mtime 0.

SamStudio8 · 2025-10-31T17:40:01Z

@vvoland Ah thanks for clarifying. Sorry for the confusion here! This MR only fixes the contemporary timestamp on the blobs/{digest} directory itself. I haven't changed anything related to the timestamps of other tar members. So the test still passes here as we'd still expect that Config to have a non-zero timestamp propagated from img.Created (when testEnv.UsingSnapshotter is false).

In the spirit of the existing code (that attempts to maintain the mtime of img.Created) I was thinking the test should:

Check all tar members are not newer than 0 if testEnv.UsingSnapshotter() is True
Otherwise check all tar members are not newer than img.Created

But do you think we should instead expand the scope here to ignore img.Created and always reset mtime for all members in the tar?

SamStudio8 · 2025-11-03T13:00:53Z

I have unapplied the patch in this MR and updated the test as I suggested above. Running this test without the containerd storage driver, I can demonstrate the updated test catches the bug described by #51364. That is, the test identifies the contemporary timestamp on the blobs/{digest} directory is newer than img.Created.

$ DOCKER_GRAPHDRIVER=overlay2 TEST_INTEGRATION_USE_GRAPHDRIVER="1" TESTFLAGS="-test.run ^TestSaveCheckTimes$" make test-integration
...
Running /go/src/github.com/docker/docker/integration/image (amd64.docker.docker.integration.image) flags=-test.v -test.timeout=10m -test.run ^TestSaveCheckTimes$                                                      12:30:03 [184/1888]
INFO: Testing against a local daemon
=== RUN   TestSaveCheckTimes
=== PAUSE TestSaveCheckTimes
=== CONT  TestSaveCheckTimes
    save_test.go:86: checking blobs/ (modtime=2020-06-02 21:19:57 +0000 UTC)
    save_test.go:86: checking blobs/sha256/ (modtime=2025-11-03 12:30:03 +0000 UTC)
    save_test.go:87: assertion failed: modtime.After(threshold) is true: blobs/sha256/ has modtime 2025-11-03 12:30:03 +0000 UTC after 2020-06-02 21:19:57.279412246 +0000 UTC
    save_test.go:86: checking blobs/sha256/1be74353c3d0fd55fb5638a52953e6f1bc441e5b1710921db9ec2aa202725569 (modtime=2020-06-02 21:19:57 +0000 UTC)
    save_test.go:86: checking blobs/sha256/1c35c441208254cb7c3844ba95a96485388cef9ccc0646d562c7fc026e04c807 (modtime=2020-06-02 21:19:57 +0000 UTC)
    save_test.go:86: checking blobs/sha256/5d68854a5204233cd7872a5a6346afcca085cd5af4d3be7c3d26f795248350bb (modtime=1970-01-01 00:00:00 +0000 UTC)
    save_test.go:86: checking blobs/sha256/5fd46bbd12cd97c83beb8a563186270bb09d0e62ad43c024859c1c2bfb20fb0f (modtime=2020-06-02 21:19:57 +0000 UTC)
    save_test.go:86: checking index.json (modtime=1970-01-01 00:00:00 +0000 UTC)
    save_test.go:86: checking manifest.json (modtime=1970-01-01 00:00:00 +0000 UTC)
    save_test.go:86: checking oci-layout (modtime=1970-01-01 00:00:00 +0000 UTC)                                                                                                                                                           
    save_test.go:86: checking repositories (modtime=1970-01-01 00:00:00 +0000 UTC)
--- FAIL: TestSaveCheckTimes (0.07s)
FAIL

The containerd case passes as expected as all the timestamps appear to be sanitised to mtime 0 anyway:

$ TESTFLAGS="-test.run ^TestSaveCheckTimes$" make test-integration
...
Running /go/src/github.com/docker/docker/integration/image (amd64.docker.docker.integration.image) flags=-test.v -test.timeout=10m -test.run ^TestSaveCheckTimes$
INFO: Testing against a local daemon                                                                                                                                                                                                      === RUN   TestSaveCheckTimes
=== PAUSE TestSaveCheckTimes                                                                                                                                                                                                              === CONT  TestSaveCheckTimes
    save_test.go:86: checking blobs/ (modtime=1970-01-01 00:00:00 +0000 UTC)
    save_test.go:86: checking blobs/sha256/ (modtime=1970-01-01 00:00:00 +0000 UTC)                                                                                                                                                           
    save_test.go:86: checking blobs/sha256/1c35c441208254cb7c3844ba95a96485388cef9ccc0646d562c7fc026e04c807 (modtime=1970-01-01 00:00:00 +0000 UTC)
    save_test.go:86: checking blobs/sha256/328dac05c07eb589d3a783c8bd2bc264f38dbcf1f71d00844804bd4c4aba3597 (modtime=1970-01-01 00:00:00 +0000 UTC)
    save_test.go:86: checking blobs/sha256/76df9210b28cbd4bc127844914d0a23937ed213048dc6289b2a2d4f7d675c75e (modtime=1970-01-01 00:00:00 +0000 UTC)
    save_test.go:86: checking index.json (modtime=1970-01-01 00:00:00 +0000 UTC)                                                                                                                                                              
    save_test.go:86: checking manifest.json (modtime=1970-01-01 00:00:00 +0000 UTC)
    save_test.go:86: checking oci-layout (modtime=1970-01-01 00:00:00 +0000 UTC)
--- PASS: TestSaveCheckTimes (0.20s)
PASS

Reapplying the patch in this MR, the test now passes under the overlay2 condition as the blobs/{digest} directory is now set to 0. In retrospect, perhaps this should actually be set to img.Created...

=== RUN   TestSaveCheckTimes
=== PAUSE TestSaveCheckTimes                                                                                                                                                                                                              === CONT  TestSaveCheckTimes
    save_test.go:86: checking blobs/ (modtime=2020-06-02 21:19:57 +0000 UTC)
    save_test.go:86: checking blobs/sha256/ (modtime=1970-01-01 00:00:00 +0000 UTC)
    save_test.go:86: checking blobs/sha256/1be74353c3d0fd55fb5638a52953e6f1bc441e5b1710921db9ec2aa202725569 (modtime=2020-06-02 21:19:57 +0000 UTC)
    save_test.go:86: checking blobs/sha256/1c35c441208254cb7c3844ba95a96485388cef9ccc0646d562c7fc026e04c807 (modtime=2020-06-02 21:19:57 +0000 UTC)
    save_test.go:86: checking blobs/sha256/5d68854a5204233cd7872a5a6346afcca085cd5af4d3be7c3d26f795248350bb (modtime=1970-01-01 00:00:00 +0000 UTC)
    save_test.go:86: checking blobs/sha256/5fd46bbd12cd97c83beb8a563186270bb09d0e62ad43c024859c1c2bfb20fb0f (modtime=2020-06-02 21:19:57 +0000 UTC)
    save_test.go:86: checking index.json (modtime=1970-01-01 00:00:00 +0000 UTC)
    save_test.go:86: checking manifest.json (modtime=1970-01-01 00:00:00 +0000 UTC)
    save_test.go:86: checking oci-layout (modtime=1970-01-01 00:00:00 +0000 UTC)
    save_test.go:86: checking repositories (modtime=1970-01-01 00:00:00 +0000 UTC)
--- PASS: TestSaveCheckTimes (0.05s)                                                                                                                                                                                                      PASS

I've updated the commit in this MR to include the changes to the test. Note that I temporarily added a log line to the test to indicate the tar member name and mtime, just to show to you here that all tar members are checked and to emphasise the existing difference in behaviour between the two storage drivers; this log line is not in the MR.

I believe this MR now correctly tests for and fixes the unexpected behaviour described by #51364. I'd suggest further discussion on whether the mtimes should always be 0 is a new issue, as the existing code clearly indicates an intention to maintain img.Created on some of the tar members, for reasons unknown to me. I'd be happy to make this change and submit a new MR to change mtime to 0 in all cases if that is desired.

Writing the OCI manifest file to the blobs/digest dir will update the directory mtime, producing a tar file containing a member with a contemporary mtime. Exported tars for the same image will therefore have different checksums. Although this was previously addressed by overriding the mtime manually to 0, this was done before the OCI manifest file was written. This change simply moves the call to system.Chtimes to set the mtime of the blobs/digest directory to 0 after writing the OCI manifest file. This commit also updates the TestSaveCheckTimes integration test to check the mtime of all members in the exported tar to ensure that all mtime are not newer than img.Created or 0 (depending on whether the containerd-snapshotter is disabled or enabled, respectively). Signed-off-by: Sam Nicholls <sam.nicholls@nanoporetech.com>

vvoland

LGTM; still think we should drop the old behavior and just set mtime for all files to 0, just like the containerd implementation does

thaJeztah · 2025-11-05T12:52:39Z

still think we should drop the old behavior and just set mtime for all files to 0, just like the containerd implementation does

Yeah, I think that would make sense; can be done in a follow-up I guess?

thaJeztah

LGTM

vvoland · 2025-11-05T13:04:35Z

Yep, fine for a follow up!

@SamStudio8 feel free to continue in a new issue/PR if you're still interested in this one.

SamStudio8 · 2025-11-05T13:56:11Z

@vvoland @thaJeztah Thanks both for your time!

feel free to continue in a new issue/PR if you're still interested in this one.

Happy to pick this up, see you on the next MR.

github-actions bot added the area/daemon Core Engine label Oct 31, 2025

vvoland added kind/bugfix PR's that fix bugs area/images Image Distribution labels Oct 31, 2025

vvoland added this to the 29.0.0 milestone Oct 31, 2025

vvoland added the impact/changelog label Oct 31, 2025

thaJeztah added the process/cherry-pick/28.x label Oct 31, 2025

thaJeztah approved these changes Oct 31, 2025

View reviewed changes

SamStudio8 force-pushed the 51364-dockersave branch from 3990e8a to daa5188 Compare November 3, 2025 12:52

SamStudio8 force-pushed the 51364-dockersave branch 2 times, most recently from ef3e08b to 34af3d7 Compare November 3, 2025 13:13

SamStudio8 force-pushed the 51364-dockersave branch from 34af3d7 to 668b546 Compare November 3, 2025 13:15

SamStudio8 requested a review from thaJeztah November 3, 2025 17:04

vvoland approved these changes Nov 4, 2025

View reviewed changes

thaJeztah approved these changes Nov 5, 2025

View reviewed changes

vvoland merged commit 25f880c into moby:master Nov 5, 2025
359 of 362 checks passed

Conversation

SamStudio8 commented Oct 31, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

vvoland commented Oct 31, 2025

Uh oh!

SamStudio8 commented Oct 31, 2025

Uh oh!

thaJeztah commented Oct 31, 2025

Uh oh!

thaJeztah left a comment

Choose a reason for hiding this comment

Uh oh!

SamStudio8 commented Oct 31, 2025

Uh oh!

vvoland commented Oct 31, 2025

Uh oh!

SamStudio8 commented Oct 31, 2025

Uh oh!

vvoland commented Oct 31, 2025

Uh oh!

SamStudio8 commented Oct 31, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

vvoland commented Oct 31, 2025

Uh oh!

vvoland commented Oct 31, 2025

Uh oh!

SamStudio8 commented Oct 31, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

SamStudio8 commented Nov 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

vvoland left a comment

Choose a reason for hiding this comment

Uh oh!

thaJeztah commented Nov 5, 2025

Uh oh!

thaJeztah left a comment

Choose a reason for hiding this comment

Uh oh!

vvoland commented Nov 5, 2025

Uh oh!

Uh oh!

SamStudio8 commented Nov 5, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

SamStudio8 commented Oct 31, 2025 •

edited

Loading

SamStudio8 commented Oct 31, 2025 •

edited

Loading

SamStudio8 commented Oct 31, 2025 •

edited

Loading

SamStudio8 commented Nov 3, 2025 •

edited

Loading