Skip to content

Commit 4296143

Browse files
fix-inplace-update-with-only-metadata-changed (#348)
* fix-inplace-update-with-only-metadata-changed * refactor comment * refactor e2e * test * Revert "refactor e2e" This reverts commit 3835723.
1 parent 2dbb758 commit 4296143

File tree

4 files changed

+126
-24
lines changed

4 files changed

+126
-24
lines changed

go.mod

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -23,25 +23,26 @@ require (
2323
k8s.io/kubernetes v0.0.0-00010101000000-000000000000
2424
k8s.io/utils v0.0.0-20241104100929-3ea5e8cea738
2525
kusionstack.io/kube-api v0.7.4-0.20250909095208-496f60eea9b5
26-
kusionstack.io/kube-utils v0.2.1-0.20250908100835-947051067cbe
26+
kusionstack.io/kube-utils v0.2.1-0.20251031115408-8f00ba110277
2727
kusionstack.io/resourceconsist v0.0.1
2828
sigs.k8s.io/controller-runtime v0.17.3
2929
)
3030

3131
require (
32+
github.com/Azure/go-autorest/autorest v0.11.18 // indirect
33+
github.com/Azure/go-autorest/autorest/adal v0.9.13 // indirect
34+
github.com/Azure/go-autorest/autorest/date v0.3.0 // indirect
35+
github.com/Azure/go-autorest/logger v0.2.1 // indirect
36+
github.com/Azure/go-autorest/tracing v0.6.0 // indirect
3237
github.com/docker/distribution v2.8.2+incompatible // indirect
38+
github.com/golang/protobuf v1.5.3 // indirect
3339
github.com/matttproud/golang_protobuf_extensions/v2 v2.0.0 // indirect
3440
github.com/samber/lo v1.47.0 // indirect
3541
)
3642

3743
require (
3844
cloud.google.com/go v0.65.0 // indirect
3945
github.com/Azure/go-autorest v14.2.0+incompatible // indirect
40-
github.com/Azure/go-autorest/autorest v0.11.18 // indirect
41-
github.com/Azure/go-autorest/autorest/adal v0.9.13 // indirect
42-
github.com/Azure/go-autorest/autorest/date v0.3.0 // indirect
43-
github.com/Azure/go-autorest/logger v0.2.1 // indirect
44-
github.com/Azure/go-autorest/tracing v0.6.0 // indirect
4546
github.com/alibabacloud-go/alibabacloud-gateway-spi v0.0.4 // indirect
4647
github.com/alibabacloud-go/darabonba-openapi/v2 v2.0.4 // indirect
4748
github.com/alibabacloud-go/debug v0.0.0-20190504072949-9472017b5c68 // indirect
@@ -63,7 +64,6 @@ require (
6364
github.com/go-logr/zapr v1.2.4 // indirect
6465
github.com/gogo/protobuf v1.3.2 // indirect
6566
github.com/golang/groupcache v0.0.0-20210331224755-41bb18bfe9da // indirect
66-
github.com/golang/protobuf v1.5.3 // indirect
6767
github.com/google/go-cmp v0.6.0 // indirect
6868
github.com/google/gofuzz v1.2.0 // indirect
6969
github.com/googleapis/gnostic v0.5.5 // indirect

go.sum

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1153,8 +1153,8 @@ k8s.io/utils v0.0.0-20241104100929-3ea5e8cea738 h1:M3sRQVHv7vB20Xc2ybTt7ODCeFj6J
11531153
k8s.io/utils v0.0.0-20241104100929-3ea5e8cea738/go.mod h1:OLgZIPagt7ERELqWJFomSt595RzquPNLL48iOWgYOg0=
11541154
kusionstack.io/kube-api v0.7.4-0.20250909095208-496f60eea9b5 h1:/jbKYMeXiYnuxyJQs72MoL6vxVoY15FX/7tCKWljscY=
11551155
kusionstack.io/kube-api v0.7.4-0.20250909095208-496f60eea9b5/go.mod h1:e1jtrQH2LK5fD2nTyfIXG6nYrYbU8VXShRxTRwVPaLk=
1156-
kusionstack.io/kube-utils v0.2.1-0.20250908100835-947051067cbe h1:vLIXXJCL3ryDj4KZzUMlXUENhPcv0cfF77PNwjrttcU=
1157-
kusionstack.io/kube-utils v0.2.1-0.20250908100835-947051067cbe/go.mod h1:pThYvlUgm58SXyax5nxHRln6EW7l6sqkNmlUqa/6OR0=
1156+
kusionstack.io/kube-utils v0.2.1-0.20251031115408-8f00ba110277 h1:jsaRBLVOfkpAgPz1JA0Cp+fs7tqi+9Uccu0JJlCeaO0=
1157+
kusionstack.io/kube-utils v0.2.1-0.20251031115408-8f00ba110277/go.mod h1:KEHTfo1Y8SWMODnckF6daO2cSIW0FJ8fkk8PBA5O2GU=
11581158
kusionstack.io/resourceconsist v0.0.1 h1:+k/jriq5Ld7fQUYfWSMGynz/FesHtl3Rk2fmQPjBe0g=
11591159
kusionstack.io/resourceconsist v0.0.1/go.mod h1:816xS/fY6EOUbPFjXIWW/TGs8/YE46qP4ElKeIiwFdU=
11601160
modernc.org/cc v1.0.0/go.mod h1:1Sk4//wdnYJiUIxnW8ddKpaOJCF37yAdqYnkxUpaYxw=

pkg/controllers/collaset/collaset_controller_test.go

Lines changed: 103 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,109 @@ var (
7070
)
7171

7272
var _ = Describe("collaset controller", func() {
73+
It("inplaceUpdate with only metadata changed", func() {
74+
testcase := "test-inplace-update-with-only-metadata-changed"
75+
Expect(createNamespace(c, testcase)).Should(BeNil())
76+
77+
cs := &appsv1alpha1.CollaSet{
78+
ObjectMeta: metav1.ObjectMeta{
79+
Namespace: testcase,
80+
Name: "foo",
81+
},
82+
Spec: appsv1alpha1.CollaSetSpec{
83+
Replicas: int32Pointer(2),
84+
Selector: &metav1.LabelSelector{
85+
MatchLabels: map[string]string{
86+
"app": "foo",
87+
},
88+
},
89+
Template: corev1.PodTemplateSpec{
90+
ObjectMeta: metav1.ObjectMeta{
91+
Labels: map[string]string{
92+
"app": "foo",
93+
},
94+
},
95+
Spec: corev1.PodSpec{
96+
Containers: []corev1.Container{
97+
{
98+
Name: "foo",
99+
Image: "nginx:v1",
100+
},
101+
},
102+
},
103+
},
104+
UpdateStrategy: appsv1alpha1.UpdateStrategy{
105+
OperationDelaySeconds: int32Pointer(1),
106+
PodUpdatePolicy: appsv1alpha1.CollaSetInPlaceIfPossiblePodUpdateStrategyType,
107+
},
108+
},
109+
}
110+
Expect(c.Create(context.TODO(), cs)).Should(BeNil())
111+
112+
podList := &corev1.PodList{}
113+
Eventually(func() bool {
114+
Expect(c.List(context.TODO(), podList, client.InNamespace(cs.Namespace))).Should(BeNil())
115+
return len(podList.Items) == 2
116+
}, 5*time.Second, 1*time.Second).Should(BeTrue())
117+
Expect(c.Get(context.TODO(), types.NamespacedName{Namespace: cs.Namespace, Name: cs.Name}, cs)).Should(BeNil())
118+
Expect(expectedStatusReplicas(c, cs, 0, 0, 0, 2, 2, 0, 0, 0)).Should(BeNil())
119+
120+
observedGeneration := cs.Status.ObservedGeneration
121+
// update CollaSet template metadata
122+
Expect(updateCollaSetWithRetry(c, cs.Namespace, cs.Name, func(cls *appsv1alpha1.CollaSet) bool {
123+
cls.Spec.Template.Annotations = map[string]string{
124+
"test": "v1",
125+
}
126+
return true
127+
})).Should(BeNil())
128+
Eventually(func() bool {
129+
Expect(c.Get(context.TODO(), types.NamespacedName{Namespace: cs.Namespace, Name: cs.Name}, cs)).Should(BeNil())
130+
return cs.Status.ObservedGeneration != observedGeneration
131+
}, 5*time.Second, 1*time.Second).Should(BeTrue())
132+
133+
// allow origin pod to update
134+
Expect(c.List(context.TODO(), podList, client.InNamespace(cs.Namespace))).Should(BeNil())
135+
for i := range podList.Items {
136+
pod := &podList.Items[i]
137+
Expect(updatePodWithRetry(c, pod.Namespace, pod.Name, func(pod *corev1.Pod) bool {
138+
labelOperate := fmt.Sprintf("%s/%s", appsv1alpha1.PodOperateLabelPrefix, collasetutils.UpdateOpsLifecycleAdapter.GetID())
139+
pod.Labels[labelOperate] = fmt.Sprintf("%d", time.Now().UnixNano())
140+
return true
141+
})).Should(BeNil())
142+
}
143+
144+
// waiting for update finished
145+
Eventually(func() error {
146+
return expectedStatusReplicas(c, cs, 0, 0, 0, 2, 2, 2, 0, 0)
147+
}, 10*time.Second, 1*time.Second).Should(BeNil())
148+
149+
// mock container status
150+
Expect(c.List(context.TODO(), podList, client.InNamespace(cs.Namespace))).Should(BeNil())
151+
for i := range podList.Items {
152+
Expect(updatePodStatusWithRetry(c, podList.Items[i].Namespace, podList.Items[i].Name, func(pod *corev1.Pod) bool {
153+
pod.Status.ContainerStatuses = []corev1.ContainerStatus{
154+
{
155+
Name: "foo",
156+
ImageID: "nginx:v1",
157+
},
158+
}
159+
return true
160+
})).Should(BeNil())
161+
}
162+
163+
// check pods update are finished
164+
Eventually(func() bool {
165+
Expect(c.List(context.TODO(), podList, client.InNamespace(cs.Namespace))).Should(BeNil())
166+
for i := range podList.Items {
167+
pod := &podList.Items[i]
168+
if podopslifecycle.IsDuringOps(collasetutils.UpdateOpsLifecycleAdapter, pod) {
169+
return false
170+
}
171+
}
172+
return true
173+
}, 10*time.Second, 1*time.Second).Should(BeTrue())
174+
})
175+
73176
It("replace pod with update", func() {
74177
for _, updateStrategy := range []appsv1alpha1.PodUpdateStrategyType{appsv1alpha1.CollaSetRecreatePodUpdateStrategyType, appsv1alpha1.CollaSetInPlaceIfPossiblePodUpdateStrategyType, appsv1alpha1.CollaSetReplacePodUpdateStrategyType} {
75178
testcase := fmt.Sprintf("test-replace-pod-with-%s-update", strings.ToLower(string(updateStrategy)))

pkg/controllers/collaset/synccontrol/update.go

Lines changed: 14 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -591,10 +591,9 @@ func (u *inPlaceIfPossibleUpdater) FulfillPodUpdatedInfo(_ context.Context, _ *a
591591
return err
592592
}
593593

594+
var podStatus *PodStatus
594595
if podUpdateInfo.OnlyMetadataChanged {
595-
if podUpdateInfo.UpdatedPod.Annotations != nil {
596-
delete(podUpdateInfo.UpdatedPod.Annotations, appsv1alpha1.LastPodStatusAnnotationKey)
597-
}
596+
podStatus = &PodStatus{ContainerStates: nil}
598597
} else {
599598
containerCurrentStatusMapping := map[string]*corev1.ContainerStatus{}
600599
for i := range podUpdateInfo.Status.ContainerStatuses {
@@ -605,7 +604,7 @@ func (u *inPlaceIfPossibleUpdater) FulfillPodUpdatedInfo(_ context.Context, _ *a
605604
}
606605
}
607606

608-
podStatus := &PodStatus{ContainerStates: map[string]*ContainerStatus{}}
607+
podStatus = &PodStatus{ContainerStates: map[string]*ContainerStatus{}}
609608
for _, container := range podUpdateInfo.UpdatedPod.Spec.Containers {
610609
podStatus.ContainerStates[container.Name] = &ContainerStatus{
611610
// store image of each container in updated Pod
@@ -620,17 +619,17 @@ func (u *inPlaceIfPossibleUpdater) FulfillPodUpdatedInfo(_ context.Context, _ *a
620619
// store image ID of each container in current Pod
621620
podStatus.ContainerStates[container.Name].LastImageID = containerCurrentStatus.ImageID
622621
}
622+
}
623623

624-
podStatusStr, err := json.Marshal(podStatus)
625-
if err != nil {
626-
return err
627-
}
624+
podStatusStr, err := json.Marshal(podStatus)
625+
if err != nil {
626+
return err
627+
}
628628

629-
if podUpdateInfo.UpdatedPod.Annotations == nil {
630-
podUpdateInfo.UpdatedPod.Annotations = map[string]string{}
631-
}
632-
podUpdateInfo.UpdatedPod.Annotations[appsv1alpha1.LastPodStatusAnnotationKey] = string(podStatusStr)
629+
if podUpdateInfo.UpdatedPod.Annotations == nil {
630+
podUpdateInfo.UpdatedPod.Annotations = map[string]string{}
633631
}
632+
podUpdateInfo.UpdatedPod.Annotations[appsv1alpha1.LastPodStatusAnnotationKey] = string(podStatusStr)
634633
return nil
635634
}
636635

@@ -709,8 +708,8 @@ func (u *inPlaceIfPossibleUpdater) diffPod(currentPod, updatedPod *corev1.Pod) (
709708
}
710709

711710
func (u *inPlaceIfPossibleUpdater) GetPodUpdateFinishStatus(_ context.Context, podUpdateInfo *PodUpdateInfo) (finished bool, msg string, err error) {
712-
if podUpdateInfo.PodDecorationChanged {
713-
return false, "add on not updated", nil
711+
if !podUpdateInfo.IsUpdatedRevision || podUpdateInfo.PodDecorationChanged {
712+
return false, "pod is not updated or add on not updated", nil
714713
}
715714

716715
if podUpdateInfo.Status.ContainerStatuses == nil {
@@ -738,7 +737,7 @@ func (u *inPlaceIfPossibleUpdater) GetPodUpdateFinishStatus(_ context.Context, p
738737
}
739738

740739
if podLastState.ContainerStates == nil {
741-
return true, "empty last container state recorded", nil
740+
return true, "pod is updated with only metadata changed", nil
742741
}
743742

744743
imageMapping := map[string]string{}

0 commit comments

Comments
 (0)