From b2ac9d930a27389ff7a10c3837f58918656ba746 Mon Sep 17 00:00:00 2001 From: Nikola Jokic Date: Mon, 6 Oct 2025 17:07:59 +0200 Subject: [PATCH] fix test --- .../horizontal_runner_autoscaler_webhook.go | 88 ++++++++++++------- ...rizontal_runner_autoscaler_webhook_test.go | 20 ++++- 2 files changed, 73 insertions(+), 35 deletions(-) diff --git a/controllers/actions.summerwind.net/horizontal_runner_autoscaler_webhook.go b/controllers/actions.summerwind.net/horizontal_runner_autoscaler_webhook.go index 0f37d0d3..20d814b5 100644 --- a/controllers/actions.summerwind.net/horizontal_runner_autoscaler_webhook.go +++ b/controllers/actions.summerwind.net/horizontal_runner_autoscaler_webhook.go @@ -321,26 +321,34 @@ func (autoscaler *HorizontalRunnerAutoscalerGitHubWebhook) findHRAsByKey(ctx con defaultListOpts = append(defaultListOpts, client.InNamespace(ns)) } - var hras []v1alpha1.HorizontalRunnerAutoscaler - - if value != "" { - opts := append([]client.ListOption{}, defaultListOpts...) - opts = append(opts, client.MatchingFields{scaleTargetKey: value}) - - if autoscaler.Namespace != "" { - opts = append(opts, client.InNamespace(autoscaler.Namespace)) - } - - var hraList v1alpha1.HorizontalRunnerAutoscalerList - - if err := autoscaler.List(ctx, &hraList, opts...); err != nil { - return nil, err - } - - hras = append(hras, hraList.Items...) + // Get all HRAs since we can't use the index for repository/organization lookup anymore + var hraList v1alpha1.HorizontalRunnerAutoscalerList + if err := autoscaler.List(ctx, &hraList, defaultListOpts...); err != nil { + return nil, err } - return hras, nil + var matchingHRAs []v1alpha1.HorizontalRunnerAutoscaler + + if value == "" { + return matchingHRAs, nil + } + + // For each HRA, resolve its ScaleTargetRef and check if it matches the requested value + for _, hra := range hraList.Items { + if hra.Spec.ScaleTargetRef.Name == "" { + continue + } + + keys := autoscaler.getHRAKeys(ctx, &hra) + for _, key := range keys { + if key == value { + matchingHRAs = append(matchingHRAs, hra) + break + } + } + } + + return matchingHRAs, nil } type ScaleTarget struct { @@ -710,10 +718,36 @@ func (autoscaler *HorizontalRunnerAutoscalerGitHubWebhook) indexer(rawObj client return nil } + // Return a simple key based on the ScaleTargetRef to avoid client calls in indexer + // The actual repository/organization resolution will be done later when needed + kind := hra.Spec.ScaleTargetRef.Kind + if kind == "" { + kind = "RunnerDeployment" // default + } + + key := fmt.Sprintf("%s/%s/%s", kind, hra.Namespace, hra.Spec.ScaleTargetRef.Name) + autoscaler.Log.V(2).Info(fmt.Sprintf("HRA indexed for HRA %s with key: %s", hra.Name, key)) + return []string{key} +} + +func enterpriseKey(name string) string { + return keyPrefixEnterprise + name +} + +func organizationalRunnerGroupKey(owner, group string) string { + return owner + keyRunnerGroup + group +} + +func enterpriseRunnerGroupKey(enterprise, group string) string { + return keyPrefixEnterprise + enterprise + keyRunnerGroup + group +} + +// getHRAKeys resolves the ScaleTargetRef and returns the repository/organization keys for an HRA +func (autoscaler *HorizontalRunnerAutoscalerGitHubWebhook) getHRAKeys(ctx context.Context, hra *v1alpha1.HorizontalRunnerAutoscaler) []string { switch hra.Spec.ScaleTargetRef.Kind { case "", "RunnerDeployment": var rd v1alpha1.RunnerDeployment - if err := autoscaler.Get(context.Background(), types.NamespacedName{Namespace: hra.Namespace, Name: hra.Spec.ScaleTargetRef.Name}, &rd); err != nil { + if err := autoscaler.Get(ctx, 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 } @@ -736,11 +770,10 @@ func (autoscaler *HorizontalRunnerAutoscalerGitHubWebhook) indexer(rawObj client 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.Get(context.Background(), types.NamespacedName{Namespace: hra.Namespace, Name: hra.Spec.ScaleTargetRef.Name}, &rs); err != nil { + if err := autoscaler.Get(ctx, 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 } @@ -761,21 +794,8 @@ func (autoscaler *HorizontalRunnerAutoscalerGitHubWebhook) indexer(rawObj client 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 } - -func enterpriseKey(name string) string { - return keyPrefixEnterprise + name -} - -func organizationalRunnerGroupKey(owner, group string) string { - return owner + keyRunnerGroup + group -} - -func enterpriseRunnerGroupKey(enterprise, group string) string { - return keyPrefixEnterprise + enterprise + keyRunnerGroup + group -} diff --git a/controllers/actions.summerwind.net/horizontal_runner_autoscaler_webhook_test.go b/controllers/actions.summerwind.net/horizontal_runner_autoscaler_webhook_test.go index 22ef62b8..eccf1501 100644 --- a/controllers/actions.summerwind.net/horizontal_runner_autoscaler_webhook_test.go +++ b/controllers/actions.summerwind.net/horizontal_runner_autoscaler_webhook_test.go @@ -9,6 +9,7 @@ import ( "net/http/httptest" "net/url" "os" + "sync" "testing" "time" @@ -418,10 +419,15 @@ func TestGetValidCapacityReservations(t *testing.T) { func installTestLogger(webhook *HorizontalRunnerAutoscalerGitHubWebhook) *bytes.Buffer { logs := &bytes.Buffer{} + + // Wrap the buffer with a synchronized writer to prevent race conditions + syncWriter := &syncWriter{ + writer: logs, + } sink := &testLogSink{ name: "testlog", - writer: logs, + writer: syncWriter, } log := logr.New(sink) @@ -517,6 +523,18 @@ func sendWebhook(server *httptest.Server, eventType string, event interface{}) ( return http.DefaultClient.Do(req) } +// syncWriter wraps an io.Writer with a mutex for thread-safe writes +type syncWriter struct { + writer io.Writer + mu sync.Mutex +} + +func (sw *syncWriter) Write(p []byte) (n int, err error) { + sw.mu.Lock() + defer sw.mu.Unlock() + return sw.writer.Write(p) +} + // testLogSink is a sample logr.Logger that logs in-memory. // It's only for testing log outputs. type testLogSink struct {