From a6304f8a4d69ba37c6058cd4e21cbbfb0968e9c2 Mon Sep 17 00:00:00 2001 From: dblinkhorn Date: Fri, 20 Dec 2024 14:13:46 -0800 Subject: [PATCH 1/4] print workflow uri immediately instead of waiting for completion when using --wait flag --- .../cmd/trigger-argo-workflow/argo.go | 45 ++++++++++++++----- 1 file changed, 35 insertions(+), 10 deletions(-) diff --git a/actions/trigger-argo-workflow/cmd/trigger-argo-workflow/argo.go b/actions/trigger-argo-workflow/cmd/trigger-argo-workflow/argo.go index ea59cdf1f..f9e0d1bc2 100644 --- a/actions/trigger-argo-workflow/cmd/trigger-argo-workflow/argo.go +++ b/actions/trigger-argo-workflow/cmd/trigger-argo-workflow/argo.go @@ -1,6 +1,7 @@ package main import ( + "bufio" "bytes" "fmt" "io" @@ -77,22 +78,48 @@ func (a App) runCmd(md GitHubActionsMetadata) (string, string, error) { args := a.args(md) cmd := exec.Command("argo", args...) - cmdOutput := &bytes.Buffer{} - cmd.Env = a.env() - cmd.Stdout = cmdOutput + + stdoutPipe, err := cmd.StdoutPipe() + if err != nil { + return "", "", fmt.Errorf("failed to get stdout pipe: %w", err) + } + cmd.Stderr = os.Stderr a.logger.With("executable", "argo", "command", cmd.Args, "retries", a.retries).Debug("running command") - err := cmd.Run() - if err != nil { - return "", "", err + if err := cmd.Start(); err != nil { + return "", "", fmt.Errorf("failed to start command: %w", err) } - uri, output := a.outputWithURI(cmdOutput) + var uri string + var outputBuilder strings.Builder + + scanner := bufio.NewScanner(stdoutPipe) + + for scanner.Scan() { + line := scanner.Text() + outputBuilder.WriteString(line + "\n") + + matches := nameRe.FindStringSubmatch(line) - return uri, output, nil + if len(matches) == 2 && uri == "" { + uri = fmt.Sprintf("https://%s/workflows/%s/%s", a.server(), a.namespace, matches[1]) + + a.logger.With("uri", uri).Info("workflow URI") + } + } + + if err := scanner.Err(); err != nil { + return uri, outputBuilder.String(), fmt.Errorf("error reading command output: %w", err) + } + + if err := cmd.Wait(); err != nil { + return uri, outputBuilder.String(), fmt.Errorf("command failed: %w", err) + } + + return uri, outputBuilder.String(), nil } func (a *App) setURIAsJobOutput(uri string, writer io.Writer) { @@ -167,8 +194,6 @@ func (a *App) Run(md GitHubActionsMetadata) error { return err } - a.logger.With("uri", uri).Info("workflow URI") - writer := a.openGitHubOutput() defer writer.Close() From e615fb150dfbc3731c45f600d0816f11af55113f Mon Sep 17 00:00:00 2001 From: dblinkhorn Date: Fri, 20 Dec 2024 23:42:56 -0800 Subject: [PATCH 2/4] replaced outputWithURI handling of scanning, matching, & logging uri that was in runCmd --- .../cmd/trigger-argo-workflow/argo.go | 53 ++++++++----------- .../cmd/trigger-argo-workflow/argo_test.go | 9 ++-- 2 files changed, 28 insertions(+), 34 deletions(-) diff --git a/actions/trigger-argo-workflow/cmd/trigger-argo-workflow/argo.go b/actions/trigger-argo-workflow/cmd/trigger-argo-workflow/argo.go index f9e0d1bc2..518c67463 100644 --- a/actions/trigger-argo-workflow/cmd/trigger-argo-workflow/argo.go +++ b/actions/trigger-argo-workflow/cmd/trigger-argo-workflow/argo.go @@ -2,7 +2,6 @@ package main import ( "bufio" - "bytes" "fmt" "io" "log/slog" @@ -59,19 +58,28 @@ func (a App) env() []string { var nameRe = regexp.MustCompile(`^Name:\s+(.+)`) -func (a App) outputWithURI(input *bytes.Buffer) (string, string) { - output := strings.TrimSuffix(input.String(), "\n") +func (a App) outputWithURI(reader io.Reader) (string, string, error) { + scanner := bufio.NewScanner(reader) - matches := nameRe.FindStringSubmatch(output) + var uri string + var outputBuilder strings.Builder - if len(matches) != 2 { - a.logger.Warn("Couldn't find workflow name in output - can't construct URI for launched workflow") - return "", output + for scanner.Scan() { + line := scanner.Text() + outputBuilder.WriteString(line + "\n") + + matches := nameRe.FindStringSubmatch(line) + if len(matches) == 2 && uri == "" { + uri = fmt.Sprintf("https://%s/workflows/%s/%s", a.server(), a.namespace, matches[1]) + a.logger.With("uri", uri).Info("workflow URI") + } } - uri := fmt.Sprintf("https://%s/workflows/%s/%s", a.server(), a.namespace, matches[1]) + if err := scanner.Err(); err != nil { + return uri, outputBuilder.String(), fmt.Errorf("error reading command output: %w", err) + } - return uri, output + return uri, outputBuilder.String(), nil } func (a App) runCmd(md GitHubActionsMetadata) (string, string, error) { @@ -93,33 +101,16 @@ func (a App) runCmd(md GitHubActionsMetadata) (string, string, error) { return "", "", fmt.Errorf("failed to start command: %w", err) } - var uri string - var outputBuilder strings.Builder - - scanner := bufio.NewScanner(stdoutPipe) - - for scanner.Scan() { - line := scanner.Text() - outputBuilder.WriteString(line + "\n") - - matches := nameRe.FindStringSubmatch(line) - - if len(matches) == 2 && uri == "" { - uri = fmt.Sprintf("https://%s/workflows/%s/%s", a.server(), a.namespace, matches[1]) - - a.logger.With("uri", uri).Info("workflow URI") - } - } - - if err := scanner.Err(); err != nil { - return uri, outputBuilder.String(), fmt.Errorf("error reading command output: %w", err) + uri, out, scanErr := a.outputWithURI(stdoutPipe) + if scanErr != nil { + return uri, out, scanErr } if err := cmd.Wait(); err != nil { - return uri, outputBuilder.String(), fmt.Errorf("command failed: %w", err) + return uri, out, fmt.Errorf("command failed: %w", err) } - return uri, outputBuilder.String(), nil + return uri, out, nil } func (a *App) setURIAsJobOutput(uri string, writer io.Writer) { diff --git a/actions/trigger-argo-workflow/cmd/trigger-argo-workflow/argo_test.go b/actions/trigger-argo-workflow/cmd/trigger-argo-workflow/argo_test.go index 858f56eaa..58a1d98f3 100644 --- a/actions/trigger-argo-workflow/cmd/trigger-argo-workflow/argo_test.go +++ b/actions/trigger-argo-workflow/cmd/trigger-argo-workflow/argo_test.go @@ -71,8 +71,8 @@ Namespace: argo ServiceAccount: unset Status: Pending Created: Wed Dec 13 00:00:00 +0000 (now) -Progress: -Parameters: +Progress: +Parameters: message: world ` @@ -115,9 +115,12 @@ Parameters: a.instance = tt.instance a.namespace = "argo" - uri, _ := a.outputWithURI(bytes.NewBuffer([]byte(tt.input))) + reader := bytes.NewBufferString(tt.input) + uri, out, err := a.outputWithURI(reader) + require.NoError(t, err, "unexpected error reading command output") require.Equal(t, tt.expected, uri) + require.Equal(t, tt.input, out) }) } } From a0e3197e7ef513431463d8ab4a79cd59a4ccbc6c Mon Sep 17 00:00:00 2001 From: dblinkhorn Date: Thu, 2 Jan 2025 09:22:18 -0800 Subject: [PATCH 3/4] added defer to runCmd to handle cleanup of pipe & cmd & improved uri if check in outputWithURI --- .../cmd/trigger-argo-workflow/argo.go | 23 +++++++++++++++---- 1 file changed, 19 insertions(+), 4 deletions(-) diff --git a/actions/trigger-argo-workflow/cmd/trigger-argo-workflow/argo.go b/actions/trigger-argo-workflow/cmd/trigger-argo-workflow/argo.go index 518c67463..ad628e217 100644 --- a/actions/trigger-argo-workflow/cmd/trigger-argo-workflow/argo.go +++ b/actions/trigger-argo-workflow/cmd/trigger-argo-workflow/argo.go @@ -68,10 +68,12 @@ func (a App) outputWithURI(reader io.Reader) (string, string, error) { line := scanner.Text() outputBuilder.WriteString(line + "\n") - matches := nameRe.FindStringSubmatch(line) - if len(matches) == 2 && uri == "" { - uri = fmt.Sprintf("https://%s/workflows/%s/%s", a.server(), a.namespace, matches[1]) - a.logger.With("uri", uri).Info("workflow URI") + if uri == "" { + matches := nameRe.FindStringSubmatch(line) + if len(matches) == 2 { + uri = fmt.Sprintf("https://%s/workflows/%s/%s", a.server(), a.namespace, matches[1]) + a.logger.With("uri", uri).Info("workflow URI") + } } } @@ -101,6 +103,18 @@ func (a App) runCmd(md GitHubActionsMetadata) (string, string, error) { return "", "", fmt.Errorf("failed to start command: %w", err) } + waitCalled := false + pipeClosed := false + + defer func() { + if !pipeClosed { + _ = stdoutPipe.Close() + } + if !waitCalled { + _ = cmd.Wait() + } + }() + uri, out, scanErr := a.outputWithURI(stdoutPipe) if scanErr != nil { return uri, out, scanErr @@ -109,6 +123,7 @@ func (a App) runCmd(md GitHubActionsMetadata) (string, string, error) { if err := cmd.Wait(); err != nil { return uri, out, fmt.Errorf("command failed: %w", err) } + waitCalled = true return uri, out, nil } From 2701aec902c255120dd89f5d120ca6ba02cb78a9 Mon Sep 17 00:00:00 2001 From: dblinkhorn Date: Fri, 3 Jan 2025 07:58:05 -0800 Subject: [PATCH 4/4] moved pipe & cmd cleanup to scanErr if block from defer func --- .../cmd/trigger-argo-workflow/argo.go | 15 ++------------- 1 file changed, 2 insertions(+), 13 deletions(-) diff --git a/actions/trigger-argo-workflow/cmd/trigger-argo-workflow/argo.go b/actions/trigger-argo-workflow/cmd/trigger-argo-workflow/argo.go index ad628e217..9a888648e 100644 --- a/actions/trigger-argo-workflow/cmd/trigger-argo-workflow/argo.go +++ b/actions/trigger-argo-workflow/cmd/trigger-argo-workflow/argo.go @@ -103,27 +103,16 @@ func (a App) runCmd(md GitHubActionsMetadata) (string, string, error) { return "", "", fmt.Errorf("failed to start command: %w", err) } - waitCalled := false - pipeClosed := false - - defer func() { - if !pipeClosed { - _ = stdoutPipe.Close() - } - if !waitCalled { - _ = cmd.Wait() - } - }() - uri, out, scanErr := a.outputWithURI(stdoutPipe) if scanErr != nil { + _ = stdoutPipe.Close() + _ = cmd.Wait() return uri, out, scanErr } if err := cmd.Wait(); err != nil { return uri, out, fmt.Errorf("command failed: %w", err) } - waitCalled = true return uri, out, nil }