fix default partition mapping#108
Conversation
and do not override ConfigMap Signed-off-by: Benny Zlotnik <bzlotnik@redhat.com>
📝 WalkthroughWalkthroughThis change introduces centralized constants for unmanaged resource handling and enhances the operator config controller. It adds guards to skip updating resources marked as unmanaged, expands partition config data with new entries, and modifies ConfigMap creation to be non-destructive by only creating if not existing. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@internal/controller/operatorconfig/controller.go`:
- Around line 488-492: The log call in the unmanaged-resource check uses
obj.GetObjectKind().GroupVersionKind().Kind which can be empty for in-memory
objects; change the log to use the reconciled object's GVK
(existing.GetObjectKind().GroupVersionKind().Kind) and, if that is empty, fall
back to a type string such as fmt.Sprintf("%T", obj) before calling r.Log.Info
(keep the same message and keys and still return nil); reference: existing, obj,
unmanagedAnnotationKey, unmanagedAnnotationTrue, and r.Log.Info.
🧹 Nitpick comments (1)
internal/controller/operatorconfig/controller.go (1)
605-616: Create-only semantics won't propagate new partition entries to existing clusters on upgrade.The switch to create-only means clusters that already have the
aib-partition-configConfigMap will not receive the newly added targets (ridesx4_r3,ride4_sa8775p_sx,ride4_sa8775p_sx_legacy, etc.). If this is intentional per the PR objective ("do not override ConfigMap"), consider documenting the upgrade path (e.g., users must manually add new entries or delete the ConfigMap to get defaults).Also, the function name
createOrUpdatePartitionConfigno longer updates — a rename toensurePartitionConfigwould better reflect the new behavior.
| // Skip update if resource is marked as unmanaged | ||
| if annotations := existing.GetAnnotations(); annotations != nil && annotations[unmanagedAnnotationKey] == unmanagedAnnotationTrue { | ||
| r.Log.Info("Skipping update for unmanaged resource", "name", obj.GetName(), "kind", obj.GetObjectKind().GroupVersionKind().Kind) | ||
| return nil | ||
| } |
There was a problem hiding this comment.
obj.GetObjectKind().GroupVersionKind().Kind may log an empty string.
In-memory objects built by your build* helpers typically don't have TypeMeta set, so obj.GetObjectKind().GroupVersionKind().Kind will resolve to "". Consider using existing (which has GVK populated after r.Get) or use fmt.Sprintf("%T", obj) as a fallback.
Proposed fix
- r.Log.Info("Skipping update for unmanaged resource", "name", obj.GetName(), "kind", obj.GetObjectKind().GroupVersionKind().Kind)
+ r.Log.Info("Skipping update for unmanaged resource", "name", obj.GetName(), "kind", existing.GetObjectKind().GroupVersionKind().Kind)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // Skip update if resource is marked as unmanaged | |
| if annotations := existing.GetAnnotations(); annotations != nil && annotations[unmanagedAnnotationKey] == unmanagedAnnotationTrue { | |
| r.Log.Info("Skipping update for unmanaged resource", "name", obj.GetName(), "kind", obj.GetObjectKind().GroupVersionKind().Kind) | |
| return nil | |
| } | |
| // Skip update if resource is marked as unmanaged | |
| if annotations := existing.GetAnnotations(); annotations != nil && annotations[unmanagedAnnotationKey] == unmanagedAnnotationTrue { | |
| r.Log.Info("Skipping update for unmanaged resource", "name", obj.GetName(), "kind", existing.GetObjectKind().GroupVersionKind().Kind) | |
| return nil | |
| } |
🤖 Prompt for AI Agents
In `@internal/controller/operatorconfig/controller.go` around lines 488 - 492, The
log call in the unmanaged-resource check uses
obj.GetObjectKind().GroupVersionKind().Kind which can be empty for in-memory
objects; change the log to use the reconciled object's GVK
(existing.GetObjectKind().GroupVersionKind().Kind) and, if that is empty, fall
back to a type string such as fmt.Sprintf("%T", obj) before calling r.Log.Info
(keep the same message and keys and still return nil); reference: existing, obj,
unmanagedAnnotationKey, unmanagedAnnotationTrue, and r.Log.Info.
1 participant
and do not override ConfigMap
Summary by CodeRabbit
Release Notes
New Features
Improvements