Skip to content

Commit 6be39f0

Browse files
mumoshumumoshu
committed
Fix failing test due to the fake client indexer change in the new controller-runtime
controller-runtime 0.14 includes kubernetes-sigs/controller-runtime#2025 which turned out to be a breaking change for any tests that use fake controller-runtime client for list operations against the index. This commit addresses that, by extracting the indexer func out of the reconciler setup function so that both the reconciler setup func and the set-only fake client setup func can use the same indexer func. This fixes integration errors like the below example: ``` --- FAIL: TestWebhookWorkflowJobWithSelfHostedLabel (0.00s) --- FAIL: TestWebhookWorkflowJobWithSelfHostedLabel/Successful (0.00s) horizontal_runner_autoscaler_webhook_test.go:256: status: 500 horizontal_runner_autoscaler_webhook_test.go:256: body: List on GroupVersionKind actions.summerwind.dev/v1alpha1, Kind=HorizontalRunnerAutoscaler specifies selector on field scaleTarget, but no index with name scaleTarget has been registered for GroupVersionKind actions.summerwind.dev/v1alpha1, Kind=HorizontalRunnerAutoscaler horizontal_runner_autoscaler_webhook_test.go:440: diagnostics: testlog] finding repository-wide runnerrepository.name=MYREPO repository.owner.login=MYORG repository.owner.type=Organization action=queued delivery= workflowJob.labels=[self-hosted label1] workflowJob.status=queued enterprise.slug= workflowJob.runID=1234567890 workflowJob.ID=1234567890 event=workflow_job hookID= repository=MYORG/MYREPO error=List on GroupVersionKind actions.summerwind.dev/v1alpha1, Kind=HorizontalRunnerAutoscaler specifies selector on field scaleTarget, but no index with name scaleTarget has been registered for GroupVersionKind actions.summerwind.dev/v1alpha1, Kind=HorizontalRunnerAutoscaler testlog] handling check_run eventevent=workflow_job hookID= workflowJob.status=queued enterprise.slug= workflowJob.runID=1234567890 workflowJob.ID=1234567890 delivery= workflowJob.labels=[self-hosted label1] repository.name=MYREPO repository.owner.login=MYORG repository.owner.type=Organization action=queued error=List on GroupVersionKind actions.summerwind.dev/v1alpha1, Kind=HorizontalRunnerAutoscaler specifies selector on field scaleTarget, but no index with name scaleTarget has been registered for GroupVersionKind actions.summerwind.dev/v1alpha1, Kind=HorizontalRunnerAutoscaler ```
1 parent 58c88d6 commit 6be39f0

File tree

2 files changed

+67
-61
lines changed

2 files changed

+67
-61
lines changed

‎controllers/actions.summerwind.net/horizontal_runner_autoscaler_webhook.go

+62-60
Original file line numberDiff line numberDiff line change
@@ -681,78 +681,80 @@ func (autoscaler *HorizontalRunnerAutoscalerGitHubWebhook) SetupWithManager(mgr
681681

682682
autoscaler.Recorder = mgr.GetEventRecorderFor(name)
683683

684-
if err := mgr.GetFieldIndexer().IndexField(context.TODO(), &v1alpha1.HorizontalRunnerAutoscaler{}, scaleTargetKey, func(rawObj client.Object) []string {
685-
hra := rawObj.(*v1alpha1.HorizontalRunnerAutoscaler)
684+
if err := mgr.GetFieldIndexer().IndexField(context.TODO(), &v1alpha1.HorizontalRunnerAutoscaler{}, scaleTargetKey, autoscaler.indexer); err != nil {
685+
return err
686+
}
687+
688+
return ctrl.NewControllerManagedBy(mgr).
689+
For(&v1alpha1.HorizontalRunnerAutoscaler{}).
690+
Named(name).
691+
Complete(autoscaler)
692+
}
693+
694+
func (autoscaler *HorizontalRunnerAutoscalerGitHubWebhook) indexer(rawObj client.Object) []string {
695+
hra := rawObj.(*v1alpha1.HorizontalRunnerAutoscaler)
696+
697+
if hra.Spec.ScaleTargetRef.Name == "" {
698+
autoscaler.Log.V(1).Info(fmt.Sprintf("scale target ref name not set for hra %s", hra.Name))
699+
return nil
700+
}
686701

687-
if hra.Spec.ScaleTargetRef.Name == "" {
688-
autoscaler.Log.V(1).Info(fmt.Sprintf("scale target ref name not set for hra %s", hra.Name))
702+
switch hra.Spec.ScaleTargetRef.Kind {
703+
case "", "RunnerDeployment":
704+
var rd v1alpha1.RunnerDeployment
705+
if err := autoscaler.Client.Get(context.Background(), types.NamespacedName{Namespace: hra.Namespace, Name: hra.Spec.ScaleTargetRef.Name}, &rd); err != nil {
706+
autoscaler.Log.V(1).Info(fmt.Sprintf("RunnerDeployment not found with scale target ref name %s for hra %s", hra.Spec.ScaleTargetRef.Name, hra.Name))
689707
return nil
690708
}
691709

692-
switch hra.Spec.ScaleTargetRef.Kind {
693-
case "", "RunnerDeployment":
694-
var rd v1alpha1.RunnerDeployment
695-
if err := autoscaler.Client.Get(context.Background(), types.NamespacedName{Namespace: hra.Namespace, Name: hra.Spec.ScaleTargetRef.Name}, &rd); err != nil {
696-
autoscaler.Log.V(1).Info(fmt.Sprintf("RunnerDeployment not found with scale target ref name %s for hra %s", hra.Spec.ScaleTargetRef.Name, hra.Name))
697-
return nil
698-
}
699-
700-
keys := []string{}
701-
if rd.Spec.Template.Spec.Repository != "" {
702-
keys = append(keys, rd.Spec.Template.Spec.Repository) // Repository runners
703-
}
704-
if rd.Spec.Template.Spec.Organization != "" {
705-
if group := rd.Spec.Template.Spec.Group; group != "" {
706-
keys = append(keys, organizationalRunnerGroupKey(rd.Spec.Template.Spec.Organization, rd.Spec.Template.Spec.Group)) // Organization runner groups
707-
} else {
708-
keys = append(keys, rd.Spec.Template.Spec.Organization) // Organization runners
709-
}
710-
}
711-
if enterprise := rd.Spec.Template.Spec.Enterprise; enterprise != "" {
712-
if group := rd.Spec.Template.Spec.Group; group != "" {
713-
keys = append(keys, enterpriseRunnerGroupKey(enterprise, rd.Spec.Template.Spec.Group)) // Enterprise runner groups
714-
} else {
715-
keys = append(keys, enterpriseKey(enterprise)) // Enterprise runners
716-
}
710+
keys := []string{}
711+
if rd.Spec.Template.Spec.Repository != "" {
712+
keys = append(keys, rd.Spec.Template.Spec.Repository) // Repository runners
713+
}
714+
if rd.Spec.Template.Spec.Organization != "" {
715+
if group := rd.Spec.Template.Spec.Group; group != "" {
716+
keys = append(keys, organizationalRunnerGroupKey(rd.Spec.Template.Spec.Organization, rd.Spec.Template.Spec.Group)) // Organization runner groups
717+
} else {
718+
keys = append(keys, rd.Spec.Template.Spec.Organization) // Organization runners
717719
}
718-
autoscaler.Log.V(2).Info(fmt.Sprintf("HRA keys indexed for HRA %s: %v", hra.Name, keys))
719-
return keys
720-
case "RunnerSet":
721-
var rs v1alpha1.RunnerSet
722-
if err := autoscaler.Client.Get(context.Background(), types.NamespacedName{Namespace: hra.Namespace, Name: hra.Spec.ScaleTargetRef.Name}, &rs); err != nil {
723-
autoscaler.Log.V(1).Info(fmt.Sprintf("RunnerSet not found with scale target ref name %s for hra %s", hra.Spec.ScaleTargetRef.Name, hra.Name))
724-
return nil
720+
}
721+
if enterprise := rd.Spec.Template.Spec.Enterprise; enterprise != "" {
722+
if group := rd.Spec.Template.Spec.Group; group != "" {
723+
keys = append(keys, enterpriseRunnerGroupKey(enterprise, rd.Spec.Template.Spec.Group)) // Enterprise runner groups
724+
} else {
725+
keys = append(keys, enterpriseKey(enterprise)) // Enterprise runners
725726
}
727+
}
728+
autoscaler.Log.V(2).Info(fmt.Sprintf("HRA keys indexed for HRA %s: %v", hra.Name, keys))
729+
return keys
730+
case "RunnerSet":
731+
var rs v1alpha1.RunnerSet
732+
if err := autoscaler.Client.Get(context.Background(), types.NamespacedName{Namespace: hra.Namespace, Name: hra.Spec.ScaleTargetRef.Name}, &rs); err != nil {
733+
autoscaler.Log.V(1).Info(fmt.Sprintf("RunnerSet not found with scale target ref name %s for hra %s", hra.Spec.ScaleTargetRef.Name, hra.Name))
734+
return nil
735+
}
726736

727-
keys := []string{}
728-
if rs.Spec.Repository != "" {
729-
keys = append(keys, rs.Spec.Repository) // Repository runners
730-
}
731-
if rs.Spec.Organization != "" {
732-
keys = append(keys, rs.Spec.Organization) // Organization runners
733-
if group := rs.Spec.Group; group != "" {
734-
keys = append(keys, organizationalRunnerGroupKey(rs.Spec.Organization, rs.Spec.Group)) // Organization runner groups
735-
}
737+
keys := []string{}
738+
if rs.Spec.Repository != "" {
739+
keys = append(keys, rs.Spec.Repository) // Repository runners
740+
}
741+
if rs.Spec.Organization != "" {
742+
keys = append(keys, rs.Spec.Organization) // Organization runners
743+
if group := rs.Spec.Group; group != "" {
744+
keys = append(keys, organizationalRunnerGroupKey(rs.Spec.Organization, rs.Spec.Group)) // Organization runner groups
736745
}
737-
if enterprise := rs.Spec.Enterprise; enterprise != "" {
738-
keys = append(keys, enterpriseKey(enterprise)) // Enterprise runners
739-
if group := rs.Spec.Group; group != "" {
740-
keys = append(keys, enterpriseRunnerGroupKey(enterprise, rs.Spec.Group)) // Enterprise runner groups
741-
}
746+
}
747+
if enterprise := rs.Spec.Enterprise; enterprise != "" {
748+
keys = append(keys, enterpriseKey(enterprise)) // Enterprise runners
749+
if group := rs.Spec.Group; group != "" {
750+
keys = append(keys, enterpriseRunnerGroupKey(enterprise, rs.Spec.Group)) // Enterprise runner groups
742751
}
743-
autoscaler.Log.V(2).Info(fmt.Sprintf("HRA keys indexed for HRA %s: %v", hra.Name, keys))
744-
return keys
745752
}
746-
747-
return nil
748-
}); err != nil {
749-
return err
753+
autoscaler.Log.V(2).Info(fmt.Sprintf("HRA keys indexed for HRA %s: %v", hra.Name, keys))
754+
return keys
750755
}
751756

752-
return ctrl.NewControllerManagedBy(mgr).
753-
For(&v1alpha1.HorizontalRunnerAutoscaler{}).
754-
Named(name).
755-
Complete(autoscaler)
757+
return nil
756758
}
757759

758760
func enterpriseKey(name string) string {

‎controllers/actions.summerwind.net/horizontal_runner_autoscaler_webhook_test.go

+5-1
Original file line numberDiff line numberDiff line change
@@ -431,7 +431,11 @@ func testServerWithInitObjs(t *testing.T, eventType string, event interface{}, w
431431

432432
hraWebhook := &HorizontalRunnerAutoscalerGitHubWebhook{}
433433

434-
client := fake.NewClientBuilder().WithScheme(sc).WithRuntimeObjects(initObjs...).Build()
434+
client := fake.NewClientBuilder().
435+
WithScheme(sc).
436+
WithRuntimeObjects(initObjs...).
437+
WithIndex(&actionsv1alpha1.HorizontalRunnerAutoscaler{}, scaleTargetKey, hraWebhook.indexer).
438+
Build()
435439

436440
logs := installTestLogger(hraWebhook)
437441

0 commit comments

Comments
 (0)