Merge pull request #775 from MikaelSmith/fix-panic

Fix panic when waiting for pod times out

plugin/kubernetes/pvc.go

@@ -136,13 +136,15 @@ func (v *pvc) inContainer(ctx context.Context, fn containerCb) (interface{}, err
 		if err != nil {
 			return nil, err
 		}
+		cleanup = func() {
+			// Use a background context to ensure deletion happens even if context was cancelled.
+			activity.Record(ctx, "Deleted temporary pod %v: %v", tempPod, tempPod.delete(context.Background()))
+		}
 		if err := tempPod.waitOnCreation(ctx); err != nil {
+			cleanup()
 			return nil, err
 		}
 		execContainer.pod = tempPod.pod
-		cleanup = func() {
-			activity.Record(ctx, "Deleted temporary pod %v: %v", tempPod, tempPod.delete(ctx))
-		}
 	} else {
 		mount := v.getMountInfo(mountingPod, volumeName)
 		execContainer.pod = mount.pod
@@ -168,15 +170,18 @@ func (v *pvc) exec(ctx context.Context, buildCmd cmdBuilder, stdin io.Reader) ([
 		streamOpts := remotecommand.StreamOptions{Stdout: &stdout, Stderr: &stderr, Stdin: stdin}
 		executor, err := c.newExecutor(ctx, cmd[0], cmd[1:], streamOpts)
 		if err != nil {
-			return []byte{}, err
+			return nil, err
 		}
 
 		err = executor.Stream()
 		activity.Record(ctx, "stdout: %v", stdout.String())
 		activity.Record(ctx, "stderr: %v", stderr.String())
 		return stdout.Bytes(), err
 	})
-	return obj.([]byte), err
+	if err != nil {
+		return nil, err
+	}
+	return obj.([]byte), nil
 }
 
 func (v *pvc) VolumeList(ctx context.Context, path string) (volume.DirMap, error) {
@@ -226,7 +231,10 @@ func (v *pvc) VolumeStream(ctx context.Context, path string) (io.ReadCloser, err
 			cleanup()
 		}}, nil
 	})
-	return obj.(io.ReadCloser), err
+	if err != nil {
+		return nil, err
+	}
+	return obj.(io.ReadCloser), nil
 }
 
 func (v *pvc) VolumeWrite(ctx context.Context, path string, b []byte, _ os.FileMode) error {

transport/ssh_test.go

@@ -10,6 +10,7 @@ import (
 	"io/ioutil"
 	"os"
 	"strconv"
+	"sync"
 	"testing"
 
 	gssh "github.com/gliderlabs/ssh"
@@ -22,7 +23,9 @@ import (
 
 type SSHTestSuite struct {
 	suite.Suite
-	s             gssh.Server
+	s gssh.Server
+	// Use a mux to avoid identifying a "data race" betweene Handler and SetupTest
+	mux           sync.Mutex
 	m             mock.Mock
 	knownHosts    string
 	badKnownHosts string
@@ -41,16 +44,22 @@ const (
 
 // Must be mocked for successful connections.
 func (suite *SSHTestSuite) Handler(s gssh.Session) {
+	suite.mux.Lock()
 	suite.m.Called(s)
+	suite.mux.Unlock()
 }
 
 // Must be mocked if Identity.Password is set.
 func (suite *SSHTestSuite) PasswordHandler(ctx gssh.Context, password string) bool {
+	suite.mux.Lock()
+	defer suite.mux.Unlock()
 	return suite.m.Called(ctx, password).Bool(0)
 }
 
 // Must be mocked.
 func (suite *SSHTestSuite) PublicKeyHandler(ctx gssh.Context, key gssh.PublicKey) bool {
+	suite.mux.Lock()
+	defer suite.mux.Unlock()
 	return suite.m.Called(ctx, key).Bool(0)
 }
 
@@ -125,7 +134,9 @@ func (suite *SSHTestSuite) SetupSuite() {
 
 func (suite *SSHTestSuite) SetupTest() {
 	// Reset mocks before every test
+	suite.mux.Lock()
 	suite.m = mock.Mock{}
+	suite.mux.Unlock()
 }
 
 func (suite *SSHTestSuite) TearDownTest() {