Skip to content

Commit

Permalink
implement minReadySeconds for machinepool
Browse files Browse the repository at this point in the history
  • Loading branch information
Dhairya-Arora01 committed May 25, 2024
1 parent 25fa000 commit 089e073
Show file tree
Hide file tree
Showing 5 changed files with 37 additions and 23 deletions.
1 change: 0 additions & 1 deletion config/crd/bases/cluster.x-k8s.io_machinepools.yaml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 0 additions & 1 deletion exp/api/v1beta1/machinepool_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,6 @@ type MachinePoolSpec struct {
// be ready.
// Defaults to 0 (machine instance will be considered available as soon as it
// is ready)
// NOTE: No logic is implemented for this field and it currently has no behaviour.
// +optional
MinReadySeconds *int32 `json:"minReadySeconds,omitempty"`

Expand Down
25 changes: 9 additions & 16 deletions exp/internal/controllers/machinepool_controller_noderef.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,11 +23,13 @@ import (

"github.com/pkg/errors"
corev1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/klog/v2"
ctrl "sigs.k8s.io/controller-runtime"
"sigs.k8s.io/controller-runtime/pkg/client"

clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1"
"sigs.k8s.io/cluster-api/controllers/noderefutil"
expv1 "sigs.k8s.io/cluster-api/exp/api/v1beta1"
"sigs.k8s.io/cluster-api/internal/util/taints"
"sigs.k8s.io/cluster-api/util"
Expand All @@ -36,9 +38,7 @@ import (
"sigs.k8s.io/cluster-api/util/patch"
)

var (
errNoAvailableNodes = errors.New("cannot find nodes with matching ProviderIDs in ProviderIDList")
)
var errNoAvailableNodes = errors.New("cannot find nodes with matching ProviderIDs in ProviderIDList")

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

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

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

var ready, available int
Expand Down Expand Up @@ -185,9 +185,11 @@ func (r *MachinePoolReconciler) getNodeReferences(ctx context.Context, c client.
continue
}
if node, ok := nodeRefsMap[providerID]; ok {
available++
if nodeIsReady(&node) {
if noderefutil.IsNodeReady(&node) {
ready++
if noderefutil.IsNodeAvailable(&node, *minReadySeconds, metav1.Now()) {
available++
}
}
nodeRefs = append(nodeRefs, corev1.ObjectReference{
APIVersion: corev1.SchemeGroupVersion.String(),
Expand Down Expand Up @@ -236,12 +238,3 @@ func (r *MachinePoolReconciler) patchNodes(ctx context.Context, c client.Client,
}
return nil
}

func nodeIsReady(node *corev1.Node) bool {
for _, n := range node.Status.Conditions {
if n.Type == corev1.NodeReady {
return n.Status == corev1.ConditionTrue
}
}
return false
}
32 changes: 27 additions & 5 deletions exp/internal/controllers/machinepool_controller_noderef_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import (
corev1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/client-go/tools/record"
"k8s.io/utils/ptr"
"sigs.k8s.io/controller-runtime/pkg/client"
"sigs.k8s.io/controller-runtime/pkg/client/fake"

Expand Down Expand Up @@ -90,10 +91,11 @@ func TestMachinePoolGetNodeReference(t *testing.T) {
client := fake.NewClientBuilder().WithObjects(nodeList...).Build()

testCases := []struct {
name string
providerIDList []string
expected *getNodeReferencesResult
err error
name string
providerIDList []string
expected *getNodeReferencesResult
err error
minReadySeconds int32
}{
{
name: "valid provider id, valid aws node",
Expand Down Expand Up @@ -175,13 +177,33 @@ func TestMachinePoolGetNodeReference(t *testing.T) {
},
},
},
{
name: "valid provider id, valid aws node, with minReadySeconds",
providerIDList: []string{"aws://us-east-1/id-node-1"},
expected: &getNodeReferencesResult{
references: []corev1.ObjectReference{{Name: "node-1"}},
available: 0,
ready: 1,
},
minReadySeconds: 20,
},
{
name: "valid provider id, valid aws node, with minReadySeconds equals 0",
providerIDList: []string{"aws://us-east-1/id-node-1"},
expected: &getNodeReferencesResult{
references: []corev1.ObjectReference{{Name: "node-1"}},
available: 1,
ready: 1,
},
minReadySeconds: 0,
},
}

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

result, err := r.getNodeReferences(ctx, client, test.providerIDList)
result, err := r.getNodeReferences(ctx, client, test.providerIDList, ptr.To(test.minReadySeconds))
if test.err == nil {
g.Expect(err).ToNot(HaveOccurred())
} else {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1616,6 +1616,7 @@ func TestReconcileMachinePoolScaleToFromZero(t *testing.T) {
},
},
},
MinReadySeconds: ptr.To[int32](0),
},
Status: expv1.MachinePoolStatus{},
}
Expand Down

0 comments on commit 089e073

Please sign in to comment.