fix: ensure image can be built on m1 macos #1602

JameelB · 2023-02-28T11:31:25Z

Description

When running any of the image build make targets on an M1 MacOS, it fails with the following error:

 => [builder 7/9] COPY . ./                                                                                                                                                                                                                                                         0.2s 
 => ERROR [builder 8/9] RUN go mod vendor                                                                                                                                                                                                                                           0.2s 
------                                                                                                                                                                                                                                                                                   
 > [builder 8/9] RUN go mod vendor:
#12 0.148 qemu-x86_64: Could not open '/lib64/ld-linux-x86-64.so.2': No such file or directory
------
executor failed running [/bin/sh -c go mod vendor]: exit code: 255

More details on this error can be found in this thread.

Note that this is a temporary workaround. An issue has been created to look into how to build KFM in multiple platforms natively, see MGDSTRM-10800.

Verification Steps

Run make image/build or make image/build/internal and ensure it builds successfully
- Verify this on MacOS
- Verity this on Linux

Checklist (Definition of Done)

pb82

Verified on linux, make image/build succeeds

pawelpaszki

Verified on MAC:

make image/build

docker --config=""/Users/pawelpaszki/kafka/managed-services-api/.docker"" build --pull -t "default-route-openshift-image-registry.apps-crc.testing/kas-fleet-manager-pawelpaszki/kas-fleet-manager:1677585248" .
[+] Building 92.1s (15/15) FINISHED                                                                                                                                                                                                            
 => [internal] load build definition from Dockerfile                                                                                                                                                                                      0.0s
 => => transferring dockerfile: 816B                                                                                                                                                                                                      0.0s
 => [internal] load .dockerignore                                                                                                                                                                                                         0.0s
 => => transferring context: 738B                                                                                                                                                                                                         0.0s
 => [internal] load metadata for registry.access.redhat.com/ubi9-minimal:9.1.0                                                                                                                                                            2.7s
 => [builder 1/9] FROM registry.access.redhat.com/ubi9-minimal:9.1.0@sha256:61925d31338b7b41bfd5b6b8cf45eaf80753d415b0269fc03613c5c5049b879e                                                                                              6.2s
 => => resolve registry.access.redhat.com/ubi9-minimal:9.1.0@sha256:61925d31338b7b41bfd5b6b8cf45eaf80753d415b0269fc03613c5c5049b879e                                                                                                      0.0s
 => => sha256:43246102cf37b0825786ca78e9a6e153c26d1a4241ddf53092e5b0ac18fe77bf 429B / 429B                                                                                                                                                0.0s
 => => sha256:3135bd90aad672f44305a68c1d7e1ab1b894ec6e0c6964d5ab40295af1920437 6.26kB / 6.26kB                                                                                                                                            0.0s
 => => sha256:ba958a445f00d91e4019b520c467e36e7f5bc07da02f2a87e9ccefbe4a70ff6d 37.85MB / 37.85MB                                                                                                                                          4.5s
 => => sha256:61925d31338b7b41bfd5b6b8cf45eaf80753d415b0269fc03613c5c5049b879e 1.47kB / 1.47kB                                                                                                                                            0.0s
 => => extracting sha256:ba958a445f00d91e4019b520c467e36e7f5bc07da02f2a87e9ccefbe4a70ff6d                                                                                                                                                 1.5s
 => [internal] load build context                                                                                                                                                                                                         3.7s
 => => transferring context: 138.86MB                                                                                                                                                                                                     3.7s
 => [builder 2/9] RUN microdnf install -y tar gzip make which git                                                                                                                                                                        10.7s
 => [builder 3/9] RUN curl -O -J https://dl.google.com/go/go1.19.6.linux-amd64.tar.gz                                                                                                                                                     5.3s
 => [builder 4/9] RUN tar -C /usr/local -xzf go1.19.6.linux-amd64.tar.gz                                                                                                                                                                  4.6s 
 => [builder 5/9] RUN ln -s /usr/local/go/bin/go /usr/local/bin/go                                                                                                                                                                        0.2s 
 => [builder 6/9] WORKDIR /workspace                                                                                                                                                                                                      0.0s 
 => [builder 7/9] COPY . ./                                                                                                                                                                                                               1.0s 
 => [builder 8/9] RUN go mod vendor                                                                                                                                                                                                      18.5s 
 => [builder 9/9] RUN make binary                                                                                                                                                                                                        41.7s
 => [stage-1 2/2] COPY --from=builder /workspace/kas-fleet-manager /usr/local/bin/                                                                                                                                                        0.2s 
 => exporting to image                                                                                                                                                                                                                    0.4s 
 => => exporting layers                                                                                                                                                                                                                   0.4s 
 => => writing image sha256:9c55c2841661c3960dcb7aa8a9ac012600b8114d06001678d77787de16b7d08a                                                                                                                                              0.0s 
 => => naming to default-route-openshift-image-registry.apps-crc.testing/kas-fleet-manager-pawelpaszki/kas-fleet-manager:1677585248                                                                                                       0.0s 

