Skip to content

Commit

Permalink
env_*: log command output (#69)
Browse files Browse the repository at this point in the history
There are two instances where we forget to check the output of a command
when it fails. This commit ensures that we log STDOUT/STDERR in the same
way that we do whenever we capture `CombinedOutput`. Specifically, this
commit adds logging for the output when we fail to recursively chmod
the shared directory as part of the cleanup process.
One note here: recursively chmod-ing the shared directory could easily
fail if a container saves a file as root and the CI user is not root. We
might want to forget about chmod-ing and instead do a `rm -rf`, which
will delete files not owned by the user executing the command.

Signed-off-by: Lucas Servén Marín <[email protected]>
  • Loading branch information
squat authored Apr 18, 2024
1 parent 6d823e2 commit c5a4b7c
Show file tree
Hide file tree
Showing 2 changed files with 4 additions and 7 deletions.
3 changes: 2 additions & 1 deletion env_docker.go
Original file line number Diff line number Diff line change
Expand Up @@ -819,7 +819,8 @@ func (e *DockerEnvironment) close() {
}

if e.dir != "" {
if err := e.exec("chmod", "-R", "777", e.dir).Run(); err != nil {
if out, err := e.exec("chmod", "-R", "777", e.dir).CombinedOutput(); err != nil {
e.logger.Log(string(out))
e.logger.Log("Error while chmod sharedDir", e.dir, "err:", err)
}
if err := os.RemoveAll(e.dir); err != nil {
Expand Down
8 changes: 2 additions & 6 deletions env_kind.go
Original file line number Diff line number Diff line change
Expand Up @@ -451,7 +451,8 @@ func (e *KindEnvironment) close() {
}

if e.dir != "" {
if err := e.exec("chmod", "-R", "777", e.dir).Run(); err != nil {
if out, err := e.exec("chmod", "-R", "777", e.dir).CombinedOutput(); err != nil {
e.logger.Log(string(out))
e.logger.Log("Error while chmod sharedDir", e.dir, "err:", err)
}
if err := os.RemoveAll(e.dir); err != nil {
Expand Down Expand Up @@ -779,11 +780,6 @@ func (r *kindRunnable) prePullImage(ctx context.Context) (err error) {
return errors.Wrapf(err, "docker image %q failed to download", r.opts.Image)
}

if err := r.env.execContext(ctx, "docker", "image", "inspect", r.opts.Image).Run(); err == nil {
return errors.Wrapf(err, "load image %q into cluster", r.opts.Image)

}

return nil
}

Expand Down

0 comments on commit c5a4b7c

Please sign in to comment.