Skip to content

Add fallback for windows platforms without osversion#8101

Merged
kevpar merged 1 commit intocontainerd:mainfrom
cpuguy83:loosen_windows_platform_match
Feb 14, 2023
Merged

Add fallback for windows platforms without osversion#8101
kevpar merged 1 commit intocontainerd:mainfrom
cpuguy83:loosen_windows_platform_match

Conversation

@cpuguy83
Copy link
Member

@cpuguy83 cpuguy83 commented Feb 13, 2023

The background for this change:

  1. Windows host-process containers do not have an OS version set
  2. Buildx v0.10 started pushing manifest lists by default, but it never has the OSVersion in the platform data (not that there is any way to specify what particular OS version you want). The change means that containerd cannot run images created with the new buildx on Windows because there is no matching OSVersion in the list of manifests.

Fixes #8070
Fixes #7431

@cpuguy83
Copy link
Member Author

@marosset

@cpuguy83 cpuguy83 requested a review from tianon February 13, 2023 22:19
samuelkarp
samuelkarp previously approved these changes Feb 13, 2023
@samuelkarp samuelkarp dismissed their stale review February 13, 2023 22:50

Unit tests need to be updated

@marosset
Copy link
Contributor

/lgtm (minus the outdated unit tests)
@jsturtevant too

The background for this change:

1. Windows host-process containers do not have an OS version set
2. Buildx v0.10 started pushing manifest lists by default, but it never
   has the OSVersion in the platform data (not that there is any way to
   specify what particular OS version you want). The change means that
   containerd cannot run images created with the new buildx on Windows
   because there is no matching OSVersion in the list of manifests.

Signed-off-by: Brian Goff <cpuguy83@gmail.com>
@cpuguy83 cpuguy83 force-pushed the loosen_windows_platform_match branch from 1689eb7 to 8442521 Compare February 13, 2023 23:07
@cpuguy83
Copy link
Member Author

Fixed the tests.

Copy link
Member

@tianon tianon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I do not like it, but it's a fair interpretation and the logic seems correct 😅

@marosset
Copy link
Contributor

I do not like it, but it's a fair interpretation and the logic seems correct 😅

I agree. I don't really like it but it should fix the issue.

@dcantah
Copy link
Member

dcantah commented Feb 14, 2023

cc @kevpar @kiashok

Copy link
Member

@kevpar kevpar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@kevpar kevpar merged commit 9862f79 into containerd:main Feb 14, 2023
@cpuguy83 cpuguy83 deleted the loosen_windows_platform_match branch February 14, 2023 20:18
@thaJeztah
Copy link
Member

backport for 1.6; #8106

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cherry-picked/1.6.x PR commits are cherry-picked into release/1.6 branch

Projects

Archived in project

9 participants