-
Notifications
You must be signed in to change notification settings - Fork 97
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 rbe-debian9 and rbe-ubuntu16_04 container #18
Conversation
rbe-debian9
and rbe-ubuntu16_04
container
rbe-debian9
and rbe-ubuntu16_04
container* we also rename debian8-clang-fully-loaded to rbe-debian8: https://pantheon.corp.google.com/launcher/details/google/rbe-debian8 Tested: * container/build.sh -p my-project -c rbe-debian8 -d debian8 -t latest -b my-debs -a * container/build.sh -p my-project -c rbe-debian9 -d debian9 -t latest -b my-debs -a * container/build.sh -d debian8 -l * container/build.sh -d debian9 -l * tested produced rbe-debian9 image for building gRPC and Bazel Change-Id: I233193c2bfaa4f9bf4f759de9c8f62f9c9179ae5
Change-Id: I65c8e4f38301afc8e1a5509012aaf10a839da421
Change-Id: Iaf0381722afbb4bd61d1ccac941e5b1b6c4ab5cc
Change-Id: I6919105867e3f564d277864b13c63d754ee8fe3c
container/build.sh
Outdated
@@ -23,9 +23,10 @@ Usage: build.sh [options] | |||
Builds the fully-loaded container, with Google Cloud Container Builder or locally. | |||
|
|||
Required parameters (when build with Google Cloud Container Builder): | |||
-d|--distro Distro of the base image, debian 8, debian 9 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
add full list of supported base images here with the same names as expected below (debian8, debian9, ubuntu16_04)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed.
|
||
container_test( | ||
name = "fl-toolchain-test", | ||
configs = ["//container/test:rbe-ubuntu16_04.yaml"], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Lets try to see if we can pass several files here and split off everything that is common from the verifications below to a single common file.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes this is do-able and I have extracted the common part into a common.yaml file to share among 3 containers.
- consolidate the common part of container_test into one common.yaml config file - fix other minor formatting problems. Change-Id: I2e1fd01682471c0888b8414b3b4716250bce3c2e
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
couple of minor nits, maybe replace fl- for rbe- in the two names i mentioned?
) | ||
|
||
container_test( | ||
name = "fl-toolchain-test", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
call this just "toolchain-test"
load("@io_bazel_rules_docker//contrib:test.bzl", "container_test") | ||
|
||
toolchain_container( | ||
name = "fl-toolchain", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
just "toolchain"? (or rbe-toolchain or something like that?)
Change-Id: I63dea016b728f8cce42f491bb553960585002c63
I have replaced "fl-toolchain" to "toolchain" everywhere and tested the changes. |
The test failed because the infra itself, and not because the test. I ran the tests locally and they all succeeded. Merging the change. |
After (many) retries the tests finally passed. We should bring it up to bazel testing infra team, but not the priority ATM. I will merge this change and started testing the new images. |
As mentioned above, this change breaks the tests on the CI: https://buildkite.com/bazel/bazel-toolchains/builds/120 |
@vladmos I have been tested the failing tests and they all passed locally
I am sending a PR to disable these two tests temporarily. However, on the other hand, is there a way for me to debug the Bazel CI system to see what might be wrong? |
- due to Bazel CI problem, see comments in bazelbuild#18 Change-Id: I819d4f424f2ab0092730fa5eeb9ad3060c78008a
Here's an example of the test logs, looks like the OS version string doesn't match the expected string: https://storage.googleapis.com/bazel-buildkite-artifacts/41fcd03a-3a4c-4ddd-937a-95ddf5a0df3f/container/experimental/rbe-debian9/toolchain-test/attempt_1.log |
* temporarily disable container tests - due to Bazel CI problem, see comments in #18
Hi Vladimir, If you take a closer look, you'll notice things in the tests output were messed up. The test itself complain the following target failed: So it looks to me that the stdout of another test, |
Manjaro is based on the Arch linux distribution, so should have the same sysroot composition.
libc++6-dev
andca-certificates-java
ca-certificates-java
Tested: