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

chore: Support build on s390x and ppc64le (follow #6441) #8890

Merged
merged 17 commits into from
Apr 22, 2022
Merged

chore: Support build on s390x and ppc64le (follow #6441) #8890

merged 17 commits into from
Apr 22, 2022

Conversation

samding01
Copy link
Contributor

This PR is following #6441 for supporting s390x and ppc64le. Verified on amd64/s390x/ppc64le platforms.

  • Changed esbuild-loader to version 2.18.0 as well as kustomize to 4.4.1 to support s390x;
  • Added g++ in the container to make node-sass working for both ppc64le and s390x;

Note on DCO:

If the DCO action in the integration test fails, one or more of your commits are not signed off. Please click on the Details link next to the DCO action for instructions on how to resolve this.

Checklist:

  • Either (a) I've created an enhancement proposal and discussed it with the community, (b) this is a bug fix, or (c) this does not need to be in the release notes.
  • The title of the PR states what changed and the related issues number (used for the release note).
  • I've included "Closes [ISSUE #]" or "Fixes [ISSUE #]" in the description to automatically close the associated issue.
  • I've updated both the CLI and UI to expose my feature, or I plan to submit a second PR with them.
  • Does this PR require documentation updates?
  • I've updated documentation as required by this PR.
  • Optional. My organization is added to USERS.md.
  • I have signed off all my commits as required by DCO
  • I have written unit and/or e2e tests for my change. PRs without these are unlikely to be merged.
  • My build is green (troubleshooting builds).

@codecov
Copy link

codecov bot commented Mar 28, 2022

Codecov Report

Merging #8890 (a91e000) into master (e5e3b0f) will decrease coverage by 0.01%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master    #8890      +/-   ##
==========================================
- Coverage   45.53%   45.51%   -0.02%     
==========================================
  Files         219      219              
  Lines       25897    25897              
==========================================
- Hits        11792    11787       -5     
- Misses      12463    12467       +4     
- Partials     1642     1643       +1     
Impacted Files Coverage Δ
applicationset/services/scm_provider/github.go 75.29% <0.00%> (-5.89%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e5e3b0f...a91e000. Read the comment docs.

Copy link
Collaborator

@alexmt alexmt left a comment

Choose a reason for hiding this comment

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

Thank you @samding01 ! Please add new platforms in CI so we can see how much longer image build it going to take:

IMAGE_PLATFORMS=linux/amd64,linux/arm64

@alexmt
Copy link
Collaborator

alexmt commented Mar 28, 2022

Codegen changes are not related to PR changes. Please pull latest master into your branch to fix the build failure.

@samding01
Copy link
Contributor Author

samding01 commented Mar 28, 2022

Thank you @samding01 ! Please add new platforms in CI so we can see how much longer image build it going to take:

IMAGE_PLATFORMS=linux/amd64,linux/arm64

@alexmt
Is it good to add like follows?

IMAGE_PLATFORMS=linux/amd64,linux/arm64,linux/s390x,linux/ppc64le

Signed-off-by: samding <samding@ca.ibm.com>
Signed-off-by: samding <samding@ca.ibm.com>
Signed-off-by: samding <samding@ca.ibm.com>
@samding01 samding01 requested a review from alexmt March 29, 2022 20:15
@krishvoor
Copy link

Hi @jopit ^^
As discussed, can you take a peek plz?

@samding01 samding01 changed the title Support build on s390x and ppc64le (follow #6441) chore: Support build on s390x and ppc64le (follow #6441) Apr 11, 2022
Signed-off-by: Sam Ding <samding@ca.ibm.com>
Signed-off-by: Sam Ding <samding@ca.ibm.com>
Signed-off-by: Sam Ding <samding@ca.ibm.com>

Solve the merge conflict

Signed-off-by: Sam Ding <samding@ca.ibm.com>
Signed-off-by: Sam Ding <samding@ca.ibm.com>
@samding01
Copy link
Contributor Author

The failure of Image / publish (pull_request) Failing after 16m testing is related to awscli-linux,
amd64 downloads it from https://awscli.amazonaws.com/awscli-exe-linux-x86_64-v2.4.6.zip.
But seems no s390x or ppc64le versions. Here is the github: https://github.com/aws/aws-cli/tree/2.4.6
How to solve this issue?

@levivic
Copy link

levivic commented Apr 12, 2022

hi @alexmt, seems the aws-cli, the missing dependency on s390x and ppc64le, is still a blocker for us. Without it, the ci workflow check will always fail while s390x/ppc64le platforms specified. I know there is the push of #8032, besides that, how can we progress this further at this moment?

i am seeing similar requests for Power, blocked for the same reason: at #6441 and #8155
and a z request redhat-developer/gitops-operator#278

@levivic
Copy link

levivic commented Apr 12, 2022

Thank you @samding01 ! Please add new platforms in CI so we can see how much longer image build it going to take:

IMAGE_PLATFORMS=linux/amd64,linux/arm64

@alexmt Not sure if ppc64le and s390x are supported in CI, if not, maybe we could remove them in image.yaml?

@alexmt
Copy link
Collaborator

alexmt commented Apr 15, 2022

@samding01 Sorry for blocking PR for so long. The #8032 has been merged and AWS CLI is no longer bundled into image. CI should pass after merging master changes

@samding01
Copy link
Contributor Author

samding01 commented Apr 18, 2022

@alexmt After updating (pull upstream) the source repo, the CI workflow tests show Image / publish (pull_request) is passed, but
Integration tests / Check changes to generated code (pull_request) is still failed, the error msg is limited and does not show the reason of the error, can you let's know what is this and how to change to avoid it? Thx.

@crenshaw-dev
Copy link
Collaborator

@samding01 that error can generally be resolved by running make codegen.

@samding01
Copy link
Contributor Author

@crenshaw-dev Thank you for your information, Integration tests / Check changes to generated code (pull_request) is a CI workflow test, how do I run make codegen? Can you provide more info? Thanks

@34fathombelow
Copy link
Member

@samding01 you do this on your local copy of your branch. More info can be found @ https://argo-cd.readthedocs.io/en/stable/developer-guide/ci/#why-does-the-codegen-step-fail Make sure you have followed the toolchain setup guide.

@alexmt
Copy link
Collaborator

alexmt commented Apr 22, 2022

@samding01, I've checked the CI logs. Looks like only generated manifests are different. This is happened because of kustomize upgrade. You can run make manifests - it requires only docker. make codegen should work too but it will take much more time.

Signed-off-by: Sam Ding <samding@ca.ibm.com>
Copy link
Collaborator

@alexmt alexmt left a comment

Choose a reason for hiding this comment

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

LGTM! Thank you!

@alexmt alexmt merged commit b760a27 into argoproj:master Apr 22, 2022
@samding01
Copy link
Contributor Author

Thanks @alexmt @34fathombelow

wojtekidd pushed a commit to wojtekidd/argo-cd that referenced this pull request Apr 25, 2022
…oproj#8890)

* Support build on s390x and ppc64le

Signed-off-by: Sam Ding <samding@ca.ibm.com>
Signed-off-by: wojtekidd <wojtek.cichon@protonmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants