-
Notifications
You must be signed in to change notification settings - Fork 184
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
feat: support in-place update for pod #7000
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #7000 +/- ##
==========================================
+ Coverage 65.64% 65.79% +0.14%
==========================================
Files 340 340
Lines 41402 41678 +276
==========================================
+ Hits 27180 27421 +241
- Misses 11876 11905 +29
- Partials 2346 2352 +6
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
for _, container := range src.Spec.InitContainers { | ||
for i, c := range dst.Spec.InitContainers { | ||
if container.Name == c.Name { | ||
dst.Spec.InitContainers[i].Image = container.Image |
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.
add a break?
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.
fixed: 382925a
dst.Spec.ActiveDeadlineSeconds = src.Spec.ActiveDeadlineSeconds | ||
mergeList(&src.Spec.Tolerations, &dst.Spec.Tolerations, func(item corev1.Toleration) func(corev1.Toleration) bool { | ||
return func(t corev1.Toleration) bool { | ||
return false |
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.
Why does it directly return false here? If the src Tolerations is not empty, won't the dst keep growing? And there's no deduplication.
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.
Tolerations
is append-only according to the description of the K8s docs.
but the append logic here is a problem, should be a replacement.
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.
fixed: 054f0ac
pkg/controller/rsm2/instance_util.go
Outdated
mergeList(&template.Tolerations, &templateExt.Spec.Tolerations, | ||
func(item corev1.Toleration) func(corev1.Toleration) bool { | ||
return func(t corev1.Toleration) bool { | ||
return false |
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.
ditto
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.
fixed: 9852672
if err != nil { | ||
return false | ||
} | ||
if semver.Compare(kubeVersion, "v1.29") >= 0 { |
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.
in place update is available since 1.27 (alpha), if there aren't two much differences(for this part) between 1.27 and 1.29, it can be set to 1.27
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.
should do a InPlacePodVerticalScaling
feature gate test in versions less than 1.29, as line 57 did.
/cherry-pick release-0.9 |
🤖 says: Error cherry-picking. |
🤖 says: |
Background
In StatefulSet, Deployment, and DaemonSet, any changes to the PodTemplate fields result in Pod recreation, which is not ideal for applications with high availability requirements, such as databases.
In Kubernetes versions below 1.27, the Pod API supports in-place updates for certain fields. Starting from version 1.27, the Resources field also supports in-place updates.
On one hand, based on feedback from the community and our customers, there is a strong demand for in-place updates. On the other hand, to meet the community's demand for in-place updates, several open-source or self-developed projects have emerged, such as Openkruise and the self-developed Resources In-Place Update by Kuaishou, as well as our own solution in the KubeBlocks commercial version.
The goal of this solution is to enable RSM to support the in-place update fields supported by the Pod API. Specifically:
labels
,annotations
,spec.containers[*].image
,spec.initContainers[*].image
,spec.activeDeadlineSeconds
, andspec.tolerations
.spec.containers[*].resources
.IgnorePodVerticalScaling
feature gate to adapt to self-developed solutions likeInPlacePodVerticalScaling
by Kuaishou and KubeBlocks commercial.Implementation
API
This solution does not involve any API changes.
RSM
The issue with this design is that the consistency between RSM Spec and Pod Spec cannot be guaranteed. When subsequent Pod recreation occurs, Resources will be set to the old values. This issue needs to be resolved by the user, such as updating both RSM Spec and Pod Spec simultaneously.
Testing
After modifying the fields that support in-place updates, the Pod objects are ultimately updated without recreation (i.e., the UID remains unchanged).
fixed #6910
Update 2024/8/7
To resolve 'unknown revision v0.0.0' error caused by dependency of "k8s.io/apiserver/pkg/util/feature", manual replaces are put into the go.mod. Check this for more info.