Repository: code-ready/crc
Pull request #2212 (2021-04-12 08:55:23)

vsock: check daemon is running in machine.Start instead of preflights
'crc daemon' needs setup to be done but also 'crc setup' requires the
daemon to be running. There is a dependency loop.

This patch removes the need of 'crc daemon' running before 'crc setup'.
'crc start' will complain before starting the VM if the daemon is not
running.

```
$ crc start -p ~/pull-secret.txt -b ~/Downloads/crc_libvirt_4.7.2.crcbundle
...
INFO Checking if vsock is correctly configured    
INFO Loading bundle: crc_libvirt_4.7.2.crcbundle ... 
ERRO Is 'crc daemon' running? Network mode 'vsock' requires 'crc daemon' to be running, run it manually on different terminal/tab 
Get "http://unix/network/services/forwarder/all": dial unix /home/guillaumerose/.crc/crc-http.sock: connect: connection refused

```

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

This pull-request has been approved by: *<a href="https://github.com/code-ready/crc/pull/2212#" title="Author self-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)~~ [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":[]} -->
openshift-ci-robot: [APPROVALNOTIFIER] This PR is **APPROVED**

This pull-request has been approved by: *<a href="https://github.com/code-ready/crc/pull/2212#pullrequestreview-633297523" title="Approved">cfergeau</a>*, *<a href="https://github.com/code-ready/crc/pull/2212#" title="Author self-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":[]} -->
openshift-ci-robot: New changes are detected. LGTM label has been removed.
openshift-ci-robot: [APPROVALNOTIFIER] This PR is **APPROVED**

This pull-request has been approved by: *<a href="https://github.com/code-ready/crc/pull/2212#pullrequestreview-633369668" title="Approved">anjannath</a>*, *<a href="https://github.com/code-ready/crc/pull/2212#pullrequestreview-633297523" title="Approved">cfergeau</a>*, *<a href="https://github.com/code-ready/crc/pull/2212#" title="Author self-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)~~ [anjannath,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":[]} -->
openshift-ci[bot]: @guillaumerose: The following test **failed**, say `/retest` to rerun all failed tests:

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

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

<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: /lgtm

Patch:
diff --git a/pkg/crc/machine/vsock.go b/pkg/crc/machine/vsock.go
index b09601ec5..e1efd0da3 100644
--- a/pkg/crc/machine/vsock.go
+++ b/pkg/crc/machine/vsock.go
@@ -5,6 +5,7 @@ import (
 
 	"github.com/code-ready/crc/pkg/crc/constants"
 	"github.com/code-ready/crc/pkg/crc/daemonclient"
+	"github.com/code-ready/crc/pkg/crc/logging"
 	"github.com/code-ready/gvisor-tap-vsock/pkg/types"
 	"github.com/pkg/errors"
 )
