Skip to content

Fix incorrect subnet name from vpc GCP #5725

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

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

jimmycgz
Copy link

@jimmycgz jimmycgz commented May 6, 2025

This PR fixes an issue where subnet names were incorrectly using the network name instead of being properly configured separately.

Changes made:

  • Modified GceNetwork class to properly handle subnet names
  • Ensured network name and subnet names are managed separately
  • Fixed references to subnet resources to use the correct network name

This change ensures that when using GCP VPC networks with custom subnets, the subnet names are correctly configured and don't default to using the network name in all places.

Test command to reproduce and verify this fix

source ../../venv/bin/activate
cd $HOME/projects/PerfKitBenchmarker
export LOG_FOLDER=../pkb-logs
export MACHINE_TYPE=c4d-standard-4

python3 ./pkb.py \
  --benchmarks=nginx \
  --temp_dir=$LOG_FOLDER \
  --cloud=GCP \
  --project=your-gcp-proj1 \
  --zone=us-central1-a \
  --machine_type=$MACHINE_TYPE \
  --run_uri=standalone \
  --nginx_server_machine_type=$MACHINE_TYPE \
  --nginx_upstream_server_machine_type=$MACHINE_TYPE \
  --nginx_client_machine_type=$MACHINE_TYPE \
  --nginx_content_size=1024 \
  --nginx_load_configs="32:300:32:1000,512:300:512:1000" \
  --gce_network_name=your-custom-vpc \
  --gce_subnet_name=your-custom-subnet \
  --owner=owner-name \
  --accept_licenses

Copy link

google-cla bot commented May 6, 2025

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

@pmkc
Copy link
Contributor

pmkc commented May 7, 2025

There was a conscious decision to always pass subnet and never pass network.
We recommend only passing --gce_subnet_name and never passing --gce_network_name

In the case of a an auto network, the subnet and network names coincide. In the case of a custom network, the network name is insufficient.

I'm not sure the exact behavior of the code today, when you pass both network and subnet, but I would recommend only passing subnet. Is that the case that you think PKB handles incorrectly?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants