chore: Support multi-arch containers build #461

kahirokunn · 2023-08-14T07:30:06Z

I have tried to start several controllers of aws-controllers-k8s on an ARM EC2 on AWS, but I get the following error.

exec ./bin/controller: exec format error

To determine the binary format in the Dockerfile, we decided to use TARGETARCH instead of go_arch to facilitate multi-platform build.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

ack-prow · 2023-08-14T07:30:17Z

Hi @kahirokunn. Thanks for your PR.

I'm waiting for a aws-controllers-k8s 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.

acornett21

If ACK is going to support multi-arch, I feel like it should support the following

PLATFORMS=linux
ARCHITECTURES=amd64 arm64 ppc64le s390x

Also, this would not be the only change to support this (possibly the release jobs/templates might need to change? I'm unsure?)

But at a mimium, if we change this, we should also claim support also update the CSV template to whatever arch's the core maintainers decide to support.

Sample of what that would look like:

labels:
   operatorframework.io/arch.s390x: supported
   operatorframework.io/os.zos: supported
   operatorframework.io/os.linux: supported
   operatorframework.io/arch.amd64: supported
   operatorframework.io/os.linux: supported
   operatorframework.io/arch.ppc64le: supported
   operatorframework.io/os.linux: supported
   operatorframework.io/arch.arm64: supported

File needing updating is here. This would be for metadate.labels field.

kahirokunn · 2023-08-24T22:43:36Z

If I do them would you please incorporate the PR?

acornett21 · 2023-08-25T14:13:43Z

@kahirokunn I'm not a maintainer of ACK, I'm just a contributor.

kahirokunn · 2023-09-08T05:26:44Z

@acornett21 I have responded to all the suggestions you have given me. Are they as intended?

kahirokunn · 2023-09-08T05:28:02Z

@a-hilaly Hi!
I am an AWS and ARM64 lover.
Could you please review this one as well?

a-hilaly

Thank you @kahirokunn !

I have few comments in line

/ok-to-test

Dockerfile

scripts/build-controller-image.sh

a-hilaly · 2023-09-08T05:31:59Z

templates/config/manifests/bases/clusterserviceversion.yaml.tpl

+    operatorframework.io/arch.s390x: supported
+    operatorframework.io/arch.amd64: supported
+    operatorframework.io/arch.ppc64le: supported
+    operatorframework.io/arch.arm64: supported
+    operatorframework.io/os.linux: supported
+    operatorframework.io/os.zos: supported


Not familiar of operatorframework, why are these needed?

@a-hilaly I am not sure either, but they asked me to put it on, so I did.
I don't think this means anything. Do you want me to remove it?
#461 (review)

I don't think it makes sense, so I will remove it.

I asked for this, since if ACK is going to distribute a multi-arch image, in order for ACK to be available on architectures other then amd64, this enables the operator to be listed on those clusters.

I see in another comment, that power/s390x aren't supported by the eks container image that ACK uses, for now we should just have

operatorframework.io/os.linux: supported operatorframework.io/arch.amd64: supported operatorframework.io/os.linux: supported operatorframework.io/arch.arm64: supported

I have tested it and it works without it, because the container image supports manifest lists!

@kahirokunn This has nothing to do with the container. This is needed for distribution of the operator bundles to OLM once released. These annotations instruct the catalog-operator in a cluster what operators can be installed for what architectures. If we leave off these annotations, then the operator will not be able to be installed on any ARM OLM distribution.

@acornett21
I think it is backward compatible since it is not annotated from the beginning.
I have never used OperatorFramework at all, so I don't know how to test it or anything.
Could you please open a PR for OperatorFramework yourself?
Thank you 🙏

@kahirokunn Please do not resolve this, this is needed like mentioned. Please include this in this PR.

@a-hilaly
I put it in because it was requested, but does it seem to be a problem?

https://access.redhat.com/documentation/ja-jp/openshift_container_platform/4.5/html/operators/olm-enabling-operator-for-multi-arch_osdk-generating-csvs

scripts/build-controller-image.sh

a-hilaly · 2023-09-08T05:35:10Z

@a-hilaly Hi!
I am an AWS and ARM64 lover.
Could you please review this one as well?

Happy to help bringing more utilities that will help you build ARM images, let's just make sure that it's done in a way that it's not breaking other system

kahirokunn · 2023-09-08T08:59:57Z

I have built it with the arm64 platform added and actually run it in an EKS environment and it works very well.

https://quay.io/repository/kahirokunn/s3-controller/manifest/sha256:7cd95f143a79be3640ee0a87f0ef9c857b27bbf279e186e68e128ce39a6159b3

kahirokunn · 2023-09-08T09:18:39Z

@acornett21 If you create a PR for eks-distro-build-tooling, we will also support the additional platform you specify at that time. Thx 🙏

└─> docker manifest inspect public.ecr.aws/eks-distro-build-tooling/eks-distro-minimal-base-nonroot:2023-08-24-1692903666.2023
{
   "schemaVersion": 2,
   "mediaType": "application/vnd.oci.image.index.v1+json",
   "manifests": [
      {
         "mediaType": "application/vnd.oci.image.manifest.v1+json",
         "size": 506,
         "digest": "sha256:4ea92dc0efd4cc177ee9e2b2a38ccf6f95deb470725db2160e6103bb6dacebf0",
         "platform": {
            "architecture": "amd64",
            "os": "linux"
         }
      },
      {
         "mediaType": "application/vnd.oci.image.manifest.v1+json",
         "size": 506,
         "digest": "sha256:ae3b415d4e334c88e12cb6f4d5416f68d04b993ec0e55ddce474a793a479be4d",
         "platform": {
            "architecture": "arm64",
            "os": "linux"
         }
      }
   ]
}

kahirokunn · 2023-09-08T09:42:01Z

@a-hilaly Do you know why the error occurs? 😢

ERROR: image: "aws-controllers-k8s:s3-v1.0.5-1-g5a93f26-dirty" not present locally
make: *** [Makefile:26: kind-test] Error 1
wrapper.sh] [TEST] Test Command exit code: 2
wrapper.sh] [CLEANUP] Cleaning up after Docker in Docker ...