@@ -14,6 +15,7 @@ func exposePorts() error {
 	portsToExpose := vsockPorts()
 	alreadyOpenedPorts, err := daemonClient.NetworkClient.List()
 	if err != nil {
+		logging.Error("Is 'crc daemon' running? Network mode 'vsock' requires 'crc daemon' to be running, run it manually on different terminal/tab")
 		return err
 	}
 	var missingPorts []types.ExposeRequest
diff --git a/pkg/crc/preflight/preflight_checks_nondarwin.go b/pkg/crc/preflight/preflight_checks_nondarwin.go
deleted file mode 100644
index 6d498088f..000000000
--- a/pkg/crc/preflight/preflight_checks_nondarwin.go
+++ /dev/null
@@ -1,36 +0,0 @@
-// +build !darwin
-
-package preflight
-
-import (
-	"errors"
-	"net"
-
-	"github.com/code-ready/crc/pkg/crc/constants"
-)
-
-const daemonNotRunningErrMsg = "Network mode 'vsock' requires 'crc daemon' to be running, run it manually on different terminal/tab"
-
-var daemonRunningChecks = Check{
-	configKeySuffix:  "check-if-daemon-running",
-	checkDescription: "Checking if crc daemon is running",
-	check:            checkIfCRCDaemonRunning,
-	fixDescription:   daemonNotRunningErrMsg,
-	flags:            NoFix,
-}
-
-func checkIfCRCDaemonRunning() error {
-	l, err := net.Dial("unix", constants.DaemonSocketPath)
-	if err != nil {
-		// Log or report the error here
-		return errors.New(daemonNotRunningErrMsg)
-	}
-
-	defer func() {
-		if l != nil {
-			l.Close()
-		}
-	}()
-
-	return nil
-}
diff --git a/pkg/crc/preflight/preflight_linux.go b/pkg/crc/preflight/preflight_linux.go
index c06c7d27a..a9cd469c8 100644
--- a/pkg/crc/preflight/preflight_linux.go
+++ b/pkg/crc/preflight/preflight_linux.go
@@ -232,8 +232,7 @@ func getNetworkChecks(networkMode network.Mode, systemdResolved bool) []Check {
 	var checks []Check
 
 	if networkMode == network.VSockMode {
-		checks = append(checks, vsockPreflightChecks)
-		return append(checks, daemonRunningChecks)
+		return append(checks, vsockPreflightChecks)
 	}
 
 	checks = append(checks, nmPreflightChecks[:]...)
diff --git a/pkg/crc/preflight/preflight_windows.go b/pkg/crc/preflight/preflight_windows.go
index 268743d6a..9ced5a2d9 100644
--- a/pkg/crc/preflight/preflight_windows.go
+++ b/pkg/crc/preflight/preflight_windows.go
@@ -148,7 +148,6 @@ func getPreflightChecks(experimentalFeatures bool, _ bool, networkMode network.M
 
 	if networkMode == network.VSockMode {
 		checks = append(checks, vsockChecks[:]...)
-		checks = append(checks, daemonRunningChecks)
 	}
 
 	// Experimental feature


Test patch:
diff --git a/pkg/crc/preflight/preflight_linux_test.go b/pkg/crc/preflight/preflight_linux_test.go
index 8e15b6644..a54bc1805 100644
--- a/pkg/crc/preflight/preflight_linux_test.go
+++ b/pkg/crc/preflight/preflight_linux_test.go
@@ -145,7 +145,6 @@ var checkListForDistros = []checkListForDistro{
 			{check: checkMachineDriverLibvirtInstalled},
 			{cleanup: removeCrcVM},
 			{check: checkVsock},
-			{check: checkIfCRCDaemonRunning},
 			{check: checkBundleExtracted},
 		},
 	},
@@ -237,7 +236,6 @@ var checkListForDistros = []checkListForDistro{
 			{check: checkMachineDriverLibvirtInstalled},
 			{cleanup: removeCrcVM},
 			{check: checkVsock},
-			{check: checkIfCRCDaemonRunning},
 			{check: checkBundleExtracted},
 		},
 	},
@@ -329,7 +327,6 @@ var checkListForDistros = []checkListForDistro{
 			{check: checkMachineDriverLibvirtInstalled},
 			{cleanup: removeCrcVM},
 			{check: checkVsock},
-			{check: checkIfCRCDaemonRunning},
 			{check: checkBundleExtracted},
 		},
 	},
@@ -424,7 +421,6 @@ var checkListForDistros = []checkListForDistro{
 			{cleanup: removeCrcVM},
 			{configKeySuffix: "check-apparmor-profile-setup"},
 			{check: checkVsock},
-			{check: checkIfCRCDaemonRunning},
 			{check: checkBundleExtracted},
 		},
 	},
diff --git a/pkg/crc/preflight/preflight_windows_test.go b/pkg/crc/preflight/preflight_windows_test.go
index 34afd227a..77724abe0 100644
--- a/pkg/crc/preflight/preflight_windows_test.go
+++ b/pkg/crc/preflight/preflight_windows_test.go
@@ -11,13 +11,13 @@ import (
 func TestCountConfigurationOptions(t *testing.T) {
 	cfg := config.New(config.NewEmptyInMemoryStorage())
 	RegisterSettings(cfg)
-	assert.Len(t, cfg.AllConfigs(), 13)
+	assert.Len(t, cfg.AllConfigs(), 12)
 }
 
 func TestCountPreflights(t *testing.T) {
 	assert.Len(t, getPreflightChecks(false, false, network.DefaultMode), 16)
 	assert.Len(t, getPreflightChecks(true, true, network.DefaultMode), 19)
 
-	assert.Len(t, getPreflightChecks(false, false, network.VSockMode), 18)
-	assert.Len(t, getPreflightChecks(true, true, network.VSockMode), 21)
+	assert.Len(t, getPreflightChecks(false, false, network.VSockMode), 17)
+	assert.Len(t, getPreflightChecks(true, true, network.VSockMode), 20)
 }