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

image-rs: Add cargo build for multiarch #491

Merged

Conversation

stevenhorsman
Copy link
Member

@stevenhorsman stevenhorsman commented Feb 29, 2024

  • Add building image-rs on s390x and powerpc64le
    Note: At the moment this will probably fail with:
error: failed to run custom build command for `ring v0.16.20`

Caused by:
  process didn't exit successfully: `/root/guest-components/target/debug/build/ring-f931d4c26d8deee2/build-script-build` (exit status: 101)
  --- stderr
  thread 'main' panicked at /root/.cargo/registry/src/index.crates.io-6f17d22bba15001f/ring-0.16.20/build.rs:358:10:
  called `Option::unwrap()` on a `None` value
  note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

whilst we wait for #422 to get fixed, so we shouldn't merge it until after that is fixed

@ChengyuZhu6
Copy link
Member

Hi Steve. #422 is fixed by @Xynnn007 . And it works on the guest pull PR https://github.com/kata-containers/kata-containers/actions/runs/8242824012/job/22542388463?pr=8484.

@stevenhorsman stevenhorsman force-pushed the image-rs-multi-arch-tests branch from b806a25 to 5c1ac60 Compare March 12, 2024 09:27
@stevenhorsman
Copy link
Member Author

Hi Steve. #422 is fixed by @Xynnn007 . And it works on the guest pull PR https://github.com/kata-containers/kata-containers/actions/runs/8242824012/job/22542388463?pr=8484.

Great - I've rebased, so we'll see if this is passes now 🤞

@stevenhorsman stevenhorsman force-pushed the image-rs-multi-arch-tests branch from 5c1ac60 to 274f355 Compare March 12, 2024 10:13
@stevenhorsman
Copy link
Member Author

stevenhorsman commented Mar 12, 2024

We're getting ssl related errors on s390x, which I have seen locally before as well, so as directed in the error:

 Make sure you also have the development packages of openssl installed.
  For example, `libssl-dev` on Ubuntu or `openssl-devel` on Fedora.

I'm trying adding libssl-dev as a dependency

@stevenhorsman
Copy link
Member Author

It doesn't seem to be working and it was previously, so maybe something in the stack has changed things?

@Xynnn007
Copy link
Member

It doesn't seem to be working and it was previously, so maybe something in the stack has changed things?

Oh. There is actually a hidden problem. openssl does not support cross-compilation in rust. The logic code in image-rs does not reference openssl, but it is used in the unit test, so the cargo test here cannot be built successfully. Let me fix this in another PR and you can rebase the code. hopefully it works.

@Xynnn007
Copy link
Member

#501

@stevenhorsman stevenhorsman force-pushed the image-rs-multi-arch-tests branch from 274f355 to 7fb616e Compare March 13, 2024 08:57
@stevenhorsman
Copy link
Member Author

stevenhorsman commented Mar 13, 2024

We're now failing with:

error: test failed, to rerun pass `-p image-rs --lib`
Error: test failed, to rerun pass `-p image-rs --lib`
Caused by:
  could not execute process `/home/runner/work/guest-components/guest-components/target/s390x-unknown-linux-gnu/debug/deps/image_rs-594bc804859e5604` (never executed)

Caused by:
  Exec format error (os error 8)

I guess this was silly of me to try and run tests against a different platform binaries, so I could switch this to just a cargo build to ensure that it compiles, unless anyone has a better suggestion?

@Xynnn007
Copy link
Member

I guess this was silly of me to try and run tests against a different platform binaries, so I could switch this to just a cargo build to ensure that it compiles, unless anyone has a better suggestion?

Makes sense. Once we have a s390x test machine, we can do the test. :P

@stevenhorsman
Copy link
Member Author

Makes sense. Once we have a s390x test machine, we can do the test. :P

We do have an s390x test-runner in the CoCo organisation, so that might be a possibility. \cc @BbolroC

- Add build of image-rs on s390x and powerpc64le

Signed-off-by: stevenhorsman <[email protected]>
@stevenhorsman stevenhorsman force-pushed the image-rs-multi-arch-tests branch from 7fb616e to 4f457ce Compare March 13, 2024 09:57
@stevenhorsman stevenhorsman changed the title image-rs: Add cargo test for multiarch image-rs: Add cargo build for multiarch Mar 13, 2024
@BbolroC
Copy link
Member

BbolroC commented Mar 13, 2024

Makes sense. Once we have a s390x test machine, we can do the test. :P

We do have an s390x test-runner in the CoCo organisation, so that might be a possibility. \cc @BbolroC

Yeah, we can use the current s390x runner for this.

@stevenhorsman stevenhorsman marked this pull request as ready for review March 13, 2024 10:22
@stevenhorsman stevenhorsman requested a review from sameo as a code owner March 13, 2024 10:22
@stevenhorsman
Copy link
Member Author

All checks are passing now, so I've moved this out of draft. Thanks so much Ding for removing the blockers on this.

Copy link
Member

@ChengyuZhu6 ChengyuZhu6 left a comment

Choose a reason for hiding this comment

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

LGTM Thanks @stevenhorsman

@arronwy arronwy merged commit 46b7b06 into confidential-containers:main Mar 14, 2024
5 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.

5 participants