Repository: code-ready/crc
Pull request #2330 (2021-05-11 07:00:21)

Use enum for openshift operator statuses

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

This pull-request has been approved by: *<a href="https://github.com/code-ready/crc/pull/2330#" 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":[]} -->
cfergeau: /lgtm

Patch:
diff --git a/pkg/crc/cluster/clusteroperator.go b/pkg/crc/cluster/clusteroperator.go
index 0d56d8835..fcf234747 100644
--- a/pkg/crc/cluster/clusteroperator.go
+++ b/pkg/crc/cluster/clusteroperator.go
@@ -71,10 +71,6 @@ func (status *Status) IsReady() bool {
 	return status.Available && !status.Progressing && !status.Degraded && !status.Disabled
 }
 
-func GetClusterOperatorStatus(ocConfig oc.Config, operator string) (*Status, error) {
-	return getStatus(ocConfig, []string{operator})
-}
-
 func GetClusterOperatorsStatus(ocConfig oc.Config) (*Status, error) {
 	return getStatus(ocConfig, []string{})
 }
@@ -102,31 +98,33 @@ func getStatus(ocConfig oc.Config, selector []string) (*Status, error) {
 		found = true
 		for _, con := range c.Status.Conditions {
 			switch con.Type {
-			case "Available":
-				if con.Status != "True" {
+			case openshiftapi.OperatorAvailable:
+				if con.Status != openshiftapi.ConditionTrue {
 					logging.Debug(c.ObjectMeta.Name, " operator not available, Reason: ", con.Reason)
 					cs.unavailable = append(cs.unavailable, c.ObjectMeta.Name)
 					cs.Available = false
 				}
-			case "Degraded":
-				if con.Status == "True" {
+			case openshiftapi.OperatorDegraded:
+				if con.Status == openshiftapi.ConditionTrue {
 					logging.Debug(c.ObjectMeta.Name, " operator is degraded, Reason: ", con.Reason)
 					cs.degraded = append(cs.degraded, c.ObjectMeta.Name)
 					cs.Degraded = true
 				}
-			case "Progressing":
-				if con.Status == "True" {
+			case openshiftapi.OperatorProgressing:
+				if con.Status == openshiftapi.ConditionTrue {
 					logging.Debug(c.ObjectMeta.Name, " operator is still progressing, Reason: ", con.Reason)
 					cs.progressing = append(cs.progressing, c.ObjectMeta.Name)
 					cs.Progressing = true
 				}
-			case "Upgradeable":
+			case openshiftapi.OperatorUpgradeable:
 				continue
-			case "Disabled":
-				if con.Status == "True" {
+			case "Disabled": // non official status, used by insights and cluster baremetal operators
+				if con.Status == openshiftapi.ConditionTrue {
 					logging.Debug(c.ObjectMeta.Name, " operator is disabled, Reason: ", con.Reason)
 					cs.Disabled = true
 				}
+			case "ManagementStateDegraded": // only for the network operator
+				continue
 			default:
 				logging.Debugf("Unexpected operator status for %s: %s", c.ObjectMeta.Name, con.Type)
 			}
diff --git a/pkg/crc/cluster/status.go b/pkg/crc/cluster/status.go
index 39f918b12..671ef44ec 100644
--- a/pkg/crc/cluster/status.go
+++ b/pkg/crc/cluster/status.go
@@ -10,7 +10,7 @@ import (
 )
 
 // WaitForClusterStable checks that the cluster is running a number of consecutive times
-func WaitForClusterStable(ctx context.Context, ocConfig oc.Config, monitoringEnabled bool) error {
+func WaitForClusterStable(ctx context.Context, ocConfig oc.Config) error {
 	if ctx.Err() != nil {
 		return ctx.Err()
 	}
diff --git a/pkg/crc/machine/start.go b/pkg/crc/machine/start.go
index 8c1cfa571..f511a5213 100644
--- a/pkg/crc/machine/start.go
+++ b/pkg/crc/machine/start.go
@@ -443,7 +443,7 @@ func (client *client) Start(ctx context.Context, startConfig types.StartConfig)
 
 	logging.Info("Starting OpenShift cluster... [waiting for the cluster to stabilize]")
 
-	if err := cluster.WaitForClusterStable(ctx, ocConfig, client.monitoringEnabled()); err != nil {
+	if err := cluster.WaitForClusterStable(ctx, ocConfig); err != nil {
 		logging.Errorf("Cluster is not ready: %v", err)
 	}
 
diff --git a/pkg/crc/preflight/preflight_linux.go b/pkg/crc/preflight/preflight_linux.go
index b47e34f97..265d60e49 100644
--- a/pkg/crc/preflight/preflight_linux.go
+++ b/pkg/crc/preflight/preflight_linux.go
@@ -228,16 +228,13 @@ func removeVsockCrcSettings() error {
 func getAllPreflightChecks() []Check {
 	usingSystemdResolved := checkSystemdResolvedIsRunning()
 	checks := getPreflightChecksForDistro(distro(), network.SystemNetworkingMode, usingSystemdResolved == nil)
-	checks = append(checks, wsl2PreflightChecks)
 	checks = append(checks, vsockPreflightChecks)
 	return checks
 }
 
 func getPreflightChecks(_ bool, _ bool, networkMode network.Mode) []Check {
 	usingSystemdResolved := checkSystemdResolvedIsRunning()
-	checks := getPreflightChecksForDistro(distro(), networkMode, usingSystemdResolved == nil)
-	checks = append(checks, wsl2PreflightChecks)
-	return checks
+	return getPreflightChecksForDistro(distro(), networkMode, usingSystemdResolved == nil)
 }
 
 func getNetworkChecks(networkMode network.Mode, systemdResolved bool) []Check {
@@ -260,6 +257,7 @@ func getNetworkChecks(networkMode network.Mode, systemdResolved bool) []Check {
 func getPreflightChecksForDistro(distro *linux.OsRelease, networkMode network.Mode, systemdResolved bool) []Check {
 	var checks []Check
 	checks = append(checks, nonWinPreflightChecks[:]...)
+	checks = append(checks, wsl2PreflightChecks)
 	checks = append(checks, genericPreflightChecks[:]...)
 	checks = append(checks, libvirtPreflightChecks(distro)...)
 	networkChecks := getNetworkChecks(networkMode, systemdResolved)


Test patch:
diff --git a/pkg/crc/cluster/clusteroperator_test.go b/pkg/crc/cluster/clusteroperator_test.go
index 3740c65fb..fea4afa03 100644
--- a/pkg/crc/cluster/clusteroperator_test.go
+++ b/pkg/crc/cluster/clusteroperator_test.go
@@ -32,25 +32,6 @@ func TestGetClusterOperatorsStatusProgressing(t *testing.T) {
 	assert.Equal(t, progressing, status)
 }
 
-func TestGetClusterOperatorStatus(t *testing.T) {
-	status, err := GetClusterOperatorStatus(ocConfig("co.json"), "authentication")
-	assert.NoError(t, err)
-	assert.Equal(t, available, status)
-
-	status, err = GetClusterOperatorStatus(ocConfig("co-progressing.json"), "authentication")
-	assert.NoError(t, err)
-	assert.Equal(t, progressing, status)
-
-	status, err = GetClusterOperatorStatus(ocConfig("co-progressing.json"), "cloud-credential")
-	assert.NoError(t, err)
-	assert.Equal(t, available, status)
-}
-
-func TestGetClusterOperatorStatusNotFound(t *testing.T) {
-	_, err := GetClusterOperatorStatus(ocConfig("co-progressing.json"), "not-found")
-	assert.EqualError(t, err, "no cluster operator found")
-}
-
 func ocConfig(s string) oc.Config {
 	return oc.Config{
 		Runner: &mockRunner{file: filepath.Join("testdata", s)},
diff --git a/pkg/crc/preflight/preflight_linux_test.go b/pkg/crc/preflight/preflight_linux_test.go
index ff2127ebb..526cea9f5 100644
--- a/pkg/crc/preflight/preflight_linux_test.go
+++ b/pkg/crc/preflight/preflight_linux_test.go
@@ -63,6 +63,7 @@ var checkListForDistros = []checkListForDistro{
 		systemdResolved: true,
 		checks: []Check{
 			{check: checkIfRunningAsNormalUser},
+			{check: checkRunningInsideWSL2},
 			{check: checkAdminHelperExecutableCached},
 			{check: checkSupportedCPUArch},
 			{configKeySuffix: "check-ram"},
@@ -96,6 +97,7 @@ var checkListForDistros = []checkListForDistro{
 		systemdResolved: false,
 		checks: []Check{
 			{check: checkIfRunningAsNormalUser},
+			{check: checkRunningInsideWSL2},
 			{check: checkAdminHelperExecutableCached},
 			{check: checkSupportedCPUArch},
 			{configKeySuffix: "check-ram"},
@@ -128,6 +130,7 @@ var checkListForDistros = []checkListForDistro{
 		systemdResolved: false,
 		checks: []Check{
 			{check: checkIfRunningAsNormalUser},
+			{check: checkRunningInsideWSL2},
 			{check: checkAdminHelperExecutableCached},
 			{check: checkSupportedCPUArch},
 			{configKeySuffix: "check-ram"},
@@ -154,6 +157,7 @@ var checkListForDistros = []checkListForDistro{
 		systemdResolved: true,
 		checks: []Check{
 			{check: checkIfRunningAsNormalUser},
+			{check: checkRunningInsideWSL2},
 			{check: checkAdminHelperExecutableCached},
 			{check: checkSupportedCPUArch},
 			{configKeySuffix: "check-ram"},
@@ -187,6 +191,7 @@ var checkListForDistros = []checkListForDistro{
 		systemdResolved: false,
 		checks: []Check{
 			{check: checkIfRunningAsNormalUser},
+			{check: checkRunningInsideWSL2},
 			{check: checkAdminHelperExecutableCached},
 			{check: checkSupportedCPUArch},
 			{configKeySuffix: "check-ram"},
@@ -219,6 +224,7 @@ var checkListForDistros = []checkListForDistro{
 		systemdResolved: false,
 		checks: []Check{
 			{check: checkIfRunningAsNormalUser},
+			{check: checkRunningInsideWSL2},
 			{check: checkAdminHelperExecutableCached},
 			{check: checkSupportedCPUArch},
 			{configKeySuffix: "check-ram"},
@@ -245,6 +251,7 @@ var checkListForDistros = []checkListForDistro{
 		systemdResolved: true,
 		checks: []Check{
 			{check: checkIfRunningAsNormalUser},
+			{check: checkRunningInsideWSL2},
 			{check: checkAdminHelperExecutableCached},
 			{check: checkSupportedCPUArch},
 			{configKeySuffix: "check-ram"},
@@ -278,6 +285,7 @@ var checkListForDistros = []checkListForDistro{
 		systemdResolved: false,
 		checks: []Check{
 			{check: checkIfRunningAsNormalUser},
+			{check: checkRunningInsideWSL2},
 			{check: checkAdminHelperExecutableCached},
 			{check: checkSupportedCPUArch},
 			{configKeySuffix: "check-ram"},
@@ -310,6 +318,7 @@ var checkListForDistros = []checkListForDistro{
 		systemdResolved: false,
 		checks: []Check{
 			{check: checkIfRunningAsNormalUser},
+			{check: checkRunningInsideWSL2},
 			{check: checkAdminHelperExecutableCached},
 			{check: checkSupportedCPUArch},
 			{configKeySuffix: "check-ram"},
@@ -336,6 +345,7 @@ var checkListForDistros = []checkListForDistro{
 		systemdResolved: true,
 		checks: []Check{
 			{check: checkIfRunningAsNormalUser},
+			{check: checkRunningInsideWSL2},
 			{check: checkAdminHelperExecutableCached},
 			{check: checkSupportedCPUArch},
 			{configKeySuffix: "check-ram"},
@@ -370,6 +380,7 @@ var checkListForDistros = []checkListForDistro{
 		systemdResolved: false,
 		checks: []Check{
 			{check: checkIfRunningAsNormalUser},
+			{check: checkRunningInsideWSL2},
 			{check: checkAdminHelperExecutableCached},
 			{check: checkSupportedCPUArch},
 			{configKeySuffix: "check-ram"},
@@ -403,6 +414,7 @@ var checkListForDistros = []checkListForDistro{
 		systemdResolved: false,
 		checks: []Check{
 			{check: checkIfRunningAsNormalUser},
+			{check: checkRunningInsideWSL2},
 			{check: checkAdminHelperExecutableCached},
 			{check: checkSupportedCPUArch},
 			{configKeySuffix: "check-ram"},