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

Add rust to eve-alpine #4145

Closed
wants to merge 1 commit into from
Closed

Conversation

deitch
Copy link
Contributor

@deitch deitch commented Aug 14, 2024

Adds rust from edge to eve-alpine, following the instructions in ALPINE.md:

  1. Found latest tag for eve-alpine from Docker Hub to be fbf39533415522ac124a2da9a695fda686adedc3
  2. Changed the FROM line in pkg/alpine/Dockerfile to use that tag
  3. Added rust to pkg/alpine/mirrors/edge
  4. Committed and pushed changes
  5. Opened this PR 😁

Offhand I do not recall if our pipeline automatically pushes out the updated lfedge/eve-alpine once built.

@deitch
Copy link
Contributor Author

deitch commented Aug 14, 2024

This failure is the oddest thing. I didn't change anything off of master except those 2 files, but go test -race is failing with something related to NIReconciler?

Signed-off-by: Avi Deitcher <[email protected]>
@deitch
Copy link
Contributor Author

deitch commented Aug 14, 2024

Also, there was a bug in build-cache.sh where it was not catching edge and calling it "vedge". This is fixed as well. Thanks to @jsfakian for finding it.

@deitch
Copy link
Contributor Author

deitch commented Aug 14, 2024

OK, now tests pass.

@rucoder
Copy link
Contributor

rucoder commented Aug 15, 2024

@deitch what rust version is that? According to best practices the right way to install rust is ti use rustup. Rust package in OS distros serves only one purpose -- build some rust tools in that distro. If your rust project uses rust-toolchain.toml e.g.

[toolchain]
channel = "1.80.0"
# The supported toolchains used to build the project
targets = ["x86_64-unknown-linux-musl", "aarch64-unknown-linux-musl"]
# riscv target doesn't have a musl target yet and std is not available for it
#, "riscv64gc-unknown-linux-gnu"]
profile = "minimal"

I'm pretty sure it will fail to build. Besides, rust toolchain is a cross toolchain. does Alpine package contains all the targets ?

@deitch
Copy link
Contributor Author

deitch commented Aug 15, 2024

@rucoder that is a good question. The current version is 1.80.1. This is how we install go as well (see the peer file mirrors/edge/community. I believe @famleebob is working on switching our alpine base to 3.20 from 3.16, at which point we can move it all into the main mirrors/3.20/.

Rust package in OS distros serves only one purpose -- build some rust tools in that distro.

Mostly (not entirely) this is how we get tools available. Right now, we only have it in one place, the incoming #4055 interactive-installer, but more are coming. To get it to work, @jsfakian had to install it manually in his package, which is problematic.

There are 2 distinct questions going on here:

  1. How can we make rust available to all downstream packages, the same way they can access go or python, and do it quickly to unblock Added interactive installer package in pkg/installer #4055. This is our current best answer to that.
  2. Is there a better way to make tools like rust (or go, for that matter) available, rather than Alpine's OS package? There might be, I have done some things like that before in our design.

Your 2 is asking a very valid question, which affects more than just rust. I don't want to slow him down, though. We should get this in, get him unblocked, and if we want to change how we install these tools, let's have that discussion separately.

@rucoder
Copy link
Contributor

rucoder commented Aug 15, 2024

@rucoder that is a good question. The current version is 1.80.1. This is how we install go as well (see the peer file mirrors/edge/community. I believe @famleebob is working on switching our alpine base to 3.20 from 3.16, at which point we can move it all into the main mirrors/3.20/.

Rust package in OS distros serves only one purpose -- build some rust tools in that distro.

Mostly (not entirely) this is how we get tools available. Right now, we only have it in one place, the incoming #4055 interactive-installer, but more are coming. To get it to work, @jsfakian had to install it manually in his package, which is problematic.

There are 2 distinct questions going on here:

1. How can we make rust available to all downstream packages, the same way they can access go or python, and do it quickly to unblock [Added interactive installer package in pkg/installer #4055](https://github.com/lf-edge/eve/pull/4055). This is our current best answer to that.

2. Is there a better way to make tools like rust (or go, for that matter) available, rather than Alpine's OS package? There might be, I have done some things like that before in our design.

Your 2 is asking a very valid question, which affects more than just rust. I don't want to slow him down, though. We should get this in, get him unblocked, and if we want to change how we install these tools, let's have that discussion separately.

@deitch I updated my comment with more questions, did not expect you to answer so fast :)

@rucoder
Copy link
Contributor

rucoder commented Aug 15, 2024

@deitch I would just create a separate container e.g. eve-rust, base it on alpine and install all the tools using rustup + cargo-chef . It will simplify later rust version updates without touching eve-alpine.

@rucoder
Copy link
Contributor

rucoder commented Aug 15, 2024

@deitch take a look how I build rust code in a container now. It allows caching the dependencie. the first part of the Dockerfile may be used for eve-rust https://github.com/rucoder/eve/blob/rucoder/eve-mon/pkg/monitor/Dockerfile

@@ -1,6 +1,6 @@
# image was bootstraped using FROM lfedge/eve-alpine-base:fad44e3702708a8d044663a20fd98d933dddb41e AS cache
# to update please see https://github.com/lf-edge/eve/blob/master/docs/BUILD.md#how-to-update-eve-alpine-package
FROM lfedge/eve-alpine:1f7685f95a475c6bbe682f0b976f12180b6c8726 AS cache
FROM lfedge/eve-alpine:fbf39533415522ac124a2da9a695fda686adedc3 AS cache
Copy link
Contributor

Choose a reason for hiding this comment

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

Shouldn't we update all of the FROM eve-alpine lines to use the same tag?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If we go ahead with this. I'm planning on leaving this open, as the eve-rust approach is better. Once that is in, I'll close this.

@deitch deitch marked this pull request as draft August 18, 2024 07:56
@deitch
Copy link
Contributor Author

deitch commented Aug 18, 2024

Converted to a draft, since we are hoping to use github.com/lf-edge/eve-rust as an image.

@rucoder
Copy link
Contributor

rucoder commented Aug 19, 2024

to whom it may concern: the repo eve-rust is created https://github.com/lf-edge/eve-rust/

@rucoder
Copy link
Contributor

rucoder commented Aug 22, 2024

@deitch I guess we can close this one

@deitch
Copy link
Contributor Author

deitch commented Aug 22, 2024

Agreed. I was waiting for everyone to be inline with the eve-rust approach, but now we can close this one.

@deitch deitch closed this Aug 22, 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