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

Support pack build --platform #2162

Merged
merged 9 commits into from
May 23, 2024
Merged

Support pack build --platform #2162

merged 9 commits into from
May 23, 2024

Conversation

natalieparellano
Copy link
Member

Fixes #2154

Has a merge conflict with #2086 - let's get that one in first and then update this one

@natalieparellano natalieparellano requested review from a team as code owners May 20, 2024 19:16
@github-actions github-actions bot added this to the 0.34.0 milestone May 20, 2024
@github-actions github-actions bot added type/enhancement Issue that requests a new feature or improvement. type/chore Issue that requests non-user facing changes. labels May 20, 2024
Fixes #2154

Signed-off-by: Natalie Arellano <narellano@vmware.com>
pkg/client/build.go Outdated Show resolved Hide resolved
// Sample error from docker engine:
// `image with reference <image> was found but does not match the specified platform: wanted linux/amd64, actual: linux`
if strings.Contains(err.Error(), "does not match the specified platform") &&
strings.HasSuffix(strings.TrimSpace(err.Error()), "actual: linux") {
Copy link
Member Author

Choose a reason for hiding this comment

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

I added this to make the acceptance test work, but I think it's actually closer to what we want anyway

Signed-off-by: Natalie Arellano <narellano@vmware.com>
pkg/client/build.go Outdated Show resolved Hide resolved
- WCOW units by checking for 'windows' in error string
- LCOW acceptance by printing a log line and checking it instead of expecting an error

Signed-off-by: Natalie Arellano <narellano@vmware.com>
Copy link

codecov bot commented May 20, 2024

Codecov Report

Attention: Patch coverage is 76.00000% with 18 lines in your changes are missing coverage. Please review.

Project coverage is 70.18%. Comparing base (7306444) to head (89cb617).

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #2162      +/-   ##
==========================================
+ Coverage   70.15%   70.18%   +0.03%     
==========================================
  Files         253      253              
  Lines       18412    18449      +37     
==========================================
+ Hits        12916    12947      +31     
- Misses       4650     4656       +6     
  Partials      846      846              
Flag Coverage Δ
os_linux 69.32% <74.67%> (+0.03%) ⬆️
os_macos 65.63% <60.00%> (+0.01%) ⬆️
os_windows 69.70% <76.00%> (+0.03%) ⬆️
unit 70.18% <76.00%> (+0.03%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Signed-off-by: Natalie Arellano <narellano@vmware.com>
Signed-off-by: Natalie Arellano <narellano@vmware.com>
LCOW appears to fallback to the arch it can find

Signed-off-by: Natalie Arellano <narellano@vmware.com>
Signed-off-by: Natalie Arellano <narellano@vmware.com>
pkg/client/build.go Outdated Show resolved Hide resolved
pkg/client/build.go Outdated Show resolved Hide resolved
@jjbustamante
Copy link
Member

This looks good to me, the only thing is adding back ImageOS: targetToUse.OS for testing because I didn't get any failure locally

Signed-off-by: Natalie Arellano <narellano@vmware.com>
Signed-off-by: Natalie Arellano <narellano@vmware.com>
@jjbustamante jjbustamante merged commit ebea3ae into main May 23, 2024
18 checks passed
@jjbustamante jjbustamante deleted the build-with-platform branch May 23, 2024 14:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type/chore Issue that requests non-user facing changes. type/enhancement Issue that requests a new feature or improvement.
Projects
None yet
2 participants