Skip to content

Commit

Permalink
Merge pull request #2837 from jasonvigil/periodic-predicates
Browse files Browse the repository at this point in the history 
Use reconcile gate to determine reconciler in periodic tests
  • Loading branch information
@google-oss-prow
google-oss-prow[bot] authored Oct 9, 2024
2 parents 544c623 + fbaa933 commit af768a4
Show file tree
Hide file tree
Showing 9 changed files with 326 additions and 189 deletions.
47 changes: 33 additions & 14 deletions pkg/controller/dynamic/dynamic_controller_integration_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -259,14 +259,18 @@ func validateCreate(ctx context.Context, t *testing.T, testContext testrunner.Te
t.Fatalf("[validateCreate] unexpected error when GET-ing '%v': %v", initialUnstruct.GetName(), err)
}
t.Logf("created resource is %v\r", gcpUnstruct)
if resourceContext.SupportsLabels(systemContext.SMLoader) {
if resourceContext.SupportsLabels(systemContext.SMLoader, initialUnstruct) {
testcontroller.AssertLabelsMatchAndHaveManagedLabel(t, gcpUnstruct.GetLabels(), reconciledUnstruct.GetLabels())
}

// Check that an "Updating" event was recorded, indicating that the
// controller tried to update the resource at all.
// TODO(acpana): figure out if we want to expose Updating event for direct resources
if !resourceContext.IsDirectResource {
rt, err := testreconciler.ReconcilerTypeForObject(initialUnstruct)
if err != nil {
t.Fatalf("error getting reconciler type: %v", err)
}
if rt != testreconciler.ReconcilerTypeDirect {
testcontroller.AssertEventRecordedforUnstruct(t, kubeClient, reconciledUnstruct, k8s.Updating)
}

Expand Down Expand Up @@ -370,9 +374,13 @@ func testNoChange(ctx context.Context, t *testing.T, testContext testrunner.Test
}

func testUpdate(ctx context.Context, t *testing.T, testContext testrunner.TestContext, systemContext testrunner.SystemContext, resourceContext contexts.ResourceContext) {
if testContext.UpdateUnstruct == nil {
t.Logf("UpdateUnstruct not set; skipping update")
return
}
// Tests with `SkipUpdate` explicitly set to 'true' or tests for
// auto-generated resources don't support update test.
if resourceContext.SkipUpdate || resourceContext.IsAutoGenerated(systemContext.SMLoader) &&
if resourceContext.SkipUpdate || resourceContext.IsAutoGenerated(systemContext.SMLoader, testContext.UpdateUnstruct) &&
// TODO: Remove the condition for BigQueryConnectionConnection after it becomes v1beta1.
// BigQueryConnectionConnection is a v1alpha1 resource supported via the
// autogen channel. Usually an update test is skipped for an autogen
Expand All @@ -382,10 +390,7 @@ func testUpdate(ctx context.Context, t *testing.T, testContext testrunner.TestCo
resourceContext.ResourceKind != "BigQueryConnectionConnection" {
return
}
if testContext.UpdateUnstruct == nil {
t.Logf("UpdateUnstruct not set; skipping update")
return
}

kubeClient := systemContext.Manager.GetClient()
initialUnstruct := testContext.CreateUnstruct.DeepCopy()
if err := kubeClient.Get(ctx, testContext.NamespacedName, initialUnstruct); err != nil {
Expand Down Expand Up @@ -437,7 +442,7 @@ func testUpdate(ctx context.Context, t *testing.T, testContext testrunner.TestCo
if err != nil {
t.Fatalf("[testUpdate] unexpected error when GET-ing '%v': %v", updateUnstruct.GetName(), err)
}
if resourceContext.SupportsLabels(systemContext.SMLoader) {
if resourceContext.SupportsLabels(systemContext.SMLoader, updateUnstruct) {
testcontroller.AssertLabelsMatchAndHaveManagedLabel(t, gcpUnstruct.GetLabels(), testContext.UpdateUnstruct.GetLabels())
}

Expand All @@ -454,7 +459,11 @@ func testUpdate(ctx context.Context, t *testing.T, testContext testrunner.TestCo
// Check that an "Updating" event was recorded, indicating that the
// controller tried to update the resource at all.
// TODO(acpana): figure out if we want to expose Updating event for direct resources
if !resourceContext.IsDirectResource {
rt, err := testreconciler.ReconcilerTypeForObject(updateUnstruct)
if err != nil {
t.Fatalf("error getting reconciler type: %v", err)
}
if rt != testreconciler.ReconcilerTypeDirect {
testcontroller.AssertEventRecordedforUnstruct(t, kubeClient, reconciledUnstruct, k8s.Updating)
}
// Check if condition is ready and update event was recorded
Expand Down Expand Up @@ -511,8 +520,13 @@ func shouldSkipDriftDetection(t *testing.T, resourceContext contexts.ResourceCon
return true
}

rt, err := testreconciler.ReconcilerTypeForObject(u)
if err != nil {
t.Fatalf("error getting reconciler type: %v", err)
}

// Skip drift detection test for dcl-based resources with server-generated id.
if resourceContext.IsDCLResource {
if rt == testreconciler.ReconcilerTypeDCL {
s, found := dclextension.GetNameFieldSchema(resourceContext.DCLSchema)
if !found {
// The resource doesn't have a 'resourceID' field.
Expand All @@ -523,7 +537,7 @@ func shouldSkipDriftDetection(t *testing.T, resourceContext contexts.ResourceCon
t.Fatalf("error parsing `resourceID` field schema: %v", err)
}
return isServerGenerated
} else if resourceContext.IsTFResource {
} else if rt == testreconciler.ReconcilerTypeTerraform {
// Skip drift detection test for tf-based resources with server-generated id.
rc := testservicemapping.GetResourceConfig(t, smLoader, u)
return hasServerGeneratedId(*rc)
Expand Down Expand Up @@ -699,7 +713,7 @@ func testReconcileAcquire(ctx context.Context, t *testing.T, testContext testrun
systemContext.Reconciler.Reconcile(ctx, initialUnstruct, testreconciler.ExpectedSuccessfulReconcileResultFor(systemContext.Reconciler, initialUnstruct), nil)

// Check labels match
if resourceContext.SupportsLabels(systemContext.SMLoader) {
if resourceContext.SupportsLabels(systemContext.SMLoader, initialUnstruct) {
gcpUnstruct, err := resourceContext.Get(ctx, t, initialUnstruct, systemContext.TFProvider, kubeClient, systemContext.SMLoader, systemContext.DCLConfig, systemContext.DCLConverter, nil)
if err != nil {
t.Fatalf("[testReconcileAcquire 2] unexpected error when GET-ing '%v': %v", initialUnstruct.GetName(), err)
Expand Down Expand Up @@ -729,7 +743,12 @@ func testReconcileAcquire(ctx context.Context, t *testing.T, testContext testrun

// TODO(b/174100391): Compare the resourceID of the retrieved GCP resource and the appliedUnstruct.
func verifyResourceIDIfSupported(t *testing.T, systemContext testrunner.SystemContext, resourceContext contexts.ResourceContext, reconciledUnstruct, appliedUnstruct *unstructured.Unstructured) {
if resourceContext.IsDCLResource {
rt, err := testreconciler.ReconcilerTypeForObject(appliedUnstruct)
if err != nil {
t.Fatalf("error getting reconciler type: %v", err)
}

if rt == testreconciler.ReconcilerTypeDCL {
s, found := dclextension.GetNameFieldSchema(resourceContext.DCLSchema)
if !found {
// The resource doesn't have a 'resourceID' field.
Expand All @@ -740,7 +759,7 @@ func verifyResourceIDIfSupported(t *testing.T, systemContext testrunner.SystemCo
t.Fatalf("error parsing `resourceID` field schema: %v", err)
}
verifyResourceID(t, isServerGeneratedID, reconciledUnstruct, appliedUnstruct)
} else if resourceContext.IsTFResource {
} else if rt == testreconciler.ReconcilerTypeTerraform {
rc, err := systemContext.SMLoader.GetResourceConfig(reconciledUnstruct)
if err != nil {
t.Fatalf("error getting resource config for Kind '%s', "+
Expand Down
34 changes: 23 additions & 11 deletions pkg/controller/iam/auditconfig/iamauditconfig_controller_integration_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import (
"context"
"errors"
"reflect"
"regexp"
"strings"
"testing"

Expand All @@ -43,6 +44,7 @@ import (
apierrors "k8s.io/apimachinery/pkg/api/errors"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
"k8s.io/apimachinery/pkg/runtime"
"sigs.k8s.io/controller-runtime/pkg/manager"
"sigs.k8s.io/controller-runtime/pkg/reconcile"
)
Expand Down Expand Up @@ -136,7 +138,7 @@ func testReconcileResourceLevelCreate(ctx context.Context, t *testing.T, mgr man
if err != nil {
t.Fatalf("error marshalling %v as unstructured: %v", k8sAuditConfig, err)
}
reconciler.ReconcileObjectMeta(ctx, k8sAuditConfig.ObjectMeta, iamv1beta1.IAMAuditConfigGVK.Kind, testreconciler.ExpectedSuccessfulReconcileResultFor(reconciler, u), nil)
reconcileIAMAuditConfig(ctx, t, reconciler, k8sAuditConfig, testreconciler.ExpectedSuccessfulReconcileResultFor(reconciler, u), nil)
gcpAuditConfig, err := tfIamClient.GetAuditConfig(ctx, k8sAuditConfig)
if err != nil {
t.Fatalf("error retrieving GCP audit config: %v", err)
Expand Down Expand Up @@ -218,7 +220,7 @@ func testReconcileResourceLevelUpdate(ctx context.Context, t *testing.T, mgr man
t.Fatalf("error creating k8s resource: %v", err)
}
preReconcileGeneration := k8sAuditConfig.GetGeneration()
reconciler.ReconcileObjectMeta(ctx, k8sAuditConfig.ObjectMeta, iamv1beta1.IAMAuditConfigGVK.Kind, expectedReconcileResult, nil)
reconcileIAMAuditConfig(ctx, t, reconciler, k8sAuditConfig, expectedReconcileResult, nil)
gcpAuditConfig, err := tfIamClient.GetAuditConfig(ctx, k8sAuditConfig)
if err != nil {
t.Fatalf("error retrieving GCP audit config: %v", err)
Expand All @@ -233,7 +235,7 @@ func testReconcileResourceLevelUpdate(ctx context.Context, t *testing.T, mgr man
t.Fatalf("error updating k8s resource: %v", err)
}
preReconcileGeneration = newK8sAuditConfig.GetGeneration()
reconciler.ReconcileObjectMeta(ctx, newK8sAuditConfig.ObjectMeta, iamv1beta1.IAMAuditConfigGVK.Kind, expectedReconcileResult, nil)
reconcileIAMAuditConfig(ctx, t, reconciler, newK8sAuditConfig, expectedReconcileResult, nil)
if err := kubeClient.Get(ctx, k8s.GetNamespacedName(newK8sAuditConfig), newK8sAuditConfig); err != nil {
t.Fatalf("unexpected error getting k8s resource: %v", err)
}
Expand Down Expand Up @@ -301,7 +303,7 @@ func testReconcileResourceLevelNoChanges(ctx context.Context, t *testing.T, mgr
t.Fatalf("error creating k8s resource: %v", err)
}
preReconcileGeneration := k8sAuditConfig.GetGeneration()
reconciler.ReconcileObjectMeta(ctx, k8sAuditConfig.ObjectMeta, iamv1beta1.IAMAuditConfigGVK.Kind, expectedReconcileResult, nil)
reconcileIAMAuditConfig(ctx, t, reconciler, k8sAuditConfig, expectedReconcileResult, nil)
gcpAuditConfig, err := tfIamClient.GetAuditConfig(ctx, k8sAuditConfig)
if err != nil {
t.Fatalf("error retrieving GCP audit config: %v", err)
Expand All @@ -312,7 +314,7 @@ func testReconcileResourceLevelNoChanges(ctx context.Context, t *testing.T, mgr
}
assertObservedGenerationEquals(t, k8sAuditConfig, preReconcileGeneration)
preReconcileGeneration = k8sAuditConfig.GetGeneration()
reconciler.ReconcileObjectMeta(ctx, k8sAuditConfig.ObjectMeta, iamv1beta1.IAMAuditConfigGVK.Kind, expectedReconcileResult, nil)
reconcileIAMAuditConfig(ctx, t, reconciler, k8sAuditConfig, expectedReconcileResult, nil)
newK8sAuditConfig := &iamv1beta1.IAMAuditConfig{}
if err := kubeClient.Get(ctx, k8s.GetNamespacedName(k8sAuditConfig), newK8sAuditConfig); err != nil {
t.Fatalf("unexpected error getting k8s resource: %v", err)
Expand Down Expand Up @@ -384,7 +386,7 @@ func testReconcileResourceLevelDelete(ctx context.Context, t *testing.T, mgr man
t.Fatalf("error creating k8s resource: %v", err)
}
preReconcileGeneration := k8sAuditConfig.GetGeneration()
reconciler.ReconcileObjectMeta(ctx, k8sAuditConfig.ObjectMeta, iamv1beta1.IAMAuditConfigGVK.Kind, expectedReconcileResult, nil)
reconcileIAMAuditConfig(ctx, t, reconciler, k8sAuditConfig, expectedReconcileResult, nil)
gcpAuditConfig, err := tfIamClient.GetAuditConfig(ctx, k8sAuditConfig)
if err != nil {
t.Fatalf("error retrieving GCP audit config: %v", err)
Expand All @@ -398,13 +400,13 @@ func testReconcileResourceLevelDelete(ctx context.Context, t *testing.T, mgr man
if err := kubeClient.Delete(ctx, k8sAuditConfig); err != nil {
t.Fatalf("error deleting k8s resource: %v", err)
}
reconciler.ReconcileObjectMeta(ctx, k8sAuditConfig.ObjectMeta, iamv1beta1.IAMAuditConfigGVK.Kind, testreconciler.ExpectedRequeueReconcileStruct, nil)
reconcileIAMAuditConfig(ctx, t, reconciler, k8sAuditConfig, testreconciler.ExpectedRequeueReconcileStruct, nil)
_, err = tfIamClient.GetAuditConfig(ctx, k8sAuditConfig)
if err != nil {
t.Fatalf("expected audit config to exist in GCP, but got error: %v", err)
}
testk8s.RemoveDeletionDefenderFinalizer(t, k8sAuditConfig, v1beta1.IAMAuditConfigGVK, kubeClient)
reconciler.ReconcileObjectMeta(ctx, k8sAuditConfig.ObjectMeta, iamv1beta1.IAMAuditConfigGVK.Kind, expectedReconcileResult, nil)
reconcileIAMAuditConfig(ctx, t, reconciler, k8sAuditConfig, expectedReconcileResult, nil)
if _, err := tfIamClient.GetAuditConfig(ctx, k8sAuditConfig); !errors.Is(err, kcciamclient.ErrNotFound) {
t.Fatalf("unexpected error value: got '%v', want '%v'", err, kcciamclient.ErrNotFound)
}
Expand Down Expand Up @@ -468,7 +470,7 @@ func testReconcileResourceLevelDeleteParentFirst(ctx context.Context, t *testing
t.Fatalf("error creating k8s resource: %v", err)
}
preReconcileGeneration := k8sAuditConfig.GetGeneration()
reconciler.ReconcileObjectMeta(ctx, k8sAuditConfig.ObjectMeta, iamv1beta1.IAMAuditConfigGVK.Kind, expectedReconcileResult, nil)
reconcileIAMAuditConfig(ctx, t, reconciler, k8sAuditConfig, expectedReconcileResult, nil)
gcpAuditConfig, err := tfIamClient.GetAuditConfig(ctx, k8sAuditConfig)
if err != nil {
t.Fatalf("error retrieving GCP audit config: %v", err)
Expand All @@ -492,7 +494,7 @@ func testReconcileResourceLevelDeleteParentFirst(ctx context.Context, t *testing
t.Fatalf("error deleting k8s resource: %v", err)
}
testk8s.RemoveDeletionDefenderFinalizer(t, k8sAuditConfig, v1beta1.IAMAuditConfigGVK, kubeClient)
reconciler.ReconcileObjectMeta(ctx, k8sAuditConfig.ObjectMeta, iamv1beta1.IAMAuditConfigGVK.Kind, expectedReconcileResult, nil)
reconcileIAMAuditConfig(ctx, t, reconciler, k8sAuditConfig, expectedReconcileResult, nil)
if err := kubeClient.Get(ctx, k8s.GetNamespacedName(k8sAuditConfig), k8sAuditConfig); err == nil || !apierrors.IsNotFound(err) {
t.Fatalf("unexpected error value: %v", err)
}
Expand Down Expand Up @@ -557,7 +559,7 @@ func testReconcileResourceLevelAcquire(ctx context.Context, t *testing.T, mgr ma
t.Fatalf("error creating k8s resource: %v", err)
}
preReconcileGeneration := k8sAuditConfig.GetGeneration()
reconciler.ReconcileObjectMeta(ctx, k8sAuditConfig.ObjectMeta, iamv1beta1.IAMAuditConfigGVK.Kind, expectedReconcileResult, nil)
reconcileIAMAuditConfig(ctx, t, reconciler, k8sAuditConfig, expectedReconcileResult, nil)
if _, err := tfIamClient.GetAuditConfig(ctx, k8sAuditConfig); err != nil {
t.Fatalf("error retrieving GCP audit config: %v", err)
}
Expand Down Expand Up @@ -606,6 +608,16 @@ func newIAMAuditConfigFixture(t *testing.T, refResource *unstructured.Unstructur
}
}

func reconcileIAMAuditConfig(ctx context.Context, t *testing.T, reconciler *testreconciler.TestReconciler, auditConfig *iamv1beta1.IAMAuditConfig, expectedResult reconcile.Result, expectedErrorRegex *regexp.Regexp) {
kcciamclient.SetGVK(auditConfig)
uObj, err := runtime.DefaultUnstructuredConverter.ToUnstructured(auditConfig)
if err != nil {
t.Fatalf("error converting to unstructured: %v", err)
}
u := &unstructured.Unstructured{Object: uObj}
reconciler.Reconcile(ctx, u, expectedResult, expectedErrorRegex)
}

func name(t *testing.T) string {
// Necessary to remove the "/$KIND" portion of the subtest name
name := strings.ToLower(testcontroller.Name(t))
Expand Down
Loading

0 comments on commit af768a4

Please sign in to comment.