From 5c66171903dff37451f62729f71458de22d3c846 Mon Sep 17 00:00:00 2001 From: Pasquale Congiusti Date: Mon, 31 Oct 2022 10:43:34 +0100 Subject: [PATCH] feat(build): parse cmd execution for errors Closes #3779 --- pkg/util/command.go | 57 +++++++++++++++++++----------- pkg/util/command_test.go | 60 ++++++++++++++++++++++++++++++++ pkg/util/jvm/keystore.go | 4 +-- pkg/util/maven/maven_log.go | 19 ++++++++-- pkg/util/maven/maven_log_test.go | 35 +++++++++++++++++++ 5 files changed, 150 insertions(+), 25 deletions(-) create mode 100644 pkg/util/command_test.go create mode 100644 pkg/util/maven/maven_log_test.go diff --git a/pkg/util/command.go b/pkg/util/command.go index a3de8522d1..37d3262deb 100644 --- a/pkg/util/command.go +++ b/pkg/util/command.go @@ -21,14 +21,18 @@ import ( "bufio" "context" "fmt" + "io" "os/exec" + "github.com/pkg/errors" "golang.org/x/sync/errgroup" ) // RunAndLog starts the provided command, scans its standard and error outputs line by line, // to feed the provided handlers, and waits until the scans complete and the command returns. -func RunAndLog(ctx context.Context, cmd *exec.Cmd, stdOutF func(string), stdErrF func(string)) error { +func RunAndLog(ctx context.Context, cmd *exec.Cmd, stdOutF func(string) string, stdErrF func(string) string) error { + scanOutMsg := "" + scanErrMsg := "" stdOutF(fmt.Sprintf("Executed command: %s", cmd.String())) stdOut, err := cmd.StdoutPipe() @@ -40,38 +44,51 @@ func RunAndLog(ctx context.Context, cmd *exec.Cmd, stdOutF func(string), stdErrF return err } err = cmd.Start() + // if the command is in error, we try to figure it out why also by parsing the log if err != nil { - scanOut := bufio.NewScanner(stdOut) - for scanOut.Scan() { - stdOutF(scanOut.Text()) - } - scanErr := bufio.NewScanner(stdErr) - for scanErr.Scan() { - stdOutF(scanErr.Text()) - } - return err + scanOutMsg = scan(stdOut, stdOutF) + scanErrMsg = scan(stdErr, stdErrF) + + return errors.Wrapf(err, formatErr(scanOutMsg, scanErrMsg)) } g, _ := errgroup.WithContext(ctx) g.Go(func() error { - scanner := bufio.NewScanner(stdOut) - for scanner.Scan() { - stdOutF(scanner.Text()) - } + scanOutMsg = scan(stdOut, stdOutF) return nil }) g.Go(func() error { - scanner := bufio.NewScanner(stdErr) - for scanner.Scan() { - stdErrF(scanner.Text()) - } + scanErrMsg = scan(stdErr, stdErrF) return nil }) if err = g.Wait(); err != nil { - return err + return errors.Wrapf(err, formatErr(scanOutMsg, scanErrMsg)) } if err = cmd.Wait(); err != nil { - return err + return errors.Wrapf(err, formatErr(scanOutMsg, scanErrMsg)) } return nil } + +func scan(stdOut io.ReadCloser, stdOutF func(string) string) string { + scanError := "" + scanner := bufio.NewScanner(stdOut) + for scanner.Scan() { + errMsg := stdOutF(scanner.Text()) + if errMsg != "" && scanError == "" { + scanError = errMsg + } + } + + return scanError +} + +func formatErr(stdout, stderr string) string { + if stderr == "" { + return stdout + } + if stdout == "" { + return stderr + } + return fmt.Sprintf("stdout: %s, stderr: %s", stdout, stderr) +} diff --git a/pkg/util/command_test.go b/pkg/util/command_test.go new file mode 100644 index 0000000000..0831b22863 --- /dev/null +++ b/pkg/util/command_test.go @@ -0,0 +1,60 @@ +/* +Licensed to the Apache Software Foundation (ASF) under one or more +contributor license agreements. See the NOTICE file distributed with +this work for additional information regarding copyright ownership. +The ASF licenses this file to You under the Apache License, Version 2.0 +(the "License"); you may not use this file except in compliance with +the License. You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package util + +import ( + "context" + "fmt" + "os/exec" + "strings" + "testing" + + "github.com/stretchr/testify/assert" +) + +var ( + loggerInfo = func(s string) string { + fmt.Println("OUT:", s) + if strings.Contains(s, "invalid") { + return s + } + return "" + } + loggerError = func(s string) string { + fmt.Println("ERR:", s) + if strings.Contains(s, "invalid") { + return s + } + return "" + } +) + +func TestRunAndLog(t *testing.T) { + cmd := exec.CommandContext(context.Background(), "/usr/bin/date") + err := RunAndLog(context.Background(), cmd, loggerInfo, loggerError) + + assert.Nil(t, err) +} + +func TestRunAndLogInvalid(t *testing.T) { + cmd := exec.CommandContext(context.Background(), "/usr/bin/date", "-dsa") + err := RunAndLog(context.Background(), cmd, loggerInfo, loggerError) + + assert.NotNil(t, err) + assert.Equal(t, "/usr/bin/date: invalid date ‘sa’: exit status 1", err.Error()) +} diff --git a/pkg/util/jvm/keystore.go b/pkg/util/jvm/keystore.go index 600a23f1a7..9788fa3629 100644 --- a/pkg/util/jvm/keystore.go +++ b/pkg/util/jvm/keystore.go @@ -33,8 +33,8 @@ import ( var ( logger = log.WithName("keytool") - loggerInfo = func(s string) { logger.Info(s) } - loggerError = func(s string) { logger.Error(nil, s) } + loggerInfo = func(s string) string { logger.Info(s); return "" } + loggerError = func(s string) string { logger.Error(nil, s); return "" } ) func GenerateKeystore(ctx context.Context, keystoreDir, keystoreName, keystorePass string, data [][]byte) error { diff --git a/pkg/util/maven/maven_log.go b/pkg/util/maven/maven_log.go index 1c1c4ab023..21f9ec8897 100644 --- a/pkg/util/maven/maven_log.go +++ b/pkg/util/maven/maven_log.go @@ -19,6 +19,7 @@ package maven import ( "encoding/json" + "strings" "github.com/apache/camel-k/pkg/util/log" ) @@ -47,7 +48,7 @@ const ( var mavenLogger = log.WithName("maven.build") -func mavenLogHandler(s string) { +func mavenLogHandler(s string) string { mavenLog, parseError := parseLog(s) if parseError == nil { normalizeLog(mavenLog) @@ -57,6 +58,13 @@ func mavenLogHandler(s string) { // etc). The build may still have succeeded, though. nonNormalizedLog(s) } + + // Return the error message according to maven log + if strings.HasPrefix(s, "[ERROR]") { + return s + } + + return "" } func parseLog(line string) (mavenLog, error) { @@ -72,10 +80,15 @@ func normalizeLog(mavenLog mavenLog) { case INFO, WARN: mavenLogger.Info(mavenLog.Msg) case ERROR, FATAL: - mavenLogger.Errorf(nil, mavenLog.Msg) + mavenLogger.Error(nil, mavenLog.Msg) } } func nonNormalizedLog(rawLog string) { - mavenLogger.Info(rawLog) + // Distinguish an error message from the rest + if strings.HasPrefix(rawLog, "[ERROR]") { + mavenLogger.Error(nil, rawLog) + } else { + mavenLogger.Info(rawLog) + } } diff --git a/pkg/util/maven/maven_log_test.go b/pkg/util/maven/maven_log_test.go new file mode 100644 index 0000000000..86f27c0f3e --- /dev/null +++ b/pkg/util/maven/maven_log_test.go @@ -0,0 +1,35 @@ +/* +Licensed to the Apache Software Foundation (ASF) under one or more +contributor license agreements. See the NOTICE file distributed with +this work for additional information regarding copyright ownership. +The ASF licenses this file to You under the Apache License, Version 2.0 +(the "License"); you may not use this file except in compliance with +the License. You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package maven + +import ( + "context" + "os/exec" + "testing" + + "github.com/apache/camel-k/pkg/util" + "github.com/stretchr/testify/assert" +) + +func TestRunAndLogErrorMvn(t *testing.T) { + cmd := exec.CommandContext(context.Background(), "/usr/bin/mvn", "package") + err := util.RunAndLog(context.Background(), cmd, mavenLogHandler, mavenLogHandler) + + assert.NotNil(t, err) + assert.ErrorContains(t, err, "[ERROR] The goal you specified requires a project to execute but there is no POM in this directory") +}