From f60dbb99f21626df6f30adf7b626871d9fa7328a Mon Sep 17 00:00:00 2001 From: Ryan Oaks Date: Tue, 7 Mar 2023 14:57:18 -0500 Subject: [PATCH] Bootstrap network cleanup (#7367) * Update bootstrapped networks for alloydb * Update bootstrapped networks for redis * Update bootstrapped networks for vertexai * Update comment for BootstrapSharedTestNetwork to be more clear --- mmv1/products/alloydb/terraform.yaml | 5 ++-- mmv1/products/redis/terraform.yaml | 2 +- mmv1/products/vertexai/terraform.yaml | 3 --- .../tests/resource_alloydb_backup_test.go | 2 +- .../tests/resource_vertex_ai_endpoint_test.go | 2 +- .../terraform/utils/bootstrap_utils_test.go | 23 +++++++++++++------ 6 files changed, 21 insertions(+), 16 deletions(-) diff --git a/mmv1/products/alloydb/terraform.yaml b/mmv1/products/alloydb/terraform.yaml index c18e4ce05d5a..ca6a52915bd9 100644 --- a/mmv1/products/alloydb/terraform.yaml +++ b/mmv1/products/alloydb/terraform.yaml @@ -76,11 +76,10 @@ overrides: !ruby/object:Overrides::ResourceOverrides alloydb_instance_name: "alloydb-instance" network_name: "alloydb-network" test_vars_overrides: - network_name: 'BootstrapSharedTestNetwork(t, "alloydb")' + network_name: 'BootstrapSharedTestNetwork(t, "alloydb-basic")' ignore_read_extra: - "reconciling" - "update_time" - examples: - !ruby/object:Provider::Terraform::Examples name: "alloydb_backup_full" primary_resource_id: "default" @@ -90,7 +89,7 @@ overrides: !ruby/object:Overrides::ResourceOverrides alloydb_instance_name: "alloydb-instance" network_name: "alloydb-network" test_vars_overrides: - network_name: 'BootstrapSharedTestNetwork(t, "alloydb")' + network_name: 'BootstrapSharedTestNetwork(t, "alloydb-full")' ignore_read_extra: - "reconciling" - "update_time" diff --git a/mmv1/products/redis/terraform.yaml b/mmv1/products/redis/terraform.yaml index 2f9b3d9c72e5..30e2c126fb74 100644 --- a/mmv1/products/redis/terraform.yaml +++ b/mmv1/products/redis/terraform.yaml @@ -46,7 +46,7 @@ overrides: !ruby/object:Overrides::ResourceOverrides instance_name: "ha-memory-cache-persis" network_name: "redis-test-network" test_vars_overrides: - network_name: 'BootstrapSharedTestNetwork(t, "redis-mrr")' + network_name: 'BootstrapSharedTestNetwork(t, "redis-full-persis")' - !ruby/object:Provider::Terraform::Examples name: "redis_instance_private_service" # Temporary for servicenetworking problems diff --git a/mmv1/products/vertexai/terraform.yaml b/mmv1/products/vertexai/terraform.yaml index 8e12e34b5a86..46103b2167ba 100644 --- a/mmv1/products/vertexai/terraform.yaml +++ b/mmv1/products/vertexai/terraform.yaml @@ -69,9 +69,6 @@ overrides: !ruby/object:Overrides::ResourceOverrides address_name: "address-name" kms_key_name: "kms-name" network_name: "network-name" - test_vars_overrides: - kms_key_name: 'BootstrapKMSKeyInLocation(t, "us-central1").CryptoKey.Name' - network_name: 'BootstrapSharedTestNetwork(t, "vertex")' properties: etag: !ruby/object:Overrides::Terraform::PropertyOverride ignore_read: true diff --git a/mmv1/third_party/terraform/tests/resource_alloydb_backup_test.go b/mmv1/third_party/terraform/tests/resource_alloydb_backup_test.go index be5b7ba3729f..3d175d703bb6 100644 --- a/mmv1/third_party/terraform/tests/resource_alloydb_backup_test.go +++ b/mmv1/third_party/terraform/tests/resource_alloydb_backup_test.go @@ -10,7 +10,7 @@ func TestAccAlloydbBackup_update(t *testing.T) { t.Parallel() context := map[string]interface{}{ - "network_name": BootstrapSharedTestNetwork(t, "alloydb"), + "network_name": BootstrapSharedTestNetwork(t, "alloydb-update"), "random_suffix": RandString(t, 10), } diff --git a/mmv1/third_party/terraform/tests/resource_vertex_ai_endpoint_test.go b/mmv1/third_party/terraform/tests/resource_vertex_ai_endpoint_test.go index 26e8af2fc4f5..985626203941 100644 --- a/mmv1/third_party/terraform/tests/resource_vertex_ai_endpoint_test.go +++ b/mmv1/third_party/terraform/tests/resource_vertex_ai_endpoint_test.go @@ -29,7 +29,7 @@ func TestAccVertexAIEndpoint_vertexAiEndpointNetwork(t *testing.T) { context := map[string]interface{}{ "endpoint_name": fmt.Sprint(RandInt(t) % 9999999999), "kms_key_name": BootstrapKMSKeyInLocation(t, "us-central1").CryptoKey.Name, - "network_name": BootstrapSharedTestNetwork(t, "vertex"), + "network_name": BootstrapSharedTestNetwork(t, "vertex-ai-endpoint-update"), "random_suffix": RandString(t, 10), } diff --git a/mmv1/third_party/terraform/utils/bootstrap_utils_test.go b/mmv1/third_party/terraform/utils/bootstrap_utils_test.go index bcb464bb4d36..88059b7ba679 100644 --- a/mmv1/third_party/terraform/utils/bootstrap_utils_test.go +++ b/mmv1/third_party/terraform/utils/bootstrap_utils_test.go @@ -255,14 +255,23 @@ func BootstrapSharedTestADDomain(t *testing.T, testId string, networkName string const SharedTestNetworkPrefix = "tf-bootstrap-net-" -// BootstrapSharedTestNetwork will return a shared compute network -// for a test or set of tests. Often resources create complementing -// tenant network resources, which we don't control and which don't get cleaned -// up after our owned resource is deleted in test. These tenant resources -// have quotas, so creating a shared test network prevents hitting these limits. +// BootstrapSharedTestNetwork will return a persistent compute network for a +// test or set of tests. // -// testId specifies the test/suite for which a shared network is used/initialized. -// Returns the name of an network, creating it if hasn't been created in the test projcet. +// Resources like service_networking_connection use a consumer network and +// create a complementing tenant network which we don't control. These tenant +// networks never get cleaned up and they can accumulate to the point where a +// limit is reached for the organization. By reusing a consumer network across +// test runs, we can reduce the number of tenant networks that are needed. +// See b/146351146 for more context. +// +// testId specifies the test for which a shared network is used/initialized. +// Note that if the network is being used for a service_networking_connection, +// the same testId should generally not be used across tests, to avoid race +// conditions where multiple tests attempt to modify the connection at once. +// +// Returns the name of a network, creating it if it hasn't been created in the +// test project. func BootstrapSharedTestNetwork(t *testing.T, testId string) string { project := GetTestProjectFromEnv() networkName := SharedTestNetworkPrefix + testId