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

[27.x backport] c8d/load: Multi-platform fixes #48376

Merged
merged 3 commits into from
Aug 27, 2024
Merged

Conversation

vvoland
Copy link
Contributor

@vvoland vvoland commented Aug 26, 2024

- What I did
Fixed some cases of docker load failing when loading a multi-platform image that contains platform incompatible with the host OS.

- How I did it
See commits.

- How to verify it

- Description for the changelog

- containerd image store: Fix early error exit from `docker load` in cases where unpacking the image would fail

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

Log the error to the progress output instead.
The image is still loaded into the content store and image service even
if the unpacking failed, so don't error out the whole operation to avoid
missing the load events for other image names loaded from the same
archive.

Signed-off-by: Paweł Gronowski <pawel.gronowski@docker.com>
(cherry picked from commit 728894b)
Signed-off-by: Paweł Gronowski <pawel.gronowski@docker.com>
When loading a multi-platform image, it's not necessary to unpack all
platforms, especially those which have a completely different OS.

Signed-off-by: Paweł Gronowski <pawel.gronowski@docker.com>
(cherry picked from commit 6ebe6a7)
Signed-off-by: Paweł Gronowski <pawel.gronowski@docker.com>
@vvoland vvoland added status/2-code-review kind/enhancement Enhancements are not bugs or new features but can improve usability or performance. area/images kind/bugfix PR's that fix bugs containerd-integration Issues and PRs related to containerd integration labels Aug 26, 2024
@vvoland vvoland added this to the 27.2.0 milestone Aug 26, 2024
@vvoland vvoland self-assigned this Aug 26, 2024
@thaJeztah
Copy link
Member

Something missing in this branch;

47.03 # github.com/docker/docker/daemon/containerd
47.03 daemon/containerd/image_exporter.go:300:10: i.hostPlatformMatcher undefined (type *ImageService has no field or method hostPlatformMatcher)
59.15 + rm -f /go/src/github.com/docker/docker/go.mod

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 if green

@thaJeztah
Copy link
Member

Arf; linting failure

daemon/containerd/platform_matchers.go:28:6: type `matchComparerProvider` is unused (unused)
type matchComparerProvider func(ocispec.Platform) platforms.MatchComparer
     ^
