-
Notifications
You must be signed in to change notification settings - Fork 121
fix: avoid extra node creation on informercache delay #1311
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix: avoid extra node creation on informercache delay #1311
Conversation
in scenarios where the informercache experiences significant delay, the controller may mistakenly create redundant nodes due to stale state. this change leverages the expectations mechanism. The controller now only reconciles when expectations are fully satisfied, ensuring observed state is sufficiently up-to-date. Signed-off-by: zhuangqh <zhuangqhc@gmail.com>
TitleAvoid extra node creation on informercache delay Description
Changes walkthrough 📝
|
PR Reviewer Guide 🔍Here are some key observations to aid the review process:
|
PR Code Suggestions ✨Explore these optional code suggestions:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
adopt suggestion No.4 |
/LGTM |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR implements an expectations mechanism to prevent duplicate node creation when the informer cache experiences delays. The controller now uses expectations to track pending NodeClaim creations and only proceeds with reconciliation when expectations are satisfied, ensuring the observed state is up-to-date before making scaling decisions.
Key changes include:
- Implementation of a ControllerExpectations system based on Kubernetes patterns
- Modified workspace controller to use expectations before creating new nodes
- Updated event handling to track NodeClaim creation events and satisfy expectations
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
File | Description |
---|---|
pkg/workspace/controllers/workspace_event_handler.go | New event handler that tracks NodeClaim creation events to satisfy controller expectations |
pkg/workspace/controllers/workspace_controller_test.go | Removes obsolete test for node creation validation logic |
pkg/workspace/controllers/workspace_controller.go | Integrates expectations mechanism and modifies node creation flow to prevent duplicates |
pkg/utils/controller.go | Implements ControllerExpectations utility copied from Kubernetes core |
The e2e test passed 3 times. |
Reason for Change:
in scenarios where the informercache experiences significant delay,
the controller may mistakenly create redundant nodes due to stale state.
this change leverages the expectations mechanism. The controller now
only reconciles when expectations are fully satisfied, ensuring observed
state is sufficiently up-to-date.
Requirements
Issue Fixed:
Fixes #1300
Notes for Reviewers:
reference: https://github.com/kubernetes/kubernetes/blob/master/pkg/controller/replicaset/replica_set.go