Fix a few small test issues (#3997)

- Remove unneeded CreateResourceGroup test helpers in favor of the
  existing CreateResource helpers (which are identical).
- Fix test to actually use the namespace it created.

v2/internal/controllers/edge_case_test.go

@@ -53,8 +53,7 @@ func storageAccountAndResourceGroupProvisionedOutOfOrderHelper(t *testing.T, wai
 	waitHelper(tc, acct)
 
 	// The resource group should be created successfully
-	_, err := tc.CreateResourceGroup(rg)
-	tc.Expect(err).ToNot(HaveOccurred())
+	tc.CreateResource(rg)
 
 	// The storage account should also be created successfully
 	tc.Eventually(acct).Should(tc.Match.BeProvisioned(0))

v2/internal/controllers/operatormode_test.go

@@ -32,8 +32,7 @@ func TestOperatorMode_Webhooks(t *testing.T) {
 	}
 	tc.Expect(rg.Spec.AzureName).To(Equal(""))
 
-	_, err := tc.CreateResourceGroup(&rg)
-	tc.Expect(err).ToNot(HaveOccurred())
+	tc.CreateResource(&rg)
 	// AzureName should have been defaulted on the group on the
 	// way in (it doesn't require waiting for a reconcile).
 	tc.Expect(rg.Spec.AzureName).To(Equal(rg.ObjectMeta.Name))
@@ -60,7 +59,7 @@ func TestOperatorMode_Watchers(t *testing.T) {
 	}
 	tc.Expect(rg.Spec.AzureName).To(Equal(""))
 
-	_, err := tc.CreateResourceGroup(&rg)
+	err := tc.CreateResourceExpectRequestFailure(&rg)
 	// We should fail because the webhook isn't registered (in a real
 	// multi-operator deployment it would be routed to a different
 	// operator running in webhook-only mode).
@@ -89,8 +88,7 @@ func TestOperatorMode_Both(t *testing.T) {
 	}
 	tc.Expect(rg.Spec.AzureName).To(Equal(""))
 
-	_, err := tc.CreateResourceGroup(&rg)
-	tc.Expect(err).NotTo(HaveOccurred())
+	tc.CreateResource(&rg)
 
 	// AzureName should have been defaulted on the group on the
 	// way in (it doesn't require waiting for a reconcile).

v2/internal/controllers/targetnamespace_test.go

@@ -52,7 +52,7 @@ func TestTargetNamespaces(t *testing.T) {
 		},
 		Spec: standardSpec,
 	}
-	tc.CreateResourceGroupAndWait(&rgDefault)
+	tc.CreateResourceAndWait(&rgDefault)
 	// Check that the instance is annotated with the operator namespace.
 	checkNamespaceAnnotation(tc, &rgDefault, podNamespace)
 
@@ -64,7 +64,7 @@ func TestTargetNamespaces(t *testing.T) {
 		},
 		Spec: standardSpec,
 	}
-	tc.CreateResourceGroupAndWait(&rgWatched)
+	tc.CreateResourceAndWait(&rgWatched)
 	checkNamespaceAnnotation(tc, &rgWatched, podNamespace)
 
 	// But the unwatched namespace isn't...
@@ -77,8 +77,7 @@ func TestTargetNamespaces(t *testing.T) {
 		},
 		Spec: standardSpec,
 	}
-	_, err = tc.CreateResourceGroup(&rgUnwatched)
-	tc.Expect(err).ToNot(HaveOccurred())
+	tc.CreateResource(&rgUnwatched)
 
 	// We can tell that the resource isn't being reconciled if it
 	// never gets a finalizer.
@@ -169,8 +168,7 @@ func TestOperatorNamespacePreventsReconciling(t *testing.T) {
 			Tags:     testcommon.CreateTestResourceGroupDefaultTags(),
 		},
 	}
-	_, err := tc.CreateResourceGroup(&notMine)
-	tc.Expect(err).NotTo(HaveOccurred())
+	tc.CreateResource(&notMine)
 
 	checkNeverGetsFinalizer(tc, &notMine, "instance claimed by some other operator got finalizer")
 
@@ -201,5 +199,5 @@ func TestOperatorNamespacePreventsReconciling(t *testing.T) {
 			Tags:     testcommon.CreateTestResourceGroupDefaultTags(),
 		},
 	}
-	tc.CreateResourceGroupAndWait(&mine)
+	tc.CreateResourceAndWait(&mine)
 }

v2/internal/controllers/to_azure_configmaps_test.go

@@ -193,7 +193,7 @@ func Test_ConfigMapInDifferentNamespace_ConfigMapNotFound(t *testing.T) {
 
 	configMap := &v1.ConfigMap{
 		ObjectMeta: ctrl.ObjectMeta{
-			Namespace: tc.Namespace,
+			Namespace: namespaceName,
 			Name:      configMapName,
 		},
 		Data: map[string]string{
@@ -248,11 +248,11 @@ func Test_UserConfigMapInDifferentNamespace_ShouldNotTriggerReconcile(t *testing
 
 	rg := tc.NewTestResourceGroup()
 	rg.Namespace = ns1
-	tc.CreateResourceGroupAndWait(rg)
+	tc.CreateResourceAndWait(rg)
 
 	rg2 := tc.NewTestResourceGroup()
 	rg2.Namespace = ns2
-	tc.CreateResourceGroupAndWait(rg2)
+	tc.CreateResourceAndWait(rg2)
 
 	mi := &managedidentity.UserAssignedIdentity{
 		ObjectMeta: metav1.ObjectMeta{

v2/internal/controllers/to_azure_secrets_test.go

@@ -149,11 +149,11 @@ func Test_UserSecretInDifferentNamespace_ShouldNotTriggerReconcile(t *testing.T)
 
 	rg := tc.NewTestResourceGroup()
 	rg.Namespace = ns1
-	tc.CreateResourceGroupAndWait(rg)
+	tc.CreateResourceAndWait(rg)
 
 	rg2 := tc.NewTestResourceGroup()
 	rg2.Namespace = ns2
-	tc.CreateResourceGroupAndWait(rg2)
+	tc.CreateResourceAndWait(rg2)
 
 	// VM1 with same secret name in ns1
 	vm1 := createNamespacedVM(tc, rg, ns1, ns1Secret)

v2/internal/testcommon/kube_per_test_context.go

@@ -6,7 +6,6 @@ Licensed under the MIT license.
 package testcommon
 
 import (
-	"context"
 	"fmt"
 	"math"
 	"os"
@@ -227,36 +226,6 @@ const (
 	DoNotWait       WaitCondition = false
 )
 
-// CreateResourceGroupAndWait creates the specified resource group, registers it to be deleted when the
-// context is cleaned up, and waits for it to finish being created.
-func (tc *KubePerTestContext) CreateResourceGroupAndWait(rg *resources.ResourceGroup) *resources.ResourceGroup {
-	gen := rg.GetGeneration()
-	createdResourceGroup, err := tc.CreateResourceGroup(rg)
-	tc.Expect(err).ToNot(gomega.HaveOccurred())
-	tc.Eventually(createdResourceGroup).Should(tc.Match.BeProvisioned(gen))
-	return createdResourceGroup
-}
-
-// CreateResourceGroup creates a new resource group and registers it
-// to be deleted up when the test context is cleaned up.
-func (tc *KubePerTestContext) CreateResourceGroup(rg *resources.ResourceGroup) (*resources.ResourceGroup, error) {
-	ctx := context.Background()
-
-	tc.LogSubsectionf("Create test resource group %s", rg.Name)
-
-	err := tc.kubeClient.Create(ctx, rg)
-	if err != nil {
-		return nil, errors.Wrapf(err, "creating resource group")
-	}
-
-	// register the RG for cleanup
-	// important to do this before waiting for it, so that
-	// we delete it even if we time out
-	tc.registerCleanup(rg)
-
-	return rg, nil
-}
-
 // registerCleanup registers the resource for cleanup at the end of the test. We must do this for every resource
 // for two reasons:
 //  1. Because OwnerReferences based deletion doesn't even run in EnvTest, see:
@@ -376,7 +345,10 @@ func (tc *KubePerTestContext) ExpectResourceIsDeletedInAzure(armId string, apiVe
 }
 
 func (tc *KubePerTestContext) CreateTestResourceGroupAndWait() *resources.ResourceGroup {
-	return tc.CreateResourceGroupAndWait(tc.NewTestResourceGroup())
+	rg := tc.NewTestResourceGroup()
+	tc.CreateResourceAndWait(rg)
+
+	return rg
 }
 
 // CreateResource creates a resource and registers it for cleanup. It does not wait for the resource