a-hilaly · 2023-09-08T17:51:48Z

@a-hilaly Do you know why the error occurs? 😢
ERROR: image: "aws-controllers-k8s:s3-v1.0.5-1-g5a93f26-dirty" not present locally
make: *** [Makefile:26: kind-test] Error 1
wrapper.sh] [TEST] Test Command exit code: 2
wrapper.sh] [CLEANUP] Cleaning up after Docker in Docker ...

@kahirokunn not very sure but i suspect docker buildx is adding a suffix to the image tag, and our systems are not configured predict those prefixes.
IMO we should also want to change this script so that it's building and pushing the images right https://github.com/aws-controllers-k8s/test-infra/blob/main/cd/scripts/release-controller.sh#L139

kahirokunn · 2023-09-10T05:28:08Z

Now I know why.
It seems that docker is not able to save the image of Multi Platform.
Therefore, when you build a multi platform image, you have to push it immediately with the -push option.

docker buildx build --platform linux/arm64,linux/amd64 -t "kahirokunn/grpc-ping-go" . --load
[+] Building 0.0s (0/0) docker-container:strange_pascal
ERROR: docker exporter does not currently support exporting manifest lists

kahirokunn · 2023-09-10T05:45:51Z

@a-hilaly aws-controllers-k8s/test-infra#383
You were pushing with test-infra, I created a PR.
I think this is all that is needed to complete the MULTI ARCH support.

a-hilaly · 2023-09-14T19:46:35Z

/test all

kahirokunn · 2023-09-14T21:54:41Z

/retest

kahirokunn · 2023-09-22T00:18:57Z

@a-hilaly Hello! What do I need to do to advance this PR?

a-hilaly

Thank you @kahirokunn !! One last thing.. and we can merge it

Dockerfile

…o use TARGETARCH instead of go_arch to facilitate multi-platform build. Signed-off-by: kahirokunn <[email protected]>

Signed-off-by: kahirokunn <[email protected]>

a-hilaly · 2023-12-20T00:24:59Z

/retest

a-hilaly

/lgtm

a-hilaly · 2023-12-25T19:37:18Z

/lgtm cancel

a-hilaly · 2023-12-25T19:37:52Z

/lgtm

ack-prow · 2023-12-25T19:37:57Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: a-hilaly, kahirokunn

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:

~~OWNERS~~ [a-hilaly]

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

a-hilaly · 2023-12-25T21:32:58Z

/retest

ack-prow bot requested review from a-hilaly and vijtrip2 August 14, 2023 07:30

ack-prow bot added the needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. label Aug 14, 2023

kahirokunn force-pushed the multi-arch branch from 1b3a6c5 to 224ea2c Compare August 14, 2023 07:36

