Skip to content

Commit 5db9784

Browse files
leon-apeleon-ape
authored
chore: fix its image registry drift (#10257)
1 parent 03c1f7f commit 5db9784

6 files changed

Lines changed: 281 additions & 119 deletions

File tree

‎pkg/controller/instanceset/in_place_update_util.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -210,7 +210,7 @@ func equalField(old, new any) bool {
210210
if index < 0 {
211211
return false
212212
}
213-
if nc.Image != ocs[index].Image {
213+
if !intctrlutil.EqualContainerImageInSpec(ocs[index].Image, nc.Image) {
214214
return false
215215
}
216216
}

‎pkg/controller/instanceset/in_place_update_util_test.go‎

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -95,12 +95,58 @@ var _ = Describe("instance util test", func() {
9595
})
9696
})
9797

98+
Context("container image comparison", func() {
99+
It("ignores registry rewrites but keeps tag, digest, and basename strict", func() {
100+
oldPod := buildRandomPod()
101+
newPod := oldPod.DeepCopy()
102+
oldPod.Spec.Containers[0].Image = "172.31.255.3:5000/apecloud/redis:8.4.0"
103+
newPod.Spec.Containers[0].Image = "192.168.173.140:6451/apecloud/redis:8.4.0"
104+
oldPod.Spec.InitContainers[0].Image = "172.31.255.3:5000/apecloud/kbagent:1.0.3-beta.5"
105+
newPod.Spec.InitContainers[0].Image = "192.168.173.140:6451/apecloud/kbagent:1.0.3-beta.5"
106+
107+
its := builder.NewInstanceSetBuilder(namespace, name).
108+
SetPodUpdatePolicy(kbappsv1.ReCreatePodUpdatePolicyType).
109+
SetPodUpgradePolicy(kbappsv1.PreferInPlacePodUpdatePolicyType).
110+
GetObject()
111+
112+
Expect(equalBasicInPlaceFields(oldPod, newPod)).Should(BeTrue())
113+
Expect(getPodUpdatePolicyInSpec(its, oldPod, newPod)).Should(Equal(kbappsv1.ReCreatePodUpdatePolicyType))
114+
115+
By("tag mismatch")
116+
tagChangedPod := newPod.DeepCopy()
117+
tagChangedPod.Spec.Containers[0].Image = "192.168.173.140:6451/apecloud/redis:8.4.1"
118+
Expect(equalBasicInPlaceFields(oldPod, tagChangedPod)).Should(BeFalse())
119+
Expect(getPodUpdatePolicyInSpec(its, oldPod, tagChangedPod)).Should(Equal(kbappsv1.PreferInPlacePodUpdatePolicyType))
120+
121+
By("init container tag mismatch")
122+
initTagChangedPod := newPod.DeepCopy()
123+
initTagChangedPod.Spec.InitContainers[0].Image = "192.168.173.140:6451/apecloud/kbagent:1.0.3-beta.6"
124+
Expect(equalBasicInPlaceFields(oldPod, initTagChangedPod)).Should(BeFalse())
125+
Expect(getPodUpdatePolicyInSpec(its, oldPod, initTagChangedPod)).Should(Equal(kbappsv1.PreferInPlacePodUpdatePolicyType))
126+
127+
By("digest mismatch")
128+
digestChangedPod := newPod.DeepCopy()
129+
oldPod.Spec.Containers[0].Image = "172.31.255.3:5000/apecloud/redis:8.4.0@sha256:old"
130+
digestChangedPod.Spec.Containers[0].Image = "192.168.173.140:6451/apecloud/redis:8.4.0@sha256:new"
131+
Expect(equalBasicInPlaceFields(oldPod, digestChangedPod)).Should(BeFalse())
132+
Expect(getPodUpdatePolicyInSpec(its, oldPod, digestChangedPod)).Should(Equal(kbappsv1.PreferInPlacePodUpdatePolicyType))
133+
134+
By("basename mismatch")
135+
basenameChangedPod := newPod.DeepCopy()
136+
oldPod.Spec.Containers[0].Image = "172.31.255.3:5000/apecloud/redis:8.4.0"
137+
basenameChangedPod.Spec.Containers[0].Image = "192.168.173.140:6451/apecloud/redis-stack:8.4.0"
138+
Expect(equalBasicInPlaceFields(oldPod, basenameChangedPod)).Should(BeFalse())
139+
Expect(getPodUpdatePolicyInSpec(its, oldPod, basenameChangedPod)).Should(Equal(kbappsv1.PreferInPlacePodUpdatePolicyType))
140+
})
141+
})
142+
98143
Context("getPodUpdatePolicy", func() {
99144
It("should work well", func() {
100145
By("build an updated pod")
101146
randStr := rand.String(16)
102147
key := randStr
103148
podTemplate := template.DeepCopy()
149+
podTemplate.Spec.Containers[0].Image = "192.168.173.140:6451/apecloud/redis:8.4.0"
104150
mergeMap(&map[string]string{key: randStr}, &podTemplate.Annotations)
105151
mergeMap(&map[string]string{key: randStr}, &podTemplate.Labels)
106152
its = builder.NewInstanceSetBuilder(namespace, name).
@@ -146,6 +192,14 @@ var _ = Describe("instance util test", func() {
146192
Expect(policy).Should(Equal(noOpsPolicy))
147193
Expect(specPolicy).Should(Equal(kbappsv1.PodUpdatePolicyType("")))
148194

195+
By("build a pod with registry rewritten by admission")
196+
podWithRewrittenRegistry := pod1.DeepCopy()
197+
podWithRewrittenRegistry.Spec.Containers[0].Image = "172.31.255.3:5000/apecloud/redis:8.4.0"
198+
policy, specPolicy, err = getPodUpdatePolicy(its, podWithRewrittenRegistry)
199+
Expect(err).Should(BeNil())
200+
Expect(policy).Should(Equal(noOpsPolicy))
201+
Expect(specPolicy).Should(Equal(kbappsv1.PodUpdatePolicyType("")))
202+
149203
By("build a pod with revision updated")
150204
pod2 := pod1.DeepCopy()
151205
pod2.Spec.Containers = append(pod2.Spec.Containers, corev1.Container{

‎pkg/controller/instanceset/instance_util.go‎

Lines changed: 1 addition & 66 deletions
Original file line numberDiff line numberDiff line change
@@ -184,78 +184,13 @@ func isImageMatched(pod *corev1.Pod) bool {
184184
status := pod.Status.ContainerStatuses[index]
185185
// Image in status may not match the image used in the PodSpec.
186186
// More info: https://kubernetes.io/docs/reference/kubernetes-api/workload-resources/pod-v1/#PodStatus
187-
specName, specTag, specDigest := imageSplit(specImage)
188-
_, _, statusDigest := imageSplit(status.ImageID)
189-
if len(specDigest) != 0 {
190-
if specDigest != statusDigest {
191-
return false
192-
}
193-
continue
194-
}
195-
statusName, statusTag, _ := imageSplit(status.Image)
196-
// if tag presents in spec, it must be same in status
197-
if len(specTag) != 0 && specTag != statusTag {
187+
if !intctrlutil.MatchContainerImageInStatus(specImage, status.Image, status.ImageID) {
198188
return false
199189
}
200-
// otherwise, statusName should be same as or has suffix of specName
201-
if specName != statusName {
202-
specNames := strings.Split(specName, "/")
203-
statusNames := strings.Split(statusName, "/")
204-
if specNames[len(specNames)-1] != statusNames[len(statusNames)-1] {
205-
return false
206-
}
207-
}
208190
}
209191
return true
210192
}
211193

212-
// imageSplit separates and returns the name and tag parts
213-
// from the image string using either colon `:` or at `@` separators.
214-
// image reference pattern: [[host[:port]/]component/]component[:tag][@digest]
215-
func imageSplit(imageName string) (name string, tag string, digest string) {
216-
// check if image name contains a domain
217-
// if domain is present, ignore domain and check for `:`
218-
searchName := imageName
219-
slashIndex := strings.Index(imageName, "/")
220-
if slashIndex > 0 {
221-
searchName = imageName[slashIndex:]
222-
} else {
223-
slashIndex = 0
224-
}
225-
226-
id := strings.Index(searchName, "@")
227-
ic := strings.Index(searchName, ":")
228-
229-
// no tag or digest
230-
if ic < 0 && id < 0 {
231-
return imageName, "", ""
232-
}
233-
234-
// digest only
235-
if id >= 0 && (id < ic || ic < 0) {
236-
id += slashIndex
237-
name = imageName[:id]
238-
digest = strings.TrimPrefix(imageName[id:], "@")
239-
return name, "", digest
240-
}
241-
242-
// tag and digest
243-
if id >= 0 && ic >= 0 {
244-
id += slashIndex
245-
ic += slashIndex
246-
name = imageName[:ic]
247-
tag = strings.TrimPrefix(imageName[ic:id], ":")
248-
digest = strings.TrimPrefix(imageName[id:], "@")
249-
return name, tag, digest
250-
}
251-
252-
// tag only
253-
ic += slashIndex
254-
name = imageName[:ic]
255-
tag = strings.TrimPrefix(imageName[ic:], ":")
256-
return name, tag, ""
257-
}
258-
259194
// getPodRevision gets the revision of Pod by inspecting the StatefulSetRevisionLabel. If pod has no revision the empty
260195
// string is returned.
261196
func getPodRevision(pod *corev1.Pod) string {

‎pkg/controller/instanceset/instance_util_test.go‎

Lines changed: 2 additions & 52 deletions
Original file line numberDiff line numberDiff line change
@@ -920,67 +920,17 @@ var _ = Describe("instance util test", func() {
920920
}}
921921
Expect(isImageMatched(pod)).Should(BeTrue())
922922

923-
By("spec: digest, status image: tag, status imageID: digest")
924-
pod.Spec.Containers = []corev1.Container{{
925-
Name: name,
926-
Image: "docker.io/nginx@sha256:0f37a86c04f8",
927-
}}
928-
pod.Status.ContainerStatuses = []corev1.ContainerStatus{{
929-
Name: name,
930-
Image: "docker.io/nginx:latest",
931-
ImageID: "docker.io/nginx@sha256:0f37a86c04f8",
932-
}}
933-
Expect(isImageMatched(pod)).Should(BeTrue())
934-
935-
By("spec: digest, status image: local image ID, status imageID: digest")
936-
pod.Status.ContainerStatuses = []corev1.ContainerStatus{{
937-
Name: name,
938-
Image: "sha256:runtime-local-image-id",
939-
ImageID: "docker.io/nginx@sha256:0f37a86c04f8",
940-
}}
941-
Expect(isImageMatched(pod)).Should(BeTrue())
942-
943-
By("spec: digest, status image: matching digest, status imageID: different digest")
944-
pod.Status.ContainerStatuses = []corev1.ContainerStatus{{
945-
Name: name,
946-
Image: "docker.io/nginx@sha256:0f37a86c04f8",
947-
ImageID: "docker.io/nginx@sha256:different",
948-
}}
949-
Expect(isImageMatched(pod)).Should(BeFalse())
950-
951-
By("spec: mutable tag, status image: different tag, status imageID: digest")
952-
pod.Spec.Containers = []corev1.Container{{
953-
Name: name,
954-
Image: "docker.io/nginx:1.0.0",
955-
}}
956-
pod.Status.ContainerStatuses = []corev1.ContainerStatus{{
957-
Name: name,
958-
Image: "docker.io/nginx:latest",
959-
ImageID: "docker.io/nginx@sha256:0f37a86c04f8",
960-
}}
961-
Expect(isImageMatched(pod)).Should(BeFalse())
962-
963923
By("digest not matches")
964924
pod.Spec.Containers = []corev1.Container{{
965925
Name: name,
966926
Image: "nginx:latest@xxxxxxxxx",
967927
}}
968-
pod.Status.ContainerStatuses = []corev1.ContainerStatus{{
969-
Name: name,
970-
Image: "nginx:latest@xxxxxxxxx",
971-
ImageID: "docker.io/nginx@sha256:0f37a86c04f8",
972-
}}
973928
Expect(isImageMatched(pod)).Should(BeFalse())
974929

975-
By("tag not matches for tag-only spec")
930+
By("tag not matches")
976931
pod.Spec.Containers = []corev1.Container{{
977932
Name: name,
978-
Image: "nginx:xxxx",
979-
}}
980-
pod.Status.ContainerStatuses = []corev1.ContainerStatus{{
981-
Name: name,
982-
Image: "docker.io/nginx:latest",
983-
ImageID: "docker.io/nginx@sha256:0f37a86c04f8",
933+
Image: "nginx:xxxx@0f37a86c04f8",
984934
}}
985935
Expect(isImageMatched(pod)).Should(BeFalse())
986936

‎pkg/controllerutil/image_util.go‎

Lines changed: 81 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -203,3 +203,84 @@ func ReplaceImageRegistry(image string) string {
203203
}
204204
return fmt.Sprintf("%v/%v/%v%v", dstRegistry, *dstNamespace, repository, remainder)
205205
}
206+
207+
// MatchContainerImageInStatus returns true if the status image matches the image
208+
// requested in PodSpec. For digest-pinned images, the status imageID digest is
209+
// the stable runtime contract; status image may be reported as a tag or local ID.
210+
func MatchContainerImageInStatus(specImage, statusImage, statusImageID string) bool {
211+
specName, specTag, specDigest := splitContainerImageRef(specImage)
212+
if specDigest != "" {
213+
_, _, statusDigest := splitContainerImageRef(statusImageID)
214+
return specDigest == statusDigest
215+
}
216+
217+
statusName, statusTag, _ := splitContainerImageRef(statusImage)
218+
if specTag != "" && specTag != statusTag {
219+
return false
220+
}
221+
return imageBaseName(specName) == imageBaseName(statusName)
222+
}
223+
224+
// EqualContainerImageInSpec returns true if two PodSpec image references point to
225+
// the same image after ignoring registry rewrites. Tags and digests remain strict.
226+
func EqualContainerImageInSpec(oldImage, newImage string) bool {
227+
if oldImage == newImage {
228+
return true
229+
}
230+
231+
oldName, oldTag, oldDigest := splitContainerImageRef(oldImage)
232+
newName, newTag, newDigest := splitContainerImageRef(newImage)
233+
if (oldDigest != "" || newDigest != "") && oldDigest != newDigest {
234+
return false
235+
}
236+
if (oldTag != "" || newTag != "") && oldTag != newTag {
237+
return false
238+
}
239+
return imageBaseName(oldName) == imageBaseName(newName)
240+
}
241+
242+
// splitContainerImageRef separates the name, tag, and digest parts from an
243+
// image reference. It is deliberately lenient because Kubernetes may surface
244+
// runtime-formatted status image strings that are not normalized references.
245+
func splitContainerImageRef(imageName string) (name string, tag string, digest string) {
246+
searchName := imageName
247+
slashIndex := strings.Index(imageName, "/")
248+
if slashIndex > 0 {
249+
searchName = imageName[slashIndex:]
250+
} else {
251+
slashIndex = 0
252+
}
253+
254+
id := strings.Index(searchName, "@")
255+
ic := strings.Index(searchName, ":")
256+
if ic < 0 && id < 0 {
257+
return imageName, "", ""
258+
}
259+
if id >= 0 && (id < ic || ic < 0) {
260+
id += slashIndex
261+
name = imageName[:id]
262+
digest = strings.TrimPrefix(imageName[id:], "@")
263+
return name, "", digest
264+
}
265+
if id >= 0 && ic >= 0 {
266+
id += slashIndex
267+
ic += slashIndex
268+
name = imageName[:ic]
269+
tag = strings.TrimPrefix(imageName[ic:id], ":")
270+
digest = strings.TrimPrefix(imageName[id:], "@")
271+
return name, tag, digest
272+
}
273+
274+
ic += slashIndex
275+
name = imageName[:ic]
276+
tag = strings.TrimPrefix(imageName[ic:], ":")
277+
return name, tag, ""
278+
}
279+
280+
func imageBaseName(name string) string {
281+
index := strings.LastIndex(name, "/")
282+
if index < 0 {
283+
return name
284+
}
285+
return name[index+1:]
286+
}

0 commit comments

Comments
 (0)