Issue #682: Remove macos virtualbox preflight checks

They are mostly untested, and VirtualBox is not a fully supported option, so using it may need manual configuration. This fixes #662
crc-org · Nov 25, 2019 · a516a98 · a516a98
1 parent 872b647
commit a516a98
Show file tree

Hide file tree

Showing 4 changed files with 22 additions and 87 deletions.
diff --git a/cmd/crc/cmd/config/config_darwin.go b/cmd/crc/cmd/config/config_darwin.go
@@ -8,8 +8,6 @@ var (
 	// Preflight checks
 	SkipCheckRootUser                = cfg.AddSetting("skip-check-root-user", nil, []cfg.ValidationFnType{cfg.ValidateBool}, []cfg.SetFn{cfg.SuccessfullyApplied})
 	WarnCheckRootUser                = cfg.AddSetting("warn-check-root-user", nil, []cfg.ValidationFnType{cfg.ValidateBool}, []cfg.SetFn{cfg.SuccessfullyApplied})
-	SkipCheckVirtualBoxInstalled     = cfg.AddSetting("skip-check-virtualbox-installed", nil, []cfg.ValidationFnType{cfg.ValidateBool}, []cfg.SetFn{cfg.SuccessfullyApplied})
-	WarnCheckVirtualBoxInstalled     = cfg.AddSetting("warn-check-virtualbox-installed", nil, []cfg.ValidationFnType{cfg.ValidateBool}, []cfg.SetFn{cfg.SuccessfullyApplied})
 	SkipCheckResolverFilePermissions = cfg.AddSetting("skip-check-resolver-file-permissions", nil, []cfg.ValidationFnType{cfg.ValidateBool}, []cfg.SetFn{cfg.SuccessfullyApplied})
 	WarnCheckResolverFilePermissions = cfg.AddSetting("warn-check-resolver-file-permissions", nil, []cfg.ValidationFnType{cfg.ValidateBool}, []cfg.SetFn{cfg.SuccessfullyApplied})
 	SkipCheckHostsFilePermissions    = cfg.AddSetting("skip-check-hosts-file-permissions", nil, []cfg.ValidationFnType{cfg.ValidateBool}, []cfg.SetFn{cfg.SuccessfullyApplied})

diff --git a/pkg/crc/preflight/preflight_checks_darwin.go b/pkg/crc/preflight/preflight_checks_darwin.go
@@ -1,12 +1,10 @@
 package preflight
 
 import (
-	"errors"
 	"fmt"
 	"golang.org/x/sys/unix"
 	neturl "net/url"
 	"os"
-	"os/exec"
 	"os/user"
 	"path"
 	"path/filepath"
@@ -20,9 +18,6 @@ import (
 )
 
 const (
-	virtualBoxDownloadURL   = "https://download.virtualbox.org/virtualbox/6.0.4/VirtualBox-6.0.4-128413-OSX.dmg"
-	virtualBoxMountLocation = "/Volumes/VirtualBox"
-
 	hyperkitDriverCommand = "crc-driver-hyperkit"
 	hyperkitDriverVersion = "0.12.6"
 
@@ -32,48 +27,11 @@ const (
 )
 
 var (
-	virtualBoxPkgLocation     = fmt.Sprintf("%s/VirtualBox.pkg", virtualBoxMountLocation)
 	hyperkitDownloadURL       = fmt.Sprintf("https://github.com/code-ready/machine-driver-hyperkit/releases/download/v%s/hyperkit", hyperkitDriverVersion)
 	hyperkitDriverDownloadURL = fmt.Sprintf("https://github.com/code-ready/machine-driver-hyperkit/releases/download/v%s/crc-driver-hyperkit", hyperkitDriverVersion)
 )
 
 // Add darwin specific checks
-func checkVirtualBoxInstalled() (bool, error) {
-	logging.Debug("Checking if VirtualBox is installed")
-	_, err := exec.LookPath("VBoxManage")
-	if err != nil {
-		return false, errors.New("VirtualBox cli VBoxManage is not found in the path")
-	}
-	logging.Debug("VirtualBox was found")
-	return true, nil
-}
-
-func fixVirtualBoxInstallation() (bool, error) {
-	logging.Debug("Downloading VirtualBox")
-	// Download the virtualbox installer in ~/.crc/cache
-	tempFilePath := filepath.Join(constants.MachineCacheDir, "virtualbox.dmg")
-	_, err := dl.Download(virtualBoxDownloadURL, tempFilePath, 0600)
-	if err != nil {
-		return false, err
-	}
-	defer os.Remove(tempFilePath)
-	logging.Debug("Installing VirtualBox")
-	stdOut, stdErr, err := crcos.RunWithPrivilege("mount VirtualBox disk image", "hdiutil", "attach", tempFilePath)
-	if err != nil {
-		return false, fmt.Errorf("Could not mount the virtualbox.dmg file: %s %v: %s", stdOut, err, stdErr)
-	}
-	stdOut, stdErr, err = crcos.RunWithPrivilege("run VirtualBox installation", "installer", "-package", virtualBoxPkgLocation, "-target", "/")
-	if err != nil {
-		return false, fmt.Errorf("Could not install VirtualBox.pkg: %s %v: %s", stdOut, err, stdErr)
-	}
-	stdOut, stdErr, err = crcos.RunWithPrivilege("unmount VirtualBox disk image", "hdiutil", "detach", virtualBoxMountLocation)
-	if err != nil {
-		return false, fmt.Errorf("Could not install VirtualBox.pkg: %s %v: %s", stdOut, err, stdErr)
-	}
-	logging.Debug("VirtualBox installed")
-	return true, nil
-}
-
 func tryRemoveDestFile(url string, destDir string) error {
 	u, err := neturl.Parse(url)
 	if err != nil {

diff --git a/pkg/crc/preflight/preflight_darwin.go b/pkg/crc/preflight/preflight_darwin.go
@@ -19,25 +19,16 @@ func StartPreflightChecks(vmDriver string) {
 		false,
 	)
 
-	switch vmDriver {
-	case "virtualbox":
-		preflightCheckSucceedsOrFails(config.GetBool(cmdConfig.SkipCheckVirtualBoxInstalled.Name),
-			checkVirtualBoxInstalled,
-			"Checking if VirtualBox is Installed",
-			config.GetBool(cmdConfig.WarnCheckVirtualBoxInstalled.Name),
-		)
-	case "hyperkit":
-		preflightCheckSucceedsOrFails(config.GetBool(cmdConfig.SkipCheckHyperKitInstalled.Name),
-			checkHyperKitInstalled,
-			"Checking if HyperKit is installed",
-			config.GetBool(cmdConfig.WarnCheckHyperKitInstalled.Name),
-		)
-		preflightCheckSucceedsOrFails(config.GetBool(cmdConfig.SkipCheckHyperKitDriver.Name),
-			checkMachineDriverHyperKitInstalled,
-			"Checking if crc-driver-hyperkit is installed",
-			config.GetBool(cmdConfig.WarnCheckHyperKitDriver.Name),
-		)
-	}
+	preflightCheckSucceedsOrFails(config.GetBool(cmdConfig.SkipCheckHyperKitInstalled.Name),
+		checkHyperKitInstalled,
+		"Checking if HyperKit is installed",
+		config.GetBool(cmdConfig.WarnCheckHyperKitInstalled.Name),
+	)
+	preflightCheckSucceedsOrFails(config.GetBool(cmdConfig.SkipCheckHyperKitDriver.Name),
+		checkMachineDriverHyperKitInstalled,
+		"Checking if crc-driver-hyperkit is installed",
+		config.GetBool(cmdConfig.WarnCheckHyperKitDriver.Name),
+	)
 
 	preflightCheckSucceedsOrFails(config.GetBool(cmdConfig.SkipCheckHostsFilePermissions.Name),
 		checkHostsFilePermissions,
@@ -67,28 +58,18 @@ func SetupHost(vmDriver string) {
 		false,
 	)
 
-	switch vmDriver {
-	case "virtualbox":
-		preflightCheckAndFix(config.GetBool(cmdConfig.SkipCheckVirtualBoxInstalled.Name),
-			checkVirtualBoxInstalled,
-			fixVirtualBoxInstallation,
-			"Setting up virtualization with VirtualBox",
-			config.GetBool(cmdConfig.WarnCheckVirtualBoxInstalled.Name),
-		)
-	case "hyperkit":
-		preflightCheckAndFix(config.GetBool(cmdConfig.SkipCheckHyperKitInstalled.Name),
-			checkHyperKitInstalled,
-			fixHyperKitInstallation,
-			"Setting up virtualization with HyperKit",
-			config.GetBool(cmdConfig.WarnCheckHyperKitInstalled.Name),
-		)
-		preflightCheckAndFix(config.GetBool(cmdConfig.SkipCheckHyperKitDriver.Name),
-			checkMachineDriverHyperKitInstalled,
-			fixMachineDriverHyperKitInstalled,
-			"Installing crc-machine-hyperkit",
-			config.GetBool(cmdConfig.WarnCheckHyperKitDriver.Name),
-		)
-	}
+	preflightCheckAndFix(config.GetBool(cmdConfig.SkipCheckHyperKitInstalled.Name),
+		checkHyperKitInstalled,
+		fixHyperKitInstallation,
+		"Setting up virtualization with HyperKit",
+		config.GetBool(cmdConfig.WarnCheckHyperKitInstalled.Name),
+	)
+	preflightCheckAndFix(config.GetBool(cmdConfig.SkipCheckHyperKitDriver.Name),
+		checkMachineDriverHyperKitInstalled,
+		fixMachineDriverHyperKitInstalled,
+		"Installing crc-machine-hyperkit",
+		config.GetBool(cmdConfig.WarnCheckHyperKitDriver.Name),
+	)
 
 	preflightCheckAndFix(config.GetBool(cmdConfig.SkipCheckResolverFilePermissions.Name),
 		checkResolverFilePermissions,

diff --git a/test/integration/features/config.feature b/test/integration/features/config.feature
@@ -75,7 +75,6 @@ Checks whether CRC `config set` command works as expected in conjunction with `c
             | warn-check-hyperkit-installed        | true   | false  |
             | warn-check-resolver-file-permissions | true   | false  |
             | warn-check-root-user                 | true   | false  |
-            | warn-check-virtualbox-installed      | true   | false  |
 
         @linux
         Examples: Config warnings on Linux
@@ -130,7 +129,6 @@ Checks whether CRC `config set` command works as expected in conjunction with `c
             | skip-check-hyperkit-installed        | true   | false  |
             | skip-check-resolver-file-permissions | true   | false  |
             | skip-check-root-user                 | true   | false  |
-            | skip-check-virtualbox-installed      | true   | false  |
 
         @linux
         Examples: