Skip to content

Commit

Permalink
Fix unsafe data race in ExecuteCommand
Browse files Browse the repository at this point in the history
**What type of PR is this?**

/kind bug

**What does does this PR do / why we need it**:

Fixes a potential flake with regards to when writing to cmdOutput at the
same time that log.ErrorF(...) is reading from it.

**Which issue(s) this PR fixes**:

Closes #2828

**How to test changes / Special notes to the reviewer**:

N/A, honestly not too sure
  • Loading branch information
cdrage committed Jun 10, 2020
1 parent 46ea6c3 commit 1ee1fec
Showing 1 changed file with 11 additions and 1 deletion.
12 changes: 11 additions & 1 deletion pkg/exec/exec.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,23 +5,29 @@ import (
"fmt"
"io"
"os"
"sync"

"k8s.io/klog"

"github.com/openshift/odo/pkg/devfile/adapters/common"
"github.com/openshift/odo/pkg/log"
)

// ExecClient is a wrapper around ExecCMDInContainer which executes a command in a specific container of a pod.
type ExecClient interface {
ExecCMDInContainer(common.ComponentInfo, []string, io.Writer, io.Writer, io.Reader, bool) error
}

// ExecuteCommand executes the given command in the pod's container
func ExecuteCommand(client ExecClient, compInfo common.ComponentInfo, command []string, show bool) (err error) {

// Create a mutex so we don't run into the issue of go routines trying to write to stdout
var mu sync.Mutex

reader, writer := io.Pipe()
var cmdOutput string

klog.V(3).Infof("Executing command %v for pod: %v in container: %v", command, compInfo.PodName, compInfo.ContainerName)
klog.V(4).Infof("Executing command %v for pod: %v in container: %v", command, compInfo.PodName, compInfo.ContainerName)

// This Go routine will automatically pipe the output from ExecCMDInContainer to
// our logger.
Expand All @@ -37,13 +43,17 @@ func ExecuteCommand(client ExecClient, compInfo common.ComponentInfo, command []
}
}

mu.Lock()
cmdOutput += fmt.Sprintln(line)
mu.Unlock()
}
}()

err = client.ExecCMDInContainer(compInfo, command, writer, writer, nil, false)
if err != nil {
mu.Lock()
log.Errorf("\nUnable to exec command %v: \n%v", command, cmdOutput)
mu.Unlock()
return err
}

Expand Down

0 comments on commit 1ee1fec

Please sign in to comment.