feat: add --node-condition flag and kubectl-get-like node output columns#31
feat: add --node-condition flag and kubectl-get-like node output columns#31
Conversation
Co-authored-by: alikhil <7482065+alikhil@users.noreply.github.com>
Co-authored-by: alikhil <7482065+alikhil@users.noreply.github.com>
pkg/handlers/universal.go
Outdated
| return true | ||
| } | ||
|
|
||
| if len(options.NodeConditions) > 0 { |
There was a problem hiding this comment.
This does not really align with current design of UniversalHandler, it supposed to handle all resources, however node conditions are heavily node resource related thing. Check code of pod handler, you will see that it handles special pod-only filters.
Evaluate current design, maybe it worth creating nodeHandler, or refactor everything to have only 1 handler, or other better option. Ask for approval before implementing
There was a problem hiding this comment.
Agreed — node condition filtering doesn't belong in UniversalHandler. Here are the options I evaluated:
Option A: Create a dedicated NodeHandler (like PodHandler)
- Follows the existing PodHandler pattern
- But duplicates ~100 lines of list/delete/patch action handling for just one extra filter
Option B: Add an optional ResourceMatcher callback to UniversalHandlerOptions
- UniversalHandler stays generic — it just calls the injected matcher alongside its own checks in
resourceMatches() - Node-condition matching logic is passed in during handler creation in
GetResourceHandler(similar to howGetColumnsForalready customizes columns per resource) - Zero code duplication, scales for future resource-specific filters
nodeConditionMatchesmoves out ofUniversalHandlerentirely
Option C: Refactor to a single handler
- Would require merging PodHandler's typed API + exec support into the universal handler — large refactor with risk of breaking existing behavior
My recommendation is Option B — it keeps UniversalHandler truly universal, requires minimal changes, and follows the same injection pattern already used for columns. The node-specific matching function would live in a separate file (e.g. nodes.go) and be wired in via GetResourceHandler.
Which approach would you prefer?
There was a problem hiding this comment.
Implemented Option B in commit a0944b2:
- Added
ResourceMatchercallback type toUniversalHandlerOptions—UniversalHandlerstays generic, just calls the injected matcher inresourceMatches() - Moved node condition logic to new
nodes.goasNodeConditionMatcher()/nodeConditionMatches() - Added
getResourceMatcher()inresource.goto wire up the correct matcher per resource type - Tests updated to use
getResourceMatcher()— all pass
…sourceMatcher injection Co-authored-by: alikhil <7482065+alikhil@users.noreply.github.com>
Adds filtering nodes by conditions (
--node-condition Ready=True,--node-condition DiskPressure=False) including custom conditions from NPD, EKS Node Monitoring Agent, etc. Also makeskubectl fd nodesoutput matchkubectl get nodeswith STATUS, ROLES, and VERSION columns.Changes
resource.go:NodeTypeGVR constant,NodeConditionstruct, new field onActionOptions,getResourceMatcher()to wire up resource-specific matchersfind.go:--node-conditionflag withConditionType=Statusformat validation, scoped to node resources onlycolumns.go: Node-specific columns — STATUS (Ready/NotReady + SchedulingDisabled), ROLES (fromnode-role.kubernetes.io/*labels), VERSION (kubelet)nodes.go(new):NodeConditionMatcher()returns aResourceMatcherfor node condition filtering — case-insensitive, all specified conditions must match (AND)universal.go: Added injectableResourceMatchercallback type and field onUniversalHandlerOptions, keepingUniversalHandlergeneric — node-specific logic is fully decoupledDesign
Node condition matching uses an injectable
ResourceMatchercallback pattern rather than being embedded directly inUniversalHandler. This keeps the universal handler truly universal while supporting resource-specific filters. The matcher is wired up viagetResourceMatcher()inresource.go, following the same injection pattern already used for columns viaGetColumnsFor().Usage
🔒 GitHub Advanced Security automatically protects Copilot coding agent pull requests. You can protect all pull requests by enabling Advanced Security for your repositories. Learn more about Advanced Security.