Skip to content

Commit 127dbbf

Browse files
feat: anyOf readiness condition requirements
Introduces a conditionPolicy field (allOf | anyOf) to NodeReadinessRuleSpec that controls how the list of node conditions is aggregated when evaluating a rule. - allOf (default): every condition must match its requiredStatus before the taint is removed — preserves existing behavior. - anyOf: at least one condition matching its requiredStatus is sufficient to remove the taint. Additional changes: - Implement IsConditionsSatisfied() with allOf/anyOf evaluation logic - Update controller to use conditionPolicy when evaluating conditions - Add webhook validation to reject conditionPolicy with bootstrap-only mode - Expand controller test coverage for conditionPolicy scenarios - Update CRD schema to include conditionPolicy field
1 parent f97f526 commit 127dbbf

6 files changed

Lines changed: 261 additions & 13 deletions

File tree

api/v1alpha1/nodereadinessrule_types.go

Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,18 @@ const (
3333
EnforcementModeContinuous EnforcementMode = "continuous"
3434
)
3535

36+
// ConditionPolicy defines how the list of conditions is aggregated when evaluating a rule.
37+
// +kubebuilder:validation:Enum=allOf;anyOf
38+
type ConditionPolicy string
39+
40+
const (
41+
// ConditionPolicyAllOf requires ALL conditions to match their requiredStatus (default).
42+
ConditionPolicyAllOf ConditionPolicy = "allOf"
43+
44+
// ConditionPolicyAnyOf requires at least ONE condition to match its requiredStatus.
45+
ConditionPolicyAnyOf ConditionPolicy = "anyOf"
46+
)
47+
3648
// TaintStatus specifies status of the Taint on Node.
3749
// +kubebuilder:validation:Enum=Present;Absent
3850
type TaintStatus string
@@ -99,6 +111,16 @@ type NodeReadinessRuleSpec struct {
99111
// +kubebuilder:validation:XValidation:rule="self == oldSelf",message="nodeSelector is immutable"
100112
NodeSelector metav1.LabelSelector `json:"nodeSelector,omitempty,omitzero"`
101113

114+
// conditionPolicy controls how the conditions list is evaluated.
115+
// "allOf" (default) requires every condition to match its requiredStatus before the taint is removed.
116+
// "anyOf" requires at least one condition to match its requiredStatus.
117+
//
118+
// Cannot be used with enforcementMode: bootstrap-only.
119+
//
120+
// +optional
121+
// +kubebuilder:validation:XValidation:rule="self == oldSelf",message="conditionPolicy is immutable"
122+
ConditionPolicy ConditionPolicy `json:"conditionPolicy,omitempty"`
123+
102124
// dryRun when set to true, The controller will evaluate Node conditions and log intended taint modifications
103125
// without persisting changes to the cluster. Proposed actions are reflected in the resource status.
104126
//
@@ -379,6 +401,50 @@ func (c *ConditionRequirement) GetDefaultStatus() corev1.ConditionStatus {
379401
return c.DefaultStatus
380402
}
381403

404+
// GetConditionPolicy returns the effective condition policy, defaulting to allOf
405+
// when the field is not explicitly set.
406+
func (spec *NodeReadinessRuleSpec) GetConditionPolicy() ConditionPolicy {
407+
if spec.ConditionPolicy == "" {
408+
return ConditionPolicyAllOf
409+
}
410+
return spec.ConditionPolicy
411+
}
412+
413+
// IsConditionsSatisfied evaluates whether the given condition results meet the
414+
// policy defined in the spec.
415+
//
416+
// - allOf: every result's effective status must equal its RequiredStatus.
417+
// - anyOf: at least one result's effective status must equal its RequiredStatus.
418+
//
419+
// The effective status is CurrentStatus, unless the condition was not found on
420+
// the Node (CurrentStatus == Unknown) and a DefaultStatus is configured — in
421+
// that case DefaultStatus is used as the fallback, matching the original
422+
// per-condition evaluation logic in the controller.
423+
func (spec *NodeReadinessRuleSpec) IsConditionsSatisfied(results []ConditionEvaluationResult) bool {
424+
effectiveStatus := func(r ConditionEvaluationResult) corev1.ConditionStatus {
425+
if r.CurrentStatus == corev1.ConditionUnknown && r.DefaultStatus != "" {
426+
return r.DefaultStatus
427+
}
428+
return r.CurrentStatus
429+
}
430+
431+
if spec.GetConditionPolicy() == ConditionPolicyAnyOf {
432+
for _, r := range results {
433+
if effectiveStatus(r) == r.RequiredStatus {
434+
return true
435+
}
436+
}
437+
return false
438+
}
439+
// allOf (default)
440+
for _, r := range results {
441+
if effectiveStatus(r) != r.RequiredStatus {
442+
return false
443+
}
444+
}
445+
return true
446+
}
447+
382448
func init() {
383449
objectTypes = append(objectTypes, &NodeReadinessRule{}, &NodeReadinessRuleList{})
384450
}

config/crd/bases/readiness.node.x-k8s.io_nodereadinessrules.yaml

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,20 @@ spec:
6262
spec:
6363
description: spec defines the desired state of NodeReadinessRule
6464
properties:
65+
conditionPolicy:
66+
description: |-
67+
conditionPolicy controls how the conditions list is evaluated.
68+
"allOf" (default) requires every condition to match its requiredStatus before the taint is removed.
69+
"anyOf" requires at least one condition to match its requiredStatus.
70+
71+
Cannot be used with enforcementMode: bootstrap-only.
72+
enum:
73+
- allOf
74+
- anyOf
75+
type: string
76+
x-kubernetes-validations:
77+
- message: conditionPolicy is immutable
78+
rule: self == oldSelf
6579
conditions:
6680
description: |-
6781
conditions contains a list of the Node conditions that defines the specific

internal/controller/nodereadinessrule_controller.go

Lines changed: 13 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -320,8 +320,7 @@ func (r *RuleReadinessController) evaluateRuleForNode(ctx context.Context, rule
320320
defer timer.ObserveDuration()
321321
log := ctrl.LoggerFrom(ctx)
322322

323-
// Evaluate all conditions (ALL logic)
324-
allConditionsSatisfied := true
323+
// Evaluate all conditions
325324
conditionResults := make([]readinessv1alpha1.ConditionEvaluationResult, 0, len(rule.Spec.Conditions))
326325

327326
for _, condReq := range rule.Spec.Conditions {
@@ -340,7 +339,6 @@ func (r *RuleReadinessController) evaluateRuleForNode(ctx context.Context, rule
340339
}
341340

342341
if !satisfied {
343-
allConditionsSatisfied = false
344342
metrics.ConditionEvaluationFailures.WithLabelValues(rule.Name, condReq.Type).Inc()
345343
}
346344

@@ -357,12 +355,12 @@ func (r *RuleReadinessController) evaluateRuleForNode(ctx context.Context, rule
357355
"satisfied", satisfied)
358356
}
359357

360-
// Determine taint action
361-
shouldRemoveTaint := allConditionsSatisfied
358+
// Determine taint action using the rule's conditionPolicy (allOf or anyOf)
359+
shouldRemoveTaint := rule.Spec.IsConditionsSatisfied(conditionResults)
362360
currentlyHasTaint := r.hasTaintBySpec(node, rule.Spec.Taint)
363361

364362
log.Info("Evaluation result", "node", node.Name, "rule", rule.Name,
365-
"allConditionsSatisfied", allConditionsSatisfied, "hasTaint", currentlyHasTaint)
363+
"conditionPolicy", rule.Spec.GetConditionPolicy(), "conditionsSatisfied", shouldRemoveTaint, "hasTaint", currentlyHasTaint)
366364

367365
isFirstEvaluation := r.getPreviousNodeEvaluation(rule, node.Name) == nil
368366

@@ -604,9 +602,9 @@ func (r *RuleReadinessController) processDryRun(ctx context.Context, rule *readi
604602

605603
affectedNodes++
606604

607-
// Simulate rule evaluation
608-
allConditionsSatisfied := true
605+
// Simulate rule evaluation using the rule's conditionPolicy
609606
missingConditions := 0
607+
dryRunResults := make([]readinessv1alpha1.ConditionEvaluationResult, 0, len(rule.Spec.Conditions))
610608

611609
for _, condReq := range rule.Spec.Conditions {
612610
currentStatus, conditionFound := r.getConditionStatus(
@@ -617,12 +615,15 @@ func (r *RuleReadinessController) processDryRun(ctx context.Context, rule *readi
617615
if !conditionFound {
618616
missingConditions++
619617
}
620-
if currentStatus != condReq.RequiredStatus {
621-
allConditionsSatisfied = false
622-
}
618+
dryRunResults = append(dryRunResults, readinessv1alpha1.ConditionEvaluationResult{
619+
Type: condReq.Type,
620+
CurrentStatus: currentStatus,
621+
RequiredStatus: condReq.RequiredStatus,
622+
DefaultStatus: condReq.GetDefaultStatus(),
623+
})
623624
}
624625

625-
shouldRemoveTaint := allConditionsSatisfied
626+
shouldRemoveTaint := rule.Spec.IsConditionsSatisfied(dryRunResults)
626627
currentlyHasTaint := r.hasTaintBySpec(&node, rule.Spec.Taint)
627628

628629
if shouldRemoveTaint && currentlyHasTaint {

internal/controller/nodereadinessrule_controller_test.go

Lines changed: 155 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2178,7 +2178,7 @@ var _ = Describe("NodeReadinessRule Controller", func() {
21782178
Expect(failedNames).To(ContainElement("fail-path-node"))
21792179
})
21802180

2181-
It("should remove stale failedNodes entry when evaluation succeeds and include the node in appliedNodes", func() {
2181+
It("stale failedNodes entries are cleared after successful evaluation", func() {
21822182
successNode := &corev1.Node{
21832183
ObjectMeta: metav1.ObjectMeta{
21842184
Name: "stale-recovery-node",
@@ -2235,4 +2235,158 @@ var _ = Describe("NodeReadinessRule Controller", func() {
22352235
Expect(failedNames).NotTo(ContainElement("stale-recovery-node"))
22362236
})
22372237
})
2238+
2239+
Context("ConditionPolicy", func() {
2240+
var (
2241+
anyOfController *RuleReadinessController
2242+
anyOfNode *corev1.Node
2243+
)
2244+
2245+
BeforeEach(func() {
2246+
anyOfController = &RuleReadinessController{
2247+
Client: k8sClient,
2248+
Scheme: scheme,
2249+
clientset: fakeClientset,
2250+
ruleCache: make(map[string]*nodereadinessiov1alpha1.NodeReadinessRule),
2251+
EventRecorder: record.NewFakeRecorder(10),
2252+
}
2253+
anyOfNode = &corev1.Node{
2254+
ObjectMeta: metav1.ObjectMeta{
2255+
Name: "anyof-test-node",
2256+
Labels: map[string]string{"anyof-test": "true"},
2257+
},
2258+
Spec: corev1.NodeSpec{},
2259+
Status: corev1.NodeStatus{},
2260+
}
2261+
})
2262+
2263+
It("anyOf: removes taint when at least one condition is satisfied", func() {
2264+
anyOfNode.Status.Conditions = []corev1.NodeCondition{
2265+
{Type: "gpu.example.com/HardwareDriverReady", Status: corev1.ConditionTrue},
2266+
{Type: "gpu.example.com/SoftwareFallbackReady", Status: corev1.ConditionFalse},
2267+
}
2268+
anyOfNode.Spec.Taints = []corev1.Taint{
2269+
{Key: "readiness.k8s.io/GPUReady", Effect: corev1.TaintEffectNoSchedule},
2270+
}
2271+
2272+
rule := &nodereadinessiov1alpha1.NodeReadinessRule{
2273+
ObjectMeta: metav1.ObjectMeta{Name: "anyof-rule-removes-taint"},
2274+
Spec: nodereadinessiov1alpha1.NodeReadinessRuleSpec{
2275+
ConditionPolicy: nodereadinessiov1alpha1.ConditionPolicyAnyOf,
2276+
Conditions: []nodereadinessiov1alpha1.ConditionRequirement{
2277+
{Type: "gpu.example.com/HardwareDriverReady", RequiredStatus: corev1.ConditionTrue},
2278+
{Type: "gpu.example.com/SoftwareFallbackReady", RequiredStatus: corev1.ConditionTrue},
2279+
},
2280+
Taint: corev1.Taint{Key: "readiness.k8s.io/GPUReady", Effect: corev1.TaintEffectNoSchedule},
2281+
NodeSelector: metav1.LabelSelector{MatchLabels: map[string]string{"anyof-test": "true"}},
2282+
EnforcementMode: nodereadinessiov1alpha1.EnforcementModeContinuous,
2283+
},
2284+
}
2285+
anyOfController.updateRuleCache(ctx, rule)
2286+
2287+
Expect(k8sClient.Create(ctx, anyOfNode)).To(Succeed())
2288+
defer func() { Expect(k8sClient.Delete(ctx, anyOfNode)).To(Succeed()) }()
2289+
Expect(anyOfController.evaluateRuleForNode(ctx, rule, anyOfNode)).To(Succeed())
2290+
2291+
// Taint should have been removed because HardwareDriverReady=True satisfies anyOf
2292+
Expect(anyOfController.hasTaintBySpec(anyOfNode, rule.Spec.Taint)).To(BeFalse())
2293+
2294+
// conditionResults in status should reflect actual observed values
2295+
eval := anyOfController.getPreviousNodeEvaluation(rule, anyOfNode.Name)
2296+
Expect(eval).NotTo(BeNil())
2297+
Expect(eval.ConditionResults).To(HaveLen(2))
2298+
})
2299+
2300+
It("anyOf: keeps taint when no conditions are satisfied", func() {
2301+
anyOfNode.Status.Conditions = []corev1.NodeCondition{
2302+
{Type: "gpu.example.com/HardwareDriverReady", Status: corev1.ConditionFalse},
2303+
{Type: "gpu.example.com/SoftwareFallbackReady", Status: corev1.ConditionFalse},
2304+
}
2305+
// Node has no taint; controller should add one
2306+
anyOfNode.Spec.Taints = nil
2307+
2308+
rule := &nodereadinessiov1alpha1.NodeReadinessRule{
2309+
ObjectMeta: metav1.ObjectMeta{Name: "anyof-rule-adds-taint"},
2310+
Spec: nodereadinessiov1alpha1.NodeReadinessRuleSpec{
2311+
ConditionPolicy: nodereadinessiov1alpha1.ConditionPolicyAnyOf,
2312+
Conditions: []nodereadinessiov1alpha1.ConditionRequirement{
2313+
{Type: "gpu.example.com/HardwareDriverReady", RequiredStatus: corev1.ConditionTrue},
2314+
{Type: "gpu.example.com/SoftwareFallbackReady", RequiredStatus: corev1.ConditionTrue},
2315+
},
2316+
Taint: corev1.Taint{Key: "readiness.k8s.io/GPUReady", Effect: corev1.TaintEffectNoSchedule},
2317+
NodeSelector: metav1.LabelSelector{MatchLabels: map[string]string{"anyof-test": "true"}},
2318+
EnforcementMode: nodereadinessiov1alpha1.EnforcementModeContinuous,
2319+
},
2320+
}
2321+
anyOfController.updateRuleCache(ctx, rule)
2322+
2323+
Expect(k8sClient.Create(ctx, anyOfNode)).To(Succeed())
2324+
defer func() { Expect(k8sClient.Delete(ctx, anyOfNode)).To(Succeed()) }()
2325+
Expect(anyOfController.evaluateRuleForNode(ctx, rule, anyOfNode)).To(Succeed())
2326+
2327+
// Taint should have been added because neither condition is satisfied
2328+
Expect(anyOfController.hasTaintBySpec(anyOfNode, rule.Spec.Taint)).To(BeTrue())
2329+
})
2330+
2331+
It("allOf (explicit): keeps taint when only one of two conditions is satisfied", func() {
2332+
anyOfNode.Status.Conditions = []corev1.NodeCondition{
2333+
{Type: "example.com/CondA", Status: corev1.ConditionTrue},
2334+
{Type: "example.com/CondB", Status: corev1.ConditionFalse},
2335+
}
2336+
anyOfNode.Spec.Taints = []corev1.Taint{
2337+
{Key: "readiness.k8s.io/MultiCond", Effect: corev1.TaintEffectNoSchedule},
2338+
}
2339+
2340+
rule := &nodereadinessiov1alpha1.NodeReadinessRule{
2341+
ObjectMeta: metav1.ObjectMeta{Name: "allof-explicit-rule"},
2342+
Spec: nodereadinessiov1alpha1.NodeReadinessRuleSpec{
2343+
ConditionPolicy: nodereadinessiov1alpha1.ConditionPolicyAllOf,
2344+
Conditions: []nodereadinessiov1alpha1.ConditionRequirement{
2345+
{Type: "example.com/CondA", RequiredStatus: corev1.ConditionTrue},
2346+
{Type: "example.com/CondB", RequiredStatus: corev1.ConditionTrue},
2347+
},
2348+
Taint: corev1.Taint{Key: "readiness.k8s.io/MultiCond", Effect: corev1.TaintEffectNoSchedule},
2349+
NodeSelector: metav1.LabelSelector{MatchLabels: map[string]string{"anyof-test": "true"}},
2350+
EnforcementMode: nodereadinessiov1alpha1.EnforcementModeContinuous,
2351+
},
2352+
}
2353+
anyOfController.updateRuleCache(ctx, rule)
2354+
2355+
Expect(k8sClient.Create(ctx, anyOfNode)).To(Succeed())
2356+
defer func() { Expect(k8sClient.Delete(ctx, anyOfNode)).To(Succeed()) }()
2357+
Expect(anyOfController.evaluateRuleForNode(ctx, rule, anyOfNode)).To(Succeed())
2358+
2359+
// Taint must remain because CondB is still False
2360+
Expect(anyOfController.hasTaintBySpec(anyOfNode, rule.Spec.Taint)).To(BeTrue())
2361+
})
2362+
2363+
It("anyOf: missing condition evaluated via defaultStatus does not auto-satisfy", func() {
2364+
// Node has NO conditions at all
2365+
anyOfNode.Status.Conditions = nil
2366+
anyOfNode.Spec.Taints = nil
2367+
2368+
rule := &nodereadinessiov1alpha1.NodeReadinessRule{
2369+
ObjectMeta: metav1.ObjectMeta{Name: "anyof-missing-condition-rule"},
2370+
Spec: nodereadinessiov1alpha1.NodeReadinessRuleSpec{
2371+
ConditionPolicy: nodereadinessiov1alpha1.ConditionPolicyAnyOf,
2372+
Conditions: []nodereadinessiov1alpha1.ConditionRequirement{
2373+
// No defaultStatus set — will resolve to Unknown, which != True
2374+
{Type: "gpu.example.com/HardwareDriverReady", RequiredStatus: corev1.ConditionTrue},
2375+
},
2376+
Taint: corev1.Taint{Key: "readiness.k8s.io/GPUReady", Effect: corev1.TaintEffectNoSchedule},
2377+
NodeSelector: metav1.LabelSelector{MatchLabels: map[string]string{"anyof-test": "true"}},
2378+
EnforcementMode: nodereadinessiov1alpha1.EnforcementModeContinuous,
2379+
},
2380+
}
2381+
anyOfController.updateRuleCache(ctx, rule)
2382+
2383+
Expect(k8sClient.Create(ctx, anyOfNode)).To(Succeed())
2384+
defer func() { Expect(k8sClient.Delete(ctx, anyOfNode)).To(Succeed()) }()
2385+
Expect(anyOfController.evaluateRuleForNode(ctx, rule, anyOfNode)).To(Succeed())
2386+
2387+
// Missing condition resolves to Unknown != True, so anyOf is not satisfied
2388+
// Taint should have been added (node has none, conditions not satisfied)
2389+
Expect(anyOfController.hasTaintBySpec(anyOfNode, rule.Spec.Taint)).To(BeTrue())
2390+
})
2391+
})
22382392
})

internal/webhook/nodereadinessgaterule_webhook.go

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -90,6 +90,19 @@ func (w *NodeReadinessRuleWebhook) validateSpec(
9090
}
9191
}
9292
}
93+
94+
// validate anyOf conditionPolicy is not combined with bootstrap-only mode.
95+
// The semantics are undefined: once the taint is permanently removed after
96+
// the first condition satisfies, subsequent condition changes have no effect,
97+
// making the "any" relationship unverifiable.
98+
if spec.ConditionPolicy == readinessv1alpha1.ConditionPolicyAnyOf &&
99+
spec.EnforcementMode == readinessv1alpha1.EnforcementModeBootstrapOnly {
100+
allErrs = append(allErrs, field.Forbidden(
101+
field.NewPath("spec", "conditionPolicy"),
102+
"anyOf conditionPolicy is not supported with bootstrap-only enforcementMode",
103+
))
104+
}
105+
93106
return allErrs
94107
}
95108

manifests.yaml

12.7 KB
Binary file not shown.

0 commit comments

Comments
 (0)