acornett21 suggested changes Aug 24, 2023

View reviewed changes

kahirokunn force-pushed the multi-arch branch from 224ea2c to 7fecb89 Compare September 8, 2023 05:25

a-hilaly reviewed Sep 8, 2023

View reviewed changes

ack-prow bot 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 Sep 8, 2023

kahirokunn force-pushed the multi-arch branch from 7fecb89 to b6164bf Compare September 8, 2023 08:55

kahirokunn force-pushed the multi-arch branch 2 times, most recently from 11041e2 to b415c8b Compare September 8, 2023 09:17

kahirokunn force-pushed the multi-arch branch from b415c8b to ef014dd Compare September 10, 2023 05:42

kahirokunn force-pushed the multi-arch branch from ef014dd to 165abfa Compare September 10, 2023 06:46

kahirokunn changed the title ~~chore: Support multi platform architecture for container image~~ chore: To determine the binary format in the Dockerfile, we decided to use TARGETARCH instead of go_arch to facilitate multi-platform Build. Sep 22, 2023

kahirokunn force-pushed the multi-arch branch 2 times, most recently from 6eecc90 to 73c8953 Compare September 22, 2023 00:27

a-hilaly reviewed Sep 26, 2023

View reviewed changes

Dockerfile Outdated Show resolved Hide resolved

kahirokunn mentioned this pull request Sep 27, 2023

chore: support Multiple architecture aws-controllers-k8s/test-infra#383

Merged

chore: To determine the binary format in the Dockerfile, we decided t…

ea924e2

…o use TARGETARCH instead of go_arch to facilitate multi-platform build. Signed-off-by: kahirokunn <[email protected]>

kahirokunn force-pushed the multi-arch branch from 73c8953 to ea924e2 Compare September 27, 2023 23:50

TARGETARCH was changed to SNAKE CASE

7822dc1

Signed-off-by: kahirokunn <[email protected]>

a-hilaly approved these changes Dec 25, 2023

View reviewed changes

ack-prow bot assigned a-hilaly Dec 25, 2023

ack-prow bot added lgtm Indicates that a PR is ready to be merged. approved labels Dec 25, 2023

ack-prow bot removed lgtm Indicates that a PR is ready to be merged. approved labels Dec 25, 2023

a-hilaly changed the title ~~chore: To determine the binary format in the Dockerfile, we decided to use TARGETARCH instead of go_arch to facilitate multi-platform build.~~ chore: Support multi-arch containers build Dec 25, 2023

ack-prow bot added the lgtm Indicates that a PR is ready to be merged. label Dec 25, 2023

ack-prow bot added the approved label Dec 25, 2023

ack-prow bot merged commit 51e8613 into aws-controllers-k8s:main Dec 25, 2023

chore: Support multi-arch containers build #461

chore: Support multi-arch containers build #461

Uh oh!

Conversation

kahirokunn commented Aug 14, 2023 • edited by a-hilaly Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ack-prow bot commented Aug 14, 2023

Uh oh!

acornett21 left a comment

Choose a reason for hiding this comment

Uh oh!

kahirokunn commented Aug 24, 2023

Uh oh!

acornett21 commented Aug 25, 2023

Uh oh!

kahirokunn commented Sep 8, 2023

Uh oh!

kahirokunn commented Sep 8, 2023

Uh oh!

a-hilaly left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

acornett21 Sep 8, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

a-hilaly commented Sep 8, 2023

Uh oh!

kahirokunn commented Sep 8, 2023

Uh oh!

kahirokunn commented Sep 8, 2023

Uh oh!

kahirokunn commented Sep 8, 2023

Uh oh!

a-hilaly commented Sep 8, 2023

Uh oh!

kahirokunn commented Sep 10, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

kahirokunn commented Sep 10, 2023

Uh oh!

a-hilaly commented Sep 14, 2023

Uh oh!

kahirokunn commented Sep 14, 2023

Uh oh!

kahirokunn commented Sep 22, 2023

Uh oh!

a-hilaly left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

a-hilaly commented Dec 20, 2023

Uh oh!

a-hilaly left a comment

Choose a reason for hiding this comment

Uh oh!

a-hilaly commented Dec 25, 2023

Uh oh!

kahirokunn commented Aug 14, 2023 •

edited by a-hilaly

Loading

acornett21 Sep 8, 2023 •

edited

Loading

kahirokunn commented Sep 10, 2023 •

edited

Loading