machine: enforce Start can only be called once

Signed-off-by: Samuel Karp <[email protected]>

Author: samuelkarp

machine.go

@@ -22,6 +22,7 @@ import (
 	"os/exec"
 	"os/signal"
 	"strconv"
+	"sync"
 	"syscall"
 	"time"
 
@@ -34,6 +35,8 @@ const (
 	userAgent = "firecracker-go-sdk"
 )
 
+var ErrAlreadyStarted = errors.New("firecracker: machine already started")
+
 // Firecracker is an interface that can be used to mock
 // out an Firecracker agent for testing purposes.
 type Firecracker interface {
@@ -146,6 +149,7 @@ type Machine struct {
 	logger        *log.Entry
 	machineConfig models.MachineConfiguration // The actual machine config as reported by Firecracker
 	errCh         chan error
+	startOnce     sync.Once
 }
 
 // Logger returns a logrus logger appropriate for logging hypervisor messages
@@ -227,13 +231,24 @@ func NewMachine(ctx context.Context, cfg Config, opts ...Opt) (*Machine, error)
 // Start will iterate through the handler list and call each handler. If an
 // error occurred during handler execution, that error will be returned. If the
 // handlers succeed, then this will start the VMM instance.
+// Start may only be called once per Machine.  Subsequent calls will return
+// ErrAlreadyStarted.
 func (m *Machine) Start(ctx context.Context) error {
 	m.logger.Debug("Called Machine.Start()")
+	alreadyStarted := true
+	m.startOnce.Do(func() {
+		m.logger.Debug("Marking Machine as Started")
+		alreadyStarted = false
+	})
+	if alreadyStarted {
+		return ErrAlreadyStarted
+	}
+
 	if err := m.Handlers.Run(ctx, m); err != nil {
 		return err
 	}
 
-	return m.StartInstance(ctx)
+	return m.startInstance(ctx)
 }
 
 // Wait will wait until the firecracker process has finished

machine_test.go

@@ -34,6 +34,7 @@ import (
 	"github.com/firecracker-microvm/firecracker-go-sdk/fctesting"
 	"github.com/golang/mock/gomock"
 	log "github.com/sirupsen/logrus"
+	"github.com/stretchr/testify/assert"
 )
 
 const (
@@ -92,13 +93,7 @@ func TestMicroVMExecution(t *testing.T) {
 		os.Remove(metricsFifo)
 	}()
 
-	vmlinuxPath := filepath.Join(testDataPath, "./vmlinux")
-	if _, err := os.Stat(vmlinuxPath); err != nil {
-		t.Fatalf("Cannot find vmlinux file: %s\n"+
-			`Verify that you have a vmlinux file at "%s" or set the `+
-			"`%s` environment variable to the correct location.",
-			err, vmlinuxPath, testDataPathEnv)
-	}
+	vmlinuxPath := getVmlinuxPath(t)
 
 	cfg := Config{
 		SocketPath:  socketPath,
@@ -183,19 +178,64 @@ func TestStartVMM(t *testing.T) {
 	defer cancel()
 	err = m.startVMM(timeout)
 	if err != nil {
-		t.Errorf("startVMM failed: %s", err)
-	} else {
-		defer m.StopVMM()
+		t.Fatalf("startVMM failed: %s", err)
+	}
+	defer m.StopVMM()
 
-		select {
-		case <-timeout.Done():
-			if timeout.Err() == context.DeadlineExceeded {
-				t.Log("firecracker ran for 250ms")
-			} else {
-				t.Errorf("startVMM returned %s", m.Wait(ctx))
-			}
+	select {
+	case <-timeout.Done():
+		if timeout.Err() == context.DeadlineExceeded {
+			t.Log("firecracker ran for 250ms")
+		} else {
+			t.Errorf("startVMM returned %s", m.Wait(ctx))
 		}
 	}
+
+}
+
+func TestStartVMMOnce(t *testing.T) {
+	socketPath := filepath.Join("testdata", "fc-start-vmm-test.sock")
+	defer os.Remove(socketPath)
+
+	cfg := Config{
+		SocketPath:        socketPath,
+		DisableValidation: true,
+		KernelImagePath:   getVmlinuxPath(t),
+		MachineCfg: models.MachineConfiguration{
+			VcpuCount: 1,
+		},
+	}
+	ctx := context.Background()
+	cmd := VMCommandBuilder{}.
+		WithSocketPath(cfg.SocketPath).
+		WithBin(getFirecrackerBinaryPath()).
+		Build(ctx)
+	m, err := NewMachine(ctx, cfg, WithProcessRunner(cmd))
+	if err != nil {
+		t.Fatalf("unexpected error: %v", err)
+	}
+	defer m.StopVMM()
+
+	timeout, cancel := context.WithTimeout(ctx, 250*time.Millisecond)
+	defer cancel()
+	err = m.Start(timeout)
+	if err != nil {
+		t.Fatalf("startVMM failed: %s", err)
+	}
+	defer m.StopVMM()
+	err = m.Start(timeout)
+	assert.Error(t, err, "should return an error when Start is called multiple times")
+	assert.Equal(t, ErrAlreadyStarted, err, "should be ErrAlreadyStarted")
+
+	select {
+	case <-timeout.Done():
+		if timeout.Err() == context.DeadlineExceeded {
+			t.Log("firecracker ran for 250ms")
+		} else {
+			t.Errorf("startVMM returned %s", m.Wait(ctx))
+		}
+	}
+
 }
 
 func getFirecrackerBinaryPath() string {
@@ -205,6 +245,18 @@ func getFirecrackerBinaryPath() string {
 	return filepath.Join(testDataPath, firecrackerBinaryPath)
 }
 
+func getVmlinuxPath(t *testing.T) string {
+	t.Helper()
+	vmlinuxPath := filepath.Join(testDataPath, "./vmlinux")
+	if _, err := os.Stat(vmlinuxPath); err != nil {
+		t.Fatalf("Cannot find vmlinux file: %s\n"+
+			`Verify that you have a vmlinux file at "%s" or set the `+
+			"`%s` environment variable to the correct location.",
+			err, vmlinuxPath, testDataPathEnv)
+	}
+	return vmlinuxPath
+}
+
 func testCreateMachine(ctx context.Context, t *testing.T, m *Machine) {
 	err := m.createMachine(ctx)
 	if err != nil {