Skip to content

Conversation

@TBBle
Copy link
Collaborator

@TBBle TBBle commented Oct 27, 2023

This fixes a bunch of places where these OCI fields were being stripped or lost during processing.

It hasn't been visible until now as they are nil on all Linux platforms as far as I know, but WCOW needs them to select the appropriate image for FROM from a manifest list, and also probably in other places that I haven't tested.

Draft because of one TODO to be completed/improved, and because I suspect this will warrant discussion. I'd also like to see if CI passes, in case I'm wrong about this not affecting Linux at all, e.g., unexpected digest shifts.

@profnandaa
Copy link
Collaborator

Thanks for the fix!

@TBBle TBBle force-pushed the include-OS-version-and-features-in-platforms branch 2 times, most recently from 20f2fe1 to ca6d8c5 Compare November 2, 2023 14:57
@TBBle TBBle marked this pull request as ready for review November 2, 2023 14:57
Trivially created by looking for every reference to .Variant and adding
OSVersion and OSFeatures, except the ones related to the string
representation of a Platform instance.

I then went through and ensured every assignment of OSFeatures that
might leak out, i.e., not local-only or for marhsalling purposes, uses
the append-to-nil idiom to avoid sharing the slice storage and allowing
accidental mutation after-the-fact.

Signed-off-by: Paul "TBBle" Hampson <[email protected]>
@TBBle TBBle force-pushed the include-OS-version-and-features-in-platforms branch from ca6d8c5 to 98e0d8d Compare November 3, 2023 03:20
@TBBle
Copy link
Collaborator Author

TBBle commented Nov 3, 2023

Single test failure in TestRuncWorkerExec, running ps -o pid,comm in a container. I can't see how that could be related to my changes, particularly since I don't think the runc worker ever cares about the Platform object. Didn't occur on rerun.

@tonistiigi tonistiigi merged commit 6a0cd7c into moby:master Nov 4, 2023
@TBBle TBBle deleted the include-OS-version-and-features-in-platforms branch November 4, 2023 02:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants