Merge pull request #2951 from dgageot/2949-remove-stopped-vbox-vm

FIX #2949 Cannot remove stopped vbox VM

drivers/virtualbox/virtualbox.go

@@ -615,19 +615,20 @@ func (d *Driver) Kill() error {
 }
 
 func (d *Driver) Remove() error {
-	_, err := d.GetState()
-	if err != nil {
+	s, err := d.GetState()
 	if err == ErrMachineNotExist {
-			log.Infof("machine does not exist, assuming it has been removed already")
 		return nil
 	}
+	if err != nil {
 		return err
 	}
+
+	if s != state.Stopped && s != state.Saved {
 		if err := d.Kill(); err != nil {
 			return err
 		}
-	// vbox will not release it's lock immediately after the stop
-	d.sleeper.Sleep(1 * time.Second)
+	}
+
 	return d.vbm("unregistervm", "--delete", d.MachineName)
 }
 

drivers/virtualbox/virtualbox_test.go

@@ -300,12 +300,28 @@ func (v *MockCreateOperations) doCall(callSignature string) (string, error) {
 	return call.output, call.err
 }
 
+func mockCalls(t *testing.T, driver *Driver, expectedCalls []Call) {
+	mockOperations := &MockCreateOperations{
+		test:          t,
+		expectedCalls: expectedCalls,
+	}
+
+	driver.Boot2DockerURL = "http://b2d.org"
+	driver.VBoxManager = mockOperations
+	driver.b2dUpdater = mockOperations
+	driver.sshKeyGenerator = mockOperations
+	driver.diskCreator = mockOperations
+	driver.logsReader = mockOperations
+	driver.ipWaiter = mockOperations
+	driver.randomInter = mockOperations
+	driver.sleeper = mockOperations
+}
+
 func TestCreateVM(t *testing.T) {
 	shareName, shareDir := getShareDriveAndName()
 
-	operations := &MockCreateOperations{
-		test: t,
-		expectedCalls: []Call{
+	driver := NewDriver("default", "path")
+	mockCalls(t, driver, []Call{
 		{"CopyIsoToMachineDir path default http://b2d.org", "", nil},
 		{"Generate path/machines/default/id_rsa", "", nil},
 		{"Create 20000 path/machines/default/id_rsa.pub path/machines/default/disk.vmdk", "", nil},
@@ -319,19 +335,7 @@ func TestCreateVM(t *testing.T) {
 		{"vbm guestproperty set default /VirtualBox/GuestAdd/SharedFolders/MountDir /", "", nil},
 		{"vbm sharedfolder add default --name " + shareName + " --hostpath " + shareDir + " --automount", "", nil},
 		{"vbm setextradata default VBoxInternal2/SharedFoldersEnableSymlinksCreate/" + shareName + " 1", "", nil},
-		},
-	}
-
-	driver := NewDriver("default", "path")
-	driver.Boot2DockerURL = "http://b2d.org"
-	driver.VBoxManager = operations
-	driver.b2dUpdater = operations
-	driver.sshKeyGenerator = operations
-	driver.diskCreator = operations
-	driver.logsReader = operations
-	driver.ipWaiter = operations
-	driver.randomInter = operations
-	driver.sleeper = operations
+	})
 
 	err := driver.CreateVM()
 
@@ -339,9 +343,8 @@ func TestCreateVM(t *testing.T) {
 }
 
 func TestStart(t *testing.T) {
-	operations := &MockCreateOperations{
-		test: t,
-		expectedCalls: []Call{
+	driver := NewDriver("default", "path")
+	mockCalls(t, driver, []Call{
 		{"vbm showvminfo default --machinereadable", `VMState="poweroff"`, nil},
 		{"vbm list hostonlyifs", "", nil},
 		{"vbm hostonlyif create", "Interface 'VirtualBox Host-Only Ethernet Adapter' was successfully created", nil},
@@ -379,19 +382,7 @@ HardwareAddress: 0a:00:27:00:00:00
 MediumType:      Ethernet
 Status:          Up
 VBoxNetworkName: HostInterfaceNetworking-VirtualBox Host-Only Ethernet Adapter`, nil},
-		},
-	}
-
-	driver := NewDriver("default", "path")
-	driver.Boot2DockerURL = "http://b2d.org"
-	driver.VBoxManager = operations
-	driver.b2dUpdater = operations
-	driver.sshKeyGenerator = operations
-	driver.diskCreator = operations
-	driver.logsReader = operations
-	driver.ipWaiter = operations
-	driver.randomInter = operations
-	driver.sleeper = operations
+	})
 
 	err := driver.Start()
 
@@ -399,9 +390,8 @@ VBoxNetworkName: HostInterfaceNetworking-VirtualBox Host-Only Ethernet Adapter`,
 }
 
 func TestStartWithHostOnlyAdapterCreationBug(t *testing.T) {
-	operations := &MockCreateOperations{
-		test: t,
-		expectedCalls: []Call{
+	driver := NewDriver("default", "path")
+	mockCalls(t, driver, []Call{
 		{"vbm showvminfo default --machinereadable", `VMState="poweroff"`, nil},
 		{"vbm list hostonlyifs", "", nil},
 		{"vbm hostonlyif create", "", errors.New("error: Failed to create the host-only adapter")},
@@ -448,21 +438,59 @@ VBoxNetworkName: HostInterfaceNetworking-VirtualBox Host-Only Ethernet Adapter`,
 		{"Sleep 5s", "", nil},
 		{"vbm startvm default --type headless", "", nil},
 		{"WaitIP", "", nil},
-		},
-	}
-
-	driver := NewDriver("default", "path")
-	driver.Boot2DockerURL = "http://b2d.org"
-	driver.VBoxManager = operations
-	driver.b2dUpdater = operations
-	driver.sshKeyGenerator = operations
-	driver.diskCreator = operations
-	driver.logsReader = operations
-	driver.ipWaiter = operations
-	driver.randomInter = operations
-	driver.sleeper = operations
+	})
 
 	err := driver.Start()
 
 	assert.NoError(t, err)
 }
+
+func TestRemoveStopped(t *testing.T) {
+	driver := NewDriver("default", "path")
+	mockCalls(t, driver, []Call{
+		{"vbm showvminfo default --machinereadable", `VMState="poweroff"`, nil},
+		{"vbm unregistervm --delete default", "", nil},
+	})
+
+	err := driver.Remove()
+
+	assert.NoError(t, err)
+}
+
+func TestRemoveStarted(t *testing.T) {
+	driver := NewDriver("default", "path")
+	mockCalls(t, driver, []Call{
+		{"vbm showvminfo default --machinereadable", `VMState="running"`, nil},
+		{"vbm controlvm default poweroff", "", nil},
+		{"vbm unregistervm --delete default", "", nil},
+	})
+
+	err := driver.Remove()
+
+	assert.NoError(t, err)
+}
+
+func TestRemoveSaved(t *testing.T) {
+	driver := NewDriver("default", "path")
+	mockCalls(t, driver, []Call{
+		{"vbm showvminfo default --machinereadable", `VMState="saved"`, nil},
+		{"vbm unregistervm --delete default", "", nil},
+	})
+
+	err := driver.Remove()
+
+	assert.NoError(t, err)
+}
+
+func TestRemovePaused(t *testing.T) {
+	driver := NewDriver("default", "path")
+	mockCalls(t, driver, []Call{
+		{"vbm showvminfo default --machinereadable", `VMState="running"`, nil},
+		{"vbm controlvm default poweroff", "", nil},
+		{"vbm unregistervm --delete default", "", nil},
+	})
+
+	err := driver.Remove()
+
+	assert.NoError(t, err)
+}