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

kata-ctl: Re-enable network check on s390x #5438

Closed
jodh-intel opened this issue Oct 15, 2022 · 4 comments · Fixed by #5447
Closed

kata-ctl: Re-enable network check on s390x #5438

jodh-intel opened this issue Oct 15, 2022 · 4 comments · Fixed by #5447
Labels
area/s390x Issues specific to the the S390x machine architecture enhancement Improvement to an existing feature kata-ctl Kata Control Tool project lang/rust Rust code

Comments

@jodh-intel
Copy link
Contributor

The kata-ctl network check needs to be disabled on s390x using #5435 as that platform apparently doesn't support rust-tls (yet).

If and when rust-tls is supported, the reqwest HTTP crate can be used unconditionally by kata-ctl.

/cc @BbolroC.

@jodh-intel jodh-intel added enhancement Improvement to an existing feature lang/rust Rust code area/s390x Issues specific to the the S390x machine architecture kata-ctl Kata Control Tool project labels Oct 15, 2022
@BbolroC
Copy link
Member

BbolroC commented Oct 15, 2022

Thanks for looking into the issue. I will keep an eye on the support.

@hbrueckner
Copy link
Contributor

fyi, the TLS library used by rust-tls, ring lacks support for s390x. There is s390x basic support PR pending since last year: briansmith/ring#1297

Not sure if there are other TLS alternatives for doing the tests or if we need to work and wait on above dependency.

@hbrueckner
Copy link
Contributor

Hi @jodh-intel

I briefly looked at https://github.com/seanmonstar/reqwest and it seems that reqwest can use the native TLS crate (openssl) and being rustls-tls an optional feature. For s390x, I wonder whether simplying removing the rustls-tls feature would be the easiest option to re-enable the test case. Wdyt?

@jodh-intel
Copy link
Contributor Author

@hbrueckner, @BbolroC - I had problems using reqwest with the native libs on x86_64 due to our rust toolchain using musl as the libc, hence switched to rust-tls.

I'm trying to land the admittedly rather ugly but I think working fix (#5439) to unbreak the Kata CI.

However, it would be great if one of you who has access to s380x h/w could raise a follow-up PR that fixed the problem "more properly". Maybe we could have something like this in the manifest:

[target.'cfg(not(target_arch = "s390x"))'.dependencies]
reqwest = { version = "0.11", default-features = false, features = ["json", "blocking", "rustls-tls"] }

[target.'cfg(target_arch = "s390x")'.dependencies]
reqwest = { version = "0.11", default-features = false, features = ["json", "blocking"] }

But I think you'd still need to find a solution to the musl problem. An alternative might be that we build kata-ctl using the standard libc though as I mentioned on jodh-intel#10 (review).

hbrueckner added a commit to hbrueckner/kata-containers that referenced this issue Oct 18, 2022
hbrueckner added a commit to hbrueckner/kata-containers that referenced this issue Oct 18, 2022
Fixes: kata-containers#5438

Co-authored-by: James O. D. Hunt <[email protected]>
Signed-off-by: Hendrik Brueckner <[email protected]>
hbrueckner added a commit to hbrueckner/kata-containers that referenced this issue Oct 20, 2022
For s390x, use native-tls for reqwest because the rustls-tls/ring
dependency is not available for s390x.

Also exclude s390x, powerpc64le, and aarch64 from running the cpu
check due to the lack of the arch-specific implementation. In this
case, rust complains about unused functions in src/check.rs (both
normal and test context).

Fixes: kata-containers#5438

Co-authored-by: James O. D. Hunt <[email protected]>
Signed-off-by: Hendrik Brueckner <[email protected]>
hbrueckner added a commit to hbrueckner/kata-containers that referenced this issue Oct 20, 2022
Add a basic s390x cpu check for the "sie" feature to be present.
Also re-enable cpu check testing.

Fixes: kata-containers#5438

Signed-off-by: Hendrik Brueckner <[email protected]>
hbrueckner added a commit to hbrueckner/kata-containers that referenced this issue Oct 21, 2022
For s390x, use native-tls for reqwest because the rustls-tls/ring
dependency is not available for s390x.

Also exclude s390x, powerpc64le, and aarch64 from running the cpu
check due to the lack of the arch-specific implementation. In this
case, rust complains about unused functions in src/check.rs (both
normal and test context).

Fixes: kata-containers#5438

Co-authored-by: James O. D. Hunt <[email protected]>
Signed-off-by: Hendrik Brueckner <[email protected]>
hbrueckner added a commit to hbrueckner/kata-containers that referenced this issue Oct 21, 2022
Add a basic s390x cpu check for the "sie" feature to be present.
Also re-enable cpu check testing.

Fixes: kata-containers#5438

Signed-off-by: Hendrik Brueckner <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/s390x Issues specific to the the S390x machine architecture enhancement Improvement to an existing feature kata-ctl Kata Control Tool project lang/rust Rust code
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

3 participants