Use 'docker scan' to run Snyk tests against images to find vulnerabilities and learn how to fix them

miguelsorianod · 2023-02-28T12:00:47Z

Maybe a cleaner way would be setting the envvar if the user running the Makefile is on a mac?

That way we don't need to hardcode the platform in the Dockerfile

miguelsorianod · 2023-02-28T12:03:47Z

This could be achieved by doing something like (pseudocode, needs to be reviewed):

OS=$(uname)
if [ "$OS" = 'Darwin' ]; then
  export DOCKER_DEFAULT_PLATFORM ?= linux/amd64
fi

It should also be verified that when running in a non-mac OS that variable is not set on the docker build

miguelsorianod · 2023-02-28T12:15:05Z

This could be achieved by doing something like (pseudocode, needs to be reviewed):
OS=$(uname)
if [ "$OS" = 'Darwin' ]; then
  export DOCKER_DEFAULT_PLATFORM ?= linux/amd64
fi
It should also be verified that when running in a non-mac OS that variable is not set on the docker build

As a side note, I wonder if the same issue happens if podman is used, and also whether podman makes use of that envvar too

JameelB · 2023-02-28T21:55:24Z

This could be achieved by doing something like (pseudocode, needs to be reviewed):
OS=$(uname)
if [ "$OS" = 'Darwin' ]; then
  export DOCKER_DEFAULT_PLATFORM ?= linux/amd64
fi
It should also be verified that when running in a non-mac OS that variable is not set on the docker build
As a side note, I wonder if the same issue happens if podman is used, and also whether podman makes use of that envvar too

I tested the suggestion with podman and docker, it works with docker but unfortunately it doesn't work with podman, seems like they don't use that env var 😞.

However, the build cmd does have a --platform parameter for both which we could specify if the os is darwin as an alternative.

ifeq ($(shell uname -s | tr A-Z a-z), darwin)
	DOCKER_BUILD_PLATFORM ?= --platform linux/amd64
endif

...
# example build cmd
image/build:
        $(DOCKER) build $(DOCKER_BUILD_PLATFORM) ...

miguelsorianod · 2023-03-01T08:52:11Z

This could be achieved by doing something like (pseudocode, needs to be reviewed):
OS=$(uname)
if [ "$OS" = 'Darwin' ]; then
  export DOCKER_DEFAULT_PLATFORM ?= linux/amd64
fi
It should also be verified that when running in a non-mac OS that variable is not set on the docker build
As a side note, I wonder if the same issue happens if podman is used, and also whether podman makes use of that envvar too
I tested the suggestion with podman and docker, it works with docker but unfortunately it doesn't work with podman, seems like they don't use that env var disappointed.

However, the build cmd does have a --platform parameter for both which we could specify if the os is darwin as an alternative.
ifeq ($(shell uname -s | tr A-Z a-z), darwin)
	DOCKER_BUILD_PLATFORM ?= --platform linux/amd64
endif

...
# example build cmd
image/build:
        $(DOCKER) build $(DOCKER_BUILD_PLATFORM) ...

Let's do that. But let's call it CONTAINER_IMAGE_BUILD_PLATFORM instead, to make the name agnostic of the container engine being used

Makefile

miguelsorianod · 2023-03-01T17:57:54Z

Makefile

 ### Environment-sourced variables with defaults
 # Can be overriden by setting environment var before running
 # Example:
 #   OCM_ENV=testing make run
 #   export OCM_ENV=testing; make run
+
+ifeq ($(shell uname -s | tr A-Z a-z), darwin)
+    CONTAINER_IMAGE_BUILD_PLATFORM ?= --platform linux/amd64


I see some issues on the CI side.
Maybe this default value has to be quoted?

It contains dashes which mght be being interpreted as some shell flag

i tried removing this assignment and i'm also getting the same issue.

In main, as soon as I removed these lines, I get the same issue as what's shown in the CI

kas-fleet-manager/Makefile

Lines 165 to 169 in 738b7b8

ifeq ($(shell uname -s | tr A-Z a-z), darwin)

PGHOST:="127.0.0.1"

else

PGHOST:="172.18.0.22"

endif

These vars never get assigned which causes the error

kas-fleet-manager/Makefile

Lines 177 to 198 in 738b7b8

ifndef OCM_ENV

OCM_ENV:=integration

endif

ifndef TEST_SUMMARY_FORMAT

TEST_SUMMARY_FORMAT=short-verbose

endif

export GOPROXY=https://proxy.golang.org

export GOPRIVATE=gitlab.cee.redhat.com

ifndef SERVER_URL

SERVER_URL:=http://localhost:8000

endif

ifndef TEST_TIMEOUT

ifeq ($(OCM_ENV), integration)

TEST_TIMEOUT=15m

else

TEST_TIMEOUT=5h

endif

endif

Going to look into this a bit further 👀

@JameelB I'd suggest we defer removing this onto another PR.

I see the spaces inside the PGHOST ifelse are whitespaces and not tabs. Might that be causing the issue?

When testing this, it seems like Makefile doesn't like var definitions between target definitions.

I've moved all of the var definitions before the target definitions starts to ensure they're set correctly.

I think I can identify where is the issue:

Inside ifndef ... endif blocks it seems that the characters used to indent matters.
For example, for the TEST_VAR_4 ifndef block:
if inside the body the TEST_VAR_4 initialization is not indented then it seems to work
if inside the body the TEST_VAR_4 initialization is indented with spaces it seems to work
if inside the body the TEST_VAR_4 initialization is indented with tabs it does not seem to work

I think the issue is that by indenting with tab the Makefile interprets the indented contend with tab as part of a Makefile target's body, but in this case we are not working in a target body so that's the issue.

This means that the order where var definitions appear shouldn't really matter (as long as they are defined/set before being used). The issue is another and has to do with indentation inside Makefile conditional directives

good spot, that looks like it.

Vars like OCM_ENV which is used across multiple make targets should probably stay at the top though.

We can re-arrange some of the target specific ones like TEST_SUMMARY_FORMAT to be grouped with the test targets if preferred. Some of the vars here are only ever used for one target as well (i.e. SERVER_URL), in that case it should probably be defined as a target variable. 🤔

There's probably more vars in there that needs to be reviewed to be removed or moved 🤔 I can create a follow on PR for this?

rearranging them is good for organization purposes so we can do that.
But in any case we have to fix the issues of tabs being used in conditionals that are not shell conditionals but makefile conditionals.
For this PR let's do the order reorganization and fixing the indentation for the makefile conditionals.
For another PR(s) we can review the removal or re-scoping of some of the variables.
I also suspect that some ifndefs can be just rewritten as MYVAR ?= defaultavalue or MYVAR := value

