Run tests on arm64 as well #600

mattmoor · 2020-09-29T23:42:05Z

Enable travis builds of arm64 in addition to amd64, and extend the multi-arch support to include everything under //base, including the container_test rules.

This required quite a few changes across a number of repos. Thanks to @nkubala for the quick turnaround on PRs to the structure test repo, and the 1.9.1 release. The one outstanding PR is to pull the 1.9.1 release into rules_docker here: bazelbuild/rules_docker#1639.

Unfortunately, Bazel's support for filtering targets by architecture is still pretty weak. I tried a half dozen options from constraints (restricted_to) to config settings (select) and at best I could make the targets on mismatched platforms emit an error, but Bazel doesn't yet support filtering them (and the issue is >3 years old).

The workaround I was able to find is similar to how Google runs integration testing internally, which is to tag the tests as tags = ["manual"] and then to run specific targets, however, I also included the architecture the test is expected to run on, e.g. tags = ["manual", "amd64"]. Then in Travis I use bazel query to pull out all of the appropriate tests for each architecture and run them explicitly. This required me to tag all of the existing tests (that haven't moved) as amd64 explicitly, but as we move these to loops over ARCHITECTURES we would use the appropriate architecture and the arm64 support should "just work" 🤞

We shouldn't merge this until I'm able to rebase on the merged rules_docker PR, but I'd appreciate it if folks would take a look and leave any other comments they might have. 🙏

examples/rust/BUILD

…images.

chanseokoh

This is cool stuff!

I don't have much expertise in Bazel, so Unfortunately I cannot comment on what would be the best approach to build and test based on the environment. (Strange, I feel like this kind of feature should be a basic component in any testing frameworks.) But as long as it works, this is a great progress.

WORKSPACE

cc/BUILD

cpus/BUILD

chanseokoh · 2020-10-01T14:36:45Z

cloudbuild.yaml

+    docker tag bazel/base:base_root_arm64_debian9        gcr.io/$PROJECT_ID/base:latest-arm64
+    docker tag bazel/base:base_root_arm64_debian9        gcr.io/$PROJECT_ID/base:nonroot-arm64
+    docker tag bazel/base:debug_root_arm64_debian9       gcr.io/$PROJECT_ID/base:debug-arm64
+    docker tag bazel/base:debug_root_arm64_debian9       gcr.io/$PROJECT_ID/base:debug-nonroot-arm64


Suggested change

docker tag bazel/base:base_root_arm64_debian9 gcr.io/$PROJECT_ID/base:latest-arm64

docker tag bazel/base:base_root_arm64_debian9 gcr.io/$PROJECT_ID/base:nonroot-arm64

docker tag bazel/base:debug_root_arm64_debian9 gcr.io/$PROJECT_ID/base:debug-arm64

docker tag bazel/base:debug_root_arm64_debian9 gcr.io/$PROJECT_ID/base:debug-nonroot-arm64

docker tag bazel/base:base_root_arm64_debian9 gcr.io/$PROJECT_ID/base:latest-arm64

docker tag bazel/base:base_nonroot_arm64_debian9 gcr.io/$PROJECT_ID/base:nonroot-arm64

docker tag bazel/base:debug_root_arm64_debian9 gcr.io/$PROJECT_ID/base:debug-arm64

docker tag bazel/base:debug_nonroot_arm64_debian9 gcr.io/$PROJECT_ID/base:debug-nonroot-arm64

And likewise, eventually we should also tag these in -debian9.

Good catch! re: -debian9 I think we need a better strategy for scripting the variants, but totally agree. This is quite tedious to keep straight as-is 😅

chanseokoh · 2020-10-01T14:39:45Z

cloudbuild.yaml

+    docker tag bazel/base:base_root_amd64_debian9        gcr.io/$PROJECT_ID/base:${COMMIT_SHA}
+    docker tag bazel/base:base_root_amd64_debian9        gcr.io/$PROJECT_ID/base:latest-amd64
+    docker tag bazel/base:base_root_amd64_debian9        gcr.io/$PROJECT_ID/base-debian9:${COMMIT_SHA}
+    docker tag bazel/base:base_root_amd64_debian9        gcr.io/$PROJECT_ID/base-debian9:latest


Ideally and eventually, I think this should be a manifest list. We've maintained that base and base-debian9 are basically identical. (And same for -debian10.) But I guess you're adding manifest lists only to base as a start?

Yeah, I wanted to plumb through the main things for each first. I think we need a bigger discussion around how we (likely) script these variations to cut down on the boilerplate (and sorts of errors I had above)

mattmoor · 2020-10-01T15:01:11Z

I pushed a commit with the couple edits called out. I added the debian9/10 variant topic to my list. Once I land the open chain I'll open issues for the remaining pieces of work on my radar (java, node, dotnet, this, other architectures like ppc and s390x).

chanseokoh

Looks good to me.

chanseokoh · 2020-10-01T15:39:02Z

base/base.bzl

+                name = "debug_" + user + "_" + arch + distro_suffix,
+                architecture = arch,
+                base = ":base_" + user + "_" + arch + distro_suffix,
+                directory = "/",


Maybe directory is unnecessary, as it's default is /?

I think this is just an indentation thing, I didn't change this.

chanseokoh · 2020-10-01T15:44:21Z

base/base.bzl

+        container_test(
+            name = "openssl_" + arch + distro_suffix + "_test",
+            configs = ["testdata/certs.yaml"],
+            image = ":base_root_" + arch + distro_suffix,


Is it meaningful to parameterize user (here and tests below) given that we're in a loop, or it is not worth?

We could look at doing this more, it wasn't clear to me if/which of these tests might do something privileged, so I tried to avoid creeping the scope too much.

The one thing I did make a point to do was make the pure-go test use static, which I believe previously had zero direct test coverage.

mattmoor · 2020-10-01T16:00:12Z

@chanseokoh if it's alright I am going to punt on further churn here (since both are pre-existing limitations, and I want to maintain sanity rebasing the other changes), but I agree with the cleanups / experiments you suggest.

I can try to follow up with these on top of the fuller chain of changes 🙏

mattmoor · 2020-10-01T16:01:24Z

Note: there are some other funny warts I haven't addressed, like plumbing root/nonroot through all the variants (I don't think //cc or python2.7 have it, but python3 does).

chanseokoh

Sounds good! Thanks for the awesome work.

googlebot added the cla: yes CLAs look good label Sep 29, 2020

mattmoor force-pushed the multi-arch branch 15 times, most recently from fe2b82b to 21a2ff9 Compare October 1, 2020 01:59

mattmoor commented Oct 1, 2020

View reviewed changes

examples/rust/BUILD Outdated Show resolved Hide resolved

mattmoor force-pushed the multi-arch branch from 21a2ff9 to 342e9e0 Compare October 1, 2020 02:17

mattmoor mentioned this pull request Oct 1, 2020

[multi-arch] Make distroless/base:{latest,nonroot,debug,debug-nonroot} multi-arch #595

Closed

mattmoor added 2 commits October 1, 2020 14:17

Make distroless/base:{latest,nonroot,debug,debug-nonroot} multi-arch …

a1859e2

…images.

Run tests on arm64 as well

95ae7a5

mattmoor force-pushed the multi-arch branch from 342e9e0 to 95ae7a5 Compare October 1, 2020 14:20

mattmoor changed the title ~~[WIP] Run tests on arm64 as well~~ Run tests on arm64 as well Oct 1, 2020

mattmoor requested review from chanseokoh and dlorenc October 1, 2020 14:20

mattmoor assigned dlorenc and chanseokoh Oct 1, 2020

chanseokoh reviewed Oct 1, 2020

View reviewed changes

chanseok feedback

f337f66

chanseokoh reviewed Oct 1, 2020

View reviewed changes

chanseokoh approved these changes Oct 1, 2020

View reviewed changes

mattmoor merged commit 34b23ec into GoogleContainerTools:master Oct 1, 2020

mattmoor deleted the multi-arch branch October 1, 2020 16:20

This was referenced Nov 13, 2020

releng: Promote new images built with go1.15.5 kubernetes/k8s.io#1423

Merged

Golang 1.15.5 updates kubernetes/release#1651

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Run tests on arm64 as well #600

Run tests on arm64 as well #600

mattmoor commented Sep 29, 2020 •

edited

Loading

chanseokoh left a comment

chanseokoh Oct 1, 2020

mattmoor Oct 1, 2020

chanseokoh Oct 1, 2020

mattmoor Oct 1, 2020

mattmoor commented Oct 1, 2020

chanseokoh left a comment

chanseokoh Oct 1, 2020

mattmoor Oct 1, 2020

chanseokoh Oct 1, 2020

mattmoor Oct 1, 2020

mattmoor commented Oct 1, 2020

mattmoor commented Oct 1, 2020

chanseokoh left a comment

Run tests on arm64 as well #600

Run tests on arm64 as well #600

Conversation

mattmoor commented Sep 29, 2020 • edited Loading

chanseokoh left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mattmoor commented Oct 1, 2020

chanseokoh left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mattmoor commented Oct 1, 2020

mattmoor commented Oct 1, 2020

chanseokoh left a comment

Choose a reason for hiding this comment

mattmoor commented Sep 29, 2020 •

edited

Loading