OCPBUGS-6075: nodelink: log and emit events for invalid labels#1496
OCPBUGS-6075: nodelink: log and emit events for invalid labels#1496RadekManak wants to merge 1 commit intoopenshift:mainfrom
Conversation
The nodelink controller silently swallowed errors when copying malformed labels from Machine to Node. The client.Update() returned a 422 but the error was only visible at --v=11 via HTTP round-tripper logs. - Validate labels before attempting the node update using metav1validation.ValidateLabels, catching invalid labels early with a clear error message - Log errors with klog.Errorf (visible at default verbosity) for both validation failures and update errors - Emit Warning events (InvalidNodeLabels, FailedUpdateNode) on the Machine object so failures are visible via oc describe machine
|
@RadekManak: This pull request references Jira Issue OCPBUGS-6075, which is invalid:
Comment The bug has been updated to refer to the pull request using the external bug tracker. DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository: openshift/coderabbit/.coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (2)
WalkthroughThe PR adds event recording and Kubernetes label validation to the NodeLink controller. It introduces an eventRecorder field, validates node labels before updates, and emits warning events when validation fails or updates encounter errors. ChangesNodeLink Controller Event Recording & Label Validation
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 11 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (11 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 golangci-lint (2.12.1)Error: can't load config: unsupported version of the configuration: "" See https://golangci-lint.run/docs/product/migration-guide for migration instructions Comment |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: 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 |
|
/hold clanker created |
|
@RadekManak: The following test failed, say
Full PR test history. Your PR dashboard. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
Summary
metav1validation.ValidateLabelsto catch malformed labels early with a clear error messageklog.Errorf) for both label validation failures and node update errors, so they're visible without needing--v=11Warningevents (InvalidNodeLabels,FailedUpdateNode) on the Machine object so failures surface viaoc describe machineFixes https://issues.redhat.com/browse/OCPBUGS-6075
Test plan
make test)make lint)TestReconcileWithInvalidLabelscovering:bar/baz) are rejected with error + eventSummary by CodeRabbit
New Features
Tests