daemon/containerd/platform_matchers.go:30:24: func `(*ImageService).matchRequestedOrDefault` is unused (unused)
func (i *ImageService) matchRequestedOrDefault(
                       ^

Subset of 842c5c5 which only adds the
`hostPlatformMatcher` method.

Signed-off-by: Paweł Gronowski <pawel.gronowski@docker.com>
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

@vvoland vvoland merged commit 2a13a38 into moby:27.x Aug 27, 2024
139 checks passed
renovate bot added a commit to earthly/dind that referenced this pull request Sep 2, 2024
This PR contains the following updates:

| Package | Update | Change |
|---|---|---|
| [docker/docker](https://redirect.github.com/docker/docker) | minor |
`27.1.2` -> `27.2.0` |

---

### Release Notes

<details>
<summary>docker/docker (docker/docker)</summary>

###
[`v27.2.0`](https://redirect.github.com/moby/moby/releases/tag/v27.2.0)

[Compare
Source](https://redirect.github.com/docker/docker/compare/v27.1.2...v27.2.0-rc.1)

#### 27.2.0

For a full list of pull requests and changes in this release, refer to
the relevant GitHub milestones:

- [docker/cli, 27.2.0
milestone](https://redirect.github.com/docker/cli/issues?q=is%3Aclosed+milestone%3A27.2.0)
- [moby/moby, 27.2.0
milestone](https://redirect.github.com/moby/moby/issues?q=is%3Aclosed+milestone%3A27.2.0)
- Deprecated and removed features, see [Deprecated
Features](https://redirect.github.com/docker/cli/blob/v27.2.0/docs/deprecated.md).
- Changes to the Engine API, see [API version
history](https://redirect.github.com/moby/moby/blob/v27.2.0/docs/api/version-history.md).

##### New

- CLI: Add support for device-code flow login when authenticating to the
official registry.
[docker/cli#5349](https://redirect.github.com/docker/cli/pull/5349)
- containerd image store: `docker image ls` now supports `--tree` flag
that shows a multiplatform-aware image list. This is experimental and
may change at any time without any backwards compatibility.
[docker/cli#5353](https://redirect.github.com/docker/cli/pull/5353)

##### API

- `GET /images/json` response now includes `Manifests` field, which
contains information about the sub-manifests included in the image
index. This includes things like platform-specific manifests and build
attestations.
The new field will only be populated if the request also sets the
`manifests` query parameter to `true`.

> \[!WARNING]
>
> This is experimental and may change at any time without any backward
compatibility.

##### Bug fixes and enhancements

- CLI: Fix issue with remote contexts over SSH where the CLI would
allocate a pseudoterminal when connecting to the remote host, which
causes issues in rare situations.
[docker/cli#5351](https://redirect.github.com/docker/cli/pull/5351)
- Fix an issue that prevented network creation with a `--ip-range`
ending on a 64-bit boundary.
[moby/moby#48326](https://redirect.github.com/moby/moby/pull/48326)
- CLI: IPv6 addresses shown by `docker ps` in port bindings are now
bracketed.
[docker/cli#5365](https://redirect.github.com/docker/cli/pull/5365)
- containerd image store: Fix early error exit from `docker load` in
cases where unpacking the image would fail.
[moby/moby#48376](https://redirect.github.com/moby/moby/pull/48376)
- containerd image store: Fix the previous image not being persisted as
dangling after `docker pull`.
[moby/moby#48380](https://redirect.github.com/moby/moby/pull/48380)

##### Packaging updates

- Update BuildKit to
[v0.15.2](https://redirect.github.com/moby/buildkit/releases/tag/v0.15.2).
[moby/moby#48341](https://redirect.github.com/moby/moby/pull/48341)
- Update Compose to
[v2.29.2](https://redirect.github.com/docker/compose/releases/tag/v2.29.2).
[docker/docker-ce-packaging#1050](https://redirect.github.com/docker/docker-ce-packaging/pull/1050)
- The canonical source for the dockerd(8) man page has been moved back
to the same source tree as dockerd itself.
[moby/moby#48378](https://redirect.github.com/moby/moby/pull/48378)
- Update containerd to
[v1.7.21](https://redirect.github.com/containerd/containerd/releases/tag/v1.7.21).
[moby/moby#48383](https://redirect.github.com/moby/moby/pull/48383),
[docker/containerd-packaging#389](https://redirect.github.com/docker/containerd-packaging/pull/389)

</details>

---

### Configuration

📅 **Schedule**: Branch creation - "after 6am on monday" (UTC), Automerge
- At any time (no schedule defined).

🚦 **Automerge**: Enabled.

♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the
rebase/retry checkbox.

🔕 **Ignore**: Close this PR and you won't be reminded about this update
again.

---

- [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check
this box

---

This PR was generated by [Mend Renovate](https://mend.io/renovate/).
View the [repository job
log](https://developer.mend.io/github/earthly/dind).

<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzOC41OS4yIiwidXBkYXRlZEluVmVyIjoiMzguNTkuMiIsInRhcmdldEJyYW5jaCI6Im1haW4iLCJsYWJlbHMiOlsicmVub3ZhdGUiXX0=-->

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
renovate bot added a commit to earthly/dind that referenced this pull request Sep 2, 2024
This PR contains the following updates:

| Package | Update | Change |
|---|---|---|
| [docker/docker](https://redirect.github.com/docker/docker) | minor |
`27.1.2` -> `27.2.0` |

---

### Release Notes

<details>
<summary>docker/docker (docker/docker)</summary>

###
[`v27.2.0`](https://redirect.github.com/moby/moby/releases/tag/v27.2.0)

[Compare
Source](https://redirect.github.com/docker/docker/compare/v27.1.2...v27.2.0-rc.1)

#### 27.2.0

For a full list of pull requests and changes in this release, refer to
the relevant GitHub milestones:

- [docker/cli, 27.2.0
milestone](https://redirect.github.com/docker/cli/issues?q=is%3Aclosed+milestone%3A27.2.0)
- [moby/moby, 27.2.0
milestone](https://redirect.github.com/moby/moby/issues?q=is%3Aclosed+milestone%3A27.2.0)
- Deprecated and removed features, see [Deprecated
Features](https://redirect.github.com/docker/cli/blob/v27.2.0/docs/deprecated.md).
- Changes to the Engine API, see [API version
history](https://redirect.github.com/moby/moby/blob/v27.2.0/docs/api/version-history.md).

##### New

- CLI: Add support for device-code flow login when authenticating to the
official registry.
[docker/cli#5349](https://redirect.github.com/docker/cli/pull/5349)
- containerd image store: `docker image ls` now supports `--tree` flag
that shows a multiplatform-aware image list. This is experimental and
may change at any time without any backwards compatibility.
[docker/cli#5353](https://redirect.github.com/docker/cli/pull/5353)

##### API

- `GET /images/json` response now includes `Manifests` field, which
contains information about the sub-manifests included in the image
index. This includes things like platform-specific manifests and build
attestations.
The new field will only be populated if the request also sets the
`manifests` query parameter to `true`.

> \[!WARNING]
>
> This is experimental and may change at any time without any backward
compatibility.

##### Bug fixes and enhancements

- CLI: Fix issue with remote contexts over SSH where the CLI would
allocate a pseudoterminal when connecting to the remote host, which
causes issues in rare situations.
[docker/cli#5351](https://redirect.github.com/docker/cli/pull/5351)
- Fix an issue that prevented network creation with a `--ip-range`
ending on a 64-bit boundary.
[moby/moby#48326](https://redirect.github.com/moby/moby/pull/48326)
- CLI: IPv6 addresses shown by `docker ps` in port bindings are now
bracketed.
[docker/cli#5365](https://redirect.github.com/docker/cli/pull/5365)
- containerd image store: Fix early error exit from `docker load` in
cases where unpacking the image would fail.
[moby/moby#48376](https://redirect.github.com/moby/moby/pull/48376)
- containerd image store: Fix the previous image not being persisted as
dangling after `docker pull`.
[moby/moby#48380](https://redirect.github.com/moby/moby/pull/48380)

##### Packaging updates

- Update BuildKit to
[v0.15.2](https://redirect.github.com/moby/buildkit/releases/tag/v0.15.2).
[moby/moby#48341](https://redirect.github.com/moby/moby/pull/48341)
- Update Compose to
[v2.29.2](https://redirect.github.com/docker/compose/releases/tag/v2.29.2).
[docker/docker-ce-packaging#1050](https://redirect.github.com/docker/docker-ce-packaging/pull/1050)
- The canonical source for the dockerd(8) man page has been moved back
to the same source tree as dockerd itself.
[moby/moby#48378](https://redirect.github.com/moby/moby/pull/48378)
- Update containerd to
[v1.7.21](https://redirect.github.com/containerd/containerd/releases/tag/v1.7.21).
[moby/moby#48383](https://redirect.github.com/moby/moby/pull/48383),
[docker/containerd-packaging#389](https://redirect.github.com/docker/containerd-packaging/pull/389)

</details>

---

### Configuration

📅 **Schedule**: Branch creation - "after 6am on monday" (UTC), Automerge
- At any time (no schedule defined).

🚦 **Automerge**: Enabled.

♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the
rebase/retry checkbox.

🔕 **Ignore**: Close this PR and you won't be reminded about this update
again.

---

- [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check
this box

---

This PR was generated by [Mend Renovate](https://mend.io/renovate/).
View the [repository job
log](https://developer.mend.io/github/earthly/dind).

<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzOC41OS4yIiwidXBkYXRlZEluVmVyIjoiMzguNTkuMiIsInRhcmdldEJyYW5jaCI6Im1haW4iLCJsYWJlbHMiOlsicmVub3ZhdGUiXX0=-->

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/images containerd-integration Issues and PRs related to containerd integration kind/bugfix PR's that fix bugs kind/enhancement Enhancements are not bugs or new features but can improve usability or performance. status/2-code-review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants