Conversation
- Update Go version from 1.23.0 to 1.24.0 - Update cert-manager from v1.9.1 to v1.18.2 - Update Kubernetes dependencies to v0.32.0 - Update controller-runtime to v0.19.0 - Update various other dependencies to latest versions 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
WalkthroughBumps Go toolchain and numerous dependencies (notably Kubernetes and controller-runtime). Updates PVC spec to use VolumeResourceRequirements in two places. Adjusts controller watch CreateFunc signatures to typed rate-limiting queues. Adds explicit discard of slices.IndexFunc return values in three ops files. No functional control-flow changes. Changes
Sequence Diagram(s)Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
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.2.2)Error: can't load config: unsupported version of the configuration: "" See https://golangci-lint.run/product/migration-guide for migration instructions ✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Pull Request Overview
This PR updates Go dependencies to their latest versions, including a major Go version bump from 1.23.0 to 1.24.0. The update includes significant upgrades to Kubernetes dependencies (v0.32.0), controller-runtime (v0.19.0), and cert-manager (v1.18.2), along with numerous other dependency updates.
- Updates Go from version 1.23.0 to 1.24.0
- Updates Kubernetes ecosystem dependencies to v0.32.0
- Adapts code to handle API changes in updated dependencies
Reviewed Changes
Copilot reviewed 7 out of 8 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| go.mod | Updates all Go dependencies to latest versions including Go 1.24.0 |
| internal/ops/sentinel/actor/actor_heal_pod.go | Adds blank identifier to handle unused return value from slices.IndexFunc |
| internal/ops/failover/engine.go | Adds blank identifier to handle unused return value from slices.IndexFunc |
| internal/ops/failover/actor/actor_patch_labels.go | Adds blank identifier to handle unused return value from slices.IndexFunc |
| internal/controller/middleware/redis_controller.go | Updates workqueue interface to use typed version for controller-runtime compatibility |
| internal/controller/middleware/redis/redisfailover.go | Updates PVC resource type from ResourceRequirements to VolumeResourceRequirements |
| internal/builder/clusterbuilder/statefulset.go | Updates PVC resource type from ResourceRequirements to VolumeResourceRequirements |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
internal/ops/failover/actor/actor_patch_labels.go (1)
66-71: Bug: missing return after RequeueWithError causes fall-through.On error fetching master, the code logs and calls
actor.RequeueWithError(err)but does not return, so execution proceeds with an invalidmasterNode/masterAddr.Apply this diff:
func (a *actorPatchLabels) Do(ctx context.Context, val types.RedisInstance) *actor.ActorResult { @@ masterNode, err := inst.Monitor().Master(ctx) if err != nil { logger.Error(err, "get master failed") - actor.RequeueWithError(err) + return actor.RequeueWithError(err) }internal/controller/middleware/redis/redisfailover.go (1)
304-317: Bug in resourceDiff: CPU limits never compared.The last comparison repeats memory; CPU limit diffs won’t be detected and could suppress needed reconciles.
Apply this diff:
func resourceDiff(r1 corev1.ResourceRequirements, r2 corev1.ResourceRequirements) bool { if result := r1.Requests.Cpu().Cmp(*r2.Requests.Cpu()); result != 0 { return true } if result := r1.Requests.Memory().Cmp(*r2.Requests.Memory()); result != 0 { return true } - if result := r1.Limits.Memory().Cmp(*r2.Limits.Memory()); result != 0 { + if result := r1.Limits.Memory().Cmp(*r2.Limits.Memory()); result != 0 { return true } - if result := r1.Limits.Memory().Cmp(*r2.Limits.Memory()); result != 0 { + if result := r1.Limits.Cpu().Cmp(*r2.Limits.Cpu()); result != 0 { return true } return false }
🧹 Nitpick comments (4)
internal/ops/failover/actor/actor_patch_labels.go (1)
79-87: Prefer a direct range lookup for clarity.Using
slices.IndexFuncsolely for side effects is non-obvious. A simple range with early break is clearer and avoids the blank identifier.Apply this diff:
- var node redis.RedisNode - _ = slices.IndexFunc(inst.Nodes(), func(i redis.RedisNode) bool { - if i.GetName() == pod.GetName() { - node = i - return true - } - return false - }) + var node redis.RedisNode + for _, n := range inst.Nodes() { + if n.GetName() == pod.GetName() { + node = n + break + } + }internal/builder/clusterbuilder/statefulset.go (1)
27-29: Duplicate import alias for apps/v1.Both
appsv1 "k8s.io/api/apps/v1"andappv1 "k8s.io/api/apps/v1"are imported; only one alias is necessary.Apply this diff:
- appsv1 "k8s.io/api/apps/v1" - appv1 "k8s.io/api/apps/v1" + appsv1 "k8s.io/api/apps/v1"and update the signature below to use
appsv1:-func IsStatefulsetChanged(newSts, sts *appv1.StatefulSet, logger logr.Logger) bool { +func IsStatefulsetChanged(newSts, sts *appsv1.StatefulSet, logger logr.Logger) bool {internal/ops/sentinel/actor/actor_heal_pod.go (1)
144-147: Fix typos in user-facing messages (“inconsist” → “inconsistent”).These appear in events/logs; polish helps operators.
Apply this diff:
- inst.SendEventf(corev1.EventTypeWarning, config.EventCleanResource, - "force delete pod with inconsist annotation %s", node.GetName()) + inst.SendEventf(corev1.EventTypeWarning, config.EventCleanResource, + "force delete pod with inconsistent annotation %s", node.GetName())Also applies to: 160-162
internal/ops/failover/engine.go (1)
298-301: LoadBalancer check should also consider Hostname, not just IP.Sentinel path already checks both; align here to avoid false negatives on LB with hostname-only ingress.
Apply this diff:
- if slices.IndexFunc(svc.Status.LoadBalancer.Ingress, func(i corev1.LoadBalancerIngress) bool { - return i.IP == announceIP - }) < 0 { + if slices.IndexFunc(svc.Status.LoadBalancer.Ingress, func(i corev1.LoadBalancerIngress) bool { + return i.IP == announceIP || i.Hostname == announceIP + }) < 0 { return actor.NewResult(CommandHealPod) }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (1)
go.sumis excluded by!**/*.sum
📒 Files selected for processing (7)
go.mod(1 hunks)internal/builder/clusterbuilder/statefulset.go(1 hunks)internal/controller/middleware/redis/redisfailover.go(1 hunks)internal/controller/middleware/redis_controller.go(2 hunks)internal/ops/failover/actor/actor_patch_labels.go(1 hunks)internal/ops/failover/engine.go(1 hunks)internal/ops/sentinel/actor/actor_heal_pod.go(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
internal/ops/sentinel/actor/actor_heal_pod.go (1)
pkg/types/redis/redis.go (1)
RedisSentinelNode(113-138)
🔇 Additional comments (11)
internal/ops/failover/actor/actor_patch_labels.go (1)
81-87: Explicitly discarding IndexFunc result is correct for Go 1.24 toolchain.This silences “unused result” linters while preserving the side-effect lookup. LGTM.
internal/builder/clusterbuilder/statefulset.go (2)
321-326: PVC Resources: switch to VolumeResourceRequirements is correct for k8s v0.32.Type update aligns with upstream API changes. No functional issues seen.
315-330: PVC specs are consistent
AllPersistentVolumeClaimSpecinstances usecorev1.VolumeResourceRequirements; nocorev1.ResourceRequirementswere found in any PVC spec.internal/ops/sentinel/actor/actor_heal_pod.go (1)
89-95: Explicitly ignoring IndexFunc return is fine.Matches the pattern used elsewhere in this PR and unblocks builds with stricter linters.
internal/ops/failover/engine.go (1)
142-149: Discarding IndexFunc result explicitly is good.Keeps the side-effect lookup while satisfying linters. No behavior change.
internal/controller/middleware/redis/redisfailover.go (1)
184-189: PVC Resources type update to VolumeResourceRequirements is correct.Matches k8s API changes; no logic change.
internal/controller/middleware/redis_controller.go (2)
874-874: LGTM! Updated for typed rate-limiting interface.The signature change from
workqueue.RateLimitingInterfacetoworkqueue.TypedRateLimitingInterface[reconcile.Request]is part of the controller-runtime upgrade to v0.19.0, providing better type safety for the queue operations.
891-891: LGTM! Updated for typed rate-limiting interface.The signature change from
workqueue.RateLimitingInterfacetoworkqueue.TypedRateLimitingInterface[reconcile.Request]is part of the controller-runtime upgrade to v0.19.0, providing better type safety for the queue operations.go.mod (3)
3-3: LGTM! Go version upgrade to 1.24.0.The Go version upgrade from 1.23.0 to 1.24.0 aligns with the PR objectives and brings the latest language features and improvements.
27-93: LGTM! Indirect dependencies updated consistently.The indirect dependency updates appear consistent with the major dependency upgrades and align with the newer Go toolchain requirements.
6-24: No deprecated Kubernetes APIs detected; manual compatibility review advised
- Scan found no
v1beta1orextensions/v1beta1usages in our Go code.- Controller-runtime reconcile patterns (
reconcile.Result{}) remain unchanged.- No direct cert-manager API imports or CRD versions detected in code.
Continue with a focused manual review of cert-manager CRD changes and any controller-runtime behavioural tweaks.
build(deps): update Go dependencies to latest versions
- Update Go version from 1.23.0 to 1.24.0
- Update cert-manager from v1.9.1 to v1.18.2
- Update Kubernetes dependencies to v0.32.0
- Update controller-runtime to v0.19.0
- Update various other dependencies to latest versions
Summary by CodeRabbit