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

Use correct targetplatform #104

Closed

Conversation

erikbosch
Copy link
Contributor

@erikbosch erikbosch commented Jun 26, 2024

Based on mail on velocitas-dev (see below)

Similar setup already used in
https://github.com/eclipse-velocitas/vehicle-app-python-template/blob/main/app/Dockerfile

When testing with --platform=linux/arm64 I did not notice any problems using scratch in the last step, and for scratch platform should not matter right, it just copies what we have from previous step, or?

FROM --platform=$TARGETPLATFORM gcr.io/distroless/base-debian12 as runner

Tests performed

Built locally with platform flag on a Debian VM for arm64

sudo apt-get install qemu binfmt-support qemu-user-static
docker run --rm --privileged multiarch/qemu-user-static --reset -p yes
docker build -f app/Dockerfile --progress=plain --platform=linux/arm64 -t ex3:latest .
docker image inspect ex3:latest | grep Arch

Original mail


Hi Velocitas-dev Team,

_We found a maybe a minor issue in the Dockerfile when building it for arm64 machines.

Background:
We are creating a POC using the vehicle-app-cpp-template.
Everything worked great when we built the docker images in our host machine (amd64) and deployed it in ubuntu test machine (amd64).
However, when we built the docker image for arm64 and still using our host machine (amd64) and to be deployed in our edge devices (arm64).
The final image is still in amd64.

This is how docker build the apps:
$ cd vehicle-app-cpp-template/app
$ docker build -f app/Dockerfile --progress=plain --build-arg TARGETPLATFORM=linux/arm64 -t localhost:12345/app-seat-adjuster:0.0.1 .

After building it we docker image inspect the docker image.: 
$ docker image inspect localhost:12345/app-seat-adjuster:0.0.1 | grep Arch
$ "Architecture": "amd64",

We fixed it by modifying cd vehicle-app-cpp-template/app/Dockerfile.
# FROM ghcr.io/eclipse-velocitas/devcontainer-base-images/cpp:v0.3 as builder
FROM --platform=$TARGETPLATFORM ghcr.io/eclipse-velocitas/devcontainer-base-images/cpp:v0.3 as builder
and
# FROM --platform=$TARGETPLATFORM scratch as runner
FROM --platform=$TARGETPLATFORM gcr.io/distroless/base-debian12 as runner

Note:
•	We did not use scratch since it will still default to linux/amd64 even supplying --platform=linux/arm64

After this minor fix. We can now successfully deploy it with our edge devices (arm64).

Does our Dockerfile changes is correct way to go forward or do you recommend another way to build docker(arm64) images using host(amd64)?

Your response is highly appreciated.

Thank you,_ 

@@ -1,3 +1,4 @@
# This file is maintained by velocitas CLI, do not modify manually. Change settings in .velocitas.json
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is needed to make CI happy, fails also on main currently, same for license checks

Copy link
Contributor

@dennismeister93 dennismeister93 Jun 27, 2024

Choose a reason for hiding this comment

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

Yes with the new version of the cli (Which is part of the new base image) this dockerfile is maintained through our devcontainer-setup. We forgot to update the versions inside velocitas.json (Just for information, the approach is -> Starting the template in devcontainer and execute: velocitas upgrade --ignore-bounds && velocitas init && velocitas sync). Shall i create a seperate PR which is doing it instead of putting this inside this PR?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Doesn't really matter for me, if you want to fix it in a separate PR I can later rebase on that PR.

Copy link
Contributor

Choose a reason for hiding this comment

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

created a PR
#106

@@ -14,7 +14,7 @@

# syntax = docker/dockerfile:1.2

FROM ghcr.io/eclipse-velocitas/devcontainer-base-images/cpp:v0.3 as builder
FROM --platform=$TARGETPLATFORM ghcr.io/eclipse-velocitas/devcontainer-base-images/cpp:v0.3 as builder
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I managed to get things running for arm without changing "scratch" or adding "--platform" there. It anyway only copy things right, so this should maybe be sufficient.

Copy link
Contributor

Choose a reason for hiding this comment

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

do you have an arm device? otherwise I would assume doing it like this still requires qemu.

so what I would suggest is to properly use cross compilation (-x )

@erikbosch
Copy link
Contributor Author

Please have a look on if this seems reasonable. There are a lot of items in the checklist which I do not really know when they are useful. Like when do we actually want to check that "Vehicle App can be deployed to local Kanto runtime and is running". No checks removed so far

@erikbosch erikbosch marked this pull request as ready for review June 26, 2024 16:25
@erikbosch
Copy link
Contributor Author

Fixes #105

@erikbosch
Copy link
Contributor Author

Converted to draft, PR might not be needed

@erikbosch
Copy link
Contributor Author

Closing this one, with right arguments no change seems to be needed, see #107

@erikbosch erikbosch closed this Jun 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants