Add additional check to dangling cluster test

Also remove unused code which was needed for failed creation test
GoogleCloudPlatform · Oct 23, 2023 · bd241c2 · bd241c2
1 parent 4499577
commit bd241c2
Show file tree

Hide file tree

Showing 2 changed files with 45 additions and 69 deletions.
diff --git a/mmv1/third_party/terraform/acctest/bootstrap_test_utils.go b/mmv1/third_party/terraform/acctest/bootstrap_test_utils.go
@@ -581,65 +581,6 @@ func BootstrapServicePerimeterProjects(t *testing.T, desiredProjects int) []*clo
 	return projects
 }
 
-func RemoveContainerServiceAgentRoleFromContainerEngineRobot(t *testing.T, project *cloudresourcemanager.Project) {
-	config := BootstrapConfig(t)
-	if config == nil {
-		return
-	}
-
-	client := config.NewResourceManagerClient(config.UserAgent)
-	containerEngineRobot := fmt.Sprintf("serviceAccount:service-%[email protected]", project.ProjectNumber)
-	getPolicyRequest := &cloudresourcemanager.GetIamPolicyRequest{}
-	policy, err := client.Projects.GetIamPolicy(project.ProjectId, getPolicyRequest).Do()
-	if err != nil {
-		t.Fatalf("error getting project iam policy: %v", err)
-	}
-	roleFound := false
-	changed := false
-	for _, binding := range policy.Bindings {
-		if binding.Role == "roles/container.serviceAgent" {
-			memberFound := false
-			for i, member := range binding.Members {
-				if member == containerEngineRobot {
-					binding.Members[i] = binding.Members[len(binding.Members)-1]
-					memberFound = true
-				}
-			}
-			if memberFound {
-				binding.Members = binding.Members[:len(binding.Members)-1]
-				changed = true
-			}
-		} else if binding.Role == "roles/editor" {
-			memberFound := false
-			for _, member := range binding.Members {
-				if member == containerEngineRobot {
-					memberFound = true
-					break
-				}
-			}
-			if !memberFound {
-				binding.Members = append(binding.Members, containerEngineRobot)
-				changed = true
-			}
-			roleFound = true
-		}
-	}
-	if !roleFound {
-		policy.Bindings = append(policy.Bindings, &cloudresourcemanager.Binding{
-			Members: []string{containerEngineRobot},
-			Role:    "roles/editor",
-		})
-		changed = true
-	}
-	if changed {
-		setPolicyRequest := &cloudresourcemanager.SetIamPolicyRequest{Policy: policy}
-		policy, err = client.Projects.SetIamPolicy(project.ProjectId, setPolicyRequest).Do()
-		if err != nil {
-			t.Fatalf("error setting project iam policy: %v", err)
-		}
-	}
-}
-
 // BootstrapProject will create or get a project named
 // "<projectIDPrefix><projectIDSuffix>" that will persist across test runs,
 // where projectIDSuffix is based off of getTestProjectFromEnv(). The reason

diff --git a/mmv1/third_party/terraform/services/container/resource_container_cluster_test.go.erb b/mmv1/third_party/terraform/services/container/resource_container_cluster_test.go.erb
@@ -3661,13 +3661,32 @@ func TestAccContainerCluster_autoprovisioningDefaultsManagement(t *testing.T) {
 func TestAccContainerCluster_errorCleanDanglingCluster(t *testing.T) {
 	t.Parallel()
 
-	prefix := acctest.RandString(t, 10)
-	clusterName := fmt.Sprintf("tf-test-cluster-%s", prefix)
-	clusterNameError := fmt.Sprintf("tf-test-cluster-err-%s", prefix)
+	suffix := acctest.RandString(t, 10)
+	clusterName := fmt.Sprintf("tf-test-cluster-%s", suffix)
+	clusterNameError := fmt.Sprintf("tf-test-cluster-err-%s", suffix)
+	clusterNameErrorWithTimeout := fmt.Sprintf("tf-test-cluster-timeout-%s", suffix)
 	containerNetName := fmt.Sprintf("tf-test-container-net-%s", acctest.RandString(t, 10))
 
 	initConfig := testAccContainerCluster_withInitialCIDR(containerNetName, clusterName)
-	overlapConfig := testAccContainerCluster_withCIDROverlap(initConfig, clusterNameError)
+	overlapConfig := testAccContainerCluster_withCIDROverlap(initConfig, clusterNameError, "")
+	overlapConfigWithTimeout := testAccContainerCluster_withCIDROverlap(initConfig, clusterNameErrorWithTimeout, "40s")
+
+	checkTaintApplied := func(st *terraform.State) error {
+		// Return an error if there is no tainted (i.e. marked for deletion) cluster.
+		ms := st.RootModule()
+		errCluster, ok := ms.Resources["google_container_cluster.cidr_error_overlap"]
+		if !ok {
+			var resourceNames []string
+			for rn := range ms.Resources {
+				resourceNames = append(resourceNames, rn)
+			}
+			return fmt.Errorf("could not find google_container_cluster.cidr_error_overlap in resources: %v", resourceNames)
+		}
+		if !errCluster.Primary.Tainted {
+			return fmt.Errorf("cluster with ID %s should be tainted, but is not", errCluster.Primary.ID)
+		}
+		return nil
+	}
 
 	acctest.VcrTest(t, resource.TestCase{
 		PreCheck:	  func() { acctest.AccTestPreCheck(t) },
@@ -3684,14 +3703,28 @@ func TestAccContainerCluster_errorCleanDanglingCluster(t *testing.T) {
 				ImportStateVerifyIgnore: []string{"deletion_protection"},
 			},
 			{
+				// First attempt to create the overlapping cluster with no timeout, this should fail and taint the resource.
 				Config:		   overlapConfig,
 				ExpectError: regexp.MustCompile("Error waiting for creating GKE cluster"),
 			},
-			// If tainted cluster won't be deleted, this step will return an error
 			{
-				Config:				      overlapConfig,
-				PlanOnly:			      true,
+				// Check that the tainted resource is in the config.
+				Config:             overlapConfig,
+				PlanOnly:           true,
 				ExpectNonEmptyPlan: true,
+				Check: checkTaintApplied,
+			},
+			{
+				// Next attempt to create the overlapping cluster with a 40s timeout. This will fail with a different error.
+				Config:		   overlapConfigWithTimeout,
+				ExpectError: regexp.MustCompile("timeout while waiting for state to become 'DONE'"),
+			},
+			{
+				// Check that the tainted resource is in the config.
+				Config:             overlapConfig,
+				PlanOnly:           true,
+				ExpectNonEmptyPlan: true,
+				Check: checkTaintApplied,
 			},
 		},
 	})
@@ -5396,7 +5429,6 @@ func TestAccContainerCluster_withEnablePrivateEndpointToggle(t *testing.T) {
 }
 
 func testAccContainerCluster_withEnablePrivateEndpoint(clusterName, flag, networkName, subnetworkName string) string {
-
 	return fmt.Sprintf(`
 data "google_container_engine_versions" "uscentral1a" {
   location = "us-central1-a"
@@ -7910,7 +7942,7 @@ resource "google_container_cluster" "cidr_error_preempt" {
 `, containerNetName, clusterName)
 }
 
-func testAccContainerCluster_withCIDROverlap(initConfig, secondCluster string) string {
+func testAccContainerCluster_withCIDROverlap(initConfig, secondCluster, createTimeout string) string {
 	return fmt.Sprintf(`
   %s
 
@@ -7929,8 +7961,11 @@ resource "google_container_cluster" "cidr_error_overlap" {
     services_ipv4_cidr_block = "10.1.0.0/16"
   }
   deletion_protection = false
+  timeouts {
+    create = "%s"
+  }
 }
-`, initConfig, secondCluster)
+`, initConfig, secondCluster, createTimeout)
 }
 
 func testAccContainerCluster_withInvalidLocation(location string) string {