chore: Migrate from gsutil to gcloud storage#2183
Conversation
|
Skipping CI for Draft Pull Request. |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
tiffanny29631
changed the title
|
/test all |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request migrates the codebase from using gsutil to gcloud storage for Google Cloud Storage operations across build environments, CI/CD configurations, documentation, and helper scripts. The review feedback highlights efficiency issues in scripts/update-component-image.sh where the migration to gcloud storage ls lacks prefix filtering and directory-only listing, which could lead to listing all objects recursively and cause performance degradation. Suggestions are provided to restore the prefix filter and use the -d flag.
| if [ "${QUERY_TYPE}" == "gcloud-storage" ]; then | ||
| FILTER="" | ||
| GREP_PATTERN="^${BASE_VERSION%-gke.*}" | ||
| fi |
There was a problem hiding this comment.
Setting FILTER to empty for gcloud-storage queries causes gcloud storage ls to list all objects/directories under GCS_PATH without any prefix filtering. This is highly inefficient if there are many tags. Instead, we should retain the prefix filter and use gcloud storage ls -d with the filter to query only matching directories, similar to how gsutil ls -d was used.
| if [ "${QUERY_TYPE}" == "gcloud-storage" ]; then | |
| FILTER="" | |
| GREP_PATTERN="^${BASE_VERSION%-gke.*}" | |
| fi | |
| if [ "${QUERY_TYPE}" == "gcloud-storage" ]; then | |
| FILTER="${BASE_VERSION%-gke.*}*" | |
| GREP_PATTERN="^${BASE_VERSION%-gke.*}" | |
| fi |
| # Query GCS and strip path/trailing slash | ||
| LATEST_TAG=$(gsutil ls -d "${GCS_PATH}${FILTER}/" | sed "s|${GCS_PATH}||; s|/||" | grep "${GREP_PATTERN}" | sort -V | tail -n 1) | ||
| LATEST_TAG=$(gcloud storage ls "${GCS_PATH}" | sed "s|${GCS_PATH}||; s|/||" | grep "${GREP_PATTERN}" | sort -V | tail -n 1) | ||
| fi |
There was a problem hiding this comment.
Use gcloud storage ls -d (or --directories) with the ${FILTER} suffix to avoid listing the entire directory recursively or listing all tags. This matches the behavior of the original gsutil ls -d command and is much more efficient.
| # Query GCS and strip path/trailing slash | |
| LATEST_TAG=$(gsutil ls -d "${GCS_PATH}${FILTER}/" | sed "s|${GCS_PATH}||; s|/||" | grep "${GREP_PATTERN}" | sort -V | tail -n 1) | |
| LATEST_TAG=$(gcloud storage ls "${GCS_PATH}" | sed "s|${GCS_PATH}||; s|/||" | grep "${GREP_PATTERN}" | sort -V | tail -n 1) | |
| fi | |
| # Query GCS and strip path/trailing slash | |
| LATEST_TAG=$(gcloud storage ls -d "${GCS_PATH}${FILTER}" | sed "s|${GCS_PATH}||; s|/||" | grep "${GREP_PATTERN}" | sort -V | tail -n 1) |
|
Replaces #2118 |
1 participant
No description provided.