chore: Set controller UsePriorityQueue to false#2129
Conversation
The UsePriorityQueue field defaults to true since controller-runtime v0.23.0. This reverts the value to match pre-v0.23.0 functionality. See: devfile/devworkspace-operator#1635 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
Skipping CI for Draft Pull Request. |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: rohanKanojia, tolusha The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
There was a problem hiding this comment.
Pull request overview
This PR disables controller-runtime’s priority queue for the Che operator controllers, aligning controller queue behavior with the referenced devworkspace-operator change.
Changes:
- Sets
UsePriorityQueuetofalsein all controllerTypedOptions. - Keeps existing
SkipNameValidationconfiguration unchanged.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
controllers/workspaceconfig/workspaces_config_controller.go |
Disables priority queue for the workspaces config controller. |
controllers/usernamespace/usernamespace_controller.go |
Disables priority queue for the user namespace controller. |
controllers/che/checluster_controller.go |
Disables priority queue for the CheCluster controller. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
/retest |
|
Hi! I'm che-ai-assistant — I help with your pull requests. Available commands:
|
|
/che-ai-assistant ok-pr-review Review is complete. Please check the review comments below. |
tolusha
left a comment
There was a problem hiding this comment.
Comprehensive Review Completed
This PR has been reviewed across three dimensions: standard code review, deep design analysis, and system-level impact assessment.
Summary: Well-executed change, ready for review
What was reviewed:
- ✅ Standard code review
- ✅ Deep design and quality analysis
- ✅ System-level impact review
Findings
No critical issues - The change is well-scoped and safe.
Positive observations:
- Consistent application across all three affected controllers
- Follows existing code patterns (
pointer.Bool()) - Minimal, focused diff - easy to review
- Correctly scoped (excludes
devworkspaceandnamespacecachewhich don't useSetupWithManagerwithTypedOptions) - Operationally safe - only affects reconciliation queue ordering, no impact on observability, security, or performance
Optional Suggestion for Future Maintainability
Consider adding a brief inline comment at one of the UsePriorityQueue sites explaining the rationale. This would help future maintainers understand why this setting overrides the controller-runtime v0.23+ default.
For example, in controllers/che/checluster_controller.go:
return bld.WithOptions(
controller.TypedOptions[reconcile.Request]{
SkipNameValidation: pointer.Bool(true),
// Disabled to avoid reconciliation issues - see devfile/devworkspace-operator#1635
UsePriorityQueue: pointer.Bool(false),
}).Complete(r)This is a suggestion for documentation, not a blocking concern.
Technical Analysis
Design Quality: The change follows the existing abstraction pattern. Each controller owns its setup configuration, which is appropriate for independent controller configuration.
Reconciliation Impact: Disabling priority queue reverts from priority-based ordering to FIFO. For che-operator (typically managing a single CheCluster instance), the practical difference is negligible.
Operational Safety: No impact on observability, failure handling, resource lifecycle, network behavior, or security. The change is a simple configuration toggle consumed at manager startup.
🤖 Comprehensive review by Claude Code using ok-pr-review
|
/retest |
What does this PR do?
Set controller UsePriorityQueue to false.
See: devfile/devworkspace-operator#1635
Screenshot/screencast of this PR
N/A
What issues does this PR fix or reference?
N/A
How to test this PR?
N/A
Common Test Scenarios
PR Checklist
As the author of this Pull Request I made sure that:
Reviewers
Reviewers, please comment how you tested the PR when approving it.