Enhanced Stability, Pause Logic, and Sentinel Improvements#59
Open
Enhanced Stability, Pause Logic, and Sentinel Improvements#59
Conversation
- Update function call from IsPodAnnonationDiff to IsPodAnnotationDiff - Maintain all other resource cleanup and finalizer functionality
- Add comprehensive IsServiceChanged function for detailed service comparison - Refactor actor ensure resource ordering and method names - Improve service change detection with proper label/annotation comparison - Enhance statefulset handling with better error checking - Add utility functions for service port and spec comparison
Refactored the annotation merging logic to specifically handle the `RestartAnnotationKey`. - Introduced `MergeRestartAnnotation` to compare and merge restart annotations based on timestamps. - Replaced the generic `MergeAnnotations` function with the new specialized function. - Ensured that the restart annotation is correctly propagated during updates.
- Modified the pause logic in the cluster, failover, and sentinel controllers to requeue the resource if nodes still exist, allowing them to scale down gracefully. The operator will now pause reconciliation only after all pods have been terminated. - Increased the initial delay for the sentinel startup probe to 30 seconds to prevent premature failures on slower systems.
- Modified cluster, failover, and sentinel command implementations - Updated initialization scripts for different deployment modes - Improved helper functionality across command modules 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
There was a problem hiding this comment.
Pull Request Overview
This pull request introduces comprehensive enhancements to the Valkey operator focusing on stability improvements, refined pause/resume functionality, and robust sentinel failover handling. The changes modernize string operations, improve service comparison logic, enhance config management with better version tracking, and strengthen resource cleanup mechanisms.
- Major refactor of string operations from
strings.Replacetostrings.ReplaceAlland adoption ofstrings.SplitSeqfor better performance - Enhanced service comparison using
go-cmplibrary with proper empty value handling and comprehensive field validation - Improved configuration management with last-applied-config tracking and better version upgrade handling
Reviewed Changes
Copilot reviewed 60 out of 60 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| pkg/valkey/valkey.go | Updates string operations to use strings.SplitSeq for improved iteration |
| pkg/kubernetes/clientset/service.go | Enhances service comparison logic with go-cmp library |
| internal/valkey/node.go | Modernizes string replacement using strings.ReplaceAll |
| internal/util/kubernetes.go | Major refactor of comparison logic and addition of comprehensive service change detection |
| internal/ops//actor/.go | Updates resource management with better service handling and configuration tracking |
| internal/builder/*.go | Improves annotation merging and configuration management |
| cmd/helper/commands//.go | Simplifies service access logic and removes redundant environment variables |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Signed-off-by: Seer <kvcnow@gmail.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 62 out of 63 changed files in this pull request and generated 6 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This release introduces significant improvements to the Valkey operator, focusing on stability enhancements, refined pause/resume functionality, and robust sentinel
failover handling. Key features include improved configuration management, better resource cleanup, and enhanced monitoring capabilities.
Changes
🚀 New Features
🐛 Bug Fixes
🔧 Technical Improvements
Test Plan
Files Changed
The changes span across 50+ files including controller logic, builder patterns, operator engines, and Valkey client implementations, with focus on cluster, failover, and sentinel operations.
This release significantly improves the operator's reliability and handling of edge cases in production environments.