From 071921ff41178b1e53ac2b698cc7ee263d2bd0ce Mon Sep 17 00:00:00 2001 From: Marco Iorio Date: Mon, 21 Sep 2026 11:52:25 +0200 Subject: [PATCH 1/2] script: output last error upon retries failure Commands that are retried may only eventually fail when the state context is canceled. However, at that point, the error that gets output is not informative, given that it references the context cancellation. Let's instead output the last error that triggered a retry, so that the cause of the failure is immediately visible. Signed-off-by: Marco Iorio --- script/engine.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/script/engine.go b/script/engine.go index a1f2d19..8bd2217 100644 --- a/script/engine.go +++ b/script/engine.go @@ -365,7 +365,7 @@ func (e *Engine) Execute(s *State, file string, script *bufio.Reader, log io.Wri select { case <-s.Context().Done(): s.RetryCount = 0 - return lineErr(s.Context().Err()) + return lineErr(err) case <-time.After(retryDuration): } s.RetryCount++ From 566ad3b32f8ef3a4f96b84c8a2c1439809408981 Mon Sep 17 00:00:00 2001 From: Marco Iorio Date: Mon, 21 Sep 2026 12:29:49 +0200 Subject: [PATCH 2/2] script: add optional limit to retrying commands retries Currently, the number of retries of retrying commands is only limited by the context; however, this can be limiting in certain circumstances, where one may want a context with a relatively long lifetime, and separately limit the number of retries. Hence, let's introduce the possibility of configuring a maximum number of retries; by default, no maximum is configured, and the previous behavior is preserved. Signed-off-by: Marco Iorio --- script/engine.go | 10 +++++++ script_test.go | 78 ++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 88 insertions(+) diff --git a/script/engine.go b/script/engine.go index 8bd2217..bdd579a 100644 --- a/script/engine.go +++ b/script/engine.go @@ -86,6 +86,11 @@ type Engine struct { // MaxRetryInterval is the maximum time to wait before retrying. MaxRetryInterval time.Duration + + // MaxRetries is the maximum number of times (excluding the first one) a + // retrying command marked with '*' is executed before giving up. If zero, + // the number of retries is bound only by the context. + MaxRetries uint } // NewEngine returns an Engine configured with a basic set of commands and conditions. @@ -360,6 +365,11 @@ func (e *Engine) Execute(s *State, file string, script *bufio.Reader, log io.Wri // Command wants retries. Retry the whole section backoff := exponentialBackoff{max: maxRetryInterval, interval: retryInterval} for err != nil { + if e.MaxRetries != 0 && s.RetryCount >= int(e.MaxRetries) { + s.RetryCount = 0 + return lineErr(err) + } + retryDuration := backoff.get() fmt.Fprintf(log, "(command %q failed, retrying in %s...)\n", line, retryDuration) select { diff --git a/script_test.go b/script_test.go index 0838b58..d46ae51 100644 --- a/script_test.go +++ b/script_test.go @@ -7,8 +7,10 @@ import ( "bufio" "bytes" "context" + "fmt" "strings" "testing" + "time" "github.com/cilium/hive" "github.com/cilium/hive/cell" @@ -73,3 +75,79 @@ hive/stop expected := `> hive/start.*> example1.*hello1.*> example2.*hello2.*> hive/stop` require.Regexp(t, expected, strings.ReplaceAll(stdout.String(), "\n", " ")) } + +func TestScriptCommandRetries(t *testing.T) { + var tests = []struct { + name string + succeedAfter uint + cancelAfter uint + maxRetries uint + assert require.ErrorAssertionFunc + }{ + { + name: "max two retries, succeed after two", + succeedAfter: 2, + maxRetries: 2, + assert: require.NoError, + }, + { + name: "max two retries, succeed after three", + succeedAfter: 3, + maxRetries: 2, + assert: func(tt require.TestingT, err error, args ...any) { + require.ErrorContains(t, err, "expected to succeed after 3 times, current: 2", args...) + }, + }, + { + name: "no limit, context cancellation only", + succeedAfter: 1000, + cancelAfter: 10, + assert: func(tt require.TestingT, err error, args ...any) { + require.ErrorContains(t, err, "expected to succeed after 1000 times, current: 10", args...) + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + + var ( + counter uint + engine = script.Engine{ + Cmds: map[string]script.Cmd{ + "test": script.Command( + script.CmdUsage{}, + func(s *script.State, args ...string) (script.WaitFunc, error) { + defer func() { counter++ }() + + s.Logf("test command called %d times", counter) + if tt.cancelAfter != 0 && counter == tt.cancelAfter { + cancel() + } + + if counter != tt.succeedAfter { + return nil, fmt.Errorf("expected to succeed after %d times, current: %d", tt.succeedAfter, counter) + } + + return nil, nil + }, + ), + }, + + RetryInterval: 10 * time.Millisecond, + MaxRetryInterval: 10 * time.Millisecond, + MaxRetries: tt.maxRetries, + } + ) + + s, err := script.NewState(ctx, t.TempDir(), nil) + require.NoError(t, err, "NewState") + + var stdout bytes.Buffer + err = engine.Execute(s, "", bufio.NewReader(strings.NewReader("* test")), &stdout) + tt.assert(t, err) + }) + } +}