Skip to content
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

Bootstrap network cleanup #7367

Merged
merged 4 commits into from
Mar 7, 2023
Merged
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
5 changes: 2 additions & 3 deletions mmv1/products/alloydb/terraform.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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")'
Copy link
Member

Choose a reason for hiding this comment

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

stray thought: I wonder if we could derive a name from t rather than require a unique one be supplied by the author. Would guard against copy-paste issues.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's a great idea! Agreed that copy-paste is going to be an issue moving forward.

ignore_read_extra:
- "reconciling"
- "update_time"
examples:
- !ruby/object:Provider::Terraform::Examples
name: "alloydb_backup_full"
primary_resource_id: "default"
Expand All @@ -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"
Expand Down
2 changes: 1 addition & 1 deletion mmv1/products/redis/terraform.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
3 changes: 0 additions & 3 deletions mmv1/products/vertexai/terraform.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Copy link
Member

Choose a reason for hiding this comment

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

note for myself: we use skip_test so this is just noise

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yep, sorry, should have made a note on this one

kms_key_name: 'BootstrapKMSKeyInLocation(t, "us-central1").CryptoKey.Name'
network_name: 'BootstrapSharedTestNetwork(t, "vertex")'
properties:
etag: !ruby/object:Overrides::Terraform::PropertyOverride
ignore_read: true
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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),
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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),
}

Expand Down
23 changes: 16 additions & 7 deletions mmv1/third_party/terraform/utils/bootstrap_utils_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down