[release/1.7] Migrate platforms package to github.com/containerd/platforms by thaJeztah · Pull Request #10292 · containerd/containerd

thaJeztah · 2024-06-03T13:37:07Z

depends on / stacked on [release/1.7 backport] go.mod: github.com/klauspost/compress v1.16.7 #10326

Also include partial backports / patches similar to;

migrate platforms package to github.com/containerd/platforms

This updates the platforms package to be an alias for the new platforms module.
This helps transitioning consumers to the new module, and makes sure that
containerd v2 and v1 use the same definitions.

platforms: mark aliases as deprecated

k8s-ci-robot · 2024-06-03T13:37:09Z

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

thaJeztah · 2024-06-03T13:46:07Z

validate vendor fails, because integration/cli is now depending on a version of hcsshim that wants a newer version of klauspost/compress, and the same is not the case in the main module;

+ verify-vendor
go: downloading github.com/Microsoft/hcsshim/test v0.0.0-20210408205431-da33ecd607e1
go: downloading github.com/klauspost/compress v1.16.5
github.com/klauspost/compress  has different values in the go.mod files require section:  v1.16.0 in root go.mod  v1.16.5 in integration/client/go.mod
Found 1 error(s).
make: *** [Makefile:488: verify-vendor] Error 1
Error: Process completed with exit code 2.

thaJeztah · 2024-06-03T15:13:35Z

Got some failure on Windows that may be related; restarted them just in case it was flaky, but ... it's possible there's some backward-incompatible changes in the platforms module 🤔

thaJeztah · 2024-06-03T17:20:50Z

Same failures again on Windows, but not sure immediately what the error is, or more accurately; how the error relates to this PR :thinking_face: https://github.com/containerd/containerd/actions/runs/9351458248/job/25741492452?pr=10292

=== Failed
=== FAIL: . TestTransferImport/Basic (0.02s)
    import_test.go:681: rpc error: code = Unimplemented desc = unknown service containerd.services.transfer.v1.Transfer
    --- FAIL: TestTransferImport/Basic (0.02s)

=== FAIL: . TestTransferImport/IndexRef (0.02s)
    import_test.go:681: rpc error: code = Unimplemented desc = unknown service containerd.services.transfer.v1.Transfer
    --- FAIL: TestTransferImport/IndexRef (0.02s)

=== FAIL: . TestTransferImport/AllRefs (0.02s)
    import_test.go:681: rpc error: code = Unimplemented desc = unknown service containerd.services.transfer.v1.Transfer
    --- FAIL: TestTransferImport/AllRefs (0.02s)

=== FAIL: . TestTransferImport/DigestRefs (0.02s)
    import_test.go:681: rpc error: code = Unimplemented desc = unknown service containerd.services.transfer.v1.Transfer
    --- FAIL: TestTransferImport/DigestRefs (0.02s)

=== FAIL: . TestTransferImport/DigestRefsSkipNamed (0.02s)
    import_test.go:681: rpc error: code = Unimplemented desc = unknown service containerd.services.transfer.v1.Transfer
    --- FAIL: TestTransferImport/DigestRefsSkipNamed (0.02s)

=== FAIL: . TestTransferImport/DigestOnly (0.02s)
    import_test.go:681: rpc error: code = Unimplemented desc = unknown service containerd.services.transfer.v1.Transfer
    --- FAIL: TestTransferImport/DigestOnly (0.02s)

=== FAIL: . TestTransferImport/OverwriteDigestRefs (0.02s)
    import_test.go:681: rpc error: code = Unimplemented desc = unknown service containerd.services.transfer.v1.Transfer
    --- FAIL: TestTransferImport/OverwriteDigestRefs (0.02s)

=== FAIL: . TestTransferImport/TagOnlyRef (0.02s)
    import_test.go:681: rpc error: code = Unimplemented desc = unknown service containerd.services.transfer.v1.Transfer
    --- FAIL: TestTransferImport/TagOnlyRef (0.02s)

=== FAIL: . TestTransferImport/TagOnlyOverwriteDigestRefs (0.02s)
    import_test.go:681: rpc error: code = Unimplemented desc = unknown service containerd.services.transfer.v1.Transfer
    --- FAIL: TestTransferImport/TagOnlyOverwriteDigestRefs (0.02s)

=== FAIL: . TestTransferImport (0.18s)
    log_hook.go:47: time="2024-06-03T15:42:15.063901100Z" level=error msg="progress stream failed to recv" func="proxy.(*proxyTransferrer).Transfer.func1" file="D:/a/containerd/containerd/src/github.com/containerd/containerd/pkg/transfer/proxy/transfer.go:67" error="grpc: the client connection is closing: context canceled" testcase=TestTransferImport
    log_hook.go:47: time="2024-06-03T15:42:15.063901100Z" level=error msg="progress stream failed to recv" func="proxy.(*proxyTransferrer).Transfer.func1" file="D:/a/containerd/containerd/src/github.com/containerd/containerd/pkg/transfer/proxy/transfer.go:67" error="grpc: the client connection is closing: context canceled" testcase=TestTransferImport

=== FAIL: . TestTransferEcho/ImportExportEchoBig (0.02s)
    transfer_test.go:50: rpc error: code = Unimplemented desc = unknown service containerd.services.transfer.v1.Transfer
    --- FAIL: TestTransferEcho/ImportExportEchoBig (0.02s)

=== FAIL: . TestTransferEcho/ImportExportEchoSmall (0.01s)
    transfer_test.go:50: rpc error: code = Unimplemented desc = unknown service containerd.services.transfer.v1.Transfer
    --- FAIL: TestTransferEcho/ImportExportEchoSmall (0.01s)

=== FAIL: . TestTransferEcho/ImportExportEchoEmpty (0.01s)
    transfer_test.go:50: rpc error: code = Unimplemented desc = unknown service containerd.services.transfer.v1.Transfer
    --- FAIL: TestTransferEcho/ImportExportEchoEmpty (0.01s)

=== FAIL: . TestTransferEcho (0.05s)

dmcgowan · 2024-06-03T17:42:18Z

platforms/platforms_deprecated.go

+//
+// Deprecated: use [platforms.DefaultString].
+func DefaultString() string {
+	return platforms.DefaultString()


Can you try to keep this as return Format(DefaultSpec()), platforms DefaultString() is now using return FormatAll(DefaultSpec()) which includes the OS version.

thx; can give that a try; I had a quick peek and it looks like that's only used in 1 place (in ctr to set the default value).

Perhaps that one should use the underlying code instead 🤔

oh, never mind, looking in the wrong place (imgcrypt). Let me look further.

I pushed a commit, but perhaps we should indeed consider to make this a separate function after all (new function to return default including os-version, and keep the old one without 🤔)

thaJeztah · 2024-06-03T19:09:23Z

Unfortunately looks like that didn't help 🤔

dmcgowan · 2024-06-07T21:40:13Z

The failure is from no matching differ, I added a log to see which match was failing

time="2024-06-07T20:35:53.586462900Z" level=info msg="Differ not used do to platform mismatch" differ=windows-lcow differ_platform="{amd64 linux  [] }" target="{amd64 windows  [] }"
time="2024-06-07T20:35:53.586462900Z" level=info msg="Differ not used do to platform mismatch" differ=walking differ_platform="{amd64 windows 10.0.17763 [] }" target="{amd64 windows  [] }"
time="2024-06-07T20:35:53.586462900Z" level=info msg="Differ not used do to platform mismatch" differ=windows differ_platform="{amd64 windows 10.0.17763 [] }" target="{amd64 windows  [] }"

The target platform doesn't have os version and does not match Windows with an os version...

thaJeztah · 2024-06-07T22:05:48Z

Thanks! So could indeed be code that's depending on "Parse" to fill in the system's version perhaps.

Also.. not near my computer, but now I want to open a PR to find where that grammar error is ("do to" -> "due to") 🤣🤣

Differ not used do to platform mismatch

dmcgowan · 2024-06-07T22:07:34Z

Thats just from a debug commit of mine, I'll track it down. Oddly the matcher seems to pass in that situation today in unit testing.

dmcgowan · 2024-06-07T23:02:21Z

Opened containerd/platforms#11 to fix

thaJeztah · 2024-06-11T07:23:27Z

I opened a draft PR on main to update the platforms package to current main; And I added a cherry-pick for that here

vendor: github.com/containerd/platforms v0.2.1 #10322

thaJeztah · 2024-06-11T07:24:48Z

client_opts.go

 func WithPlatform(platform string) RemoteOpt {
 	if platform == "" {
-		platform = platforms.DefaultString()
+		platform = platforms.Format(platforms.DefaultSpec()) // For 1.7 continue using the old format without os-version included.


If CI is happy with the updated platforms package, I'll also see if it's still happy if I would revert these changes

thaJeztah · 2024-06-12T00:31:04Z

CI was happy; now reverting one commit to see if it was actually needed

thaJeztah · 2024-06-12T15:51:35Z

ok; it fails with that commit reverted, so looks like we need to keep it indeed

This updates the platforms package to be an alias for the new platforms module. This helps transitioning consumers to the new module, and makes sure that containerd v2 and v1 use the same definitions. Signed-off-by: Sebastiaan van Stijn <[email protected]>

The platforms.DefaultString() function in the new module returns a string including the os-version. The existing code does not expect that to be the case, so using the previous format instead. Signed-off-by: Sebastiaan van Stijn <[email protected]>

Signed-off-by: Sebastiaan van Stijn <[email protected]>

- Remove hcsshim import from repo - un-exports GetOsVersion - Update windows matcher to not compare empty os version full diff: containerd/platforms@v0.2.0...v0.2.1 Signed-off-by: Sebastiaan van Stijn <[email protected]> (cherry picked from commit 87dd430) Signed-off-by: Sebastiaan van Stijn <[email protected]>

dmcgowan · 2024-06-13T00:45:38Z

/retest

dims · 2024-07-16T14:27:42Z

xref: #9662

k8s-ci-robot added the do-not-merge/work-in-progress label Jun 3, 2024

thaJeztah self-assigned this Jun 3, 2024

k8s-ci-robot added the size/XXL label Jun 3, 2024

thaJeztah force-pushed the 1.7_switch_to_platforms_module branch from 026143a to 76e75ee Compare June 3, 2024 13:53

thaJeztah mentioned this pull request Jun 3, 2024

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

Merged

12 tasks

dmcgowan reviewed Jun 3, 2024

View reviewed changes

thaJeztah force-pushed the 1.7_switch_to_platforms_module branch from de1d82c to 187adf6 Compare June 5, 2024 12:11

thaJeztah force-pushed the 1.7_switch_to_platforms_module branch from 187adf6 to df7298b Compare June 11, 2024 07:22

thaJeztah commented Jun 11, 2024

View reviewed changes

thaJeztah force-pushed the 1.7_switch_to_platforms_module branch from df7298b to 3732e8d Compare June 11, 2024 07:41

thaJeztah force-pushed the 1.7_switch_to_platforms_module branch from f3e8bc0 to f6a7b49 Compare June 12, 2024 00:35

thaJeztah mentioned this pull request Jun 12, 2024

[release/1.7 backport] go.mod: github.com/klauspost/compress v1.16.7 #10326

Merged

thaJeztah force-pushed the 1.7_switch_to_platforms_module branch 3 times, most recently from 0b32be5 to 1bd1ee1 Compare June 12, 2024 21:01

thaJeztah added 3 commits June 13, 2024 00:58

platforms: mark aliases as deprecated

6ccdf69

Signed-off-by: Sebastiaan van Stijn <[email protected]>

thaJeztah force-pushed the 1.7_switch_to_platforms_module branch from 1bd1ee1 to 869b786 Compare June 12, 2024 22:58

dmcgowan marked this pull request as ready for review June 12, 2024 23:56

k8s-ci-robot removed the do-not-merge/work-in-progress label Jun 12, 2024

dmcgowan approved these changes Jun 12, 2024

View reviewed changes

fuweid approved these changes Jun 13, 2024

View reviewed changes

fuweid merged commit e9d70a5 into containerd:release/1.7 Jun 13, 2024

thaJeztah deleted the 1.7_switch_to_platforms_module branch June 13, 2024 06:25

dmcgowan added the impact/changelog label Jun 26, 2024

dmcgowan changed the title ~~[release/1.7] migrate platforms package to github.com/containerd/platforms~~ [release/1.7] Migrate platforms package to github.com/containerd/platforms Jun 26, 2024

dmcgowan mentioned this pull request Jun 26, 2024

[release/1.7] Prepare release notes for v1.7.19 #10391

Merged

mdelapenya mentioned this pull request Jul 11, 2024

chore(deps): use "github.com/containerd/platforms" instead testcontainers/testcontainers-go#2638

Merged

Comments

Conversation

thaJeztah commented Jun 3, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

migrate platforms package to github.com/containerd/platforms

platforms: mark aliases as deprecated

Uh oh!

k8s-ci-robot commented Jun 3, 2024

Uh oh!

thaJeztah commented Jun 3, 2024

Uh oh!

thaJeztah commented Jun 3, 2024

Uh oh!

thaJeztah commented Jun 3, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dmcgowan Jun 3, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

thaJeztah Jun 3, 2024

Choose a reason for hiding this comment

Uh oh!

thaJeztah Jun 3, 2024

Choose a reason for hiding this comment

Uh oh!

thaJeztah Jun 3, 2024

Choose a reason for hiding this comment

Uh oh!

thaJeztah commented Jun 3, 2024

Uh oh!

dmcgowan commented Jun 7, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

thaJeztah commented Jun 7, 2024

Uh oh!

dmcgowan commented Jun 7, 2024

Uh oh!

dmcgowan commented Jun 7, 2024

Uh oh!

thaJeztah commented Jun 11, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

thaJeztah Jun 11, 2024

Choose a reason for hiding this comment

Uh oh!

thaJeztah commented Jun 12, 2024

Uh oh!

thaJeztah commented Jun 12, 2024

Uh oh!

dmcgowan commented Jun 13, 2024

Uh oh!

dims commented Jul 16, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

thaJeztah commented Jun 3, 2024 •

edited

Loading

thaJeztah commented Jun 3, 2024 •

edited

Loading

dmcgowan Jun 3, 2024 •

edited

Loading

dmcgowan commented Jun 7, 2024 •

edited

Loading

thaJeztah commented Jun 11, 2024 •

edited

Loading