Merge pull request #16 from xibz/master

cmd/firectl: adding more verbose error messages and testdata folder

Author: xibz

.gitignore

@@ -1,4 +1,3 @@
-*.img
 firecracker
 firecracker-*
-vmlinux
+vmlinux

cmd/firectl/main.go

@@ -26,32 +26,6 @@ import (
 	log "github.com/sirupsen/logrus"
 )
 
-func checkConfig(cfg firecracker.Config) error {
-	var err error
-
-	// Check for the existence of some required files:
-	_, err = os.Stat(cfg.BinPath)
-	if err != nil {
-		return err
-	}
-	_, err = os.Stat(cfg.KernelImagePath)
-	if err != nil {
-		return err
-	}
-	_, err = os.Stat(cfg.RootDrive.HostPath)
-	if err != nil {
-		return err
-	}
-
-	// Check the non-existence of some files:
-	_, err = os.Stat(cfg.SocketPath)
-	if err == nil {
-		msg := fmt.Sprintf("Socket %s already exists.", cfg.SocketPath)
-		return errors.New(msg)
-	}
-	return nil
-}
-
 func parseBlockDevices(entries []string) ([]firecracker.BlockDevice, error) {
 	var devices []firecracker.BlockDevice
 	for _, entry := range entries {
@@ -207,13 +181,11 @@ func main() {
 		Debug:             opts.Debug,
 	}
 
-	err = checkConfig(fcCfg)
+	m, err := firecracker.NewMachine(fcCfg, firecracker.WithLogger(log.NewEntry(logger)))
 	if err != nil {
-		log.Fatalf("Configuration error: %s", err)
+		log.Fatalf("Failed creating machine: %s", err)
 	}
 
-	m := firecracker.NewMachine(fcCfg, firecracker.WithLogger(log.NewEntry(logger)))
-
 	ctx := context.Background()
 	vmmCtx, vmmCancel := context.WithCancel(ctx)
 	defer vmmCancel()

machine.go

@@ -82,6 +82,31 @@ type Config struct {
 	Console           string
 	Debug             bool
 	machineCfg        models.MachineConfiguration
+
+	// Allows for easier mock testing
+	disableValidation bool
+}
+
+// Validate will ensure that the required fields are set and that
+// the fields are valid values.
+func (cfg *Config) Validate() error {
+	if cfg.disableValidation {
+		return nil
+	}
+
+	if _, err := os.Stat(cfg.KernelImagePath); err != nil {
+		return fmt.Errorf("Failed to stat kernal image path, %q: %v", cfg.KernelImagePath, err)
+	}
+	if _, err := os.Stat(cfg.RootDrive.HostPath); err != nil {
+		return fmt.Errorf("Failed to stat host path, %q: %v", cfg.RootDrive.HostPath, err)
+	}
+
+	// Check the non-existence of some files:
+	if _, err := os.Stat(cfg.SocketPath); err == nil {
+		return fmt.Errorf("Socket %s already exists", cfg.SocketPath)
+	}
+
+	return nil
 }
 
 // Machine is the main object for manipulating firecracker VMs
@@ -136,7 +161,11 @@ func (m Machine) LogLevel() string {
 }
 
 // NewMachine initializes a new Machine instance
-func NewMachine(cfg Config, opts ...Opt) *Machine {
+func NewMachine(cfg Config, opts ...Opt) (*Machine, error) {
+	if err := cfg.Validate(); err != nil {
+		return nil, err
+	}
+
 	m := &Machine{}
 
 	for _, opt := range opts {
@@ -173,7 +202,7 @@ func NewMachine(cfg Config, opts ...Opt) *Machine {
 		m.binPath = fcExecutable
 	}
 
-	return m
+	return m, nil
 }
 
 // Init starts the VMM and attaches drives and network interfaces.

machine_test.go

@@ -28,22 +28,37 @@ import (
 )
 
 const (
-	firecrackerBinaryPath        = "./firecracker"
+	firecrackerBinaryPath        = "firecracker"
 	firecrackerBinaryOverrideEnv = "FC_TEST_BIN"
 
 	defaultTuntapName = "fc-test-tap0"
 	tuntapOverrideEnv = "FC_TEST_TAP"
+
+	testDataPathEnv = "FC_TEST_DATA_PATH"
 )
 
+var testDataPath = "./testdata"
+
 var skipTuntap bool
 
 func init() {
 	flag.BoolVar(&skipTuntap, "test.skip-tuntap", false, "Disables tests that require a tuntap device")
+
+	if val := os.Getenv(testDataPathEnv); val != "" {
+		testDataPath = val
+	}
 }
 
 // Ensure that we can create a new machine
 func TestNewMachine(t *testing.T) {
-	m := NewMachine(Config{Debug: true})
+	m, err := NewMachine(Config{
+		Debug:             true,
+		disableValidation: true,
+	})
+	if err != nil {
+		t.Fatalf("unexpected error: %v", err)
+	}
+
 	if m == nil {
 		t.Errorf("NewMachine did not create a Machine")
 	}
@@ -67,8 +82,13 @@ func TestMicroVMExecution(t *testing.T) {
 		CPUTemplate: cpuTemplate,
 		MemInMiB:    memSz,
 		Debug:       true,
+		disableValidation: true,
 	}
-	m := NewMachine(cfg)
+	m, err := NewMachine(cfg)
+	if err != nil {
+		t.Fatalf("unexpectd error: %v", err)
+	}
+
 	ctx := context.Background()
 	vmmCtx, vmmCancel := context.WithTimeout(ctx, 30*time.Second)
 	defer vmmCancel()
@@ -101,8 +121,13 @@ func TestStartVMM(t *testing.T) {
 	cfg := Config{
 		SocketPath: filepath.Join("fc-start-vmm-test.sock"),
 		BinPath:    getFirecrackerBinaryPath(),
+		disableValidation: true,
 	}
-	m := NewMachine(cfg)
+	m, err := NewMachine(cfg)
+	if err != nil {
+		t.Fatalf("unexpected error: %v", err)
+	}
+
 	timeout, cancel := context.WithTimeout(context.Background(), 250*time.Millisecond)
 	defer cancel()
 	errchan, err := m.startVMM(timeout)
@@ -124,7 +149,7 @@ func getFirecrackerBinaryPath() string {
 	if val := os.Getenv(firecrackerBinaryOverrideEnv); val != "" {
 		return val
 	}
-	return firecrackerBinaryPath
+	return filepath.Join(testDataPath, firecrackerBinaryPath)
 }
 
 func testCreateMachine(ctx context.Context, t *testing.T, m *Machine) {
@@ -148,7 +173,7 @@ func testMachineConfigApplication(ctx context.Context, t *testing.T, m *Machine,
 }
 
 func testCreateBootSource(ctx context.Context, t *testing.T, m *Machine) {
-	err := m.createBootSource(ctx, "./vmlinux", "ro console=ttyS0 noapic reboot=k panic=1 pci=off nomodules")
+	err := m.createBootSource(ctx, filepath.Join(testDataPath, "./vmlinux"), "ro console=ttyS0 noapic reboot=k panic=1 pci=off nomodules")
 	if err != nil {
 		t.Errorf("failed to create boot source: %s", err)
 	}
@@ -179,15 +204,15 @@ func getTapName() string {
 }
 
 func testAttachRootDrive(ctx context.Context, t *testing.T, m *Machine) {
-	drive := BlockDevice{HostPath: "root-drive.img", Mode: "ro"}
+	drive := BlockDevice{HostPath: filepath.Join(testDataPath, "root-drive.img"), Mode: "ro"}
 	err := m.attachRootDrive(ctx, drive)
 	if err != nil {
 		t.Errorf("attaching root drive failed: %s", err)
 	}
 }
 
 func testAttachSecondaryDrive(ctx context.Context, t *testing.T, m *Machine) {
-	drive := BlockDevice{HostPath: "drive-2.img", Mode: "ro"}
+	drive := BlockDevice{HostPath: filepath.Join(testDataPath, "drive-2.img"), Mode: "ro"}
 	err := m.attachDrive(ctx, drive, 2, false)
 	if err != nil {
 		t.Errorf("attaching secondary drive failed: %s", err)

testdata/drive-2.img

@@ -0,0 +1 @@
+