chore: bump chart (#1138)

There is a race condition between ARC and GitHub service about deleting runner pod.

- The ARC use REST API to find a particular runner in a pod that is not running any jobs, so it decides to delete the pod.
- A job is queued on the GitHub service side, and it sends the job to this idle runner right before ARC deletes the pod.
- The ARC delete the runner pod which cause the in-progress job to end up canceled.

To avoid this race condition, I am calling `r.unregisterRunner()` before deleting the pod.
- `r.unregisterRunner()` will return 204 to indicate the runner is deleted from the GitHub service, we should be safe to delete the pod.
- `r.unregisterRunner` will return 400 to indicate the runner is still running a job, so we will leave this runner pod as it is.

TODO: I need to do some E2E tests to force the race condition to happen.

Ref #911

.github/workflows/on-push-lint-charts.yml

@@ -10,7 +10,7 @@ on:
   workflow_dispatch:
 env:
   KUBE_SCORE_VERSION: 1.10.0
-  HELM_VERSION: v3.4.1
+  HELM_VERSION: v3.8.0
 
 jobs:
   lint-test:
@@ -23,7 +23,7 @@ jobs:
           fetch-depth: 0
 
       - name: Set up Helm
-        uses: azure/setup-helm@v1
+        uses: azure/setup-helm@v2.0
         with:
           version: ${{ env.HELM_VERSION }}
 

.github/workflows/on-push-master-publish-chart.yml

@@ -13,7 +13,7 @@ on:
 
 env:
   KUBE_SCORE_VERSION: 1.10.0
-  HELM_VERSION: v3.4.1
+  HELM_VERSION: v3.8.0
 
 jobs:
   lint-chart:
@@ -28,7 +28,7 @@ jobs:
           fetch-depth: 0
 
       - name: Set up Helm
-        uses: azure/setup-helm@v1
+        uses: azure/setup-helm@v2.0
         with:
           version: ${{ env.HELM_VERSION }}
 

.github/workflows/release.yml

@@ -18,6 +18,10 @@ jobs:
       - name: Checkout
         uses: actions/checkout@v2
 
+      - uses: actions/setup-go@v2
+        with:
+          go-version: '^1.17.7'
+
       - name: Install tools
         run: |
           curl -L -O https://github.com/kubernetes-sigs/kubebuilder/releases/download/v2.2.0/kubebuilder_2.2.0_linux_amd64.tar.gz

.github/workflows/test.yaml

@@ -24,7 +24,7 @@ jobs:
       uses: actions/checkout@v2
     - uses: actions/setup-go@v2
       with:
-        go-version: '^1.17.5'
+        go-version: '^1.17.7'
     - run: go version
     - name: Install kubebuilder
       run: |

README.md

@@ -1071,6 +1071,20 @@ spec:
       group: NewGroup
 ```
 
+GitHub supports custom visilibity in a Runner Group to make it available to a specific set of repositories only. By default if no GitHub
+authentication is included in the GitHub webhook server it will be assumed that all runner groups to be usable in all repositories.
+Supporting custom visibility requires to do a few GitHub API calls to find out what are the potential runner groups that are visible to
+the webhook's repository, this may incur in increased API rate limiting when using github.com
+
+This option will be enabled when proper GitHub authentication options (token, app or basic auth is provided) in the GitHub webhook server and `useRunnerGroupsVisibility` is set to true, e.g.
+
+```yaml
+githubWebhookServer:
+  enabled: false
+  replicaCount: 1
+  useRunnerGroupsVisibility: true
+```
+
 ### Runner Entrypoint Features
 
 > Environment variable values must all be strings

acceptance/deploy.sh

@@ -6,6 +6,8 @@ tpe=${ACCEPTANCE_TEST_SECRET_TYPE}
 
 VALUES_FILE=${VALUES_FILE:-$(dirname $0)/values.yaml}
 
+kubectl delete secret controller-manager || :
+
 if [ "${tpe}" == "token" ]; then
   if ! kubectl get secret controller-manager -n actions-runner-system >/dev/null; then
     kubectl create secret generic controller-manager \
@@ -23,6 +25,16 @@ else
   exit 1
 fi
 
+if [ -n "${WEBHOOK_GITHUB_TOKEN}" ]; then
+  kubectl -n actions-runner-system delete secret \
+      github-webhook-server || :
+  kubectl -n actions-runner-system create secret generic \
+      github-webhook-server \
+      --from-literal=github_token=${WEBHOOK_GITHUB_TOKEN:?WEBHOOK_GITHUB_TOKEN must not be empty}
+else
+  echo 'Skipped deploying secret "github-webhook-server". Set WEBHOOK_GITHUB_TOKEN to deploy.' 1>&2
+fi
+
 tool=${ACCEPTANCE_TEST_DEPLOYMENT_TOOL}
 
 if [ "${tool}" == "helm" ]; then
@@ -35,7 +47,9 @@ if [ "${tool}" == "helm" ]; then
     --set image.repository=${NAME} \
     --set image.tag=${VERSION} \
     -f ${VALUES_FILE}
-  kubectl apply -f charts/actions-runner-controller/crds
+  # To prevent `CustomResourceDefinition.apiextensions.k8s.io "runners.actions.summerwind.dev" is invalid: metadata.annotations: Too long: must have at most 262144 bytes`
+  # errors
+  kubectl create -f charts/actions-runner-controller/crds || kubectl replace -f charts/actions-runner-controller/crds
   kubectl -n actions-runner-system wait deploy/actions-runner-controller --for condition=available --timeout 60s
 else
   kubectl apply \
@@ -50,7 +64,7 @@ sleep 20
 RUNNER_LABEL=${RUNNER_LABEL:-self-hosted}
 
 if [ -n "${TEST_REPO}" ]; then
-  if [ -n "USE_RUNNERSET" ]; then
+  if [ "${USE_RUNNERSET}" -ne "false" ]; then
       cat acceptance/testdata/repo.runnerset.yaml | envsubst | kubectl apply -f -
       cat acceptance/testdata/repo.runnerset.hra.yaml | envsubst | kubectl apply -f -
   else
@@ -63,13 +77,25 @@ else
 fi
 
 if [ -n "${TEST_ORG}" ]; then
-  cat acceptance/testdata/org.runnerdeploy.yaml | envsubst | kubectl apply -f -
+  cat acceptance/testdata/runnerdeploy.envsubst.yaml | TEST_ENTERPRISE= TEST_REPO= NAME=org-runnerdeploy envsubst | kubectl apply -f -
 
-  if [ -n "${TEST_ORG_REPO}" ]; then
-    cat acceptance/testdata/org.hra.yaml | envsubst | kubectl apply -f -
+  if [ -n "${TEST_ORG_GROUP}" ]; then
+    cat acceptance/testdata/runnerdeploy.envsubst.yaml | TEST_ENTERPRISE= TEST_REPO= TEST_GROUP=${TEST_ORG_GROUP} NAME=orggroup-runnerdeploy envsubst | kubectl apply -f -
   else
-    echo 'Skipped deploying organizational hra. Set TEST_ORG_REPO to "yourorg/yourrepo" to deploy.'
+    echo 'Skipped deploying enterprise runnerdeployment. Set TEST_ORG_GROUP to deploy.'
   fi
 else
   echo 'Skipped deploying organizational runnerdeployment. Set TEST_ORG to deploy.'
 fi
+
+if [ -n "${TEST_ENTERPRISE}" ]; then
+  cat acceptance/testdata/runnerdeploy.envsubst.yaml | TEST_ORG= TEST_REPO= NAME=enterprise-runnerdeploy envsubst | kubectl apply -f -
+
+  if [ -n "${TEST_ENTERPRISE_GROUP}" ]; then
+    cat acceptance/testdata/runnerdeploy.envsubst.yaml | TEST_ORG= TEST_REPO= TEST_GROUP=${TEST_ENTERPRISE_GROUP} NAME=enterprisegroup-runnerdeploy envsubst | kubectl apply -f -
+  else
+    echo 'Skipped deploying enterprise runnerdeployment. Set TEST_ENTERPRISE_GROUP to deploy.'
+  fi
+else
+  echo 'Skipped deploying enterprise runnerdeployment. Set TEST_ENTERPRISE to deploy.'
+fi

acceptance/testdata/org.runnerdeploy.yaml

@@ -14,6 +14,11 @@ spec:
       image: ${RUNNER_NAME}:${RUNNER_TAG}
       imagePullPolicy: IfNotPresent
 
+      # Whether to pass --ephemeral (true) or --once (false, deprecated)
+      env:
+      - name: RUNNER_FEATURE_FLAG_EPHEMERAL
+        value: "${RUNNER_FEATURE_FLAG_EPHEMERAL}"
+
       #
       # dockerd within runner container
       #
@@ -30,6 +35,8 @@ spec:
       # labels:
       # - "mylabel 1"
       # - "mylabel 2"
+      labels:
+      - "${RUNNER_LABEL}"
 
       #
       # Non-standard working directory

acceptance/testdata/repo.runnerdeploy.yaml

@@ -14,6 +14,11 @@ spec:
       image: ${RUNNER_NAME}:${RUNNER_TAG}
       imagePullPolicy: IfNotPresent
 
+      # Whether to pass --ephemeral (true) or --once (false, deprecated)
+      env:
+      - name: RUNNER_FEATURE_FLAG_EPHEMERAL
+        value: "${RUNNER_FEATURE_FLAG_EPHEMERAL}"
+
       #
       # dockerd within runner container
       #
@@ -30,6 +35,8 @@ spec:
       # labels:
       # - "mylabel 1"
       # - "mylabel 2"
+      labels:
+      - "${RUNNER_LABEL}"
 
       #
       # Non-standard working directory

acceptance/testdata/runnerdeploy.envsubst.yaml

@@ -0,0 +1,61 @@
+apiVersion: actions.summerwind.dev/v1alpha1
+kind: RunnerDeployment
+metadata:
+  name: ${NAME}
+spec:
+#  replicas: 1
+  template:
+    spec:
+      enterprise: ${TEST_ENTERPRISE}
+      group: ${TEST_GROUP}
+      organization: ${TEST_ORG}
+      repository: ${TEST_REPO}
+
+      #
+      # Custom runner image
+      #
+      image: ${RUNNER_NAME}:${RUNNER_TAG}
+      imagePullPolicy: IfNotPresent
+
+      # Whether to pass --ephemeral (true) or --once (false, deprecated)
+      env:
+      - name: RUNNER_FEATURE_FLAG_EPHEMERAL
+        value: "${RUNNER_FEATURE_FLAG_EPHEMERAL}"
+
+      #
+      # dockerd within runner container
+      #
+      ## Replace `mumoshu/actions-runner-dind:dev` with your dind image
+      #dockerdWithinRunnerContainer: true
+      #image: mumoshu/actions-runner-dind:dev
+
+      #
+      # Set the MTU used by dockerd-managed network interfaces (including docker-build-ubuntu)
+      #
+      #dockerMTU: 1450
+
+      #Runner group
+      # labels:
+      # - "mylabel 1"
+      # - "mylabel 2"
+      labels:
+      - "${RUNNER_LABEL}"
+
+      #
+      # Non-standard working directory
+      #
+      # workDir: "/"
+---
+apiVersion: actions.summerwind.dev/v1alpha1
+kind: HorizontalRunnerAutoscaler
+metadata:
+  name: ${NAME}
+spec:
+  scaleTargetRef:
+    name: ${NAME}
+  scaleUpTriggers:
+  - githubEvent: {}
+    amount: 1
+    duration: "1m"
+  minReplicas: 0
+  maxReplicas: 10

acceptance/values.yaml

@@ -1,12 +1,15 @@
 # Set actions-runner-controller settings for testing
 githubAPICacheDuration: 10s
 githubWebhookServer:
+  logLevel: debug
   enabled: true
   labels: {}
   replicaCount: 1
   syncPeriod: 10m
+  useRunnerGroupsVisibility: true
   secret:
-    create: true
+    enabled: true
+    # create: true
     name: "github-webhook-server"
     ### GitHub Webhook Configuration
     #github_webhook_secret_token: ""

charts/actions-runner-controller/Chart.yaml

@@ -15,10 +15,10 @@ type: application
 # This is the chart version. This version number should be incremented each time you make changes
 # to the chart and its templates, including the app version.
 # Versions are expected to follow Semantic Versioning (https://semver.org/)
-version: 0.15.3
+version: 0.16.1
 
 # Used as the default manager tag value when no tag property is provided in the values.yaml
-appVersion: 0.20.4
+appVersion: 0.21.1
 
 home: https://github.com/actions-runner-controller/actions-runner-controller
 

charts/actions-runner-controller/README.md

@@ -75,8 +75,10 @@ All additional docs are kept in the `docs/` folder, this README is solely for do
 | `admissionWebHooks.caBundle`                             | Base64-encoded PEM bundle containing the CA that signed the webhook's serving certificate                                  |                                                                      |
 | `githubWebhookServer.logLevel`                           | Set the log level of the githubWebhookServer container                                                                     |                                                                      |
 | `githubWebhookServer.replicaCount`                       | Set the number of webhook server pods                                                                                      | 1                                                                    |
+| `githubWebhookServer.useRunnerGroupsVisibility`          | Enable supporting runner groups with custom visibility. This will incur in extra API calls and may blow up your budget. Currently, you also need to set `githubWebhookServer.secret.enabled` to enable this feature. | false                                                                |
 | `githubWebhookServer.syncPeriod`                         | Set the period in which the controller reconciles the resources                                                            | 10m                                                                  |
 | `githubWebhookServer.enabled`                            | Deploy the webhook server pod                                                                                              | false                                                                |
+| `githubWebhookServer.secret.enabled`                      | Passes the webhook hook secret to the github-webhook-server                                                                             | false                                                                |
 | `githubWebhookServer.secret.create`                      | Deploy the webhook hook secret                                                                                             | false                                                                |
 | `githubWebhookServer.secret.name`                        | Set the name of the webhook hook secret                                                                                    | github-webhook-server                                                |
 | `githubWebhookServer.secret.github_webhook_secret_token` | Set the webhook secret token value                                                                                         |                                                                      |

charts/actions-runner-controller/templates/_helpers.tpl

@@ -68,6 +68,10 @@ Create the name of the service account to use
 {{- default (include "actions-runner-controller.fullname" .) .Values.authSecret.name -}}
 {{- end }}
 
+{{- define "actions-runner-controller.githubWebhookServerSecretName" -}}
+{{- default (include "actions-runner-controller.fullname" .) .Values.githubWebhookServer.secret.name -}}
+{{- end }}
+
 {{- define "actions-runner-controller.leaderElectionRoleName" -}}
 {{- include "actions-runner-controller.fullname" . }}-leader-election
 {{- end }}

charts/actions-runner-controller/templates/githubwebhook.deployment.yaml

@@ -69,30 +69,30 @@ spec:
         - name: GITHUB_UPLOAD_URL
           value: {{ .Values.githubUploadURL }}
         {{- end }}
-        {{- if .Values.authSecret.enabled }}
+        {{- if and .Values.githubWebhookServer.useRunnerGroupsVisibility .Values.githubWebhookServer.secret.enabled }}
         - name: GITHUB_TOKEN
           valueFrom:
             secretKeyRef:
               key: github_token
-              name: {{ include "actions-runner-controller.secretName" . }}
+              name: {{ include "actions-runner-controller.githubWebhookServerSecretName" . }}
               optional: true
         - name: GITHUB_APP_ID
           valueFrom:
             secretKeyRef:
               key: github_app_id
-              name: {{ include "actions-runner-controller.secretName" . }}
+              name: {{ include "actions-runner-controller.githubWebhookServerSecretName" . }}
               optional: true
         - name: GITHUB_APP_INSTALLATION_ID
           valueFrom:
             secretKeyRef:
               key: github_app_installation_id
-              name: {{ include "actions-runner-controller.secretName" . }}
+              name: {{ include "actions-runner-controller.githubWebhookServerSecretName" . }}
               optional: true
         - name: GITHUB_APP_PRIVATE_KEY
           valueFrom:
             secretKeyRef:
               key: github_app_private_key
-              name: {{ include "actions-runner-controller.secretName" . }}
+              name: {{ include "actions-runner-controller.githubWebhookServerSecretName" . }}
               optional: true
         {{- if .Values.authSecret.github_basicauth_username  }}
         - name: GITHUB_BASICAUTH_USERNAME

charts/actions-runner-controller/values.yaml

@@ -169,7 +169,9 @@ githubWebhookServer:
   enabled: false
   replicaCount: 1
   syncPeriod: 10m
+  useRunnerGroupsVisibility: false
   secret:
+    enabled: false
     create: false
     name: "github-webhook-server"
     ### GitHub Webhook Configuration

cmd/githubwebhookserver/main.go

@@ -130,6 +130,8 @@ func main() {
 		switch logLevel {
 		case logLevelDebug:
 			o.Development = true
+			lvl := zaplib.NewAtomicLevelAt(-2) // maps to logr's V(2)
+			o.Level = &lvl
 		case logLevelInfo:
 			lvl := zaplib.NewAtomicLevelAt(zaplib.InfoLevel)
 			o.Level = &lvl
@@ -142,6 +144,13 @@ func main() {
 		}
 	})
 
+	ctrl.SetLogger(logger)
+
+	// In order to support runner groups with custom visibility (selected repositories), we need to perform some GitHub API calls.
+	// Let the user define if they want to opt-in supporting this option by providing the proper GitHub authentication parameters
+	// Without an opt-in, runner groups with custom visibility won't be supported to save API calls
+	// That is, all runner groups managed by ARC are assumed to be visible to any repositories,
+	// which is wrong when you have one or more non-default runner groups in your organization or enterprise.
 	if len(c.Token) > 0 || (c.AppID > 0 && c.AppInstallationID > 0 && c.AppPrivateKey != "") || (len(c.BasicauthUsername) > 0 && len(c.BasicauthPassword) > 0) {
 		ghClient, err = c.NewClient()
 		if err != nil {
@@ -149,10 +158,10 @@ func main() {
 			setupLog.Error(err, "unable to create controller", "controller", "Runner")
 			os.Exit(1)
 		}
+	} else {
+		setupLog.Info("GitHub client is not initialized. Runner groups with custom visibility are not supported. If needed, please provide GitHub authentication. This will incur in extra GitHub API calls")
 	}
 
-	ctrl.SetLogger(logger)
-
 	mgr, err := ctrl.NewManager(ctrl.GetConfigOrDie(), ctrl.Options{
 		Scheme:             scheme,
 		SyncPeriod:         &syncPeriod,
@@ -167,8 +176,9 @@ func main() {
 	}
 
 	hraGitHubWebhook := &controllers.HorizontalRunnerAutoscalerGitHubWebhook{
+		Name:           "webhookbasedautoscaler",
 		Client:         mgr.GetClient(),
-		Log:            ctrl.Log.WithName("controllers").WithName("Runner"),
+		Log:            ctrl.Log.WithName("controllers").WithName("webhookbasedautoscaler"),
 		Recorder:       nil,
 		Scheme:         mgr.GetScheme(),
 		SecretKeyBytes: []byte(webhookSecretToken),
@@ -177,7 +187,7 @@ func main() {
 	}
 
 	if err = hraGitHubWebhook.SetupWithManager(mgr); err != nil {
-		setupLog.Error(err, "unable to create controller", "controller", "Runner")
+		setupLog.Error(err, "unable to create controller", "controller", "webhookbasedautoscaler")
 		os.Exit(1)
 	}
 

controllers/horizontal_runner_autoscaler_webhook.go

@@ -38,6 +38,7 @@ import (
 
 	"github.com/actions-runner-controller/actions-runner-controller/api/v1alpha1"
 	"github.com/actions-runner-controller/actions-runner-controller/github"
+	"github.com/actions-runner-controller/actions-runner-controller/simulator"
 )
 
 const (
@@ -92,7 +93,7 @@ func (autoscaler *HorizontalRunnerAutoscalerGitHubWebhook) Handle(w http.Respons
 			if err != nil {
 				msg := err.Error()
 				if written, err := w.Write([]byte(msg)); err != nil {
-					autoscaler.Log.Error(err, "failed writing http error response", "msg", msg, "written", written)
+					autoscaler.Log.V(1).Error(err, "failed writing http error response", "msg", msg, "written", written)
 				}
 			}
 		}
@@ -289,7 +290,7 @@ func (autoscaler *HorizontalRunnerAutoscalerGitHubWebhook) Handle(w http.Respons
 	}
 
 	if target == nil {
-		log.Info(
+		log.V(1).Info(
 			"Scale target not found. If this is unexpected, ensure that there is exactly one repository-wide or organizational runner deployment that matches this webhook event",
 		)
 
@@ -476,95 +477,105 @@ func (autoscaler *HorizontalRunnerAutoscalerGitHubWebhook) getScaleUpTargetWithF
 		return nil, nil
 	}
 
-	// Search for organization runner HRAs in default runner group
-	if target, err := scaleTarget(owner); err != nil {
-		log.Error(err, "finding organizational runner", "organization", owner)
-		return nil, err
-	} else if target != nil {
-		log.Info("job scale up target is organizational runners", "organization", owner)
-		return target, nil
-	}
-
-	if enterprise != "" {
-		// Search for enterprise runner HRAs in default runner group
-		if target, err := scaleTarget(enterpriseKey(enterprise)); err != nil {
-			log.Error(err, "finding enterprise runner", "enterprise", enterprise)
-			return nil, err
-		} else if target != nil {
-			log.Info("scale up target is default enterprise runners", "enterprise", enterprise)
-			return target, nil
-		}
-	}
-
-	// At this point there were no default organization/enterprise runners available to use, try now
-	// searching in runner groups
-
-	// We need to get the potential runner groups first to avoid spending API queries needless. Once/if GitHub improves an
+	// Find the potential runner groups first to avoid spending API queries needless. Once/if GitHub improves an
 	// API to find related/linked runner groups from a specific repository this logic could be removed
-	availableEnterpriseGroups, availableOrganizationGroups, err := autoscaler.getPotentialGroupsFromHRAs(ctx, enterprise, owner)
+	managedRunnerGroups, err := autoscaler.getManagedRunnerGroupsFromHRAs(ctx, enterprise, owner)
 	if err != nil {
-		log.Error(err, "finding potential organization runner groups from HRAs", "organization", owner)
+		log.Error(err, "finding potential organization/enterprise runner groups from HRAs", "organization", owner)
 		return nil, err
 	}
-	if len(availableEnterpriseGroups) == 0 && len(availableOrganizationGroups) == 0 {
+	if managedRunnerGroups.IsEmpty() {
 		log.V(1).Info("no repository/organizational/enterprise runner found",
 			"repository", repositoryRunnerKey,
 			"organization", owner,
 			"enterprises", enterprise,
 		)
+	} else {
+		log.V(1).Info("Found some runner groups are managed by ARC", "groups", managedRunnerGroups)
 	}
 
-	var enterpriseGroups []string
-	var organizationGroups []string
+	var visibleGroups *simulator.VisibleRunnerGroups
 	if autoscaler.GitHubClient != nil {
+		simu := &simulator.Simulator{
+			Client: autoscaler.GitHubClient,
+		}
 		// Get available organization runner groups and enterprise runner groups for a repository
-		// These are the sum of runner groups with repository access = All repositories plus
-		// runner groups where owner/repo has access to
-		enterpriseGroups, organizationGroups, err = autoscaler.GitHubClient.GetRunnerGroupsFromRepository(ctx, owner, repositoryRunnerKey, availableEnterpriseGroups, availableOrganizationGroups)
-		log.V(1).Info("Searching in runner groups", "enterprise.groups", enterpriseGroups, "organization.groups", organizationGroups)
+		// These are the sum of runner groups with repository access = All repositories and runner groups
+		// where owner/repo has access to as well. The list will include default runner group also if it has access to
+		visibleGroups, err = simu.GetRunnerGroupsVisibleToRepository(ctx, owner, repositoryRunnerKey, managedRunnerGroups)
+		log.V(1).Info("Searching in runner groups", "groups", visibleGroups)
 		if err != nil {
 			log.Error(err, "Unable to find runner groups from repository", "organization", owner, "repository", repo)
-			return nil, nil
+			return nil, fmt.Errorf("error while finding visible runner groups: %v", err)
 		}
 	} else {
 		// For backwards compatibility if GitHub authentication is not configured, we assume all runner groups have
 		// visibility=all to honor the previous implementation, therefore any available enterprise/organization runner
-		// is a potential target for scaling
-		enterpriseGroups = availableEnterpriseGroups
-		organizationGroups = availableOrganizationGroups
+		// is a potential target for scaling. This will also avoid doing extra API calls caused by
+		// GitHubClient.GetRunnerGroupsVisibleToRepository in case users are not using custom visibility on their runner
+		// groups or they are using only default runner groups
+		visibleGroups = managedRunnerGroups
 	}
 
-	for _, group := range organizationGroups {
-		if target, err := scaleTarget(organizationalRunnerGroupKey(owner, group)); err != nil {
-			log.Error(err, "finding organizational runner group", "organization", owner)
-			return nil, err
-		} else if target != nil {
-			log.Info(fmt.Sprintf("job scale up target is organizational runner group %s", target.Name), "organization", owner)
-			return target, nil
+	scaleTargetKey := func(rg simulator.RunnerGroup) string {
+		switch rg.Kind {
+		case simulator.Default:
+			switch rg.Scope {
+			case simulator.Organization:
+				return owner
+			case simulator.Enterprise:
+				return enterpriseKey(enterprise)
+			}
+		case simulator.Custom:
+			switch rg.Scope {
+			case simulator.Organization:
+				return organizationalRunnerGroupKey(owner, rg.Name)
+			case simulator.Enterprise:
+				return enterpriseRunnerGroupKey(enterprise, rg.Name)
+			}
 		}
+		return ""
 	}
 
-	for _, group := range enterpriseGroups {
-		if target, err := scaleTarget(enterpriseRunnerGroupKey(enterprise, group)); err != nil {
-			log.Error(err, "finding enterprise runner group", "enterprise", owner)
-			return nil, err
-		} else if target != nil {
-			log.Info(fmt.Sprintf("job scale up target is enterprise runner group %s", target.Name), "enterprise", owner)
-			return target, nil
+	log.V(1).Info("groups", "groups", visibleGroups)
+
+	var t *ScaleTarget
+
+	traverseErr := visibleGroups.Traverse(func(rg simulator.RunnerGroup) (bool, error) {
+		key := scaleTargetKey(rg)
+
+		target, err := scaleTarget(key)
+
+		if err != nil {
+			log.Error(err, "finding runner group", "enterprise", enterprise, "organization", owner, "repository", repo, "key", key)
+			return false, err
+		} else if target == nil {
+			return false, nil
 		}
+
+		t = target
+		log.V(1).Info("job scale up target found", "enterprise", enterprise, "organization", owner, "repository", repo, "key", key)
+
+		return true, nil
+	})
+
+	if traverseErr != nil {
+		return nil, err
 	}
 
-	log.V(1).Info("no repository/organizational/enterprise runner found",
-		"repository", repositoryRunnerKey,
-		"organization", owner,
-		"enterprises", enterprise,
-	)
-	return nil, nil
+	if t == nil {
+		log.V(1).Info("no repository/organizational/enterprise runner found",
+			"repository", repositoryRunnerKey,
+			"organization", owner,
+			"enterprise", enterprise,
+		)
+	}
+
+	return t, nil
 }
 
-func (autoscaler *HorizontalRunnerAutoscalerGitHubWebhook) getPotentialGroupsFromHRAs(ctx context.Context, enterprise, org string) ([]string, []string, error) {
-	var enterpriseRunnerGroups []string
-	var orgRunnerGroups []string
+func (autoscaler *HorizontalRunnerAutoscalerGitHubWebhook) getManagedRunnerGroupsFromHRAs(ctx context.Context, enterprise, org string) (*simulator.VisibleRunnerGroups, error) {
+	groups := simulator.NewVisibleRunnerGroups()
 	ns := autoscaler.Namespace
 
 	var defaultListOpts []client.ListOption
@@ -579,36 +590,63 @@ func (autoscaler *HorizontalRunnerAutoscalerGitHubWebhook) getPotentialGroupsFro
 
 	var hraList v1alpha1.HorizontalRunnerAutoscalerList
 	if err := autoscaler.List(ctx, &hraList, opts...); err != nil {
-		return orgRunnerGroups, enterpriseRunnerGroups, err
+		return groups, err
 	}
 
 	for _, hra := range hraList.Items {
-		switch hra.Spec.ScaleTargetRef.Kind {
+		var o, e, g string
+
+		kind := hra.Spec.ScaleTargetRef.Kind
+		switch kind {
 		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 {
-				return orgRunnerGroups, enterpriseRunnerGroups, err
-			}
-			if rs.Spec.Organization == org && rs.Spec.Group != "" {
-				orgRunnerGroups = append(orgRunnerGroups, rs.Spec.Group)
-			}
-			if rs.Spec.Enterprise == enterprise && rs.Spec.Group != "" {
-				enterpriseRunnerGroups = append(enterpriseRunnerGroups, rs.Spec.Group)
+				return groups, err
 			}
+			o, e, g = rs.Spec.Organization, rs.Spec.Enterprise, rs.Spec.Group
 		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 {
-				return orgRunnerGroups, enterpriseRunnerGroups, err
-			}
-			if rd.Spec.Template.Spec.Organization == org && rd.Spec.Template.Spec.Group != "" {
-				orgRunnerGroups = append(orgRunnerGroups, rd.Spec.Template.Spec.Group)
-			}
-			if rd.Spec.Template.Spec.Enterprise == enterprise && rd.Spec.Template.Spec.Group != "" {
-				enterpriseRunnerGroups = append(enterpriseRunnerGroups, rd.Spec.Template.Spec.Group)
+				return groups, err
 			}
+			o, e, g = rd.Spec.Template.Spec.Organization, rd.Spec.Template.Spec.Enterprise, rd.Spec.Template.Spec.Group
+		default:
+			return nil, fmt.Errorf("unsupported scale target kind: %v", kind)
+		}
+
+		if g != "" && e == "" && o == "" {
+			autoscaler.Log.V(1).Info(
+				"invalid runner group config in scale target: spec.group must be set along with either spec.enterprise or spec.organization",
+				"scaleTargetKind", kind,
+				"group", g,
+				"enterprise", e,
+				"organization", o,
+			)
+
+			continue
+		}
+
+		if e != enterprise && o != org {
+			autoscaler.Log.V(1).Info(
+				"Skipped scale target irrelevant to event",
+				"eventOrganization", org,
+				"eventEnterprise", enterprise,
+				"scaleTargetKind", kind,
+				"scaleTargetGroup", g,
+				"scaleTargetEnterprise", e,
+				"scaleTargetOrganization", o,
+			)
+
+			continue
+		}
+
+		rg := simulator.NewRunnerGroupFromProperties(e, o, g)
+
+		if err := groups.Add(rg); err != nil {
+			return groups, fmt.Errorf("failed adding visible group from HRA %s/%s: %w", hra.Namespace, hra.Name, err)
 		}
 	}
-	return enterpriseRunnerGroups, orgRunnerGroups, nil
+	return groups, nil
 }
 
 func (autoscaler *HorizontalRunnerAutoscalerGitHubWebhook) getJobScaleTarget(ctx context.Context, name string, labels []string) (*ScaleTarget, error) {
@@ -820,7 +858,7 @@ func (autoscaler *HorizontalRunnerAutoscalerGitHubWebhook) SetupWithManager(mgr
 					keys = append(keys, enterpriseKey(enterprise)) // Enterprise runners
 				}
 			}
-			autoscaler.Log.V(1).Info(fmt.Sprintf("HRA keys indexed for HRA %s: %v", hra.Name, keys))
+			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
@@ -845,7 +883,7 @@ func (autoscaler *HorizontalRunnerAutoscalerGitHubWebhook) SetupWithManager(mgr
 					keys = append(keys, enterpriseRunnerGroupKey(enterprise, rs.Spec.Group)) // Enterprise runner groups
 				}
 			}
-			autoscaler.Log.V(1).Info(fmt.Sprintf("HRA keys indexed for HRA %s: %v", hra.Name, keys))
+			autoscaler.Log.V(2).Info(fmt.Sprintf("HRA keys indexed for HRA %s: %v", hra.Name, keys))
 			return keys
 		}
 

controllers/integration_test.go

@@ -1077,6 +1077,169 @@ var _ = Context("INTEGRATION: Inside of a new namespace", func() {
 			}
 		})
 
+		It("should be able to scale visible organization runner group with default labels", func() {
+			name := "example-runnerdeploy"
+
+			{
+				rd := &actionsv1alpha1.RunnerDeployment{
+					ObjectMeta: metav1.ObjectMeta{
+						Name:      name,
+						Namespace: ns.Name,
+					},
+					Spec: actionsv1alpha1.RunnerDeploymentSpec{
+						Replicas: intPtr(1),
+						Selector: &metav1.LabelSelector{
+							MatchLabels: map[string]string{
+								"foo": "bar",
+							},
+						},
+						Template: actionsv1alpha1.RunnerTemplate{
+							ObjectMeta: metav1.ObjectMeta{
+								Labels: map[string]string{
+									"foo": "bar",
+								},
+							},
+							Spec: actionsv1alpha1.RunnerSpec{
+								RunnerConfig: actionsv1alpha1.RunnerConfig{
+									Repository: "test/valid",
+									Image:      "bar",
+									Group:      "baz",
+								},
+								RunnerPodSpec: actionsv1alpha1.RunnerPodSpec{
+									Env: []corev1.EnvVar{
+										{Name: "FOO", Value: "FOOVALUE"},
+									},
+								},
+							},
+						},
+					},
+				}
+
+				ExpectCreate(ctx, rd, "test RunnerDeployment")
+
+				hra := &actionsv1alpha1.HorizontalRunnerAutoscaler{
+					ObjectMeta: metav1.ObjectMeta{
+						Name:      name,
+						Namespace: ns.Name,
+					},
+					Spec: actionsv1alpha1.HorizontalRunnerAutoscalerSpec{
+						ScaleTargetRef: actionsv1alpha1.ScaleTargetRef{
+							Name: name,
+						},
+						MinReplicas:                       intPtr(1),
+						MaxReplicas:                       intPtr(5),
+						ScaleDownDelaySecondsAfterScaleUp: intPtr(1),
+						ScaleUpTriggers: []actionsv1alpha1.ScaleUpTrigger{
+							{
+								GitHubEvent: &actionsv1alpha1.GitHubEventScaleUpTriggerSpec{},
+								Amount:      1,
+								Duration:    metav1.Duration{Duration: time.Minute},
+							},
+						},
+					},
+				}
+
+				ExpectCreate(ctx, hra, "test HorizontalRunnerAutoscaler")
+
+				ExpectRunnerSetsCountEventuallyEquals(ctx, ns.Name, 1)
+				ExpectRunnerSetsManagedReplicasCountEventuallyEquals(ctx, ns.Name, 1)
+			}
+
+			{
+				env.ExpectRegisteredNumberCountEventuallyEquals(1, "count of fake list runners")
+			}
+
+			// Scale-up to 2 replicas on first workflow_job webhook event
+			{
+				env.SendWorkflowJobEvent("test", "valid", "pending", "created", []string{"self-hosted"})
+				ExpectRunnerSetsCountEventuallyEquals(ctx, ns.Name, 1, "runner sets after webhook")
+				ExpectRunnerSetsManagedReplicasCountEventuallyEquals(ctx, ns.Name, 2, "runners after first webhook event")
+				env.ExpectRegisteredNumberCountEventuallyEquals(2, "count of fake list runners")
+			}
+		})
+
+		It("should be able to scale visible organization runner group with custom labels", func() {
+			name := "example-runnerdeploy"
+
+			{
+				rd := &actionsv1alpha1.RunnerDeployment{
+					ObjectMeta: metav1.ObjectMeta{
+						Name:      name,
+						Namespace: ns.Name,
+					},
+					Spec: actionsv1alpha1.RunnerDeploymentSpec{
+						Replicas: intPtr(1),
+						Selector: &metav1.LabelSelector{
+							MatchLabels: map[string]string{
+								"foo": "bar",
+							},
+						},
+						Template: actionsv1alpha1.RunnerTemplate{
+							ObjectMeta: metav1.ObjectMeta{
+								Labels: map[string]string{
+									"foo": "bar",
+								},
+							},
+							Spec: actionsv1alpha1.RunnerSpec{
+								RunnerConfig: actionsv1alpha1.RunnerConfig{
+									Repository: "test/valid",
+									Image:      "bar",
+									Group:      "baz",
+									Labels:     []string{"custom-label"},
+								},
+								RunnerPodSpec: actionsv1alpha1.RunnerPodSpec{
+									Env: []corev1.EnvVar{
+										{Name: "FOO", Value: "FOOVALUE"},
+									},
+								},
+							},
+						},
+					},
+				}
+
+				ExpectCreate(ctx, rd, "test RunnerDeployment")
+
+				hra := &actionsv1alpha1.HorizontalRunnerAutoscaler{
+					ObjectMeta: metav1.ObjectMeta{
+						Name:      name,
+						Namespace: ns.Name,
+					},
+					Spec: actionsv1alpha1.HorizontalRunnerAutoscalerSpec{
+						ScaleTargetRef: actionsv1alpha1.ScaleTargetRef{
+							Name: name,
+						},
+						MinReplicas:                       intPtr(1),
+						MaxReplicas:                       intPtr(5),
+						ScaleDownDelaySecondsAfterScaleUp: intPtr(1),
+						ScaleUpTriggers: []actionsv1alpha1.ScaleUpTrigger{
+							{
+								GitHubEvent: &actionsv1alpha1.GitHubEventScaleUpTriggerSpec{},
+								Amount:      1,
+								Duration:    metav1.Duration{Duration: time.Minute},
+							},
+						},
+					},
+				}
+
+				ExpectCreate(ctx, hra, "test HorizontalRunnerAutoscaler")
+
+				ExpectRunnerSetsCountEventuallyEquals(ctx, ns.Name, 1)
+				ExpectRunnerSetsManagedReplicasCountEventuallyEquals(ctx, ns.Name, 1)
+			}
+
+			{
+				env.ExpectRegisteredNumberCountEventuallyEquals(1, "count of fake list runners")
+			}
+
+			// Scale-up to 2 replicas on first workflow_job webhook event
+			{
+				env.SendWorkflowJobEvent("test", "valid", "pending", "created", []string{"custom-label"})
+				ExpectRunnerSetsCountEventuallyEquals(ctx, ns.Name, 1, "runner sets after webhook")
+				ExpectRunnerSetsManagedReplicasCountEventuallyEquals(ctx, ns.Name, 2, "runners after first webhook event")
+				env.ExpectRegisteredNumberCountEventuallyEquals(2, "count of fake list runners")
+			}
+		})
+
 	})
 })
 
@@ -1208,6 +1371,28 @@ func (env *testEnvironment) SendUserCheckRunEvent(owner, repo, status, action st
 	ExpectWithOffset(1, resp.StatusCode).To(Equal(200))
 }
 
+func (env *testEnvironment) SendWorkflowJobEvent(owner, repo, status, action string, labels []string) {
+	resp, err := sendWebhook(env.webhookServer, "workflow_job", &github.WorkflowJobEvent{
+		Org: &github.Organization{
+			Name: github.String(owner),
+		},
+		WorkflowJob: &github.WorkflowJob{
+			Labels: labels,
+		},
+		Action: github.String("queued"),
+		Repo: &github.Repository{
+			Name: github.String(repo),
+			Owner: &github.User{
+				Login: github.String(owner),
+				Type:  github.String("Organization"),
+			},
+		},
+	})
+
+	ExpectWithOffset(1, err).NotTo(HaveOccurred(), "failed to send check_run event")
+
+	ExpectWithOffset(1, resp.StatusCode).To(Equal(200))
+}
 func (env *testEnvironment) SyncRunnerRegistrations() {
 	var runnerList actionsv1alpha1.RunnerList
 

controllers/runner_controller.go

@@ -404,14 +404,31 @@ func (r *RunnerReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctr
 		return ctrl.Result{}, nil
 	}
 
-	// Delete current pod if recreation is needed
-	if err := r.Delete(ctx, &pod); err != nil {
-		log.Error(err, "Failed to delete pod resource")
-		return ctrl.Result{}, err
+	// Try to delete current pod if recreation is needed
+	safeToDeletePod := false
+	ok, err := r.unregisterRunner(ctx, runner.Spec.Enterprise, runner.Spec.Organization, runner.Spec.Repository, runner.Name)
+	if err != nil {
+		log.Error(err, "Failed to unregister runner before deleting the pod.", "runner", runner.Name)
+	} else {
+		// `r.unregisterRunner()` will returns `false, nil` if the runner is not found on GitHub.
+		if !ok {
+			log.Info("Runner no longer exists on GitHub", "runner", runner.Name)
+		}
+
+		safeToDeletePod = true
 	}
 
-	r.Recorder.Event(&runner, corev1.EventTypeNormal, "PodDeleted", fmt.Sprintf("Deleted pod '%s'", newPod.Name))
-	log.Info("Deleted runner pod", "repository", runner.Spec.Repository)
+	if safeToDeletePod {
+		// Only delete the pod if we successfully unregistered the runner or the runner is already deleted from the service.
+		// This should help us avoid race condition between runner pickup job after we think the runner is not busy.
+		if err := r.Delete(ctx, &pod); err != nil {
+			log.Error(err, "Failed to delete pod resource")
+			return ctrl.Result{}, err
+		}
+
+		r.Recorder.Event(&runner, corev1.EventTypeNormal, "PodDeleted", fmt.Sprintf("Deleted pod '%s'", newPod.Name))
+		log.Info("Deleted runner pod", "repository", runner.Spec.Repository)
+	}
 
 	return ctrl.Result{}, nil
 }
@@ -531,32 +548,15 @@ func (r *RunnerReconciler) processRunnerCreation(ctx context.Context, runner v1a
 	return ctrl.Result{}, nil
 }
 
+// unregisterRunner unregisters the runner from GitHub Actions by name.
+//
+// This function returns:
+// - (true, nil) when it has successfully unregistered the runner.
+// - (false, nil) when the runner has been already unregistered.
+// - (false, err) when it postponed unregistration due to the runner being busy, or it tried to unregister the runner but failed due to
+//   an error returned by GitHub API.
 func (r *RunnerReconciler) unregisterRunner(ctx context.Context, enterprise, org, repo, name string) (bool, error) {
-	runners, err := r.GitHubClient.ListRunners(ctx, enterprise, org, repo)
-	if err != nil {
-		return false, err
-	}
-
-	id := int64(0)
-	for _, runner := range runners {
-		if runner.GetName() == name {
-			if runner.GetBusy() {
-				return false, fmt.Errorf("runner is busy")
-			}
-			id = runner.GetID()
-			break
-		}
-	}
-
-	if id == int64(0) {
-		return false, nil
-	}
-
-	if err := r.GitHubClient.RemoveRunner(ctx, enterprise, org, repo, id); err != nil {
-		return false, err
-	}
-
-	return true, nil
+	return unregisterRunner(ctx, r.GitHubClient, enterprise, org, repo, name)
 }
 
 func (r *RunnerReconciler) updateRegistrationToken(ctx context.Context, runner v1alpha1.Runner) (bool, error) {
@@ -626,6 +626,11 @@ func (r *RunnerReconciler) newPod(runner v1alpha1.Runner) (corev1.Pod, error) {
 		runner.ObjectMeta.Annotations,
 		runner.Spec,
 		r.GitHubClient.GithubBaseURL,
+		// Token change should trigger replacement.
+		// We need to include this explicitly here because
+		// runner.Spec does not contain the possibly updated token stored in the
+		// runner status yet.
+		runner.Status.Registration.Token,
 	)
 
 	objectMeta := metav1.ObjectMeta{

controllers/runner_pod_controller.go

@@ -378,42 +378,7 @@ func (r *RunnerPodReconciler) Reconcile(ctx context.Context, req ctrl.Request) (
 }
 
 func (r *RunnerPodReconciler) unregisterRunner(ctx context.Context, enterprise, org, repo, name string) (bool, error) {
-	runners, err := r.GitHubClient.ListRunners(ctx, enterprise, org, repo)
-	if err != nil {
-		return false, err
-	}
-
-	var busy bool
-
-	id := int64(0)
-	for _, runner := range runners {
-		if runner.GetName() == name {
-			// Sometimes a runner can stuck "busy" even though it is already "offline".
-			// Thus removing the condition on status can block the runner pod from being terminated forever.
-			busy = runner.GetBusy()
-			if runner.GetStatus() != "offline" && busy {
-				r.Log.Info("This runner will delay the runner pod deletion and the runner deregistration until it becomes either offline or non-busy", "name", runner.GetName(), "status", runner.GetStatus(), "busy", runner.GetBusy())
-				return false, fmt.Errorf("runner is busy")
-			}
-			id = runner.GetID()
-			break
-		}
-	}
-
-	if id == int64(0) {
-		return false, nil
-	}
-
-	// Sometimes a runner can stuck "busy" even though it is already "offline".
-	// Trying to remove the offline but busy runner can result in errors like the following:
-	//    failed to remove runner: DELETE https://api.github.com/repos/actions-runner-controller/mumoshu-actions-test/actions/runners/47: 422 Bad request - Runner \"example-runnerset-0\" is still running a job\" []
-	if !busy {
-		if err := r.GitHubClient.RemoveRunner(ctx, enterprise, org, repo, id); err != nil {
-			return false, err
-		}
-	}
-
-	return true, nil
+	return unregisterRunner(ctx, r.GitHubClient, enterprise, org, repo, name)
 }
 
 func (r *RunnerPodReconciler) SetupWithManager(mgr ctrl.Manager) error {

controllers/unregister.go

@@ -0,0 +1,49 @@
+package controllers
+
+import (
+	"context"
+	"fmt"
+
+	"github.com/actions-runner-controller/actions-runner-controller/github"
+)
+
+// unregisterRunner unregisters the runner from GitHub Actions by name.
+//
+// This function returns:
+// - (true, nil) when it has successfully unregistered the runner.
+// - (false, nil) when the runner has been already unregistered.
+// - (false, err) when it postponed unregistration due to the runner being busy, or it tried to unregister the runner but failed due to
+//   an error returned by GitHub API.
+func unregisterRunner(ctx context.Context, client *github.Client, enterprise, org, repo, name string) (bool, error) {
+	runners, err := client.ListRunners(ctx, enterprise, org, repo)
+	if err != nil {
+		return false, err
+	}
+
+	id := int64(0)
+	for _, runner := range runners {
+		if runner.GetName() == name {
+			// Note that sometimes a runner can stuck "busy" even though it is already "offline".
+			// But we assume that it's not actually offline and still running a job.
+			if runner.GetBusy() {
+				return false, fmt.Errorf("runner is busy")
+			}
+			id = runner.GetID()
+			break
+		}
+	}
+
+	if id == int64(0) {
+		return false, nil
+	}
+
+	// Trying to remove a busy runner can result in errors like the following:
+	//    failed to remove runner: DELETE https://api.github.com/repos/actions-runner-controller/mumoshu-actions-test/actions/runners/47: 422 Bad request - Runner \"example-runnerset-0\" is still running a job\" []
+	//
+	// TODO: Probably we can just remove the runner by ID without seeing if the runner is busy, by treating it as busy when a remove-runner call failed with 422?
+	if err := client.RemoveRunner(ctx, enterprise, org, repo, id); err != nil {
+		return false, err
+	}
+
+	return true, nil
+}

github/github.go

@@ -224,74 +224,9 @@ func (c *Client) ListRunners(ctx context.Context, enterprise, org, repo string)
 	return runners, nil
 }
 
-func (c *Client) GetRunnerGroupsFromRepository(ctx context.Context, org, repo string, potentialEnterpriseGroups []string, potentialOrgGroups []string) ([]string, []string, error) {
-
-	var enterpriseRunnerGroups []string
-	var orgRunnerGroups []string
-
-	if org != "" {
-		runnerGroups, err := c.getOrganizationRunnerGroups(ctx, org, repo)
-		if err != nil {
-			return enterpriseRunnerGroups, orgRunnerGroups, err
-		}
-		for _, runnerGroup := range runnerGroups {
-			if runnerGroup.GetInherited() { // enterprise runner groups
-				if !containsString(potentialEnterpriseGroups, runnerGroup.GetName()) {
-					continue
-				}
-				if runnerGroup.GetVisibility() == "all" {
-					enterpriseRunnerGroups = append(enterpriseRunnerGroups, runnerGroup.GetName())
-				} else {
-					hasAccess, err := c.hasRepoAccessToOrganizationRunnerGroup(ctx, org, runnerGroup.GetID(), repo)
-					if err != nil {
-						return enterpriseRunnerGroups, orgRunnerGroups, err
-					}
-					if hasAccess {
-						enterpriseRunnerGroups = append(enterpriseRunnerGroups, runnerGroup.GetName())
-					}
-				}
-			} else { // organization runner groups
-				if !containsString(potentialOrgGroups, runnerGroup.GetName()) {
-					continue
-				}
-				if runnerGroup.GetVisibility() == "all" {
-					orgRunnerGroups = append(orgRunnerGroups, runnerGroup.GetName())
-				} else {
-					hasAccess, err := c.hasRepoAccessToOrganizationRunnerGroup(ctx, org, runnerGroup.GetID(), repo)
-					if err != nil {
-						return enterpriseRunnerGroups, orgRunnerGroups, err
-					}
-					if hasAccess {
-						orgRunnerGroups = append(orgRunnerGroups, runnerGroup.GetName())
-					}
-				}
-			}
-		}
-	}
-	return enterpriseRunnerGroups, orgRunnerGroups, nil
-}
-
-func (c *Client) hasRepoAccessToOrganizationRunnerGroup(ctx context.Context, org string, runnerGroupId int64, repo string) (bool, error) {
-	opts := github.ListOptions{PerPage: 100}
-	for {
-		list, res, err := c.Client.Actions.ListRepositoryAccessRunnerGroup(ctx, org, runnerGroupId, &opts)
-		if err != nil {
-			return false, fmt.Errorf("failed to list repository access for runner group: %w", err)
-		}
-		for _, githubRepo := range list.Repositories {
-			if githubRepo.GetFullName() == repo {
-				return true, nil
-			}
-		}
-		if res.NextPage == 0 {
-			break
-		}
-		opts.Page = res.NextPage
-	}
-	return false, nil
-}
-
-func (c *Client) getOrganizationRunnerGroups(ctx context.Context, org, repo string) ([]*github.RunnerGroup, error) {
+// ListOrganizationRunnerGroups returns all the runner groups defined in the organization and
+// inherited to the organization from an enterprise.
+func (c *Client) ListOrganizationRunnerGroups(ctx context.Context, org string) ([]*github.RunnerGroup, error) {
 	var runnerGroups []*github.RunnerGroup
 
 	opts := github.ListOptions{PerPage: 100}
@@ -311,6 +246,27 @@ func (c *Client) getOrganizationRunnerGroups(ctx context.Context, org, repo stri
 	return runnerGroups, nil
 }
 
+func (c *Client) ListRunnerGroupRepositoryAccesses(ctx context.Context, org string, runnerGroupId int64) ([]*github.Repository, error) {
+	var repos []*github.Repository
+
+	opts := github.ListOptions{PerPage: 100}
+	for {
+		list, res, err := c.Client.Actions.ListRepositoryAccessRunnerGroup(ctx, org, runnerGroupId, &opts)
+		if err != nil {
+			return nil, fmt.Errorf("failed to list repository access for runner group: %w", err)
+		}
+
+		repos = append(repos, list.Repositories...)
+		if res.NextPage == 0 {
+			break
+		}
+
+		opts.Page = res.NextPage
+	}
+
+	return repos, nil
+}
+
 // cleanup removes expired registration tokens.
 func (c *Client) cleanup() {
 	c.mu.Lock()
@@ -480,12 +436,3 @@ func (r *Client) IsRunnerBusy(ctx context.Context, enterprise, org, repo, name s
 
 	return false, &RunnerNotFound{runnerName: name}
 }
-
-func containsString(list []string, value string) bool {
-	for _, item := range list {
-		if item == value {
-			return true
-		}
-	}
-	return false
-}

go.mod

@@ -13,6 +13,7 @@ require (
 	github.com/onsi/ginkgo v1.16.5
 	github.com/onsi/gomega v1.17.0
 	github.com/prometheus/client_golang v1.11.0
+	github.com/stretchr/testify v1.7.0
 	github.com/teambition/rrule-go v1.7.2
 	go.uber.org/zap v1.20.0
 	golang.org/x/oauth2 v0.0.0-20211104180415-d3ed0bb246c8
@@ -47,6 +48,7 @@ require (
 	github.com/modern-go/reflect2 v1.0.2 // indirect
 	github.com/nxadm/tail v1.4.8 // indirect
 	github.com/pkg/errors v0.9.1 // indirect
+	github.com/pmezard/go-difflib v1.0.0 // indirect
 	github.com/prometheus/client_model v0.2.0 // indirect
 	github.com/prometheus/common v0.28.0 // indirect
 	github.com/prometheus/procfs v0.6.0 // indirect

go.sum

@@ -508,7 +508,6 @@ go.uber.org/multierr v1.6.0/go.mod h1:cdWPpRnG4AhwMwsgIHip0KRBQjJy5kYEpYjJxpXp9i
 go.uber.org/zap v1.10.0/go.mod h1:vwi/ZaCAaUcBkycHslxD9B2zi4UTXhF60s6SWpuDF0Q=
 go.uber.org/zap v1.17.0/go.mod h1:MXVU+bhUf/A7Xi2HNOnopQOrmycQ5Ih87HtOu4q5SSo=
 go.uber.org/zap v1.19.0/go.mod h1:xg/QME4nWcxGxrpdeYfq7UvYrLh66cuVKdrbD1XF/NI=
-go.uber.org/zap v1.19.1 h1:ue41HOKd1vGURxrmeKIgELGb3jPW9DMUDGtsinblHwI=
 go.uber.org/zap v1.19.1/go.mod h1:j3DNczoxDZroyBnOT1L/Q79cfUMGZxlv/9dzN7SM1rI=
 go.uber.org/zap v1.20.0 h1:N4oPlghZwYG55MlU6LXk/Zp00FVNE9X9wrYO8CEs4lc=
 go.uber.org/zap v1.20.0/go.mod h1:wjWOCqI0f2ZZrJF/UufIOkiC8ii6tm1iqIsLo76RfJw=

simulator/runnergroup_visibility.go

@@ -0,0 +1,63 @@
+package simulator
+
+import (
+	"context"
+	"fmt"
+
+	"github.com/actions-runner-controller/actions-runner-controller/github"
+)
+
+type Simulator struct {
+	Client *github.Client
+}
+
+func (c *Simulator) GetRunnerGroupsVisibleToRepository(ctx context.Context, org, repo string, managed *VisibleRunnerGroups) (*VisibleRunnerGroups, error) {
+	visible := NewVisibleRunnerGroups()
+
+	if org == "" {
+		panic(fmt.Sprintf("BUG: owner should not be empty in this context. repo=%v", repo))
+	}
+
+	runnerGroups, err := c.Client.ListOrganizationRunnerGroups(ctx, org)
+	if err != nil {
+		return visible, err
+	}
+
+	for _, runnerGroup := range runnerGroups {
+		ref := NewRunnerGroupFromGitHub(runnerGroup)
+
+		if !managed.Includes(ref) {
+			continue
+		}
+
+		if runnerGroup.GetVisibility() != "all" {
+			hasAccess, err := c.hasRepoAccessToOrganizationRunnerGroup(ctx, org, runnerGroup.GetID(), repo)
+			if err != nil {
+				return visible, err
+			}
+
+			if !hasAccess {
+				continue
+			}
+		}
+
+		visible.Add(ref)
+	}
+
+	return visible, nil
+}
+
+func (c *Simulator) hasRepoAccessToOrganizationRunnerGroup(ctx context.Context, org string, runnerGroupId int64, repo string) (bool, error) {
+	repos, err := c.Client.ListRunnerGroupRepositoryAccesses(ctx, org, runnerGroupId)
+	if err != nil {
+		return false, err
+	}
+
+	for _, githubRepo := range repos {
+		if githubRepo.GetFullName() == repo {
+			return true, nil
+		}
+	}
+
+	return false, nil
+}

simulator/runnergroups.go

@@ -0,0 +1,194 @@
+package simulator
+
+import (
+	"fmt"
+	"sort"
+	"strings"
+
+	"github.com/google/go-github/v39/github"
+)
+
+type RunnerGroupScope int
+
+const (
+	Organization RunnerGroupScope = iota
+	Enterprise
+)
+
+func (s RunnerGroupScope) String() string {
+	switch s {
+	case Organization:
+		return "Organization"
+	case Enterprise:
+		return "Enterprise"
+	default:
+		panic(fmt.Sprintf("unimplemented RunnerGroupScope: %v", int(s)))
+	}
+}
+
+type RunnerGroupKind int
+
+const (
+	Default RunnerGroupKind = iota
+	Custom
+)
+
+func (s RunnerGroupKind) String() string {
+	switch s {
+	case Default:
+		return "Default"
+	case Custom:
+		return "Custom"
+	default:
+		panic(fmt.Sprintf("unimplemented RunnerGroupKind: %v", int(s)))
+	}
+}
+
+func NewRunnerGroupFromGitHub(g *github.RunnerGroup) RunnerGroup {
+	var name string
+	if !g.GetDefault() {
+		name = g.GetName()
+	}
+
+	var scope RunnerGroupScope
+
+	if g.GetInherited() {
+		scope = Enterprise
+	} else {
+		scope = Organization
+	}
+
+	return newRunnerGroup(scope, name)
+}
+
+func NewRunnerGroupFromProperties(enterprise, organization, group string) RunnerGroup {
+	var scope RunnerGroupScope
+
+	if enterprise != "" {
+		scope = Enterprise
+	} else {
+		scope = Organization
+	}
+
+	return newRunnerGroup(scope, group)
+}
+
+// newRunnerGroup creates a new RunnerGroup instance from the provided arguments.
+// There's a convention that an empty name implies a default runner group.
+func newRunnerGroup(scope RunnerGroupScope, name string) RunnerGroup {
+	if name == "" {
+		return RunnerGroup{
+			Scope: scope,
+			Kind:  Default,
+			Name:  "",
+		}
+	}
+
+	return RunnerGroup{
+		Scope: scope,
+		Kind:  Custom,
+		Name:  name,
+	}
+}
+
+type RunnerGroup struct {
+	Scope RunnerGroupScope
+	Kind  RunnerGroupKind
+	Name  string
+}
+
+func (r RunnerGroup) String() string {
+	return fmt.Sprintf("RunnerGroup{Scope:%s, Kind:%s, Name:%s}", r.Scope, r.Kind, r.Name)
+}
+
+// VisibleRunnerGroups is a set of enterprise and organization runner groups
+// that are visible to a GitHub repository.
+// GitHub Actions chooses one of such visible group on which the workflow job is scheduled.
+// ARC chooses the same group as Actions as the scale target.
+type VisibleRunnerGroups struct {
+	// sortedGroups is a pointer to a mutable list of RunnerGroups that contains all the runner sortedGroups
+	// that are visible to the repository, including organization sortedGroups defined at the organization level,
+	// and enterprise sortedGroups that are inherited down to the organization.
+	sortedGroups []RunnerGroup
+}
+
+func NewVisibleRunnerGroups() *VisibleRunnerGroups {
+	return &VisibleRunnerGroups{}
+}
+
+func (g *VisibleRunnerGroups) String() string {
+	var gs []string
+	for _, g := range g.sortedGroups {
+		gs = append(gs, g.String())
+	}
+
+	return strings.Join(gs, ", ")
+}
+
+func (g *VisibleRunnerGroups) IsEmpty() bool {
+	return len(g.sortedGroups) == 0
+}
+
+func (r *VisibleRunnerGroups) Includes(ref RunnerGroup) bool {
+	for _, r := range r.sortedGroups {
+		if r.Scope == ref.Scope && r.Kind == ref.Kind && r.Name == ref.Name {
+			return true
+		}
+	}
+	return false
+}
+
+// Add adds a runner group into VisibleRunnerGroups
+// at a certain position in the list so that
+// Traverse can return runner groups in order of higher precedence to lower precedence.
+func (g *VisibleRunnerGroups) Add(rg RunnerGroup) error {
+	n := len(g.sortedGroups)
+	i := sort.Search(n, func(i int) bool {
+		data := g.sortedGroups[i]
+
+		if rg.Kind > data.Kind {
+			return false
+		} else if rg.Kind < data.Kind {
+			return true
+		}
+
+		if rg.Scope > data.Scope {
+			return false
+		} else if rg.Scope < data.Scope {
+			return true
+		}
+
+		return false
+	})
+
+	g.insert(rg, i)
+
+	return nil
+}
+
+func (g *VisibleRunnerGroups) insert(rg RunnerGroup, i int) {
+	var result []RunnerGroup
+
+	result = append(result, g.sortedGroups[:i]...)
+	result = append(result, rg)
+	result = append(result, g.sortedGroups[i:]...)
+
+	g.sortedGroups = result
+}
+
+// Traverse traverses all the runner groups visible to a repository
+// in order of higher precedence to lower precedence.
+func (g *VisibleRunnerGroups) Traverse(f func(RunnerGroup) (bool, error)) error {
+	for _, rg := range g.sortedGroups {
+		ok, err := f(rg)
+		if err != nil {
+			return err
+		}
+
+		if ok {
+			return nil
+		}
+	}
+
+	return nil
+}

simulator/runnergroups_test.go

@@ -0,0 +1,94 @@
+package simulator
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/require"
+)
+
+func TestVisibleRunnerGroupsInsert(t *testing.T) {
+	g := NewVisibleRunnerGroups()
+
+	orgDefault := NewRunnerGroupFromProperties("", "myorg1", "")
+	orgCustom := NewRunnerGroupFromProperties("", "myorg1", "myorg1group1")
+	enterpriseDefault := NewRunnerGroupFromProperties("myenterprise1", "", "")
+
+	g.insert(orgCustom, 0)
+	g.insert(orgDefault, 0)
+	g.insert(enterpriseDefault, 1)
+
+	var got []RunnerGroup
+
+	err := g.Traverse(func(rg RunnerGroup) (bool, error) {
+		got = append(got, rg)
+		return false, nil
+	})
+
+	require.NoError(t, err)
+	require.Equal(t, []RunnerGroup{orgDefault, enterpriseDefault, orgCustom}, got, "Unexpected result")
+}
+
+func TestVisibleRunnerGroups(t *testing.T) {
+	v := NewVisibleRunnerGroups()
+
+	requireGroups := func(t *testing.T, included, notIncluded []RunnerGroup) {
+		t.Helper()
+
+		for _, rg := range included {
+			if !v.Includes(rg) {
+				t.Errorf("%v must be included", rg)
+			}
+		}
+
+		for _, rg := range notIncluded {
+			if v.Includes(rg) {
+				t.Errorf("%v must not be included", rg)
+			}
+		}
+
+		var got []RunnerGroup
+
+		err := v.Traverse(func(rg RunnerGroup) (bool, error) {
+			got = append(got, rg)
+
+			return false, nil
+		})
+
+		require.NoError(t, err)
+		require.Equal(t, included, got)
+	}
+
+	orgDefault := NewRunnerGroupFromProperties("", "myorg1", "")
+	orgCustom := NewRunnerGroupFromProperties("", "myorg1", "myorg1group1")
+	enterpriseDefault := NewRunnerGroupFromProperties("myenterprise1", "", "")
+	enterpriseCustom := NewRunnerGroupFromProperties("myenterprise1", "", "myenterprise1group1")
+
+	requireGroups(t, nil, []RunnerGroup{orgDefault, enterpriseDefault, orgCustom, enterpriseCustom})
+
+	v.Add(orgCustom)
+
+	requireGroups(t, []RunnerGroup{orgCustom}, []RunnerGroup{orgDefault, enterpriseDefault, enterpriseCustom})
+
+	v.Add(orgDefault)
+
+	requireGroups(t, []RunnerGroup{orgDefault, orgCustom}, []RunnerGroup{enterpriseDefault, enterpriseCustom})
+
+	v.Add(enterpriseCustom)
+
+	requireGroups(t, []RunnerGroup{orgDefault, orgCustom, enterpriseCustom}, []RunnerGroup{enterpriseDefault})
+
+	v.Add(enterpriseDefault)
+
+	requireGroups(t, []RunnerGroup{orgDefault, enterpriseDefault, orgCustom, enterpriseCustom}, nil)
+
+	var first []RunnerGroup
+
+	err := v.Traverse(func(rg RunnerGroup) (bool, error) {
+		first = append(first, rg)
+
+		return true, nil
+	})
+
+	require.NoError(t, err)
+	require.Equal(t, []RunnerGroup{orgDefault}, first)
+}

test/e2e/cmd/main.go

@@ -0,0 +1,63 @@
+package main
+
+import (
+	"bufio"
+	"bytes"
+	"fmt"
+	"log"
+	"os"
+	"os/exec"
+	"strings"
+)
+
+func main() {
+	if err := run(); err != nil {
+		fmt.Fprintf(os.Stderr, "%v\n", err)
+		os.Exit(1)
+	}
+}
+
+func run() error {
+	var configMapNames []string
+
+	output, err := output()
+	if err != nil {
+		log.Printf("Command failed with output: %s", string(output))
+		return err
+	}
+
+	s := bufio.NewScanner(bytes.NewBuffer(output))
+
+	for s.Scan() {
+		if t := s.Text(); strings.Contains(t, "test-info") || strings.Contains(t, "test-result-") {
+			configMapNames = append(configMapNames, s.Text())
+		}
+	}
+
+	for _, n := range configMapNames {
+		println(n)
+
+		if output, err := delete(n); err != nil {
+			log.Printf("Command failed with output: %s", string(output))
+			return err
+		}
+	}
+
+	return nil
+}
+
+func output() ([]byte, error) {
+	cmd := exec.Command("kubectl", "get", "cm", "-o", `jsonpath={range .items[*]}{.metadata.name}{"\n"}{end}`)
+	data, err := cmd.CombinedOutput()
+	return data, err
+}
+
+func delete(cmName string) ([]byte, error) {
+	cmd := exec.Command("kubectl", "delete", "cm", cmName)
+	return cmd.CombinedOutput()
+}
+
+func deleteControllerManagerSecret() ([]byte, error) {
+	cmd := exec.Command("kubectl", "-n", "actions-runner-system", "delete", "secret", "controller-manager")
+	return cmd.CombinedOutput()
+}

test/e2e/e2e_test.go

@@ -4,6 +4,7 @@ import (
 	"context"
 	"fmt"
 	"path/filepath"
+	"strconv"
 	"time"
 
 	"github.com/actions-runner-controller/actions-runner-controller/testing"
@@ -166,7 +167,11 @@ type env struct {
 	useRunnerSet bool
 
 	testID                                                   string
+	repoToCommit                                             string
 	runnerLabel, githubToken, testRepo, testOrg, testOrgRepo string
+	githubTokenWebhook                                       string
+	testEnterprise                                           string
+	featureFlagEphemeral                                     bool
 	testJobs                                                 []job
 }
 
@@ -186,10 +191,15 @@ func initTestEnv(t *testing.T) *env {
 	e.testID = testID
 	e.runnerLabel = "test-" + id
 	e.githubToken = testing.Getenv(t, "GITHUB_TOKEN")
-	e.testRepo = testing.Getenv(t, "TEST_REPO")
-	e.testOrg = testing.Getenv(t, "TEST_ORG")
-	e.testOrgRepo = testing.Getenv(t, "TEST_ORG_REPO")
-	e.testJobs = createTestJobs(id, testResultCMNamePrefix, 2)
+	e.githubTokenWebhook = testing.Getenv(t, "WEBHOOK_GITHUB_TOKEN")
+	e.repoToCommit = testing.Getenv(t, "TEST_COMMIT_REPO")
+	e.testRepo = testing.Getenv(t, "TEST_REPO", "")
+	e.testOrg = testing.Getenv(t, "TEST_ORG", "")
+	e.testOrgRepo = testing.Getenv(t, "TEST_ORG_REPO", "")
+	e.testEnterprise = testing.Getenv(t, "TEST_ENTERPRISE")
+	e.testJobs = createTestJobs(id, testResultCMNamePrefix, 10)
+	ephemeral, _ := strconv.ParseBool(testing.Getenv(t, "TEST_FEATURE_FLAG_EPHEMERAL"))
+	e.featureFlagEphemeral = ephemeral
 
 	return e
 }
@@ -237,11 +247,14 @@ func (e *env) installActionsRunnerController(t *testing.T) {
 	}
 
 	varEnv := []string{
+		"TEST_ENTERPRISE=" + e.testEnterprise,
 		"TEST_REPO=" + e.testRepo,
 		"TEST_ORG=" + e.testOrg,
 		"TEST_ORG_REPO=" + e.testOrgRepo,
 		"GITHUB_TOKEN=" + e.githubToken,
+		"WEBHOOK_GITHUB_TOKEN=" + e.githubTokenWebhook,
 		"RUNNER_LABEL=" + e.runnerLabel,
+		fmt.Sprintf("RUNNER_FEATURE_FLAG_EPHEMERAL=%v", e.featureFlagEphemeral),
 	}
 
 	scriptEnv = append(scriptEnv, varEnv...)
@@ -260,7 +273,7 @@ func (e *env) createControllerNamespaceAndServiceAccount(t *testing.T) {
 func (e *env) installActionsWorkflow(t *testing.T) {
 	t.Helper()
 
-	installActionsWorkflow(t, e.testID, e.runnerLabel, testResultCMNamePrefix, e.testRepo, e.testJobs)
+	installActionsWorkflow(t, e.testID, e.runnerLabel, testResultCMNamePrefix, e.repoToCommit, e.testJobs)
 }
 
 func (e *env) verifyActionsWorkflowRun(t *testing.T) {
@@ -287,6 +300,8 @@ func createTestJobs(id, testResultCMNamePrefix string, numJobs int) []job {
 	return testJobs
 }
 
+const Branch = "main"
+
 func installActionsWorkflow(t *testing.T, testID, runnerLabel, testResultCMNamePrefix, testRepo string, testJobs []job) {
 	t.Helper()
 
@@ -298,7 +313,7 @@ func installActionsWorkflow(t *testing.T, testID, runnerLabel, testResultCMNameP
 		Name: wfName,
 		On: testing.On{
 			Push: &testing.Push{
-				Branches: []string{"master"},
+				Branches: []string{Branch},
 			},
 		},
 		Jobs: map[string]testing.Job{},
@@ -346,6 +361,7 @@ kubectl create cm %s$id --from-literal=status=ok
 			".github/workflows/workflow.yaml": wfContent,
 			"test.sh":                         script,
 		},
+		Branch: Branch,
 	}
 
 	if err := g.Sync(ctx); err != nil {

testing/getenv.go

@@ -5,12 +5,15 @@ import (
 	"testing"
 )
 
-func Getenv(t *testing.T, name string) string {
+func Getenv(t *testing.T, name string, opts ...string) string {
 	t.Helper()
 
 	v := os.Getenv(name)
 	if v == "" {
-		t.Fatal(name + " must be set")
+		if len(opts) == 0 {
+			t.Fatal(name + " must be set")
+		}
+		v = opts[0]
 	}
 	return v
 }

testing/git.go

@@ -16,6 +16,7 @@ type GitRepo struct {
 	Name          string
 	CommitMessage string
 	Contents      map[string][]byte
+	Branch        string
 
 	runtime.Cmdr
 }
@@ -43,6 +44,11 @@ func (g *GitRepo) Sync(ctx context.Context) error {
 
 	for path, content := range g.Contents {
 		absPath := filepath.Join(dir, path)
+		d := filepath.Dir(absPath)
+
+		if err := os.MkdirAll(d, 0755); err != nil {
+			return fmt.Errorf("error creating dir %s: %v", d, err)
+		}
 
 		if err := os.WriteFile(absPath, content, 0755); err != nil {
 			return fmt.Errorf("error writing %s: %w", path, err)
@@ -89,7 +95,7 @@ func (g *GitRepo) gitCommitCmd(ctx context.Context, dir, msg string) *exec.Cmd {
 }
 
 func (g *GitRepo) gitPushCmd(ctx context.Context, dir string) *exec.Cmd {
-	cmd := exec.CommandContext(ctx, "git", "push", "origin", "master")
+	cmd := exec.CommandContext(ctx, "git", "push", "origin", g.Branch)
 	cmd.Dir = dir
 	return cmd
 }

testing/testing.go

@@ -323,6 +323,11 @@ func (k *Kind) Start(ctx context.Context) error {
 		kindConfig := []byte(fmt.Sprintf(`kind: Cluster
 apiVersion: kind.x-k8s.io/v1alpha4
 name: %s
+networking:
+  apiServerAddress: 0.0.0.0
+nodes:
+  - role: control-plane
+  - role: worker
 `, k.Name))
 
 		if err := os.WriteFile(f.Name(), kindConfig, 0644); err != nil {