Skip service cleanup for EphemeralRunners that never registered

[pujitha24]

Motivation:
When an AutoscalingRunnerSet is configured with a githubConfigSecret
that does not exist, its EphemeralRunners get both finalizers added
immediately on creation, before the secret is ever validated. If the
resource is then deleted, the finalizer-cleanup path always tries to
remove the runner from the GitHub Actions service, which requires
reading that (missing) secret to build an API client. That lookup
fails every time, so the finalizer is never removed and deletion
retries forever. The object is stuck in this infinite requeue loop
until the secret name is fixed; it is not a crash or data loss, and
other objects continue to reconcile normally.

Approach:
Status.RunnerID is only ever set after a JIT config has been
successfully obtained from a real secret, so RunnerID == 0 reliably
means the runner was never registered with the service. In the
deletion branch of EphemeralRunnerReconciler.Reconcile, skip the
call to cleanupRunnerFromService when RunnerID == 0 (there is
nothing registered to remove) and proceed directly to finalizer
removal. This mirrors the same RunnerID == 0 check already used in
EphemeralRunnerSetReconciler for the same purpose.

Validation:
Added a Ginkgo test, "It should delete an ephemeral runner whose
config secret never existed", that creates an EphemeralRunner
pointing at a nonexistent secret, waits for its finalizers to be
added, deletes it, and asserts it is fully removed within the
existing 20s timeout. Confirmed the test fails (times out waiting
for deletion) against the pre-fix code and passes after the fix,
reproducing the reported hang as a targeted regression test.

Ran, from the repo root with KUBEBUILDER_ASSETS pointed at the
existing envtest 1.36.2 binaries:
go build ./controllers/...
go vet ./controllers/actions.github.com/...
golangci-lint run ./controllers/actions.github.com/... (0 issues)
go test ./controllers/actions.github.com/... (ok, 79 specs)

Not run: a full-repo go build ./... / go test ./..., because the
local machine ran out of disk space while linking unrelated binaries
in this module. The change is confined to
controllers/actions.github.com, which was fully built and tested
above. Also not run: make generate / mockery codegen checks; this
change touches no API types and no interfaces, so both should be
no-ops.

Report: #4093
Signed-off-by: Pujitha Paladugu 10557236+pujitha24@users.noreply.github.com
Assisted-by: claude-sonnet-5 (via Claude Code)

Fixes #4093

[Copilot]

🟡 Changes recommended

The new RunnerID == 0 heuristic can skip cleanup for runners that may have been registered but whose status wasn’t updated yet, potentially orphaning service-side runners and also producing a misleading cleanup log message.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

This PR prevents EphemeralRunner deletion from getting stuck in an infinite requeue loop when the referenced GitHubConfigSecret never existed by skipping GitHub service cleanup for runners that were never registered.

Changes:

• In EphemeralRunnerReconciler.Reconcile deletion handling, skip cleanupRunnerFromService when Status.RunnerID == 0, then proceed to finalizer removal.
• Add a Ginkgo regression test that reproduces and asserts successful deletion for an EphemeralRunner pointing at a nonexistent config secret.

File summaries

File	Description
controllers/actions.github.com/ephemeralrunner_controller.go	Skips Actions service cleanup during deletion when RunnerID == 0 to avoid failing on missing GitHub credentials secret and unblock finalizer removal.
controllers/actions.github.com/ephemeralrunner_controller_test.go	Adds an envtest/Ginkgo spec ensuring EphemeralRunner deletion completes when its config secret never existed.

Review details

Suppressed comments (1)

controllers/actions.github.com/ephemeralrunner_controller.go:123

• This log line is misleading when cleanup is skipped (RunnerID==0): it says the runner was cleaned up from the service even though no service call was made. Consider making the message generic (e.g., referring to finalizer removal) or conditional on cleanup actually running.

			log.Info("Runner is cleaned up from the service, removing finalizer")
			if controllerutil.RemoveFinalizer(&ephemeralRunner, ephemeralRunnerActionsFinalizerName) {

• Files reviewed: 2/2 changed files
• Comments generated: 1
• Review effort level: Lite

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.