Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Add Docker image support multi OS archs #1462

Merged
merged 7 commits into from
Nov 9, 2022

Conversation

wilsonwu
Copy link
Contributor

@wilsonwu wilsonwu commented Oct 18, 2022

What type of PR is this?
Update Dockerfile to support different OS archs support, and update related script.
/kind feature

What this PR does / why we need it:
More requirements for ARM support.

@k8s-ci-robot k8s-ci-robot added kind/documentation Categorizes issue or PR as related to documentation. kind/feature Categorizes issue or PR as related to a new feature. labels Oct 18, 2022
@linux-foundation-easycla
Copy link

linux-foundation-easycla bot commented Oct 18, 2022

CLA Signed

The committers listed above are authorized under a signed CLA.

  • ✅ login: wilsonwu / name: Wilson Wu (3d1809c)

@k8s-ci-robot k8s-ci-robot added the cncf-cla: no Indicates the PR's author has not signed the CNCF CLA. label Oct 18, 2022
@k8s-ci-robot
Copy link
Contributor

Welcome @wilsonwu!

It looks like this is your first PR to kubernetes-sigs/gateway-api 🎉. Please refer to our pull request process documentation to help your PR have a smooth ride to approval.

You will be prompted by a bot to use commands during the review process. Do not be afraid to follow the prompts! It is okay to experiment. Here is the bot commands documentation.

You can also check if kubernetes-sigs/gateway-api has its own contribution guidelines.

You may want to refer to our testing guide if you run into trouble with your tests not passing.

If you are having difficulty getting your pull request seen, please follow the recommended escalation practices. Also, for tips and tricks in the contribution process you may want to read the Kubernetes contributor cheat sheet. We want to make sure your contribution gets all the attention it needs!

Thank you, and welcome to Kubernetes. 😃

@k8s-ci-robot k8s-ci-robot added the needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. label Oct 18, 2022
@k8s-ci-robot
Copy link
Contributor

Hi @wilsonwu. Thanks for your PR.

I'm waiting for a kubernetes-sigs member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@k8s-ci-robot k8s-ci-robot added the size/S Denotes a PR that changes 10-29 lines, ignoring generated files. label Oct 18, 2022
@wilsonwu wilsonwu changed the title Add Docker image support muliti OS archs with document update Add Docker image support multi OS archs with document update Oct 18, 2022
@k8s-ci-robot k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. and removed cncf-cla: no Indicates the PR's author has not signed the CNCF CLA. labels Oct 18, 2022
Copy link
Member

@shaneutt shaneutt left a comment

Choose a reason for hiding this comment

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

/ok-to-test

What version of docker did you use to test this?

@k8s-ci-robot k8s-ci-robot added ok-to-test Indicates a non-member PR verified by an org member that is safe to test. and removed needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Oct 18, 2022
@wilsonwu
Copy link
Contributor Author

/ok-to-test

What version of docker did you use to test this?

Tested in 20.10.11

@wilsonwu wilsonwu requested review from shaneutt and removed request for bowei and youngnick October 30, 2022 14:38
@shaneutt
Copy link
Member

shaneutt commented Nov 1, 2022

I'm a little confused how this compares/contrasts with #1402. Have you see this one? It would seem that with #1402 we would simply have a multi-arch image rather than having the ability to build a single image for separate architectures. What are your thoughts?

@wilsonwu
Copy link
Contributor Author

wilsonwu commented Nov 2, 2022

I'm a little confused how this compares/contrasts with #1402. Have you see this one? It would seem that with #1402 we would simply have a multi-arch image rather than having the ability to build a single image for separate architectures. What are your thoughts?

#1402 is same as this PR, I think the #1402 is OK for support multi OS Archs, let close this after the #1402 done.

And use buildx to build images by manifest is better way.

@robscott
Copy link
Member

robscott commented Nov 3, 2022

Hey @wilsonwu, since #1402 has gone stale, I think we can use yours. Do you mind updating this PR to include hack/build-and-push.sh updates as well?

README.md Outdated Show resolved Hide resolved
@wilsonwu
Copy link
Contributor Author

wilsonwu commented Nov 4, 2022

Hey @wilsonwu, since #1402 has gone stale, I think we can use yours. Do you mind updating this PR to include hack/build-and-push.sh updates as well?

No problem, I can update the script for docker image build ASAP.

@wilsonwu wilsonwu requested review from robscott and removed request for shaneutt November 8, 2022 09:32
@wilsonwu wilsonwu changed the title Add Docker image support multi OS archs with document update Add Docker image support multi OS archs Nov 8, 2022
hack/build-and-push.sh Outdated Show resolved Hide resolved
@wilsonwu wilsonwu requested review from howardjohn and removed request for robscott November 9, 2022 04:33
hack/build-and-push.sh Outdated Show resolved Hide resolved
Copy link
Member

@shaneutt shaneutt 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!

@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: howardjohn, keithmattix, shaneutt, wilsonwu

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@k8s-ci-robot k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Nov 9, 2022
@shaneutt shaneutt added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Nov 9, 2022
@k8s-ci-robot k8s-ci-robot merged commit 9568c67 into kubernetes-sigs:main Nov 9, 2022
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. kind/documentation Categorizes issue or PR as related to documentation. kind/feature Categorizes issue or PR as related to a new feature. lgtm "Looks good to me", indicates that a PR is ready to be merged. ok-to-test Indicates a non-member PR verified by an org member that is safe to test. size/S Denotes a PR that changes 10-29 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants