From aace16469606955ec41559a8b5c5db04348a8fae Mon Sep 17 00:00:00 2001 From: Alex Pavel Date: Mon, 31 Mar 2025 15:33:09 -0700 Subject: [PATCH] release-payload-controller/*test.go: improve tests This PR updates all Release Payload Controller unit tests to use `New...Controller` functions instead of manually creating the structs. It also adds `t.Parallel` to all tests. This reduces test runtime for the `pkg/cmd/release-payload-controller` package from 7 seconds to 1.4 seconds on my system. --- .../job_state_controller_test.go | 29 +++------ .../legacy_job_status_controller_test.go | 35 ++++------ .../payload_accepted_controller_test.go | 24 ++----- .../payload_creation_controller_test.go | 40 ++---------- .../payload_rejected_controller_test.go | 24 ++----- .../payload_verification_controller_test.go | 24 ++----- .../prowjob_controller_test.go | 59 ++++------------- .../release_creation_job_controller_test.go | 24 ++----- ...release_creation_status_controller_test.go | 64 ++----------------- 9 files changed, 69 insertions(+), 254 deletions(-) diff --git a/pkg/cmd/release-payload-controller/job_state_controller_test.go b/pkg/cmd/release-payload-controller/job_state_controller_test.go index b7989924b..cfc9360dd 100644 --- a/pkg/cmd/release-payload-controller/job_state_controller_test.go +++ b/pkg/cmd/release-payload-controller/job_state_controller_test.go @@ -13,10 +13,10 @@ import ( releasepayloadinformers "github.com/openshift/release-controller/pkg/client/informers/externalversions" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/client-go/tools/cache" - "k8s.io/client-go/util/workqueue" ) func TestJobStateSync(t *testing.T) { + t.Parallel() testCases := []struct { name string input *v1alpha1.ReleasePayload @@ -367,22 +367,9 @@ func TestJobStateSync(t *testing.T) { releasePayloadInformerFactory := releasepayloadinformers.NewSharedInformerFactory(releasePayloadClient, controllerDefaultResyncDuration) releasePayloadInformer := releasePayloadInformerFactory.Release().V1alpha1().ReleasePayloads() - c := &JobStateController{ - ReleasePayloadController: NewReleasePayloadController("Job State Controller", - releasePayloadInformer, - releasePayloadClient.ReleaseV1alpha1(), - events.NewInMemoryRecorder("job-state-controller-test"), - workqueue.NewNamedRateLimitingQueue(workqueue.DefaultControllerRateLimiter(), "JobStateController")), - } - - if _, err := releasePayloadInformer.Informer().AddEventHandler(&cache.ResourceEventHandlerFuncs{ - AddFunc: c.Enqueue, - UpdateFunc: func(oldObj, newObj interface{}) { - c.Enqueue(newObj) - }, - DeleteFunc: c.Enqueue, - }); err != nil { - t.Errorf("Failed to add release payload event handler: %v", err) + c, err := NewJobStateController(releasePayloadInformer, releasePayloadClient.ReleaseV1alpha1(), events.NewInMemoryRecorder("job-state-controller-test")) + if err != nil { + t.Fatalf("Failed to create Job State Controller: %v", err) } releasePayloadInformerFactory.Start(context.Background().Done()) @@ -392,8 +379,7 @@ func TestJobStateSync(t *testing.T) { return } - err := c.sync(context.TODO(), fmt.Sprintf("%s/%s", testCase.input.Namespace, testCase.input.Name)) - if err != nil { + if err := c.sync(context.TODO(), fmt.Sprintf("%s/%s", testCase.input.Namespace, testCase.input.Name)); err != nil { t.Errorf("%s: unexpected err: %v", testCase.name, err) } @@ -407,6 +393,7 @@ func TestJobStateSync(t *testing.T) { } func TestComputeJobState(t *testing.T) { + t.Parallel() testCases := []struct { name string input v1alpha1.JobStatus @@ -666,6 +653,7 @@ func TestComputeJobState(t *testing.T) { } func TestInterrogateJobResults(t *testing.T) { + t.Parallel() testCases := []struct { name string results []v1alpha1.JobRunResult @@ -757,6 +745,7 @@ func TestInterrogateJobResults(t *testing.T) { } func TestProcessJobResults(t *testing.T) { + t.Parallel() testCases := []struct { name string results []v1alpha1.JobRunResult @@ -864,6 +853,7 @@ func TestProcessJobResults(t *testing.T) { } func TestProcessAnalysisJobResults(t *testing.T) { + t.Parallel() testCases := []struct { name string results []v1alpha1.JobRunResult @@ -944,6 +934,7 @@ func TestProcessAnalysisJobResults(t *testing.T) { } func TestProcessRetryJobResults(t *testing.T) { + t.Parallel() testCases := []struct { name string results []v1alpha1.JobRunResult diff --git a/pkg/cmd/release-payload-controller/legacy_job_status_controller_test.go b/pkg/cmd/release-payload-controller/legacy_job_status_controller_test.go index 3fc1fb28e..8f7ee945c 100644 --- a/pkg/cmd/release-payload-controller/legacy_job_status_controller_test.go +++ b/pkg/cmd/release-payload-controller/legacy_job_status_controller_test.go @@ -19,7 +19,6 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/client-go/tools/cache" - "k8s.io/client-go/util/workqueue" ) func newLegacyJobStatus(name string, results []v1alpha1.JobRunResult) v1alpha1.JobStatus { @@ -57,6 +56,7 @@ func newImageStream(name, namespace string, tags []imagev1.TagReference) *imagev } func TestSetLegacyJobStatus(t *testing.T) { + t.Parallel() testCases := []struct { name string input *v1alpha1.ReleasePayloadStatus @@ -138,6 +138,7 @@ func TestSetLegacyJobStatus(t *testing.T) { } func TestFindLegacyJobStatus(t *testing.T) { + t.Parallel() testCases := []struct { name string payloadName string @@ -233,6 +234,7 @@ func TestFindLegacyJobStatus(t *testing.T) { } func TestLegacyJobStatusSync(t *testing.T) { + t.Parallel() testCases := []struct { name string imagestream []runtime.Object @@ -931,25 +933,13 @@ func TestLegacyJobStatusSync(t *testing.T) { releasePayloadInformerFactory := releasepayloadinformers.NewSharedInformerFactory(releasePayloadClient, controllerDefaultResyncDuration) releasePayloadInformer := releasePayloadInformerFactory.Release().V1alpha1().ReleasePayloads() - c := &LegacyJobStatusController{ - ReleasePayloadController: NewReleasePayloadController("Legacy Job Status Controller", - releasePayloadInformer, - releasePayloadClient.ReleaseV1alpha1(), - events.NewInMemoryRecorder("legacy-job-status-controller"), - workqueue.NewNamedRateLimitingQueue(workqueue.DefaultControllerRateLimiter(), "LegacyJobStatusController")), - imageStreamLister: imageStreamInformer.Lister(), + c, err := NewLegacyJobStatusController(releasePayloadInformer, releasePayloadClient.ReleaseV1alpha1(), imageStreamInformer, events.NewInMemoryRecorder("legacy-job-status-controller")) + if err != nil { + t.Fatalf("Failed to create Legacy Job Status Controller: %v", err) } c.cachesToSync = append(c.cachesToSync, imageStreamInformer.Informer().HasSynced, releasePayloadInformer.Informer().HasSynced) - if _, err := releasePayloadInformer.Informer().AddEventHandler(&cache.ResourceEventHandlerFuncs{ - AddFunc: c.Enqueue, - UpdateFunc: func(old, new interface{}) { c.Enqueue(new) }, - DeleteFunc: c.Enqueue, - }); err != nil { - t.Errorf("Failed to add release payload event handler: %v", err) - } - releasePayloadInformerFactory.Start(context.Background().Done()) imageStreamInformerFactory.Start(context.Background().Done()) @@ -958,12 +948,13 @@ func TestLegacyJobStatusSync(t *testing.T) { return } - err := c.sync(context.TODO(), fmt.Sprintf("%s/%s", testCase.input.Namespace, testCase.input.Name)) - if err != nil && testCase.expectedErr == nil { - t.Fatalf("%s - encountered unexpected error: %v", testCase.name, err) - } - if err != nil && !cmp.Equal(err.Error(), testCase.expectedErr.Error()) { - t.Errorf("%s - expected error: %v, got: %v", testCase.name, testCase.expectedErr, err) + if err := c.sync(context.TODO(), fmt.Sprintf("%s/%s", testCase.input.Namespace, testCase.input.Name)); err != nil { + if testCase.expectedErr == nil { + t.Fatalf("%s - encountered unexpected error: %v", testCase.name, err) + } + if !cmp.Equal(err.Error(), testCase.expectedErr.Error()) { + t.Errorf("%s - expected error: %v, got: %v", testCase.name, testCase.expectedErr, err) + } } // Performing a live lookup instead of having to wait for the cache to sink (again)... diff --git a/pkg/cmd/release-payload-controller/payload_accepted_controller_test.go b/pkg/cmd/release-payload-controller/payload_accepted_controller_test.go index 2c6506352..d3f3c6ebb 100644 --- a/pkg/cmd/release-payload-controller/payload_accepted_controller_test.go +++ b/pkg/cmd/release-payload-controller/payload_accepted_controller_test.go @@ -13,10 +13,10 @@ import ( releasepayloadinformers "github.com/openshift/release-controller/pkg/client/informers/externalversions" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/client-go/tools/cache" - "k8s.io/client-go/util/workqueue" ) func TestPayloadAcceptedSync(t *testing.T) { + t.Parallel() testCases := []struct { name string input *v1alpha1.ReleasePayload @@ -446,22 +446,9 @@ func TestPayloadAcceptedSync(t *testing.T) { releasePayloadInformerFactory := releasepayloadinformers.NewSharedInformerFactory(releasePayloadClient, controllerDefaultResyncDuration) releasePayloadInformer := releasePayloadInformerFactory.Release().V1alpha1().ReleasePayloads() - c := &PayloadAcceptedController{ - ReleasePayloadController: NewReleasePayloadController("Payload Accepted Controller", - releasePayloadInformer, - releasePayloadClient.ReleaseV1alpha1(), - events.NewInMemoryRecorder("payload-accepted-controller-test"), - workqueue.NewRateLimitingQueueWithConfig(workqueue.DefaultControllerRateLimiter(), workqueue.RateLimitingQueueConfig{Name: "PayloadAcceptedController"})), - } - - if _, err := releasePayloadInformer.Informer().AddEventHandler(&cache.ResourceEventHandlerFuncs{ - AddFunc: c.Enqueue, - UpdateFunc: func(oldObj, newObj interface{}) { - c.Enqueue(newObj) - }, - DeleteFunc: c.Enqueue, - }); err != nil { - t.Errorf("Failed to add release payload event handler: %v", err) + c, err := NewPayloadAcceptedController(releasePayloadInformer, releasePayloadClient.ReleaseV1alpha1(), events.NewInMemoryRecorder("payload-accepted-controller-test")) + if err != nil { + t.Fatalf("Failed to create Payload Accepted Controller: %v", err) } releasePayloadInformerFactory.Start(context.Background().Done()) @@ -471,8 +458,7 @@ func TestPayloadAcceptedSync(t *testing.T) { return } - err := c.sync(context.TODO(), fmt.Sprintf("%s/%s", testCase.input.Namespace, testCase.input.Name)) - if err != nil { + if err := c.sync(context.TODO(), fmt.Sprintf("%s/%s", testCase.input.Namespace, testCase.input.Name)); err != nil { t.Errorf("%s: unexpected err: %v", testCase.name, err) } diff --git a/pkg/cmd/release-payload-controller/payload_creation_controller_test.go b/pkg/cmd/release-payload-controller/payload_creation_controller_test.go index 46d310a83..07b41c323 100644 --- a/pkg/cmd/release-payload-controller/payload_creation_controller_test.go +++ b/pkg/cmd/release-payload-controller/payload_creation_controller_test.go @@ -8,16 +8,15 @@ import ( "github.com/google/go-cmp/cmp" "github.com/google/go-cmp/cmp/cmpopts" "github.com/openshift/library-go/pkg/operator/events" - "github.com/openshift/library-go/pkg/operator/v1helpers" "github.com/openshift/release-controller/pkg/apis/release/v1alpha1" "github.com/openshift/release-controller/pkg/client/clientset/versioned/fake" releasepayloadinformers "github.com/openshift/release-controller/pkg/client/informers/externalversions" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/client-go/tools/cache" - "k8s.io/client-go/util/workqueue" ) func TestPayloadCreationSync(t *testing.T) { + t.Parallel() testCases := []struct { name string payload *v1alpha1.ReleasePayload @@ -250,37 +249,9 @@ func TestPayloadCreationSync(t *testing.T) { releasePayloadInformerFactory := releasepayloadinformers.NewSharedInformerFactory(releasePayloadClient, controllerDefaultResyncDuration) releasePayloadInformer := releasePayloadInformerFactory.Release().V1alpha1().ReleasePayloads() - c := &PayloadCreationController{ - ReleasePayloadController: NewReleasePayloadController("Payload Creation Controller", - releasePayloadInformer, - releasePayloadClient.ReleaseV1alpha1(), - events.NewInMemoryRecorder("payload-creation-controller-test"), - workqueue.NewNamedRateLimitingQueue(workqueue.DefaultControllerRateLimiter(), "ReleaseCreationJobController")), - } - - releasePayloadFilter := func(obj interface{}) bool { - if releasePayload, ok := obj.(*v1alpha1.ReleasePayload); ok { - // If the conditions are both in their respective terminal states, then there is nothing else to do... - if (v1helpers.IsConditionTrue(releasePayload.Status.Conditions, v1alpha1.ConditionPayloadCreated) || - v1helpers.IsConditionFalse(releasePayload.Status.Conditions, v1alpha1.ConditionPayloadCreated)) && - (v1helpers.IsConditionTrue(releasePayload.Status.Conditions, v1alpha1.ConditionPayloadFailed) || - v1helpers.IsConditionFalse(releasePayload.Status.Conditions, v1alpha1.ConditionPayloadFailed)) { - return false - } - return true - } - return false - } - - if _, err := releasePayloadInformer.Informer().AddEventHandler(cache.FilteringResourceEventHandler{ - FilterFunc: releasePayloadFilter, - Handler: cache.ResourceEventHandlerFuncs{ - AddFunc: c.Enqueue, - UpdateFunc: func(old, new interface{}) { c.Enqueue(new) }, - DeleteFunc: c.Enqueue, - }, - }); err != nil { - t.Errorf("Failed to add release payload event handler: %v", err) + c, err := NewPayloadCreationController(releasePayloadInformer, releasePayloadClient.ReleaseV1alpha1(), events.NewInMemoryRecorder("payload-creation-controller-test")) + if err != nil { + t.Fatalf("Failed to create Payload Creation Controller: %v", err) } releasePayloadInformerFactory.Start(context.Background().Done()) @@ -290,8 +261,7 @@ func TestPayloadCreationSync(t *testing.T) { return } - err := c.sync(context.TODO(), fmt.Sprintf("%s/%s", testCase.payload.Namespace, testCase.payload.Name)) - if err != nil { + if err := c.sync(context.TODO(), fmt.Sprintf("%s/%s", testCase.payload.Namespace, testCase.payload.Name)); err != nil { t.Errorf("%s: unexpected err: %v", testCase.name, err) } diff --git a/pkg/cmd/release-payload-controller/payload_rejected_controller_test.go b/pkg/cmd/release-payload-controller/payload_rejected_controller_test.go index c829ce035..7b892c7d8 100644 --- a/pkg/cmd/release-payload-controller/payload_rejected_controller_test.go +++ b/pkg/cmd/release-payload-controller/payload_rejected_controller_test.go @@ -13,10 +13,10 @@ import ( releasepayloadinformers "github.com/openshift/release-controller/pkg/client/informers/externalversions" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/client-go/tools/cache" - "k8s.io/client-go/util/workqueue" ) func TestPayloadRejectedSync(t *testing.T) { + t.Parallel() testCases := []struct { name string input *v1alpha1.ReleasePayload @@ -446,22 +446,9 @@ func TestPayloadRejectedSync(t *testing.T) { releasePayloadInformerFactory := releasepayloadinformers.NewSharedInformerFactory(releasePayloadClient, controllerDefaultResyncDuration) releasePayloadInformer := releasePayloadInformerFactory.Release().V1alpha1().ReleasePayloads() - c := &PayloadRejectedController{ - ReleasePayloadController: NewReleasePayloadController("Payload Rejected Controller", - releasePayloadInformer, - releasePayloadClient.ReleaseV1alpha1(), - events.NewInMemoryRecorder("payload-rejected-controller-test"), - workqueue.NewRateLimitingQueueWithConfig(workqueue.DefaultControllerRateLimiter(), workqueue.RateLimitingQueueConfig{Name: "PayloadRejectedController"})), - } - - if _, err := releasePayloadInformer.Informer().AddEventHandler(&cache.ResourceEventHandlerFuncs{ - AddFunc: c.Enqueue, - UpdateFunc: func(oldObj, newObj interface{}) { - c.Enqueue(newObj) - }, - DeleteFunc: c.Enqueue, - }); err != nil { - t.Errorf("Failed to add release payload event handler: %v", err) + c, err := NewPayloadRejectedController(releasePayloadInformer, releasePayloadClient.ReleaseV1alpha1(), events.NewInMemoryRecorder("payload-rejected-controller-test")) + if err != nil { + t.Fatalf("Failed to create Payload Rejected Controller: %v", err) } releasePayloadInformerFactory.Start(context.Background().Done()) @@ -471,8 +458,7 @@ func TestPayloadRejectedSync(t *testing.T) { return } - err := c.sync(context.TODO(), fmt.Sprintf("%s/%s", testCase.input.Namespace, testCase.input.Name)) - if err != nil { + if err := c.sync(context.TODO(), fmt.Sprintf("%s/%s", testCase.input.Namespace, testCase.input.Name)); err != nil { t.Errorf("%s: unexpected err: %v", testCase.name, err) } diff --git a/pkg/cmd/release-payload-controller/payload_verification_controller_test.go b/pkg/cmd/release-payload-controller/payload_verification_controller_test.go index bc216a601..4a9a59919 100644 --- a/pkg/cmd/release-payload-controller/payload_verification_controller_test.go +++ b/pkg/cmd/release-payload-controller/payload_verification_controller_test.go @@ -12,10 +12,10 @@ import ( releasepayloadinformers "github.com/openshift/release-controller/pkg/client/informers/externalversions" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/client-go/tools/cache" - "k8s.io/client-go/util/workqueue" ) func TestPayloadVerificationSync(t *testing.T) { + t.Parallel() testCases := []struct { name string input *v1alpha1.ReleasePayload @@ -407,22 +407,9 @@ func TestPayloadVerificationSync(t *testing.T) { releasePayloadInformerFactory := releasepayloadinformers.NewSharedInformerFactory(releasePayloadClient, controllerDefaultResyncDuration) releasePayloadInformer := releasePayloadInformerFactory.Release().V1alpha1().ReleasePayloads() - c := &PayloadVerificationController{ - ReleasePayloadController: NewReleasePayloadController("Payload Verification Controller", - releasePayloadInformer, - releasePayloadClient.ReleaseV1alpha1(), - events.NewInMemoryRecorder("payload-verification-controller-test"), - workqueue.NewNamedRateLimitingQueue(workqueue.DefaultControllerRateLimiter(), "PayloadVerificationController")), - } - - if _, err := releasePayloadInformer.Informer().AddEventHandler(&cache.ResourceEventHandlerFuncs{ - AddFunc: c.Enqueue, - UpdateFunc: func(oldObj, newObj interface{}) { - c.Enqueue(newObj) - }, - DeleteFunc: c.Enqueue, - }); err != nil { - t.Errorf("Failed to add release payload event handler: %v", err) + c, err := NewPayloadVerificationController(releasePayloadInformer, releasePayloadClient.ReleaseV1alpha1(), events.NewInMemoryRecorder("payload-verification-controller-test")) + if err != nil { + t.Fatalf("Failed to create Payload Verification Controller: %v", err) } releasePayloadInformerFactory.Start(context.Background().Done()) @@ -432,8 +419,7 @@ func TestPayloadVerificationSync(t *testing.T) { return } - err := c.sync(context.TODO(), fmt.Sprintf("%s/%s", testCase.input.Namespace, testCase.input.Name)) - if err != nil { + if err := c.sync(context.TODO(), fmt.Sprintf("%s/%s", testCase.input.Namespace, testCase.input.Name)); err != nil { t.Errorf("%s: unexpected err: %v", testCase.name, err) } diff --git a/pkg/cmd/release-payload-controller/prowjob_controller_test.go b/pkg/cmd/release-payload-controller/prowjob_controller_test.go index b082c92d4..b21b95e0d 100644 --- a/pkg/cmd/release-payload-controller/prowjob_controller_test.go +++ b/pkg/cmd/release-payload-controller/prowjob_controller_test.go @@ -16,7 +16,6 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/client-go/tools/cache" - "k8s.io/client-go/util/workqueue" v1 "sigs.k8s.io/prow/pkg/apis/prowjobs/v1" prowfake "sigs.k8s.io/prow/pkg/client/clientset/versioned/fake" prowjobinformers "sigs.k8s.io/prow/pkg/client/informers/externalversions" @@ -78,6 +77,7 @@ func newProwJob(name, namespace, release, jobName, source, cluster string, state } func TestSetJobStatus(t *testing.T) { + t.Parallel() testCases := []struct { name string input *v1alpha1.ReleasePayloadStatus @@ -143,6 +143,7 @@ func TestSetJobStatus(t *testing.T) { } func TestFindJobStatus(t *testing.T) { + t.Parallel() testCases := []struct { name string payloadName string @@ -247,6 +248,7 @@ func TestFindJobStatus(t *testing.T) { } func TestProwJobStatusSync(t *testing.T) { + t.Parallel() testCases := []struct { name string prowjob []runtime.Object @@ -797,46 +799,12 @@ func TestProwJobStatusSync(t *testing.T) { releasePayloadInformerFactory := releasepayloadinformers.NewSharedInformerFactory(releasePayloadClient, controllerDefaultResyncDuration) releasePayloadInformer := releasePayloadInformerFactory.Release().V1alpha1().ReleasePayloads() - c := &ProwJobStatusController{ - ReleasePayloadController: NewReleasePayloadController("ProwJob Status Controller", - releasePayloadInformer, - releasePayloadClient.ReleaseV1alpha1(), - events.NewInMemoryRecorder("prowjob-status-controller-test"), - workqueue.NewNamedRateLimitingQueue(workqueue.DefaultControllerRateLimiter(), "ProwJobStatusController")), - prowJobLister: prowJobInformer.Lister(), - } - c.cachesToSync = append(c.cachesToSync, prowJobInformer.Informer().HasSynced) - - prowJobFilter := func(obj interface{}) bool { - if prowJob, ok := obj.(*v1.ProwJob); ok { - if _, ok := prowJob.Labels[releasecontroller.ReleaseLabelVerify]; ok { - if _, ok := prowJob.Labels[releasecontroller.ReleaseLabelPayload]; ok { - return true - } - } - } - return false - } - - if _, err := prowJobInformer.Informer().AddEventHandler(cache.FilteringResourceEventHandler{ - FilterFunc: prowJobFilter, - Handler: cache.ResourceEventHandlerFuncs{ - AddFunc: c.lookupReleasePayload, - UpdateFunc: func(old, new interface{}) { c.lookupReleasePayload(new) }, - DeleteFunc: c.lookupReleasePayload, - }, - }); err != nil { - t.Errorf("Failed to add release payload event handler: %v", err) - } - - if _, err := releasePayloadInformer.Informer().AddEventHandler(&cache.ResourceEventHandlerFuncs{ - AddFunc: c.Enqueue, - UpdateFunc: func(old, new interface{}) { c.Enqueue(new) }, - DeleteFunc: c.Enqueue, - }); err != nil { - t.Errorf("Failed to add release payload event handler: %v", err) + c, err := NewProwJobStatusController(releasePayloadInformer, releasePayloadClient.ReleaseV1alpha1(), prowJobInformer, events.NewInMemoryRecorder("prowjob-status-controller-test")) + if err != nil { + t.Fatalf("Failed to create ProwJob Status Controller: %v", err) } + c.cachesToSync = append(c.cachesToSync, prowJobInformer.Informer().HasSynced) releasePayloadInformerFactory.Start(context.Background().Done()) prowJobInformerFactory.Start(context.Background().Done()) @@ -845,12 +813,13 @@ func TestProwJobStatusSync(t *testing.T) { return } - err := c.sync(context.TODO(), fmt.Sprintf("%s/%s", testCase.input.Namespace, testCase.input.Name)) - if err != nil && testCase.expectedErr == nil { - t.Fatalf("%s - encountered unexpected error: %v", testCase.name, err) - } - if err != nil && !cmp.Equal(err.Error(), testCase.expectedErr.Error()) { - t.Errorf("%s - expected error: %v, got: %v", testCase.name, testCase.expectedErr, err) + if err := c.sync(context.TODO(), fmt.Sprintf("%s/%s", testCase.input.Namespace, testCase.input.Name)); err != nil { + if testCase.expectedErr == nil { + t.Fatalf("%s - encountered unexpected error: %v", testCase.name, err) + } + if !cmp.Equal(err.Error(), testCase.expectedErr.Error()) { + t.Errorf("%s - expected error: %v, got: %v", testCase.name, testCase.expectedErr, err) + } } // Performing a live lookup instead of having to wait for the cache to sink (again)... diff --git a/pkg/cmd/release-payload-controller/release_creation_job_controller_test.go b/pkg/cmd/release-payload-controller/release_creation_job_controller_test.go index cafe41aa6..2c6793e4f 100644 --- a/pkg/cmd/release-payload-controller/release_creation_job_controller_test.go +++ b/pkg/cmd/release-payload-controller/release_creation_job_controller_test.go @@ -13,10 +13,10 @@ import ( releasepayloadinformers "github.com/openshift/release-controller/pkg/client/informers/externalversions" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/client-go/tools/cache" - "k8s.io/client-go/util/workqueue" ) func TestReleaseCreationJobSync(t *testing.T) { + t.Parallel() testCases := []struct { name string payload *v1alpha1.ReleasePayload @@ -116,22 +116,9 @@ func TestReleaseCreationJobSync(t *testing.T) { releasePayloadInformerFactory := releasepayloadinformers.NewSharedInformerFactory(releasePayloadClient, controllerDefaultResyncDuration) releasePayloadInformer := releasePayloadInformerFactory.Release().V1alpha1().ReleasePayloads() - c := &ReleaseCreationJobController{ - ReleasePayloadController: NewReleasePayloadController("Release Creation Job Controller", - releasePayloadInformer, - releasePayloadClient.ReleaseV1alpha1(), - events.NewInMemoryRecorder("release-creation-job-controller-test"), - workqueue.NewRateLimitingQueueWithConfig(workqueue.DefaultControllerRateLimiter(), workqueue.RateLimitingQueueConfig{Name: "ReleaseCreationJobController"})), - } - - if _, err := releasePayloadInformer.Informer().AddEventHandler(&cache.ResourceEventHandlerFuncs{ - AddFunc: c.Enqueue, - UpdateFunc: func(oldObj, newObj interface{}) { - c.Enqueue(newObj) - }, - DeleteFunc: c.Enqueue, - }); err != nil { - t.Errorf("Failed to add release payload event handler: %v", err) + c, err := NewReleaseCreationJobController(releasePayloadInformer, releasePayloadClient.ReleaseV1alpha1(), events.NewInMemoryRecorder("release-creation-job-controller-test")) + if err != nil { + t.Fatalf("Failed to create Release Creation Job Controller: %v", err) } releasePayloadInformerFactory.Start(context.Background().Done()) @@ -141,8 +128,7 @@ func TestReleaseCreationJobSync(t *testing.T) { return } - err := c.sync(context.TODO(), fmt.Sprintf("%s/%s", testCase.payload.Namespace, testCase.payload.Name)) - if err != nil { + if err := c.sync(context.TODO(), fmt.Sprintf("%s/%s", testCase.payload.Namespace, testCase.payload.Name)); err != nil { t.Errorf("%s: unexpected err: %v", testCase.name, err) } diff --git a/pkg/cmd/release-payload-controller/release_creation_status_controller_test.go b/pkg/cmd/release-payload-controller/release_creation_status_controller_test.go index 9c69bd977..fe4946407 100644 --- a/pkg/cmd/release-payload-controller/release_creation_status_controller_test.go +++ b/pkg/cmd/release-payload-controller/release_creation_status_controller_test.go @@ -12,7 +12,6 @@ import ( "github.com/openshift/release-controller/pkg/apis/release/v1alpha1" "github.com/openshift/release-controller/pkg/client/clientset/versioned/fake" releasepayloadinformers "github.com/openshift/release-controller/pkg/client/informers/externalversions" - releasecontroller "github.com/openshift/release-controller/pkg/release-controller" batchv1 "k8s.io/api/batch/v1" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -20,10 +19,10 @@ import ( "k8s.io/client-go/informers" fake2 "k8s.io/client-go/kubernetes/fake" "k8s.io/client-go/tools/cache" - "k8s.io/client-go/util/workqueue" ) func TestComputeReleaseCreationJobStatus(t *testing.T) { + t.Parallel() testCases := []struct { name string job *batchv1.Job @@ -106,6 +105,7 @@ func TestComputeReleaseCreationJobStatus(t *testing.T) { } func TestReleaseCreationStatusSync(t *testing.T) { + t.Parallel() testCases := []struct { name string job runtime.Object @@ -445,62 +445,12 @@ func TestReleaseCreationStatusSync(t *testing.T) { releasePayloadInformerFactory := releasepayloadinformers.NewSharedInformerFactory(releasePayloadClient, controllerDefaultResyncDuration) releasePayloadInformer := releasePayloadInformerFactory.Release().V1alpha1().ReleasePayloads() - c := &ReleaseCreationStatusController{ - ReleasePayloadController: NewReleasePayloadController("Release Creation Status Controller", - releasePayloadInformer, - releasePayloadClient.ReleaseV1alpha1(), - events.NewInMemoryRecorder("release-creation-status-controller-test"), - workqueue.NewRateLimitingQueueWithConfig(workqueue.DefaultControllerRateLimiter(), workqueue.RateLimitingQueueConfig{Name: "ReleaseCreationStatusController"})), - batchJobLister: batchJobInformer.Lister(), + c, err := NewReleaseCreationStatusController(releasePayloadInformer, releasePayloadClient.ReleaseV1alpha1(), batchJobInformer, events.NewInMemoryRecorder("release-creation-status-controller-test")) + if err != nil { + t.Fatalf("Failed to create Release Creation Status Controller: %v", err) } c.cachesToSync = append(c.cachesToSync, batchJobInformer.Informer().HasSynced) - batchJobFilter := func(obj interface{}) bool { - if batchJob, ok := obj.(*batchv1.Job); ok { - if _, ok := batchJob.Annotations[releasecontroller.ReleaseAnnotationReleaseTag]; ok { - return true - } - } - return false - } - - if _, err := batchJobInformer.Informer().AddEventHandler(cache.FilteringResourceEventHandler{ - FilterFunc: batchJobFilter, - Handler: cache.ResourceEventHandlerFuncs{ - AddFunc: c.lookupReleasePayload, - UpdateFunc: func(old, new interface{}) { c.lookupReleasePayload(new) }, - DeleteFunc: c.lookupReleasePayload, - }, - }); err != nil { - t.Errorf("Failed to add batch job event handler: %v", err) - } - - // In case someone/something deletes the ReleaseCreationJobResult.Status, try and rectify it... - releasePayloadFilter := func(obj interface{}) bool { - if releasePayload, ok := obj.(*v1alpha1.ReleasePayload); ok { - switch { - // Check that we have the necessary information to proceed - case len(releasePayload.Status.ReleaseCreationJobResult.Coordinates.Namespace) == 0 || len(releasePayload.Status.ReleaseCreationJobResult.Coordinates.Name) == 0: - return false - // Check if we need to process this ReleasePayload at all - case len(releasePayload.Status.ReleaseCreationJobResult.Status) == 0 || len(releasePayload.Status.ReleaseCreationJobResult.Message) == 0: - return true - } - } - return false - } - - if _, err := releasePayloadInformer.Informer().AddEventHandler(cache.FilteringResourceEventHandler{ - FilterFunc: releasePayloadFilter, - Handler: cache.ResourceEventHandlerFuncs{ - AddFunc: c.Enqueue, - UpdateFunc: func(old, new interface{}) { c.Enqueue(new) }, - DeleteFunc: c.Enqueue, - }, - }); err != nil { - t.Errorf("Failed to add release payload event handler: %v", err) - } - releasePayloadInformerFactory.Start(context.Background().Done()) kubeFactory.Start(context.Background().Done()) @@ -509,8 +459,7 @@ func TestReleaseCreationStatusSync(t *testing.T) { return } - err := c.sync(context.TODO(), fmt.Sprintf("%s/%s", testCase.input.Namespace, testCase.input.Name)) - if err != nil && err != testCase.expectedErr { + if err := c.sync(context.TODO(), fmt.Sprintf("%s/%s", testCase.input.Namespace, testCase.input.Name)); err != nil && err != testCase.expectedErr { t.Errorf("%s - expected error: %v, got: %v", testCase.name, testCase.expectedErr, err) } @@ -524,6 +473,7 @@ func TestReleaseCreationStatusSync(t *testing.T) { } func TestComputeReleaseCreationJobMessage(t *testing.T) { + t.Parallel() var value int32 = 1 testCases := []struct { name string