Fix node label matching in triggerReconciliation#719
Conversation
Replace reflect.DeepEqual with proper label selector matching. This ensures the operator correctly creates IngressNodeFirewallNodeState objects for nodes added after the operator starts. The previous implementation used DeepEqual which required exact label match. The new implementation uses labels.SelectorFromSet which properly handles label selectors per Kubernetes semantics.
WalkthroughIn ChangesLabel Selector Matching Fix
Estimated code review effort🎯 2 (Simple) | ⏱️ ~5 minutes 🚥 Pre-merge checks | ✅ 13 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (13 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Hi @smulje. Thanks for your PR. I'm waiting for a openshift member to verify that this patch is reasonable to test. If it is, they should reply with Regular contributors should join the org to skip this step. Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. 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. |
|
/ok-to-test |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: danwinship, smulje The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
@smulje: all tests passed! 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. |
Replace reflect.DeepEqual with proper label selector matching.
This ensures the operator correctly creates IngressNodeFirewallNodeState
objects for nodes added after the operator starts.
The previous implementation used DeepEqual which required exact
label match. The new implementation uses labels.SelectorFromSet
which properly handles label selectors per Kubernetes semantics.
- What this PR does and why is it needed
When a new worker node is added to the cluster after the Ingress Node Firewall operator has started, the operator does NOT create an IngressNodeFirewallNodeState object for that node.
Root Cause: The triggerReconciliation function was using reflect.DeepEqual to compare node labels against the IngressNodeFirewall's nodeSelector.MatchLabels. This method requires exact label equality, which doesn't match Kubernetes label
selector semantics.
The Fix: Replace reflect.DeepEqual with the proper labels.SelectorFromSet().Matches() approach. This ensures the operator correctly evaluates whether a node matches the IngressNodeFirewall selector when new nodes are added or existing
nodes are labeled.
- Special notes for reviewers
- How to verify it
Created test image with fix and verified that fix is working :
- Description for the changelog
Fix operator not creating IngressNodeFirewallNodeState for nodes added after startup.
The triggerReconciliation function now uses proper Kubernetes label selector matching
(labels.SelectorFromSet) instead of reflect.DeepEqual, ensuring nodes added dynamically
or through label changes are correctly reconciled.
Summary by CodeRabbit