Repository: keikoproj/upgrade-manager
Pull request #130 (2020-11-04 22:31:08)

Readiness gates implementation for eager mode
Fix for #126.
This is work in progress

uthark: @eytan-avisror what is the best place to check readiness gates for lazy mode? 
right now `isNodeReady` check doesn't work in lazy mode too.
codecov[bot]: # [Codecov](https://codecov.io/gh/keikoproj/upgrade-manager/pull/130?src=pr&el=h1) Report
> Merging [#130](https://codecov.io/gh/keikoproj/upgrade-manager/pull/130?src=pr&el=desc) into [master](https://codecov.io/gh/keikoproj/upgrade-manager/commit/308f9206dc6fded740f4e3475315c8496d3f8399?el=desc) will **decrease** coverage by `0.85%`.
> The diff coverage is `23.80%`.

[![Impacted file tree graph](https://codecov.io/gh/keikoproj/upgrade-manager/pull/130/graphs/tree.svg?width=650&height=150&src=pr&token=PbUYkNX6wz)](https://codecov.io/gh/keikoproj/upgrade-manager/pull/130?src=pr&el=tree)

```diff
@@            Coverage Diff             @@
##           master     #130      +/-   ##
==========================================
- Coverage   66.77%   65.92%   -0.86%     
==========================================
  Files          10       10              
  Lines         885      904      +19     
==========================================
+ Hits          591      596       +5     
- Misses        260      274      +14     
  Partials       34       34              
```

| Flag | Coverage Δ | |
|---|---|---|
| unittests | `65.92% <23.80%> (-0.86%)` | :arrow_down: |

Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags#carryforward-flags-in-the-pull-request-comment) to find out more.

| [Impacted Files](https://codecov.io/gh/keikoproj/upgrade-manager/pull/130?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [api/v1alpha1/rollingupgrade\_types.go](https://codecov.io/gh/keikoproj/upgrade-manager/pull/130/diff?src=pr&el=tree#diff-YXBpL3YxYWxwaGExL3JvbGxpbmd1cGdyYWRlX3R5cGVzLmdv) | `50.00% <ø> (ø)` | |
| [api/v1alpha1/zz\_generated.deepcopy.go](https://codecov.io/gh/keikoproj/upgrade-manager/pull/130/diff?src=pr&el=tree#diff-YXBpL3YxYWxwaGExL3p6X2dlbmVyYXRlZC5kZWVwY29weS5nbw==) | `0.00% <0.00%> (ø)` | |
| [controllers/rollingupgrade\_controller.go](https://codecov.io/gh/keikoproj/upgrade-manager/pull/130/diff?src=pr&el=tree#diff-Y29udHJvbGxlcnMvcm9sbGluZ3VwZ3JhZGVfY29udHJvbGxlci5nbw==) | `66.83% <0.00%> (ø)` | |
| [controllers/helpers.go](https://codecov.io/gh/keikoproj/upgrade-manager/pull/130/diff?src=pr&el=tree#diff-Y29udHJvbGxlcnMvaGVscGVycy5nbw==) | `90.66% <100.00%> (+0.66%)` | :arrow_up: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/keikoproj/upgrade-manager/pull/130?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/keikoproj/upgrade-manager/pull/130?src=pr&el=footer). Last update [308f920...a486d9b](https://codecov.io/gh/keikoproj/upgrade-manager/pull/130?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).

uthark: Tested eager mode and confirmed feature works as expected.

Patch:
diff --git a/api/v1alpha1/rollingupgrade_types.go b/api/v1alpha1/rollingupgrade_types.go
index 7b5b27b..8526be5 100644
--- a/api/v1alpha1/rollingupgrade_types.go
+++ b/api/v1alpha1/rollingupgrade_types.go
@@ -52,6 +52,12 @@ type RollingUpgradeSpec struct {
 	IgnoreDrainFailures bool `json:"ignoreDrainFailures,omitempty"`
 	// ForceRefresh enables draining and terminating the node even if the launch config/template hasn't changed.
 	ForceRefresh bool `json:"forceRefresh,omitempty"`
+	// ReadinessGates allow to specify label selectors that node must match to be considered ready.
+	ReadinessGates []NodeReadinessGate `json:"readinessGates,omitempty"`
+}
+
+type NodeReadinessGate struct {
+	MatchLabels map[string]string `json:"matchLabels,omitempty" protobuf:"bytes,1,rep,name=matchLabels"`
 }
 
 // RollingUpgradeStatus defines the observed state of RollingUpgrade
diff --git a/api/v1alpha1/zz_generated.deepcopy.go b/api/v1alpha1/zz_generated.deepcopy.go
index 72ab391..3f4d426 100644
--- a/api/v1alpha1/zz_generated.deepcopy.go
+++ b/api/v1alpha1/zz_generated.deepcopy.go
@@ -23,6 +23,28 @@ import (
 	runtime "k8s.io/apimachinery/pkg/runtime"
 )
 
+// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil.
+func (in *NodeReadinessGate) DeepCopyInto(out *NodeReadinessGate) {
+	*out = *in
+	if in.MatchLabels != nil {
+		in, out := &in.MatchLabels, &out.MatchLabels
+		*out = make(map[string]string, len(*in))
+		for key, val := range *in {
+			(*out)[key] = val
+		}
+	}
+}
+
+// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new NodeReadinessGate.
+func (in *NodeReadinessGate) DeepCopy() *NodeReadinessGate {
+	if in == nil {
+		return nil
+	}
+	out := new(NodeReadinessGate)
+	in.DeepCopyInto(out)
+	return out
+}
+
 // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil.
 func (in *PostDrainSpec) DeepCopyInto(out *PostDrainSpec) {
 	*out = *in
@@ -73,7 +95,7 @@ func (in *RollingUpgrade) DeepCopyInto(out *RollingUpgrade) {
 	*out = *in
 	out.TypeMeta = in.TypeMeta
 	in.ObjectMeta.DeepCopyInto(&out.ObjectMeta)
-	out.Spec = in.Spec
+	in.Spec.DeepCopyInto(&out.Spec)
 	in.Status.DeepCopyInto(&out.Status)
 }
 
@@ -149,6 +171,13 @@ func (in *RollingUpgradeSpec) DeepCopyInto(out *RollingUpgradeSpec) {
 	out.PostDrain = in.PostDrain
 	out.PostTerminate = in.PostTerminate
 	out.Strategy = in.Strategy
+	if in.ReadinessGates != nil {
+		in, out := &in.ReadinessGates, &out.ReadinessGates
+		*out = make([]NodeReadinessGate, len(*in))
+		for i := range *in {
+			(*in)[i].DeepCopyInto(&(*out)[i])
+		}
+	}
 }
 
 // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new RollingUpgradeSpec.
diff --git a/config/crd/bases/upgrademgr.keikoproj.io_rollingupgrades.yaml b/config/crd/bases/upgrademgr.keikoproj.io_rollingupgrades.yaml
index bffc937..4833907 100644
--- a/config/crd/bases/upgrademgr.keikoproj.io_rollingupgrades.yaml
+++ b/config/crd/bases/upgrademgr.keikoproj.io_rollingupgrades.yaml
@@ -92,6 +92,17 @@ spec:
                 script:
                   type: string
               type: object
+            readinessGates:
+              description: ReadinessGates allow to specify label selectors that node
+                must match to be considered ready.
+              items:
+                properties:
+                  matchLabels:
+                    additionalProperties:
+                      type: string
+                    type: object
+                type: object
+              type: array
             strategy:
               description: UpdateStrategy holds the information needed to perform
                 update based on different update strategies
diff --git a/controllers/helpers.go b/controllers/helpers.go
index 3b8a09b..e3ead12 100644
--- a/controllers/helpers.go
+++ b/controllers/helpers.go
@@ -50,6 +50,21 @@ func isNodeReady(node corev1.Node) bool {
 	return false
 }
 
+func IsNodePassesReadinessGates(node corev1.Node, requiredReadinessGates []upgrademgrv1alpha1.NodeReadinessGate) bool {
+
+	if len(requiredReadinessGates) == 0 {
+		return true
+	}
+	for _, gate := range requiredReadinessGates {
+		for key, value := range gate.MatchLabels {
+			if node.Labels[key] != value {
+				return false
+			}
+		}
+	}
+	return true
+}
+
 func getInServiceCount(instances []*autoscaling.Instance) int64 {
 	var count int64
 	for _, instance := range instances {
diff --git a/controllers/rollingupgrade_controller.go b/controllers/rollingupgrade_controller.go
index 689a31e..f994823 100644
--- a/controllers/rollingupgrade_controller.go
+++ b/controllers/rollingupgrade_controller.go
@@ -308,7 +308,7 @@ func (r *RollingUpgradeReconciler) WaitForDesiredNodes(ruObj *upgrademgrv1alpha1
 		for _, node := range r.NodeList.Items {
 			tokens := strings.Split(node.Spec.ProviderID, "/")
 			instanceID := tokens[len(tokens)-1]
-			if contains(inServiceInstances, instanceID) && isNodeReady(node) {
+			if contains(inServiceInstances, instanceID) && isNodeReady(node) && IsNodePassesReadinessGates(node, ruObj.Spec.ReadinessGates) {
 				foundCount++
 			}
 		}


Test patch:
diff --git a/controllers/helpers_test.go b/controllers/helpers_test.go
index a8f828b..bda85f8 100644
--- a/controllers/helpers_test.go
+++ b/controllers/helpers_test.go
@@ -3,6 +3,7 @@ package controllers
 import (
 	"encoding/json"
 	"fmt"
+	v1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 
 	"github.com/aws/aws-sdk-go/aws"
 
@@ -45,6 +46,105 @@ func TestIsNodeReady(t *testing.T) {
 	}
 }
 
+func TestIsNodePassesReadinessGates(t *testing.T) {
+	g := gomega.NewGomegaWithT(t)
+
+	type test struct {
+		gate   []map[string]string
+		labels map[string]string
+		want   bool
+	}
+	tests := []test{
+		{
+			gate: []map[string]string{
+				{
+					"healthy": "true",
+				},
+			},
+			labels: map[string]string{
+				"healthy": "true",
+			},
+			want: true,
+		},
+
+		{
+			gate: []map[string]string{},
+			labels: map[string]string{
+				"healthy": "true",
+			},
+			want: true,
+		},
+
+		{
+			gate: []map[string]string{
+				{"healthy": "true"},
+			},
+			labels: map[string]string{
+				"healthy": "false",
+			},
+			want: false,
+		},
+
+		{
+			gate: []map[string]string{
+				{"healthy": "true"},
+			},
+			labels: map[string]string{
+
+			},
+			want: false,
+		},
+
+		{
+			gate: []map[string]string{
+				{"healthy": "true"},
+				{"second-check": "true"},
+			},
+			labels: map[string]string{
+				"healthy": "true",
+			},
+			want: false,
+		},
+		{
+			gate: []map[string]string{
+				{"healthy": "true"},
+				{"second-check": "true"},
+			},
+			labels: map[string]string{
+				"healthy": "true",
+				"second-check": "true",
+			},
+			want: true,
+		},
+		{
+			gate: []map[string]string{
+				{"healthy": "true"},
+				{"second-check": "true"},
+			},
+			labels: map[string]string{
+				"healthy": "true",
+				"second-check": "false",
+			},
+			want: false,
+		},	}
+
+	for _, tt := range tests {
+		readinessGates := make([]upgrademgrv1alpha1.NodeReadinessGate, len(tt.gate))
+		for i, g := range tt.gate {
+			readinessGates[i] = upgrademgrv1alpha1.NodeReadinessGate{
+				MatchLabels: g,
+			}
+		}
+		node := corev1.Node{
+			ObjectMeta: v1.ObjectMeta{
+				Labels: tt.labels,
+			},
+		}
+		g.Expect(IsNodePassesReadinessGates(node, readinessGates)).To(gomega.Equal(tt.want))
+	}
+
+}
+
 func TestGetInServiceCount(t *testing.T) {
 	g := gomega.NewGomegaWithT(t)
 	tt := map[*autoscaling.Instance]int64{