Skip to content
This repository has been archived by the owner on Jun 28, 2024. It is now read-only.

ci: Always install newest Rust version #4288

Merged
merged 1 commit into from
Dec 21, 2021
Merged

ci: Always install newest Rust version #4288

merged 1 commit into from
Dec 21, 2021

Conversation

Jakob-Naucke
Copy link
Member

In the unit tests and the agent shutdown test, we install Rust iff it
isn't installed. On CI instances that are not reinstated for every job
(e.g. ARM & s390x), this leads to Rust never getting updated (1.47 at
the moment). The Rust installation script already does what we should
do: It installs the required version of Rust unless it is already
installed. Use that without checks.

Fixes: #4179
Signed-off-by: Jakob Naucke [email protected]

/cc @dgibson @stevenhorsman

@Jakob-Naucke Jakob-Naucke added needs-backport Changes need to be applied to an older branch / repository area/ci labels Dec 14, 2021
Copy link
Member

@stevenhorsman stevenhorsman left a comment

Choose a reason for hiding this comment

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

The code LGTM, thanks Jakob! I'll kick of the tests and check the output tomorrow

@stevenhorsman
Copy link
Member

/test

Copy link
Contributor

@dgibson dgibson left a comment

Choose a reason for hiding this comment

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

So, I think this problem arises because of some other broken conditions:

  • Having the test scripts manage their own environment is super fragile
  • Installing the latest rust version, rather than the earliest required is basically asking for people to add things that don't actually work with the supposedly earliest supported version.

Nonetheless this addresses a real problem in the short term, and at least makes the scripts simpler and behaviour more consistent across test cases. So, I approve.

@stevenhorsman
Copy link
Member

It looks like the change worked and s390x and ARM jobs do Install rust 1.54.0, though both are still failing

@Jakob-Naucke
Copy link
Member Author

As for the test failures on ARM & s390x: Those instances have rustup installed into a user home as root. This is something I'd like to eliminate altogether. I'm going to go the admittedly cringeworthy route of running them with a temporary commit that removes rustup. Adding DNM for the time being. /test-arm /test-s390x

So, I think this problem arises because of some other broken conditions:

  • Having the test scripts manage their own environment is super fragile

@dgibson you mean it would be better to provision environments without the test scripts, and have the test scripts not install anything? I'm sure it would work, it is a major undertaking though.

  • Installing the latest rust version, rather than the earliest required is basically asking for people to add things that don't actually work with the supposedly earliest supported version.

We are already doing this on all other jobs. DYT we should change this to the oldest supported version? (Probably a separate PR if we do this).

version=$(get_version "languages.rust.meta.newest-version")

@Jakob-Naucke Jakob-Naucke added the do-not-merge PR has problems or depends on another label Dec 15, 2021
@Jakob-Naucke
Copy link
Member Author

Should have pushed before... 🤦 /test-arm /test-s390x

@Jakob-Naucke
Copy link
Member Author

/test-arm /test-s390x

1 similar comment
@Jakob-Naucke
Copy link
Member Author

/test-arm /test-s390x

@Jakob-Naucke Jakob-Naucke removed the do-not-merge PR has problems or depends on another label Dec 15, 2021
@Jakob-Naucke
Copy link
Member Author

Okay, both are past that point. Removing the "cleanup" commit and DNM. Interestingly, I don't need to run most tests because they have already been run on that log. /test-arm /test-s390x

@Jakob-Naucke
Copy link
Member Author

Missed a source if I'm not mistaken -- /test-s390x

@Jakob-Naucke
Copy link
Member Author

forgot to stage, sorry for the noise -- /test-s390x

@Jakob-Naucke
Copy link
Member Author

another source, /test-s390x

@Jakob-Naucke
Copy link
Member Author

s390x:

169713
[cleanup_env.sh:129] ERROR: Found unexpected /usr/local/bin/containerd-shim-kata-v2 present
[run_kubernetes_tests.sh:1] ERROR: popd

this is #3998, /test

@Jakob-Naucke
Copy link
Member Author

ARM:

Step 1/2 : FROM quay.io/libpod/ubuntu
received unexpected HTTP status: 504 Gateway Time-out

s390x: #3998 again (?)
VFIO:

E: Failed to fetch https://packages.cloud.google.com/apt/dists/kubernetes-xenial-unstable/main/binary-amd64/by-hash/SHA256/bb15f9e88b01ede4d1d4701f0ed95125e5535ab248cddbde4784557018ec5c44  404  Not Found [IP: 2607:f8b0:4005:80a::200e 443]
E: Some index files failed to download. They have been ignored, or old ones used instead.

/retest-arm /retest-s390x /retest-vfio

@Jakob-Naucke
Copy link
Member Author

ARM still has

Set environment variables for the libseccomp crate to link the libseccomp library statically
utils.mk:142: "WARNING: aarch64-musl needs extra symbols from libgcc"
make -C src/agent
make[1]: Entering directory '/home/jenkins/workspace/kata-containers-2.0-tests-ubuntu-ARM-PR/go/src/github.com/kata-containers/kata-containers/src/agent'
../../utils.mk:142: "WARNING: aarch64-musl needs extra symbols from libgcc"
/bin/sh: 1: cargo: not found

even after all the sourcing, and s390x, which also runs the unit tests in the Jenkins job, does not have this issue. I can't figure out the difference (this still seems to work with secure_path set). /cc @jongwu maybe?

@jongwu
Copy link
Contributor

jongwu commented Dec 17, 2021

ARM still has

Set environment variables for the libseccomp crate to link the libseccomp library statically
utils.mk:142: "WARNING: aarch64-musl needs extra symbols from libgcc"
make -C src/agent
make[1]: Entering directory '/home/jenkins/workspace/kata-containers-2.0-tests-ubuntu-ARM-PR/go/src/github.com/kata-containers/kata-containers/src/agent'
../../utils.mk:142: "WARNING: aarch64-musl needs extra symbols from libgcc"
/bin/sh: 1: cargo: not found

even after all the sourcing, and s390x, which also runs the unit tests in the Jenkins job, does not have this issue. I can't figure out the difference (this still seems to work with secure_path set). /cc @jongwu maybe?

Maybe the ci machine is not clear. I remove the ".cargo" and the ci passes.

@Jakob-Naucke
Copy link
Member Author

Maybe the ci machine is not clear. I remove the ".cargo" and the ci passes.

@jongwu hmmm, okay. Thanks! Just got to fix #3998 now for s390x...

@Jakob-Naucke
Copy link
Member Author

/retest-s390x

In the unit tests and the agent shutdown test, we install Rust iff it
isn't installed. On CI instances that are not reinstated for every job
(e.g. ARM & s390x), this leads to Rust never getting updated (1.47 at
the moment). The Rust installation script already does what we should
do: It installs the required version of Rust unless it is already
installed. Use that without checks.

Fixes: #4179
Signed-off-by: Jakob Naucke <[email protected]>
@Jakob-Naucke
Copy link
Member Author

Ah. Because there can be root-owned ~/.cargos left, we should always chown. /test

@Jakob-Naucke Jakob-Naucke merged commit a2be2f4 into kata-containers:main Dec 21, 2021
@Jakob-Naucke Jakob-Naucke deleted the update-rust branch December 21, 2021 11:17
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area/ci needs-backport Changes need to be applied to an older branch / repository
Projects
None yet
Development

Successfully merging this pull request may close these issues.

ARM and s390x test cases use an older rustc version
4 participants