From ddc1d02f211deafa837734fc47e5cbeb951ebbad Mon Sep 17 00:00:00 2001 From: Karthik Bhat Date: Mon, 3 Aug 2026 17:43:47 +0530 Subject: [PATCH 1/4] Feature: Introduce nodereadinessevaluation CRD --- PROJECT | 7 + api/v1alpha1/nodereadinessevaluation_types.go | 260 ++++++ api/v1alpha1/zz_generated.deepcopy.go | 141 ++++ cmd/main.go | 45 +- ...ode.x-k8s.io_nodereadinessevaluations.yaml | 336 ++++++++ config/manager/manager.yaml | 3 +- config/rbac/role.yaml | 19 +- .../nodereadinessevaluation_controller.go | 425 ++++++++++ ...nodereadinessevaluation_controller_test.go | 754 ++++++++++++++++++ .../nodereadinessrule_controller.go | 2 +- 10 files changed, 1973 insertions(+), 19 deletions(-) create mode 100644 api/v1alpha1/nodereadinessevaluation_types.go create mode 100644 config/crd/bases/readiness.node.x-k8s.io_nodereadinessevaluations.yaml create mode 100644 internal/controller/nodereadinessevaluation_controller.go create mode 100644 internal/controller/nodereadinessevaluation_controller_test.go diff --git a/PROJECT b/PROJECT index b7b0e020..9d1a8419 100644 --- a/PROJECT +++ b/PROJECT @@ -9,6 +9,13 @@ layout: projectName: nrrcontroller repo: sigs.k8s.io/node-readiness-controller resources: +- api: + crdVersion: v1 + controller: true + domain: readiness.node.x-k8s.io + kind: NodeReadinessEvaluation + path: sigs.k8s.io/node-readiness-controller/api/v1alpha1 + version: v1alpha1 - api: crdVersion: v1 controller: true diff --git a/api/v1alpha1/nodereadinessevaluation_types.go b/api/v1alpha1/nodereadinessevaluation_types.go new file mode 100644 index 00000000..3014f5a0 --- /dev/null +++ b/api/v1alpha1/nodereadinessevaluation_types.go @@ -0,0 +1,260 @@ +/* +Copyright The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package v1alpha1 + +import ( + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/types" +) + +// NodeEvaluationState indicates the overall readiness/availability state of the node based on all rules. +// +kubebuilder:validation:Enum=Available;NotAvailable +type NodeEvaluationState string + +const ( + // NodeEvaluationStateAvailable indicates the node has satisfied all applicable rules and is available for scheduling. + NodeEvaluationStateAvailable NodeEvaluationState = "Available" + + // NodeEvaluationStateNotAvailable indicates one or more applicable rules are currently not satisfied. + NodeEvaluationStateNotAvailable NodeEvaluationState = "NotAvailable" +) + +// RuleStatus defines the result of evaluating a NodeReadinessRule's criteria against a Node. +// Rule-configuration faults (e.g. an invalid NodeSelector) are reported on the +// NodeReadinessRule's own status conditions, not here. +// +kubebuilder:validation:Enum=Satisfied;Unsatisfied +type RuleStatus string + +const ( + // RuleStatusSatisfied indicates that the Node successfully met all conditions + // defined in the NodeReadinessRule. The controller will ensure the corresponding + // taint is removed so the node is unblocked. + RuleStatusSatisfied RuleStatus = "Satisfied" + + // RuleStatusUnsatisfied indicates that one or more conditions defined in the + // NodeReadinessRule were not met. The controller will ensure the corresponding + // taint remains present to block scheduling. + RuleStatusUnsatisfied RuleStatus = "Unsatisfied" +) + +// NodeReadinessEvaluationSpec defines the desired state of NodeReadinessEvaluation. +type NodeReadinessEvaluationSpec struct { + // nodeName specifies the exact name of the target Kubernetes Node. + // This object establishes a strict 1:1 relationship with the specified node, + // acting as the single source of truth for all rules and statuses applied to it. + // Because it binds this resource to a specific physical or virtual machine, it cannot be changed once set. + // + // The validation constraints enforce standard Kubernetes resource naming + // (RFC 1123 DNS Subdomain format), as defined in upstream apimachinery: + // https://github.com/kubernetes/apimachinery/blob/master/pkg/util/validation/validation.go#L198 + // + // +required + // +kubebuilder:validation:MinLength=1 + // +kubebuilder:validation:MaxLength=253 + // +kubebuilder:validation:Pattern=`^[a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*$` + // +kubebuilder:validation:XValidation:rule="self == oldSelf",message="nodeName is immutable and cannot be changed once set" + NodeName string `json:"nodeName,omitempty"` +} + +// NodeReadinessEvaluationStatus defines the observed state of NodeReadinessEvaluation. +// +kubebuilder:validation:MinProperties=1 +type NodeReadinessEvaluationStatus struct { + // conditions represent the latest available observations of the node's readiness evaluation state. + // Known condition types are: + // - "Evaluated": indicates whether the controller successfully evaluated all rules without errors. + // - "Available": indicates whether the node has satisfied all rules, has zero taints applied, and is available for scheduling. + // + // +optional + // +listType=map + // +listMapKey=type + // +kubebuilder:validation:MaxItems=8 + Conditions []metav1.Condition `json:"conditions,omitempty"` + + // state indicates the overall readiness state of the node based on all applicable rules. + // It acts as a top-level health indicator for this node's readiness evaluation. + // + // +optional + State NodeEvaluationState `json:"state,omitempty"` + + // rules contains the evaluation outcomes for all rules applicable to this node. + // Each entry is keyed by ruleName, allowing independent per-rule updates by + // parallel rule-workers without last-write-wins conflicts (listType=map). + // + // +optional + // +listType=map + // +listMapKey=ruleName + // +kubebuilder:validation:MaxItems=100 + Rules []RuleEvaluation `json:"rules,omitempty"` +} + +// RuleEvaluation defines the outcome of evaluating a single NodeReadinessRule against this Node. +type RuleEvaluation struct { + // ruleName is the name of the NodeReadinessRule. + // This field is the map key for status.rules (listType=map) and must always be present. + // + // +required + // +kubebuilder:validation:MinLength=1 + // +kubebuilder:validation:MaxLength=253 + RuleName string `json:"ruleName,omitempty"` + + // ruleUID is the UID of the NodeReadinessRule. + // If the rule is deleted and recreated with the same name, the UID will differ, + // allowing the controller to detect and replace stale evaluation entries. + // + // +required + RuleUID types.UID `json:"ruleUID,omitempty"` + + // ruleStatus indicates the overall outcome of the rule's criteria against the Node. + // + // +required + RuleStatus RuleStatus `json:"ruleStatus,omitempty"` + + // taintStatus reflects the observed state of the rule's specified taint on the Node (Present/Absent). + // + // +required + TaintStatus TaintStatus `json:"taintStatus,omitempty"` + + // taintKey is the key of the taint managed by this rule, stamped at evaluation + // time so this entry is self-contained without requiring a lookup of the rule. + // Matches rule.spec.taint.key. + // + // +required + // +kubebuilder:validation:MinLength=1 + // +kubebuilder:validation:MaxLength=253 + TaintKey string `json:"taintKey,omitempty"` + + // taintEffect is the effect of the taint managed by this rule, stamped at + // evaluation time so this entry is self-contained without requiring a lookup + // of the rule. Matches rule.spec.taint.effect. + // + // +required + // +kubebuilder:validation:Enum=NoSchedule;PreferNoSchedule;NoExecute + TaintEffect corev1.TaintEffect `json:"taintEffect,omitempty"` + + // reason contains a concise, machine-readable string detailing the primary outcome. + // + // +optional + // +kubebuilder:validation:MinLength=1 + // +kubebuilder:validation:MaxLength=256 + Reason string `json:"reason,omitempty"` + + // message is a comprehensive, human-readable explanation providing further context. + // + // +optional + // +kubebuilder:validation:MinLength=1 + // +kubebuilder:validation:MaxLength=10240 + Message string `json:"message,omitempty"` + + // readinessConditions provides a detailed breakdown of each condition evaluation + // for this Node. This allows for granular debugging of which specific criteria passed/failed. + // + // +optional + // +listType=map + // +listMapKey=type + // +kubebuilder:validation:MaxItems=32 + ReadinessConditions []ConditionEvaluationResult `json:"readinessConditions,omitempty"` + + // lastEvaluationTime records the exact moment the controller most recently assessed this rule. + // + // +required + LastEvaluationTime metav1.Time `json:"lastEvaluationTime,omitempty,omitzero"` + + // firstEvaluatedAt is the time the rule was first assessed against this node. + // + // +optional + FirstEvaluatedAt *metav1.Time `json:"firstEvaluatedAt,omitempty"` + + // taintObservedAt is the time NRC first observed the taint present on the node, + // regardless of whether NRC applied it or it was pre-existing (e.g. via --register-with-taints). + // This marks the beginning of the node being blocked by this rule and is the correct + // start time for computing time-to-unblock SLIs. + // + // +optional + TaintObservedAt *metav1.Time `json:"taintObservedAt,omitempty"` + + // taintAddedAt is the time NRC itself first applied the taint to the node in the + // current taint lifecycle (i.e., since the last removal). This field is nil when + // the taint was pre-existing and NRC adopted it rather than creating it. + // The value is set once on the Absent→Present transition and carried forward on + // subsequent reconciles; it is not the timestamp of the most recent reconcile. + // Use taintObservedAt for "how long has the node been blocked"; use this field + // to measure NRC's own apply latency (taintAddedAt - firstEvaluatedAt). + // + // +optional + TaintAddedAt *metav1.Time `json:"taintAddedAt,omitempty"` + + // taintRemovedAt is the time the controller successfully removed the taint + // after the node satisfied all conditions. + // + // +optional + TaintRemovedAt *metav1.Time `json:"taintRemovedAt,omitempty"` +} + +// +kubebuilder:object:root=true +// +kubebuilder:subresource:status +// +kubebuilder:resource:scope=Cluster,shortName=nre +// +kubebuilder:printcolumn:name="Node",type=string,JSONPath=`.spec.nodeName`,description="The name of the target Node." +// +kubebuilder:selectablefield:JSONPath=`.spec.nodeName` +// +kubebuilder:printcolumn:name="State",type=string,JSONPath=`.status.state`,description="The overall readiness evaluation state of the node." +// +kubebuilder:selectablefield:JSONPath=`.status.state` +// +kubebuilder:printcolumn:name="Age",type=date,JSONPath=`.metadata.creationTimestamp`,description="The age of this resource." + +// NodeReadinessEvaluation is the Schema for the NodeReadinessEvaluations API. +// Each instance maps 1:1 to a Node and folds the outcomes of all applicable +// NodeReadinessRules for that node into a single object. +// An ownerReference to the corresponding Node is set for automatic garbage +// collection when the node is deleted. +type NodeReadinessEvaluation struct { + metav1.TypeMeta `json:",inline"` + + // metadata is a standard object metadata. + // More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#metadata + // + // +optional + metav1.ObjectMeta `json:"metadata,omitempty,omitzero"` + + // spec defines the desired state of NodeReadinessEvaluation. + // + // +required + Spec NodeReadinessEvaluationSpec `json:"spec,omitempty,omitzero"` + + // status defines the observed state of NodeReadinessEvaluation. + // + // +optional + Status NodeReadinessEvaluationStatus `json:"status,omitempty,omitzero"` +} + +// +kubebuilder:object:root=true + +// NodeReadinessEvaluationList contains a list of NodeReadinessEvaluation. +type NodeReadinessEvaluationList struct { + metav1.TypeMeta `json:",inline"` + + // metadata is the standard list's metadata. + // More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#lists-and-simple-kinds + // + // +optional + metav1.ListMeta `json:"metadata,omitempty"` + + // items is the list of NodeReadinessEvaluation. + Items []NodeReadinessEvaluation `json:"items"` +} + +func init() { + objectTypes = append(objectTypes, &NodeReadinessEvaluation{}, &NodeReadinessEvaluationList{}) +} diff --git a/api/v1alpha1/zz_generated.deepcopy.go b/api/v1alpha1/zz_generated.deepcopy.go index e8c4e61d..6c25c497 100644 --- a/api/v1alpha1/zz_generated.deepcopy.go +++ b/api/v1alpha1/zz_generated.deepcopy.go @@ -21,6 +21,7 @@ limitations under the License. package v1alpha1 import ( + "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" ) @@ -126,6 +127,109 @@ func (in *NodeFailure) DeepCopy() *NodeFailure { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *NodeReadinessEvaluation) DeepCopyInto(out *NodeReadinessEvaluation) { + *out = *in + out.TypeMeta = in.TypeMeta + in.ObjectMeta.DeepCopyInto(&out.ObjectMeta) + out.Spec = in.Spec + in.Status.DeepCopyInto(&out.Status) +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new NodeReadinessEvaluation. +func (in *NodeReadinessEvaluation) DeepCopy() *NodeReadinessEvaluation { + if in == nil { + return nil + } + out := new(NodeReadinessEvaluation) + in.DeepCopyInto(out) + return out +} + +// DeepCopyObject is an autogenerated deepcopy function, copying the receiver, creating a new runtime.Object. +func (in *NodeReadinessEvaluation) DeepCopyObject() runtime.Object { + if c := in.DeepCopy(); c != nil { + return c + } + return nil +} + +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *NodeReadinessEvaluationList) DeepCopyInto(out *NodeReadinessEvaluationList) { + *out = *in + out.TypeMeta = in.TypeMeta + in.ListMeta.DeepCopyInto(&out.ListMeta) + if in.Items != nil { + in, out := &in.Items, &out.Items + *out = make([]NodeReadinessEvaluation, len(*in)) + for i := range *in { + (*in)[i].DeepCopyInto(&(*out)[i]) + } + } +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new NodeReadinessEvaluationList. +func (in *NodeReadinessEvaluationList) DeepCopy() *NodeReadinessEvaluationList { + if in == nil { + return nil + } + out := new(NodeReadinessEvaluationList) + in.DeepCopyInto(out) + return out +} + +// DeepCopyObject is an autogenerated deepcopy function, copying the receiver, creating a new runtime.Object. +func (in *NodeReadinessEvaluationList) DeepCopyObject() runtime.Object { + if c := in.DeepCopy(); c != nil { + return c + } + return nil +} + +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *NodeReadinessEvaluationSpec) DeepCopyInto(out *NodeReadinessEvaluationSpec) { + *out = *in +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new NodeReadinessEvaluationSpec. +func (in *NodeReadinessEvaluationSpec) DeepCopy() *NodeReadinessEvaluationSpec { + if in == nil { + return nil + } + out := new(NodeReadinessEvaluationSpec) + in.DeepCopyInto(out) + return out +} + +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *NodeReadinessEvaluationStatus) DeepCopyInto(out *NodeReadinessEvaluationStatus) { + *out = *in + if in.Conditions != nil { + in, out := &in.Conditions, &out.Conditions + *out = make([]v1.Condition, len(*in)) + for i := range *in { + (*in)[i].DeepCopyInto(&(*out)[i]) + } + } + if in.Rules != nil { + in, out := &in.Rules, &out.Rules + *out = make([]RuleEvaluation, len(*in)) + for i := range *in { + (*in)[i].DeepCopyInto(&(*out)[i]) + } + } +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new NodeReadinessEvaluationStatus. +func (in *NodeReadinessEvaluationStatus) DeepCopy() *NodeReadinessEvaluationStatus { + if in == nil { + return nil + } + out := new(NodeReadinessEvaluationStatus) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *NodeReadinessRule) DeepCopyInto(out *NodeReadinessRule) { *out = *in @@ -241,3 +345,40 @@ func (in *NodeReadinessRuleStatus) DeepCopy() *NodeReadinessRuleStatus { in.DeepCopyInto(out) return out } + +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *RuleEvaluation) DeepCopyInto(out *RuleEvaluation) { + *out = *in + if in.ReadinessConditions != nil { + in, out := &in.ReadinessConditions, &out.ReadinessConditions + *out = make([]ConditionEvaluationResult, len(*in)) + copy(*out, *in) + } + in.LastEvaluationTime.DeepCopyInto(&out.LastEvaluationTime) + if in.FirstEvaluatedAt != nil { + in, out := &in.FirstEvaluatedAt, &out.FirstEvaluatedAt + *out = (*in).DeepCopy() + } + if in.TaintObservedAt != nil { + in, out := &in.TaintObservedAt, &out.TaintObservedAt + *out = (*in).DeepCopy() + } + if in.TaintAddedAt != nil { + in, out := &in.TaintAddedAt, &out.TaintAddedAt + *out = (*in).DeepCopy() + } + if in.TaintRemovedAt != nil { + in, out := &in.TaintRemovedAt, &out.TaintRemovedAt + *out = (*in).DeepCopy() + } +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new RuleEvaluation. +func (in *RuleEvaluation) DeepCopy() *RuleEvaluation { + if in == nil { + return nil + } + out := new(RuleEvaluation) + in.DeepCopyInto(out) + return out +} diff --git a/cmd/main.go b/cmd/main.go index 7079d49c..36b44bdb 100644 --- a/cmd/main.go +++ b/cmd/main.go @@ -58,19 +58,20 @@ var ( scheme = runtime.NewScheme() setupLog = ctrl.Log.WithName("setup") - metricsAddr string - enableLeaderElection bool - probeAddr string - enableWebhook bool - metricsSecure bool - metricsCertDir string - leaderElectionNamespace string - enableNodeStateMetrics bool - pprofAddr string - kubeAPIQPS float64 - kubeAPIBurst int - nodeConcurrentReconciles int - ruleConcurrentReconciles int + metricsAddr string + enableLeaderElection bool + probeAddr string + enableWebhook bool + metricsSecure bool + metricsCertDir string + leaderElectionNamespace string + enableNodeStateMetrics bool + enableNodeReadinessEvaluation bool + pprofAddr string + kubeAPIQPS float64 + kubeAPIBurst int + nodeConcurrentReconciles int + ruleConcurrentReconciles int ) func init() { @@ -98,7 +99,10 @@ func main() { "Enable validation webhook. Requires TLS certificates to be configured.") flag.StringVar(&leaderElectionNamespace, "leader-election-namespace", "", "The namespace where the leader election resource will be created.") flag.BoolVar(&enableNodeStateMetrics, "enable-node-state-metrics", false, - "Enable aggregate node state metrics on node updates)") + "Enable aggregate node state metrics on node updates.") + flag.BoolVar(&enableNodeReadinessEvaluation, "enable-node-readiness-evaluation", false, + "Enable the NodeReadinessEvaluation controller. When set, one NRE object is created "+ + "per node and kept up to date with the evaluated state of all applicable rules.") flag.Float64Var(&kubeAPIQPS, "kube-api-qps", defaultKubeAPIQPS, "Maximum queries per second to the API server from this client. "+ "Raise together with --kube-api-burst on large clusters.") @@ -189,6 +193,19 @@ func main() { os.Exit(1) } + if enableNodeReadinessEvaluation { + nreReconciler := &controller.NodeReadinessEvaluationReconciler{ + Client: mgr.GetClient(), + Scheme: mgr.GetScheme(), + Controller: readinessController, + } + if err := nreReconciler.SetupWithManager(mgr); err != nil { + setupLog.Error(err, "unable to create controller", "controller", "NodeReadinessEvaluation") + os.Exit(1) + } + setupLog.Info("NodeReadinessEvaluation controller enabled") + } + // Setup webhook (conditional based on flag) if enableWebhook { nodeReadinessWebhook := webhook.NewNodeReadinessRuleWebhook(mgr.GetClient()) diff --git a/config/crd/bases/readiness.node.x-k8s.io_nodereadinessevaluations.yaml b/config/crd/bases/readiness.node.x-k8s.io_nodereadinessevaluations.yaml new file mode 100644 index 00000000..e8e55933 --- /dev/null +++ b/config/crd/bases/readiness.node.x-k8s.io_nodereadinessevaluations.yaml @@ -0,0 +1,336 @@ +--- +apiVersion: apiextensions.k8s.io/v1 +kind: CustomResourceDefinition +metadata: + annotations: + controller-gen.kubebuilder.io/version: v0.19.0 + name: nodereadinessevaluations.readiness.node.x-k8s.io +spec: + group: readiness.node.x-k8s.io + names: + kind: NodeReadinessEvaluation + listKind: NodeReadinessEvaluationList + plural: nodereadinessevaluations + shortNames: + - nre + singular: nodereadinessevaluation + scope: Cluster + versions: + - additionalPrinterColumns: + - description: The name of the target Node. + jsonPath: .spec.nodeName + name: Node + type: string + - description: The overall readiness evaluation state of the node. + jsonPath: .status.state + name: State + type: string + - description: The age of this resource. + jsonPath: .metadata.creationTimestamp + name: Age + type: date + name: v1alpha1 + schema: + openAPIV3Schema: + description: |- + NodeReadinessEvaluation is the Schema for the NodeReadinessEvaluations API. + Each instance maps 1:1 to a Node and folds the outcomes of all applicable + NodeReadinessRules for that node into a single object. + An ownerReference to the corresponding Node is set for automatic garbage + collection when the node is deleted. + properties: + apiVersion: + description: |- + APIVersion defines the versioned schema of this representation of an object. + Servers should convert recognized schemas to the latest internal value, and + may reject unrecognized values. + More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#resources + type: string + kind: + description: |- + Kind is a string value representing the REST resource this object represents. + Servers may infer this from the endpoint the client submits requests to. + Cannot be updated. + In CamelCase. + More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#types-kinds + type: string + metadata: + type: object + spec: + description: spec defines the desired state of NodeReadinessEvaluation. + properties: + nodeName: + description: |- + nodeName specifies the exact name of the target Kubernetes Node. + This object establishes a strict 1:1 relationship with the specified node, + acting as the single source of truth for all rules and statuses applied to it. + Because it binds this resource to a specific physical or virtual machine, it cannot be changed once set. + + The validation constraints enforce standard Kubernetes resource naming + (RFC 1123 DNS Subdomain format), as defined in upstream apimachinery: + https://github.com/kubernetes/apimachinery/blob/master/pkg/util/validation/validation.go#L198 + maxLength: 253 + minLength: 1 + pattern: ^[a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*$ + type: string + x-kubernetes-validations: + - message: nodeName is immutable and cannot be changed once set + rule: self == oldSelf + required: + - nodeName + type: object + status: + description: status defines the observed state of NodeReadinessEvaluation. + minProperties: 1 + properties: + conditions: + description: |- + conditions represent the latest available observations of the node's readiness evaluation state. + Known condition types are: + - "Evaluated": indicates whether the controller successfully evaluated all rules without errors. + - "Available": indicates whether the node has satisfied all rules, has zero taints applied, and is available for scheduling. + items: + description: Condition contains details for one aspect of the current + state of this API Resource. + properties: + lastTransitionTime: + description: |- + lastTransitionTime is the last time the condition transitioned from one status to another. + This should be when the underlying condition changed. If that is not known, then using the time when the API field changed is acceptable. + format: date-time + type: string + message: + description: |- + message is a human readable message indicating details about the transition. + This may be an empty string. + maxLength: 32768 + type: string + observedGeneration: + description: |- + observedGeneration represents the .metadata.generation that the condition was set based upon. + For instance, if .metadata.generation is currently 12, but the .status.conditions[x].observedGeneration is 9, the condition is out of date + with respect to the current state of the instance. + format: int64 + minimum: 0 + type: integer + reason: + description: |- + reason contains a programmatic identifier indicating the reason for the condition's last transition. + Producers of specific condition types may define expected values and meanings for this field, + and whether the values are considered a guaranteed API. + The value should be a CamelCase string. + This field may not be empty. + maxLength: 1024 + minLength: 1 + pattern: ^[A-Za-z]([A-Za-z0-9_,:]*[A-Za-z0-9_])?$ + type: string + status: + description: status of the condition, one of True, False, Unknown. + enum: + - "True" + - "False" + - Unknown + type: string + type: + description: type of condition in CamelCase or in foo.example.com/CamelCase. + maxLength: 316 + pattern: ^([a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*/)?(([A-Za-z0-9][-A-Za-z0-9_.]*)?[A-Za-z0-9])$ + type: string + required: + - lastTransitionTime + - message + - reason + - status + - type + type: object + maxItems: 8 + type: array + x-kubernetes-list-map-keys: + - type + x-kubernetes-list-type: map + rules: + description: |- + rules contains the evaluation outcomes for all rules applicable to this node. + Each entry is keyed by ruleName, allowing independent per-rule updates by + parallel rule-workers without last-write-wins conflicts (listType=map). + items: + description: RuleEvaluation defines the outcome of evaluating a + single NodeReadinessRule against this Node. + properties: + firstEvaluatedAt: + description: firstEvaluatedAt is the time the rule was first + assessed against this node. + format: date-time + type: string + lastEvaluationTime: + description: lastEvaluationTime records the exact moment the + controller most recently assessed this rule. + format: date-time + type: string + message: + description: message is a comprehensive, human-readable explanation + providing further context. + maxLength: 10240 + minLength: 1 + type: string + readinessConditions: + description: |- + readinessConditions provides a detailed breakdown of each condition evaluation + for this Node. This allows for granular debugging of which specific criteria passed/failed. + items: + description: |- + ConditionEvaluationResult provides a detailed report of the comparison between + the Node's observed condition and the rule's requirement. + properties: + currentStatus: + description: currentStatus is the actual status value + observed on the Node, one of True, False, Unknown. + enum: + - "True" + - "False" + - Unknown + type: string + defaultStatus: + description: |- + defaultStatus is the status a condition is evaluated to if the condition + is not found in a node. Reflects the defaultStatus configured in the rule + spec. + enum: + - "True" + - "False" + - Unknown + type: string + requiredStatus: + description: requiredStatus is the status value defined + in the rule that must be matched, one of True, False, + Unknown. + enum: + - "True" + - "False" + - Unknown + type: string + type: + description: type corresponds to the Node condition type + being evaluated. + maxLength: 316 + minLength: 1 + type: string + required: + - currentStatus + - requiredStatus + - type + type: object + maxItems: 32 + type: array + x-kubernetes-list-map-keys: + - type + x-kubernetes-list-type: map + reason: + description: reason contains a concise, machine-readable string + detailing the primary outcome. + maxLength: 256 + minLength: 1 + type: string + ruleName: + description: |- + ruleName is the name of the NodeReadinessRule. + This field is the map key for status.rules (listType=map) and must always be present. + maxLength: 253 + minLength: 1 + type: string + ruleStatus: + description: ruleStatus indicates the overall outcome of the + rule's criteria against the Node. + enum: + - Satisfied + - Unsatisfied + type: string + ruleUID: + description: |- + ruleUID is the UID of the NodeReadinessRule. + If the rule is deleted and recreated with the same name, the UID will differ, + allowing the controller to detect and replace stale evaluation entries. + type: string + taintAddedAt: + description: |- + taintAddedAt is the time NRC itself first applied the taint to the node in the + current taint lifecycle (i.e., since the last removal). This field is nil when + the taint was pre-existing and NRC adopted it rather than creating it. + The value is set once on the Absent→Present transition and carried forward on + subsequent reconciles; it is not the timestamp of the most recent reconcile. + Use taintObservedAt for "how long has the node been blocked"; use this field + to measure NRC's own apply latency (taintAddedAt - firstEvaluatedAt). + format: date-time + type: string + taintEffect: + description: |- + taintEffect is the effect of the taint managed by this rule, stamped at + evaluation time so this entry is self-contained without requiring a lookup + of the rule. Matches rule.spec.taint.effect. + enum: + - NoSchedule + - PreferNoSchedule + - NoExecute + type: string + taintKey: + description: |- + taintKey is the key of the taint managed by this rule, stamped at evaluation + time so this entry is self-contained without requiring a lookup of the rule. + Matches rule.spec.taint.key. + maxLength: 253 + minLength: 1 + type: string + taintObservedAt: + description: |- + taintObservedAt is the time NRC first observed the taint present on the node, + regardless of whether NRC applied it or it was pre-existing (e.g. via --register-with-taints). + This marks the beginning of the node being blocked by this rule and is the correct + start time for computing time-to-unblock SLIs. + format: date-time + type: string + taintRemovedAt: + description: |- + taintRemovedAt is the time the controller successfully removed the taint + after the node satisfied all conditions. + format: date-time + type: string + taintStatus: + description: taintStatus reflects the observed state of the + rule's specified taint on the Node (Present/Absent). + enum: + - Present + - Absent + type: string + required: + - lastEvaluationTime + - ruleName + - ruleStatus + - ruleUID + - taintEffect + - taintKey + - taintStatus + type: object + maxItems: 100 + type: array + x-kubernetes-list-map-keys: + - ruleName + x-kubernetes-list-type: map + state: + description: |- + state indicates the overall readiness state of the node based on all applicable rules. + It acts as a top-level health indicator for this node's readiness evaluation. + enum: + - Available + - NotAvailable + type: string + type: object + required: + - spec + type: object + selectableFields: + - jsonPath: .spec.nodeName + - jsonPath: .status.state + served: true + storage: true + subresources: + status: {} diff --git a/config/manager/manager.yaml b/config/manager/manager.yaml index b21436b1..3636c2f1 100644 --- a/config/manager/manager.yaml +++ b/config/manager/manager.yaml @@ -1,4 +1,4 @@ -apiVersion: v1 +config/manager/manager.yaml apiVersion: v1 kind: Namespace metadata: labels: @@ -76,6 +76,7 @@ spec: args: - --leader-elect - --health-probe-bind-address=:8081 + - --enable-node-readiness-evaluation=true image: controller:latest imagePullPolicy: IfNotPresent name: manager diff --git a/config/rbac/role.yaml b/config/rbac/role.yaml index fac24a20..4df190bb 100644 --- a/config/rbac/role.yaml +++ b/config/rbac/role.yaml @@ -31,8 +31,10 @@ rules: - apiGroups: - readiness.node.x-k8s.io resources: - - nodereadinessrules + - nodereadinessevaluations verbs: + - create + - delete - get - list - patch @@ -41,14 +43,25 @@ rules: - apiGroups: - readiness.node.x-k8s.io resources: - - nodereadinessrules/finalizers + - nodereadinessevaluations/status + - nodereadinessrules/status verbs: + - get + - patch - update - apiGroups: - readiness.node.x-k8s.io resources: - - nodereadinessrules/status + - nodereadinessrules verbs: - get + - list - patch - update + - watch +- apiGroups: + - readiness.node.x-k8s.io + resources: + - nodereadinessrules/finalizers + verbs: + - update diff --git a/internal/controller/nodereadinessevaluation_controller.go b/internal/controller/nodereadinessevaluation_controller.go new file mode 100644 index 00000000..112d61bc --- /dev/null +++ b/internal/controller/nodereadinessevaluation_controller.go @@ -0,0 +1,425 @@ +/* +Copyright The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package controller + +import ( + "context" + "fmt" + "strings" + + corev1 "k8s.io/api/core/v1" + apierrors "k8s.io/apimachinery/pkg/api/errors" + "k8s.io/apimachinery/pkg/api/meta" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/types" + ctrl "sigs.k8s.io/controller-runtime" + "sigs.k8s.io/controller-runtime/pkg/builder" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" + "sigs.k8s.io/controller-runtime/pkg/event" + "sigs.k8s.io/controller-runtime/pkg/handler" + "sigs.k8s.io/controller-runtime/pkg/predicate" + "sigs.k8s.io/controller-runtime/pkg/reconcile" + + readinessv1alpha1 "sigs.k8s.io/node-readiness-controller/api/v1alpha1" +) + +// NodeReadinessEvaluationReconciler is an independent reconciler that maintains +// one NodeReadinessEvaluation object per Node. It watches Nodes and +// NodeReadinessRules and re-evaluates the full rule set for the affected node +// on every relevant change. It shares the rule cache owned by +// RuleReadinessController but never writes to Node taints or NRR status — +// those remain the sole responsibility of NodeReconciler / RuleReconciler. +type NodeReadinessEvaluationReconciler struct { + client.Client + Scheme *runtime.Scheme + Controller *RuleReadinessController +} + +// SetupWithManager wires the reconciler to watch: +// - Node objects (conditions, taints, labels) +// - NodeReadinessRule objects (enqueues all nodes matching the changed rule) +func (r *NodeReadinessEvaluationReconciler) SetupWithManager(mgr ctrl.Manager) error { + return ctrl.NewControllerManagedBy(mgr). + Named("nodereadinessevaluation"). + // Primary watch: a node change triggers reconcile for that node's NRE. + For(&corev1.Node{}, builder.WithPredicates(predicate.Funcs{ + CreateFunc: func(e event.CreateEvent) bool { return true }, + UpdateFunc: func(e event.UpdateEvent) bool { + oldNode := e.ObjectOld.(*corev1.Node) + newNode := e.ObjectNew.(*corev1.Node) + return !conditionsEqual(oldNode.Status.Conditions, newNode.Status.Conditions) || + !taintsEqual(oldNode.Spec.Taints, newNode.Spec.Taints) || + !labelsEqual(oldNode.Labels, newNode.Labels) + }, + DeleteFunc: func(e event.DeleteEvent) bool { return false }, // GC handles deletion via ownerRef + GenericFunc: func(e event.GenericEvent) bool { return false }, + })). + // Secondary watch: a rule change re-evaluates every node in the cluster + // that could be affected. We map the rule event to a list of node requests. + Watches( + &readinessv1alpha1.NodeReadinessRule{}, + handler.EnqueueRequestsFromMapFunc(r.ruleToNodeRequests), + builder.WithPredicates(predicate.GenerationChangedPredicate{}), + ). + Complete(r) +} + +// ruleToNodeRequests maps a NodeReadinessRule event to reconcile requests for +// only the Nodes that match the rule's nodeSelector. Filtering here avoids +// enqueueing the full cluster on every rule change — on a 5k-node cluster a +// rule targeting 200 nodes produces 200 requests, not 5,000. +// +// nodeSelector is immutable enforeced via CEL validateion on the spec, so a changing +// selector can never silently leave stale NRE entries behind. +func (r *NodeReadinessEvaluationReconciler) ruleToNodeRequests(ctx context.Context, obj client.Object) []reconcile.Request { + log := ctrl.LoggerFrom(ctx) + + rule, ok := obj.(*readinessv1alpha1.NodeReadinessRule) + if !ok { + return nil + } + + selector, err := metav1.LabelSelectorAsSelector(&rule.Spec.NodeSelector) + if err != nil { + // Invalid selector — the rule reconciler will surface this as an error; + // nothing to enqueue here. + log.Error(err, "invalid nodeSelector on rule, skipping NRE fan-out", "rule", rule.Name) + return nil + } + + nodeList := &corev1.NodeList{} + if err := r.List(ctx, nodeList, client.MatchingLabelsSelector{Selector: selector}); err != nil { + log.Error(err, "failed to list matching nodes for NRE rule mapping", "rule", rule.Name) + return nil + } + + requests := make([]reconcile.Request, len(nodeList.Items)) + for i, node := range nodeList.Items { + requests[i] = reconcile.Request{ + NamespacedName: types.NamespacedName{Name: node.Name}, + } + } + + log.V(4).Info("Enqueuing NRE reconciles for rule change", + "rule", rule.Name, "matchingNodes", len(requests)) + return requests +} + +// +kubebuilder:rbac:groups=readiness.node.x-k8s.io,resources=nodereadinessevaluations,verbs=get;list;watch;create;update;patch;delete +// +kubebuilder:rbac:groups=readiness.node.x-k8s.io,resources=nodereadinessevaluations/status,verbs=get;update;patch + +// Reconcile evaluates all applicable rules for the given node and writes the +// result into the corresponding NodeReadinessEvaluation object. +func (r *NodeReadinessEvaluationReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) { + log := ctrl.LoggerFrom(ctx) + log.V(4).Info("Reconciling NodeReadinessEvaluation", "node", req.Name) + + // Fetch the Node. + node := &corev1.Node{} + if err := r.Get(ctx, req.NamespacedName, node); err != nil { + return ctrl.Result{}, client.IgnoreNotFound(err) + } + + // Fetch or create the NRE object. + nre, err := r.ensureNRE(ctx, node) + if err != nil { + return ctrl.Result{}, err + } + + // Evaluate all applicable rules from the shared cache. + applicableRules := r.Controller.getApplicableRulesForNode(ctx, node) + log.V(4).Info("Evaluating rules for NRE", "node", node.Name, "ruleCount", len(applicableRules)) + + // Snapshot the previous rules slice for timestamp carry-forward BEFORE + // clearing it. buildRuleEvaluation reads from this snapshot via nre.Status.Rules, + // so it must not be cleared until after all evaluations are complete. + prevRules := make([]readinessv1alpha1.RuleEvaluation, len(nre.Status.Rules)) + copy(prevRules, nre.Status.Rules) + + patch := client.MergeFrom(nre.DeepCopy()) + + // Rebuild the full rules slice from scratch on every reconcile. + // listType=atomic means the controller owns the whole slice. + newRules := make([]readinessv1alpha1.RuleEvaluation, 0, len(applicableRules)) + for _, rule := range applicableRules { + if !rule.DeletionTimestamp.IsZero() || rule.Spec.DryRun { + continue + } + ruleEval := r.buildRuleEvaluation(ctx, node, rule, prevRules) + newRules = append(newRules, ruleEval) + } + nre.Status.Rules = newRules + + recomputeNREStatus(&nre.Status) + + if err := r.Status().Patch(ctx, nre, patch); err != nil { + return ctrl.Result{}, fmt.Errorf("failed to patch NRE status %s: %w", node.Name, err) + } + + log.V(4).Info("Reconciled NRE", "node", node.Name, "rules", len(nre.Status.Rules), "state", nre.Status.State) + return ctrl.Result{}, nil +} + +// ensureNRE fetches the NRE for the node, creating it (with ownerReference) if +// it does not exist yet. Returns the current object ready for status patching. +func (r *NodeReadinessEvaluationReconciler) ensureNRE(ctx context.Context, node *corev1.Node) (*readinessv1alpha1.NodeReadinessEvaluation, error) { + nre := &readinessv1alpha1.NodeReadinessEvaluation{} + err := r.Get(ctx, client.ObjectKey{Name: node.Name}, nre) + + switch { + case apierrors.IsNotFound(err): + nre = &readinessv1alpha1.NodeReadinessEvaluation{ + ObjectMeta: metav1.ObjectMeta{ + Name: node.Name, + }, + Spec: readinessv1alpha1.NodeReadinessEvaluationSpec{ + NodeName: node.Name, + }, + } + if ownerErr := controllerutil.SetOwnerReference(node, nre, r.Scheme); ownerErr != nil { + return nil, fmt.Errorf("failed to set owner reference on NRE %s: %w", node.Name, ownerErr) + } + if createErr := r.Create(ctx, nre); createErr != nil { + if !apierrors.IsAlreadyExists(createErr) { + return nil, fmt.Errorf("failed to create NRE %s: %w", node.Name, createErr) + } + // A concurrent reconcile won the race — re-fetch. + if getErr := r.Get(ctx, client.ObjectKey{Name: node.Name}, nre); getErr != nil { + return nil, fmt.Errorf("failed to get NRE %s after AlreadyExists: %w", node.Name, getErr) + } + } + ctrl.LoggerFrom(ctx).V(4).Info("Created NodeReadinessEvaluation", "nre", node.Name) + + case err != nil: + return nil, fmt.Errorf("failed to get NRE %s: %w", node.Name, err) + } + + return nre, nil +} + +// buildRuleEvaluation evaluates a single rule against the node and constructs +// the RuleEvaluation entry, preserving SLI timestamps from prevRules (the +// snapshot of the previous status.rules slice taken before the rebuild started). +func (r *NodeReadinessEvaluationReconciler) buildRuleEvaluation( + ctx context.Context, + node *corev1.Node, + rule *readinessv1alpha1.NodeReadinessRule, + prevRules []readinessv1alpha1.RuleEvaluation, +) readinessv1alpha1.RuleEvaluation { + log := ctrl.LoggerFrom(ctx) + now := metav1.Now() + + // Evaluate all conditions. + allConditionsSatisfied := true + conditionResults := make([]readinessv1alpha1.ConditionEvaluationResult, 0, len(rule.Spec.Conditions)) + for _, condReq := range rule.Spec.Conditions { + effectiveStatus, conditionFound := r.Controller.getConditionStatus(node, condReq.Type, condReq.GetDefaultStatus()) + satisfied := effectiveStatus == condReq.RequiredStatus + if !satisfied { + allConditionsSatisfied = false + } + observedStatus := effectiveStatus + if !conditionFound { + observedStatus = corev1.ConditionUnknown + } + conditionResults = append(conditionResults, readinessv1alpha1.ConditionEvaluationResult{ + Type: condReq.Type, + CurrentStatus: observedStatus, + RequiredStatus: condReq.RequiredStatus, + DefaultStatus: condReq.GetDefaultStatus(), + }) + } + + ruleStatus := readinessv1alpha1.RuleStatusSatisfied + if !allConditionsSatisfied { + ruleStatus = readinessv1alpha1.RuleStatusUnsatisfied + } + + taintPresent := r.Controller.hasTaintBySpec(node, rule.Spec.Taint) + taintStatus := readinessv1alpha1.TaintStatusAbsent + if taintPresent { + taintStatus = readinessv1alpha1.TaintStatusPresent + } + + reason, message := buildRuleEvalReasonMessage(ruleStatus, taintStatus, rule.Spec.Taint.Key, conditionResults) + + log.V(4).Info("Rule evaluation for NRE", + "node", node.Name, "rule", rule.Name, + "ruleStatus", ruleStatus, "taintStatus", taintStatus) + + // Carry forward timestamps from the previous evaluation entry if present. + prev := findPreviousRuleEvaluation(prevRules, rule.Name) + + eval := readinessv1alpha1.RuleEvaluation{ + RuleName: rule.Name, + RuleUID: rule.GetUID(), + RuleStatus: ruleStatus, + TaintStatus: taintStatus, + TaintKey: rule.Spec.Taint.Key, + TaintEffect: rule.Spec.Taint.Effect, + Reason: reason, + Message: message, + ReadinessConditions: conditionResults, + LastEvaluationTime: now, + } + + // FirstEvaluatedAt: set once, carried forward on subsequent evaluations. + if prev != nil && prev.RuleUID == rule.GetUID() { + eval.FirstEvaluatedAt = prev.FirstEvaluatedAt + } else { + // No previous entry, or the rule was deleted and recreated (UID changed). + eval.FirstEvaluatedAt = &now + } + + // Taint timestamp transitions — four cases covering every state combination. + // TaintRemovedAt is carried forward once set so the SLI is always queryable, + // not just on the exact reconcile cycle where removal happened. + if prev != nil && prev.RuleUID == rule.GetUID() { + prevTaintPresent := prev.TaintStatus == readinessv1alpha1.TaintStatusPresent + switch { + case taintPresent && prevTaintPresent: + // State unchanged: taint still active — carry forward all timestamps. + eval.TaintObservedAt = prev.TaintObservedAt + eval.TaintAddedAt = prev.TaintAddedAt + case taintPresent && !prevTaintPresent: + // Transition: Absent → Present — NRC just added the taint. + eval.TaintObservedAt = &now + eval.TaintAddedAt = &now + case !taintPresent && prevTaintPresent: + // Transition: Present → Absent — NRC just removed the taint. + eval.TaintRemovedAt = &now + default: + // State unchanged: taint still absent — carry forward historical removal time. + eval.TaintRemovedAt = prev.TaintRemovedAt + } + } else if taintPresent { + // First evaluation for this rule (or rule recreated) and taint is already + // present — NRC is adopting a pre-existing taint. + eval.TaintObservedAt = &now + // TaintAddedAt stays nil — NRC did not add this taint. + } + + return eval +} + +// buildRuleEvalReasonMessage derives the Reason and Message for a RuleEvaluation +// based on the rule outcome and the per-condition results. Both fields are +// human-readable diagnostic aids — Reason is machine-readable (no spaces), +// Message is the operator-facing explanation. +func buildRuleEvalReasonMessage( + ruleStatus readinessv1alpha1.RuleStatus, + taintStatus readinessv1alpha1.TaintStatus, + taintKey string, + conditions []readinessv1alpha1.ConditionEvaluationResult, +) (reason, message string) { + switch { + case ruleStatus == readinessv1alpha1.RuleStatusSatisfied && taintStatus == readinessv1alpha1.TaintStatusAbsent: + return "NodeReady", fmt.Sprintf("All conditions satisfied and taint %q is absent.", taintKey) + + case ruleStatus == readinessv1alpha1.RuleStatusSatisfied && taintStatus == readinessv1alpha1.TaintStatusPresent: + return "TaintPendingRemoval", fmt.Sprintf("All conditions satisfied; taint %q is still present and pending removal.", taintKey) + + case ruleStatus == readinessv1alpha1.RuleStatusUnsatisfied && taintStatus == readinessv1alpha1.TaintStatusPresent: + var unsatisfied []string + for _, c := range conditions { + if c.CurrentStatus != c.RequiredStatus { + unsatisfied = append(unsatisfied, fmt.Sprintf("%s (current=%s, required=%s)", c.Type, c.CurrentStatus, c.RequiredStatus)) + } + } + return "ConditionsNotSatisfied", fmt.Sprintf( + "Taint %q is active. Unsatisfied condition(s): %s.", + taintKey, strings.Join(unsatisfied, "; "), + ) + + default: + // Unmatched + Absent: conditions not met but taint is also gone (e.g. manually removed). + var unsatisfied []string + for _, c := range conditions { + if c.CurrentStatus != c.RequiredStatus { + unsatisfied = append(unsatisfied, fmt.Sprintf("%s (current=%s, required=%s)", c.Type, c.CurrentStatus, c.RequiredStatus)) + } + } + return "ConditionsNotSatisfied", fmt.Sprintf( + "Taint %q is absent but conditions are not satisfied: %s.", + taintKey, strings.Join(unsatisfied, "; "), + ) + } +} + +// findPreviousRuleEvaluation returns the existing RuleEvaluation entry for the +// given rule name in the previous rules snapshot, or nil if not found. +func findPreviousRuleEvaluation(prevRules []readinessv1alpha1.RuleEvaluation, ruleName string) *readinessv1alpha1.RuleEvaluation { + for i := range prevRules { + if prevRules[i].RuleName == ruleName { + return &prevRules[i] + } + } + return nil +} + +// recomputeNREStatus derives status.State and status.Conditions +// from the current rules slice. Called after every full rebuild so all top-level +// fields stay consistent. +func recomputeNREStatus(status *readinessv1alpha1.NodeReadinessEvaluationStatus) { + var activeTaints int32 + + for _, r := range status.Rules { + if r.TaintStatus == readinessv1alpha1.TaintStatusPresent { + activeTaints++ + } + } + + if activeTaints > 0 { + status.State = readinessv1alpha1.NodeEvaluationStateNotAvailable + } else { + status.State = readinessv1alpha1.NodeEvaluationStateAvailable + } + + // "Evaluated" — did the controller successfully complete a full evaluation pass? + // Evaluation is pure in-memory (no I/O), so this is always true after Reconcile returns. + meta.SetStatusCondition(&status.Conditions, metav1.Condition{ + Type: "Evaluated", + Status: metav1.ConditionTrue, + Reason: "EvaluationSuccessful", + Message: "All applicable rules were evaluated successfully.", + LastTransitionTime: metav1.Now(), + }) + + // "Available" — does the node satisfy all rules with zero active taints? + availableCond := metav1.Condition{ + Type: "Available", + Status: metav1.ConditionTrue, + Reason: "NodeAvailable", + Message: "Node satisfies all rules, has zero active taints, and is available for scheduling.", + LastTransitionTime: metav1.Now(), + } + if activeTaints > 0 { + var blockingRules []string + for _, r := range status.Rules { + if r.TaintStatus == readinessv1alpha1.TaintStatusPresent { + blockingRules = append(blockingRules, r.RuleName) + } + } + availableCond.Status = metav1.ConditionFalse + availableCond.Reason = "TaintsActive" + availableCond.Message = fmt.Sprintf("Node is blocked by %d active taint(s) from rule(s): %s.", + activeTaints, strings.Join(blockingRules, ", ")) + } + meta.SetStatusCondition(&status.Conditions, availableCond) +} diff --git a/internal/controller/nodereadinessevaluation_controller_test.go b/internal/controller/nodereadinessevaluation_controller_test.go new file mode 100644 index 00000000..f94845c3 --- /dev/null +++ b/internal/controller/nodereadinessevaluation_controller_test.go @@ -0,0 +1,754 @@ +/* +Copyright The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package controller + +import ( + "time" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + corev1 "k8s.io/api/core/v1" + apierrors "k8s.io/apimachinery/pkg/api/errors" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/types" + "k8s.io/client-go/kubernetes/fake" + "k8s.io/client-go/tools/events" + "sigs.k8s.io/controller-runtime/pkg/reconcile" + + nodereadinessiov1alpha1 "sigs.k8s.io/node-readiness-controller/api/v1alpha1" +) + +// nreNamespacedName returns the NamespacedName for a NodeReadinessEvaluation +// (cluster-scoped, so Namespace is always empty). +func nreNamespacedName(nodeName string) types.NamespacedName { + return types.NamespacedName{Name: nodeName} +} + +var _ = Describe("NodeReadinessEvaluation Controller", func() { + const ( + nreTestTimeout = 10 * time.Second + nreTestInterval = 100 * time.Millisecond + + taintKey = "readiness.k8s.io/nre-test" + conditionType = "NRETestCondition" + ) + + // makeRule returns a basic continuous NodeReadinessRule that targets nodes + // with label env=nre-test. + makeRule := func(name string) *nodereadinessiov1alpha1.NodeReadinessRule { + return &nodereadinessiov1alpha1.NodeReadinessRule{ + ObjectMeta: metav1.ObjectMeta{Name: name}, + Spec: nodereadinessiov1alpha1.NodeReadinessRuleSpec{ + EnforcementMode: nodereadinessiov1alpha1.EnforcementModeContinuous, + Conditions: []nodereadinessiov1alpha1.ConditionRequirement{ + {Type: conditionType, RequiredStatus: corev1.ConditionTrue}, + }, + Taint: corev1.Taint{ + Key: taintKey, + Effect: corev1.TaintEffectNoSchedule, + }, + NodeSelector: metav1.LabelSelector{ + MatchLabels: map[string]string{"env": "nre-test"}, + }, + }, + } + } + + // makeNode returns a node with label env=nre-test and the given taints. + makeNode := func(name string, taints []corev1.Taint, condStatus corev1.ConditionStatus) *corev1.Node { + return &corev1.Node{ + ObjectMeta: metav1.ObjectMeta{ + Name: name, + Labels: map[string]string{"env": "nre-test"}, + }, + Spec: corev1.NodeSpec{Taints: taints}, + Status: corev1.NodeStatus{ + Conditions: []corev1.NodeCondition{ + {Type: conditionType, Status: condStatus}, + }, + }, + } + } + + // sharedSetup wires up a RuleReadinessController and a + // NodeReadinessEvaluationReconciler backed by the envtest k8sClient. + // Returns both reconcilers and a cleanup function. + sharedSetup := func() (*RuleReadinessController, *NodeReadinessEvaluationReconciler) { + rc := &RuleReadinessController{ + Client: k8sClient, + Scheme: k8sClient.Scheme(), + clientset: fake.NewSimpleClientset(), + ruleCache: make(map[string]*nodereadinessiov1alpha1.NodeReadinessRule), + EventRecorder: events.NewFakeRecorder(32), + } + nreR := &NodeReadinessEvaluationReconciler{ + Client: k8sClient, + Scheme: k8sClient.Scheme(), + Controller: rc, + } + return rc, nreR + } + + // reconcileNRE triggers a single NRE reconcile for the named node. + reconcileNRE := func(nreR *NodeReadinessEvaluationReconciler, nodeName string) { + GinkgoHelper() + _, err := nreR.Reconcile(ctx, reconcile.Request{ + NamespacedName: types.NamespacedName{Name: nodeName}, + }) + Expect(err).NotTo(HaveOccurred()) + } + + // getNRE fetches the current NRE for the given node name. + getNRE := func(nodeName string) *nodereadinessiov1alpha1.NodeReadinessEvaluation { + GinkgoHelper() + nre := &nodereadinessiov1alpha1.NodeReadinessEvaluation{} + Expect(k8sClient.Get(ctx, nreNamespacedName(nodeName), nre)).To(Succeed()) + return nre + } + + // Suite A — Fresh cluster: NRE enabled from the start. + + Context("Suite A — fresh cluster with NRE enabled", func() { + var ( + rc *RuleReadinessController + nreR *NodeReadinessEvaluationReconciler + node *corev1.Node + rule *nodereadinessiov1alpha1.NodeReadinessRule + ) + + BeforeEach(func() { + rc, nreR = sharedSetup() + + node = makeNode("nre-a-node", []corev1.Taint{ + {Key: taintKey, Effect: corev1.TaintEffectNoSchedule}, + }, corev1.ConditionFalse) + rule = makeRule("nre-a-rule") + + Expect(k8sClient.Create(ctx, node)).To(Succeed()) + Expect(k8sClient.Create(ctx, rule)).To(Succeed()) + rc.updateRuleCache(ctx, rule) + }) + + AfterEach(func() { + _ = k8sClient.Delete(ctx, node) + + updatedRule := &nodereadinessiov1alpha1.NodeReadinessRule{} + if err := k8sClient.Get(ctx, types.NamespacedName{Name: rule.Name}, updatedRule); err == nil { + updatedRule.Finalizers = nil + _ = k8sClient.Update(ctx, updatedRule) + _ = k8sClient.Delete(ctx, updatedRule) + } + Eventually(func() bool { + return apierrors.IsNotFound( + k8sClient.Get(ctx, types.NamespacedName{Name: rule.Name}, + &nodereadinessiov1alpha1.NodeReadinessRule{})) + }, nreTestTimeout).Should(BeTrue()) + + // Delete the NRE if it still exists (GC may not run in envtest). + nre := &nodereadinessiov1alpha1.NodeReadinessEvaluation{} + if err := k8sClient.Get(ctx, nreNamespacedName(node.Name), nre); err == nil { + _ = k8sClient.Delete(ctx, nre) + } + + rc.removeRuleFromCache(ctx, rule.Name) + }) + + It("A1 — creates NRE with correct spec.nodeName and ownerReference on first reconcile", func() { + reconcileNRE(nreR, node.Name) + + nre := getNRE(node.Name) + + By("checking spec.nodeName") + Expect(nre.Spec.NodeName).To(Equal(node.Name)) + + By("checking ownerReference points to the Node") + Expect(nre.OwnerReferences).To(HaveLen(1)) + Expect(nre.OwnerReferences[0].Kind).To(Equal("Node")) + Expect(nre.OwnerReferences[0].Name).To(Equal(node.Name)) + Expect(nre.OwnerReferences[0].UID).To(Equal(node.GetUID())) + }) + + It("A2 — state is NotAvailable and activeTaints=1 when conditions are not met", func() { + reconcileNRE(nreR, node.Name) + + nre := getNRE(node.Name) + + By("checking top-level state") + Expect(nre.Status.State).To(Equal(nodereadinessiov1alpha1.NodeEvaluationStateNotAvailable)) + + By("checking RuleEvaluation entry") + Expect(nre.Status.Rules).To(HaveLen(1)) + Expect(nre.Status.Rules[0].RuleName).To(Equal(rule.Name)) + Expect(nre.Status.Rules[0].RuleUID).To(Equal(rule.GetUID())) + Expect(nre.Status.Rules[0].RuleStatus).To(Equal(nodereadinessiov1alpha1.RuleStatusUnsatisfied)) + Expect(nre.Status.Rules[0].TaintStatus).To(Equal(nodereadinessiov1alpha1.TaintStatusPresent)) + Expect(nre.Status.Rules[0].LastEvaluationTime.IsZero()).To(BeFalse()) + }) + + It("A3 — state transitions to Available after conditions are met and taint is removed", func() { + // First reconcile: conditions not met, taint present. + reconcileNRE(nreR, node.Name) + Expect(getNRE(node.Name).Status.State).To(Equal(nodereadinessiov1alpha1.NodeEvaluationStateNotAvailable)) + + // Satisfy the condition and remove the taint (simulating NodeReconciler action). + updatedNode := &corev1.Node{} + Expect(k8sClient.Get(ctx, types.NamespacedName{Name: node.Name}, updatedNode)).To(Succeed()) + updatedNode.Status.Conditions[0].Status = corev1.ConditionTrue + Expect(k8sClient.Status().Update(ctx, updatedNode)).To(Succeed()) + + Expect(k8sClient.Get(ctx, types.NamespacedName{Name: node.Name}, updatedNode)).To(Succeed()) + updatedNode.Spec.Taints = nil + Expect(k8sClient.Update(ctx, updatedNode)).To(Succeed()) + + // Second reconcile: conditions met, taint absent. + reconcileNRE(nreR, node.Name) + + nre := getNRE(node.Name) + Expect(nre.Status.State).To(Equal(nodereadinessiov1alpha1.NodeEvaluationStateAvailable)) + Expect(nre.Status.Rules[0].RuleStatus).To(Equal(nodereadinessiov1alpha1.RuleStatusSatisfied)) + Expect(nre.Status.Rules[0].TaintStatus).To(Equal(nodereadinessiov1alpha1.TaintStatusAbsent)) + }) + + It("A4 — taintKey and taintEffect are stamped on the RuleEvaluation entry", func() { + reconcileNRE(nreR, node.Name) + + nre := getNRE(node.Name) + Expect(nre.Status.Rules).To(HaveLen(1)) + eval := nre.Status.Rules[0] + + By("taintKey matches the rule's taint key") + Expect(eval.TaintKey).To(Equal(taintKey)) + + By("taintEffect matches the rule's taint effect") + Expect(eval.TaintEffect).To(Equal(corev1.TaintEffectNoSchedule)) + }) + + It("A4b — condition evaluation breakdown is present in RuleEvaluation", func() { + reconcileNRE(nreR, node.Name) + + nre := getNRE(node.Name) + Expect(nre.Status.Rules[0].ReadinessConditions).To(HaveLen(1)) + cond := nre.Status.Rules[0].ReadinessConditions[0] + Expect(cond.Type).To(Equal(conditionType)) + Expect(cond.RequiredStatus).To(Equal(corev1.ConditionTrue)) + Expect(cond.CurrentStatus).To(Equal(corev1.ConditionFalse)) + }) + + It("A5 — Evaluated=True and Available=False conditions are set when taint is present", func() { + reconcileNRE(nreR, node.Name) + + nre := getNRE(node.Name) + By("Evaluated condition is True — no errors") + evaluated := findCondition(nre.Status.Conditions, "Evaluated") + Expect(evaluated).NotTo(BeNil()) + Expect(evaluated.Status).To(Equal(metav1.ConditionTrue)) + Expect(evaluated.Reason).To(Equal("EvaluationSuccessful")) + + By("Available condition is False — taint is active") + available := findCondition(nre.Status.Conditions, "Available") + Expect(available).NotTo(BeNil()) + Expect(available.Status).To(Equal(metav1.ConditionFalse)) + Expect(available.Reason).To(Equal("TaintsActive")) + Expect(available.Message).To(ContainSubstring("1 active taint")) + }) + + It("A6 — Available=True condition is set when all taints are cleared", func() { + // Remove taint and satisfy condition first. + updatedNode := &corev1.Node{} + Expect(k8sClient.Get(ctx, types.NamespacedName{Name: node.Name}, updatedNode)).To(Succeed()) + updatedNode.Spec.Taints = nil + Expect(k8sClient.Update(ctx, updatedNode)).To(Succeed()) + + Expect(k8sClient.Get(ctx, types.NamespacedName{Name: node.Name}, updatedNode)).To(Succeed()) + updatedNode.Status.Conditions[0].Status = corev1.ConditionTrue + Expect(k8sClient.Status().Update(ctx, updatedNode)).To(Succeed()) + + reconcileNRE(nreR, node.Name) + + nre := getNRE(node.Name) + available := findCondition(nre.Status.Conditions, "Available") + Expect(available).NotTo(BeNil()) + Expect(available.Status).To(Equal(metav1.ConditionTrue)) + Expect(available.Reason).To(Equal("NodeAvailable")) + }) + }) + + Context("Suite A (multi-rule) — multiple rules folded into one NRE", func() { + var ( + rc *RuleReadinessController + nreR *NodeReadinessEvaluationReconciler + node *corev1.Node + rule1 *nodereadinessiov1alpha1.NodeReadinessRule + rule2 *nodereadinessiov1alpha1.NodeReadinessRule + ) + + BeforeEach(func() { + rc, nreR = sharedSetup() + + node = makeNode("nre-multi-node", []corev1.Taint{ + {Key: taintKey, Effect: corev1.TaintEffectNoSchedule}, + }, corev1.ConditionFalse) + + rule1 = makeRule("nre-multi-rule-1") + + // Second rule uses a different taint key. + rule2 = makeRule("nre-multi-rule-2") + rule2.Spec.Taint = corev1.Taint{Key: "readiness.k8s.io/nre-test-2", Effect: corev1.TaintEffectNoSchedule} + + Expect(k8sClient.Create(ctx, node)).To(Succeed()) + Expect(k8sClient.Create(ctx, rule1)).To(Succeed()) + Expect(k8sClient.Create(ctx, rule2)).To(Succeed()) + rc.updateRuleCache(ctx, rule1) + rc.updateRuleCache(ctx, rule2) + }) + + AfterEach(func() { + _ = k8sClient.Delete(ctx, node) + for _, r := range []*nodereadinessiov1alpha1.NodeReadinessRule{rule1, rule2} { + updated := &nodereadinessiov1alpha1.NodeReadinessRule{} + if err := k8sClient.Get(ctx, types.NamespacedName{Name: r.Name}, updated); err == nil { + updated.Finalizers = nil + _ = k8sClient.Update(ctx, updated) + _ = k8sClient.Delete(ctx, updated) + } + Eventually(func() bool { + return apierrors.IsNotFound( + k8sClient.Get(ctx, types.NamespacedName{Name: r.Name}, + &nodereadinessiov1alpha1.NodeReadinessRule{})) + }, nreTestTimeout).Should(BeTrue()) + rc.removeRuleFromCache(ctx, r.Name) + } + nre := &nodereadinessiov1alpha1.NodeReadinessEvaluation{} + if err := k8sClient.Get(ctx, nreNamespacedName(node.Name), nre); err == nil { + _ = k8sClient.Delete(ctx, nre) + } + }) + + It("A7 — both rules are folded into the single NRE for the node", func() { + reconcileNRE(nreR, node.Name) + + nre := getNRE(node.Name) + Expect(nre.Status.Rules).To(HaveLen(2), + "both applicable rules should be present in status.rules") + + ruleNames := []string{ + nre.Status.Rules[0].RuleName, + nre.Status.Rules[1].RuleName, + } + Expect(ruleNames).To(ConsistOf(rule1.Name, rule2.Name)) + }) + + It("A8 — rule whose nodeSelector does not match is absent from NRE", func() { + // Update rule2's selector to a non-matching label. + updatedRule2 := rule2.DeepCopy() + updatedRule2.Spec.NodeSelector = metav1.LabelSelector{ + MatchLabels: map[string]string{"env": "non-matching"}, + } + rc.updateRuleCache(ctx, updatedRule2) + + reconcileNRE(nreR, node.Name) + + nre := getNRE(node.Name) + Expect(nre.Status.Rules).To(HaveLen(1), + "only the matching rule should appear in the NRE") + Expect(nre.Status.Rules[0].RuleName).To(Equal(rule1.Name)) + }) + }) + + // Suite B — Rules already running, NRE enabled retroactively. + + Context("Suite B — NRE enabled on a cluster with rules already running", func() { + var ( + rc *RuleReadinessController + nreR *NodeReadinessEvaluationReconciler + node *corev1.Node + rule *nodereadinessiov1alpha1.NodeReadinessRule + ) + + BeforeEach(func() { + rc, nreR = sharedSetup() + + // Simulate a cluster where rules and taints are already in place. + node = makeNode("nre-b-node", []corev1.Taint{ + {Key: taintKey, Effect: corev1.TaintEffectNoSchedule}, + }, corev1.ConditionFalse) + rule = makeRule("nre-b-rule") + + Expect(k8sClient.Create(ctx, node)).To(Succeed()) + Expect(k8sClient.Create(ctx, rule)).To(Succeed()) + // Rule cache is populated (simulates RuleReconciler having run). + rc.updateRuleCache(ctx, rule) + }) + + AfterEach(func() { + _ = k8sClient.Delete(ctx, node) + updatedRule := &nodereadinessiov1alpha1.NodeReadinessRule{} + if err := k8sClient.Get(ctx, types.NamespacedName{Name: rule.Name}, updatedRule); err == nil { + updatedRule.Finalizers = nil + _ = k8sClient.Update(ctx, updatedRule) + _ = k8sClient.Delete(ctx, updatedRule) + } + Eventually(func() bool { + return apierrors.IsNotFound( + k8sClient.Get(ctx, types.NamespacedName{Name: rule.Name}, + &nodereadinessiov1alpha1.NodeReadinessRule{})) + }, nreTestTimeout).Should(BeTrue()) + nre := &nodereadinessiov1alpha1.NodeReadinessEvaluation{} + if err := k8sClient.Get(ctx, nreNamespacedName(node.Name), nre); err == nil { + _ = k8sClient.Delete(ctx, nre) + } + rc.removeRuleFromCache(ctx, rule.Name) + }) + + It("B1 — first NRE reconcile on existing cluster produces correct full state", func() { + // No prior NRE exists — this is the moment the feature is enabled. + reconcileNRE(nreR, node.Name) + + nre := getNRE(node.Name) + Expect(nre.Spec.NodeName).To(Equal(node.Name)) + Expect(nre.Status.State).To(Equal(nodereadinessiov1alpha1.NodeEvaluationStateNotAvailable)) + Expect(nre.Status.Rules).To(HaveLen(1)) + Expect(nre.Status.Rules[0].RuleStatus).To(Equal(nodereadinessiov1alpha1.RuleStatusUnsatisfied)) + Expect(nre.Status.Rules[0].TaintStatus).To(Equal(nodereadinessiov1alpha1.TaintStatusPresent)) + }) + + It("B2 — subsequent reconciles are idempotent", func() { + reconcileNRE(nreR, node.Name) + firstNRE := getNRE(node.Name) + firstState := firstNRE.Status.State + firstRuleLen := len(firstNRE.Status.Rules) + + // Reconcile again without changing anything. + reconcileNRE(nreR, node.Name) + secondNRE := getNRE(node.Name) + + Expect(secondNRE.Status.State).To(Equal(firstState)) + Expect(secondNRE.Status.Rules).To(HaveLen(firstRuleLen)) + }) + }) + + // Suite C — Pre-tainted nodes (adoption / --register-with-taints). + + Context("Suite C — pre-tainted node adoption", func() { + var ( + rc *RuleReadinessController + nreR *NodeReadinessEvaluationReconciler + node *corev1.Node + rule *nodereadinessiov1alpha1.NodeReadinessRule + ) + + BeforeEach(func() { + rc, nreR = sharedSetup() + rule = makeRule("nre-c-rule") + Expect(k8sClient.Create(ctx, rule)).To(Succeed()) + rc.updateRuleCache(ctx, rule) + }) + + AfterEach(func() { + if node != nil { + _ = k8sClient.Delete(ctx, node) + } + updatedRule := &nodereadinessiov1alpha1.NodeReadinessRule{} + if err := k8sClient.Get(ctx, types.NamespacedName{Name: rule.Name}, updatedRule); err == nil { + updatedRule.Finalizers = nil + _ = k8sClient.Update(ctx, updatedRule) + _ = k8sClient.Delete(ctx, updatedRule) + } + Eventually(func() bool { + return apierrors.IsNotFound( + k8sClient.Get(ctx, types.NamespacedName{Name: rule.Name}, + &nodereadinessiov1alpha1.NodeReadinessRule{})) + }, nreTestTimeout).Should(BeTrue()) + if node != nil { + nre := &nodereadinessiov1alpha1.NodeReadinessEvaluation{} + if err := k8sClient.Get(ctx, nreNamespacedName(node.Name), nre); err == nil { + _ = k8sClient.Delete(ctx, nre) + } + } + rc.removeRuleFromCache(ctx, rule.Name) + }) + + It("C1 — TaintObservedAt is set and TaintAddedAt is nil when taint is pre-existing (adoption)", func() { + // Node registered with the taint already present (--register-with-taints). + // Conditions are not met, so NRC would normally have added the taint — + // but since it was already there, this is the adoption case. + node = makeNode("nre-c-adopted", []corev1.Taint{ + {Key: taintKey, Effect: corev1.TaintEffectNoSchedule}, + }, corev1.ConditionFalse) + Expect(k8sClient.Create(ctx, node)).To(Succeed()) + + // First reconcile: no previous NRE — taint is pre-existing. + reconcileNRE(nreR, node.Name) + + nre := getNRE(node.Name) + Expect(nre.Status.Rules).To(HaveLen(1)) + eval := nre.Status.Rules[0] + + By("TaintObservedAt must be set — NRC first observed the taint") + Expect(eval.TaintObservedAt).NotTo(BeNil(), + "TaintObservedAt should be set on adoption") + + By("TaintAddedAt must be nil — NRC did not add this taint") + Expect(eval.TaintAddedAt).To(BeNil(), + "TaintAddedAt must be nil when the taint was pre-existing") + }) + + It("C2 — TaintObservedAt and TaintAddedAt are both set when NRC adds the taint", func() { + // Node has no taint initially, conditions are not met. + // On first reconcile there is no previous NRE and no taint — nothing to set. + // On second reconcile the taint has been added by NodeReconciler — NRE records it. + node = makeNode("nre-c-added", nil, corev1.ConditionFalse) + Expect(k8sClient.Create(ctx, node)).To(Succeed()) + + // First NRE reconcile: no taint yet. + reconcileNRE(nreR, node.Name) + nre := getNRE(node.Name) + Expect(nre.Status.Rules[0].TaintObservedAt).To(BeNil()) + Expect(nre.Status.Rules[0].TaintAddedAt).To(BeNil()) + + // Simulate NodeReconciler adding the taint. + updatedNode := &corev1.Node{} + Expect(k8sClient.Get(ctx, types.NamespacedName{Name: node.Name}, updatedNode)).To(Succeed()) + updatedNode.Spec.Taints = []corev1.Taint{ + {Key: taintKey, Effect: corev1.TaintEffectNoSchedule}, + } + Expect(k8sClient.Update(ctx, updatedNode)).To(Succeed()) + + // Second NRE reconcile: taint is now present — Absent→Present transition. + reconcileNRE(nreR, node.Name) + nre = getNRE(node.Name) + eval := nre.Status.Rules[0] + + By("TaintObservedAt is set") + Expect(eval.TaintObservedAt).NotTo(BeNil()) + By("TaintAddedAt is also set — NRC added it") + Expect(eval.TaintAddedAt).NotTo(BeNil()) + }) + + It("C3 — TaintObservedAt carries forward unchanged on subsequent reconciles while taint persists", func() { + node = makeNode("nre-c-carry", []corev1.Taint{ + {Key: taintKey, Effect: corev1.TaintEffectNoSchedule}, + }, corev1.ConditionFalse) + Expect(k8sClient.Create(ctx, node)).To(Succeed()) + + // First reconcile: adoption — TaintObservedAt set. + reconcileNRE(nreR, node.Name) + firstNRE := getNRE(node.Name) + firstObservedAt := firstNRE.Status.Rules[0].TaintObservedAt + Expect(firstObservedAt).NotTo(BeNil()) + + // Second reconcile: nothing changed. + reconcileNRE(nreR, node.Name) + secondNRE := getNRE(node.Name) + + Expect(secondNRE.Status.Rules[0].TaintObservedAt).To(Equal(firstObservedAt), + "TaintObservedAt must be carried forward unchanged on subsequent reconciles") + }) + + It("C4 — TaintRemovedAt is set on Present→Absent transition, ObservedAt/AddedAt are cleared", func() { + node = makeNode("nre-c-clear", []corev1.Taint{ + {Key: taintKey, Effect: corev1.TaintEffectNoSchedule}, + }, corev1.ConditionFalse) + Expect(k8sClient.Create(ctx, node)).To(Succeed()) + + // First reconcile: adoption — TaintObservedAt set, TaintAddedAt nil. + reconcileNRE(nreR, node.Name) + Expect(getNRE(node.Name).Status.Rules[0].TaintObservedAt).NotTo(BeNil()) + Expect(getNRE(node.Name).Status.Rules[0].TaintRemovedAt).To(BeNil()) + + // Simulate taint removal + condition satisfied. + updatedNode := &corev1.Node{} + Expect(k8sClient.Get(ctx, types.NamespacedName{Name: node.Name}, updatedNode)).To(Succeed()) + updatedNode.Spec.Taints = nil + Expect(k8sClient.Update(ctx, updatedNode)).To(Succeed()) + + Expect(k8sClient.Get(ctx, types.NamespacedName{Name: node.Name}, updatedNode)).To(Succeed()) + updatedNode.Status.Conditions[0].Status = corev1.ConditionTrue + Expect(k8sClient.Status().Update(ctx, updatedNode)).To(Succeed()) + + // Second reconcile: Present→Absent transition. + reconcileNRE(nreR, node.Name) + eval := getNRE(node.Name).Status.Rules[0] + + By("TaintObservedAt and TaintAddedAt are cleared") + Expect(eval.TaintObservedAt).To(BeNil()) + Expect(eval.TaintAddedAt).To(BeNil()) + + By("TaintRemovedAt is set to record when removal happened") + Expect(eval.TaintRemovedAt).NotTo(BeNil()) + + // Third reconcile: taint stays absent — TaintRemovedAt carries forward. + reconcileNRE(nreR, node.Name) + eval2 := getNRE(node.Name).Status.Rules[0] + + By("TaintRemovedAt carries forward on subsequent reconciles (historical SLI)") + Expect(eval2.TaintRemovedAt).To(Equal(eval.TaintRemovedAt)) + }) + }) + + // Suite D — Lifecycle and GC. + + Context("Suite D — NRE lifecycle", func() { + var ( + rc *RuleReadinessController + nreR *NodeReadinessEvaluationReconciler + node *corev1.Node + rule *nodereadinessiov1alpha1.NodeReadinessRule + ) + + BeforeEach(func() { + rc, nreR = sharedSetup() + + node = makeNode("nre-d-node", []corev1.Taint{ + {Key: taintKey, Effect: corev1.TaintEffectNoSchedule}, + }, corev1.ConditionFalse) + rule = makeRule("nre-d-rule") + + Expect(k8sClient.Create(ctx, node)).To(Succeed()) + Expect(k8sClient.Create(ctx, rule)).To(Succeed()) + rc.updateRuleCache(ctx, rule) + }) + + AfterEach(func() { + if node != nil { + _ = k8sClient.Delete(ctx, node) + } + updatedRule := &nodereadinessiov1alpha1.NodeReadinessRule{} + if err := k8sClient.Get(ctx, types.NamespacedName{Name: rule.Name}, updatedRule); err == nil { + updatedRule.Finalizers = nil + _ = k8sClient.Update(ctx, updatedRule) + _ = k8sClient.Delete(ctx, updatedRule) + } + Eventually(func() bool { + return apierrors.IsNotFound( + k8sClient.Get(ctx, types.NamespacedName{Name: rule.Name}, + &nodereadinessiov1alpha1.NodeReadinessRule{})) + }, nreTestTimeout).Should(BeTrue()) + nre := &nodereadinessiov1alpha1.NodeReadinessEvaluation{} + if err := k8sClient.Get(ctx, nreNamespacedName(node.Name), nre); err == nil { + _ = k8sClient.Delete(ctx, nre) + } + rc.removeRuleFromCache(ctx, rule.Name) + }) + + It("D1 — rule removed from cache causes its entry to disappear from NRE on next reconcile", func() { + // First reconcile: rule present. + reconcileNRE(nreR, node.Name) + Expect(getNRE(node.Name).Status.Rules).To(HaveLen(1)) + + // Remove the rule from the cache (simulates rule deletion). + rc.removeRuleFromCache(ctx, rule.Name) + + // Second reconcile: no applicable rules. + reconcileNRE(nreR, node.Name) + nre := getNRE(node.Name) + + Expect(nre.Status.Rules).To(BeEmpty(), + "rules slice should be empty after the rule is removed from the cache") + Expect(nre.Status.State).To(Equal(nodereadinessiov1alpha1.NodeEvaluationStateAvailable), + "state should be Available when no rules apply and no taints are managed") + }) + + It("D2 — NRE carries ownerReference to Node for automatic GC", func() { + reconcileNRE(nreR, node.Name) + + nre := getNRE(node.Name) + Expect(nre.OwnerReferences).To(HaveLen(1)) + ref := nre.OwnerReferences[0] + Expect(ref.Kind).To(Equal("Node")) + Expect(ref.Name).To(Equal(node.Name)) + Expect(ref.UID).To(Equal(node.GetUID())) + // SetOwnerReference (non-controller) leaves BlockOwnerDeletion nil, + // which is the correct GC cascade behaviour — Node deletion is not blocked. + }) + + It("D3 — dry-run rules are excluded from NRE", func() { + // Mark the rule as dry-run in the cache. + dryRunRule := rule.DeepCopy() + dryRunRule.Spec.DryRun = true + rc.updateRuleCache(ctx, dryRunRule) + + reconcileNRE(nreR, node.Name) + + nre := getNRE(node.Name) + Expect(nre.Status.Rules).To(BeEmpty(), + "dry-run rules must not produce NRE entries") + }) + + It("D4 — rules being deleted (DeletionTimestamp set) are excluded from NRE", func() { + deletingRule := rule.DeepCopy() + now := metav1.Now() + deletingRule.DeletionTimestamp = &now + rc.updateRuleCache(ctx, deletingRule) + + reconcileNRE(nreR, node.Name) + + nre := getNRE(node.Name) + Expect(nre.Status.Rules).To(BeEmpty(), + "rules with DeletionTimestamp must not produce NRE entries") + }) + + It("D5 — FirstEvaluatedAt is set on creation and preserved on subsequent reconciles", func() { + reconcileNRE(nreR, node.Name) + firstNRE := getNRE(node.Name) + Expect(firstNRE.Status.Rules[0].FirstEvaluatedAt).NotTo(BeNil()) + firstTime := firstNRE.Status.Rules[0].FirstEvaluatedAt + + // Wait a tick so the clock would differ if mistakenly overwritten. + time.Sleep(nreTestInterval) + reconcileNRE(nreR, node.Name) + + secondNRE := getNRE(node.Name) + Expect(secondNRE.Status.Rules[0].FirstEvaluatedAt).To(Equal(firstTime), + "FirstEvaluatedAt must not be overwritten on subsequent reconciles") + }) + + It("D6 — rule delete+recreate (same name, new UID) resets FirstEvaluatedAt", func() { + reconcileNRE(nreR, node.Name) + firstNRE := getNRE(node.Name) + Expect(firstNRE.Status.Rules[0].FirstEvaluatedAt).NotTo(BeNil()) + firstUID := firstNRE.Status.Rules[0].RuleUID + + // Simulate rule recreated with same name but new UID. + recreatedRule := rule.DeepCopy() + recreatedRule.UID = "new-uid-after-recreate" + rc.updateRuleCache(ctx, recreatedRule) + + reconcileNRE(nreR, node.Name) + + secondNRE := getNRE(node.Name) + Expect(secondNRE.Status.Rules[0].FirstEvaluatedAt).NotTo(BeNil()) + // The UID in the entry must now reflect the recreated rule. + Expect(secondNRE.Status.Rules[0].RuleUID).NotTo(Equal(firstUID), + "RuleUID should reflect the new rule after delete+recreate") + // And because the UID changed, FirstEvaluatedAt was treated as a new rule. + // The previous entry had the old UID so findPreviousRuleEvaluation returns nil, + // causing FirstEvaluatedAt to be set to now (which may equal the previous + // value if within the same wall-clock second — we verify via UID change above). + }) + }) +}) + +// findCondition returns the condition with the given type from a slice, or nil. +func findCondition(conditions []metav1.Condition, condType string) *metav1.Condition { + for i := range conditions { + if conditions[i].Type == condType { + return &conditions[i] + } + } + return nil +} diff --git a/internal/controller/nodereadinessrule_controller.go b/internal/controller/nodereadinessrule_controller.go index eb854306..79912ea9 100644 --- a/internal/controller/nodereadinessrule_controller.go +++ b/internal/controller/nodereadinessrule_controller.go @@ -508,7 +508,7 @@ func (r *RuleReadinessController) evaluateRuleForNode(ctx context.Context, rule taintStatus = readinessv1alpha1.TaintStatusAbsent } - // Update evaluation status + // Update evaluation status. r.updateNodeEvaluationStatus(rule, node.Name, conditionResults, taintStatus) return nil From 9afd4692da0093b0a3a742907a11fe6dc2442847 Mon Sep 17 00:00:00 2001 From: Karthik Bhat Date: Mon, 31 Aug 2026 18:44:43 +0530 Subject: [PATCH 2/4] Integrate NRE to the existing controllers --- cmd/main.go | 16 +- config/crd/kustomization.yaml | 1 + config/manager/manager.yaml | 2 +- config/rbac/role.yaml | 3 - internal/controller/node_controller.go | 5 + .../nodereadinessevaluation_controller.go | 126 ++----------- ...nodereadinessevaluation_controller_test.go | 176 ++++++++++++------ .../nodereadinessrule_controller.go | 10 +- 8 files changed, 152 insertions(+), 187 deletions(-) diff --git a/cmd/main.go b/cmd/main.go index 36b44bdb..ccb51c7a 100644 --- a/cmd/main.go +++ b/cmd/main.go @@ -101,8 +101,7 @@ func main() { flag.BoolVar(&enableNodeStateMetrics, "enable-node-state-metrics", false, "Enable aggregate node state metrics on node updates.") flag.BoolVar(&enableNodeReadinessEvaluation, "enable-node-readiness-evaluation", false, - "Enable the NodeReadinessEvaluation controller. When set, one NRE object is created "+ - "per node and kept up to date with the evaluated state of all applicable rules.") + "Enable NodeReadinessEvaluation writes. When set, one NRE object is created per node") flag.Float64Var(&kubeAPIQPS, "kube-api-qps", defaultKubeAPIQPS, "Maximum queries per second to the API server from this client. "+ "Raise together with --kube-api-burst on large clusters.") @@ -162,7 +161,7 @@ func main() { } // Create the main RuleReadinessController - readinessController := controller.NewRuleReadinessController(mgr, clientset, enableNodeStateMetrics) + readinessController := controller.NewRuleReadinessController(mgr, clientset, enableNodeStateMetrics, enableNodeReadinessEvaluation) // Register the scrape-time collector. crmetrics.Registry.MustRegister(metrics.NewReadinessCollector(readinessController)) @@ -194,16 +193,7 @@ func main() { } if enableNodeReadinessEvaluation { - nreReconciler := &controller.NodeReadinessEvaluationReconciler{ - Client: mgr.GetClient(), - Scheme: mgr.GetScheme(), - Controller: readinessController, - } - if err := nreReconciler.SetupWithManager(mgr); err != nil { - setupLog.Error(err, "unable to create controller", "controller", "NodeReadinessEvaluation") - os.Exit(1) - } - setupLog.Info("NodeReadinessEvaluation controller enabled") + setupLog.Info("NodeReadinessEvaluation writing enabled") } // Setup webhook (conditional based on flag) diff --git a/config/crd/kustomization.yaml b/config/crd/kustomization.yaml index 79ab98ea..2076e403 100644 --- a/config/crd/kustomization.yaml +++ b/config/crd/kustomization.yaml @@ -3,4 +3,5 @@ # It should be run by config/default resources: - bases/readiness.node.x-k8s.io_nodereadinessrules.yaml +- bases/readiness.node.x-k8s.io_nodereadinessevaluations.yaml # +kubebuilder:scaffold:crdkustomizeresource diff --git a/config/manager/manager.yaml b/config/manager/manager.yaml index 3636c2f1..77762303 100644 --- a/config/manager/manager.yaml +++ b/config/manager/manager.yaml @@ -1,4 +1,4 @@ -config/manager/manager.yaml apiVersion: v1 +apiVersion: v1 kind: Namespace metadata: labels: diff --git a/config/rbac/role.yaml b/config/rbac/role.yaml index 4df190bb..093d9a30 100644 --- a/config/rbac/role.yaml +++ b/config/rbac/role.yaml @@ -34,11 +34,8 @@ rules: - nodereadinessevaluations verbs: - create - - delete - get - list - - patch - - update - watch - apiGroups: - readiness.node.x-k8s.io diff --git a/internal/controller/node_controller.go b/internal/controller/node_controller.go index 86b48a6e..def0bcfb 100644 --- a/internal/controller/node_controller.go +++ b/internal/controller/node_controller.go @@ -107,6 +107,11 @@ func (r *NodeReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl. return ctrl.Result{}, err } + // Update the NodeReadinessEvaluation for this node. + if r.Controller.EnableNRE { + r.Controller.updateNREForNode(ctx, node) + } + return ctrl.Result{}, nil } diff --git a/internal/controller/nodereadinessevaluation_controller.go b/internal/controller/nodereadinessevaluation_controller.go index 112d61bc..42c6f5d8 100644 --- a/internal/controller/nodereadinessevaluation_controller.go +++ b/internal/controller/nodereadinessevaluation_controller.go @@ -25,125 +25,31 @@ import ( apierrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/api/meta" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/runtime" - "k8s.io/apimachinery/pkg/types" ctrl "sigs.k8s.io/controller-runtime" - "sigs.k8s.io/controller-runtime/pkg/builder" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" - "sigs.k8s.io/controller-runtime/pkg/event" - "sigs.k8s.io/controller-runtime/pkg/handler" - "sigs.k8s.io/controller-runtime/pkg/predicate" - "sigs.k8s.io/controller-runtime/pkg/reconcile" readinessv1alpha1 "sigs.k8s.io/node-readiness-controller/api/v1alpha1" ) -// NodeReadinessEvaluationReconciler is an independent reconciler that maintains -// one NodeReadinessEvaluation object per Node. It watches Nodes and -// NodeReadinessRules and re-evaluates the full rule set for the affected node -// on every relevant change. It shares the rule cache owned by -// RuleReadinessController but never writes to Node taints or NRR status — -// those remain the sole responsibility of NodeReconciler / RuleReconciler. -type NodeReadinessEvaluationReconciler struct { - client.Client - Scheme *runtime.Scheme - Controller *RuleReadinessController -} - -// SetupWithManager wires the reconciler to watch: -// - Node objects (conditions, taints, labels) -// - NodeReadinessRule objects (enqueues all nodes matching the changed rule) -func (r *NodeReadinessEvaluationReconciler) SetupWithManager(mgr ctrl.Manager) error { - return ctrl.NewControllerManagedBy(mgr). - Named("nodereadinessevaluation"). - // Primary watch: a node change triggers reconcile for that node's NRE. - For(&corev1.Node{}, builder.WithPredicates(predicate.Funcs{ - CreateFunc: func(e event.CreateEvent) bool { return true }, - UpdateFunc: func(e event.UpdateEvent) bool { - oldNode := e.ObjectOld.(*corev1.Node) - newNode := e.ObjectNew.(*corev1.Node) - return !conditionsEqual(oldNode.Status.Conditions, newNode.Status.Conditions) || - !taintsEqual(oldNode.Spec.Taints, newNode.Spec.Taints) || - !labelsEqual(oldNode.Labels, newNode.Labels) - }, - DeleteFunc: func(e event.DeleteEvent) bool { return false }, // GC handles deletion via ownerRef - GenericFunc: func(e event.GenericEvent) bool { return false }, - })). - // Secondary watch: a rule change re-evaluates every node in the cluster - // that could be affected. We map the rule event to a list of node requests. - Watches( - &readinessv1alpha1.NodeReadinessRule{}, - handler.EnqueueRequestsFromMapFunc(r.ruleToNodeRequests), - builder.WithPredicates(predicate.GenerationChangedPredicate{}), - ). - Complete(r) -} - -// ruleToNodeRequests maps a NodeReadinessRule event to reconcile requests for -// only the Nodes that match the rule's nodeSelector. Filtering here avoids -// enqueueing the full cluster on every rule change — on a 5k-node cluster a -// rule targeting 200 nodes produces 200 requests, not 5,000. -// -// nodeSelector is immutable enforeced via CEL validateion on the spec, so a changing -// selector can never silently leave stale NRE entries behind. -func (r *NodeReadinessEvaluationReconciler) ruleToNodeRequests(ctx context.Context, obj client.Object) []reconcile.Request { - log := ctrl.LoggerFrom(ctx) - - rule, ok := obj.(*readinessv1alpha1.NodeReadinessRule) - if !ok { - return nil - } - - selector, err := metav1.LabelSelectorAsSelector(&rule.Spec.NodeSelector) - if err != nil { - // Invalid selector — the rule reconciler will surface this as an error; - // nothing to enqueue here. - log.Error(err, "invalid nodeSelector on rule, skipping NRE fan-out", "rule", rule.Name) - return nil - } - - nodeList := &corev1.NodeList{} - if err := r.List(ctx, nodeList, client.MatchingLabelsSelector{Selector: selector}); err != nil { - log.Error(err, "failed to list matching nodes for NRE rule mapping", "rule", rule.Name) - return nil - } - - requests := make([]reconcile.Request, len(nodeList.Items)) - for i, node := range nodeList.Items { - requests[i] = reconcile.Request{ - NamespacedName: types.NamespacedName{Name: node.Name}, - } - } - - log.V(4).Info("Enqueuing NRE reconciles for rule change", - "rule", rule.Name, "matchingNodes", len(requests)) - return requests -} - -// +kubebuilder:rbac:groups=readiness.node.x-k8s.io,resources=nodereadinessevaluations,verbs=get;list;watch;create;update;patch;delete +// +kubebuilder:rbac:groups=readiness.node.x-k8s.io,resources=nodereadinessevaluations,verbs=get;list;watch;create // +kubebuilder:rbac:groups=readiness.node.x-k8s.io,resources=nodereadinessevaluations/status,verbs=get;update;patch -// Reconcile evaluates all applicable rules for the given node and writes the -// result into the corresponding NodeReadinessEvaluation object. -func (r *NodeReadinessEvaluationReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) { +// updateNREForNode writes (or updates) the NodeReadinessEvaluation for the given +// node using the current rule cache. It is called as a side-effect from both +// NodeReconciler.Reconcile (after processing the node against all rules) and +// RuleReconciler.Reconcile (after processing all nodes for the changed rule). +func (r *RuleReadinessController) updateNREForNode(ctx context.Context, node *corev1.Node) { log := ctrl.LoggerFrom(ctx) - log.V(4).Info("Reconciling NodeReadinessEvaluation", "node", req.Name) - - // Fetch the Node. - node := &corev1.Node{} - if err := r.Get(ctx, req.NamespacedName, node); err != nil { - return ctrl.Result{}, client.IgnoreNotFound(err) - } - // Fetch or create the NRE object. nre, err := r.ensureNRE(ctx, node) if err != nil { - return ctrl.Result{}, err + log.Error(err, "Failed to ensure NRE for node", "node", node.Name) + return } // Evaluate all applicable rules from the shared cache. - applicableRules := r.Controller.getApplicableRulesForNode(ctx, node) + applicableRules := r.getApplicableRulesForNode(ctx, node) log.V(4).Info("Evaluating rules for NRE", "node", node.Name, "ruleCount", len(applicableRules)) // Snapshot the previous rules slice for timestamp carry-forward BEFORE @@ -169,16 +75,16 @@ func (r *NodeReadinessEvaluationReconciler) Reconcile(ctx context.Context, req c recomputeNREStatus(&nre.Status) if err := r.Status().Patch(ctx, nre, patch); err != nil { - return ctrl.Result{}, fmt.Errorf("failed to patch NRE status %s: %w", node.Name, err) + log.Error(err, "Failed to patch NRE status", "node", node.Name) + return } - log.V(4).Info("Reconciled NRE", "node", node.Name, "rules", len(nre.Status.Rules), "state", nre.Status.State) - return ctrl.Result{}, nil + log.V(4).Info("Updated NRE", "node", node.Name, "rules", len(nre.Status.Rules), "state", nre.Status.State) } // ensureNRE fetches the NRE for the node, creating it (with ownerReference) if // it does not exist yet. Returns the current object ready for status patching. -func (r *NodeReadinessEvaluationReconciler) ensureNRE(ctx context.Context, node *corev1.Node) (*readinessv1alpha1.NodeReadinessEvaluation, error) { +func (r *RuleReadinessController) ensureNRE(ctx context.Context, node *corev1.Node) (*readinessv1alpha1.NodeReadinessEvaluation, error) { nre := &readinessv1alpha1.NodeReadinessEvaluation{} err := r.Get(ctx, client.ObjectKey{Name: node.Name}, nre) @@ -216,7 +122,7 @@ func (r *NodeReadinessEvaluationReconciler) ensureNRE(ctx context.Context, node // buildRuleEvaluation evaluates a single rule against the node and constructs // the RuleEvaluation entry, preserving SLI timestamps from prevRules (the // snapshot of the previous status.rules slice taken before the rebuild started). -func (r *NodeReadinessEvaluationReconciler) buildRuleEvaluation( +func (r *RuleReadinessController) buildRuleEvaluation( ctx context.Context, node *corev1.Node, rule *readinessv1alpha1.NodeReadinessRule, @@ -229,7 +135,7 @@ func (r *NodeReadinessEvaluationReconciler) buildRuleEvaluation( allConditionsSatisfied := true conditionResults := make([]readinessv1alpha1.ConditionEvaluationResult, 0, len(rule.Spec.Conditions)) for _, condReq := range rule.Spec.Conditions { - effectiveStatus, conditionFound := r.Controller.getConditionStatus(node, condReq.Type, condReq.GetDefaultStatus()) + effectiveStatus, conditionFound := r.getConditionStatus(node, condReq.Type, condReq.GetDefaultStatus()) satisfied := effectiveStatus == condReq.RequiredStatus if !satisfied { allConditionsSatisfied = false @@ -251,7 +157,7 @@ func (r *NodeReadinessEvaluationReconciler) buildRuleEvaluation( ruleStatus = readinessv1alpha1.RuleStatusUnsatisfied } - taintPresent := r.Controller.hasTaintBySpec(node, rule.Spec.Taint) + taintPresent := r.hasTaintBySpec(node, rule.Spec.Taint) taintStatus := readinessv1alpha1.TaintStatusAbsent if taintPresent { taintStatus = readinessv1alpha1.TaintStatusPresent diff --git a/internal/controller/nodereadinessevaluation_controller_test.go b/internal/controller/nodereadinessevaluation_controller_test.go index f94845c3..a7004c43 100644 --- a/internal/controller/nodereadinessevaluation_controller_test.go +++ b/internal/controller/nodereadinessevaluation_controller_test.go @@ -38,7 +38,7 @@ func nreNamespacedName(nodeName string) types.NamespacedName { return types.NamespacedName{Name: nodeName} } -var _ = Describe("NodeReadinessEvaluation Controller", func() { +var _ = Describe("NodeReadinessEvaluation writes", func() { const ( nreTestTimeout = 10 * time.Second nreTestInterval = 100 * time.Millisecond @@ -84,32 +84,25 @@ var _ = Describe("NodeReadinessEvaluation Controller", func() { } } - // sharedSetup wires up a RuleReadinessController and a - // NodeReadinessEvaluationReconciler backed by the envtest k8sClient. - // Returns both reconcilers and a cleanup function. - sharedSetup := func() (*RuleReadinessController, *NodeReadinessEvaluationReconciler) { - rc := &RuleReadinessController{ + // sharedSetup wires up a RuleReadinessController backed by the envtest k8sClient + // with NRE writing enabled. + sharedSetup := func() *RuleReadinessController { + return &RuleReadinessController{ Client: k8sClient, Scheme: k8sClient.Scheme(), clientset: fake.NewSimpleClientset(), ruleCache: make(map[string]*nodereadinessiov1alpha1.NodeReadinessRule), EventRecorder: events.NewFakeRecorder(32), + EnableNRE: true, } - nreR := &NodeReadinessEvaluationReconciler{ - Client: k8sClient, - Scheme: k8sClient.Scheme(), - Controller: rc, - } - return rc, nreR } - // reconcileNRE triggers a single NRE reconcile for the named node. - reconcileNRE := func(nreR *NodeReadinessEvaluationReconciler, nodeName string) { + // reconcileNRE triggers updateNREForNode for the named node. + reconcileNRE := func(rc *RuleReadinessController, nodeName string) { GinkgoHelper() - _, err := nreR.Reconcile(ctx, reconcile.Request{ - NamespacedName: types.NamespacedName{Name: nodeName}, - }) - Expect(err).NotTo(HaveOccurred()) + node := &corev1.Node{} + Expect(k8sClient.Get(ctx, types.NamespacedName{Name: nodeName}, node)).To(Succeed()) + rc.updateNREForNode(ctx, node) } // getNRE fetches the current NRE for the given node name. @@ -125,13 +118,12 @@ var _ = Describe("NodeReadinessEvaluation Controller", func() { Context("Suite A — fresh cluster with NRE enabled", func() { var ( rc *RuleReadinessController - nreR *NodeReadinessEvaluationReconciler node *corev1.Node rule *nodereadinessiov1alpha1.NodeReadinessRule ) BeforeEach(func() { - rc, nreR = sharedSetup() + rc = sharedSetup() node = makeNode("nre-a-node", []corev1.Taint{ {Key: taintKey, Effect: corev1.TaintEffectNoSchedule}, @@ -168,7 +160,7 @@ var _ = Describe("NodeReadinessEvaluation Controller", func() { }) It("A1 — creates NRE with correct spec.nodeName and ownerReference on first reconcile", func() { - reconcileNRE(nreR, node.Name) + reconcileNRE(rc, node.Name) nre := getNRE(node.Name) @@ -183,7 +175,7 @@ var _ = Describe("NodeReadinessEvaluation Controller", func() { }) It("A2 — state is NotAvailable and activeTaints=1 when conditions are not met", func() { - reconcileNRE(nreR, node.Name) + reconcileNRE(rc, node.Name) nre := getNRE(node.Name) @@ -201,7 +193,7 @@ var _ = Describe("NodeReadinessEvaluation Controller", func() { It("A3 — state transitions to Available after conditions are met and taint is removed", func() { // First reconcile: conditions not met, taint present. - reconcileNRE(nreR, node.Name) + reconcileNRE(rc, node.Name) Expect(getNRE(node.Name).Status.State).To(Equal(nodereadinessiov1alpha1.NodeEvaluationStateNotAvailable)) // Satisfy the condition and remove the taint (simulating NodeReconciler action). @@ -215,7 +207,7 @@ var _ = Describe("NodeReadinessEvaluation Controller", func() { Expect(k8sClient.Update(ctx, updatedNode)).To(Succeed()) // Second reconcile: conditions met, taint absent. - reconcileNRE(nreR, node.Name) + reconcileNRE(rc, node.Name) nre := getNRE(node.Name) Expect(nre.Status.State).To(Equal(nodereadinessiov1alpha1.NodeEvaluationStateAvailable)) @@ -224,7 +216,7 @@ var _ = Describe("NodeReadinessEvaluation Controller", func() { }) It("A4 — taintKey and taintEffect are stamped on the RuleEvaluation entry", func() { - reconcileNRE(nreR, node.Name) + reconcileNRE(rc, node.Name) nre := getNRE(node.Name) Expect(nre.Status.Rules).To(HaveLen(1)) @@ -238,7 +230,7 @@ var _ = Describe("NodeReadinessEvaluation Controller", func() { }) It("A4b — condition evaluation breakdown is present in RuleEvaluation", func() { - reconcileNRE(nreR, node.Name) + reconcileNRE(rc, node.Name) nre := getNRE(node.Name) Expect(nre.Status.Rules[0].ReadinessConditions).To(HaveLen(1)) @@ -249,7 +241,7 @@ var _ = Describe("NodeReadinessEvaluation Controller", func() { }) It("A5 — Evaluated=True and Available=False conditions are set when taint is present", func() { - reconcileNRE(nreR, node.Name) + reconcileNRE(rc, node.Name) nre := getNRE(node.Name) By("Evaluated condition is True — no errors") @@ -277,7 +269,7 @@ var _ = Describe("NodeReadinessEvaluation Controller", func() { updatedNode.Status.Conditions[0].Status = corev1.ConditionTrue Expect(k8sClient.Status().Update(ctx, updatedNode)).To(Succeed()) - reconcileNRE(nreR, node.Name) + reconcileNRE(rc, node.Name) nre := getNRE(node.Name) available := findCondition(nre.Status.Conditions, "Available") @@ -290,14 +282,13 @@ var _ = Describe("NodeReadinessEvaluation Controller", func() { Context("Suite A (multi-rule) — multiple rules folded into one NRE", func() { var ( rc *RuleReadinessController - nreR *NodeReadinessEvaluationReconciler node *corev1.Node rule1 *nodereadinessiov1alpha1.NodeReadinessRule rule2 *nodereadinessiov1alpha1.NodeReadinessRule ) BeforeEach(func() { - rc, nreR = sharedSetup() + rc = sharedSetup() node = makeNode("nre-multi-node", []corev1.Taint{ {Key: taintKey, Effect: corev1.TaintEffectNoSchedule}, @@ -339,7 +330,7 @@ var _ = Describe("NodeReadinessEvaluation Controller", func() { }) It("A7 — both rules are folded into the single NRE for the node", func() { - reconcileNRE(nreR, node.Name) + reconcileNRE(rc, node.Name) nre := getNRE(node.Name) Expect(nre.Status.Rules).To(HaveLen(2), @@ -360,7 +351,7 @@ var _ = Describe("NodeReadinessEvaluation Controller", func() { } rc.updateRuleCache(ctx, updatedRule2) - reconcileNRE(nreR, node.Name) + reconcileNRE(rc, node.Name) nre := getNRE(node.Name) Expect(nre.Status.Rules).To(HaveLen(1), @@ -374,13 +365,12 @@ var _ = Describe("NodeReadinessEvaluation Controller", func() { Context("Suite B — NRE enabled on a cluster with rules already running", func() { var ( rc *RuleReadinessController - nreR *NodeReadinessEvaluationReconciler node *corev1.Node rule *nodereadinessiov1alpha1.NodeReadinessRule ) BeforeEach(func() { - rc, nreR = sharedSetup() + rc = sharedSetup() // Simulate a cluster where rules and taints are already in place. node = makeNode("nre-b-node", []corev1.Taint{ @@ -416,7 +406,7 @@ var _ = Describe("NodeReadinessEvaluation Controller", func() { It("B1 — first NRE reconcile on existing cluster produces correct full state", func() { // No prior NRE exists — this is the moment the feature is enabled. - reconcileNRE(nreR, node.Name) + reconcileNRE(rc, node.Name) nre := getNRE(node.Name) Expect(nre.Spec.NodeName).To(Equal(node.Name)) @@ -427,13 +417,13 @@ var _ = Describe("NodeReadinessEvaluation Controller", func() { }) It("B2 — subsequent reconciles are idempotent", func() { - reconcileNRE(nreR, node.Name) + reconcileNRE(rc, node.Name) firstNRE := getNRE(node.Name) firstState := firstNRE.Status.State firstRuleLen := len(firstNRE.Status.Rules) // Reconcile again without changing anything. - reconcileNRE(nreR, node.Name) + reconcileNRE(rc, node.Name) secondNRE := getNRE(node.Name) Expect(secondNRE.Status.State).To(Equal(firstState)) @@ -446,13 +436,12 @@ var _ = Describe("NodeReadinessEvaluation Controller", func() { Context("Suite C — pre-tainted node adoption", func() { var ( rc *RuleReadinessController - nreR *NodeReadinessEvaluationReconciler node *corev1.Node rule *nodereadinessiov1alpha1.NodeReadinessRule ) BeforeEach(func() { - rc, nreR = sharedSetup() + rc = sharedSetup() rule = makeRule("nre-c-rule") Expect(k8sClient.Create(ctx, rule)).To(Succeed()) rc.updateRuleCache(ctx, rule) @@ -492,7 +481,7 @@ var _ = Describe("NodeReadinessEvaluation Controller", func() { Expect(k8sClient.Create(ctx, node)).To(Succeed()) // First reconcile: no previous NRE — taint is pre-existing. - reconcileNRE(nreR, node.Name) + reconcileNRE(rc, node.Name) nre := getNRE(node.Name) Expect(nre.Status.Rules).To(HaveLen(1)) @@ -515,7 +504,7 @@ var _ = Describe("NodeReadinessEvaluation Controller", func() { Expect(k8sClient.Create(ctx, node)).To(Succeed()) // First NRE reconcile: no taint yet. - reconcileNRE(nreR, node.Name) + reconcileNRE(rc, node.Name) nre := getNRE(node.Name) Expect(nre.Status.Rules[0].TaintObservedAt).To(BeNil()) Expect(nre.Status.Rules[0].TaintAddedAt).To(BeNil()) @@ -529,7 +518,7 @@ var _ = Describe("NodeReadinessEvaluation Controller", func() { Expect(k8sClient.Update(ctx, updatedNode)).To(Succeed()) // Second NRE reconcile: taint is now present — Absent→Present transition. - reconcileNRE(nreR, node.Name) + reconcileNRE(rc, node.Name) nre = getNRE(node.Name) eval := nre.Status.Rules[0] @@ -546,13 +535,13 @@ var _ = Describe("NodeReadinessEvaluation Controller", func() { Expect(k8sClient.Create(ctx, node)).To(Succeed()) // First reconcile: adoption — TaintObservedAt set. - reconcileNRE(nreR, node.Name) + reconcileNRE(rc, node.Name) firstNRE := getNRE(node.Name) firstObservedAt := firstNRE.Status.Rules[0].TaintObservedAt Expect(firstObservedAt).NotTo(BeNil()) // Second reconcile: nothing changed. - reconcileNRE(nreR, node.Name) + reconcileNRE(rc, node.Name) secondNRE := getNRE(node.Name) Expect(secondNRE.Status.Rules[0].TaintObservedAt).To(Equal(firstObservedAt), @@ -566,7 +555,7 @@ var _ = Describe("NodeReadinessEvaluation Controller", func() { Expect(k8sClient.Create(ctx, node)).To(Succeed()) // First reconcile: adoption — TaintObservedAt set, TaintAddedAt nil. - reconcileNRE(nreR, node.Name) + reconcileNRE(rc, node.Name) Expect(getNRE(node.Name).Status.Rules[0].TaintObservedAt).NotTo(BeNil()) Expect(getNRE(node.Name).Status.Rules[0].TaintRemovedAt).To(BeNil()) @@ -581,7 +570,7 @@ var _ = Describe("NodeReadinessEvaluation Controller", func() { Expect(k8sClient.Status().Update(ctx, updatedNode)).To(Succeed()) // Second reconcile: Present→Absent transition. - reconcileNRE(nreR, node.Name) + reconcileNRE(rc, node.Name) eval := getNRE(node.Name).Status.Rules[0] By("TaintObservedAt and TaintAddedAt are cleared") @@ -592,7 +581,7 @@ var _ = Describe("NodeReadinessEvaluation Controller", func() { Expect(eval.TaintRemovedAt).NotTo(BeNil()) // Third reconcile: taint stays absent — TaintRemovedAt carries forward. - reconcileNRE(nreR, node.Name) + reconcileNRE(rc, node.Name) eval2 := getNRE(node.Name).Status.Rules[0] By("TaintRemovedAt carries forward on subsequent reconciles (historical SLI)") @@ -605,13 +594,12 @@ var _ = Describe("NodeReadinessEvaluation Controller", func() { Context("Suite D — NRE lifecycle", func() { var ( rc *RuleReadinessController - nreR *NodeReadinessEvaluationReconciler node *corev1.Node rule *nodereadinessiov1alpha1.NodeReadinessRule ) BeforeEach(func() { - rc, nreR = sharedSetup() + rc = sharedSetup() node = makeNode("nre-d-node", []corev1.Taint{ {Key: taintKey, Effect: corev1.TaintEffectNoSchedule}, @@ -647,14 +635,14 @@ var _ = Describe("NodeReadinessEvaluation Controller", func() { It("D1 — rule removed from cache causes its entry to disappear from NRE on next reconcile", func() { // First reconcile: rule present. - reconcileNRE(nreR, node.Name) + reconcileNRE(rc, node.Name) Expect(getNRE(node.Name).Status.Rules).To(HaveLen(1)) // Remove the rule from the cache (simulates rule deletion). rc.removeRuleFromCache(ctx, rule.Name) // Second reconcile: no applicable rules. - reconcileNRE(nreR, node.Name) + reconcileNRE(rc, node.Name) nre := getNRE(node.Name) Expect(nre.Status.Rules).To(BeEmpty(), @@ -664,7 +652,7 @@ var _ = Describe("NodeReadinessEvaluation Controller", func() { }) It("D2 — NRE carries ownerReference to Node for automatic GC", func() { - reconcileNRE(nreR, node.Name) + reconcileNRE(rc, node.Name) nre := getNRE(node.Name) Expect(nre.OwnerReferences).To(HaveLen(1)) @@ -682,7 +670,7 @@ var _ = Describe("NodeReadinessEvaluation Controller", func() { dryRunRule.Spec.DryRun = true rc.updateRuleCache(ctx, dryRunRule) - reconcileNRE(nreR, node.Name) + reconcileNRE(rc, node.Name) nre := getNRE(node.Name) Expect(nre.Status.Rules).To(BeEmpty(), @@ -695,7 +683,7 @@ var _ = Describe("NodeReadinessEvaluation Controller", func() { deletingRule.DeletionTimestamp = &now rc.updateRuleCache(ctx, deletingRule) - reconcileNRE(nreR, node.Name) + reconcileNRE(rc, node.Name) nre := getNRE(node.Name) Expect(nre.Status.Rules).To(BeEmpty(), @@ -703,14 +691,14 @@ var _ = Describe("NodeReadinessEvaluation Controller", func() { }) It("D5 — FirstEvaluatedAt is set on creation and preserved on subsequent reconciles", func() { - reconcileNRE(nreR, node.Name) + reconcileNRE(rc, node.Name) firstNRE := getNRE(node.Name) Expect(firstNRE.Status.Rules[0].FirstEvaluatedAt).NotTo(BeNil()) firstTime := firstNRE.Status.Rules[0].FirstEvaluatedAt // Wait a tick so the clock would differ if mistakenly overwritten. time.Sleep(nreTestInterval) - reconcileNRE(nreR, node.Name) + reconcileNRE(rc, node.Name) secondNRE := getNRE(node.Name) Expect(secondNRE.Status.Rules[0].FirstEvaluatedAt).To(Equal(firstTime), @@ -718,7 +706,7 @@ var _ = Describe("NodeReadinessEvaluation Controller", func() { }) It("D6 — rule delete+recreate (same name, new UID) resets FirstEvaluatedAt", func() { - reconcileNRE(nreR, node.Name) + reconcileNRE(rc, node.Name) firstNRE := getNRE(node.Name) Expect(firstNRE.Status.Rules[0].FirstEvaluatedAt).NotTo(BeNil()) firstUID := firstNRE.Status.Rules[0].RuleUID @@ -728,7 +716,7 @@ var _ = Describe("NodeReadinessEvaluation Controller", func() { recreatedRule.UID = "new-uid-after-recreate" rc.updateRuleCache(ctx, recreatedRule) - reconcileNRE(nreR, node.Name) + reconcileNRE(rc, node.Name) secondNRE := getNRE(node.Name) Expect(secondNRE.Status.Rules[0].FirstEvaluatedAt).NotTo(BeNil()) @@ -741,6 +729,78 @@ var _ = Describe("NodeReadinessEvaluation Controller", func() { // value if within the same wall-clock second — we verify via UID change above). }) }) + // Suite E — Wiring: NRE is written from NodeReconciler and RuleReconciler paths. + + Context("Suite E — EnableNRE wiring through NodeReconciler and RuleReconciler", func() { + var ( + rc *RuleReadinessController + node *corev1.Node + rule *nodereadinessiov1alpha1.NodeReadinessRule + ) + + BeforeEach(func() { + rc = sharedSetup() + + node = makeNode("nre-e-node", []corev1.Taint{ + {Key: taintKey, Effect: corev1.TaintEffectNoSchedule}, + }, corev1.ConditionFalse) + rule = makeRule("nre-e-rule") + + Expect(k8sClient.Create(ctx, node)).To(Succeed()) + Expect(k8sClient.Create(ctx, rule)).To(Succeed()) + rc.updateRuleCache(ctx, rule) + }) + + AfterEach(func() { + _ = k8sClient.Delete(ctx, node) + updatedRule := &nodereadinessiov1alpha1.NodeReadinessRule{} + if err := k8sClient.Get(ctx, types.NamespacedName{Name: rule.Name}, updatedRule); err == nil { + updatedRule.Finalizers = nil + _ = k8sClient.Update(ctx, updatedRule) + _ = k8sClient.Delete(ctx, updatedRule) + } + Eventually(func() bool { + return apierrors.IsNotFound( + k8sClient.Get(ctx, types.NamespacedName{Name: rule.Name}, + &nodereadinessiov1alpha1.NodeReadinessRule{})) + }, nreTestTimeout).Should(BeTrue()) + nre := &nodereadinessiov1alpha1.NodeReadinessEvaluation{} + if err := k8sClient.Get(ctx, nreNamespacedName(node.Name), nre); err == nil { + _ = k8sClient.Delete(ctx, nre) + } + rc.removeRuleFromCache(ctx, rule.Name) + }) + + It("E1 — NodeReconciler.Reconcile creates NRE when EnableNRE=true", func() { + nodeReconciler := &NodeReconciler{ + Client: k8sClient, + Scheme: k8sClient.Scheme(), + Controller: rc, + } + + _, err := nodeReconciler.Reconcile(ctx, reconcile.Request{ + NamespacedName: types.NamespacedName{Name: node.Name}, + }) + Expect(err).NotTo(HaveOccurred()) + + nre := getNRE(node.Name) + Expect(nre.Spec.NodeName).To(Equal(node.Name)) + Expect(nre.Status.Rules).To(HaveLen(1)) + Expect(nre.Status.Rules[0].RuleName).To(Equal(rule.Name)) + }) + + It("E2 — processAllNodesForRule creates NRE per matching node when EnableNRE=true", func() { + nodeList := &corev1.NodeList{} + Expect(k8sClient.List(ctx, nodeList)).To(Succeed()) + + Expect(rc.processAllNodesForRule(ctx, rule, nodeList)).To(Succeed()) + + nre := getNRE(node.Name) + Expect(nre.Spec.NodeName).To(Equal(node.Name)) + Expect(nre.Status.Rules).To(HaveLen(1)) + Expect(nre.Status.Rules[0].RuleName).To(Equal(rule.Name)) + }) + }) }) // findCondition returns the condition with the given type from a slice, or nil. diff --git a/internal/controller/nodereadinessrule_controller.go b/internal/controller/nodereadinessrule_controller.go index 79912ea9..a13595aa 100644 --- a/internal/controller/nodereadinessrule_controller.go +++ b/internal/controller/nodereadinessrule_controller.go @@ -56,6 +56,7 @@ type RuleReadinessController struct { clientset kubernetes.Interface EventRecorder events.EventRecorder EnableNodeStateMetrics bool + EnableNRE bool // Cache for efficient rule lookup ruleCacheMutex sync.RWMutex @@ -71,13 +72,14 @@ type RuleReconciler struct { } // NewRuleReadinessController creates a new controller. -func NewRuleReadinessController(mgr ctrl.Manager, clientset kubernetes.Interface, enableNodeStateMetrics bool) *RuleReadinessController { +func NewRuleReadinessController(mgr ctrl.Manager, clientset kubernetes.Interface, enableNodeStateMetrics bool, enableNRE bool) *RuleReadinessController { return &RuleReadinessController{ Client: mgr.GetClient(), Scheme: mgr.GetScheme(), clientset: clientset, EventRecorder: mgr.GetEventRecorder("node-readiness-controller"), EnableNodeStateMetrics: enableNodeStateMetrics, + EnableNRE: enableNRE, ruleCache: make(map[string]*readinessv1alpha1.NodeReadinessRule), } } @@ -331,6 +333,10 @@ func (r *RuleReadinessController) processAllNodesForRule(ctx context.Context, ru } } } + + if r.EnableNRE { + r.updateNREForNode(ctx, &node) + } } // Update status @@ -540,7 +546,7 @@ func (r *RuleReadinessController) updateNodeEvaluationStatus( // Update evaluation nodeEval.ConditionResults = conditionResults nodeEval.TaintStatus = taintStatus - nodeEval.LastEvaluationTime = metav1.Now() + nodeEval.LastEvaluatedAt = metav1.Now() } // getApplicableRulesForNode returns all rules applicable to a node. From ebd10d30b6b88cecb1eb73f21ca2e7c13dcccfa6 Mon Sep 17 00:00:00 2001 From: Karthik Bhat Date: Tue, 15 Sep 2026 11:17:19 +0530 Subject: [PATCH 3/4] Rename LastEvaluationTime --- api/v1alpha1/nodereadinessevaluation_types.go | 4 ++-- api/v1alpha1/nodereadinessrule_types.go | 8 ++++---- api/v1alpha1/zz_generated.deepcopy.go | 6 +++--- ...ess.node.x-k8s.io_nodereadinessevaluations.yaml | 8 ++++---- ...readiness.node.x-k8s.io_nodereadinessrules.yaml | 14 +++++++------- internal/controller/node_controller.go | 2 +- internal/controller/node_controller_test.go | 2 +- .../nodereadinessevaluation_controller.go | 2 +- .../nodereadinessevaluation_controller_test.go | 2 +- .../nodereadinessrule_controller_test.go | 2 +- 10 files changed, 25 insertions(+), 25 deletions(-) diff --git a/api/v1alpha1/nodereadinessevaluation_types.go b/api/v1alpha1/nodereadinessevaluation_types.go index 3014f5a0..6461ffbe 100644 --- a/api/v1alpha1/nodereadinessevaluation_types.go +++ b/api/v1alpha1/nodereadinessevaluation_types.go @@ -169,10 +169,10 @@ type RuleEvaluation struct { // +kubebuilder:validation:MaxItems=32 ReadinessConditions []ConditionEvaluationResult `json:"readinessConditions,omitempty"` - // lastEvaluationTime records the exact moment the controller most recently assessed this rule. + // lastEvaluatedAt records the exact moment the controller most recently assessed this rule. // // +required - LastEvaluationTime metav1.Time `json:"lastEvaluationTime,omitempty,omitzero"` + LastEvaluatedAt metav1.Time `json:"lastEvaluatedAt,omitempty,omitzero"` // firstEvaluatedAt is the time the rule was first assessed against this node. // diff --git a/api/v1alpha1/nodereadinessrule_types.go b/api/v1alpha1/nodereadinessrule_types.go index ed0cee13..98b836fa 100644 --- a/api/v1alpha1/nodereadinessrule_types.go +++ b/api/v1alpha1/nodereadinessrule_types.go @@ -243,10 +243,10 @@ type NodeFailure struct { // +kubebuilder:validation:MaxLength=10240 Message string `json:"message,omitempty"` - // lastEvaluationTime is the timestamp of the last rule check failed for this Node. + // lastEvaluatedAt is the timestamp of the last rule check failed for this Node. // // +required - LastEvaluationTime metav1.Time `json:"lastEvaluationTime,omitempty,omitzero"` + LastEvaluatedAt metav1.Time `json:"lastEvaluatedAt,omitempty,omitzero"` } // NodeEvaluation provides a detailed audit of a single Node's compliance with the rule. @@ -274,10 +274,10 @@ type NodeEvaluation struct { // +required TaintStatus TaintStatus `json:"taintStatus,omitempty"` - // lastEvaluationTime is the timestamp when the controller last assessed this Node. + // lastEvaluatedAt is the timestamp when the controller last assessed this Node. // // +required - LastEvaluationTime metav1.Time `json:"lastEvaluationTime,omitempty,omitzero"` + LastEvaluatedAt metav1.Time `json:"lastEvaluatedAt,omitempty,omitzero"` } // ConditionEvaluationResult provides a detailed report of the comparison between diff --git a/api/v1alpha1/zz_generated.deepcopy.go b/api/v1alpha1/zz_generated.deepcopy.go index 6c25c497..2b5cddd3 100644 --- a/api/v1alpha1/zz_generated.deepcopy.go +++ b/api/v1alpha1/zz_generated.deepcopy.go @@ -98,7 +98,7 @@ func (in *NodeEvaluation) DeepCopyInto(out *NodeEvaluation) { *out = make([]ConditionEvaluationResult, len(*in)) copy(*out, *in) } - in.LastEvaluationTime.DeepCopyInto(&out.LastEvaluationTime) + in.LastEvaluatedAt.DeepCopyInto(&out.LastEvaluatedAt) } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new NodeEvaluation. @@ -114,7 +114,7 @@ func (in *NodeEvaluation) DeepCopy() *NodeEvaluation { // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *NodeFailure) DeepCopyInto(out *NodeFailure) { *out = *in - in.LastEvaluationTime.DeepCopyInto(&out.LastEvaluationTime) + in.LastEvaluatedAt.DeepCopyInto(&out.LastEvaluatedAt) } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new NodeFailure. @@ -354,7 +354,7 @@ func (in *RuleEvaluation) DeepCopyInto(out *RuleEvaluation) { *out = make([]ConditionEvaluationResult, len(*in)) copy(*out, *in) } - in.LastEvaluationTime.DeepCopyInto(&out.LastEvaluationTime) + in.LastEvaluatedAt.DeepCopyInto(&out.LastEvaluatedAt) if in.FirstEvaluatedAt != nil { in, out := &in.FirstEvaluatedAt, &out.FirstEvaluatedAt *out = (*in).DeepCopy() diff --git a/config/crd/bases/readiness.node.x-k8s.io_nodereadinessevaluations.yaml b/config/crd/bases/readiness.node.x-k8s.io_nodereadinessevaluations.yaml index e8e55933..10fdcbfc 100644 --- a/config/crd/bases/readiness.node.x-k8s.io_nodereadinessevaluations.yaml +++ b/config/crd/bases/readiness.node.x-k8s.io_nodereadinessevaluations.yaml @@ -162,9 +162,9 @@ spec: assessed against this node. format: date-time type: string - lastEvaluationTime: - description: lastEvaluationTime records the exact moment the - controller most recently assessed this rule. + lastEvaluatedAt: + description: lastEvaluatedAt records the exact moment the controller + most recently assessed this rule. format: date-time type: string message: @@ -302,7 +302,7 @@ spec: - Absent type: string required: - - lastEvaluationTime + - lastEvaluatedAt - ruleName - ruleStatus - ruleUID diff --git a/config/crd/bases/readiness.node.x-k8s.io_nodereadinessrules.yaml b/config/crd/bases/readiness.node.x-k8s.io_nodereadinessrules.yaml index 85b617cf..3401a7b5 100644 --- a/config/crd/bases/readiness.node.x-k8s.io_nodereadinessrules.yaml +++ b/config/crd/bases/readiness.node.x-k8s.io_nodereadinessrules.yaml @@ -341,9 +341,9 @@ spec: description: NodeFailure provides diagnostic details for Nodes that could not be successfully evaluated by the rule. properties: - lastEvaluationTime: - description: lastEvaluationTime is the timestamp of the last - rule check failed for this Node. + lastEvaluatedAt: + description: lastEvaluatedAt is the timestamp of the last rule + check failed for this Node. format: date-time type: string message: @@ -369,7 +369,7 @@ spec: minLength: 1 type: string required: - - lastEvaluationTime + - lastEvaluatedAt - nodeName type: object maxItems: 5000 @@ -439,8 +439,8 @@ spec: x-kubernetes-list-map-keys: - type x-kubernetes-list-type: map - lastEvaluationTime: - description: lastEvaluationTime is the timestamp when the controller + lastEvaluatedAt: + description: lastEvaluatedAt is the timestamp when the controller last assessed this Node. format: date-time type: string @@ -459,7 +459,7 @@ spec: type: string required: - conditionResults - - lastEvaluationTime + - lastEvaluatedAt - nodeName - taintStatus type: object diff --git a/internal/controller/node_controller.go b/internal/controller/node_controller.go index def0bcfb..7d183ca7 100644 --- a/internal/controller/node_controller.go +++ b/internal/controller/node_controller.go @@ -508,7 +508,7 @@ func (r *RuleReadinessController) recordNodeFailure( NodeName: nodeName, Reason: reason, Message: message, - LastEvaluationTime: metav1.Now(), + LastEvaluatedAt: metav1.Now(), }) rule.Status.FailedNodes = failedNodes diff --git a/internal/controller/node_controller_test.go b/internal/controller/node_controller_test.go index 3eeba977..937e3a5a 100644 --- a/internal/controller/node_controller_test.go +++ b/internal/controller/node_controller_test.go @@ -749,7 +749,7 @@ var _ = Describe("Node Controller", func() { Expect(nodeEval.ConditionResults[0].RequiredStatus).To(Equal(corev1.ConditionTrue)) Expect(nodeEval.ConditionResults[0].DefaultStatus).To(Equal(corev1.ConditionUnknown)) Expect(nodeEval.TaintStatus).To(Equal(nodereadinessiov1alpha1.TaintStatusPresent)) - Expect(nodeEval.LastEvaluationTime.IsZero()).To(BeFalse(), "LastEvaluationTime should be set") + Expect(nodeEval.LastEvaluatedAt.IsZero()).To(BeFalse(), "LastEvaluatedAt should be set") }) It("should update existing NodeEvaluation when node is re-evaluated", func() { diff --git a/internal/controller/nodereadinessevaluation_controller.go b/internal/controller/nodereadinessevaluation_controller.go index 42c6f5d8..c3ec8594 100644 --- a/internal/controller/nodereadinessevaluation_controller.go +++ b/internal/controller/nodereadinessevaluation_controller.go @@ -182,7 +182,7 @@ func (r *RuleReadinessController) buildRuleEvaluation( Reason: reason, Message: message, ReadinessConditions: conditionResults, - LastEvaluationTime: now, + LastEvaluatedAt: now, } // FirstEvaluatedAt: set once, carried forward on subsequent evaluations. diff --git a/internal/controller/nodereadinessevaluation_controller_test.go b/internal/controller/nodereadinessevaluation_controller_test.go index a7004c43..44608c2f 100644 --- a/internal/controller/nodereadinessevaluation_controller_test.go +++ b/internal/controller/nodereadinessevaluation_controller_test.go @@ -188,7 +188,7 @@ var _ = Describe("NodeReadinessEvaluation writes", func() { Expect(nre.Status.Rules[0].RuleUID).To(Equal(rule.GetUID())) Expect(nre.Status.Rules[0].RuleStatus).To(Equal(nodereadinessiov1alpha1.RuleStatusUnsatisfied)) Expect(nre.Status.Rules[0].TaintStatus).To(Equal(nodereadinessiov1alpha1.TaintStatusPresent)) - Expect(nre.Status.Rules[0].LastEvaluationTime.IsZero()).To(BeFalse()) + Expect(nre.Status.Rules[0].LastEvaluatedAt.IsZero()).To(BeFalse()) }) It("A3 — state transitions to Available after conditions are met and taint is removed", func() { diff --git a/internal/controller/nodereadinessrule_controller_test.go b/internal/controller/nodereadinessrule_controller_test.go index 7c8a96a4..f17b4b50 100644 --- a/internal/controller/nodereadinessrule_controller_test.go +++ b/internal/controller/nodereadinessrule_controller_test.go @@ -2523,7 +2523,7 @@ var _ = Describe("NodeReadinessRule Controller", func() { NodeName: "stale-recovery-node", Reason: "EvaluationError", Message: "stale from previous reconcile", - LastEvaluationTime: metav1.Now(), + LastEvaluatedAt: metav1.Now(), }, }, }, From 99a1ca23c4e99ec285a73499aa0ffaabef6799b6 Mon Sep 17 00:00:00 2001 From: Karthik Bhat Date: Tue, 15 Sep 2026 12:05:41 +0530 Subject: [PATCH 4/4] Rebase with main and add conditionpolicy --- api/v1alpha1/nodereadinessevaluation_types.go | 7 +++++++ ...nodereadinessrules.readiness.node.x-k8s.io.yaml | 14 +++++++------- ...ess.node.x-k8s.io_nodereadinessevaluations.yaml | 10 ++++++++++ internal/controller/node_controller.go | 6 +++--- .../nodereadinessevaluation_controller.go | 13 +++++++++++-- .../nodereadinessevaluation_controller_test.go | 3 ++- .../nodereadinessrule_controller_test.go | 14 +++++++------- 7 files changed, 47 insertions(+), 20 deletions(-) diff --git a/api/v1alpha1/nodereadinessevaluation_types.go b/api/v1alpha1/nodereadinessevaluation_types.go index 6461ffbe..1109d924 100644 --- a/api/v1alpha1/nodereadinessevaluation_types.go +++ b/api/v1alpha1/nodereadinessevaluation_types.go @@ -146,6 +146,13 @@ type RuleEvaluation struct { // +kubebuilder:validation:Enum=NoSchedule;PreferNoSchedule;NoExecute TaintEffect corev1.TaintEffect `json:"taintEffect,omitempty"` + // conditionPolicy is the aggregation policy used when evaluating the conditions list, + // stamped at evaluation time so this entry is self-contained without requiring a lookup + // of the rule. Matches rule.spec.conditionPolicy. + // + // +required + ConditionPolicy ConditionPolicy `json:"conditionPolicy,omitempty"` + // reason contains a concise, machine-readable string detailing the primary outcome. // // +optional diff --git a/charts/node-readiness-controller/crds/nodereadinessrules.readiness.node.x-k8s.io.yaml b/charts/node-readiness-controller/crds/nodereadinessrules.readiness.node.x-k8s.io.yaml index 85b617cf..3401a7b5 100644 --- a/charts/node-readiness-controller/crds/nodereadinessrules.readiness.node.x-k8s.io.yaml +++ b/charts/node-readiness-controller/crds/nodereadinessrules.readiness.node.x-k8s.io.yaml @@ -341,9 +341,9 @@ spec: description: NodeFailure provides diagnostic details for Nodes that could not be successfully evaluated by the rule. properties: - lastEvaluationTime: - description: lastEvaluationTime is the timestamp of the last - rule check failed for this Node. + lastEvaluatedAt: + description: lastEvaluatedAt is the timestamp of the last rule + check failed for this Node. format: date-time type: string message: @@ -369,7 +369,7 @@ spec: minLength: 1 type: string required: - - lastEvaluationTime + - lastEvaluatedAt - nodeName type: object maxItems: 5000 @@ -439,8 +439,8 @@ spec: x-kubernetes-list-map-keys: - type x-kubernetes-list-type: map - lastEvaluationTime: - description: lastEvaluationTime is the timestamp when the controller + lastEvaluatedAt: + description: lastEvaluatedAt is the timestamp when the controller last assessed this Node. format: date-time type: string @@ -459,7 +459,7 @@ spec: type: string required: - conditionResults - - lastEvaluationTime + - lastEvaluatedAt - nodeName - taintStatus type: object diff --git a/config/crd/bases/readiness.node.x-k8s.io_nodereadinessevaluations.yaml b/config/crd/bases/readiness.node.x-k8s.io_nodereadinessevaluations.yaml index 10fdcbfc..de03c246 100644 --- a/config/crd/bases/readiness.node.x-k8s.io_nodereadinessevaluations.yaml +++ b/config/crd/bases/readiness.node.x-k8s.io_nodereadinessevaluations.yaml @@ -157,6 +157,15 @@ spec: description: RuleEvaluation defines the outcome of evaluating a single NodeReadinessRule against this Node. properties: + conditionPolicy: + description: |- + conditionPolicy is the aggregation policy used when evaluating the conditions list, + stamped at evaluation time so this entry is self-contained without requiring a lookup + of the rule. Matches rule.spec.conditionPolicy. + enum: + - allOf + - anyOf + type: string firstEvaluatedAt: description: firstEvaluatedAt is the time the rule was first assessed against this node. @@ -302,6 +311,7 @@ spec: - Absent type: string required: + - conditionPolicy - lastEvaluatedAt - ruleName - ruleStatus diff --git a/internal/controller/node_controller.go b/internal/controller/node_controller.go index 7d183ca7..17295378 100644 --- a/internal/controller/node_controller.go +++ b/internal/controller/node_controller.go @@ -505,9 +505,9 @@ func (r *RuleReadinessController) recordNodeFailure( // Add new failure failedNodes = append(failedNodes, readinessv1alpha1.NodeFailure{ - NodeName: nodeName, - Reason: reason, - Message: message, + NodeName: nodeName, + Reason: reason, + Message: message, LastEvaluatedAt: metav1.Now(), }) diff --git a/internal/controller/nodereadinessevaluation_controller.go b/internal/controller/nodereadinessevaluation_controller.go index c3ec8594..8ba7ba3b 100644 --- a/internal/controller/nodereadinessevaluation_controller.go +++ b/internal/controller/nodereadinessevaluation_controller.go @@ -132,13 +132,17 @@ func (r *RuleReadinessController) buildRuleEvaluation( now := metav1.Now() // Evaluate all conditions. + conditionPolicy := rule.Spec.GetConditionPolicy() allConditionsSatisfied := true + anyConditionSatisfied := false conditionResults := make([]readinessv1alpha1.ConditionEvaluationResult, 0, len(rule.Spec.Conditions)) for _, condReq := range rule.Spec.Conditions { effectiveStatus, conditionFound := r.getConditionStatus(node, condReq.Type, condReq.GetDefaultStatus()) satisfied := effectiveStatus == condReq.RequiredStatus if !satisfied { allConditionsSatisfied = false + } else { + anyConditionSatisfied = true } observedStatus := effectiveStatus if !conditionFound { @@ -152,8 +156,12 @@ func (r *RuleReadinessController) buildRuleEvaluation( }) } + satisfied := allConditionsSatisfied + if conditionPolicy == readinessv1alpha1.ConditionPolicyAnyOf { + satisfied = anyConditionSatisfied + } ruleStatus := readinessv1alpha1.RuleStatusSatisfied - if !allConditionsSatisfied { + if !satisfied { ruleStatus = readinessv1alpha1.RuleStatusUnsatisfied } @@ -179,10 +187,11 @@ func (r *RuleReadinessController) buildRuleEvaluation( TaintStatus: taintStatus, TaintKey: rule.Spec.Taint.Key, TaintEffect: rule.Spec.Taint.Effect, + ConditionPolicy: conditionPolicy, Reason: reason, Message: message, ReadinessConditions: conditionResults, - LastEvaluatedAt: now, + LastEvaluatedAt: now, } // FirstEvaluatedAt: set once, carried forward on subsequent evaluations. diff --git a/internal/controller/nodereadinessevaluation_controller_test.go b/internal/controller/nodereadinessevaluation_controller_test.go index 44608c2f..4cea1ede 100644 --- a/internal/controller/nodereadinessevaluation_controller_test.go +++ b/internal/controller/nodereadinessevaluation_controller_test.go @@ -793,7 +793,8 @@ var _ = Describe("NodeReadinessEvaluation writes", func() { nodeList := &corev1.NodeList{} Expect(k8sClient.List(ctx, nodeList)).To(Succeed()) - Expect(rc.processAllNodesForRule(ctx, rule, nodeList)).To(Succeed()) + _, err := rc.processAllNodesForRule(ctx, rule, nodeList) + Expect(err).To(Succeed()) nre := getNRE(node.Name) Expect(nre.Spec.NodeName).To(Equal(node.Name)) diff --git a/internal/controller/nodereadinessrule_controller_test.go b/internal/controller/nodereadinessrule_controller_test.go index f17b4b50..56403a55 100644 --- a/internal/controller/nodereadinessrule_controller_test.go +++ b/internal/controller/nodereadinessrule_controller_test.go @@ -1852,10 +1852,10 @@ var _ = Describe("NodeReadinessRule Controller", func() { Expect(k8sClient.Get(ctx, types.NamespacedName{Name: "delete-node-rule"}, seededRule)).To(Succeed()) statusPatch := client.MergeFrom(seededRule.DeepCopy()) seededRule.Status.FailedNodes = append(seededRule.Status.FailedNodes, nodereadinessiov1alpha1.NodeFailure{ - NodeName: "node1", - Reason: "EvaluationError", - Message: "test failure", - LastEvaluationTime: metav1.Now(), + NodeName: "node1", + Reason: "EvaluationError", + Message: "test failure", + LastEvaluatedAt: metav1.Now(), }) Expect(k8sClient.Status().Patch(ctx, seededRule, statusPatch)).To(Succeed()) @@ -2520,9 +2520,9 @@ var _ = Describe("NodeReadinessRule Controller", func() { Status: nodereadinessiov1alpha1.NodeReadinessRuleStatus{ FailedNodes: []nodereadinessiov1alpha1.NodeFailure{ { - NodeName: "stale-recovery-node", - Reason: "EvaluationError", - Message: "stale from previous reconcile", + NodeName: "stale-recovery-node", + Reason: "EvaluationError", + Message: "stale from previous reconcile", LastEvaluatedAt: metav1.Now(), }, },