Add support to build NPI images via cloudbuild #50
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Summary of Changes
Hello @kislaykishore, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!
This pull request establishes a robust automated build process for GCSFuse performance testing images. By integrating Cloud Build, it streamlines the creation and distribution of multi-architecture Docker images required for various benchmark scenarios, ensuring consistent and efficient environment setup for performance evaluations.
Highlights
- Automated Multi-Platform Image Builds: This PR introduces a new cloudbuild.yaml configuration to automate the building and pushing of multi-platform (amd64 and arm64) Docker images for GCSFuse performance benchmarks using Google Cloud Build.
- GCSFuse Benchmark Image Generation: The Cloud Build pipeline is configured to build several specific benchmark images, including a base image, and FIO-based images for read, write, fullsweep, and Orbax emulated benchmarks.
- Enhanced Build Environment: The configuration utilizes docker buildx for multi-platform support and specifies an E2_HIGHCPU_8 machine type for the Cloud Build job to handle the resource-intensive multi-architecture builds efficiently.
Using Gemini Code Assist
The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.
Invoking Gemini
You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.
| Feature | Command | Description |
|---|---|---|
| Code Review | /gemini review |
Performs a code review for the current pull request in its current state. |
| Pull Request Summary | /gemini summary |
Provides a summary of the current pull request in its current state. |
| Comment | @gemini-code-assist | Responds in comments when explicitly tagged, both in issue comments and review comments. |
| Help | /gemini help |
Displays a list of available commands. |
Customization
To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.
Limitations & Feedback
Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.
You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.
Footnotes
-
Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩
There was a problem hiding this comment.
Code Review
This pull request introduces a cloudbuild.yaml file to automate the building of NPI images using Google Cloud Build. The configuration correctly sets up multi-platform builds with buildx and defines a logical sequence of build steps. The parallel execution for the benchmark images is efficient. I have two main points of feedback. Firstly, using the master branch for the _GCSFUSE_VERSION can affect build reproducibility. Secondly, there is significant duplication across the benchmark image build steps, which could pose a maintenance challenge. I've provided specific comments with suggestions to address these points.
| # limitations under the License. | ||
|
|
||
| substitutions: | ||
| _GCSFUSE_VERSION: 'master' |
There was a problem hiding this comment.
Using master for _GCSFUSE_VERSION can lead to non-reproducible or broken builds if the master branch of the gcsfuse repository contains breaking changes or is temporarily unstable. For more stable and predictable builds, it is best practice to pin dependencies to a specific version. Consider using a specific Git tag or commit SHA instead.
_GCSFUSE_VERSION: 'v2.1.0' # Or another specific tag/commit| # Build and push the read-benchmark image. | ||
| - name: 'gcr.io/cloud-builders/docker' | ||
| id: 'read-benchmark' | ||
| waitFor: ['base'] | ||
| args: | ||
| - 'buildx' | ||
| - 'build' | ||
| - '--push' | ||
| - '-f' | ||
| - 'fio/read.dockerfile' | ||
| - '--platform' | ||
| - 'linux/amd64,linux/arm64' | ||
| - '--build-arg' | ||
| - 'GCSFUSE_VERSION=${_GCSFUSE_VERSION}' | ||
| - '--build-arg' | ||
| - 'GO_VERSION=${_GO_VERSION}' | ||
| - '--build-arg' | ||
| - 'UBUNTU_VERSION=${_UBUNTU_VERSION}' | ||
| - '--build-arg' | ||
| - 'REGISTRY=${_REGISTRY}' | ||
| - '-t' | ||
| - '${_REGISTRY}/gcs-fuse-test/gcsfuse-benchmarks/fio-read-benchmark-${_GCSFUSE_VERSION}:latest' | ||
| - 'fio' | ||
|
|
||
| # Build and push the write-benchmark image. | ||
| - name: 'gcr.io/cloud-builders/docker' | ||
| id: 'write-benchmark' | ||
| waitFor: ['base'] | ||
| args: | ||
| - 'buildx' | ||
| - 'build' | ||
| - '--push' | ||
| - '-f' | ||
| - 'fio/write.dockerfile' | ||
| - '--platform' | ||
| - 'linux/amd64,linux/arm64' | ||
| - '--build-arg' | ||
| - 'GCSFUSE_VERSION=${_GCSFUSE_VERSION}' | ||
| - '--build-arg' | ||
| - 'GO_VERSION=${_GO_VERSION}' | ||
| - '--build-arg' | ||
| - 'UBUNTU_VERSION=${_UBUNTU_VERSION}' | ||
| - '--build-arg' | ||
| - 'REGISTRY=${_REGISTRY}' | ||
| - '-t' | ||
| - '${_REGISTRY}/gcs-fuse-test/gcsfuse-benchmarks/fio-write-benchmark-${_GCSFUSE_VERSION}:latest' | ||
| - 'fio' | ||
|
|
||
| # Build and push the fullsweep-benchmark image. | ||
| - name: 'gcr.io/cloud-builders/docker' | ||
| id: 'fullsweep-benchmark' | ||
| waitFor: ['base'] | ||
| args: | ||
| - 'buildx' | ||
| - 'build' | ||
| - '--push' | ||
| - '-f' | ||
| - 'fio/fullsweep.dockerfile' | ||
| - '--platform' | ||
| - 'linux/amd64,linux/arm64' | ||
| - '--build-arg' | ||
| - 'GCSFUSE_VERSION=${_GCSFUSE_VERSION}' | ||
| - '--build-arg' | ||
| - 'GO_VERSION=${_GO_VERSION}' | ||
| - '--build-arg' | ||
| - 'UBUNTU_VERSION=${_UBUNTU_VERSION}' | ||
| - '--build-arg' | ||
| - 'REGISTRY=${_REGISTRY}' | ||
| - '-t' | ||
| - '${_REGISTRY}/gcs-fuse-test/gcsfuse-benchmarks/fio-fullsweep-benchmark-${_GCSFUSE_VERSION}:latest' | ||
| - 'fio' | ||
|
|
||
| # Build and push the orbax-benchmark image. | ||
| - name: 'gcr.io/cloud-builders/docker' | ||
| id: 'orbax-benchmark' | ||
| waitFor: ['base'] | ||
| args: | ||
| - 'buildx' | ||
| - 'build' | ||
| - '--push' | ||
| - '-f' | ||
| - 'fio/orbax_emulated.dockerfile' | ||
| - '--platform' | ||
| - 'linux/amd64,linux/arm64' | ||
| - '--build-arg' | ||
| - 'GCSFUSE_VERSION=${_GCSFUSE_VERSION}' | ||
| - '--build-arg' | ||
| - 'GO_VERSION=${_GO_VERSION}' | ||
| - '--build-arg' | ||
| - 'UBUNTU_VERSION=${_UBUNTU_VERSION}' | ||
| - '--build-arg' | ||
| - 'REGISTRY=${_REGISTRY}' | ||
| - '-t' | ||
| - '${_REGISTRY}/gcs-fuse-test/gcsfuse-benchmarks/orbax-emulated-benchmark-${_GCSFUSE_VERSION}:latest' | ||
| - 'fio' |
There was a problem hiding this comment.
The build steps for read-benchmark, write-benchmark, fullsweep-benchmark, and orbax-benchmark are highly repetitive. This duplication violates the DRY (Don't Repeat Yourself) principle and can make the build configuration difficult to maintain. If you need to add a new build argument or change the supported platforms, you'll have to update it in four different places, which is error-prone.
To improve maintainability, I recommend refactoring to reduce this duplication. A good approach would be to use a single build step that executes a shell script. The script could then loop through the different benchmark configurations and run the docker buildx build command for each, centralizing the build logic and arguments.
No description provided.