Skip to content

Commit cfb5e27

Browse files
k8s-ci-robotk8s-ci-robot
authored
Merge pull request #9837 from Dhairya-Arora01/minReadySeconds
🌱 MinReadySeconds for machinepools
2 parents 6941e8c + 2e9b3b1 commit cfb5e27

File tree

5 files changed

+121
-23
lines changed

5 files changed

+121
-23
lines changed

‎config/crd/bases/cluster.x-k8s.io_machinepools.yaml

Lines changed: 0 additions & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎exp/api/v1beta1/machinepool_types.go

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,6 @@ type MachinePoolSpec struct {
4949
// be ready.
5050
// Defaults to 0 (machine instance will be considered available as soon as it
5151
// is ready)
52-
// NOTE: No logic is implemented for this field and it currently has no behaviour.
5352
// +optional
5453
MinReadySeconds *int32 `json:"minReadySeconds,omitempty"`
5554

‎exp/internal/controllers/machinepool_controller_noderef.go

Lines changed: 9 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -23,11 +23,13 @@ import (
2323

2424
"github.com/pkg/errors"
2525
corev1 "k8s.io/api/core/v1"
26+
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
2627
"k8s.io/klog/v2"
2728
ctrl "sigs.k8s.io/controller-runtime"
2829
"sigs.k8s.io/controller-runtime/pkg/client"
2930

3031
clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1"
32+
"sigs.k8s.io/cluster-api/controllers/noderefutil"
3133
expv1 "sigs.k8s.io/cluster-api/exp/api/v1beta1"
3234
"sigs.k8s.io/cluster-api/internal/util/taints"
3335
"sigs.k8s.io/cluster-api/util"
@@ -36,9 +38,7 @@ import (
3638
"sigs.k8s.io/cluster-api/util/patch"
3739
)
3840

39-
var (
40-
errNoAvailableNodes = errors.New("cannot find nodes with matching ProviderIDs in ProviderIDList")
41-
)
41+
var errNoAvailableNodes = errors.New("cannot find nodes with matching ProviderIDs in ProviderIDList")
4242

4343
type getNodeReferencesResult struct {
4444
references []corev1.ObjectReference
@@ -82,7 +82,7 @@ func (r *MachinePoolReconciler) reconcileNodeRefs(ctx context.Context, cluster *
8282
}
8383

8484
// Get the Node references.
85-
nodeRefsResult, err := r.getNodeReferences(ctx, clusterClient, mp.Spec.ProviderIDList)
85+
nodeRefsResult, err := r.getNodeReferences(ctx, clusterClient, mp.Spec.ProviderIDList, mp.Spec.MinReadySeconds)
8686
if err != nil {
8787
if err == errNoAvailableNodes {
8888
log.Info("Cannot assign NodeRefs to MachinePool, no matching Nodes")
@@ -153,7 +153,7 @@ func (r *MachinePoolReconciler) deleteRetiredNodes(ctx context.Context, c client
153153
return nil
154154
}
155155

156-
func (r *MachinePoolReconciler) getNodeReferences(ctx context.Context, c client.Client, providerIDList []string) (getNodeReferencesResult, error) {
156+
func (r *MachinePoolReconciler) getNodeReferences(ctx context.Context, c client.Client, providerIDList []string, minReadySeconds *int32) (getNodeReferencesResult, error) {
157157
log := ctrl.LoggerFrom(ctx, "providerIDList", len(providerIDList))
158158

159159
var ready, available int
@@ -185,9 +185,11 @@ func (r *MachinePoolReconciler) getNodeReferences(ctx context.Context, c client.
185185
continue
186186
}
187187
if node, ok := nodeRefsMap[providerID]; ok {
188-
available++
189-
if nodeIsReady(&node) {
188+
if noderefutil.IsNodeReady(&node) {
190189
ready++
190+
if noderefutil.IsNodeAvailable(&node, *minReadySeconds, metav1.Now()) {
191+
available++
192+
}
191193
}
192194
nodeRefs = append(nodeRefs, corev1.ObjectReference{
193195
APIVersion: corev1.SchemeGroupVersion.String(),
@@ -236,12 +238,3 @@ func (r *MachinePoolReconciler) patchNodes(ctx context.Context, c client.Client,
236238
}
237239
return nil
238240
}
239-
240-
func nodeIsReady(node *corev1.Node) bool {
241-
for _, n := range node.Status.Conditions {
242-
if n.Type == corev1.NodeReady {
243-
return n.Status == corev1.ConditionTrue
244-
}
245-
}
246-
return false
247-
}

‎exp/internal/controllers/machinepool_controller_noderef_test.go

Lines changed: 111 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ import (
2323
corev1 "k8s.io/api/core/v1"
2424
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
2525
"k8s.io/client-go/tools/record"
26+
"k8s.io/utils/ptr"
2627
"sigs.k8s.io/controller-runtime/pkg/client"
2728
"sigs.k8s.io/controller-runtime/pkg/client/fake"
2829

@@ -44,6 +45,14 @@ func TestMachinePoolGetNodeReference(t *testing.T) {
4445
Spec: corev1.NodeSpec{
4546
ProviderID: "aws://us-east-1/id-node-1",
4647
},
48+
Status: corev1.NodeStatus{
49+
Conditions: []corev1.NodeCondition{
50+
{
51+
Type: corev1.NodeReady,
52+
Status: corev1.ConditionTrue,
53+
},
54+
},
55+
},
4756
},
4857
&corev1.Node{
4958
ObjectMeta: metav1.ObjectMeta{
@@ -52,6 +61,22 @@ func TestMachinePoolGetNodeReference(t *testing.T) {
5261
Spec: corev1.NodeSpec{
5362
ProviderID: "aws://us-west-2/id-node-2",
5463
},
64+
Status: corev1.NodeStatus{
65+
Conditions: []corev1.NodeCondition{
66+
{
67+
Type: corev1.NodeReady,
68+
Status: corev1.ConditionTrue,
69+
},
70+
},
71+
},
72+
},
73+
&corev1.Node{
74+
ObjectMeta: metav1.ObjectMeta{
75+
Name: "node-3",
76+
},
77+
Spec: corev1.NodeSpec{
78+
ProviderID: "aws://us-west-2/id-node-3",
79+
},
5580
},
5681
&corev1.Node{
5782
ObjectMeta: metav1.ObjectMeta{
@@ -60,6 +85,14 @@ func TestMachinePoolGetNodeReference(t *testing.T) {
6085
Spec: corev1.NodeSpec{
6186
ProviderID: "gce://us-central1/gce-id-node-2",
6287
},
88+
Status: corev1.NodeStatus{
89+
Conditions: []corev1.NodeCondition{
90+
{
91+
Type: corev1.NodeReady,
92+
Status: corev1.ConditionTrue,
93+
},
94+
},
95+
},
6396
},
6497
&corev1.Node{
6598
ObjectMeta: metav1.ObjectMeta{
@@ -68,6 +101,14 @@ func TestMachinePoolGetNodeReference(t *testing.T) {
68101
Spec: corev1.NodeSpec{
69102
ProviderID: "azure://westus2/id-node-4",
70103
},
104+
Status: corev1.NodeStatus{
105+
Conditions: []corev1.NodeCondition{
106+
{
107+
Type: corev1.NodeReady,
108+
Status: corev1.ConditionTrue,
109+
},
110+
},
111+
},
71112
},
72113
&corev1.Node{
73114
ObjectMeta: metav1.ObjectMeta{
@@ -76,6 +117,14 @@ func TestMachinePoolGetNodeReference(t *testing.T) {
76117
Spec: corev1.NodeSpec{
77118
ProviderID: "azure://westus2/id-nodepool1/0",
78119
},
120+
Status: corev1.NodeStatus{
121+
Conditions: []corev1.NodeCondition{
122+
{
123+
Type: corev1.NodeReady,
124+
Status: corev1.ConditionTrue,
125+
},
126+
},
127+
},
79128
},
80129
&corev1.Node{
81130
ObjectMeta: metav1.ObjectMeta{
@@ -84,16 +133,25 @@ func TestMachinePoolGetNodeReference(t *testing.T) {
84133
Spec: corev1.NodeSpec{
85134
ProviderID: "azure://westus2/id-nodepool2/0",
86135
},
136+
Status: corev1.NodeStatus{
137+
Conditions: []corev1.NodeCondition{
138+
{
139+
Type: corev1.NodeReady,
140+
Status: corev1.ConditionTrue,
141+
},
142+
},
143+
},
87144
},
88145
}
89146

90147
client := fake.NewClientBuilder().WithObjects(nodeList...).Build()
91148

92149
testCases := []struct {
93-
name string
94-
providerIDList []string
95-
expected *getNodeReferencesResult
96-
err error
150+
name string
151+
providerIDList []string
152+
expected *getNodeReferencesResult
153+
err error
154+
minReadySeconds int32
97155
}{
98156
{
99157
name: "valid provider id, valid aws node",
@@ -102,6 +160,8 @@ func TestMachinePoolGetNodeReference(t *testing.T) {
102160
references: []corev1.ObjectReference{
103161
{Name: "node-1"},
104162
},
163+
available: 1,
164+
ready: 1,
105165
},
106166
},
107167
{
@@ -111,6 +171,19 @@ func TestMachinePoolGetNodeReference(t *testing.T) {
111171
references: []corev1.ObjectReference{
112172
{Name: "node-2"},
113173
},
174+
available: 1,
175+
ready: 1,
176+
},
177+
},
178+
{
179+
name: "valid provider id, valid aws node, nodeReady condition set to false",
180+
providerIDList: []string{"aws://us-west-2/id-node-3"},
181+
expected: &getNodeReferencesResult{
182+
references: []corev1.ObjectReference{
183+
{Name: "node-3"},
184+
},
185+
available: 0,
186+
ready: 0,
114187
},
115188
},
116189
{
@@ -120,6 +193,8 @@ func TestMachinePoolGetNodeReference(t *testing.T) {
120193
references: []corev1.ObjectReference{
121194
{Name: "gce-node-2"},
122195
},
196+
available: 1,
197+
ready: 1,
123198
},
124199
},
125200
{
@@ -129,6 +204,8 @@ func TestMachinePoolGetNodeReference(t *testing.T) {
129204
references: []corev1.ObjectReference{
130205
{Name: "azure-node-4"},
131206
},
207+
available: 1,
208+
ready: 1,
132209
},
133210
},
134211
{
@@ -139,6 +216,8 @@ func TestMachinePoolGetNodeReference(t *testing.T) {
139216
{Name: "node-1"},
140217
{Name: "azure-node-4"},
141218
},
219+
available: 2,
220+
ready: 2,
142221
},
143222
},
144223
{
@@ -163,6 +242,8 @@ func TestMachinePoolGetNodeReference(t *testing.T) {
163242
references: []corev1.ObjectReference{
164243
{Name: "azure-nodepool1-0"},
165244
},
245+
available: 1,
246+
ready: 1,
166247
},
167248
},
168249
{
@@ -173,15 +254,37 @@ func TestMachinePoolGetNodeReference(t *testing.T) {
173254
{Name: "azure-nodepool1-0"},
174255
{Name: "azure-nodepool2-0"},
175256
},
257+
available: 2,
258+
ready: 2,
176259
},
177260
},
261+
{
262+
name: "valid provider id, valid aws node, with minReadySeconds",
263+
providerIDList: []string{"aws://us-east-1/id-node-1"},
264+
expected: &getNodeReferencesResult{
265+
references: []corev1.ObjectReference{{Name: "node-1"}},
266+
available: 0,
267+
ready: 1,
268+
},
269+
minReadySeconds: 20,
270+
},
271+
{
272+
name: "valid provider id, valid aws node, with minReadySeconds equals 0",
273+
providerIDList: []string{"aws://us-east-1/id-node-1"},
274+
expected: &getNodeReferencesResult{
275+
references: []corev1.ObjectReference{{Name: "node-1"}},
276+
available: 1,
277+
ready: 1,
278+
},
279+
minReadySeconds: 0,
280+
},
178281
}
179282

180283
for _, test := range testCases {
181284
t.Run(test.name, func(t *testing.T) {
182285
g := NewWithT(t)
183286

184-
result, err := r.getNodeReferences(ctx, client, test.providerIDList)
287+
result, err := r.getNodeReferences(ctx, client, test.providerIDList, ptr.To(test.minReadySeconds))
185288
if test.err == nil {
186289
g.Expect(err).ToNot(HaveOccurred())
187290
} else {
@@ -195,6 +298,9 @@ func TestMachinePoolGetNodeReference(t *testing.T) {
195298

196299
g.Expect(result.references).To(HaveLen(len(test.expected.references)), "Expected NodeRef count to be %v, got %v", len(result.references), len(test.expected.references))
197300

301+
g.Expect(result.available).To(Equal(test.expected.available), "Expected available node count to be %v, got %v", test.expected.available, result.available)
302+
g.Expect(result.ready).To(Equal(test.expected.ready), "Expected ready node count to be %v, got %v", test.expected.ready, result.ready)
303+
198304
for n := range test.expected.references {
199305
g.Expect(result.references[n].Name).To(Equal(test.expected.references[n].Name), "Expected NodeRef's name to be %v, got %v", result.references[n].Name, test.expected.references[n].Name)
200306
g.Expect(result.references[n].Namespace).To(Equal(test.expected.references[n].Namespace), "Expected NodeRef's namespace to be %v, got %v", result.references[n].Namespace, test.expected.references[n].Namespace)

‎exp/internal/controllers/machinepool_controller_phases_test.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1616,6 +1616,7 @@ func TestReconcileMachinePoolScaleToFromZero(t *testing.T) {
16161616
},
16171617
},
16181618
},
1619+
MinReadySeconds: ptr.To[int32](0),
16191620
},
16201621
Status: expv1.MachinePoolStatus{},
16211622
}

0 commit comments

Comments
 (0)