Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[24.0 backport] c8d/legacybuilder: Fix mismatched image rootfs errors #46310

Merged
merged 1 commit into from Aug 24, 2023

Conversation

vvoland
Copy link
Contributor

@vvoland vvoland commented Aug 24, 2023

- What I did
Fixed CreateImage losing the parent layers when creating a child image.

This fixes failed to export image: mismatched image rootfs and manifest layers errors in integration-cli build tests with c8d enabled (#45232). Although this doesn't make all these tests green, as they now fail due to: #46194.

- How I did it
Clone the passed Image and replace its ID instead of recreating it from scratch (and not setting all needed fields).

- How to verify it
Before

$ make  DOCKER_GRAPHDRIVER=overlayfs TEST_INTEGRATION_USE_SNAPSHOTTER=1  TEST_FILTER='TestBuildAddBadLinks'  test-integration
...
=== RUN   TestDockerCLIBuildSuite/TestBuildAddBadLinks
    docker_cli_build_test.go:998: assertion failed: 
        Command:  /usr/local/cli-integration/docker build -t test-link-absolute .
        ExitCode: 1
        Error:    exit status 1
        Stdout:   Sending build context to Docker daemon   5.12kB


        Step 1/3 : FROM scratch
         ---> 
        Step 2/3 : ADD links.tar /
         ---> 7f60960c7521
        Step 3/3 : ADD foo.txt /symlink/
        
        Stderr:   failed to export image: mismatched image rootfs and manifest layers
        
        
        Failures:
        ExitCode was 1 expected 0
        Expected no error
...

After

$ make  DOCKER_GRAPHDRIVER=overlayfs TEST_INTEGRATION_USE_SNAPSHOTTER=1  TEST_FILTER='TestBuildAddBadLinks'  test-integration
...
--- PASS: TestDockerCLIBuildSuite (3.24s)
    --- PASS: TestDockerCLIBuildSuite/TestBuildAddBadLinks (2.32s)
    --- PASS: TestDockerCLIBuildSuite/TestBuildAddBadLinksVolume (0.92s)
...

- Description for the changelog

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

Previous image created a new partially filled image.
This caused child images to lose their parent's layers.

Instead of creating a new object and trying to replace its fields, just
clone the original passed image and change its ID to the manifest
digest.

Signed-off-by: Paweł Gronowski <pawel.gronowski@docker.com>
(cherry picked from commit 01214ba)
Signed-off-by: Paweł Gronowski <pawel.gronowski@docker.com>
@vvoland vvoland added area/builder kind/bugfix PR's that fix bugs area/builder/classic-builder Issues affecting the classic builder containerd-integration Issues and PRs related to containerd integration labels Aug 24, 2023
@vvoland vvoland added this to the 24.0.6 milestone Aug 24, 2023
Copy link
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

@thaJeztah
Copy link
Member

Some unrelated failure;

/usr/bin/docker buildx bake --set dev.cache-from=type=gha,scope=dev --metadata-file /tmp/docker-build-push-cWA6A1/metadata-file dev
error: dial unix /run/buildkit/buildkitd.sock: connect: no such file or directory
ERROR: listing workers for Build: failed to list workers: Unavailable: connection error: desc = "error reading server preface: EOF"
Error: buildx bake failed with: ERROR: listing workers for Build: failed to list workers: Unavailable: connection error: desc = "error reading server preface: EOF"

@thaJeztah thaJeztah merged commit 8ff9ef2 into moby:24.0 Aug 24, 2023
105 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/builder/classic-builder Issues affecting the classic builder area/builder containerd-integration Issues and PRs related to containerd integration kind/bugfix PR's that fix bugs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants