mirror of
https://github.com/actions-runner-controller/actions-runner-controller.git
synced 2026-09-30 21:51:25 +02:00
Scope the cleanup marker clear to a real revision advance
Layer 5 clears Status.FinishedRunnerCleanupPatchID beneath an early return that this layer removes, so a straight integration left the marker being cleared on every call. Guard the clear on the same advance it belongs to, and keep the phase recompute unconditional, which is what lets a spec update clear the Outdated phase immediately. Also register the resource owner index on the fake client, since the phase recompute lists the child runners the way the real manager does. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
This commit is contained in:
co-authored by
Copilot App
parent
bd0f75c3ec
commit
d099740f86
@@ -335,7 +335,24 @@ func (r *EphemeralRunnerSetReconciler) patchAppliedActionableRevisionStatus(ctx
|
||||
}
|
||||
|
||||
original := latest.DeepCopy()
|
||||
latest.Status.AppliedActionableRevision = targetAppliedRevision
|
||||
|
||||
// Only an advance means the idle and pending runners were just deleted and
|
||||
// the listener restarted. Guarding both writes on it keeps this callable
|
||||
// as a plain "make sure status reflects revision N" without disturbing a
|
||||
// marker that still describes the live patch sequence.
|
||||
if latest.Status.AppliedActionableRevision < targetAppliedRevision {
|
||||
latest.Status.AppliedActionableRevision = targetAppliedRevision
|
||||
|
||||
// The marker records a patch ID from the sequence that was current
|
||||
// before this spec change. Applying a new revision deletes the idle and
|
||||
// pending runners, so the shortfall that follows belongs to the new spec
|
||||
// and must be filled. Worse, a spec change restarts the listener, and a
|
||||
// restarted listener numbers its patches from 0 upwards, counting
|
||||
// through every integer. It therefore passes through a leftover marker
|
||||
// value with near-certainty, and would suppress the very scale up that
|
||||
// rebuilds the pool.
|
||||
latest.Status.FinishedRunnerCleanupPatchID = 0
|
||||
}
|
||||
|
||||
ephemeralRunnerList := new(v1alpha1.EphemeralRunnerList)
|
||||
if err := r.List(ctx, ephemeralRunnerList, client.InNamespace(latest.Namespace), client.MatchingFields{resourceOwnerKey: latest.Name}); err != nil {
|
||||
@@ -360,16 +377,6 @@ func (r *EphemeralRunnerSetReconciler) patchAppliedActionableRevisionStatus(ctx
|
||||
latest.Status.Phase = v1alpha1.EphemeralRunnerSetPhaseRunning
|
||||
}
|
||||
|
||||
// The marker records a patch ID from the sequence that was current before
|
||||
// this spec change. Applying a new revision deletes the idle and pending
|
||||
// runners, so the shortfall that follows belongs to the new spec and must
|
||||
// be filled. Worse, a spec change restarts the listener, and a restarted
|
||||
// listener numbers its patches from 0 upwards, counting through every
|
||||
// integer. It therefore passes through a leftover marker value with
|
||||
// near-certainty, and would suppress the very scale up that rebuilds the
|
||||
// pool.
|
||||
latest.Status.FinishedRunnerCleanupPatchID = 0
|
||||
|
||||
// Checked after every field above has been set, so that clearing the
|
||||
// marker alone is still enough to issue the patch.
|
||||
if original.Status == latest.Status {
|
||||
|
||||
@@ -153,6 +153,10 @@ func TestPatchAppliedActionableRevisionStatusClearsFinishedRunnerCleanupPatchID(
|
||||
WithScheme(scheme).
|
||||
WithObjects(object).
|
||||
WithStatusSubresource(&v1alpha1.EphemeralRunnerSet{}).
|
||||
// patchAppliedActionableRevisionStatus lists the child runners to
|
||||
// recompute the phase, so the fake client needs the same index
|
||||
// SetupIndexers registers on the manager.
|
||||
WithIndex(&v1alpha1.EphemeralRunner{}, resourceOwnerKey, newGroupVersionOwnerKindIndexer("EphemeralRunnerSet")).
|
||||
Build()
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user