local-path-provisioner icon indicating copy to clipboard operation
local-path-provisioner copied to clipboard

Add support for RISC-V

Open chazapis opened this issue 1 year ago • 10 comments

Adds the RISC-V architecture to the scripts involved in building and packaging Local Path Provisioner; no code changes.

The resulting container has been tested in QEMU with a RISC-V build of K3s and is working fine.

Notes

  • A riscv64 binary is generated by cross-compilation when $ARCH == riscv64.
  • Testing and validation steps that are part of the ci script are skipped (options set via SKIP_TEST and SKIP_VALIDATE environment variables).
  • The container image is built using Docker BuildKit. The base container image for Local Path Provisioner is now an argument, so it can be easily changed.

chazapis avatar Jul 08 '23 05:07 chazapis

LGTM @Anarkis do we have RISC-V machines for building?

There is no need to build natively on RISC-V. The patch sets GOARCH to cross-compile a binary. This seems to work ok in the Drone CI environment. However, packaging up the container image fails... Could it be that the underlying Docker instance does not support BuildKit?

chazapis avatar Jul 10 '23 14:07 chazapis

LGTM @Anarkis do we have RISC-V machines for building?

There is no need to build natively on RISC-V. The patch sets GOARCH to cross-compile a binary. This seems to work ok in the Drone CI environment. However, packaging up the container image fails... Could it be that the underlying Docker instance does not support BuildKit?

We still have CI test before releasing, so RISC-V is required...

However, packaging up the container image fails... Could it be that the underlying Docker instance does not support BuildKit?

Need to check it.

derekbit avatar Jul 10 '23 14:07 derekbit

LGTM @Anarkis do we have RISC-V machines for building and release?

@pgonin any info for this topic?

Anarkis avatar Jul 13 '23 06:07 Anarkis

@derekbit, @Anarkis, @pgonin Sorry to bother you, but it has been over 3 months with no response. This is a really helpful feature that requires no code changes. Could it possibly be included in the next release?

chazapis avatar Oct 25 '23 09:10 chazapis

@chazapis Sorry for late reply. I am reviewing and preparing a new release. I am fine with the PR. Currently, we don't have machines for RISC-V test, so I'm thinking the workaround. Any feedback is appreciated.

derekbit avatar Oct 25 '23 12:10 derekbit

While we have some hardware in our lab, we generally use QEMU for testing on RISC-V. However, I am not sure how this would tie up with running the CI jobs using Drone...

On the other hand, since the RISC-V community is still small, I think it would be perfectly acceptable to publish a build marked as experimental/untested.

chazapis avatar Oct 26 '23 08:10 chazapis

@chazapis It sounds good.

derekbit avatar Oct 27 '23 02:10 derekbit

@chazapis We currently lack RISC-V machines for the building and release processes. To prevent triggering the drone task, could you please remove the .drone.yaml file.

I'm fine with the other changes. Could you please update the README.md to include instructions on building the image for the RISC-V platform? I think RISC-V users can build the image by themselves first.

WDYT?

derekbit avatar Nov 18 '23 17:11 derekbit

@derekbit, I think it is worth trying to include a RISC-V image in each release. As local-path-provisioner is meant to run in containerized form within Kubernetes, it is not really helpful to tell users to recompile and package up everything themselves. The goal is to run K3s out of the box on RISC-V. In a recent discussion with @brandond in the respective K3s PR, he also mentions that the most important issue for RISC-V support right now is the lack of architecture-compatible container images.

So, if there is any way that local-path-provisioner can build and release RISC-V images, I suggest that we pursue that path. If it requires more work, I am eager to help. On the other hand, if you think that this is not possible at this point, I can remove the Drone.io part of this PR and add it in another.

chazapis avatar Nov 22 '23 11:11 chazapis