Skip to content

Conversation

@RoEdAl
Copy link
Contributor

@RoEdAl RoEdAl commented Feb 14, 2025

Thanks to docker/build-push-action@v6 action now it's easy to build multiarchitecture Docker images. So, I decided to modify docker-ghcrio.yml workflow and build (publish) Docker images for ARM64 (linux/arm64) and ARM (linux/arm/v7,linux/arm/v6) architectures. Dockerfile was also improved a bit.

Related PR: go-graphite/carbonapi#858

@deniszh
Copy link
Member

deniszh commented Feb 14, 2025

Hi @RoEdAl

Thanks, I like your idea, but:

  1. tests are broken, could you please check?
  2. image build run for 30m and not even finished, maybe it's stuck because of broken tests. But if not, maybe it's better to stick to amd64 and linux/arm64 only ?

@RoEdAl
Copy link
Contributor Author

RoEdAl commented Feb 14, 2025

  1. Dockerfile was fixed. Please run tests again.
  2. Building all images takes about 45m. This is normal because non-x86 architectures uses QEMU which is rather slow. Furthermore, tests are performed on every platform (architecture).

@deniszh
Copy link
Member

deniszh commented Feb 14, 2025

45 min is too long, I'm afraid. We need to do that somehow asynchronously, for not waiting 45 minutes to merge PR

@RoEdAl
Copy link
Contributor Author

RoEdAl commented Feb 14, 2025

Maybe docker-ghcrio.yml workflow triggered by PR should build only one linux/amd64 Docker image?

@deniszh
Copy link
Member

deniszh commented Feb 14, 2025

Make sense. We can build everything only for releases.

@deniszh
Copy link
Member

deniszh commented Feb 14, 2025

PS: tests still not fixed :(
You can just test them using "make image"

@RoEdAl
Copy link
Contributor Author

RoEdAl commented Feb 14, 2025

The problem is that make image command works fine on my computer.
My next try is to execute make clean before make inside Docker container so please run tests again.

@RoEdAl
Copy link
Contributor Author

RoEdAl commented Feb 15, 2025

Updating workflow. Now only linux/amd64 image is build when triggered by PR.

@deniszh
Copy link
Member

deniszh commented Feb 15, 2025

Looks good, merging!

@deniszh deniszh merged commit d952e3d into go-graphite:master Feb 15, 2025
9 checks passed
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.

2 participants