Conversation
|
Hello! Thank you for your contribution. Please review our contribution guidelines to understand the project's testing and code conventions. |
| env: | ||
| KUBE_SCORE_VERSION: 1.16.1 | ||
| HELM_VERSION: v3.17.0 | ||
| HELM_VERSION: v3.19.4 |
There was a problem hiding this comment.
Out of curiosity, why not upgrade to the latest stable version?
There was a problem hiding this comment.
We tend to pin the version for all releases but we can change this if you want
| # 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.13.1 |
There was a problem hiding this comment.
Would the chart version be 0.14.0?
There was a problem hiding this comment.
I can bump it when during the release, but I can bump it now. I just used this for local testing so I don't need to re-build the image with 0.14.0 version to test charts.
There was a problem hiding this comment.
Pull request overview
Introduces a new experimental Helm chart release track (controller + scale set) to enable deeper customization (pod spec passthrough, sidecars, hook extensions, secret resolution options), while keeping the existing chart available as a stable option.
Changes:
- Adds new
gha-runner-scale-set-controller-experimentalandgha-runner-scale-set-experimentalcharts with extensive helm-unittest coverage. - Updates E2E test scripts and GitHub Actions workflows to run v2 (experimental) scenarios alongside existing tests.
- Refactors E2E helper utilities to make chart version selection explicit per test.
Reviewed changes
Copilot reviewed 117 out of 121 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| test/actions.github.com/update-strategy.test.sh | Sets VERSION via Chart.yaml helper |
| test/actions.github.com/update-strategy-v2.test.sh | New E2E test for experimental charts |
| test/actions.github.com/single-namespace-setup.test.sh | Sets VERSION via Chart.yaml helper |
| test/actions.github.com/single-namespace-setup-v2.test.sh | New single-namespace E2E test (experimental) |
| test/actions.github.com/self-signed-ca-setup.test.sh | Sets VERSION via Chart.yaml helper |
| test/actions.github.com/self-signed-ca-setup-v2.test.sh | New self-signed CA E2E test (experimental) |
| test/actions.github.com/kubernetes-mode-setup.test.sh | Sets VERSION via Chart.yaml helper |
| test/actions.github.com/kubernetes-mode-setup-v2.test.sh | New kubernetes-mode E2E test (experimental) |
| test/actions.github.com/init-with-min-runners.test.sh | Sets VERSION via Chart.yaml helper |
| test/actions.github.com/init-with-min-runners-v2.test.sh | New min runners E2E test (experimental) |
| test/actions.github.com/dind-mode-setup.test.sh | Sets VERSION via Chart.yaml helper |
| test/actions.github.com/dind-mode-setup-v2.test.sh | New dind-mode E2E test (experimental) |
| test/actions.github.com/default-setup.test.sh | Sets VERSION via Chart.yaml helper |
| test/actions.github.com/default-setup-v2.test.sh | New default E2E test (experimental) |
| test/actions.github.com/auth-proxy-setup.test.sh | Sets VERSION via Chart.yaml helper |
| test/actions.github.com/auth-proxy-setup-v2.test.sh | New auth proxy E2E test (experimental) |
| test/actions.github.com/anonymous-proxy-setup.test.sh | Sets VERSION via Chart.yaml helper |
| test/actions.github.com/anonymous-proxy-setup-v2.test.sh | New anonymous proxy E2E test (experimental) |
| test/actions.github.com/helper.sh | Refactors version extraction + workflow dispatch logic |
| hack/e2e-test.sh | Minor cleanup in E2E runner script |
| charts/gha-runner-scale-set/tests/autoscaling_runner_set_test.yaml | Adds helm-unittest suite for stable chart |
| charts/gha-runner-scale-set/.helmignore | Excludes tests/ from chart packaging |
| charts/gha-runner-scale-set-experimental/Chart.yaml | New experimental scale set chart metadata |
| charts/gha-runner-scale-set-experimental/.helmignore | New helmignore for experimental chart |
| charts/gha-runner-scale-set-experimental/templates/no_permission_serviceaccount.yaml | Adds no-permission SA template |
| charts/gha-runner-scale-set-experimental/templates/manager_role_binding.yaml | Adds manager RoleBinding template |
| charts/gha-runner-scale-set-experimental/templates/manager_role.yaml | Adds manager Role template (extra rules support) |
| charts/gha-runner-scale-set-experimental/templates/kube_mode_serviceaccount.yaml | Adds kube-mode SA template |
| charts/gha-runner-scale-set-experimental/templates/kube_mode_role_binding.yaml | Adds kube-mode RoleBinding template |
| charts/gha-runner-scale-set-experimental/templates/kube_mode_role.yaml | Adds kube-mode Role template (extra rules support) |
| charts/gha-runner-scale-set-experimental/templates/hook_extension.yaml | Adds hook extension ConfigMap rendering |
| charts/gha-runner-scale-set-experimental/templates/githubsecret.yaml | Adds GitHub auth Secret rendering/validation |
| charts/gha-runner-scale-set-experimental/templates/_runner_pod.tpl | Adds runner pod metadata merge helpers |
| charts/gha-runner-scale-set-experimental/templates/_mode_empty.tpl | Adds “mode empty” runner container expansion |
| charts/gha-runner-scale-set-experimental/templates/_mode_dind.tpl | Adds dind-mode runner/dind container fragments |
| charts/gha-runner-scale-set-experimental/templates/_manager_role.tpl | Adds manager role/rolebinding label+annotation helpers |
| charts/gha-runner-scale-set-experimental/templates/_listener_template.tpl | Adds listenerPodTemplate rendering helper |
| charts/gha-runner-scale-set-experimental/templates/_helpers.tpl | Adds shared helpers incl. TLS injection snippets |
| charts/gha-runner-scale-set-experimental/templates/_defaults.tpl | Adds naming/namespace/common label defaults |
| charts/gha-runner-scale-set-experimental/templates/_autoscalingrunnerset.tpl | Adds autoscaling runner set helper templates |
| charts/gha-runner-scale-set-experimental/tests/namespace_override_test.yaml | Tests namespaceOverride behavior |
| charts/gha-runner-scale-set-experimental/tests/manager_role_extra_rules_test.yaml | Tests manager role extraRules validation |
| charts/gha-runner-scale-set-experimental/tests/manager_role_binding_labels_test.yaml | Tests manager RoleBinding label merging |
| charts/gha-runner-scale-set-experimental/tests/manager_role_binding_annotations_test.yaml | Tests manager RoleBinding annotation merging |
| charts/gha-runner-scale-set-experimental/tests/kube_mode_serviceaccount_test.yaml | Tests kube-mode SA rendering and metadata |
| charts/gha-runner-scale-set-experimental/tests/kube_mode_role_test.yaml | Tests kube-mode role rules + extraRules |
| charts/gha-runner-scale-set-experimental/tests/kube_mode_role_binding_test.yaml | Tests kube-mode RoleBinding behavior |
| charts/gha-runner-scale-set-experimental/tests/hook_extension_namespace_test.yaml | Tests hook extension namespace alignment |
| charts/gha-runner-scale-set-experimental/tests/hook_extension_configmap_test.yaml | Tests hook extension ConfigMap data rendering |
| charts/gha-runner-scale-set-experimental/tests/github_secret_predefined_secret_test.yaml | Tests skipping secret when secretName provided |
| charts/gha-runner-scale-set-experimental/tests/github_secret_labels_test.yaml | Tests GitHub secret labels + reserved protection |
| charts/gha-runner-scale-set-experimental/tests/github_secret_app_test.yaml | Tests PAT vs App auth secret data + validation |
| charts/gha-runner-scale-set-experimental/tests/github_secret_annotations_test.yaml | Tests GitHub secret annotations behavior |
| charts/gha-runner-scale-set-experimental/tests/autoscaling_runner_set_vault_config_test.yaml | Tests vault config rendering/validation |
| charts/gha-runner-scale-set-experimental/tests/autoscaling_runner_set_runner_pod_spec_volumes_validation_test.yaml | Tests runner.pod.spec.volumes type validation |
| charts/gha-runner-scale-set-experimental/tests/autoscaling_runner_set_runner_pod_spec_passthrough_fields_test.yaml | Tests runner pod spec passthrough fields |
| charts/gha-runner-scale-set-experimental/tests/autoscaling_runner_set_runner_pod_spec_init_containers_validation_test.yaml | Tests initContainers validation cases |
| charts/gha-runner-scale-set-experimental/tests/autoscaling_runner_set_runner_pod_metadata_test.yaml | Tests runner pod labels/annotations merge rules |
| charts/gha-runner-scale-set-experimental/tests/autoscaling_runner_set_mode_empty_runner_container_test.yaml | Tests “mode empty” container requirements/defaults |
| charts/gha-runner-scale-set-experimental/tests/autoscaling_runner_set_min_max_runners_test.yaml | Tests min/max runner constraints |
| charts/gha-runner-scale-set-experimental/tests/autoscaling_runner_set_listener_pod_template_test.yaml | Tests listenerPodTemplate mapping |
| charts/gha-runner-scale-set-experimental/tests/autoscaling_runner_set_listener_metrics_test.yaml | Tests listener metrics passthrough |
| charts/gha-runner-scale-set-experimental/tests/autoscaling_runner_set_kubernetes_mode_spec_test.yaml | Tests kubernetes-mode defaults/overrides |
| charts/gha-runner-scale-set-experimental/tests/autoscaling_runner_set_kubernetes_mode_hook_extension_test.yaml | Tests hook extension wiring into runner pod |
| charts/gha-runner-scale-set-experimental/tests/autoscaling_runner_set_github_server_tls_test.yaml | Tests TLS fields + RBAC for configmap |
| charts/gha-runner-scale-set-experimental/tests/autoscaling_runner_set_github_server_tls_runner_injection_test.yaml | Tests TLS injection into runner containers |
| charts/gha-runner-scale-set-experimental/tests/autoscaling_runner_set_extra_init_containers_test.yaml | Tests extra initContainers passthrough |
| charts/gha-runner-scale-set-experimental/tests/autoscaling_runner_set_extra_containers_test.yaml | Tests sidecar containers and validation |
| charts/gha-runner-scale-set-experimental/tests/autoscaling_runner_set_auth_test.yaml | Tests auth/url/secret/name validation behavior |
| charts/gha-runner-scale-set-experimental/tests/autoscaling_runner_set_annotations_test.yaml | Tests annotations merge + reserved protection |
| charts/gha-runner-scale-set-experimental/tests/autoscaling_runner_set_dind_mode_spec_test.yaml | Tests dind-mode rendering and overrides |
| charts/gha-runner-scale-set-experimental/tests/autoscaling_runner_set_labels_test.yaml | Tests label merge + reserved protection |
| charts/gha-runner-scale-set-experimental/tests/autoscaling_runner_set_proxy_test.yaml | Tests proxy rendering paths |
| charts/gha-runner-scale-set-controller-experimental/Chart.yaml | New experimental controller chart metadata |
| charts/gha-runner-scale-set-controller-experimental/.helmignore | New helmignore for controller experimental chart |
| charts/gha-runner-scale-set-controller-experimental/values.yaml | New controller experimental values structure |
| charts/gha-runner-scale-set-controller-experimental/ci/ci-values.yaml | Adds CI values file for chart testing |
| charts/gha-runner-scale-set-controller-experimental/templates/serviceaccount.yaml | Adds controller ServiceAccount template |
| charts/gha-runner-scale-set-controller-experimental/templates/deployment.yaml | Adds controller Deployment template |
| charts/gha-runner-scale-set-controller-experimental/templates/manager_single_namespace_watch_role_binding.yaml | Adds RBAC for watch namespace |
| charts/gha-runner-scale-set-controller-experimental/templates/manager_single_namespace_watch_role.yaml | Adds watch-namespace Role rules |
| charts/gha-runner-scale-set-controller-experimental/templates/manager_single_namespace_controller_role_binding.yaml | Adds controller-namespace RoleBinding |
| charts/gha-runner-scale-set-controller-experimental/templates/manager_single_namespace_controller_role.yaml | Adds controller-namespace Role rules |
| charts/gha-runner-scale-set-controller-experimental/templates/manager_listener_role_binding.yaml | Adds listener RoleBinding |
| charts/gha-runner-scale-set-controller-experimental/templates/manager_listener_role.yaml | Adds listener Role |
| charts/gha-runner-scale-set-controller-experimental/templates/manager_cluster_role_binding.yaml | Adds ClusterRoleBinding (cluster mode) |
| charts/gha-runner-scale-set-controller-experimental/templates/manager_cluster_role.yaml | Adds ClusterRole (cluster mode) |
| charts/gha-runner-scale-set-controller-experimental/templates/leader_election_role_binding.yaml | Adds leader election RoleBinding |
| charts/gha-runner-scale-set-controller-experimental/templates/leader_election_role.yaml | Adds leader election Role |
| charts/gha-runner-scale-set-controller-experimental/templates/_controller_template.tpl | Adds manager container args/ports/env wiring |
| charts/gha-runner-scale-set-controller-experimental/templates/_controller.tpl | Adds naming/labels/serviceAccount helpers |
| charts/gha-runner-scale-set-controller-experimental/templates/_defaults.tpl | Adds common label/name helpers |
| charts/gha-runner-scale-set-controller-experimental/templates/_helpers.tpl | Adds label filtering + selector labels |
| charts/gha-runner-scale-set-controller-experimental/templates/NOTES.txt | Adds Helm NOTES output |
| charts/gha-runner-scale-set-controller-experimental/tests/controller_serviceaccount_validation_test.yaml | Tests SA validation failures |
| charts/gha-runner-scale-set-controller-experimental/tests/controller_serviceaccount_test.yaml | Tests SA rendering behavior |
| charts/gha-runner-scale-set-controller-experimental/tests/controller_serviceaccount_create_toggle_test.yaml | Tests SA create toggle behavior |
| charts/gha-runner-scale-set-controller-experimental/tests/controller_rbac_single_namespace_test.yaml | Tests single-namespace RBAC behavior |
| charts/gha-runner-scale-set-controller-experimental/tests/controller_rbac_listener_role_test.yaml | Tests listener RBAC behavior |
| charts/gha-runner-scale-set-controller-experimental/tests/controller_rbac_leader_election_test.yaml | Tests leader election RBAC toggling |
| charts/gha-runner-scale-set-controller-experimental/tests/controller_rbac_clusterrole_test.yaml | Tests cluster RBAC toggling |
| charts/gha-runner-scale-set-controller-experimental/tests/controller_namespace_override_test.yaml | Tests namespaceOverride behavior |
| charts/gha-runner-scale-set-controller-experimental/tests/controller_manager_clusterrolebinding_test.yaml | Tests ClusterRoleBinding toggling |
| charts/gha-runner-scale-set-controller-experimental/tests/controller_deployment_volumes_test.yaml | Tests extra volumes passthrough |
| charts/gha-runner-scale-set-controller-experimental/tests/controller_deployment_volume_mounts_test.yaml | Tests extra volumeMounts passthrough |
| charts/gha-runner-scale-set-controller-experimental/tests/controller_deployment_smoke_test.yaml | Smoke tests basic deployment rendering |
| charts/gha-runner-scale-set-controller-experimental/tests/controller_deployment_pod_extra_fields_test.yaml | Tests pod spec passthrough + SA protection |
| charts/gha-runner-scale-set-controller-experimental/tests/controller_deployment_imagepullsecrets_test.yaml | Tests imagePullSecrets wiring |
| charts/gha-runner-scale-set-controller-experimental/tests/controller_deployment_extra_containers_test.yaml | Tests extra containers appended after manager |
| charts/gha-runner-scale-set-controller-experimental/tests/controller_deployment_env_test.yaml | Tests env + leader election args behavior |
| charts/gha-runner-scale-set-controller-experimental/tests/controller_deployment_args_test.yaml | Tests args/metrics/watch flags rendering |
| Makefile | Copies CRDs into experimental controller chart |
| .github/workflows/gha-validate-chart.yaml | Runs helm-unittest for experimental charts; updates Helm version |
| .github/workflows/gha-e2e-tests.yaml | Runs v2 E2E tests alongside existing ones |
Comments suppressed due to low confidence (1)
.github/workflows/gha-e2e-tests.yaml:164
- The
auth-proxy-setupjob is running thesingle-namespace-setuptests (both v1 and v2) instead of the auth-proxy tests. This looks like a copy/paste error and will leave auth-proxy behavior untested while duplicating single-namespace coverage.
- name: Run single namespace setup test
run: hack/e2e-test.sh single-namespace-setup-v2
env:
GITHUB_TOKEN: "${{steps.config-token.outputs.token}}"
shell: bash
- name: Run single namespace setup test
run: hack/e2e-test.sh single-namespace-setup
env:
GITHUB_TOKEN: "${{steps.config-token.outputs.token}}"
shell: bash
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| securityContext: | ||
| {{- if $dind.container.securityContext }} | ||
| {{- toYaml $dind.container.securityContext | nindent 2 }} | ||
| {{ else }} | ||
| {{- toYaml (dict "privileged" true) | nindent 2 }} | ||
| {{- end }} | ||
| restartPolicy: Always | ||
| startupProbe: | ||
| {{- include "runner-mode-dind.startup-probe" . | nindent 2 }} |
There was a problem hiding this comment.
restartPolicy is a PodSpec field, not a container field. Rendering it inside the dind container spec will produce invalid Kubernetes YAML. Move this to the pod spec (or remove it if not needed).
charts/gha-runner-scale-set/tests/autoscaling_runner_set_test.yaml
Outdated
Show resolved
Hide resolved
| echo "Waiting for run to complete" | ||
| local code | ||
| code=$(gh run watch "${run_id}" -R "${TARGET_ORG}/${TARGET_REPO}" --exit-status &>/dev/null) | ||
| if [[ "${code}" -ne 0 ]]; then | ||
| echo "Run failed with exit code ${code}" | ||
| gh run watch "${run_id}" -R "${repo}" --exit-status &>/dev/null | ||
| local status=$? | ||
| if [[ "${status}" -ne 0 ]]; then | ||
| echo "Run failed with exit code ${status}" | ||
| return 1 |
There was a problem hiding this comment.
With set -e enabled in the calling test scripts, gh run watch --exit-status will terminate the script immediately on failure, so the subsequent status handling/logging won't run. Wrap the command in an if ! ...; then block (or temporarily disable errexit) so failures are handled as intended.
The chart redesign intention is to:
The new chart comes with one limitation: There is no longer kubernetes resolution searching for the controller service account.
This chart should be released next to the current one. In case there is a bug in the chart, users should have a way to keep using the stable version of the chart while we are working on the fix.
Fixes #3819
Fixes #4350