Skip to content

fix: update same-name Helm images by image path - #4913

Open
Cynthia-0203 wants to merge 13 commits into
koderover:mainfrom
Cynthia-0203:fix/helm-same-name-image-update
Open

fix: update same-name Helm images by image path#4913
Cynthia-0203 wants to merge 13 commits into
koderover:mainfrom
Cynthia-0203:fix/helm-same-name-image-update

Conversation

@Cynthia-0203

@Cynthia-0203 Cynthia-0203 commented Aug 19, 2026

Copy link
Copy Markdown
Contributor

What this PR does / Why we need it:

Fixes Helm services where multiple components share the same image name but use different values paths. Previously, components were merged by name, causing one component to overwrite another and preventing simultaneous image updates.

What is changed and how it works?

  • Uses name + image_path as the identity for Helm service modules.
  • Preserves same-name components with different image paths when resolving service_module records.
  • Updates environment containers by exact name + image_path matching.
  • Uses the selected service revision from service_module as the authoritative container list.
  • Ensures image updates are applied to all matching Helm values paths.

Does this PR introduce a user-facing change?

  • API change
  • database schema change
  • upgrade assistant change
  • change in non-functional attributes such as efficiency or availability
  • fix of a previous issue

This change is Reviewable

Signed-off-by: Cynthia <chenchen@koderover.com>
Signed-off-by: Cynthia <chenchen@koderover.com>
Signed-off-by: Cynthia <chenchen@koderover.com>
Signed-off-by: Cynthia <chenchen@koderover.com>
Signed-off-by: Cynthia <chenchen@koderover.com>
Signed-off-by: Cynthia <chenchen@koderover.com>
Signed-off-by: Cynthia-0203 <chenchen@koderover.com>
Signed-off-by: Cynthia-0203 <chenchen@koderover.com>
Signed-off-by: Cynthia <chenchen@koderover.com>
Signed-off-by: Cynthia <chenchen@koderover.com>
Signed-off-by: Cynthia <chenchen@koderover.com>

@PetrusZ PetrusZ left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

最后有准备升级前的数据,然后升级后再测试吗?

if i == nil {
return ""
}
return fmt.Sprintf("%s\x00%s\x00%s\x00%s", i.Repo, i.Namespace, i.Image, i.Tag)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

\x00是什么?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

就是作为分隔符,这些数据可能是包含-,_之类的,就没有使用-来作为分隔符

Comment on lines +429 to +431
if _, ok := imageMap[name]; !ok {
imageMap[name] = image
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

这几行的作用是?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

镜像名 → 完整镜像地址。同名组件可能导致输入中出现多个相同镜像名,这里采用 first-win,保证后续所有同名组件统一使用第一个镜像,避免后面的重复项覆盖前面的选择。

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

不统一使用第一个会有什么问题吗?因为这样改了以后可能会造成产品逻辑跟之前不一样

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

这个我再看一下

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

已经改了

container.Image = imageMap[container.ImageName]
}
mergedContainers = append(mergedContainers, container)
continue

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

最好用if else吧,continue太乱了

Comment on lines 467 to 484
if container.ImageName == name {
// find corresponding image in values
// path present in values: skip when unchanged and no build image
if imageMap[name] == "" && container.Image == imageUrl {
continue
}
if imageMap[name] != "" {
// if found image in images, and the images are from build job, we should override it
container.Image = imageMap[name]
mergedContainers = append(mergedContainers, container)
}
} else {
// not found corresponding image in values
// add container image into values
if imageMap[container.ImageName] != "" {
// if found image in images, and the images are from build job, we should override it
container.Image = imageMap[container.ImageName]
}
mergedContainers = append(mergedContainers, container)
continue
}

// path not in values: always write back
if imageMap[container.ImageName] != "" {
container.Image = imageMap[container.ImageName]
}
mergedContainers = append(mergedContainers, container)
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

补一下注释吧,没有以前清晰

Comment on lines +468 to +471
// path present in values: skip when unchanged and no build image
if imageMap[name] == "" && container.Image == imageUrl {
continue
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

这几行再解释一下呢?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

imageMap[name] == "":本次没有构建任务产出的镜像需要覆盖
container.Image == imageUrl:环境中记录的镜像与当前 Values 路径解析出的镜像一致
两个条件同时满足时,不需要加入 mergedContainers 再次回写 Values,所以直接跳过。

Signed-off-by: Cynthia <chenchen@koderover.com>
Signed-off-by: Cynthia <chenchen@koderover.com>
@landylee007
landylee007 requested a review from PetrusZ August 27, 2026 15:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants