From 7e91d3b9823b52fe6d0f563d692c8af57faa6005 Mon Sep 17 00:00:00 2001 From: Jon Date: Mon, 12 Feb 2024 09:01:55 -0700 Subject: [PATCH] fix: improve cmd failure messaging (#2301) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit …s with no retries ## Description Add check to see whether action failure was actually due to timeout or not. Currently Zarf reports an error of "timed out after 0 seconds" when a `cmd` within an action fails (with no retries) even if no timeout was set. ## Related Issue Fixes #2299 ## Type of change - [X] Bug fix (non-breaking change which fixes an issue) ## Checklist before merging - [X] Test, docs, adr added or updated as needed - [X] [Contributor Guide Steps](https://github.com/defenseunicorns/zarf/blob/main/CONTRIBUTING.md#developer-workflow) followed --------- Co-authored-by: Wayne Starr --- src/pkg/packager/actions.go | 11 +++++++---- src/test/e2e/02_component_actions_test.go | 2 ++ 2 files changed, 9 insertions(+), 4 deletions(-) diff --git a/src/pkg/packager/actions.go b/src/pkg/packager/actions.go index 78340d515f..f565bb727a 100644 --- a/src/pkg/packager/actions.go +++ b/src/pkg/packager/actions.go @@ -160,12 +160,15 @@ retryCmd: select { case <-timeout: - // If we reached this point, the timeout was reached. - return fmt.Errorf("command \"%s\" timed out after %d seconds", cmdEscaped, cfg.MaxTotalSeconds) - + // If we reached this point, the timeout was reached or command failed with no retries. + if cfg.MaxTotalSeconds < 1 { + return fmt.Errorf("command %q failed after %d retries", cmdEscaped, cfg.MaxRetries) + } else { + return fmt.Errorf("command %q timed out after %d seconds", cmdEscaped, cfg.MaxTotalSeconds) + } default: // If we reached this point, the retry limit was reached. - return fmt.Errorf("command \"%s\" failed after %d retries", cmdEscaped, cfg.MaxRetries) + return fmt.Errorf("command %q failed after %d retries", cmdEscaped, cfg.MaxRetries) } } diff --git a/src/test/e2e/02_component_actions_test.go b/src/test/e2e/02_component_actions_test.go index bb3b25ffd3..6eda343103 100644 --- a/src/test/e2e/02_component_actions_test.go +++ b/src/test/e2e/02_component_actions_test.go @@ -150,5 +150,7 @@ func TestComponentActions(t *testing.T) { stdOut, stdErr, err = e2e.Zarf("package", "deploy", path, "--components=on-deploy-immediate-failure", "--confirm") require.Error(t, err, stdOut, stdErr) require.Contains(t, stdErr, "Failed to deploy package") + // regression test to ensure that failed commands are not erroneously flagged as a timeout + require.NotContains(t, stdErr, "timed out") }) }