✨ Add nodereadinessrulereport CRD and controller#133
✨ Add nodereadinessrulereport CRD and controller#133Karthik-K-N wants to merge 1 commit intokubernetes-sigs:mainfrom
Conversation
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: Karthik-K-N The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
✅ Deploy Preview for node-readiness-controller canceled.
|
2f89354 to
598570f
Compare
| } | ||
|
|
||
| // ReadinessReport defines the outcome of evaluating a single NodeReadinessRule against a specific Node. | ||
| type ReadinessReport struct { |
There was a problem hiding this comment.
Not strongly opinionated, but does NodeReadinessEvaluation (since this is extracting out NodeEvaluation from rule.status) a better fit for name?
There was a problem hiding this comment.
sure, thats looks good too.
| // +listType=map | ||
| // +listMapKey=ruleName | ||
| // +kubebuilder:validation:MaxItems=100 | ||
| ReadinessReports []ReadinessReport `json:"readinessReports,omitempty"` |
There was a problem hiding this comment.
wouldn't this new array of objects in NodeReadinessRuleReport affect scalability as that could exceed 1.5MB? It seem to me that we are moving the problem around. IMO, the evaluation results should be independent CRs not listed below a uber struct.
but it feels like we should really find where the burden is before refactoring.
There was a problem hiding this comment.
My thought was given that we might have few rules and this report is per node. It should be good. But can explore more.
There was a problem hiding this comment.
ah okay, having this per node makes sense.
I'm still on it, I'll take a second look later this week.. have been juggling between few things
There was a problem hiding this comment.
Sure no issues, Thank you.
Description
This PR creates nodereadinessrulereport CRD and controller
Related Issue
Fixes #89
Type of Change
Testing
Checklist
make testpassesmake lintpassesDoes this PR introduce a user-facing change?
Doc #(issue)