Files
actions-runner-controller/controllers/actions.github.com/helpers_outdated_test.go
T
Nikola JokicandCopilot App 854c0517a7 Make the Outdated phase revision-aware
An EphemeralRunner that exits as Outdated marks its EphemeralRunnerSet
Outdated, and the AutoscalingRunnerSet then stops scaling it. Without a way
to tell which runner spec an Outdated report refers to, a report from a
runner built before the spec was updated keeps the set switched off after
the update that was supposed to fix it.

Stamp each runner with the actionable revision it was built from, and judge
Outdated reports against the revision the set has applied:

  - A runner whose revision is older than the applied one is reporting on a
    spec that has already been replaced. It is deleted and rebuilt from the
    current spec, and it does not hold the set Outdated.
  - A runner whose revision is current is reporting on the live spec, so the
    set stays Outdated. It is deliberately not delete-and-replaced: a fresh
    runner at the same revision would report Outdated again, forever.

Runners without the annotation parse to revision 0, which matches the zero
value of Status.AppliedActionableRevision, so existing runners keep their
current behaviour across an upgrade.

The phase is derived inside patchAppliedActionableRevisionStatus, from a
list read through the same authoritative reader as the set itself, because
the optimistic lock on that patch covers the EphemeralRunnerSet object only
and cannot vouch for a separately-read list. The runners are classified
against the live applied revision rather than the revision this call was
asked to apply: the caller reads the spec from the cache while this function
re-reads the status from the API server, so a lagging reconcile can arrive
with a target behind the live marker, and judging against it would flip a
set that has already moved on back to Outdated.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
2026-09-12 23:18:26 +02:00

252 lines
8.9 KiB
Go

package actionsgithubcom
import (
"strconv"
"testing"
"github.com/actions/actions-runner-controller/apis/actions.github.com/v1alpha1"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
)
func outdatedRunnerAtRevision(name string, revision int64) v1alpha1.EphemeralRunner {
return v1alpha1.EphemeralRunner{
ObjectMeta: metav1.ObjectMeta{
Name: name,
Annotations: map[string]string{
AnnotationKeyActionableRevision: strconv.FormatInt(revision, 10),
},
},
Status: v1alpha1.EphemeralRunnerStatus{
Phase: v1alpha1.EphemeralRunnerPhaseOutdated,
},
}
}
// TestNewEphemeralRunnersByStates_OutdatedIsRevisionScoped covers the core of the
// outdated-recovery behaviour: a runner that reported Outdated against a runner
// spec that has since been replaced must not be treated as evidence about the
// current spec.
func TestNewEphemeralRunnersByStates_OutdatedIsRevisionScoped(t *testing.T) {
tests := []struct {
name string
runners []v1alpha1.EphemeralRunner
appliedRevision int64
wantOutdatedNames []string
wantStaleOutdatedName []string
}{
{
name: "runner at the applied revision is genuinely outdated",
runners: []v1alpha1.EphemeralRunner{outdatedRunnerAtRevision("current", 3)},
appliedRevision: 3,
wantOutdatedNames: []string{"current"},
},
{
name: "runner from before the last spec update is stale",
runners: []v1alpha1.EphemeralRunner{outdatedRunnerAtRevision("old", 2)},
appliedRevision: 3,
wantStaleOutdatedName: []string{"old"},
},
{
name: "mixed revisions are split",
runners: []v1alpha1.EphemeralRunner{
outdatedRunnerAtRevision("old", 1),
outdatedRunnerAtRevision("current", 4),
},
appliedRevision: 4,
wantOutdatedNames: []string{"current"},
wantStaleOutdatedName: []string{"old"},
},
{
name: "runner without the annotation is treated as revision 0",
runners: []v1alpha1.EphemeralRunner{{
ObjectMeta: metav1.ObjectMeta{Name: "legacy"},
Status: v1alpha1.EphemeralRunnerStatus{Phase: v1alpha1.EphemeralRunnerPhaseOutdated},
}},
appliedRevision: 0,
wantOutdatedNames: []string{"legacy"},
},
{
name: "legacy runner becomes stale once a revision is applied",
runners: []v1alpha1.EphemeralRunner{{
ObjectMeta: metav1.ObjectMeta{Name: "legacy"},
Status: v1alpha1.EphemeralRunnerStatus{Phase: v1alpha1.EphemeralRunnerPhaseOutdated},
}},
appliedRevision: 1,
wantStaleOutdatedName: []string{"legacy"},
},
{
name: "a runner being deleted is never classified as outdated",
runners: []v1alpha1.EphemeralRunner{func() v1alpha1.EphemeralRunner {
runner := outdatedRunnerAtRevision("terminating", 3)
now := metav1.Now()
runner.DeletionTimestamp = &now
runner.Finalizers = []string{ephemeralRunnerFinalizerName}
return runner
}()},
appliedRevision: 3,
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
list := &v1alpha1.EphemeralRunnerList{Items: tt.runners}
state := newEphemeralRunnersByStates(list, tt.appliedRevision)
assert.Equal(t, tt.wantOutdatedNames, runnerNames(state.outdated))
assert.Equal(t, tt.wantStaleOutdatedName, runnerNames(state.staleOutdated))
})
}
}
// TestEphemeralRunnersByState_TerminatedIncludesStaleOutdated ensures the cleanup
// paths still collect stale outdated runners; they are excluded from the phase
// decision, not from garbage collection.
func TestEphemeralRunnersByState_TerminatedIncludesStaleOutdated(t *testing.T) {
list := &v1alpha1.EphemeralRunnerList{Items: []v1alpha1.EphemeralRunner{
outdatedRunnerAtRevision("stale", 1),
outdatedRunnerAtRevision("current", 5),
{
ObjectMeta: metav1.ObjectMeta{Name: "succeeded"},
Status: v1alpha1.EphemeralRunnerStatus{Phase: v1alpha1.EphemeralRunnerPhaseSucceeded},
},
{
ObjectMeta: metav1.ObjectMeta{Name: "failed"},
Status: v1alpha1.EphemeralRunnerStatus{Phase: v1alpha1.EphemeralRunnerPhaseFailed},
},
}}
state := newEphemeralRunnersByStates(list, 5)
assert.ElementsMatch(t,
[]string{"succeeded", "failed", "current", "stale"},
runnerNames(state.terminated()),
)
}
// TestEphemeralRunnersByState_TerminatedDoesNotAliasBackingArrays guards against
// terminated() corrupting the slices it concatenates, which would silently
// reclassify runners.
//
// A concatenation written as append(s.finished, others...) does not copy when
// s.finished has spare capacity: it writes the other runners into that spare
// room and hands back a slice sharing the caller's backing array. The corruption
// only becomes visible once something appends to s.finished again, which reuses
// the same slots and overwrites entries in the already-returned slice. So the
// test needs a state whose finished slice has room to spare, a retained result,
// and a subsequent append.
func TestEphemeralRunnersByState_TerminatedDoesNotAliasBackingArrays(t *testing.T) {
finished := func(name string) v1alpha1.EphemeralRunner {
return v1alpha1.EphemeralRunner{
ObjectMeta: metav1.ObjectMeta{Name: name},
Status: v1alpha1.EphemeralRunnerStatus{Phase: v1alpha1.EphemeralRunnerPhaseSucceeded},
}
}
// Three finished runners, because the classifier builds the slice with
// append and the backing array grows 1, 2, 4: the result is length 3 with
// room for a fourth.
list := &v1alpha1.EphemeralRunnerList{Items: []v1alpha1.EphemeralRunner{
finished("succeeded-a"),
finished("succeeded-b"),
finished("succeeded-c"),
{
ObjectMeta: metav1.ObjectMeta{Name: "failed"},
Status: v1alpha1.EphemeralRunnerStatus{Phase: v1alpha1.EphemeralRunnerPhaseFailed},
},
}}
state := newEphemeralRunnersByStates(list, 0)
// Asserted rather than assumed: if the classifier ever stops leaving spare
// capacity, the concatenation is forced to allocate, no aliasing is possible
// and the rest of this test would quietly stop proving anything.
require.Greater(t, cap(state.finished), len(state.finished),
"precondition: finished needs spare capacity for aliasing to be reproducible")
terminated := state.terminated()
require.Contains(t, runnerNames(terminated), "failed")
// Reuses the spare slot that an aliasing concatenation would have written
// the failed runner into.
state.finished = append(state.finished, &v1alpha1.EphemeralRunner{
ObjectMeta: metav1.ObjectMeta{Name: "succeeded-d"},
})
assert.Contains(t, runnerNames(terminated), "failed",
"terminated() must own its backing array: appending to state.finished overwrote a runner in the slice already returned to the caller")
assert.Equal(t, []string{"succeeded-a", "succeeded-b", "succeeded-c"}, runnerNames(state.finished[:3]))
assert.Equal(t, []string{"failed"}, runnerNames(state.failed))
}
// TestEphemeralRunnerSetOutdatedForAppliedRevision covers the guard that stops the
// AutoscalingRunnerSet from tearing the scale set down on an Outdated verdict that
// predates the runner spec it has just published.
func TestEphemeralRunnerSetOutdatedForAppliedRevision(t *testing.T) {
tests := []struct {
name string
phase v1alpha1.EphemeralRunnerSetPhase
specRevision int64
appliedRevision int64
want bool
}{
{
name: "nil-safe: running set is not outdated",
phase: v1alpha1.EphemeralRunnerSetPhaseRunning,
want: false,
},
{
name: "outdated against the spec it is running",
phase: v1alpha1.EphemeralRunnerSetPhaseOutdated,
specRevision: 4,
appliedRevision: 4,
want: true,
},
{
name: "outdated verdict predates a spec update that is still propagating",
phase: v1alpha1.EphemeralRunnerSetPhaseOutdated,
specRevision: 5,
appliedRevision: 4,
want: false,
},
{
name: "legacy set with no revisions recorded still tears down",
phase: v1alpha1.EphemeralRunnerSetPhaseOutdated,
specRevision: 0,
appliedRevision: 0,
want: true,
},
{
name: "running set with a pending revision is not outdated",
phase: v1alpha1.EphemeralRunnerSetPhaseRunning,
specRevision: 5,
appliedRevision: 4,
want: false,
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
ephemeralRunnerSet := &v1alpha1.EphemeralRunnerSet{
Spec: v1alpha1.EphemeralRunnerSetSpec{ActionableRevision: tt.specRevision},
Status: v1alpha1.EphemeralRunnerSetStatus{Phase: tt.phase, AppliedActionableRevision: tt.appliedRevision},
}
assert.Equal(t, tt.want, ephemeralRunnerSetOutdatedForAppliedRevision(ephemeralRunnerSet))
})
}
assert.False(t, ephemeralRunnerSetOutdatedForAppliedRevision(nil))
}
func runnerNames(runners []*v1alpha1.EphemeralRunner) []string {
if len(runners) == 0 {
return nil
}
names := make([]string, 0, len(runners))
for _, runner := range runners {
names = append(names, runner.Name)
}
return names
}