Update worker.Platforms() in builder-next worker. #50038
Conversation
|
Thanks @thaJeztah for the review and comments, I've responded to all and updated the code. PTAL when you get a chance. |
|
Note: CI failures are unrelated to this PR. |
There was a problem hiding this comment.
LGTM
left two suggestions; no blockers, but let me know what you think.
Also /cc @tonistiigi to review if this is what you had in mind
7496d47 to
c075f20
Compare
|
|
||
| // platformsFn looks up supported platforms on the host, | ||
| // and is overridden in unit-tests. | ||
| platformsFn func(noCache bool) []ocispec.Platform |
There was a problem hiding this comment.
Don't add this to the worker. If you need a function for the unit tests then create a separate function with configurable parameters for it.
There was a problem hiding this comment.
Not sure I follow the suggestion; I need a way to mock archutils.SupportedPlatforms() when called within Worker.Platforms(). Not sure what alternative you are suggesting to the approach I took.
There was a problem hiding this comment.
Worker, or Worker.Platforms() doesn't support providing custom values for set of defined and supported platforms. If you want to test combining these arrays with these conditions then you would need to define a new function like allPlatforms(defined, supported []ocispec.Platforms) []ocispec.Platforms, write unit test for that and make worker.Platforms() call it internally.
There was a problem hiding this comment.
OK done, thanks for the suggestion (see mergePlatforms()).
|
Note: CI passes (test failure is unrelated to this change). |
Use platform MatchComparer when checking for matching platforms. Also, add unit test to ensure the merging of defined and host-supported platforms works correctly. Signed-off-by: Cesar Talledo <cesar.talledo@docker.com>
- What I did
Updated worker.Platforms() in builder-next worker to use platform
MatchComparerwhen checking for matching platforms (same as we do in buildkit, see here.Added a unit test.
Fixes #50037.
- How to verify it
- A picture of a cute animal (not mandatory but encouraged)