Update metalnet load balancers on instance updates#487
Conversation
Fix missing updates metalnet load balancers when the desired spec via an instance does not match the current metalnet load balancer spec. Add test case verifying this behavior.
📝 WalkthroughWalkthroughThis change adds semantic equality support for Metalnet API types and implements logic to update existing Metalnet load balancers when their specifications differ from desired values during instance reconciliation. The controller now compares current vs. desired specs and applies patches when changes are detected. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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 |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@metalnetlet/controllers/instance_controller_test.go`:
- Around line 63-69: The update callback passed to Eventually(Update(...)) must
be idempotent: replace the append mutation with a direct assignment to
inst.Spec.LoadBalancerPorts using the full desired slice (e.g.
inst.Spec.LoadBalancerPorts = []v1alpha1.LoadBalancerPort{ { Protocol:
&protocol, Port: 1001 } }) so retries won't duplicate port 1001; update the
callback that currently calls append to assign the slice instead, keeping the
same types (v1alpha1.LoadBalancerPort) and values.
In `@metalnetlet/controllers/instance_controller.go`:
- Around line 155-166: The current updateMetalnetLoadBalancerIfNecessary builds
desiredSpec from the existing metalnetLoadBalancer state, preventing drift
detection for NetworkRef, NodeName and LBtype; change
updateMetalnetLoadBalancerIfNecessary to accept the current desired inputs
(e.g., networkUID types.UID, metalnetNodeName string, metalnetLoadBalancerType
metalnetv1alpha1.LoadBalancerType, and the matched ip) and call
r.metalnetLoadBalancerSpec(inst, networkUID, metalnetNodeName,
metalnetLoadBalancerType, ip) so desiredSpec is derived from current Instance
inputs; then update the caller manageMetalnetLoadBalancers to pass those current
values when invoking updateMetalnetLoadBalancerIfNecessary (also fix the
analogous call/site referenced at the other location).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 1814388e-c141-4bcc-97c6-768b409ceece
📒 Files selected for processing (3)
metalnetlet/controllers/common.gometalnetlet/controllers/instance_controller.gometalnetlet/controllers/instance_controller_test.go
Fix missing updates metalnet load balancers when the desired spec via an instance does not match the current metalnet load balancer spec.
Add test case verifying this behavior.
Summary by CodeRabbit
Bug Fixes
Tests