i've updated this to remove the indentation from the make if statements.

I've also moved the ones in this block to group them with their related make targets but I haven't looked at any of the other var definitions outside of this.

codecov · 2023-03-02T16:27:44Z

Codecov Report

Merging #1602 (3306946) into main (1122f01) will decrease coverage by 0.14%.
The diff coverage is n/a.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1602      +/-   ##
==========================================
- Coverage   81.81%   81.67%   -0.14%     
==========================================
  Files         158      158              
  Lines       14791    14839      +48     
==========================================
+ Hits        12101    12120      +19     
- Misses       2266     2288      +22     
- Partials      424      431       +7

Flag	Coverage Δ
unittests	`81.67% <ø> (-0.14%)`	⬇️

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

Impacted Files	Coverage Δ
...al/kafka/internal/workers/kafka_mgrs/kafkas_mgr.go	`83.73% <0.00%> (-6.61%)`	⬇️
internal/kafka/internal/services/clusters.go	`74.50% <0.00%> (-0.25%)`	⬇️
...kafka/internal/services/quota/ams_quota_service.go	`71.69% <0.00%> (ø)`

miguelsorianod · 2023-03-03T09:57:58Z

Makefile

+export GOPRIVATE=gitlab.cee.redhat.com
+
+ifeq ($(shell uname -s | tr A-Z a-z), darwin)
+	CONTAINER_IMAGE_BUILD_PLATFORM ?= --platform linux/amd64


Could we double quote the value?

updated, note that I had to use echo for this in the image build commands to get rid of the quotes as docker build doesn't accept "--platform linux/amd64"

Ok then let's unquote it. It seems that Makefile doesn't really interpret single/double quotes and it considers them literally

quotes removed 👍

When running any of the image build make targets on an m1 macos, it fails due to the following error: qemu-x87_64: Could not open '/lib64/ld-linux-x86-64.so.2 Specify platform to ensure build uses the correct architecture.

…ions with their related make targets

JameelB requested a review from a team as a code owner February 28, 2023 11:31

github-actions bot added kafka service-template Label categorizing all the service templates changes labels Feb 28, 2023

pb82 approved these changes Feb 28, 2023

View reviewed changes

pawelpaszki approved these changes Feb 28, 2023

View reviewed changes

JameelB force-pushed the fix/docker-image-build-on-macos branch from 2a5f67e to 04e62b3 Compare March 1, 2023 13:10

github-actions bot removed service-template Label categorizing all the service templates changes kafka labels Mar 1, 2023

machi1990 reviewed Mar 1, 2023

View reviewed changes

Makefile Outdated Show resolved Hide resolved

JameelB force-pushed the fix/docker-image-build-on-macos branch from 04e62b3 to 8062ff7 Compare March 1, 2023 13:15

JameelB commented Mar 1, 2023

View reviewed changes

Makefile Outdated Show resolved Hide resolved

miguelsorianod self-requested a review March 1, 2023 14:01

JameelB force-pushed the fix/docker-image-build-on-macos branch from 8062ff7 to 766ecf6 Compare March 1, 2023 17:41

miguelsorianod reviewed Mar 1, 2023

View reviewed changes

miguelsorianod reviewed Mar 3, 2023

View reviewed changes

JameelB force-pushed the fix/docker-image-build-on-macos branch 3 times, most recently from c7639ef to 49bb9eb Compare March 6, 2023 17:44

JameelB added 2 commits March 6, 2023 17:48

fix: ensure image can be built on m1 macos

732f14d

When running any of the image build make targets on an m1 macos, it fails due to the following error: qemu-x87_64: Could not open '/lib64/ld-linux-x86-64.so.2 Specify platform to ensure build uses the correct architecture.

fix: remove indentation from make if statements and group var definit…

3306946

…ions with their related make targets

