docs: add in-place workload vertical scaling proposal #1336
Conversation
Sonatype Lift is retiringSonatype Lift will be retiring on Sep 12, 2023, with its analysis stopping on Aug 12, 2023. We understand that this news may come as a disappointment, and Sonatype is committed to helping you transition off it seamlessly. If you’d like to retain your data, please export your issues from the web console. |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
Welcome @LavenderQAQ! It looks like this is your first PR to openkruise/kruise 🎉 |
|
For the case of vscale failure, my current idea is to start the rollback mechanism, is it necessary to provide users with some configurable interfaces? Because for vscale, rolling back is more like starting another vscale. |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #1336 +/- ##
==========================================
+ Coverage 48.55% 48.62% +0.06%
==========================================
Files 157 157
Lines 22480 22480
==========================================
+ Hits 10916 10930 +14
+ Misses 10360 10349 -11
+ Partials 1204 1201 -3
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
|
Do I need to add more details to |
LavenderQAQ
force-pushed
the
docs/inplace-workload-vertical-scaling
branch
from
1dd1eaf
to
d556c31
Compare
|
/lgtm |
|
/intive |
|
@cubxxw: changing LGTM is restricted to collaborators In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
|
/chat |
|
|
||
| ##### Enhance Expectation | ||
|
|
||
| Create a new VScaleExpectations for use by different controllers. |
There was a problem hiding this comment.
why not reuse ResourceVersionExpectations ?
There was a problem hiding this comment.
I think there might be some judgment here about the vscale status, many Pods might be in the Proposed or InProgress state, and there seems to be no way to judge only resource_version.
There was a problem hiding this comment.
I think there might be some judgment here about the vscale status, many Pods might be in the Proposed or InProgress state, and there seems to be no way to judge only resource_version.
ok, I got it :）
| type expectationDiffs struct { | ||
| ... | ||
| // vscale identifies whether resources have changed | ||
| vscale bool |
There was a problem hiding this comment.
Yes, I should change the quantity here. This design is not perfect for the time being, and we need to consider the vertical scaling of some Pods (not all) in the future. Then I'll update it with a commit.
|
|
||
| #### Design Objectives | ||
|
|
||
| - Use vscale as a standalone operation rather than integrated in update. For example, for cloneset, three operations will be improved to scale, vscale, and update. |
There was a problem hiding this comment.
Good idea! But we should consider how to avoid to break maxUnavailable limitation in some scene, for example, users update both resources and images fields, and settings RestartRequired of vscale.
|
|
||
| - Users only need to modify template in workload to achieve vertical scaling for all managed Pods | ||
| - Simple and clear configuration strategy, support grayscale capabilities | ||
| - Provides the rollback function, and attempts to return to the previous state when the capacity expansion and shrinkage fails. If the rollback still fails, no further action is required |
There was a problem hiding this comment.
Maybe, we should provide more policies. For example. if a vscale proposal is rejected by kubelet (Infeasible), we should allow it to be demote to rebuild upgrade.
|
|
||
| ```go | ||
| // CloneSetVScaleStrategy defines strategies for pods vscale. | ||
| type CloneSetVScaleStrategy struct { |
There was a problem hiding this comment.
Maybe, it's better to reuse UpdateStrategy filed for the following considers:
- vscale also should consider
maxUnavailable; - vscale also need grayscale capabilities (
partition) ；
LavenderQAQ
force-pushed
the
docs/inplace-workload-vertical-scaling
branch
from
d556c31
to
888d446
Compare
|
Sorry, there may be some significant design updates, and after trying to implement a demo, I believe that treating workload vertical scaling as part of the update operation may be a better choice. We can discuss this new design in the community meeting. I have a wonderful demo here. |
|
/hold |
LavenderQAQ
force-pushed
the
docs/inplace-workload-vertical-scaling
branch
2 times, most recently
from
7ed0cf9
to
7bbca45
Compare
|
/unhold |
LavenderQAQ
changed the title
|
/hold |
|
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions. |
LavenderQAQ
force-pushed
the
docs/inplace-workload-vertical-scaling
branch
2 times, most recently
from
60e4bd7
to
9b46a52
Compare
|
/unhold |
LavenderQAQ
force-pushed
the
docs/inplace-workload-vertical-scaling
branch
from
9b46a52
to
0c6aa76
Compare
|
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions. |
Signed-off-by: LavenderQAQ <1254297317@qq.com>
LavenderQAQ
force-pushed
the
docs/inplace-workload-vertical-scaling
branch
from
0c6aa76
to
f9116e7
Compare
Describe what this PR does
Add a proposal for In-place Workload Vertical Scaling.
Fixes #1212