diff --git a/cmd/mesh-controller/plan_retry.go b/cmd/mesh-controller/plan_retry.go index 38171760..9556bef5 100644 --- a/cmd/mesh-controller/plan_retry.go +++ b/cmd/mesh-controller/plan_retry.go @@ -407,22 +407,37 @@ func retryPlan(ctx context.Context, open *stores, id string) (string, error) { } } waiting := busWaitingIn(p) - resumed(&p, fmt.Sprintf("tier %d retried by hand: %s asked again", p.Tier, strings.Join(slicesWithout(failed, waiting), ", "))) - if len(waiting) > 0 { - p.Note += "; " + busWaitWords(p, waiting) - } + note, said := retryWords(p, failed, asked, waiting) + resumed(&p, note) if err := inv.SavePlan(ctx, &p); err != nil { return "", err } if p.State != inventory.PlanBuilding { return "", fmt.Errorf("%s could not be resumed: %s", p.ID, p.Note) } - said := fmt.Sprintf("%s retried at tier %d of %d: asked %s; the plan goes on from there as any plan does", - p.ID, p.Tier, len(p.Tiers), strings.Join(asked, ", ")) + return said, nil +} + +// retryWords are the plan's note and the verb's answer for a retry: what was asked again (as the builds it got), and what waits +// for the bus. A retry whose modules all wait for the bus asked none, and says so rather than "asked" with nothing after it +// (issue 1403). +func retryWords(p inventory.Plan, failed, asked, waiting []string) (note, said string) { + again := slicesWithout(failed, waiting) + switch { + case len(again) > 0: + note = fmt.Sprintf("tier %d retried by hand: %s asked again", p.Tier, strings.Join(again, ", ")) + said = fmt.Sprintf("%s retried at tier %d of %d: asked %s; the plan goes on from there as any plan does", + p.ID, p.Tier, len(p.Tiers), strings.Join(asked, ", ")) + default: + note = fmt.Sprintf("tier %d retried by hand: nothing could be asked yet", p.Tier) + said = fmt.Sprintf("%s retried at tier %d of %d: nothing asked yet; the plan goes on from there as any plan does", + p.ID, p.Tier, len(p.Tiers)) + } if len(waiting) > 0 { + note += "; " + busWaitWords(p, waiting) said += "; " + busWaitWords(p, waiting) } - return said, nil + return note, said } // joinAPlan asks a module again for the plan that holds it unbuilt or failed, if one does: open plans diff --git a/cmd/mesh-controller/retry_words_test.go b/cmd/mesh-controller/retry_words_test.go new file mode 100644 index 00000000..4796ff52 --- /dev/null +++ b/cmd/mesh-controller/retry_words_test.go @@ -0,0 +1,45 @@ +package main + +import ( + "strings" + "testing" + + "github.com/novox/mesh-controller/internal/inventory" +) + +// Issue 1403: a retry whose every module still waits for the bus asked nothing, and does not say "asked" with an empty list. +func TestARetryWhoseModulesAllWaitForTheBusSaysNothingWasAsked(t *testing.T) { + p := inventory.Plan{ID: "plan-9", Tier: 1, Tiers: [][]string{{"a"}, {"b", "c"}}} + note, said := retryWords(p, []string{"b", "c"}, nil, []string{"b", "c"}) + for _, text := range []string{note, said} { + if strings.Contains(text, "asked ;") || strings.Contains(text, "asked ") || strings.HasSuffix(strings.Split(text, ";")[0], "asked") { + t.Errorf("a retry that asked nothing says %q", text) + } + if !strings.Contains(text, "b, c not asked yet: the bus did not answer") { + t.Errorf("it does not say what waits for the bus: %q", text) + } + } + if !strings.Contains(said, "nothing asked yet") || !strings.Contains(note, "nothing could be asked yet") { + t.Errorf("it does not say that nothing was asked: %q / %q", said, note) + } +} + +func TestARetryThatAskedSomeAndLeftSomeForTheBusSaysBoth(t *testing.T) { + p := inventory.Plan{ID: "plan-9", Tier: 0, Tiers: [][]string{{"a", "b"}}} + note, said := retryWords(p, []string{"a", "b"}, []string{"a as build-1"}, []string{"b"}) + if !strings.Contains(note, "a asked again") || strings.Contains(note, "a, b asked again") { + t.Errorf("the note names b as asked again: %q", note) + } + if !strings.Contains(said, "asked a as build-1;") || !strings.Contains(said, "b not asked yet: the bus did not answer") { + t.Errorf("the answer says %q", said) + } +} + +func TestARetryThatAskedEverythingSaysSoAsBefore(t *testing.T) { + p := inventory.Plan{ID: "plan-9", Tier: 0, Tiers: [][]string{{"a"}}} + note, said := retryWords(p, []string{"a"}, []string{"a as build-1"}, nil) + if note != "tier 0 retried by hand: a asked again" || + said != "plan-9 retried at tier 0 of 1: asked a as build-1; the plan goes on from there as any plan does" { + t.Errorf("a retry that asked everything says %q / %q", note, said) + } +}