Repository: code-ready/crc
Pull request #1732 (2020-11-30 17:09:48)

Add fedora 33 support
**Fixes:** Issue #1572

**Relates to:** Issue #1624

## Solution/Idea

This adds support for NetworkManager + systemd-resolved as used by fedora 33 (and ubuntu desktop).
This makes use of the per-distro preflight checks which were introduced during last sprint so that older distro releases can keep using NM + dnsmasq while newer distros will use NM + systemd-resolved.
Some limitations of this series:
- ubuntu support is not enabled, it can be added after #1647 lands
- this does not check if systemd-resolved is listening on port 53, this can be configured through /etc/systemd/resolved.conf
- there is no support for systemd-networkd + systemd-resolved (no NetworkManager) setups
- the selection of systemd-resolved/dnsmasq is static depending on the detected distro

## Proposed changes

List main as well as consequential changes you introduced or had to introduce.

1. Modifies `crc setup` to run different preflight checks depending on the distro which is in use (fedora 33 VS others)

## Testing

1. `crc setup` DNS configuration should not regress on previously working distros (fedora <= 32, RHEL8)
2. `crc setup` DNS configuration should work on fedora 33, including wildcard DNS (`anything.apps-crc.testing`)

openshift-ci-robot: [APPROVALNOTIFIER] This PR is **APPROVED**

This pull-request has been approved by: *<a href="https://github.com/code-ready/crc/pull/1732#" title="Author self-approved">cfergeau</a>*