JameelB force-pushed the fix/docker-image-build-on-macos branch from 49bb9eb to 3306946 Compare March 6, 2023 17:51

miguelsorianod approved these changes Mar 7, 2023

View reviewed changes

miguelsorianod merged commit 23b6479 into bf2fc6cc711aee1a0c2a:main Mar 7, 2023

JameelB mentioned this pull request Apr 18, 2023

fix: ensure image build uses correct architecture for macos bf2fc6cc711aee1a0c2a/cos-fleet-manager#263

Open

11 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: ensure image can be built on m1 macos #1602

fix: ensure image can be built on m1 macos #1602

JameelB commented Feb 28, 2023 •

edited

Loading

pb82 left a comment

pawelpaszki left a comment

miguelsorianod commented Feb 28, 2023 •

edited

Loading

miguelsorianod commented Feb 28, 2023 •

edited

Loading

miguelsorianod commented Feb 28, 2023

JameelB commented Feb 28, 2023

miguelsorianod commented Mar 1, 2023 •

edited

Loading

miguelsorianod Mar 1, 2023

JameelB Mar 2, 2023

machi1990 Mar 2, 2023

miguelsorianod Mar 2, 2023

JameelB Mar 2, 2023

miguelsorianod Mar 6, 2023 •

edited

Loading

miguelsorianod Mar 6, 2023 •

edited

Loading

JameelB Mar 6, 2023

miguelsorianod Mar 6, 2023 •

edited

Loading

JameelB Mar 6, 2023

codecov bot commented Mar 2, 2023 •

edited

Loading

miguelsorianod Mar 3, 2023

JameelB Mar 6, 2023

miguelsorianod Mar 6, 2023

JameelB Mar 6, 2023

	ifeq ($(shell uname -s \| tr A-Z a-z), darwin)
	PGHOST:="127.0.0.1"
	else
	PGHOST:="172.18.0.22"
	endif

	ifndef OCM_ENV
	OCM_ENV:=integration
	endif

	ifndef TEST_SUMMARY_FORMAT
	TEST_SUMMARY_FORMAT=short-verbose
	endif

	export GOPROXY=https://proxy.golang.org
	export GOPRIVATE=gitlab.cee.redhat.com

	ifndef SERVER_URL
	SERVER_URL:=http://localhost:8000
	endif

	ifndef TEST_TIMEOUT
	ifeq ($(OCM_ENV), integration)
	TEST_TIMEOUT=15m
	else
	TEST_TIMEOUT=5h
	endif
	endif

fix: ensure image can be built on m1 macos #1602

fix: ensure image can be built on m1 macos #1602

Conversation

JameelB commented Feb 28, 2023 • edited Loading

Description

Verification Steps

Checklist (Definition of Done)

pb82 left a comment

Choose a reason for hiding this comment

pawelpaszki left a comment

Choose a reason for hiding this comment

miguelsorianod commented Feb 28, 2023 • edited Loading

miguelsorianod commented Feb 28, 2023 • edited Loading

miguelsorianod commented Feb 28, 2023

JameelB commented Feb 28, 2023

miguelsorianod commented Mar 1, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

miguelsorianod Mar 6, 2023 • edited Loading

Choose a reason for hiding this comment

miguelsorianod Mar 6, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

miguelsorianod Mar 6, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codecov bot commented Mar 2, 2023 • edited Loading

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

JameelB commented Feb 28, 2023 •

edited

Loading

miguelsorianod commented Feb 28, 2023 •

edited

Loading

miguelsorianod commented Feb 28, 2023 •

edited

Loading

miguelsorianod commented Mar 1, 2023 •

edited

Loading

miguelsorianod Mar 6, 2023 •

edited

Loading

miguelsorianod Mar 6, 2023 •

edited

Loading

miguelsorianod Mar 6, 2023 •

edited

Loading

codecov bot commented Mar 2, 2023 •

edited

Loading