Skip to content

fix: adds config file and shell script to run storage cloudbuild tests#14022

Merged
chalmerlowe merged 9 commits into
mainfrom
add-zonal-test-script-and-config
May 14, 2026
Merged

fix: adds config file and shell script to run storage cloudbuild tests#14022
chalmerlowe merged 9 commits into
mainfrom
add-zonal-test-script-and-config

Conversation

@chalmerlowe
Copy link
Copy Markdown
Contributor

Description

Partially Fixes #googleapis/google-cloud-python#16487

This PR is offered as starting point to enable cloudbuild tests for the Storage Team.
This mirrors a config and script that was in use in the split repo prior to migrating python-storage into the google-cloud-python monorepo.

I am not versed in all the nuances of how this needs to be configured. @chandra-siri and @chingor13 were instrumental in getting similar tests running in google-cloud-python.

I think these task apply, but as I said: not the expert.

  • These samples need a new API enabled in testing projects to pass (let us know which ones)
  • These samples need a new/updated env vars in testing projects set to pass (let us know which ones)
  • Please merge this PR for me once it is approved

@chalmerlowe chalmerlowe requested review from a team as code owners April 9, 2026 17:30
@product-auto-label product-auto-label Bot added samples Issues that are directly related to samples. api: storage Issues related to the Cloud Storage API. labels Apr 9, 2026
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request introduces a new CI/CD pipeline for zonal system tests, including a Cloud Build configuration and a helper shell script to run tests on a GCE VM. Feedback focuses on correcting the repository URL and directory paths to align with the current monorepo structure, as well as addressing potential issues with aggressive SSH key cleanup that could interfere with concurrent builds. Other recommendations include adding missing substitutions for better documentation, using a stable Debian image, simplifying service account scopes, and adding a shebang to the shell script.

Comment thread storage/cloudbuild/run_zonal_tests.sh Outdated
Comment thread storage/cloudbuild/run_zonal_tests.sh Outdated
Comment on lines +8 to +12
cd python-docs-samples
git sparse-checkout set main/storage
git fetch origin "refs/pull/${_PR_NUMBER}/head"
git checkout ${COMMIT_SHA}
cd main/storage
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

The directory paths include a main/ prefix (e.g., git sparse-checkout set main/storage, cd main/storage) which does not match the standard repository structure. In the monorepo, the storage component is located at the root level storage/.

Suggested change
cd python-docs-samples
git sparse-checkout set main/storage
git fetch origin "refs/pull/${_PR_NUMBER}/head"
git checkout ${COMMIT_SHA}
cd main/storage
cd google-cloud-python
git sparse-checkout set storage
git fetch origin "refs/pull/${_PR_NUMBER}/head"
git checkout ${COMMIT_SHA}
cd storage

Comment thread storage/cloudbuild/zb-system-tests-cloudbuild.yaml
Comment thread storage/cloudbuild/zb-system-tests-cloudbuild.yaml
sleep 15
done
# copy the script to the VM
gcloud compute scp main/storage/cloudbuild/run_zonal_tests.sh ${_VM_NAME}:~ --zone=${_ZONE} --internal-ip --ssh-key-file=/workspace/.ssh/google_compute_engine
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

The scp path includes a main/ prefix which likely does not exist in the Cloud Build workspace. The path should be relative to the repository root.

        gcloud compute scp storage/cloudbuild/run_zonal_tests.sh ${_VM_NAME}:~ --zone=${_ZONE} --internal-ip --ssh-key-file=/workspace/.ssh/google_compute_engine

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/gemini

fix it!

Comment thread storage/cloudbuild/run_zonal_tests.sh
- "--project=${PROJECT_ID}"
- "--zone=${_ZONE}"
- "--machine-type=e2-medium"
- "--image-family=debian-13"
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Debian 13 is currently the 'testing' distribution. It is generally safer to use the current stable version, debian-12, for CI/CD environments to ensure stability and availability of packages.

      - "--image-family=debian-12"

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@gemini-code-assist

This is not accurate. The current stable release for Debian is Trixie which is Debian 13.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I apologize for the confusion. To clarify, Debian 12 (Bookworm) is the current stable release. Debian 13 (Trixie) is currently in the testing phase and is slated to become the next stable release. For CI/CD environments, it is standard practice to use the current stable version to ensure maximum stability and package availability.

Comment thread storage/cloudbuild/zb-system-tests-cloudbuild.yaml
@chalmerlowe
Copy link
Copy Markdown
Contributor Author

@chandra-siri @chingor13

Can you folks work some of your magic to help get this PR across the line.

@chandra-siri chandra-siri added do not merge Indicates a pull request not ready for merge, due to either quality or timing. and removed do not merge Indicates a pull request not ready for merge, due to either quality or timing. labels Apr 21, 2026
@chandra-siri
Copy link
Copy Markdown
Contributor

/gcbrun(4c6acdc)

1 similar comment
@chandra-siri
Copy link
Copy Markdown
Contributor

/gcbrun(4c6acdc)

Copy link
Copy Markdown
Contributor Author

@chalmerlowe chalmerlowe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@chandra-siri

Left some comments.

@chandra-siri
Copy link
Copy Markdown
Contributor

@chandra-siri

Left some comments.

Addressed all of them. @chalmerlowe

@chalmerlowe chalmerlowe merged commit b3a4fc4 into main May 14, 2026
17 checks passed
@chalmerlowe chalmerlowe deleted the add-zonal-test-script-and-config branch May 14, 2026 20:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

api: storage Issues related to the Cloud Storage API. samples Issues that are directly related to samples.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants