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

vendor: github.com/containerd/containerd v1.7.19, migrate to github.com/containerd/platforms module #47142

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

thaJeztah
Copy link
Member

@thaJeztah thaJeztah commented Jan 20, 2024

Migrate to github.com/containerd/platforms module

Containerd's "platforms" package is being moved to a separate module. This
allows updating the platforms parsing independent of the containerd module
itself.

For existing containerd versions (1.6, 1.7), the package in containerd will
be an alias for the new module.

- Description for the changelog

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

@thaJeztah thaJeztah added status/2-code-review kind/refactor PR's that refactor, or clean-up code labels Jan 20, 2024
@thaJeztah thaJeztah added this to the 26.0.0 milestone Jan 20, 2024
@thaJeztah thaJeztah self-assigned this Jan 20, 2024
@thaJeztah
Copy link
Member Author

Ah, looks like some updates are needed;

# github.com/docker/docker/libcontainerd/local
libcontainerd/local/local_windows.go:944:34: cannot use p[i].CreateTimestamp (variable of type time.Time) as *timestamppb.Timestamp value in struct literal

@thaJeztah
Copy link
Member Author

@thaJeztah
Copy link
Member Author

Two tests failing that look related, looks like there's some change in behavior in the package;

=== FAIL: github.com/docker/docker/integration/build TestBuildPlatformInvalid (0.06s)
    build_test.go:685: assertion failed: expression is false: errdefs.IsInvalidParameter(err)

Platform: "foobar",
})
assert.Assert(t, err != nil)
assert.ErrorContains(t, err, "unknown operating system or architecture")
assert.Assert(t, errdefs.IsInvalidParameter(err))

=== FAIL: amd64.integration.image TestImagePullPlatformInvalid (0.01s)
    pull_test.go:37: assertion failed: expression is false: errdefs.IsInvalidParameter(err)

_, err := client.ImagePull(ctx, "docker.io/library/hello-world:latest", types.ImagePullOptions{Platform: "foobar"})
assert.Assert(t, err != nil)
assert.ErrorContains(t, err, "unknown operating system or architecture")
assert.Assert(t, errdefs.IsInvalidParameter(err))

@thaJeztah
Copy link
Member Author

Improved the test-assert a bit, and it looks like it's returning a 500 / internal server error;

=== RUN   TestImagePullPlatformInvalid
    pull_test.go:37: assertion failed: invalid type for expected: func(error) error: errdefs.errSystem: Error response from daemon: "foobar": unknown operating system or architecture: invalid argument
--- FAIL: TestImagePullPlatformInvalid (0.01s)

@thaJeztah
Copy link
Member Author

Daemon logs show that it returns an untyped error, hence using the default (500 / Internal Server error);

time="2024-01-21T00:18:31.260074294Z" level=debug msg="Calling POST /v1.45/images/create?fromImage=hello-world&platform=foobar&tag=latest"
time="2024-01-21T00:18:31.260220544Z" level=debug msg="FIXME: Got an API for which error does not match any expected type!!!" error="\"foobar\": unknown operating system or architecture: invalid argument" error_type="*fmt.wrapError" module=api
time="2024-01-21T00:18:31.260320336Z" level=error msg="Handler for POST /v1.45/images/create returned error: \"foobar\": unknown operating system or architecture: invalid argument"
time="2024-01-21T00:18:31.260407169Z" level=debug msg="FIXME: Got an API for which error does not match any expected type!!!" error="\"foobar\": unknown operating system or architecture: invalid argument" error_type="*fmt.wrapError" module=api

@thaJeztah
Copy link
Member Author

Found the likely cause; the new module doesn't use containerd errdefs;

Screenshot 2024-01-21 at 01 25 19

@thaJeztah
Copy link
Member Author

Some error messages need to be updated in tests;

=== Failed
=== FAIL: amd64.integration.image TestImportWithCustomPlatformReject/_______ (0.00s)
    import_test.go:182: assertion failed: expected error to contain "is an invalid component", got "Error response from daemon: \"       \" is an invalid OS component of \"       \": OSAndVersion specifier component must match \"^([A-Za-z0-9_-]+)(?:\\\\(([A-Za-z0-9_.-]*)\\\\))?$\": invalid argument"
        Error response from daemon: "       " is an invalid OS component of "       ": OSAndVersion specifier component must match "^([A-Za-z0-9_-]+)(?:\\(([A-Za-z0-9_.-]*)\\))?$": invalid argument
    --- FAIL: TestImportWithCustomPlatformReject/_______ (0.00s)

=== FAIL: amd64.integration.image TestImportWithCustomPlatformReject// (0.00s)
    import_test.go:182: assertion failed: expected error to contain "is an invalid component", got "Error response from daemon: \"\" is an invalid OS component of \"/\": OSAndVersion specifier component must match \"^([A-Za-z0-9_-]+)(?:\\\\(([A-Za-z0-9_.-]*)\\\\))?$\": invalid argument"
        Error response from daemon: "" is an invalid OS component of "/": OSAndVersion specifier component must match "^([A-Za-z0-9_-]+)(?:\\(([A-Za-z0-9_.-]*)\\))?$": invalid argument
    --- FAIL: TestImportWithCustomPlatformReject// (0.00s)

=== FAIL: amd64.integration.image TestImportWithCustomPlatformReject (0.01s)

@thaJeztah
Copy link
Member Author

Yay! CodeCov action is having a bad day on Windows;

Screenshot 2024-06-13 at 12 34 25

Highlights

- Fix support for OTLP config
- Add API go module
- Remove overlayfs volatile option on temp mounts
- Update runc binary to v1.1.13
- Migrate platforms package to github.com/containerd/platforms
- Migrate reference/docker package to github.com/distribution/reference

Container Runtime Interface (CRI)

- Fix panic in NRI from nil CRI reference
- Fix Windows HPC working directory

full diff: containerd/containerd@v1.7.18...v1.7.19

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Switch to use github.com/containerd/platforms module, because containerd's
platforms package has moved to a separate module. This allows updating the
platforms parsing independent of the containerd module itself.

The package in containerd is deprecated, but kept as an alias to provide
compatibility between codebases.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
@thaJeztah thaJeztah force-pushed the migrate_to_platforms_module branch from 6f9f5b9 to d0aa3ea Compare July 2, 2024 19:20
@thaJeztah thaJeztah changed the title [WIP] Migrate to github.com/containerd/platforms module vendor: github.com/containerd/containerd v1.7.19 ,migrate to github.com/containerd/platforms module Jul 2, 2024
@thaJeztah thaJeztah changed the title vendor: github.com/containerd/containerd v1.7.19 ,migrate to github.com/containerd/platforms module vendor: github.com/containerd/containerd v1.7.19, migrate to github.com/containerd/platforms module Jul 2, 2024
@thaJeztah thaJeztah marked this pull request as ready for review July 2, 2024 19:21
@thaJeztah thaJeztah requested a review from dmcgowan July 2, 2024 19:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: In Progress
Development

Successfully merging this pull request may close these issues.

None yet

2 participants