-
Notifications
You must be signed in to change notification settings - Fork 52
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
chore: bake the grpc plugin in the hermetic library generation image #3045
Changes from 2 commits
8de1c01
15b2fc5
0a95554
034edf9
9795e23
ecfe8cd
bfac93f
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -19,7 +19,9 @@ SHELL [ "/bin/bash", "-c" ] | |
|
||
ARG OWLBOT_CLI_COMMITTISH=ac84fa5c423a0069bbce3d2d869c9730c8fdf550 | ||
ARG PROTOC_VERSION=25.3 | ||
ARG GRPC_VERSION=1.62.2 | ||
ENV HOME=/home | ||
ENV OS_ARCHITECTURE="linux-x86_64" | ||
|
||
# install OS tools | ||
RUN apt-get update && apt-get install -y \ | ||
|
@@ -32,11 +34,19 @@ COPY library_generation /src | |
# install protoc | ||
WORKDIR /protoc | ||
RUN source /src/utils/utilities.sh \ | ||
&& download_protoc "${PROTOC_VERSION}" "linux-x86_64" | ||
&& download_protoc "${PROTOC_VERSION}" "${OS_ARCHITECTURE}" | ||
# we indicate protoc is available in the container via env vars | ||
ENV DOCKER_PROTOC_LOCATION=/protoc | ||
ENV DOCKER_PROTOC_VERSION="${PROTOC_VERSION}" | ||
|
||
# install grpc | ||
WORKDIR /grpc | ||
RUN source /src/utils/utilities.sh \ | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
to
Then here we can just do
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is something I considered initially when working on the Dockerfile. The effects of There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Cool, that's good to know! |
||
&& download_grpc_plugin "${GRPC_VERSION}" "${OS_ARCHITECTURE}" | ||
# similar to protoc, we indicate grpc is available in the container via env vars | ||
ENV DOCKER_GRPC_LOCATION="/grpc/protoc-gen-grpc-java-${GRPC_VERSION}-${OS_ARCHITECTURE}.exe" | ||
ENV DOCKER_GRPC_VERSION="${GRPC_VERSION}" | ||
|
||
# use python 3.11 (the base image has several python versions; here we define the default one) | ||
RUN rm $(which python3) | ||
RUN ln -s $(which python3.11) /usr/local/bin/python | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -102,9 +102,18 @@ download_gapic_generator_pom_parent() { | |
download_generator_artifact "${gapic_generator_version}" "gapic-generator-java-pom-parent-${gapic_generator_version}.pom" "gapic-generator-java-pom-parent" | ||
} | ||
|
||
# This function returns the version of the grpc plugin to generate the libraries. If | ||
# DOCKER_GRPC_VERSION is set, this will be the version. Otherwise, it will be | ||
# computed from the gapic-generator-pom-parent artifact at the specified | ||
# gapic_generator_version. | ||
get_grpc_version() { | ||
local gapic_generator_version=$1 | ||
local grpc_version | ||
if [[ -n "${DOCKER_GRPC_VERSION}" ]]; then | ||
>&2 echo "Using grpc version baked into the container: ${DOCKER_GRPC_VERSION}" | ||
echo "${DOCKER_GRPC_VERSION}" | ||
return | ||
fi | ||
pushd "${output_folder}" > /dev/null | ||
# get grpc version from gapic-generator-java-pom-parent/pom.xml | ||
download_gapic_generator_pom_parent "${gapic_generator_version}" | ||
|
@@ -113,6 +122,10 @@ get_grpc_version() { | |
echo "${grpc_version}" | ||
} | ||
|
||
# This function returns the version of protoc to generate the libraries. If | ||
# DOCKER_PROTOC_VERSION is set, this will be the version. Otherwise, it will be | ||
# computed from the gapic-generator-pom-parent artifact at the specified | ||
# gapic_generator_version. | ||
get_protoc_version() { | ||
local gapic_generator_version=$1 | ||
local protoc_version | ||
|
@@ -129,6 +142,16 @@ get_protoc_version() { | |
echo "${protoc_version}" | ||
} | ||
|
||
# Given the versions of the gapic generator, protoc and the protoc-grpc plugin, | ||
# this function will download each one of the tools and create the environment | ||
# variables "protoc_path" and "grpc_path" which are expected upstream. Note that | ||
# if the specified versions of protoc and grpc match DOCKER_PROTOC_VERSION and | ||
# DOCKER_GRPC_VERSION respectively, this function will instead set "protoc_path" | ||
# and "grpc_path" to DOCKER_PROTOC_PATH and DOCKER_GRPC_PATH respectively (no | ||
# download), since the docker image will have downloaded these tools beforehand. | ||
# For the case of gapic-generator-java, no env var will be exported for the | ||
# upstream flow, but instead it will be assigned a default filename that will be | ||
# referenced by the file `library_generation/gapic-generator-java-wrapper`. | ||
download_tools() { | ||
local gapic_generator_version=$1 | ||
local protoc_version=$2 | ||
|
@@ -147,7 +170,15 @@ download_tools() { | |
export protoc_path=$(download_protoc "${protoc_version}" "${os_architecture}") | ||
fi | ||
|
||
download_grpc_plugin "${grpc_version}" "${os_architecture}" | ||
# similar case with grpc | ||
if [[ "${DOCKER_GRPC_VERSION}" == "${grpc_version}" ]]; then | ||
# if the specified protoc_version matches the one baked in the docker | ||
diegomarquezp marked this conversation as resolved.
Show resolved
Hide resolved
|
||
# container, we just point grpc_path to its location. | ||
export grpc_path="${DOCKER_GRPC_LOCATION}" | ||
else | ||
export grpc_path=$(download_grpc_plugin "${grpc_version}" "${os_architecture}") | ||
fi | ||
|
||
popd | ||
} | ||
|
||
|
@@ -198,13 +229,15 @@ download_protoc() { | |
download_grpc_plugin() { | ||
local grpc_version=$1 | ||
local os_architecture=$2 | ||
if [ ! -f "protoc-gen-grpc-java-${grpc_version}-${os_architecture}.exe" ]; then | ||
grpc_filename="protoc-gen-grpc-java-${grpc_version}-${os_architecture}.exe" | ||
if [ ! -f "${grpc_filename}" ]; then | ||
# download protoc-gen-grpc-java plugin from Google maven central mirror. | ||
download_from \ | ||
"https://maven-central.storage-download.googleapis.com/maven2/io/grpc/protoc-gen-grpc-java/${grpc_version}/protoc-gen-grpc-java-${grpc_version}-${os_architecture}.exe" \ | ||
"protoc-gen-grpc-java-${grpc_version}-${os_architecture}.exe" | ||
chmod +x "protoc-gen-grpc-java-${grpc_version}-${os_architecture}.exe" | ||
"https://maven-central.storage-download.googleapis.com/maven2/io/grpc/protoc-gen-grpc-java/${grpc_version}/${grpc_filename}" \ | ||
"${grpc_filename}" | ||
chmod +x "${grpc_filename}" | ||
fi | ||
echo "$(pwd)/${grpc_filename}" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is this a debug log? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. No, it's the return statement of the function. Many functions in |
||
} | ||
|
||
download_from() { | ||
|
@@ -282,7 +315,7 @@ get_proto_path_from_preprocessed_sources() { | |
pushd "${sources}" > /dev/null | ||
local proto_library=$(find . -maxdepth 1 -type d -name 'proto-*' | sed 's/\.\///') | ||
local found_libraries=$(echo "${proto_library}" | wc -l) | ||
if [ -z ${proto_library} ]; then | ||
if [[ -z ${proto_library} ]]; then | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I noticed that the shell unit tests were expecting the binary brackets because
|
||
echo "no proto libraries found in the supplied sources path" | ||
exit 1 | ||
elif [ ${found_libraries} -gt 1 ]; then | ||
|
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.
Can we configure this with renovate bot, and group it with other grpc update? e.g. There is an existing PR that updates gRPC to the latest.
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
sdk-platform-java/renovate.json
Lines 55 to 67 in 9795e23
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.
This version can be updated to 1.65.1 now since this PR is merged..
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