Skip to content

Commit 6a11470

Browse files
author
Sedef
committed
Changed summarizeNodeConditions signature
1 parent 054fab6 commit 6a11470

File tree

3 files changed

+98
-31
lines changed

3 files changed

+98
-31
lines changed

‎controllers/machine_controller.go

+1-5
Original file line numberDiff line numberDiff line change
@@ -623,11 +623,7 @@ func (r *MachineReconciler) reconcileDeleteExternal(ctx context.Context, m *clus
623623
obj.GroupVersionKind(), obj.GetName(), m.Name, m.Namespace)
624624
}
625625
// If delete is successful, then return empty object.
626-
obj, err = external.Get(ctx, r.Client, ref, m.Namespace)
627-
if err != nil && !apierrors.IsNotFound(errors.Cause(err)) {
628-
return nil, errors.Wrapf(err, "failed to get %s %q for Machine %q in namespace %q",
629-
ref.GroupVersionKind(), ref.Name, m.Name, m.Namespace)
630-
}
626+
obj = nil
631627
}
632628

633629
// Return true if there are no more external objects.

‎controllers/machine_controller_node.go

+36-23
Original file line numberDiff line numberDiff line change
@@ -18,9 +18,10 @@ package controllers
1818

1919
import (
2020
"context"
21+
"fmt"
22+
2123
"github.com/pkg/errors"
22-
apicorev1 "k8s.io/api/core/v1"
23-
kerrors "k8s.io/apimachinery/pkg/util/errors"
24+
corev1 "k8s.io/api/core/v1"
2425
clusterv1 "sigs.k8s.io/cluster-api/api/v1alpha3"
2526
"sigs.k8s.io/cluster-api/controllers/noderefutil"
2627
"sigs.k8s.io/cluster-api/util"
@@ -52,35 +53,36 @@ func (r *MachineReconciler) reconcileNode(ctx context.Context, cluster *clusterv
5253
node, err := r.getNode(remoteClient, providerID)
5354
if err != nil {
5455
logger.Error(err, "Failed to retrieve Node by ProviderID")
55-
r.recorder.Event(machine, apicorev1.EventTypeWarning, "Failed to retrieve Node by ProviderID", err.Error())
56+
r.recorder.Event(machine, corev1.EventTypeWarning, "Failed to retrieve Node by ProviderID", err.Error())
5657
return reconcile.Result{}, err
5758
}
5859
if node == nil {
5960
// While a NodeRef is set in the status, failing to get that node means the node is deleted.
6061
// If Status.NodeRef is not set before, node still can be in the provisioning state.
6162
if machine.Status.NodeRef != nil {
6263
conditions.MarkFalse(machine, clusterv1.MachineNodeHealthyCondition, clusterv1.NodeNotFoundReason, clusterv1.ConditionSeverityError, "")
64+
return reconcile.Result{}, errors.Wrapf(err, "no matching Node for Machine %q in namespace %q", machine.Name, machine.Namespace)
6365
}
64-
return reconcile.Result{}, errors.Wrapf(err, "no matching Node for Machine %q in namespace %q", machine.Name, machine.Namespace)
66+
conditions.MarkFalse(machine, clusterv1.MachineNodeHealthyCondition, clusterv1.NodeProvisioningReason, clusterv1.ConditionSeverityWarning, "")
67+
return reconcile.Result{Requeue: true}, nil
6568
}
6669

6770
// Set the Machine NodeRef.
6871
if machine.Status.NodeRef == nil {
69-
machine.Status.NodeRef = &apicorev1.ObjectReference{
72+
machine.Status.NodeRef = &corev1.ObjectReference{
7073
Kind: node.Kind,
7174
APIVersion: node.APIVersion,
7275
Name: node.Name,
7376
UID: node.UID,
7477
}
7578
logger.Info("Set Machine's NodeRef", "noderef", machine.Status.NodeRef.Name)
76-
r.recorder.Event(machine, apicorev1.EventTypeNormal, "SuccessfulSetNodeRef", machine.Status.NodeRef.Name)
77-
conditions.MarkFalse(machine, clusterv1.MachineNodeHealthyCondition, clusterv1.NodeProvisioningReason, clusterv1.ConditionSeverityWarning, "")
78-
return reconcile.Result{}, nil
79+
r.recorder.Event(machine, corev1.EventTypeNormal, "SuccessfulSetNodeRef", machine.Status.NodeRef.Name)
7980
}
8081

8182
// Do the remaining node health checks, then set the node health to true if all checks pass.
82-
if err := summarizeNodeConditions(node); err != nil {
83-
conditions.MarkFalse(machine, clusterv1.MachineNodeHealthyCondition, clusterv1.NodeConditionsFailedReason, clusterv1.ConditionSeverityWarning, err.Error())
83+
status, message := summarizeNodeConditions(node)
84+
if status != corev1.ConditionTrue {
85+
conditions.MarkFalse(machine, clusterv1.MachineNodeHealthyCondition, clusterv1.NodeConditionsFailedReason, clusterv1.ConditionSeverityWarning, message)
8486
return reconcile.Result{}, nil
8587
}
8688

@@ -89,29 +91,40 @@ func (r *MachineReconciler) reconcileNode(ctx context.Context, cluster *clusterv
8991
}
9092

9193
// summarizeNodeConditions checks if a Node is healthy and does not have any semantically negative Conditions.
92-
// Returns error if NodeMemoryPressure, NodeDiskPressure, or NodePIDPressure Conditions are false or Ready Condition is true.
93-
func summarizeNodeConditions(node *apicorev1.Node) error {
94-
errList := []error{}
95-
94+
// Returns the summary of condition statuses (e.g., if all semantically correct, then status is true with nil message)
95+
// and concatenate failed condition messages (semantically true conditions: NodeMemoryPressure/NodeDiskPressure/NodePIDPressure == false or Ready == true.)
96+
func summarizeNodeConditions(node *corev1.Node) (corev1.ConditionStatus, string) {
97+
status := corev1.ConditionTrue
98+
message := ""
9699
for _, condition := range node.Status.Conditions {
97100
switch condition.Type {
98-
case apicorev1.NodeMemoryPressure, apicorev1.NodeDiskPressure, apicorev1.NodePIDPressure:
99-
if condition.Status == apicorev1.ConditionTrue {
100-
errList = append(errList, errors.Errorf("Condition %s is true", condition.Type))
101+
case corev1.NodeMemoryPressure, corev1.NodeDiskPressure, corev1.NodePIDPressure:
102+
if condition.Status != corev1.ConditionFalse {
103+
message += fmt.Sprintf("Condition %s is %s", condition.Type, condition.Status) + ". "
104+
if condition.Status == corev1.ConditionUnknown {
105+
status = corev1.ConditionUnknown
106+
continue
107+
}
108+
status = corev1.ConditionFalse
101109
}
102-
case apicorev1.NodeReady:
103-
if condition.Status == apicorev1.ConditionFalse {
104-
errList = append(errList, errors.Errorf("Condition %s is false", condition.Type))
110+
case corev1.NodeReady:
111+
if condition.Status != corev1.ConditionTrue {
112+
message += fmt.Sprintf("Condition %s is %s", condition.Type, condition.Status) + ". "
113+
if condition.Status == corev1.ConditionUnknown {
114+
status = corev1.ConditionUnknown
115+
continue
116+
}
117+
status = corev1.ConditionFalse
105118
}
106119
}
107120
}
108-
return kerrors.NewAggregate(errList)
121+
return status, message
109122
}
110123

111-
func (r *MachineReconciler) getNode(c client.Reader, providerID *noderefutil.ProviderID) (*apicorev1.Node, error) {
124+
func (r *MachineReconciler) getNode(c client.Reader, providerID *noderefutil.ProviderID) (*corev1.Node, error) {
112125
logger := r.Log.WithValues("providerID", providerID)
113126

114-
nodeList := apicorev1.NodeList{}
127+
nodeList := corev1.NodeList{}
115128
for {
116129
if err := c.List(context.TODO(), &nodeList, client.Continue(nodeList.Continue)); err != nil {
117130
return nil, err

‎controllers/machine_controller_node_test.go

+61-3
Original file line numberDiff line numberDiff line change
@@ -26,11 +26,10 @@ import (
2626
"k8s.io/apimachinery/pkg/runtime"
2727
"k8s.io/client-go/kubernetes/scheme"
2828
"k8s.io/client-go/tools/record"
29-
"sigs.k8s.io/controller-runtime/pkg/client/fake"
30-
"sigs.k8s.io/controller-runtime/pkg/log"
31-
3229
clusterv1 "sigs.k8s.io/cluster-api/api/v1alpha3"
3330
"sigs.k8s.io/cluster-api/controllers/noderefutil"
31+
"sigs.k8s.io/controller-runtime/pkg/client/fake"
32+
"sigs.k8s.io/controller-runtime/pkg/log"
3433
)
3534

3635
func TestGetNodeReference(t *testing.T) {
@@ -126,3 +125,62 @@ func TestGetNodeReference(t *testing.T) {
126125

127126
}
128127
}
128+
129+
func TestSummarizeNodeConditions(t *testing.T) {
130+
testCases := []struct {
131+
name string
132+
conditions []corev1.NodeCondition
133+
messageLen int
134+
status corev1.ConditionStatus
135+
}{
136+
{
137+
name: "node is healthy",
138+
conditions: []corev1.NodeCondition{
139+
{Type: corev1.NodeReady, Status: corev1.ConditionTrue},
140+
{Type: corev1.NodeMemoryPressure, Status: corev1.ConditionFalse},
141+
{Type: corev1.NodeDiskPressure, Status: corev1.ConditionFalse},
142+
{Type: corev1.NodePIDPressure, Status: corev1.ConditionFalse},
143+
},
144+
messageLen: 0,
145+
status: corev1.ConditionTrue,
146+
},
147+
{
148+
name: "unknown condition",
149+
conditions: []corev1.NodeCondition{
150+
{Type: corev1.NodeReady, Status: corev1.ConditionUnknown},
151+
{Type: corev1.NodeMemoryPressure, Status: corev1.ConditionFalse},
152+
{Type: corev1.NodeDiskPressure, Status: corev1.ConditionFalse},
153+
{Type: corev1.NodePIDPressure, Status: corev1.ConditionFalse},
154+
},
155+
messageLen: 10,
156+
status: corev1.ConditionUnknown,
157+
},
158+
{
159+
name: "multiple semantically failed condition",
160+
conditions: []corev1.NodeCondition{
161+
{Type: corev1.NodeReady, Status: corev1.ConditionUnknown},
162+
{Type: corev1.NodeMemoryPressure, Status: corev1.ConditionTrue},
163+
{Type: corev1.NodeDiskPressure, Status: corev1.ConditionTrue},
164+
{Type: corev1.NodePIDPressure, Status: corev1.ConditionTrue},
165+
},
166+
messageLen: 50,
167+
status: corev1.ConditionFalse,
168+
},
169+
}
170+
for _, test := range testCases {
171+
t.Run(test.name, func(t *testing.T) {
172+
g := NewWithT(t)
173+
node := &corev1.Node{
174+
ObjectMeta: metav1.ObjectMeta{
175+
Name: "node-1",
176+
},
177+
Status: corev1.NodeStatus{
178+
Conditions: test.conditions,
179+
},
180+
}
181+
status, message := summarizeNodeConditions(node)
182+
g.Expect(status).To(Equal(test.status))
183+
g.Expect(len(message)).Should(BeNumerically(">=", test.messageLen))
184+
})
185+
}
186+
}

0 commit comments

Comments
 (0)