-
Notifications
You must be signed in to change notification settings - Fork 55
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 synthtool and owl-bot into library_generation docker image #2615
Changes from 35 commits
0b65f15
a6bb064
d4f27e2
19a0b04
bc693b5
f335cbb
aab50ef
f97cdca
d68c14a
eca0d92
6aa8533
53c5dea
cc4309b
7a4b6b8
4838d5b
1b01713
c149461
4ee1692
e77bacc
9120c94
3d18426
427c564
7b79763
240742a
8498f2c
f893e5c
2faf45d
7ce9548
8474452
b58e990
00cb2a7
b27a57e
6c6432a
eca8c62
0041c6b
7ec0f4c
4399bfe
ef105e7
b1d3fc3
1df1aa1
bf95938
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 |
---|---|---|
|
@@ -15,26 +15,68 @@ | |
# build from the root of this repo: | ||
FROM gcr.io/cloud-devrel-public-resources/python | ||
|
||
# install tools | ||
ARG SYNTHTOOL_COMMITTISH=63cc541da2c45fcfca2136c43e638da1fbae174d | ||
ARG OWLBOT_CLI_COMMITTISH=ac84fa5c423a0069bbce3d2d869c9730c8fdf550 | ||
ENV HOME=/home | ||
|
||
# install OS tools | ||
RUN apt-get update && apt-get install -y \ | ||
unzip openjdk-17-jdk rsync maven jq \ | ||
unzip openjdk-17-jdk rsync maven jq less vim \ | ||
&& apt-get clean | ||
|
||
COPY library_generation /src | ||
|
||
# use python 3.11 (the base image has several python versions; here we define the default one) | ||
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. Because the base image 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.
We could use a higher version too, or even a lower one if it's still compatible with the dependencies
Judging from a documentation entry in the cloud client libraries g3doc, I think it's the Cloud Client Libraries team |
||
RUN rm $(which python3) | ||
RUN ln -s $(which python3.11) /usr/local/bin/python | ||
RUN ln -s $(which python3.11) /usr/local/bin/python3 | ||
RUN python -m pip install --upgrade pip | ||
RUN cd /src && python -m pip install -r requirements.txt | ||
RUN cd /src && python -m pip install . | ||
|
||
# set dummy git credentials for empty commit used in postprocessing | ||
RUN git config --global user.email "[email protected]" | ||
RUN git config --global user.name "Cloud Java Bot" | ||
# copy source code | ||
COPY library_generation /src | ||
|
||
WORKDIR /workspace | ||
RUN chmod 750 /workspace | ||
RUN chmod 750 /src/generate_repo.py | ||
# install scripts as a python package | ||
WORKDIR /src | ||
RUN python -m pip install -r requirements.txt | ||
RUN python -m pip install . | ||
|
||
# install synthtool | ||
WORKDIR /tools | ||
RUN git clone https://github.com/googleapis/synthtool | ||
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 think the only reason we need synthtool is java.py and its dependencies? 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. Yes, mainly the |
||
WORKDIR /tools/synthtool | ||
RUN git checkout "${SYNTHTOOL_COMMITTISH}" | ||
RUN python3 -m pip install --no-deps -e . | ||
RUN python3 -m pip install -r requirements.in | ||
|
||
CMD [ "/src/generate_repo.py" ] | ||
# Install nvm with node and npm | ||
ENV NODE_VERSION 20.12.0 | ||
WORKDIR /home | ||
RUN curl -o- https://raw.githubusercontent.com/nvm-sh/nvm/v0.35.3/install.sh | bash | ||
RUN chmod o+rx /home/.nvm | ||
ENV NODE_PATH=/home/.nvm/versions/node/v${NODE_VERSION}/bin | ||
ENV PATH=${PATH}:${NODE_PATH} | ||
RUN node --version | ||
RUN npm --version | ||
|
||
# install the owl-bot CLI | ||
WORKDIR /tools | ||
RUN git clone https://github.com/googleapis/repo-automation-bots | ||
WORKDIR /tools/repo-automation-bots/packages/owl-bot | ||
RUN git checkout "${OWLBOT_CLI_COMMITTISH}" | ||
RUN npm i && npm run compile && npm link | ||
RUN owl-bot copy-code --version | ||
RUN chmod -R o+rx ${NODE_PATH} | ||
RUN ln -sf ${NODE_PATH}/* /usr/local/bin | ||
|
||
# allow users to access the script folders | ||
RUN chmod -R o+rx /src | ||
|
||
# set dummy git credentials for the empty commit used in postprocessing | ||
# we use system so all users using the container will use this configuration | ||
RUN git config --system user.email "[email protected]" | ||
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 think the only reason we need 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. yes, but the root cause is that the owl-bot CLI assumes the source folder (the one we generate and arrange) is a git repository. Extra points to process owlbot yamls using a custom script. |
||
RUN git config --system user.name "Cloud Java Bot" | ||
|
||
# allow read-write for /home and execution for binaries in /home/.nvm | ||
RUN chmod -R a+rw /home | ||
RUN chmod -R a+rx /home/.nvm | ||
|
||
WORKDIR /workspace | ||
ENTRYPOINT [ "python", "/src/cli/entry_point.py", "generate" ] | ||
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. Can the user run the image with different scripts/methods? 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. yes, I declared this |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,101 @@ | ||
# Linting | ||
|
||
When contributing, ensure your changes to python code have a valid | ||
format. | ||
|
||
``` | ||
python -m pip install black | ||
black library_generation | ||
``` | ||
|
||
# Running the integration tests | ||
diegomarquezp marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
The integration tests build the docker image declared in | ||
`.cloudbuild/library_generation/library_generation.Dockerfile`, pull GAPIC | ||
repositories, generate the libraries and compares the results with the source | ||
code declared in a "golden branch" of the repo. | ||
|
||
It requires docker and python 3.x to be installed. | ||
|
||
``` | ||
cd library_generation | ||
diegomarquezp marked this conversation as resolved.
Show resolved
Hide resolved
|
||
python -m pip install . | ||
python -m pip install -r requirements.txt | ||
diegomarquezp marked this conversation as resolved.
Show resolved
Hide resolved
|
||
python -m unittest test/integration_tests.py | ||
``` | ||
|
||
# Running the scripts in your local environment | ||
diegomarquezp marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
Although the scripts are designed to be run in a Docker container, you can also | ||
run them directly. This section explains how to run the entrypoint script | ||
(`library_generation/cli/entry_point.py`). | ||
|
||
## Installing prerequisites | ||
|
||
In order to run the generation scripts directly, there are a few tools we | ||
need to install beforehand. | ||
|
||
### Install synthtool | ||
|
||
It requires python 3.x to be installed. | ||
You will need to specify a committish of the synthtool repo in order to have | ||
your generation results matching exactly what the docker image would produce. | ||
You can achieve this by inspecting `SYNTHTOOL_COMMITISH` in | ||
`.cloudbuild/library_generation/library_generation.Dockerfile`. | ||
|
||
```bash | ||
# obtained from .cloudbuild/library_generation/library_generation.Dockerfile | ||
export SYNTHTOOL_COMMITTISH=6612ab8f3afcd5e292aecd647f0fa68812c9f5b5 | ||
``` | ||
|
||
```bash | ||
git clone https://github.com/googleapis/synthtool | ||
cd synthtool | ||
git checkout "${SYNTHTOOL_COMMITTISH}" | ||
python -m pip install --require-hashes -r requirements.txt | ||
python -m pip install --no-deps -e . | ||
python -m synthtool --version | ||
``` | ||
|
||
### Install the owl-bot CLI | ||
|
||
Requires node.js to be installed. | ||
Check this [installation guide](https://github.com/nvm-sh/nvm?tab=readme-ov-file#install--update-script) | ||
for NVM, Node.js's version manager. | ||
|
||
After you install it, you can install the owl-bot CLI with the following | ||
commands: | ||
```bash | ||
git clone https://github.com/googleapis/repo-automation-bots | ||
cd repo-automation-bots/packages/owl-bot | ||
npm i && npm run compile && npm link | ||
owl-bot copy-code --version | ||
``` | ||
|
||
The key step is `npm link`, which will make the command available in you current | ||
shell session. | ||
|
||
## Running the script | ||
The entrypoint script (`library_generation/cli/entry_point.py`) allows you to | ||
update the target repository with the latest changes starting from the | ||
googleapis committish declared in `generation_config.yaml`. | ||
|
||
### Download the repo | ||
For example, google-cloud-java | ||
``` | ||
git clone https://github.com/googleapis/google-cloud-java | ||
export path_to_repo="$(pwd)/google-cloud-java" | ||
``` | ||
|
||
### Install the scripts | ||
``` | ||
cd library_generation | ||
python -m pip install . | ||
``` | ||
|
||
### Run the script | ||
``` | ||
cd library_generation | ||
python cli/entry_point.py --repository-path "${path_to_repo}" | ||
diegomarquezp marked this conversation as resolved.
Show resolved
Hide resolved
|
||
``` | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -70,7 +70,12 @@ echo "Fixing missing license headers..." | |
python3 "${scripts_root}/owlbot/src/fix-license-headers.py" | ||
echo "...done" | ||
|
||
# ensure formatting on all .java files in the repository | ||
# Ensure formatting on all .java files in the repository. | ||
# Here we manually set the user.home system variable. Unfortunately, Maven | ||
# user.home inference involves the /etc/passwd file (confirmed empirically), | ||
# instead of the presumable $HOME env var, which may not work properly | ||
# when `docker run`ning with the -u flag because we may incur in users | ||
# not registered in the container's /etc/passwd file | ||
echo "Reformatting source..." | ||
mvn fmt:format -V --batch-mode --no-transfer-progress | ||
mvn fmt:format -Duser.home="${HOME}" -V --batch-mode --no-transfer-progress | ||
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. So the permission issue (the generated files can't be removed by 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. Docker does indeed take ownership of the volumes (i.e. host machine folders) you pass, which makes the The |
||
echo "...done" |
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.
I don't see
less
andvim
being used, I guess the reason we need them is for testing purpose?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.
It's just for inspecting the container files in case things go wrong during local development. Do we want to only keep the production dependencies?
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.
For local development, I think you can mention them in the development.md. No need to install it in production.
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.
Makes sense, I removed these tools from the image and added a small section in the development guide.