Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 31 additions & 0 deletions storage/cloudbuild/run_zonal_tests.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@

set -euxo pipefail
Comment on lines +1 to +2
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

The shell script is missing a shebang line. It is recommended to start with #!/bin/bash to ensure it is executed with the correct interpreter.

Suggested change
set -euxo pipefail
#!/bin/bash
set -euxo pipefail

echo '--- Installing git and cloning repository on VM ---'
sudo apt-get update && sudo apt-get install -y git python3-pip python3-venv

# Clone the repository and checkout the specific commit from the build trigger.
git clone --no-checkout --depth 1 --sparse --filter=blob:none https://github.com/googleapis/python-docs-samples.git
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
Comment on lines +8 to +12
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



echo '--- Installing Python and dependencies on VM ---'
python3 -m venv env
source env/bin/activate

echo 'Install testing libraries explicitly, as they are not in setup.py'
pip install --upgrade pip
pip install pytest pytest-timeout pytest-subtests pytest-asyncio
pip install google-cloud-testutils google-cloud-kms
pip install -e .

echo '--- Setting up environment variables on VM ---'
export ZONAL_BUCKET=${_ZONAL_BUCKET}
export RUN_ZONAL_SYSTEM_TESTS=True
export GCE_METADATA_MTLS_MODE=None
CURRENT_ULIMIT=$(ulimit -n)
echo '--- Running Zonal tests on VM with ulimit set to ---' $CURRENT_ULIMIT
pytest -vv -s --log-format='%(asctime)s %(levelname)s %(message)s' --log-date-format='%H:%M:%S' samples/snippets/zonal_buckets/zonal_snippets_test.py
137 changes: 137 additions & 0 deletions storage/cloudbuild/zb-system-tests-cloudbuild.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,137 @@
substitutions:
_REGION: "us-central1"
_ZONE: "us-central1-a"
_SHORT_BUILD_ID: ${BUILD_ID:0:8}
_VM_NAME: "py-sdk-sys-test-${_SHORT_BUILD_ID}"
_ULIMIT: "10000" # 10k, for gRPC bidi streams
Comment on lines +1 to +6
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

Several variables used in the build steps (such as _ZONAL_VM_SERVICE_ACCOUNT, _ZONAL_BUCKET, and _PR_NUMBER) are missing from the substitutions block. Adding them with default values improves readability and ensures the template is self-documenting.

substitutions:
  _REGION: "us-central1"
  _ZONE: "us-central1-a"
  _SHORT_BUILD_ID: ${BUILD_ID:0:8}
  _VM_NAME: "py-sdk-sys-test-${_SHORT_BUILD_ID}"
  _ULIMIT: "10000" # 10k, for gRPC bidi streams
  _ZONAL_VM_SERVICE_ACCOUNT: ""
  _ZONAL_BUCKET: ""
  _CROSS_REGION_BUCKET: ""
  _PR_NUMBER: ""




steps:
# Step 0: Generate a persistent SSH key for this build run.
# This prevents gcloud from adding a new key to the OS Login profile on every ssh/scp command.
- name: "gcr.io/google.com/cloudsdktool/cloud-sdk"
id: "generate-ssh-key"
entrypoint: "bash"
args:
- "-c"
- |
mkdir -p /workspace/.ssh
# Generate the SSH key
ssh-keygen -t rsa -f /workspace/.ssh/google_compute_engine -N '' -C gcb
# Save the public key content to a file for the cleanup step
cat /workspace/.ssh/google_compute_engine.pub > /workspace/gcb_ssh_key.pub
waitFor: ["-"]

- name: "gcr.io/google.com/cloudsdktool/cloud-sdk"
id: "cleanup-old-keys"
entrypoint: "bash"
args:
- "-c"
- |
#!/bin/bash
set -e

echo "Fetching OS Login SSH keys..."
echo "Removing all keys."
echo "---------------------------------------------------------------------"

FINGERPRINTS_TO_DELETE=$$(gcloud compute os-login ssh-keys list \
--format="value(fingerprint)")

echo "Keys to delete: $$FINGERPRINTS_TO_DELETE"

if [ -z "$$FINGERPRINTS_TO_DELETE" ]; then
echo "No keys found to delete. Nothing to do."
exit 0
fi

while IFS= read -r FINGERPRINT; do
if [ -n "$$FINGERPRINT" ]; then
echo "Deleting key with fingerprint: $$FINGERPRINT"
gcloud compute os-login ssh-keys remove \
--key="$$FINGERPRINT" \
--quiet || true
fi
done <<< "$$FINGERPRINTS_TO_DELETE"

echo "---------------------------------------------------------------------"
echo "Cleanup complete."
Comment on lines +27 to +59
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 cleanup-old-keys step is extremely aggressive as it deletes all OS Login SSH keys associated with the service account. This can cause intermittent failures for concurrent builds using the same service account in the same project. It is safer to rely on the specific cleanup-ssh-key step at the end of the build.


# Step 1 Create a GCE VM to run the tests.
# The VM is created in the same zone as the buckets to test rapid storage features.
# It's given the 'cloud-platform' scope to allow it to access GCS and other services.
- name: "gcr.io/google.com/cloudsdktool/cloud-sdk"
id: "create-vm"
entrypoint: "gcloud"
args:
- "compute"
- "instances"
- "create"
- "${_VM_NAME}"
- "--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"

- "--image-project=debian-cloud"
- "--service-account=${_ZONAL_VM_SERVICE_ACCOUNT}"
- "--scopes=https://www.googleapis.com/auth/devstorage.full_control,https://www.googleapis.com/auth/devstorage.read_only,https://www.googleapis.com/auth/devstorage.read_write"
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

The scopes provided are redundant. https://www.googleapis.com/auth/devstorage.full_control already includes read and write permissions. Using https://www.googleapis.com/auth/cloud-platform is the recommended best practice when using IAM for access control.

      - "--scopes=https://www.googleapis.com/auth/cloud-platform"

- "--metadata=enable-oslogin=TRUE"
waitFor: ["-"]

# Step 2: Run the integration tests inside the newly created VM and cleanup.
# This step uses 'gcloud compute ssh' to execute a remote script.
# The VM is deleted after tests are run, regardless of success.
- name: "gcr.io/google.com/cloudsdktool/cloud-sdk"
id: "run-tests-and-delete-vm"
entrypoint: "bash"
args:
- "-c"
- |
set -e
# Wait for the VM to be fully initialized and SSH to be ready.
for i in {1..10}; do
if gcloud compute ssh ${_VM_NAME} --zone=${_ZONE} --internal-ip --ssh-key-file=/workspace/.ssh/google_compute_engine --command="echo VM is ready"; then
break
fi
echo "Waiting for VM to become available... (attempt $i/10)"
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


# Execute the script on the VM via SSH.
# Capture the exit code to ensure cleanup happens before the build fails.
set +e
gcloud compute ssh ${_VM_NAME} --zone=${_ZONE} --internal-ip --ssh-key-file=/workspace/.ssh/google_compute_engine --command="ulimit -n ${_ULIMIT}; COMMIT_SHA=${COMMIT_SHA} _ZONAL_BUCKET=${_ZONAL_BUCKET} CROSS_REGION_BUCKET=${_CROSS_REGION_BUCKET} _PR_NUMBER=${_PR_NUMBER} bash run_zonal_tests.sh"
EXIT_CODE=$?
set -e

echo "--- Deleting GCE VM ---"
gcloud compute instances delete "${_VM_NAME}" --zone=${_ZONE} --quiet

# Exit with the original exit code from the test script.
exit $$EXIT_CODE
waitFor:
- "create-vm"
- "generate-ssh-key"
- "cleanup-old-keys"

- name: "gcr.io/google.com/cloudsdktool/cloud-sdk"
id: "cleanup-ssh-key"
entrypoint: "bash"
args:
- "-c"
- |
echo "--- Removing SSH key from OS Login profile to prevent accumulation ---"
gcloud compute os-login ssh-keys remove \
--key-file=/workspace/gcb_ssh_key.pub || true
waitFor:
- "run-tests-and-delete-vm"

timeout: "3600s" # 60 minutes

options:
logging: CLOUD_LOGGING_ONLY
pool:
name: "projects/${PROJECT_ID}/locations/us-central1/workerPools/cloud-build-worker-pool"
Loading