From bc4f4fee1261e095881e38c93e235a599060c51f Mon Sep 17 00:00:00 2001 From: Yusuke Kuoka Date: Fri, 13 Jan 2023 07:14:36 +0900 Subject: [PATCH] Fix various golangci-lint errors (#2147) that we introduced via controller-runtime upgrade and via the removal of legacy pull-based scale triggers (#2001). --- .../horizontal_runner_autoscaler_webhook.go | 85 ------------------- .../integration_test.go | 3 - .../actions.summerwind.net/suite_test.go | 15 ++-- 3 files changed, 6 insertions(+), 97 deletions(-) diff --git a/controllers/actions.summerwind.net/horizontal_runner_autoscaler_webhook.go b/controllers/actions.summerwind.net/horizontal_runner_autoscaler_webhook.go index 093e91e5..e0cee795 100644 --- a/controllers/actions.summerwind.net/horizontal_runner_autoscaler_webhook.go +++ b/controllers/actions.summerwind.net/horizontal_runner_autoscaler_webhook.go @@ -22,7 +22,6 @@ import ( "fmt" "io" "net/http" - "strings" "sync" "time" @@ -333,24 +332,6 @@ func (autoscaler *HorizontalRunnerAutoscalerGitHubWebhook) findHRAsByKey(ctx con return hras, nil } -func matchTriggerConditionAgainstEvent(types []string, eventAction *string) bool { - if len(types) == 0 { - return true - } - - if eventAction == nil { - return false - } - - for _, tpe := range types { - if tpe == *eventAction { - return true - } - } - - return false -} - type ScaleTarget struct { v1alpha1.HorizontalRunnerAutoscaler v1alpha1.ScaleUpTrigger @@ -358,72 +339,6 @@ type ScaleTarget struct { log *logr.Logger } -func (autoscaler *HorizontalRunnerAutoscalerGitHubWebhook) searchScaleTargets(hras []v1alpha1.HorizontalRunnerAutoscaler, f func(v1alpha1.ScaleUpTrigger) bool) []ScaleTarget { - var matched []ScaleTarget - - for _, hra := range hras { - if !hra.ObjectMeta.DeletionTimestamp.IsZero() { - continue - } - - for _, scaleUpTrigger := range hra.Spec.ScaleUpTriggers { - if !f(scaleUpTrigger) { - continue - } - - matched = append(matched, ScaleTarget{ - HorizontalRunnerAutoscaler: hra, - ScaleUpTrigger: scaleUpTrigger, - }) - } - } - - return matched -} - -func (autoscaler *HorizontalRunnerAutoscalerGitHubWebhook) getScaleTarget(ctx context.Context, name string, f func(v1alpha1.ScaleUpTrigger) bool) (*ScaleTarget, error) { - hras, err := autoscaler.findHRAsByKey(ctx, name) - if err != nil { - return nil, err - } - - autoscaler.Log.V(1).Info(fmt.Sprintf("Found %d HRAs by key", len(hras)), "key", name) - - targets := autoscaler.searchScaleTargets(hras, f) - - n := len(targets) - - if n == 0 { - return nil, nil - } - - if n > 1 { - var scaleTargetIDs []string - - for _, t := range targets { - scaleTargetIDs = append(scaleTargetIDs, t.HorizontalRunnerAutoscaler.Name) - } - - autoscaler.Log.Info( - "Found too many scale targets: "+ - "It must be exactly one to avoid ambiguity. "+ - "Either set Namespace for the webhook-based autoscaler to let it only find HRAs in the namespace, "+ - "or update Repository, Organization, or Enterprise fields in your RunnerDeployment resources to fix the ambiguity.", - "scaleTargets", strings.Join(scaleTargetIDs, ",")) - - return nil, nil - } - - return &targets[0], nil -} - -func (autoscaler *HorizontalRunnerAutoscalerGitHubWebhook) getScaleUpTarget(ctx context.Context, log logr.Logger, repo, owner, ownerType, enterprise string, f func(v1alpha1.ScaleUpTrigger) bool) (*ScaleTarget, error) { - scaleTarget := func(value string) (*ScaleTarget, error) { - return autoscaler.getScaleTarget(ctx, value, f) - } - return autoscaler.getScaleUpTargetWithFunction(ctx, log, repo, owner, ownerType, enterprise, scaleTarget) -} - func (autoscaler *HorizontalRunnerAutoscalerGitHubWebhook) getJobScaleUpTargetForRepoOrOrg( ctx context.Context, log logr.Logger, repo, owner, ownerType, enterprise string, labels []string, ) (*ScaleTarget, error) { diff --git a/controllers/actions.summerwind.net/integration_test.go b/controllers/actions.summerwind.net/integration_test.go index b6dd6e32..bf9d8607 100644 --- a/controllers/actions.summerwind.net/integration_test.go +++ b/controllers/actions.summerwind.net/integration_test.go @@ -40,9 +40,6 @@ var ( workflowRunsFor3Replicas = `{"total_count": 5, "workflow_runs":[{"status":"queued"}, {"status":"queued"}, {"status":"in_progress"}, {"status":"in_progress"}, {"status":"completed"}]}"` workflowRunsFor3Replicas_queued = `{"total_count": 2, "workflow_runs":[{"status":"queued"}, {"status":"queued"}]}"` workflowRunsFor3Replicas_in_progress = `{"total_count": 1, "workflow_runs":[{"status":"in_progress"}]}"` - workflowRunsFor1Replicas = `{"total_count": 6, "workflow_runs":[{"status":"queued"}, {"status":"completed"}, {"status":"completed"}, {"status":"completed"}, {"status":"completed"}]}"` - workflowRunsFor1Replicas_queued = `{"total_count": 1, "workflow_runs":[{"status":"queued"}]}"` - workflowRunsFor1Replicas_in_progress = `{"total_count": 0, "workflow_runs":[]}"` ) // SetupIntegrationTest will set up a testing environment. diff --git a/controllers/actions.summerwind.net/suite_test.go b/controllers/actions.summerwind.net/suite_test.go index 442f28d1..6918bd91 100644 --- a/controllers/actions.summerwind.net/suite_test.go +++ b/controllers/actions.summerwind.net/suite_test.go @@ -57,19 +57,16 @@ func TestAPIs(t *testing.T) { var _ = BeforeSuite(func(done Done) { logf.SetLogger(zap.New(zap.UseDevMode(true), zap.WriteTo(GinkgoWriter))) - var apiServerFlags []string - - apiServerFlags = append(apiServerFlags, envtest.DefaultKubeAPIServerFlags...) - // Avoids the following error: - // 2021-03-19T15:14:11.673+0900 ERROR controller-runtime.controller Reconciler error {"controller": "testns-tvjzjrunner", "request": "testns-gdnyx/example-runnerdeploy-zps4z-j5562", "error": "Pod \"example-runnerdeploy-zps4z-j5562\" is invalid: [spec.containers[1].image: Required value, spec.containers[1].securityContext.privileged: Forbidden: disallowed by cluster policy]"} - apiServerFlags = append(apiServerFlags, "--allow-privileged=true") - By("bootstrapping test environment") testEnv = &envtest.Environment{ - CRDDirectoryPaths: []string{filepath.Join("../..", "config", "crd", "bases")}, - KubeAPIServerFlags: apiServerFlags, + CRDDirectoryPaths: []string{filepath.Join("../..", "config", "crd", "bases")}, } + // Avoids the following error: + // 2021-03-19T15:14:11.673+0900 ERROR controller-runtime.controller Reconciler error {"controller": "testns-tvjzjrunner", "request": "testns-gdnyx/example-runnerdeploy-zps4z-j5562", "error": "Pod \"example-runnerdeploy-zps4z-j5562\" is invalid: [spec.containers[1].image: Required value, spec.containers[1].securityContext.privileged: Forbidden: disallowed by cluster policy]"} + testEnv.ControlPlane.GetAPIServer().Configure(). + Append("allow-privileged", "true") + var err error cfg, err = testEnv.Start() Expect(err).ToNot(HaveOccurred())