The full list of commands accepted by this bot can be found [here](https://go.k8s.io/bot-commands?repo=code-ready%2Fcrc).

The pull request process is described [here](https://git.k8s.io/community/contributors/guide/owners.md#the-code-review-process)

<details >
Needs approval from an approver in each of these files:

- ~~[OWNERS](https://github.com/code-ready/crc/blob/master/OWNERS)~~ [cfergeau]

Approvers can indicate their approval by writing `/approve` in a comment
Approvers can cancel approval by writing `/approve cancel` in a comment
</details>
<!-- META={"approvers":[]} -->
cfergeau: (since they conflict quite a bit, https://github.com/cfergeau/crc/tree/f33-ubuntu has a version of this PR rebased on top of https://github.com/code-ready/crc/pull/1647 )
openshift-merge-robot: @cfergeau: The following test **failed**, say `/retest` to rerun all failed tests:

Test name | Commit | Details | Rerun command
--- | --- | --- | ---
ci/prow/e2e-crc | 8420f1db5dc209cde5ce3f950a37e4ca31c5eb83 | [link](https://prow.ci.openshift.org/view/gs/origin-ci-test/pr-logs/pull/code-ready_crc/1732/pull-ci-code-ready-crc-master-e2e-crc/1333458222383632384) | `/test e2e-crc`

[Full PR test history](https://prow.ci.openshift.org/pr-history?org=code-ready&repo=crc&pr=1732). [Your PR dashboard](https://prow.ci.openshift.org/pr?query=is:pr+state:open+author:cfergeau).

<details>

Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md).  If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository. I understand the commands that are listed [here](https://go.k8s.io/bot-commands).
</details>
<!-- test report -->
praveenkumar: @cfergeau we also need to adjust the integration test to incorporate the changes in the output messages. 
cfergeau: > @cfergeau we also need to adjust the integration test to incorporate the changes in the output messages.

I did not have time to look at this, I'll do it tomorrow, hopefully the CI run will help.
openshift-merge-robot: @cfergeau: The following test **failed**, say `/retest` to rerun all failed tests:

Test name | Commit | Details | Rerun command
--- | --- | --- | ---
ci/prow/e2e-crc | 649830f2aa8db43dd1754578a3684936335b68fb | [link](https://prow.ci.openshift.org/view/gs/origin-ci-test/pr-logs/pull/code-ready_crc/1732/pull-ci-code-ready-crc-master-e2e-crc/1333827959089270784) | `/test e2e-crc`

[Full PR test history](https://prow.ci.openshift.org/pr-history?org=code-ready&repo=crc&pr=1732). [Your PR dashboard](https://prow.ci.openshift.org/pr?query=is:pr+state:open+author:cfergeau).

<details>

Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md).  If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository. I understand the commands that are listed [here](https://go.k8s.io/bot-commands).
</details>
<!-- test report -->
guillaumerose: I think you can remove all the lines checking logs in integration test (`stderr should contain ...`). They bring very little value:
* we check already the number of preflight tests, so we will hardly miss one. 
* `And stderr should contain` makes you think log lines are in a particular order but it is not.
* logs are not something we deliver or something that need to be stable for the user. 
openshift-ci-robot: [APPROVALNOTIFIER] This PR is **APPROVED**

This pull-request has been approved by: *<a href="https://github.com/code-ready/crc/pull/1732#" title="Author self-approved">cfergeau</a>*, *<a href="https://github.com/code-ready/crc/pull/1732#pullrequestreview-542263082" title="Approved">guillaumerose</a>*

The full list of commands accepted by this bot can be found [here](https://go.k8s.io/bot-commands?repo=code-ready%2Fcrc).

The pull request process is described [here](https://git.k8s.io/community/contributors/guide/owners.md#the-code-review-process)

<details >
Needs approval from an approver in each of these files:

- ~~[OWNERS](https://github.com/code-ready/crc/blob/master/OWNERS)~~ [cfergeau,guillaumerose]

Approvers can indicate their approval by writing `/approve` in a comment
Approvers can cancel approval by writing `/approve cancel` in a comment
</details>
<!-- META={"approvers":[]} -->
cfergeau: > I think you can remove all the lines checking logs in integration test (`stderr should contain ...`). They bring very little value:
> 
>     * we check already the number of preflight tests, so we will hardly miss one.
> 
>     * `And stderr should contain` makes you think log lines are in a particular order but it is not.
> 
>     * logs are not something we deliver or something that need to be stable for the user.

We could introduce a regression in one of the 'check' method, which would return the wrong result. The integration 
test could detect some of these regressions which would be harder to catch through unit tests.

Patch:
diff --git a/pkg/crc/preflight/preflight_checks_linux.go b/pkg/crc/preflight/preflight_checks_linux.go
index 48cb4a963..9f057ec90 100644
--- a/pkg/crc/preflight/preflight_checks_linux.go
+++ b/pkg/crc/preflight/preflight_checks_linux.go
@@ -24,23 +24,10 @@ import (
 )
 
 const (
-	crcDnsmasqConfigFile        = "crc.conf"
-	crcNetworkManagerConfigFile = "crc-nm-dnsmasq.conf"
 	// This is defined in https://github.com/code-ready/machine-driver-libvirt/blob/master/go.mod#L5
 	minSupportedLibvirtVersion = "3.4.0"
 )
 
-var (
-	crcDnsmasqConfigPath = filepath.Join(string(filepath.Separator), "etc", "NetworkManager", "dnsmasq.d", crcDnsmasqConfigFile)
-	crcDnsmasqConfig     = `server=/apps-crc.testing/192.168.130.11
-server=/crc.testing/192.168.130.11
-`
-	crcNetworkManagerConfigPath = filepath.Join(string(filepath.Separator), "etc", "NetworkManager", "conf.d", crcNetworkManagerConfigFile)
-	crcNetworkManagerConfig     = `[main]
-dns=dnsmasq
-`
-)
-
 func checkVirtualizationEnabled() error {
 	logging.Debug("Checking if the vmx/svm flags are present in /proc/cpuinfo")
 	// Check if the cpu flags vmx or svm is present
diff --git a/pkg/crc/preflight/preflight_checks_network_linux.go b/pkg/crc/preflight/preflight_checks_network_linux.go
new file mode 100644
index 000000000..aea17ee4c
--- /dev/null
+++ b/pkg/crc/preflight/preflight_checks_network_linux.go
@@ -0,0 +1,273 @@
+// +build linux
+
+package preflight
+
+import (
+	"fmt"
+	"os"
+	"os/exec"
+	"path/filepath"
+
+	"github.com/code-ready/crc/pkg/crc/logging"
+	"github.com/code-ready/crc/pkg/crc/systemd"
+	"github.com/code-ready/crc/pkg/crc/systemd/states"
+	crcos "github.com/code-ready/crc/pkg/os"
+)
+
+var nmPreflightChecks = [...]Check{
+	{
+		configKeySuffix:  "check-systemd-networkd-running",
+		checkDescription: "Checking if systemd-networkd is running",
+		check:            checkSystemdNetworkdIsNotRunning,
+		fixDescription:   "network configuration with systemd-networkd is not supported",
+		flags:            NoFix,
+	},
+	{
+		configKeySuffix:  "check-network-manager-installed",
+		checkDescription: "Checking if NetworkManager is installed",
+		check:            checkNetworkManagerInstalled,
+		fixDescription:   "NetworkManager is required and must be installed manually",
+		flags:            NoFix,
+	},
+	{
+		configKeySuffix:  "check-network-manager-running",
+		checkDescription: "Checking if NetworkManager service is running",
+		check:            checkNetworkManagerIsRunning,
+		fixDescription:   "NetworkManager is required. Please make sure it is installed and running manually",
+		flags:            NoFix,
+	},
+}
+
+var dnsmasqPreflightChecks = [...]Check{
+	{
+		configKeySuffix:    "check-network-manager-config",
+		checkDescription:   "Checking if /etc/NetworkManager/conf.d/crc-nm-dnsmasq.conf exists",
+		check:              checkCrcNetworkManagerConfig,
+		fixDescription:     "Writing Network Manager config for crc",
+		fix:                fixCrcNetworkManagerConfig,
+		cleanupDescription: "Removing /etc/NetworkManager/conf.d/crc-nm-dnsmasq.conf file",
+		cleanup:            removeCrcNetworkManagerConfig,
+	},
+	{
+		configKeySuffix:    "check-crc-dnsmasq-file",
+		checkDescription:   "Checking if /etc/NetworkManager/dnsmasq.d/crc.conf exists",
+		check:              checkCrcDnsmasqConfigFile,
+		fixDescription:     "Writing dnsmasq config for crc",
+		fix:                fixCrcDnsmasqConfigFile,
+		cleanupDescription: "Removing /etc/NetworkManager/dnsmasq.d/crc.conf file",
+		cleanup:            removeCrcDnsmasqConfigFile,
+	},
+}
+
+var (
+	crcNetworkManagerRootPath = filepath.Join(string(filepath.Separator), "etc", "NetworkManager")
+
+	crcDnsmasqConfigPath = filepath.Join(crcNetworkManagerRootPath, "dnsmasq.d", "crc.conf")
+	crcDnsmasqConfig     = `server=/apps-crc.testing/192.168.130.11
+server=/crc.testing/192.168.130.11
+`
+
+	crcNetworkManagerConfigPath = filepath.Join(crcNetworkManagerRootPath, "conf.d", "crc-nm-dnsmasq.conf")
+	crcNetworkManagerConfig     = `[main]
+dns=dnsmasq
+`
+
+	crcNetworkManagerDispatcherPath   = filepath.Join(crcNetworkManagerRootPath, "dispatcher.d", "pre-up.d", "99-crc.sh")
+	crcNetworkManagerDispatcherConfig = `#!/bin/sh
+# This is a NetworkManager dispatcher script to configure split DNS for
+# the 'crc' libvirt network.
+# The corresponding crc bridge is recreated each time the system reboots, so
+# it cannot be configured permanently through NetworkManager.
+# Changing DNS settings with nmcli requires the connection to go down/up,
+# so we directly make the change using resolvectl
+
+export LC_ALL=C
+
+if [ "$1" = crc ]; then
+        resolvectl domain "$1" ~testing
+        resolvectl dns "$1" 192.168.130.11
+        resolvectl default-route "$1" false
+fi
+
+exit 0
+`
+)
+
+var systemdResolvedPreflightChecks = [...]Check{
+	{
+		configKeySuffix:  "check-systemd-resolved-running",
+		checkDescription: "Checking if the systemd-resolved service is running",
+		check:            checkSystemdResolvedIsRunning,
+		fixDescription:   "systemd-resolved is required on this distribution. Please make sure it is installed and running manually",
+		flags:            NoFix,
+	},
+	{
+		configKeySuffix:    "check-crc-nm-dispatcher-file",
+		checkDescription:   fmt.Sprintf("Checking if %s exists", crcNetworkManagerDispatcherPath),
+		check:              checkCrcNetworkManagerDispatcherFile,
+		fixDescription:     "Writing NetworkManager dispatcher file for crc",
+		fix:                fixCrcNetworkManagerDispatcherFile,
+		cleanupDescription: fmt.Sprintf("Removing %s file", crcNetworkManagerDispatcherPath),
+		cleanup:            removeCrcNetworkManagerDispatcherFile,
+	},
+}
+
+func fixNetworkManagerConfigFile(path string, content string, perms os.FileMode) error {
+	err := crcos.WriteToFileAsRoot(
+		fmt.Sprintf("write NetworkManager configuration to %s", path),
+		content,
+		path,
+		perms,
+	)
+	if err != nil {
+		return fmt.Errorf("Failed to write config file: %s: %v", path, err)
+	}
+
+	logging.Debug("Reloading NetworkManager")
+	sd := systemd.NewHostSystemdCommander()
+	if err := sd.Reload("NetworkManager"); err != nil {
+		return fmt.Errorf("Failed to restart NetworkManager: %v", err)
+	}
+
+	return nil
+}
+
+func removeNetworkManagerConfigFile(path string) error {
+	if err := checkNetworkManagerInstalled(); err != nil {
+		// When NetworkManager is not installed, its config files won't exist
+		return nil
+	}
+	if _, err := os.Stat(path); !os.IsNotExist(err) {
+		logging.Debugf("Removing NetworkManager configuration file: %s", path)
+		err := crcos.RemoveFileAsRoot(
+			fmt.Sprintf("removing NetworkManager configuration file in %s", path),
+			path,
+		)
+		if err != nil {
+			return fmt.Errorf("Failed to remove NetworkManager configuration file: %s: %v", path, err)
+		}
+
+		logging.Debug("Reloading NetworkManager")
+		sd := systemd.NewHostSystemdCommander()
+		if err := sd.Reload("NetworkManager"); err != nil {
+			return fmt.Errorf("Failed to restart NetworkManager: %v", err)
+		}
+	}
+	return nil
+}
+
+func checkCrcDnsmasqConfigFile() error {
+	logging.Debug("Checking dnsmasq configuration")
+	err := crcos.FileContentMatches(crcDnsmasqConfigPath, []byte(crcDnsmasqConfig))
+	if err != nil {
+		return err
+	}
+	logging.Debug("dnsmasq configuration is good")
+	return nil
+}
+
+func fixCrcDnsmasqConfigFile() error {
+	logging.Debug("Fixing dnsmasq configuration")
+	err := fixNetworkManagerConfigFile(crcDnsmasqConfigPath, crcDnsmasqConfig, 0644)
+	if err != nil {
+		return err
+	}
+
+	logging.Debug("dnsmasq configuration fixed")
+	return nil
+}
+
+func removeCrcDnsmasqConfigFile() error {
+	return removeNetworkManagerConfigFile(crcDnsmasqConfigPath)
+}
+
+func checkCrcNetworkManagerConfig() error {
+	logging.Debug("Checking NetworkManager configuration")
+	err := crcos.FileContentMatches(crcNetworkManagerConfigPath, []byte(crcNetworkManagerConfig))
+	if err != nil {
+		return err
+	}
+	logging.Debug("NetworkManager configuration is good")
+	return nil
+}
+
+func fixCrcNetworkManagerConfig() error {
+	logging.Debug("Fixing NetworkManager configuration")
+	err := fixNetworkManagerConfigFile(crcNetworkManagerConfigPath, crcNetworkManagerConfig, 0644)
+	if err != nil {
+		return err
+	}
+	logging.Debug("NetworkManager configuration fixed")
+	return nil
+}
+
+func removeCrcNetworkManagerConfig() error {
+	return removeNetworkManagerConfigFile(crcNetworkManagerConfigPath)
+}
+
+func checkSystemdNetworkdIsNotRunning() error {
+	err := checkSystemdServiceRunning("systemd-networkd.service")
+	if err == nil {
+		return fmt.Errorf("systemd-networkd.service is running")
+	}
+
+	logging.Debugf("systemd-networkd.service is not running")
+	return nil
+}
+
+func checkNetworkManagerInstalled() error {
+	logging.Debug("Checking if 'nmcli' is available")
+	path, err := exec.LookPath("nmcli")
+	if err != nil {
+		return fmt.Errorf("NetworkManager cli nmcli was not found in path")
+	}
+	logging.Debug("'nmcli' was found in ", path)
+	return nil
+}
+
+func checkSystemdServiceRunning(service string) error {
+	logging.Debugf("Checking if %s is running", service)
+	sd := systemd.NewHostSystemdCommander()
+	status, err := sd.Status(service)
+	if err != nil {
+		return err
+	}
+	if status != states.Running {
+		return fmt.Errorf("%s is not running", service)
+	}
+	logging.Debugf("%s is already running", service)
+	return nil
+}
+
+func checkNetworkManagerIsRunning() error {
+	return checkSystemdServiceRunning("NetworkManager.service")
+}
+
+func checkSystemdResolvedIsRunning() error {
+	return checkSystemdServiceRunning("systemd-resolved.service")
+}
+
+func checkCrcNetworkManagerDispatcherFile() error {
+	logging.Debug("Checking NetworkManager dispatcher file for crc network")
+	err := crcos.FileContentMatches(crcNetworkManagerDispatcherPath, []byte(crcNetworkManagerDispatcherConfig))
+	if err != nil {
+		return err
+	}
+	logging.Debug("Dispatcher file has the expected content")
+	return nil
+}
+
+func fixCrcNetworkManagerDispatcherFile() error {
+	logging.Debug("Fixing NetworkManager dispatcher configuration")
+	err := fixNetworkManagerConfigFile(crcNetworkManagerDispatcherPath, crcNetworkManagerDispatcherConfig, 0755)
+	if err != nil {
+		return err
+	}
+
+	logging.Debug("NetworkManager dispatcher configuration fixed")
+	return nil
+}
+
+func removeCrcNetworkManagerDispatcherFile() error {
+	return removeNetworkManagerConfigFile(crcNetworkManagerDispatcherPath)
+}
diff --git a/pkg/crc/preflight/preflight_linux.go b/pkg/crc/preflight/preflight_linux.go
index 21beeee5f..246465c9c 100644
--- a/pkg/crc/preflight/preflight_linux.go
+++ b/pkg/crc/preflight/preflight_linux.go
@@ -64,6 +64,14 @@ var libvirtPreflightChecks = [...]Check{
 		fixDescription:   "Installing crc-driver-libvirt",
 		fix:              fixMachineDriverLibvirtInstalled,
 	},
+	{
+		cleanupDescription: "Removing the crc VM if exists",
+		cleanup:            removeCrcVM,
+		flags:              CleanUpOnly,
+	},
+}
+
+var libvirtNetworkPreflightChecks = [...]Check{
 	{
 		configKeySuffix:    "check-crc-network",
 		checkDescription:   "Checking if libvirt 'crc' network is available",
@@ -80,11 +88,6 @@ var libvirtPreflightChecks = [...]Check{
 		fixDescription:   "Starting libvirt 'crc' network",
 		fix:              fixLibvirtCrcNetworkActive,
 	},
-	{
-		cleanupDescription: "Removing the crc VM if exists",
-		cleanup:            removeCrcVM,
-		flags:              CleanUpOnly,
-	},
 }
 
 var vsockPreflightChecks = Check{
@@ -162,42 +165,64 @@ func getPreflightChecks(_ bool, networkMode network.Mode) []Check {
 	return getPreflightChecksForDistro(distro(), networkMode)
 }
 
-func getPreflightChecksForDistro(distro linux.OsType, networkMode network.Mode) []Check {
-	checks := commonChecks()
+func getNetworkChecksForDistro(distro *linux.OsRelease, networkMode network.Mode) []Check {
+	var checks []Check
 
 	if networkMode == network.VSockMode {
-		checks = append(checks, vsockPreflightChecks)
+		return append(checks, vsockPreflightChecks)
 	}
 
-	switch distro {
-	case linux.Ubuntu:
-	case linux.RHEL, linux.CentOS, linux.Fedora:
-		if networkMode == network.DefaultMode {
-			checks = append(checks, redhatPreflightChecks[:]...)
-		}
+	switch distroID(distro) {
 	default:
-		logging.Warnf("distribution-specific preflight checks are not implemented for %s", distro)
-		if networkMode == network.DefaultMode {
-			checks = append(checks, redhatPreflightChecks[:]...)
+		logging.Warnf("distribution-specific preflight checks are not implemented for '%s'", distroID(distro))
+		fallthrough
+	case linux.RHEL, linux.CentOS, linux.Fedora:
+		checks = append(checks, nmPreflightChecks[:]...)
+		if usesSystemdResolved(distro) {
+			checks = append(checks, systemdResolvedPreflightChecks[:]...)
+		} else {
+			checks = append(checks, dnsmasqPreflightChecks[:]...)
 		}
+	case linux.Ubuntu:
+		break
 	}
 
 	return checks
 }
 
-func commonChecks() []Check {
+func getPreflightChecksForDistro(distro *linux.OsRelease, networkMode network.Mode) []Check {
 	var checks []Check
 	checks = append(checks, genericPreflightChecks[:]...)
 	checks = append(checks, nonWinPreflightChecks[:]...)
 	checks = append(checks, libvirtPreflightChecks[:]...)
+	networkChecks := getNetworkChecksForDistro(distro, networkMode)
+	checks = append(checks, networkChecks...)
+	checks = append(checks, libvirtNetworkPreflightChecks[:]...)
 	return checks
 }
 
-func distro() linux.OsType {
+func usesSystemdResolved(osRelease *linux.OsRelease) bool {
+	switch distroID(osRelease) {
+	case linux.Fedora:
+		return osRelease.VersionID >= "33"
+	default:
+		return false
+	}
+}
+
+func distroID(osRelease *linux.OsRelease) linux.OsType {
+	if osRelease == nil {
+		return "unknown"
+	}
+	// FIXME: should also use IDLike
+	return osRelease.ID
+}
+
+func distro() *linux.OsRelease {
 	distro, err := linux.GetOsRelease()
 	if err != nil {
 		logging.Warnf("cannot get distribution name: %v", err)
-		return "unknown"
+		return nil
 	}
-	return distro.ID
+	return distro
 }
diff --git a/pkg/crc/preflight/preflight_redhat.go b/pkg/crc/preflight/preflight_redhat.go
deleted file mode 100644
index 2376b14f9..000000000
--- a/pkg/crc/preflight/preflight_redhat.go
+++ /dev/null
@@ -1,214 +0,0 @@
-// +build linux
-
-package preflight
-
-import (
-	"bytes"
-	"fmt"
-	"io/ioutil"
-	"os"
-	"os/exec"
-	"path/filepath"
-
-	"github.com/code-ready/crc/pkg/crc/logging"
-	"github.com/code-ready/crc/pkg/crc/systemd"
-	"github.com/code-ready/crc/pkg/crc/systemd/states"
-	crcos "github.com/code-ready/crc/pkg/os"
-)
-
-var redhatPreflightChecks = [...]Check{
-	{
-		configKeySuffix:  "check-network-manager-installed",
-		checkDescription: "Checking if NetworkManager is installed",
-		check:            checkNetworkManagerInstalled,
-		fixDescription:   "Checking if NetworkManager is installed",
-		fix:              fixNetworkManagerInstalled,
-	},
-	{
-		configKeySuffix:  "check-network-manager-running",
-		checkDescription: "Checking if NetworkManager service is running",
-		check:            checkNetworkManagerIsRunning,
-		fixDescription:   "Checking if NetworkManager service is running",
-		fix:              fixNetworkManagerIsRunning,
-	},
-	{
-		configKeySuffix:    "check-network-manager-config",
-		checkDescription:   "Checking if /etc/NetworkManager/conf.d/crc-nm-dnsmasq.conf exists",
-		check:              checkCrcNetworkManagerConfig,
-		fixDescription:     "Writing Network Manager config for crc",
-		fix:                fixCrcNetworkManagerConfig,
-		cleanupDescription: "Removing /etc/NetworkManager/conf.d/crc-nm-dnsmasq.conf file",
-		cleanup:            removeCrcNetworkManagerConfig,
-	},
-	{
-		configKeySuffix:    "check-crc-dnsmasq-file",
-		checkDescription:   "Checking if /etc/NetworkManager/dnsmasq.d/crc.conf exists",
-		check:              checkCrcDnsmasqConfigFile,
-		fixDescription:     "Writing dnsmasq config for crc",
-		fix:                fixCrcDnsmasqConfigFile,
-		cleanupDescription: "Removing /etc/NetworkManager/dnsmasq.d/crc.conf file",
-		cleanup:            removeCrcDnsmasqConfigFile,
-	},
-}
-
-func checkCrcDnsmasqConfigFile() error {
-	logging.Debug("Checking dnsmasq configuration")
-	c := []byte(crcDnsmasqConfig)
-	_, err := os.Stat(crcDnsmasqConfigPath)
-	if err != nil {
-		return fmt.Errorf("File not found: %s: %s", crcDnsmasqConfigPath, err.Error())
-	}
-	config, err := ioutil.ReadFile(filepath.Clean(crcDnsmasqConfigPath))
-	if err != nil {
-		return fmt.Errorf("Error opening file: %s: %s", crcDnsmasqConfigPath, err.Error())
-	}
-	if !bytes.Equal(config, c) {
-		return fmt.Errorf("Config file contains changes: %s", crcDnsmasqConfigPath)
-	}
-	logging.Debug("dnsmasq configuration is good")
-	return nil
-}
-
-func fixCrcDnsmasqConfigFile() error {
-	logging.Debug("Fixing dnsmasq configuration")
-	err := crcos.WriteToFileAsRoot(
-		fmt.Sprintf("write dnsmasq configuration in %s", crcDnsmasqConfigPath),
-		crcDnsmasqConfig,
-		crcDnsmasqConfigPath,
-		0644,
-	)
-	if err != nil {
-		return fmt.Errorf("Failed to write dnsmasq config file: %s: %v", crcDnsmasqConfigPath, err)
-	}
-
-	logging.Debug("Reloading NetworkManager")
-	sd := systemd.NewHostSystemdCommander()
-	if err := sd.Reload("NetworkManager"); err != nil {
-		return fmt.Errorf("Failed to restart NetworkManager: %v", err)
-	}
-
-	logging.Debug("dnsmasq configuration fixed")
-	return nil
-}
-
-func removeCrcDnsmasqConfigFile() error {
-	if err := checkNetworkManagerInstalled(); err != nil {
-		// When NetworkManager is not installed, this file won't exist
-		return nil
-	}
-	// Delete the `crcDnsmasqConfigPath` file if exists,
-	// ignore all the os PathError except `IsNotExist` one.
-	if _, err := os.Stat(crcDnsmasqConfigPath); !os.IsNotExist(err) {
-		logging.Debug("Removing dnsmasq configuration")
-		err := crcos.RemoveFileAsRoot(
-			fmt.Sprintf("removing dnsmasq configuration in %s", crcDnsmasqConfigPath),
-			crcDnsmasqConfigPath,
-		)
-		if err != nil {
-			return fmt.Errorf("Failed to remove dnsmasq config file: %s: %v", crcDnsmasqConfigPath, err)
-		}
-
-		logging.Debug("Reloading NetworkManager")
-		sd := systemd.NewHostSystemdCommander()
-		if err := sd.Reload("NetworkManager"); err != nil {
-			return fmt.Errorf("Failed to restart NetworkManager: %v", err)
-		}
-	}
-	return nil
-}
-
-func checkCrcNetworkManagerConfig() error {
-	logging.Debug("Checking NetworkManager configuration")
-	c := []byte(crcNetworkManagerConfig)
-	_, err := os.Stat(crcNetworkManagerConfigPath)
-	if err != nil {
-		return fmt.Errorf("File not found: %s: %s", crcNetworkManagerConfigPath, err.Error())
-	}
-	config, err := ioutil.ReadFile(filepath.Clean(crcNetworkManagerConfigPath))
-	if err != nil {
-		return fmt.Errorf("Error opening file: %s: %s", crcNetworkManagerConfigPath, err.Error())
-	}
-	if !bytes.Equal(config, c) {
-		return fmt.Errorf("Config file contains changes: %s", crcNetworkManagerConfigPath)
-	}
-	logging.Debug("NetworkManager configuration is good")
-	return nil
-}
-
-func fixCrcNetworkManagerConfig() error {
-	logging.Debug("Fixing NetworkManager configuration")
-	err := crcos.WriteToFileAsRoot(
-		fmt.Sprintf("write NetworkManager config in %s", crcNetworkManagerConfigPath),
-		crcNetworkManagerConfig,
-		crcNetworkManagerConfigPath,
-		0644,
-	)
-	if err != nil {
-		return fmt.Errorf("Failed to write NetworkManager config file: %s: %v", crcNetworkManagerConfigPath, err)
-	}
-
-	logging.Debug("Reloading NetworkManager")
-	sd := systemd.NewHostSystemdCommander()
-	if err := sd.Reload("NetworkManager"); err != nil {
-		return fmt.Errorf("Failed to restart NetworkManager: %v", err)
-	}
-
-	logging.Debug("NetworkManager configuration fixed")
-	return nil
-}
-
-func removeCrcNetworkManagerConfig() error {
-	if err := checkNetworkManagerInstalled(); err != nil {
-		// When NetworkManager is not installed, this file won't exist
-		return nil
-	}
-	if _, err := os.Stat(crcNetworkManagerConfigPath); !os.IsNotExist(err) {
-		logging.Debug("Removing NetworkManager configuration")
-		err := crcos.RemoveFileAsRoot(
-			fmt.Sprintf("Removing NetworkManager config in %s", crcNetworkManagerConfigPath),
-			crcNetworkManagerConfigPath,
-		)
-		if err != nil {
-			return fmt.Errorf("Failed to remove NetworkManager config file: %s: %v", crcNetworkManagerConfigPath, err)
-		}
-
-		logging.Debug("Reloading NetworkManager")
-		sd := systemd.NewHostSystemdCommander()
-		if err := sd.Reload("NetworkManager"); err != nil {
-			return fmt.Errorf("Failed to restart NetworkManager: %v", err)
-		}
-	}
-	return nil
-}
-
-func checkNetworkManagerInstalled() error {
-	logging.Debug("Checking if 'nmcli' is available")
-	path, err := exec.LookPath("nmcli")
-	if err != nil {
-		return fmt.Errorf("NetworkManager cli nmcli was not found in path")
-	}
-	logging.Debug("'nmcli' was found in ", path)
-	return nil
-}
-
-func fixNetworkManagerInstalled() error {
-	return fmt.Errorf("NetworkManager is required and must be installed manually")
-}
-
-func checkNetworkManagerIsRunning() error {
-	logging.Debug("Checking if NetworkManager.service is running")
-	sd := systemd.NewHostSystemdCommander()
-	status, err := sd.Status("NetworkManager")
-	if err != nil {
-		return err
-	}
-	if status != states.Running {
-		return fmt.Errorf("NetworkManager.service is not running")
-	}
-	logging.Debug("NetworkManager.service is already running")
-	return nil
-}
-
-func fixNetworkManagerIsRunning() error {
-	return fmt.Errorf("NetworkManager is required. Please make sure it is installed and running manually")
-}
diff --git a/pkg/os/util.go b/pkg/os/util.go
index 35fc42af6..da41e5b57 100644
--- a/pkg/os/util.go
+++ b/pkg/os/util.go
@@ -61,13 +61,29 @@ func CopyFileContents(src string, dst string, permission os.FileMode) error {
 	return nil
 }
 
+func FileContentMatches(path string, expectedContent []byte) error {
+	_, err := os.Stat(path)
+	if err != nil {
+		return fmt.Errorf("File not found: %s: %s", path, err.Error())
+	}
+	content, err := ioutil.ReadFile(filepath.Clean(path))
+	if err != nil {
+		return fmt.Errorf("Error opening file: %s: %s", path, err.Error())
+	}
+	if !bytes.Equal(content, expectedContent) {
+		return fmt.Errorf("File has unexpected content: %s", path)
+
+	}
+	return nil
+}
+
 func WriteFileIfContentChanged(path string, newContent []byte, perm os.FileMode) (bool, error) {
-	oldContent, err := ioutil.ReadFile(filepath.Clean(path))
-	if (err == nil) && (bytes.Equal(oldContent, newContent)) {
+	err := FileContentMatches(path, newContent)
+	if err == nil {
 		return false, nil
 	}
-	/* Intentionally ignore errors, just try to write the file if we can't read it */
 
+	/* Intentionally ignore errors, just try to write the file if we can't read it */
 	err = ioutil.WriteFile(path, newContent, perm)
 	if err != nil {
 		return false, err
diff --git a/pkg/os/util_linux.go b/pkg/os/util_linux.go
index 8e38cdb69..e39aa003a 100644
--- a/pkg/os/util_linux.go
+++ b/pkg/os/util_linux.go
@@ -20,7 +20,7 @@ func WriteToFileAsRoot(reason, content, filepath string, mode os.FileMode) error
 	if err := cmd.Run(); err != nil {
 		return fmt.Errorf("Failed writing to file as root: %s: %s: %v", filepath, buf.String(), err)
 	}
-	if _, _, err := RunWithPrivilege(fmt.Sprintf("Changing permission for %s to %d ", filepath, mode),
+	if _, _, err := RunWithPrivilege(fmt.Sprintf("Changing permission for %s to %o ", filepath, mode),
 		"chmod", strconv.FormatUint(uint64(mode), 8), filepath); err != nil {
 		return err
 	}


Test patch:
diff --git a/pkg/crc/preflight/preflight_linux_test.go b/pkg/crc/preflight/preflight_linux_test.go
index c8664d09d..f7171b440 100644
--- a/pkg/crc/preflight/preflight_linux_test.go
+++ b/pkg/crc/preflight/preflight_linux_test.go
@@ -13,16 +13,40 @@ func TestCountConfigurationOptions(t *testing.T) {
 	cfg := config.New(config.NewEmptyInMemoryStorage())
 	RegisterSettings(cfg)
 	options := len(cfg.AllConfigs())
-	assert.True(t, options == 38 || options == 30)
+
+	var preflightChecksCount int
+	for _, check := range getAllPreflightChecks() {
+		if check.configKeySuffix != "" {
+			preflightChecksCount += 2
+		}
+	}
+	assert.True(t, options == preflightChecksCount, "Unexpected number of preflight configuration flags, got %d, expected %d", options, preflightChecksCount)
 }
 
+var (
+	rhel crcos.OsRelease = crcos.OsRelease{
+		ID:        crcos.RHEL,
+		VersionID: "8.2",
+	}
+
+	ubuntu crcos.OsRelease = crcos.OsRelease{
+		ID:        crcos.Ubuntu,
+		VersionID: "20.04",
+	}
+
+	unexpected crcos.OsRelease = crcos.OsRelease{
+		ID:        "unexpected",
+		VersionID: "1234",
+	}
+)
+
 func TestCountPreflights(t *testing.T) {
-	assert.Len(t, getPreflightChecksForDistro(crcos.RHEL, network.DefaultMode), 20)
-	assert.Len(t, getPreflightChecksForDistro(crcos.RHEL, network.VSockMode), 17)
+	assert.Len(t, getPreflightChecksForDistro(&rhel, network.DefaultMode), 21)
+	assert.Len(t, getPreflightChecksForDistro(&rhel, network.VSockMode), 17)
 
-	assert.Len(t, getPreflightChecksForDistro("unexpected", network.DefaultMode), 20)
-	assert.Len(t, getPreflightChecksForDistro("unexpected", network.VSockMode), 17)
+	assert.Len(t, getPreflightChecksForDistro(&unexpected, network.DefaultMode), 21)
+	assert.Len(t, getPreflightChecksForDistro(&unexpected, network.VSockMode), 17)
 
-	assert.Len(t, getPreflightChecksForDistro(crcos.Ubuntu, network.DefaultMode), 16)
-	assert.Len(t, getPreflightChecksForDistro(crcos.Ubuntu, network.VSockMode), 17)
+	assert.Len(t, getPreflightChecksForDistro(&ubuntu, network.DefaultMode), 16)
+	assert.Len(t, getPreflightChecksForDistro(&ubuntu, network.VSockMode), 17)
 }
diff --git a/pkg/os/util_test.go b/pkg/os/util_test.go
index cb6bc4ecf..04ee8328d 100644
--- a/pkg/os/util_test.go
+++ b/pkg/os/util_test.go
@@ -1,6 +1,9 @@
 package os
 
 import (
+	"io/ioutil"
+	"os"
+	"path/filepath"
 	"testing"
 
 	"github.com/stretchr/testify/assert"
@@ -22,3 +25,31 @@ func TestAddEnv(t *testing.T) {
 	assert.Len(t, replaced, 4)
 	assert.Equal(t, replaced[3], "KUBECONFIG=some-data")
 }
+
+func TestFileContentFuncs(t *testing.T) {
+	dir, err := ioutil.TempDir("", "filecontent")
+	assert.NoError(t, err)
+	defer os.RemoveAll(dir)
+
+	filename := filepath.Join(dir, "testfile")
+
+	assert.Error(t, FileContentMatches(filename, []byte("aaaa")))
+
+	written, err := WriteFileIfContentChanged(filename, []byte("aaaa"), 0600)
+	assert.NoError(t, err)
+	assert.Equal(t, written, true)
+
+	written, err = WriteFileIfContentChanged(filename, []byte("aaaa"), 0600)
+	assert.NoError(t, err)
+	assert.Equal(t, written, false)
+
+	assert.NoError(t, FileContentMatches(filename, []byte("aaaa")))
+	assert.Error(t, FileContentMatches(filename, []byte("aaaaa")))
+
+	written, err = WriteFileIfContentChanged(filename, []byte("aaaaa"), 0600)
+	assert.NoError(t, err)
+	assert.Equal(t, written, true)
+
+	assert.Error(t, FileContentMatches(filename, []byte("aaaa")))
+	assert.NoError(t, FileContentMatches(filename, []byte("aaaaa")))
+}
diff --git a/test/integration/features/basic.feature b/test/integration/features/basic.feature
index 33f4f9b36..f69f2e9d1 100644
--- a/test/integration/features/basic.feature
+++ b/test/integration/features/basic.feature
@@ -51,12 +51,12 @@ Feature: Basic test
         And stderr should contain "Checking if NetworkManager service is running"
         And stderr should contain "Checking if /etc/NetworkManager/conf.d/crc-nm-dnsmasq.conf exists"
         And stderr should contain "Writing Network Manager config for crc"
-        And stderr should contain "Will use root access: write NetworkManager config in /etc/NetworkManager/conf.d/crc-nm-dnsmasq.conf"
+        And stderr should contain "Will use root access: write NetworkManager configuration to /etc/NetworkManager/conf.d/crc-nm-dnsmasq.conf"
         And stderr should contain "Will use root access: executing systemctl daemon-reload command"
         And stderr should contain "Will use root access: executing systemctl reload NetworkManager"
         And stderr should contain "Checking if /etc/NetworkManager/dnsmasq.d/crc.conf exists"
-        And stderr should contain "Writing dnsmasq config for crc"
-        And stderr should contain "Will use root access: write dnsmasq configuration in /etc/NetworkManager/dnsmasq.d/crc.conf"
+	And stderr should contain "Writing dnsmasq config for crc"
+	And stderr should contain "Will use root access: write NetworkManager configuration to /etc/NetworkManager/dnsmasq.d/crc.conf"
         And stderr should contain "Will use root access: executing systemctl daemon-reload command"
         And stderr should contain "Will use root access: executing systemctl reload NetworkManager"
         And stdout should contain "Setup is complete, you can now run 'crc start -b $bundlename' to start the OpenShift cluster" if bundle is not embedded
@@ -150,11 +150,11 @@ Feature: Basic test
         When executing "crc cleanup" succeeds
         Then stderr should contain "Removing the crc VM if exists"
         And stderr should contain "Removing /etc/NetworkManager/dnsmasq.d/crc.conf file"
-        And stderr should contain "Will use root access: removing dnsmasq configuration in /etc/NetworkManager/dnsmasq.d/crc.conf"
+	And stderr should contain "Will use root access: removing NetworkManager configuration file in /etc/NetworkManager/dnsmasq.d/crc.conf"
         And stderr should contain "Will use root access: executing systemctl daemon-reload command"
         And stderr should contain "Will use root access: executing systemctl reload NetworkManager"
         And stderr should contain "Removing /etc/NetworkManager/conf.d/crc-nm-dnsmasq.conf file"
-        And stderr should contain "Will use root access: Removing NetworkManager config in /etc/NetworkManager/conf.d/crc-nm-dnsmasq.conf"
+        And stderr should contain "Will use root access: removing NetworkManager configuration file in /etc/NetworkManager/conf.d/crc-nm-dnsmasq.conf"
         And stderr should contain "Will use root access: executing systemctl daemon-reload command"
         And stderr should contain "Will use root access: executing systemctl reload NetworkManager"
         And stderr should contain "Removing 'crc' network from libvirt"