diff --git a/internal/agent/agent.go b/internal/agent/agent.go index 0eb72e2..7c70a47 100644 --- a/internal/agent/agent.go +++ b/internal/agent/agent.go @@ -318,6 +318,11 @@ type notComposedMark struct { // name, and only the node's composition of it failed. const NotComposedDetail = "the node cannot compose it, and keeps running what it ran: " +// composerMark says h is a mark NotComposed wrote: failed, its detail opening with NotComposedDetail. +func composerMark(h Have) bool { + return h.Phase == string(delivery.PhaseFailed) && strings.HasPrefix(h.Detail, NotComposedDetail) +} + // HavesReadErr is what a Haves may answer besides Read: a have absent (false, nil) told from one that could not be read // (false, the error). type HavesReadErr interface { @@ -375,6 +380,9 @@ func (a *Agent) NotComposed(ctx context.Context, why string) { a.mu.Unlock() sort.Strings(modules) for _, module := range modules { + if ctx.Err() != nil { + return // the run ended: what it would mark or clear is the next run's to say (issue 1126) + } a.mu.Lock() mark, marked := a.notComposed[module] r, ran := a.running[module] @@ -405,6 +413,9 @@ func (a *Agent) NotComposed(ctx context.Context, why string) { } else if h, found, unread := a.readHave(ctx, module); found { mark.prior = h // what its last have said runs: this agent has run nothing of it since its start } else if unread { + if ctx.Err() != nil { + return + } // What its have names cannot be read (the bus paused, issue 1126): a mark written now would drop the build // and digest it names. Nothing is written; the next refusal marks it once the have reads. a.mu.Lock() @@ -413,6 +424,19 @@ func (a *Agent) NotComposed(ctx context.Context, why string) { continue } } + // A have that already says another cause (an apply that failed, a backup held, an apply under way) keeps it: a + // composer's mark over it would hide that cause, and baseline would take the mark for what runs (review of the + // switch, round 1). + if p := mark.prior.Phase; !marked && p != "" && p != string(delivery.PhaseRunning) && !composerMark(mark.prior) { + a.mu.Lock() + a.sayOnce("unmarked-cause:"+module, fmt.Sprintf("%s is not marked: its have says %s (%s), and the node cannot compose it: %s", + module, p, mark.prior.Detail, why)) + a.mu.Unlock() + continue + } + if ctx.Err() != nil { + return + } mark.why = why a.mu.Lock() a.notComposed[module] = mark @@ -948,7 +972,7 @@ func (a *Agent) baseline(ctx context.Context, module string, w wantNow, comp com // A have a composer marked failed (NotComposed) names what still runs: taken as running, so a mark this // agent did not make clears once what it names is what the want names (issue 1126, the stale marks after // the seed's revert). - marked := h.Phase == string(delivery.PhaseFailed) && strings.HasPrefix(h.Detail, NotComposedDetail) + marked := composerMark(h) if marked { h.Detail = "" } diff --git a/internal/agent/agent_test.go b/internal/agent/agent_test.go index d9d1e67..c3d4a56 100644 --- a/internal/agent/agent_test.go +++ b/internal/agent/agent_test.go @@ -342,6 +342,21 @@ func TestStartingOnWhatTheKeptDeclarationHoldsAppliesNothing(t *testing.T) { // What the node-engine itself last applied is what runs after a restart, not the kept declaration. func TestStartingOnItsOwnHaveAppliesNothing(t *testing.T) { t.Run("says the have again", testStartingOnItsOwnHaveSaysTheHaveAgainSoItsHealthIsThisEngines) + // An apply that failed is not what runs, though its have names the want's build and digest: the agent starting on + // it applies again (review of the switch, round 1). + t.Run("applies again after an apply failure", func(t *testing.T) { + b := newBed(t) + fp := fpOf("a") + ck, craw, dig := composition(t, "avahi", fp, false, file("avahi", "x", "1")) + wk, wraw := wantFor(t, "avahi", fp, dig, "") + b.haves.stored = map[string]Have{"avahi": {Module: "avahi", FP: fp, Composed: dig, Phase: string(delivery.PhaseFailed), + Detail: "apply: unit failed to start"}} + b.comps.before(ck, craw) + b.wants.before(wk, wraw) + stop := b.run() + defer stop() + eventually(t, "a failed apply was not applied again", func() bool { return len(b.appliedList()) == 1 }) + }) // Issue 1126, the stale marks after the seed's revert: a have a composer marked failed names what still runs; the // agent starting on it applies nothing and says it running, so a mark it did not make clears. t.Run("clears a composer's mark", func(t *testing.T) { @@ -1084,6 +1099,11 @@ func TestTheNodesOwnCompositionIsAppliedWhenItDiffersAndARecordMoveWaitsForAYes( b.haves.mu.Unlock() want("unread", fpOf("d"), "") want("fresh", fpOf("e"), "") // never ran, its have absent: marked, naming nothing it runs + // A have that already says another cause keeps it (review of the switch, round 1): an apply that failed. + brokenFP, brokenDig := fpOf("f"), "sha256:"+strings.Repeat("8", 64) + b.haves.Write(context.Background(), Have{Module: "broken", FP: brokenFP, Composed: brokenDig, Phase: string(delivery.PhaseFailed), + Detail: "apply: unit failed to start"}) + want("broken", brokenFP, "") eventually(t, "unread's want was not taken", func() bool { b.ag.mu.Lock() defer b.ag.mu.Unlock() @@ -1096,6 +1116,17 @@ func TestTheNodesOwnCompositionIsAppliedWhenItDiffersAndARecordMoveWaitsForAYes( if h := b.haves.last("fresh"); h.Phase != string(delivery.PhaseFailed) || h.FP != "" { t.Errorf("a module that never ran, its have absent, is not marked: %+v", h) } + if h := b.haves.last("broken"); h.Detail != "apply: unit failed to start" || h.Composed != brokenDig { + t.Errorf("an apply failure was overwritten by a composer's mark: %+v", h) + } + // A run that ended marks nothing: what it would say is the next run's (review of the switch, round 1). + ended, end := context.WithCancel(context.Background()) + end() + before := len(b.haves.log) + b.ag.NotComposed(ended, "a refusal from a run that ended") + if len(b.haves.log) != before { + t.Errorf("a run that ended wrote %d haves", len(b.haves.log)-before) + } b.ag.NotComposed(context.Background(), "") for _, row := range []struct{ module, composed string }{{"web", dig}, {"idle", idleDig}} { if h := b.haves.last(row.module); h.Phase != string(delivery.PhaseRunning) || h.Composed != row.composed || len(h.Sealed) != 1 {