Skip to content

Commit

Permalink
Fix failing test due to the fake client indexer change in the new con…
Browse files Browse the repository at this point in the history 
…troller-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
```

Signed-off-by: Yusuke Kuoka <ykuoka@gmail.com>
  • Loading branch information
@mumoshu
mumoshu committed Jan 23, 2023
1 parent 1b61488 commit ade9ccd
Show file tree
Hide file tree
Showing 2 changed files with 67 additions and 61 deletions.
122 changes: 62 additions & 60 deletions controllers/actions.summerwind.net/horizontal_runner_autoscaler_webhook.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -681,78 +681,80 @@ func (autoscaler *HorizontalRunnerAutoscalerGitHubWebhook) SetupWithManager(mgr

autoscaler.Recorder = mgr.GetEventRecorderFor(name)

if err := mgr.GetFieldIndexer().IndexField(context.TODO(), &v1alpha1.HorizontalRunnerAutoscaler{}, scaleTargetKey, func(rawObj client.Object) []string {
hra := rawObj.(*v1alpha1.HorizontalRunnerAutoscaler)
if err := mgr.GetFieldIndexer().IndexField(context.TODO(), &v1alpha1.HorizontalRunnerAutoscaler{}, scaleTargetKey, autoscaler.indexer); err != nil {
return err
}

return ctrl.NewControllerManagedBy(mgr).
For(&v1alpha1.HorizontalRunnerAutoscaler{}).
Named(name).
Complete(autoscaler)
}

func (autoscaler *HorizontalRunnerAutoscalerGitHubWebhook) indexer(rawObj client.Object) []string {
hra := rawObj.(*v1alpha1.HorizontalRunnerAutoscaler)

if hra.Spec.ScaleTargetRef.Name == "" {
autoscaler.Log.V(1).Info(fmt.Sprintf("scale target ref name not set for hra %s", hra.Name))
return nil
}

if hra.Spec.ScaleTargetRef.Name == "" {
autoscaler.Log.V(1).Info(fmt.Sprintf("scale target ref name not set for hra %s", hra.Name))
switch hra.Spec.ScaleTargetRef.Kind {
case "", "RunnerDeployment":
var rd v1alpha1.RunnerDeployment
if err := autoscaler.Client.Get(context.Background(), types.NamespacedName{Namespace: hra.Namespace, Name: hra.Spec.ScaleTargetRef.Name}, &rd); err != nil {
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))
return nil
}

switch hra.Spec.ScaleTargetRef.Kind {
case "", "RunnerDeployment":
var rd v1alpha1.RunnerDeployment
if err := autoscaler.Client.Get(context.Background(), types.NamespacedName{Namespace: hra.Namespace, Name: hra.Spec.ScaleTargetRef.Name}, &rd); err != nil {
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))
return nil
}

keys := []string{}
if rd.Spec.Template.Spec.Repository != "" {
keys = append(keys, rd.Spec.Template.Spec.Repository) // Repository runners
}
if rd.Spec.Template.Spec.Organization != "" {
if group := rd.Spec.Template.Spec.Group; group != "" {
keys = append(keys, organizationalRunnerGroupKey(rd.Spec.Template.Spec.Organization, rd.Spec.Template.Spec.Group)) // Organization runner groups
} else {
keys = append(keys, rd.Spec.Template.Spec.Organization) // Organization runners
}
}
if enterprise := rd.Spec.Template.Spec.Enterprise; enterprise != "" {
if group := rd.Spec.Template.Spec.Group; group != "" {
keys = append(keys, enterpriseRunnerGroupKey(enterprise, rd.Spec.Template.Spec.Group)) // Enterprise runner groups
} else {
keys = append(keys, enterpriseKey(enterprise)) // Enterprise runners
}
keys := []string{}
if rd.Spec.Template.Spec.Repository != "" {
keys = append(keys, rd.Spec.Template.Spec.Repository) // Repository runners
}
if rd.Spec.Template.Spec.Organization != "" {
if group := rd.Spec.Template.Spec.Group; group != "" {
keys = append(keys, organizationalRunnerGroupKey(rd.Spec.Template.Spec.Organization, rd.Spec.Template.Spec.Group)) // Organization runner groups
} else {
keys = append(keys, rd.Spec.Template.Spec.Organization) // Organization runners
}
autoscaler.Log.V(2).Info(fmt.Sprintf("HRA keys indexed for HRA %s: %v", hra.Name, keys))
return keys
case "RunnerSet":
var rs v1alpha1.RunnerSet
if err := autoscaler.Client.Get(context.Background(), types.NamespacedName{Namespace: hra.Namespace, Name: hra.Spec.ScaleTargetRef.Name}, &rs); err != nil {
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))
return nil
}
if enterprise := rd.Spec.Template.Spec.Enterprise; enterprise != "" {
if group := rd.Spec.Template.Spec.Group; group != "" {
keys = append(keys, enterpriseRunnerGroupKey(enterprise, rd.Spec.Template.Spec.Group)) // Enterprise runner groups
} else {
keys = append(keys, enterpriseKey(enterprise)) // Enterprise runners
}
}
autoscaler.Log.V(2).Info(fmt.Sprintf("HRA keys indexed for HRA %s: %v", hra.Name, keys))
return keys
case "RunnerSet":
var rs v1alpha1.RunnerSet
if err := autoscaler.Client.Get(context.Background(), types.NamespacedName{Namespace: hra.Namespace, Name: hra.Spec.ScaleTargetRef.Name}, &rs); err != nil {
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))
return nil
}

keys := []string{}
if rs.Spec.Repository != "" {
keys = append(keys, rs.Spec.Repository) // Repository runners
}
if rs.Spec.Organization != "" {
keys = append(keys, rs.Spec.Organization) // Organization runners
if group := rs.Spec.Group; group != "" {
keys = append(keys, organizationalRunnerGroupKey(rs.Spec.Organization, rs.Spec.Group)) // Organization runner groups
}
keys := []string{}
if rs.Spec.Repository != "" {
keys = append(keys, rs.Spec.Repository) // Repository runners
}
if rs.Spec.Organization != "" {
keys = append(keys, rs.Spec.Organization) // Organization runners
if group := rs.Spec.Group; group != "" {
keys = append(keys, organizationalRunnerGroupKey(rs.Spec.Organization, rs.Spec.Group)) // Organization runner groups
}
if enterprise := rs.Spec.Enterprise; enterprise != "" {
keys = append(keys, enterpriseKey(enterprise)) // Enterprise runners
if group := rs.Spec.Group; group != "" {
keys = append(keys, enterpriseRunnerGroupKey(enterprise, rs.Spec.Group)) // Enterprise runner groups
}
}
if enterprise := rs.Spec.Enterprise; enterprise != "" {
keys = append(keys, enterpriseKey(enterprise)) // Enterprise runners
if group := rs.Spec.Group; group != "" {
keys = append(keys, enterpriseRunnerGroupKey(enterprise, rs.Spec.Group)) // Enterprise runner groups
}
autoscaler.Log.V(2).Info(fmt.Sprintf("HRA keys indexed for HRA %s: %v", hra.Name, keys))
return keys
}

return nil
}); err != nil {
return err
autoscaler.Log.V(2).Info(fmt.Sprintf("HRA keys indexed for HRA %s: %v", hra.Name, keys))
return keys
}

return ctrl.NewControllerManagedBy(mgr).
For(&v1alpha1.HorizontalRunnerAutoscaler{}).
Named(name).
Complete(autoscaler)
return nil
}

func enterpriseKey(name string) string {
Expand Down
6 changes: 5 additions & 1 deletion controllers/actions.summerwind.net/horizontal_runner_autoscaler_webhook_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -431,7 +431,11 @@ func testServerWithInitObjs(t *testing.T, eventType string, event interface{}, w

hraWebhook := &HorizontalRunnerAutoscalerGitHubWebhook{}

client := fake.NewClientBuilder().WithScheme(sc).WithRuntimeObjects(initObjs...).Build()
client := fake.NewClientBuilder().
WithScheme(sc).
WithRuntimeObjects(initObjs...).
WithIndex(&actionsv1alpha1.HorizontalRunnerAutoscaler{}, scaleTargetKey, hraWebhook.indexer).
Build()

logs := installTestLogger(hraWebhook)

Expand Down

0 comments on commit ade9ccd

Please sign in to comment.