-
Notifications
You must be signed in to change notification settings - Fork 136
Create gcloud-bazel image for running e2e tests #156
Conversation
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: fejta The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Hi @fejta. Thanks for your PR. I'm waiting for a bazelbuild member to verify that this patch is reasonable to test. If it is, they should reply with I understand the commands that are listed here. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
781518a
to
b633424
Compare
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.
Most of the part LGTM, especially starting the e2e tests from a docker container.
Some minor comments.
images/gcloud-bazel/Dockerfile
Outdated
|
||
# Ubuntu with bazel, gcloud and its dependencies preinstalled. | ||
|
||
FROM ubuntu:16.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.
Could we use a google-maintained base image?
https://console.cloud.google.com/launcher/details/google/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.
Done
@@ -0,0 +1,2 @@ | |||
# TODO(fejta): replace Dockerfile with an equivalent bazel rule. This seems promising: | |||
# https://github.com/GoogleContainerTools/base-images-docker/blob/master/package_managers/bootstrap_image.bzl |
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 I think (for TODO) package manager is the way to go.
@xingao267 has done quite many works around package manager.
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. Using package manager is promising. We've been using that to install java, wget, git, bazel, docker, and etc in our containers. But we haven't tried gcloud yet (it's on our list), and we are not sure if extra work will be needed to make that work (it could be just as simple as what we did below for Bazel).
I think we can use Dockerfile for now. I'll look into installing gcloud, and once that works. We can change this Dockerfile to bazel rules.
For your interest, we use rules here https://github.com/bazelbuild/bazel-toolchains/blob/master/container/rules/docker_toolchains.bzl to create containers. These rules wrap rules in https://github.com/GoogleContainerTools/base-images-docker/blob/master/package_managers. Example usage of the rules are:
https://github.com/bazelbuild/bazel-toolchains/blob/master/container/ubuntu16_04/builders/bazel/BUILD#L23
https://github.com/bazelbuild/bazel-toolchains/blob/master/container/ubuntu16_04/layers/bazel/BUILD
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.
Ack
I'll circle back later and try and figure out how to use these rules.
images/gcloud-bazel/Makefile
Outdated
@@ -0,0 +1,30 @@ | |||
# Copyright 2017 The Bazel Authors. All rights reserved. |
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.
Using a Makefile is not ideal to me...
We usually use a cloudbuild.yaml to build a container even for only one step (build from a Dockerfile). In this way we can ensure all the containers are build in a controlled environment with Google Cloud Container Builder
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.
Done
The Makefile is necessary to get the tags I want, but it now uses gcb to push builds.
@@ -0,0 +1,2 @@ | |||
# TODO(fejta): replace Dockerfile with an equivalent bazel rule. This seems promising: |
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.
License header
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.
Done
@@ -0,0 +1,2 @@ | |||
# TODO(fejta): replace Dockerfile with an equivalent bazel rule. This seems promising: | |||
# https://github.com/GoogleContainerTools/base-images-docker/blob/master/package_managers/bootstrap_image.bzl |
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. Using package manager is promising. We've been using that to install java, wget, git, bazel, docker, and etc in our containers. But we haven't tried gcloud yet (it's on our list), and we are not sure if extra work will be needed to make that work (it could be just as simple as what we did below for Bazel).
I think we can use Dockerfile for now. I'll look into installing gcloud, and once that works. We can change this Dockerfile to bazel rules.
For your interest, we use rules here https://github.com/bazelbuild/bazel-toolchains/blob/master/container/rules/docker_toolchains.bzl to create containers. These rules wrap rules in https://github.com/GoogleContainerTools/base-images-docker/blob/master/package_managers. Example usage of the rules are:
https://github.com/bazelbuild/bazel-toolchains/blob/master/container/ubuntu16_04/builders/bazel/BUILD#L23
https://github.com/bazelbuild/bazel-toolchains/blob/master/container/ubuntu16_04/layers/bazel/BUILD
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.
Thanks for all the great comments!
@@ -0,0 +1,2 @@ | |||
# TODO(fejta): replace Dockerfile with an equivalent bazel rule. This seems promising: |
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.
Done
@@ -0,0 +1,2 @@ | |||
# TODO(fejta): replace Dockerfile with an equivalent bazel rule. This seems promising: | |||
# https://github.com/GoogleContainerTools/base-images-docker/blob/master/package_managers/bootstrap_image.bzl |
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.
Ack
I'll circle back later and try and figure out how to use these rules.
images/gcloud-bazel/Dockerfile
Outdated
|
||
# Ubuntu with bazel, gcloud and its dependencies preinstalled. | ||
|
||
FROM ubuntu:16.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.
Done
images/gcloud-bazel/Makefile
Outdated
@@ -0,0 +1,30 @@ | |||
# Copyright 2017 The Bazel Authors. All rights reserved. |
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.
Done
The Makefile is necessary to get the tags I want, but it now uses gcb to push builds.
/assign @erain
Next will create a prow job which uses this image and runs
test-e2e.sh