Repository: Azure/ARO-RP
Pull request #1974 (2022-02-19 07:31:59)

Bump url-parse from 1.5.3 to 1.5.7 in /portal
Bumps [url-parse](https://github.com/unshiftio/url-parse) from 1.5.3 to 1.5.7.
<details>
<summary>Commits</summary>
<ul>
<li><a href="https://github.com/unshiftio/url-parse/commit/8b3f5f2c88a4cfc2880f2319c307994cb25bb10a"><code>8b3f5f2</code></a> 1.5.7</li>
<li><a href="https://github.com/unshiftio/url-parse/commit/ef45a1355375a8244063793a19059b4f62fc8788"><code>ef45a13</code></a> [fix] Readd the empty userinfo to <code>url.href</code> (<a href="https://github-redirect.dependabot.com/unshiftio/url-parse/issues/226">#226</a>)</li>
<li><a href="https://github.com/unshiftio/url-parse/commit/88df2346855f70cec9713b362ca32a4691dc271a"><code>88df234</code></a> [doc] Add soft deprecation notice</li>
<li><a href="https://github.com/unshiftio/url-parse/commit/78e9f2f41285d83e7d91706be5bd439656fe3bc3"><code>78e9f2f</code></a> [security] Fix nits</li>
<li><a href="https://github.com/unshiftio/url-parse/commit/e6fa43422c52f34c73146552ec9916125dc59525"><code>e6fa434</code></a> [security] Add credits for incorrect handling of userinfo vulnerability</li>
<li><a href="https://github.com/unshiftio/url-parse/commit/4c9fa234c01dca52698666378360ad2fdfb05470"><code>4c9fa23</code></a> 1.5.6</li>
<li><a href="https://github.com/unshiftio/url-parse/commit/7b0b8a6671f806458e88b1f44feb0fdd742cdf06"><code>7b0b8a6</code></a> Merge pull request <a href="https://github-redirect.dependabot.com/unshiftio/url-parse/issues/223">#223</a> from unshiftio/fix/at-sign-handling-in-userinfo</li>
<li><a href="https://github.com/unshiftio/url-parse/commit/e4a5807d95b971577e4d888f5b99d64a40851386"><code>e4a5807</code></a> 1.5.5</li>
<li><a href="https://github.com/unshiftio/url-parse/commit/193b44baf3d203560735e05eedc99d8244c9e16c"><code>193b44b</code></a> [minor] Simplify whitespace regex</li>
<li><a href="https://github.com/unshiftio/url-parse/commit/319851bf1c294796fc73e29ff31b14d9084e4a0d"><code>319851b</code></a> [fix] Remove CR, HT, and LF</li>
<li>Additional commits viewable in <a href="https://github.com/unshiftio/url-parse/compare/1.5.3...1.5.7">compare view</a></li>
</ul>
</details>
<br />


[![Dependabot compatibility score](https://dependabot-badges.githubapp.com/badges/compatibility_score?dependency-name=url-parse&package-manager=npm_and_yarn&previous-version=1.5.3&new-version=1.5.7)](https://docs.github.com/en/github/managing-security-vulnerabilities/about-dependabot-security-updates#about-compatibility-scores)

Dependabot will resolve any conflicts with this PR as long as you don't alter it yourself. You can also trigger a rebase manually by commenting `@dependabot rebase`.

[//]: # (dependabot-automerge-start)
[//]: # (dependabot-automerge-end)

---

<details>
<summary>Dependabot commands and options</summary>
<br />

You can trigger Dependabot actions by commenting on this PR:
- `@dependabot rebase` will rebase this PR
- `@dependabot recreate` will recreate this PR, overwriting any edits that have been made to it
- `@dependabot merge` will merge this PR after your CI passes on it
- `@dependabot squash and merge` will squash and merge this PR after your CI passes on it
- `@dependabot cancel merge` will cancel a previously requested merge and block automerging
- `@dependabot reopen` will reopen this PR if it is closed
- `@dependabot close` will close this PR and stop Dependabot recreating it. You can achieve the same result by closing it manually
- `@dependabot ignore this major version` will close this PR and stop Dependabot creating any more for this major version (unless you reopen the PR or upgrade to it yourself)
- `@dependabot ignore this minor version` will close this PR and stop Dependabot creating any more for this minor version (unless you reopen the PR or upgrade to it yourself)
- `@dependabot ignore this dependency` will close this PR and stop Dependabot creating any more for this dependency (unless you reopen the PR or upgrade to it yourself)
- `@dependabot use these labels` will set the current labels as the default for future PRs for this repo and language
- `@dependabot use these reviewers` will set the current reviewers as the default for future PRs for this repo and language
- `@dependabot use these assignees` will set the current assignees as the default for future PRs for this repo and language
- `@dependabot use this milestone` will set the current milestone as the default for future PRs for this repo and language

You can disable automated security fix PRs for this repo from the [Security Alerts page](https://github.com/Azure/ARO-RP/network/alerts).

</details>


Patch:
diff --git a/pkg/monitor/cluster/cluster.go b/pkg/monitor/cluster/cluster.go
index fce45a0c0..feadd1e14 100644
--- a/pkg/monitor/cluster/cluster.go
+++ b/pkg/monitor/cluster/cluster.go
@@ -12,6 +12,7 @@ import (
 	"github.com/Azure/go-autorest/autorest/azure"
 	configv1 "github.com/openshift/api/config/v1"
 	configclient "github.com/openshift/client-go/config/clientset/versioned"
+	maoclient "github.com/openshift/machine-api-operator/pkg/generated/clientset/versioned"
 	mcoclient "github.com/openshift/machine-config-operator/pkg/generated/clientset/versioned"
 	"github.com/sirupsen/logrus"
 	appsv1 "k8s.io/api/apps/v1"
@@ -34,6 +35,7 @@ type Monitor struct {
 	restconfig *rest.Config
 	cli        kubernetes.Interface
 	configcli  configclient.Interface
+	maocli     maoclient.Interface
 	mcocli     mcoclient.Interface
 	m          metrics.Interface
 	arocli     aroclient.Interface
@@ -70,6 +72,11 @@ func NewMonitor(ctx context.Context, log *logrus.Entry, restConfig *rest.Config,
 		return nil, err
 	}
 
+	maocli, err := maoclient.NewForConfig(restConfig)
+	if err != nil {
+		return nil, err
+	}
+
 	mcocli, err := mcoclient.NewForConfig(restConfig)
 	if err != nil {
 		return nil, err
@@ -90,6 +97,7 @@ func NewMonitor(ctx context.Context, log *logrus.Entry, restConfig *rest.Config,
 		restconfig: restConfig,
 		cli:        cli,
 		configcli:  configcli,
+		maocli:     maocli,
 		mcocli:     mcocli,
 		arocli:     arocli,
 		m:          m,
diff --git a/pkg/monitor/cluster/nodeconditions.go b/pkg/monitor/cluster/nodeconditions.go
index 1a11c7c52..89a639460 100644
--- a/pkg/monitor/cluster/nodeconditions.go
+++ b/pkg/monitor/cluster/nodeconditions.go
@@ -5,9 +5,13 @@ package cluster
 
 import (
 	"context"
+	"encoding/json"
+	"strconv"
 
 	"github.com/sirupsen/logrus"
 	corev1 "k8s.io/api/core/v1"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	azureproviderv1beta1 "sigs.k8s.io/cluster-api-provider-azure/pkg/apis/azureprovider/v1beta1"
 )
 
 var nodeConditionsExpected = map[corev1.NodeConditionType]corev1.ConditionStatus{
@@ -23,27 +27,34 @@ func (mon *Monitor) emitNodeConditions(ctx context.Context) error {
 		return err
 	}
 
+	spotInstances := mon.getSpotInstances(ctx)
+
 	mon.emitGauge("node.count", int64(len(ns.Items)), nil)
 
 	for _, n := range ns.Items {
+
 		for _, c := range n.Status.Conditions {
 			if c.Status == nodeConditionsExpected[c.Type] {
 				continue
 			}
 
+			_, isSpotInstance := spotInstances[n.Name]
+
 			mon.emitGauge("node.conditions", 1, map[string]string{
-				"nodeName": n.Name,
-				"status":   string(c.Status),
-				"type":     string(c.Type),
+				"nodeName":     n.Name,
+				"status":       string(c.Status),
+				"type":         string(c.Type),
+				"spotInstance": strconv.FormatBool(isSpotInstance),
 			})
 
 			if mon.hourlyRun {
 				mon.log.WithFields(logrus.Fields{
-					"metric":  "node.conditions",
-					"name":    n.Name,
-					"status":  c.Status,
-					"type":    c.Type,
-					"message": c.Message,
+					"metric":       "node.conditions",
+					"name":         n.Name,
+					"status":       c.Status,
+					"type":         c.Type,
+					"message":      c.Message,
+					"spotInstance": isSpotInstance,
 				}).Print()
 			}
 		}
@@ -57,3 +68,30 @@ func (mon *Monitor) emitNodeConditions(ctx context.Context) error {
 
 	return nil
 }
+
+// getSpotInstances returns a map where the keys are the machine name and only exist if the machine is a spot instance
+func (mon *Monitor) getSpotInstances(ctx context.Context) map[string]struct{} {
+	spotInstances := make(map[string]struct{})
+	machines, err := mon.maocli.MachineV1beta1().Machines("openshift-machine-api").List(ctx, metav1.ListOptions{})
+
+	if err != nil {
+		// when this call fails we may report spot vms as non spot until the next successful call
+		mon.log.Error(err)
+		return spotInstances
+	}
+
+	for _, machine := range machines.Items {
+		var spec azureproviderv1beta1.AzureMachineProviderSpec
+		err = json.Unmarshal(machine.Spec.ProviderSpec.Value.Raw, &spec)
+		if err != nil {
+			mon.log.Error(err)
+			continue
+		}
+
+		if spec.SpotVMOptions != nil {
+			spotInstances[machine.Name] = struct{}{}
+		}
+	}
+
+	return spotInstances
+}
diff --git a/pkg/operator/controllers/checker/checker_controller.go b/pkg/operator/controllers/checker/checker_controller.go
index 965370423..579cad709 100644
--- a/pkg/operator/controllers/checker/checker_controller.go
+++ b/pkg/operator/controllers/checker/checker_controller.go
@@ -83,7 +83,11 @@ func (r *Reconciler) Reconcile(ctx context.Context, request ctrl.Request) (ctrl.
 		}
 	}
 
-	return reconcile.Result{RequeueAfter: time.Hour, Requeue: true}, err
+	// We always requeue here:
+	// * Either immediately (with rate limiting) based on the error
+	//   when err != nil.
+	// * Or based on RequeueAfter when err == nil.
+	return reconcile.Result{RequeueAfter: time.Hour}, err
 }
 
 // SetupWithManager setup our manager
diff --git a/pkg/operator/controllers/node/node_controller.go b/pkg/operator/controllers/node/node_controller.go
index 879a164fc..2b53dfc83 100644
--- a/pkg/operator/controllers/node/node_controller.go
+++ b/pkg/operator/controllers/node/node_controller.go
@@ -101,9 +101,7 @@ func (r *Reconciler) Reconcile(ctx context.Context, request ctrl.Request) (ctrl.
 	deadline := t.Add(gracePeriod)
 	now := time.Now()
 	if deadline.After(now) {
-		return reconcile.Result{
-			RequeueAfter: deadline.Sub(now),
-		}, err
+		return reconcile.Result{RequeueAfter: deadline.Sub(now)}, nil
 	}
 
 	// drain the node disabling eviction
diff --git a/pkg/operator/controllers/workaround/workaround_controller.go b/pkg/operator/controllers/workaround/workaround_controller.go
index cd8fef05e..a99c63614 100644
--- a/pkg/operator/controllers/workaround/workaround_controller.go
+++ b/pkg/operator/controllers/workaround/workaround_controller.go
@@ -87,7 +87,7 @@ func (r *Reconciler) Reconcile(ctx context.Context, request ctrl.Request) (ctrl.
 			return reconcile.Result{}, err
 		}
 	}
-	return reconcile.Result{RequeueAfter: time.Hour, Requeue: true}, nil
+	return reconcile.Result{RequeueAfter: time.Hour}, nil
 }
 
 // SetupWithManager setup our manager
diff --git a/portal/package-lock.json b/portal/package-lock.json
index c1ece3eca..caa7fbe7a 100644
--- a/portal/package-lock.json
+++ b/portal/package-lock.json
@@ -5700,9 +5700,9 @@
       }
     },
     "url-parse": {
-      "version": "1.5.3",
-      "resolved": "https://registry.npmjs.org/url-parse/-/url-parse-1.5.3.tgz",
-      "integrity": "sha512-IIORyIQD9rvj0A4CLWsHkBBJuNqWpFQe224b6j9t/ABmquIS0qDU2pY6kl6AuOrL5OkCXHMCFNe1jBcuAggjvQ==",
+      "version": "1.5.7",
+      "resolved": "https://registry.npmjs.org/url-parse/-/url-parse-1.5.7.tgz",
+      "integrity": "sha512-HxWkieX+STA38EDk7CE9MEryFeHCKzgagxlGvsdS7WBImq9Mk+PGwiT56w82WI3aicwJA8REp42Cxo98c8FZMA==",
       "dev": true,
       "requires": {
         "querystringify": "^2.1.1",


Test patch:
diff --git a/pkg/monitor/cluster/nodeconditions_test.go b/pkg/monitor/cluster/nodeconditions_test.go
index eb8c6c4d0..6abb2057d 100644
--- a/pkg/monitor/cluster/nodeconditions_test.go
+++ b/pkg/monitor/cluster/nodeconditions_test.go
@@ -5,12 +5,19 @@ package cluster
 
 import (
 	"context"
+	"encoding/json"
 	"testing"
 
 	"github.com/golang/mock/gomock"
+	machinev1beta1 "github.com/openshift/machine-api-operator/pkg/apis/machine/v1beta1"
+	maoclient "github.com/openshift/machine-api-operator/pkg/generated/clientset/versioned"
+	maofake "github.com/openshift/machine-api-operator/pkg/generated/clientset/versioned/fake"
+	"github.com/sirupsen/logrus"
 	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	kruntime "k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/client-go/kubernetes/fake"
+	azureproviderv1beta1 "sigs.k8s.io/cluster-api-provider-azure/pkg/apis/azureprovider/v1beta1"
 
 	mock_metrics "github.com/Azure/ARO-RP/pkg/util/mocks/metrics"
 )
@@ -18,9 +25,19 @@ import (
 func TestEmitNodeConditions(t *testing.T) {
 	ctx := context.Background()
 
+	provSpec, err := json.Marshal(azureproviderv1beta1.AzureMachineProviderSpec{})
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	kubeletVersion := "v1.17.1+9d33dd3"
+
 	cli := fake.NewSimpleClientset(&corev1.Node{
 		ObjectMeta: metav1.ObjectMeta{
 			Name: "aro-master-0",
+			Annotations: map[string]string{
+				"machine.openshift.io/machine": "openshift-machine-api/master-0",
+			},
 		},
 		Status: corev1.NodeStatus{
 			Conditions: []corev1.NodeCondition{
@@ -30,12 +47,15 @@ func TestEmitNodeConditions(t *testing.T) {
 				},
 			},
 			NodeInfo: corev1.NodeSystemInfo{
-				KubeletVersion: "v1.17.1+9d33dd3",
+				KubeletVersion: kubeletVersion,
 			},
 		},
 	}, &corev1.Node{
 		ObjectMeta: metav1.ObjectMeta{
 			Name: "aro-master-1",
+			Annotations: map[string]string{
+				"machine.openshift.io/machine": "openshift-machine-api/master-1",
+			},
 		},
 		Status: corev1.NodeStatus{
 			Conditions: []corev1.NodeCondition{
@@ -45,10 +65,38 @@ func TestEmitNodeConditions(t *testing.T) {
 				},
 			},
 			NodeInfo: corev1.NodeSystemInfo{
-				KubeletVersion: "v1.17.1+9d33dd3",
+				KubeletVersion: kubeletVersion,
 			},
 		},
 	})
+	maoclient := maofake.NewSimpleClientset(
+		&machinev1beta1.Machine{
+			Spec: machinev1beta1.MachineSpec{
+				ProviderSpec: machinev1beta1.ProviderSpec{
+					Value: &kruntime.RawExtension{
+						Raw: provSpec,
+					},
+				},
+			},
+			ObjectMeta: metav1.ObjectMeta{
+				Name:      "openshift-machine-api/master-0",
+				Namespace: "openshift-machine-api",
+			},
+		},
+		&machinev1beta1.Machine{
+			Spec: machinev1beta1.MachineSpec{
+				ProviderSpec: machinev1beta1.ProviderSpec{
+					Value: &kruntime.RawExtension{
+						Raw: provSpec,
+					},
+				},
+			},
+			ObjectMeta: metav1.ObjectMeta{
+				Name:      "openshift-machine-api/master-1",
+				Namespace: "openshift-machine-api",
+			},
+		},
+	)
 
 	controller := gomock.NewController(t)
 	defer controller.Finish()
@@ -56,33 +104,171 @@ func TestEmitNodeConditions(t *testing.T) {
 	m := mock_metrics.NewMockInterface(controller)
 
 	mon := &Monitor{
-		cli: cli,
-		m:   m,
+		cli:    cli,
+		maocli: maoclient,
+		m:      m,
 	}
 
 	m.EXPECT().EmitGauge("node.count", int64(2), map[string]string{})
 	m.EXPECT().EmitGauge("node.conditions", int64(1), map[string]string{
-		"nodeName": "aro-master-0",
-		"status":   "True",
-		"type":     "MemoryPressure",
+		"nodeName":     "aro-master-0",
+		"status":       "True",
+		"type":         "MemoryPressure",
+		"spotInstance": "false",
 	})
 	m.EXPECT().EmitGauge("node.conditions", int64(1), map[string]string{
-		"nodeName": "aro-master-1",
-		"status":   "False",
-		"type":     "Ready",
+		"nodeName":     "aro-master-1",
+		"status":       "False",
+		"type":         "Ready",
+		"spotInstance": "false",
 	})
 
 	m.EXPECT().EmitGauge("node.kubelet.version", int64(1), map[string]string{
 		"nodeName":       "aro-master-0",
-		"kubeletVersion": "v1.17.1+9d33dd3",
+		"kubeletVersion": kubeletVersion,
 	})
 	m.EXPECT().EmitGauge("node.kubelet.version", int64(1), map[string]string{
 		"nodeName":       "aro-master-1",
-		"kubeletVersion": "v1.17.1+9d33dd3",
+		"kubeletVersion": kubeletVersion,
 	})
 
-	err := mon.emitNodeConditions(ctx)
+	err = mon.emitNodeConditions(ctx)
 	if err != nil {
 		t.Fatal(err)
 	}
 }
+
+func TestGetSpotInstances(t *testing.T) {
+	ctx := context.Background()
+
+	spotProvSpec, err := json.Marshal(azureproviderv1beta1.AzureMachineProviderSpec{
+		SpotVMOptions: &azureproviderv1beta1.SpotVMOptions{},
+	})
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	provSpec, err := json.Marshal(azureproviderv1beta1.AzureMachineProviderSpec{})
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	for _, tt := range []struct {
+		name                 string
+		maocli               maoclient.Interface
+		node                 corev1.Node
+		expectedSpotInstance bool
+	}{
+		{
+			name: "node is a spot instance",
+			maocli: maofake.NewSimpleClientset(&machinev1beta1.Machine{
+				Spec: machinev1beta1.MachineSpec{
+					ProviderSpec: machinev1beta1.ProviderSpec{
+						Value: &kruntime.RawExtension{
+							Raw: spotProvSpec,
+						},
+					},
+				},
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      "aro-spot-0",
+					Namespace: "openshift-machine-api",
+				},
+			}),
+			node: corev1.Node{
+				ObjectMeta: metav1.ObjectMeta{
+					Name: "aro-spot-0",
+					Annotations: map[string]string{
+						"machine.openshift.io/machine": "openshift-machine-api/spot-0",
+					},
+				},
+			},
+			expectedSpotInstance: true,
+		},
+		{
+			name: "node is not a spot instance",
+			maocli: maofake.NewSimpleClientset(&machinev1beta1.Machine{
+				Spec: machinev1beta1.MachineSpec{
+					ProviderSpec: machinev1beta1.ProviderSpec{
+						Value: &kruntime.RawExtension{
+							Raw: provSpec,
+						},
+					},
+				},
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      "master-0",
+					Namespace: "openshift-machine-api",
+				},
+			}),
+			node: corev1.Node{
+				ObjectMeta: metav1.ObjectMeta{
+					Name: "aro-master-0",
+					Annotations: map[string]string{
+						"machine.openshift.io/machine": "openshift-machine-api/master-0",
+					},
+				},
+			},
+			expectedSpotInstance: false,
+		},
+		{
+			name: "node is missing annotation",
+			maocli: maofake.NewSimpleClientset(&machinev1beta1.Machine{
+				Spec: machinev1beta1.MachineSpec{
+					ProviderSpec: machinev1beta1.ProviderSpec{
+						Value: &kruntime.RawExtension{
+							Raw: provSpec,
+						},
+					},
+				},
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      "master-0",
+					Namespace: "openshift-machine-api",
+				},
+			}),
+			node: corev1.Node{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:        "aro-master-0",
+					Annotations: map[string]string{},
+				},
+			},
+			expectedSpotInstance: false,
+		},
+		{
+			name: "malformed json in providerSpec",
+			maocli: maofake.NewSimpleClientset(&machinev1beta1.Machine{
+				Spec: machinev1beta1.MachineSpec{
+					ProviderSpec: machinev1beta1.ProviderSpec{
+						Value: &kruntime.RawExtension{
+							Raw: []byte(";df9j"),
+						},
+					},
+				},
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      "aro-spot-1",
+					Namespace: "openshift-machine-api",
+				},
+			}),
+			node: corev1.Node{
+				ObjectMeta: metav1.ObjectMeta{
+					Name: "aro-spot-1",
+					Annotations: map[string]string{
+						"machine.openshift.io/machine": "openshift-machine-api/spot-0",
+					},
+				},
+			},
+			expectedSpotInstance: false,
+		},
+	} {
+		controller := gomock.NewController(t)
+		defer controller.Finish()
+
+		mon := &Monitor{
+			maocli: tt.maocli,
+			log:    logrus.NewEntry(logrus.StandardLogger()),
+		}
+
+		_, isSpotInstance := mon.getSpotInstances(ctx)[tt.node.Name]
+		if isSpotInstance != tt.expectedSpotInstance {
+			t.Fatalf("test %s: isSpotInstance should be %t but got %t", tt.name, tt.expectedSpotInstance, isSpotInstance)
+		}
+	}
+}
diff --git a/pkg/operator/controllers/workaround/workaround_controller_test.go b/pkg/operator/controllers/workaround/workaround_controller_test.go
index bd5c5181b..9bd6e774f 100644
--- a/pkg/operator/controllers/workaround/workaround_controller_test.go
+++ b/pkg/operator/controllers/workaround/workaround_controller_test.go
@@ -68,7 +68,7 @@ func TestWorkaroundReconciler(t *testing.T) {
 				c := mw.EXPECT().IsRequired(gomock.Any()).Return(true)
 				mw.EXPECT().Ensure(gomock.Any()).After(c).Return(nil)
 			},
-			want: ctrl.Result{Requeue: true, RequeueAfter: time.Hour},
+			want: ctrl.Result{RequeueAfter: time.Hour},
 		},
 		{
 			name: "is not required",
@@ -76,7 +76,7 @@ func TestWorkaroundReconciler(t *testing.T) {
 				c := mw.EXPECT().IsRequired(gomock.Any()).Return(false)
 				mw.EXPECT().Remove(gomock.Any()).After(c).Return(nil)
 			},
-			want: ctrl.Result{Requeue: true, RequeueAfter: time.Hour},
+			want: ctrl.Result{RequeueAfter: time.Hour},
 		},
 		{
 			name: "has error",