From 05583522b4de3e1ce041c926a974fb508db8abe3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cl=C3=A9ment=20Loiselet?= Date: Wed, 17 Nov 2021 12:35:56 +0100 Subject: [PATCH] fix(#951): add exception for self-hosted label in webhook search The exception should be temporary, ideally the labels implicitely created (self-hosted, OS, architecture) should be searchable. --- README.md | 2 ++ .../horizontal_runner_autoscaler_webhook.go | 24 ++++++++++++------- 2 files changed, 18 insertions(+), 8 deletions(-) diff --git a/README.md b/README.md index 7461668ae7..aaf486dee0 100644 --- a/README.md +++ b/README.md @@ -610,6 +610,8 @@ spec: duration: "30m" ``` +This webhook requires you to explicitely set the labels in the RunnerDeployment / RunnerSet if you are using them in your workflow to match the agents (field `runs-on`). Only `self-hosted` will be considered as included by default. + You can configure your GitHub webhook settings to only include `Workflows Job` events, so that it sends us three kinds of `workflow_job` events per a job run. Each kind has a `status` of `queued`, `in_progress` and `completed`. With the above configuration, `actions-runner-controller` adds one runner for a `workflow_job` event whose `status` is `queued`. Similarly, it removes one runner for a `workflow_job` event whose `status` is `completed`. The cavaet to this to remember is that this the scale down is within the bounds of your `scaleDownDelaySecondsAfterScaleOut` configuration, if this time hasn't past the scale down will be defered. diff --git a/controllers/horizontal_runner_autoscaler_webhook.go b/controllers/horizontal_runner_autoscaler_webhook.go index 22ab630078..e968513ae7 100644 --- a/controllers/horizontal_runner_autoscaler_webhook.go +++ b/controllers/horizontal_runner_autoscaler_webhook.go @@ -580,13 +580,17 @@ HRA: return nil, err } - if len(labels) == 1 && labels[0] == "self-hosted" { - return &ScaleTarget{HorizontalRunnerAutoscaler: hra, ScaleUpTrigger: v1alpha1.ScaleUpTrigger{Duration: duration}}, nil - } - // Ensure that the RunnerSet-managed runners have all the labels requested by the workflow_job. for _, l := range labels { var matched bool + + // ignore "self-hosted" label as all instance here are self-hosted + if l == "self-hosted" { + continue + } + + // TODO labels related to OS and architecture needs to be explicitely declared or the current implementation will not be able to find them. + for _, l2 := range rs.Spec.Labels { if l == l2 { matched = true @@ -607,13 +611,17 @@ HRA: return nil, err } - if len(labels) == 1 && labels[0] == "self-hosted" { - return &ScaleTarget{HorizontalRunnerAutoscaler: hra, ScaleUpTrigger: v1alpha1.ScaleUpTrigger{Duration: duration}}, nil - } - // Ensure that the RunnerDeployment-managed runners have all the labels requested by the workflow_job. for _, l := range labels { var matched bool + + // ignore "self-hosted" label as all instance here are self-hosted + if l == "self-hosted" { + continue + } + + // TODO labels related to OS and architecture needs to be explicitely declared or the current implementation will not be able to find them. + for _, l2 := range rd.Spec.Template.Spec.Labels { if l == l2 { matched = true