From 78c01fd31d3d60cce2be7494352b200e99ef2467 Mon Sep 17 00:00:00 2001 From: Felipe Galindo Sanchez Date: Mon, 16 May 2022 02:38:32 -0700 Subject: [PATCH] cleanup some unused code and minor refactors (#1274) Co-authored-by: Yusuke Kuoka --- controllers/constants.go | 2 -- controllers/horizontal_runner_autoscaler_webhook_on_push.go | 6 +----- controllers/runner_graceful_stop.go | 6 +++--- controllers/runnerdeployment_controller.go | 4 +--- controllers/runnerset_controller.go | 4 +--- 5 files changed, 6 insertions(+), 16 deletions(-) diff --git a/controllers/constants.go b/controllers/constants.go index 24dba05d..d1f5d7b3 100644 --- a/controllers/constants.go +++ b/controllers/constants.go @@ -47,8 +47,6 @@ const ( // A pod that is timed out can be terminated if needed. registrationTimeout = 10 * time.Minute - defaultRegistrationCheckInterval = time.Minute - // DefaultRunnerPodRecreationDelayAfterWebhookScale is the delay until syncing the runners with the desired replicas // after a webhook-based scale up. // This is used to prevent ARC from recreating completed runner pods that are deleted soon without being used at all. diff --git a/controllers/horizontal_runner_autoscaler_webhook_on_push.go b/controllers/horizontal_runner_autoscaler_webhook_on_push.go index 6d1e85f6..2587f31a 100644 --- a/controllers/horizontal_runner_autoscaler_webhook_on_push.go +++ b/controllers/horizontal_runner_autoscaler_webhook_on_push.go @@ -15,10 +15,6 @@ func (autoscaler *HorizontalRunnerAutoscalerGitHubWebhook) MatchPushEvent(event push := g.Push - if push == nil { - return false - } - - return true + return push != nil } } diff --git a/controllers/runner_graceful_stop.go b/controllers/runner_graceful_stop.go index 82fa0322..ab53ae3a 100644 --- a/controllers/runner_graceful_stop.go +++ b/controllers/runner_graceful_stop.go @@ -134,7 +134,7 @@ func ensureRunnerUnregistration(ctx context.Context, retryDelay time.Duration, l "lastState.message", lts.Message, "pod.phase", pod.Status.Phase, ) - } else if ok, err := unregisterRunner(ctx, ghClient, enterprise, organization, repository, runner, *runnerID); err != nil { + } else if ok, err := unregisterRunner(ctx, ghClient, enterprise, organization, repository, *runnerID); err != nil { if errors.Is(err, &gogithub.RateLimitError{}) { // We log the underlying error when we failed calling GitHub API to list or unregisters, // or the runner is still busy. @@ -193,7 +193,7 @@ func ensureRunnerUnregistration(ctx context.Context, retryDelay time.Duration, l // So we can just wait for the completion without actively retrying unregistration. ephemeral := getRunnerEnv(pod, EnvVarEphemeral) if ephemeral == "true" { - pod, err = annotatePodOnce(ctx, c, log, pod, AnnotationKeyRunnerCompletionWaitStartTimestamp, time.Now().Format(time.RFC3339)) + _, err = annotatePodOnce(ctx, c, log, pod, AnnotationKeyRunnerCompletionWaitStartTimestamp, time.Now().Format(time.RFC3339)) if err != nil { return &ctrl.Result{}, err } @@ -370,7 +370,7 @@ func setRunnerEnv(pod *corev1.Pod, key, value string) { // There isn't a single right grace period that works for everyone. // The longer the grace period is, the earlier a cluster resource shortage can occur due to throttoled runner pod deletions, // while the shorter the grace period is, the more likely you may encounter the race issue. -func unregisterRunner(ctx context.Context, client *github.Client, enterprise, org, repo, name string, id int64) (bool, error) { +func unregisterRunner(ctx context.Context, client *github.Client, enterprise, org, repo string, id int64) (bool, error) { // For the record, historically ARC did not try to call RemoveRunner on a busy runner, but it's no longer true. // The reason ARC did so was to let a runner running a job to not stop prematurely. // diff --git a/controllers/runnerdeployment_controller.go b/controllers/runnerdeployment_controller.go index e893f33c..5ebc12f5 100644 --- a/controllers/runnerdeployment_controller.go +++ b/controllers/runnerdeployment_controller.go @@ -421,9 +421,7 @@ func getSelector(rd *v1alpha1.RunnerDeployment) *metav1.LabelSelector { func newRunnerReplicaSet(rd *v1alpha1.RunnerDeployment, commonRunnerLabels []string, scheme *runtime.Scheme) (*v1alpha1.RunnerReplicaSet, error) { newRSTemplate := *rd.Spec.Template.DeepCopy() - for _, l := range commonRunnerLabels { - newRSTemplate.Spec.Labels = append(newRSTemplate.Spec.Labels, l) - } + newRSTemplate.Spec.Labels = append(newRSTemplate.Spec.Labels, commonRunnerLabels...) templateHash := ComputeHash(&newRSTemplate) diff --git a/controllers/runnerset_controller.go b/controllers/runnerset_controller.go index 2cb64c46..5aae774f 100644 --- a/controllers/runnerset_controller.go +++ b/controllers/runnerset_controller.go @@ -188,9 +188,7 @@ var LabelValuePodMutation = "true" func (r *RunnerSetReconciler) newStatefulSet(runnerSet *v1alpha1.RunnerSet) (*appsv1.StatefulSet, error) { runnerSetWithOverrides := *runnerSet.Spec.DeepCopy() - for _, l := range r.CommonRunnerLabels { - runnerSetWithOverrides.Labels = append(runnerSetWithOverrides.Labels, l) - } + runnerSetWithOverrides.Labels = append(runnerSetWithOverrides.Labels, r.CommonRunnerLabels...) template := corev1.Pod{ ObjectMeta: runnerSetWithOverrides.StatefulSetSpec.Template.ObjectMeta,