Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixed: failed to save outputs: verify serviceaccount default:default has necessary privileges #1362

Merged
merged 14 commits into from
May 31, 2019
Merged
7 changes: 4 additions & 3 deletions cmd/argoexec/commands/root.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package commands

import (
"encoding/json"
"github.com/argoproj/argo/errors"
"os"

"github.com/argoproj/pkg/cli"
Expand Down Expand Up @@ -66,7 +67,7 @@ func NewRootCommand() *cobra.Command {

func initExecutor() *executor.WorkflowExecutor {
config, err := clientConfig.ClientConfig()
checkErr(err)
checkErr(errors.Wrap(err, errors.CodeInternal, "Failed to initialize client config in Sidecar"))

namespace, _, err := clientConfig.Namespace()
checkErr(err)
Expand All @@ -80,7 +81,7 @@ func initExecutor() *executor.WorkflowExecutor {
}

tmpl, err := executor.LoadTemplate(podAnnotationsPath)
checkErr(err)
checkErr(errors.Wrap(err, errors.CodeInternal, "Failed to load the template"))

var cre executor.ContainerRuntimeExecutor
switch os.Getenv(common.EnvVarContainerRuntimeExecutor) {
Expand All @@ -93,7 +94,7 @@ func initExecutor() *executor.WorkflowExecutor {
default:
cre, err = docker.NewDockerExecutor()
}
checkErr(err)
checkErr(errors.Wrap(err, errors.CodeInternal, "Failed to initialize Executor"))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No need to wrap errors in our own libraries.


wfExecutor := executor.NewExecutor(clientset, podName, namespace, podAnnotationsPath, cre, *tmpl)
yamlBytes, _ := json.Marshal(&wfExecutor.Template)
Expand Down
16 changes: 9 additions & 7 deletions cmd/argoexec/commands/wait.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package commands

import (
"github.com/argoproj/argo/errors"
"time"

"github.com/argoproj/pkg/stats"
Expand Down Expand Up @@ -31,40 +32,41 @@ func waitContainer() error {
// Wait for main container to complete
err := wfExecutor.Wait()
if err != nil {
wfExecutor.AddError(err)

wfExecutor.AddError(errors.Wrap(err, errors.CodeInternal, " Wait container failed to wait for main container to complete "))
// do not return here so we can still try to kill sidecars & save outputs
}
err = wfExecutor.KillSidecars()
if err != nil {
wfExecutor.AddError(err)
wfExecutor.AddError(errors.Wrap(err, errors.CodeInternal, " Wait container kill failed"))
// do not return here so we can still try save outputs
}
logArt, err := wfExecutor.SaveLogs()
if err != nil {
wfExecutor.AddError(err)
wfExecutor.AddError(errors.Wrap(err, errors.CodeInternal, " Wait container failed to save the logs"))
return err
}
// Saving output parameters
err = wfExecutor.SaveParameters()
if err != nil {
wfExecutor.AddError(err)
wfExecutor.AddError(errors.Wrap(err, errors.CodeInternal, " Wait container failed to save the parameters"))
return err
}
// Saving output artifacts
err = wfExecutor.SaveArtifacts()
if err != nil {
wfExecutor.AddError(err)
wfExecutor.AddError(errors.Wrap(err, errors.CodeInternal, " Wait container failed to save the artifacts"))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No need to wrap errors coming from our own libraries.

return err
}
// Capture output script result
err = wfExecutor.CaptureScriptResult()
if err != nil {
wfExecutor.AddError(err)
wfExecutor.AddError(errors.Wrap(err, errors.CodeInternal, " Wait container failed to capture the script results"))
return err
}
err = wfExecutor.AnnotateOutputs(logArt)
if err != nil {
wfExecutor.AddError(err)
wfExecutor.AddError(errors.Wrap(err, errors.CodeInternal, " Wait container failed to annotate the outputs"))
return err
}
return nil
Expand Down
4 changes: 2 additions & 2 deletions workflow/executor/executor.go
Original file line number Diff line number Diff line change
Expand Up @@ -180,7 +180,7 @@ func (we *WorkflowExecutor) LoadArtifacts() error {
if art.Mode != nil {
err = os.Chmod(artPath, os.FileMode(*art.Mode))
if err != nil {
return errors.InternalWrapError(err)
return errors.InternalWrapError(err, "Container failed to load the artifacts")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please leave original code errors.InternalWrapError(err). The original message is lost when you give it your own.

}
}
}
Expand All @@ -205,7 +205,7 @@ func (we *WorkflowExecutor) StageFiles() error {
}
err := ioutil.WriteFile(filePath, body, 0644)
if err != nil {
return errors.InternalWrapError(err)
return errors.InternalWrapError(err, "Container failed to load the stage files")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please leave original code errors.InternalWrapError(err). The original message is lost when you give it your own.

}
return nil
}
Expand Down