Skip to content

Conversation

@tfujiwar
Copy link
Contributor

@tfujiwar tfujiwar commented Nov 27, 2024

WHAT

  • Allow the controller to watch Secrets / ConfigMaps in the single namespace mode.
  • Enable the k8s API client cache for Secrets / ConfigMaps in the single namespace mode.

WHY

To reduce latency of runner pod creation (#3276).

The EphemeralRunnerReconciler retrieves a JITConfig Secret for every reconciliation once the Secret is created. It can be a performance bottleneck because the k8s API client cache is disabled for Secrets.

The cache has been disabled for Secrets because it requires cluster-wide list/watch permissions in the default mode. But in the single namespace mode, we can narrow down the permissions only to the single namespace and the controller namespace, which would be acceptable.

This change is aligned with ADR 2023-04-11: Limit Permissions for Service Accounts in Actions-Runner-Controller.

In this mode, you will end up with a manager Role that has all Get/List/Create/Delete/Update/Patch/Watch permissions on resources we need, and a RoleBinding to bind the Role with the controller ServiceAccount in the watched single namespace and the controller namespace

TESTING

We have tested the change with a controller that manages 300-400 EphemeralRunners at peak times. We confirmed that the duration of getting k8s Secrets from the EphemeralRunnerReconciler has decreased due to the caching.

Screenshot 2024-11-27 at 12 06 56

We also confirmed that the number of Pending EphemeralRunners has decreased because they are reconciled faster.

Before

Screenshot 2024-11-27 at 12 23 39

After

Screenshot 2024-11-27 at 12 22 42

@tfujiwar tfujiwar marked this pull request as ready for review November 27, 2024 03:31
@tfujiwar tfujiwar changed the title Allow the controller to watch Secrets / ConfigMaps in the single namespace mode Allow the controller to cache Secrets / ConfigMaps in the single namespace mode Dec 5, 2024
@tfujiwar tfujiwar changed the title Allow the controller to cache Secrets / ConfigMaps in the single namespace mode Enable k8s client cache for Secrets and ConfigMaps in the single namespace mode Dec 5, 2024
@Copilot Copilot AI review requested due to automatic review settings October 21, 2025 15:27
@azrsh azrsh requested review from a team and nikola-jokic as code owners October 21, 2025 15:27
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR enables caching of Secrets and ConfigMaps in the Kubernetes API client when the controller operates in single namespace mode, improving performance by reducing reconciliation latency for EphemeralRunners.

Key Changes:

  • Conditionally enables client cache for Secrets/ConfigMaps based on watch mode
  • Adds list/watch permissions for Secrets and ConfigMaps in single namespace RBAC roles
  • Updates test assertions to reflect the additional RBAC rules

Reviewed Changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.

File Description
main.go Moves cache disable configuration into a conditional block that only applies when not in single namespace mode
charts/gha-runner-scale-set-controller/tests/template_test.go Updates expected rule counts to account for new Secret and ConfigMap permissions
charts/gha-runner-scale-set-controller/templates/manager_single_namespace_watch_role.yaml Adds list/watch permissions for Secrets and ConfigMaps in the watched namespace
charts/gha-runner-scale-set-controller/templates/manager_single_namespace_controller_role.yaml Adds list/watch permissions for Secrets and ConfigMaps in the controller namespace

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants