{{ message }}
Use Patch instead of Update#4533
Open
nikola-jokic wants to merge 15 commits into
Open
Conversation
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
nikola-jokic
requested review from
Steve-Glass,
mumoshu,
rentziass and
toast-gear
as code owners
Contributor
Contributor
There was a problem hiding this comment.
Pull request overview
This PR updates the controller and related manifests to prefer Kubernetes Patch operations over Update, and adjusts CRD/type optionality to support patching without schema/validation issues (per the dependency on #4528). It also includes a runner version bump and several integrity-hash/metadata propagation adjustments.
Changes:
- Replace multiple
Update/Status().Updatecalls withPatch(..., client.MergeFrom(...)), and update RBAC to allowpatchwhere needed. - Refine integrity-hash and metadata propagation logic (filter/merge annotations/labels; update listener in-place rather than delete/recreate).
- Update API types/CRDs (optional fields, printcolumns, proxy field rename
Url→URL) and bump runner version to2.335.1.
Reviewed changes
Copilot reviewed 37 out of 37 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| test/e2e/e2e_test.go | Bumps e2e runner version constant. |
| runner/VERSION | Bumps runner version used for images/builds. |
| runner/Makefile | Bumps runner version default for runner image build. |
| Makefile | Bumps runner version default at repo root. |
| controllers/actions.github.com/utils.go | Adds mustJSON helper for structured spec logging. |
| controllers/actions.github.com/secretresolver/secret_resolver.go | Renames proxy config field usage Url → URL. |
| controllers/actions.github.com/resourcebuilder.go | Exports integrity-hash annotation key, refactors label/annotation merge, adds integrity hash helper. |
| controllers/actions.github.com/resourcebuilder_test.go | Updates tests for new annotation key name. |
| controllers/actions.github.com/ephemeralrunnerset_controller.go | Switches integrity-hash annotation writes from Update to Patch; refactors status patching. |
| controllers/actions.github.com/ephemeralrunnerset_controller_test.go | Updates e2e-style controller tests to use Patch and URL field name. |
| controllers/actions.github.com/ephemeralrunner_controller_test.go | Updates tests to patch status/pod updates instead of Update; uses deep copies for MergeFrom. |
| controllers/actions.github.com/autoscalingrunnerset_controller.go | Uses spec-hash helpers and patch-based updates; updates listener in-place instead of delete/recreate. |
| controllers/actions.github.com/autoscalingrunnerset_controller_test.go | Adjusts expectations for in-place updates and integrity-hash behavior; formatting tweaks. |
| controllers/actions.github.com/autoscalinglistener_controller.go | Updates RBAC markers for patch; patches dependent resources; logs spec diffs when recreating pod. |
| controllers/actions.github.com/autoscalinglistener_controller_test.go | Updates tests to patch pod status; Url → URL. |
| config/rbac/role.yaml | Adjusts ClusterRole verbs to use patch for secrets/serviceaccounts and roles/rolebindings. |
| config/crd/bases/actions.github.com_ephemeralrunnersets.yaml | CRD schema/printcolumns updates (phase column, optionality, minimums, proxy url required). |
| config/crd/bases/actions.github.com_ephemeralrunners.yaml | CRD schema/printcolumns updates (Phase column name, TLS/proxy required subfields). |
| config/crd/bases/actions.github.com_autoscalingrunnersets.yaml | CRD schema/printcolumns updates (phase column, failed runners column, minimums, doc text). |
| config/crd/bases/actions.github.com_autoscalinglisteners.yaml | CRD schema updates (required descriptions removed, proxy url required, runnerScaleSetId minimum). |
| charts/gha-runner-scale-set/tests/template_test.go | Updates chart test for proxy field rename Url → URL. |
| charts/gha-runner-scale-set/templates/manager_role.yaml | Adds patch/update verbs in chart role template. |
| charts/gha-runner-scale-set-experimental/templates/manager_role.yaml | Adds patch/update verbs in experimental chart role template. |
| charts/gha-runner-scale-set-experimental/templates/_mode_kubernetes.tpl | Renders custom volume mounts and integrates them with GitHub server TLS mount helper. |
| charts/gha-runner-scale-set-controller/crds/actions.github.com_ephemeralrunnersets.yaml | Vendored CRD copy updated to match base CRD changes. |
| charts/gha-runner-scale-set-controller/crds/actions.github.com_ephemeralrunners.yaml | Vendored CRD copy updated to match base CRD changes. |
| charts/gha-runner-scale-set-controller/crds/actions.github.com_autoscalingrunnersets.yaml | Vendored CRD copy updated to match base CRD changes. |
| charts/gha-runner-scale-set-controller/crds/actions.github.com_autoscalinglisteners.yaml | Vendored CRD copy updated to match base CRD changes. |
| charts/gha-runner-scale-set-controller-experimental/crds/actions.github.com_ephemeralrunnersets.yaml | Vendored experimental CRD copy updated to match base CRD changes. |
| charts/gha-runner-scale-set-controller-experimental/crds/actions.github.com_ephemeralrunners.yaml | Vendored experimental CRD copy updated to match base CRD changes. |
| charts/gha-runner-scale-set-controller-experimental/crds/actions.github.com_autoscalingrunnersets.yaml | Vendored experimental CRD copy updated to match base CRD changes. |
| charts/gha-runner-scale-set-controller-experimental/crds/actions.github.com_autoscalinglisteners.yaml | Vendored experimental CRD copy updated to match base CRD changes. |
| apis/actions.github.com/v1alpha1/proxy_config_test.go | Updates API tests for Url → URL. |
| apis/actions.github.com/v1alpha1/ephemeralrunnerset_types.go | Marks fields optional and adds minimum validations / printcolumn updates. |
| apis/actions.github.com/v1alpha1/ephemeralrunner_types.go | Adjusts printcolumn name and optional markers; makes spec fields optional for patching. |
| apis/actions.github.com/v1alpha1/autoscalingrunnerset_types.go | Updates status printcolumns and validations; renames proxy field; removes AutoscalingRunnerSet.Hash(). |
| apis/actions.github.com/v1alpha1/autoscalinglistener_types.go | Marks spec fields optional; adds/adjusts validations; minor list type documentation tweak. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
130
to
134
| assert.Equal(t, labelValueKubernetesPartOf, listener.Labels[LabelKeyKubernetesPartOf]) | ||
| assert.Equal(t, "runner-scale-set-listener", listener.Labels[LabelKeyKubernetesComponent]) | ||
| assert.Equal(t, autoscalingRunnerSet.Labels[LabelKeyKubernetesVersion], listener.Labels[LabelKeyKubernetesVersion]) | ||
| assert.NotEmpty(t, ephemeralRunnerSet.Annotations[annotationKeyIntegrityHash]) | ||
| assert.NotEmpty(t, ephemeralRunnerSet.Annotations[AnnotationKeyIntegrityHash]) | ||
| assert.Equal(t, autoscalingRunnerSet.Name, listener.Labels[LabelKeyGitHubScaleSetName]) |
Comment on lines
+293
to
297
nikola-jokic
force-pushed
the
nikola-jokic/update-to-patch-request
branch
from
77b79db to
094078c
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Depends on #4528