Skip to content
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: hgctl upgrade support from-helm flag #824

Draft
wants to merge 7 commits into
base: main
Choose a base branch
from

Conversation

sjcsjc123
Copy link
Collaborator

@sjcsjc123 sjcsjc123 commented Feb 7, 2024

Ⅰ. Describe what this PR did

upgrade命令支持读取helm最新版本的信息

Ⅱ. Does this pull request fix one issue?

fixes: #570

Ⅲ. Why don't you add test cases (unit test/integration test)?

Ⅳ. Describe how to verify it

Ⅴ. Special notes for reviews

简单debug了一下,运行hgctl upgrade --from-helm命令后进入如下地方,已经可以看到更换到latest版本号

image

upgrade本质上也是install命令

image

@2456868764
Copy link
Collaborator

当 higress 用 helm 安装后,执行hgctl upgrade --from-helm 表示从用helm安装higress时设置参数生成 hgctl install profile。可以通过 helm status RELEASENAME -oyaml 命令读取 helm 安装配置参数。

@johnlanni
Copy link
Collaborator

johnlanni commented Feb 20, 2024

@sjcsjc123 如上面Jun所说,这个issue的目的是继承helm安装时的参数,从而完全替代helm安装方式

@codecov-commenter
Copy link

codecov-commenter commented Feb 21, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 38.17%. Comparing base (9c112a0) to head (e1cc05b).

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #824      +/-   ##
==========================================
+ Coverage   38.14%   38.17%   +0.02%     
==========================================
  Files          61       61              
  Lines       10434    10434              
==========================================
+ Hits         3980     3983       +3     
+ Misses       6154     6152       -2     
+ Partials      300      299       -1     

see 1 file with indirect coverage changes

@sjcsjc123
Copy link
Collaborator Author

helm upgrade的时候要将profileName设置为k8s还是local-k8s,目前的思路是如果指定了from-helm参数,通过helm status获取config的值,将其转化为valuesOverlay,通过GenProfile(profileName, valuesOverlay, setFlags)方法生成profile;然后继续往下执行upgrade流程

Signed-off-by: sjcsjc123 <1401189096@qq.com>
Signed-off-by: sjcsjc123 <1401189096@qq.com>
Signed-off-by: sjcsjc123 <1401189096@qq.com>
Signed-off-by: sjcsjc123 <1401189096@qq.com>
@2456868764
Copy link
Collaborator

2456868764 commented Feb 28, 2024

helm upgrade的时候要将profileName设置为k8s还是local-k8s,目前的思路是如果指定了from-helm参数,通过helm status获取config的值,将其转化为valuesOverlay,通过GenProfile(profileName, valuesOverlay, setFlags)方法生成profile;然后继续往下执行upgrade流程

是否是 local-k8s 是根据 global.local是否 True来判断, 同时 profile里的global 设置是否可以从 helm 安装安装参数中恢复?

@sjcsjc123
Copy link
Collaborator Author

helm upgrade的时候要将profileName设置为k8s还是local-k8s,目前的思路是如果指定了from-helm参数,通过helm status获取config的值,将其转化为valuesOverlay,通过GenProfile(profileName, valuesOverlay, setFlags)方法生成profile;然后继续往下执行upgrade流程

是否是 local-k8s 是根据 global.local是否 True来判断, 同时 profile里的global 设置是否可以从 helm 安装安装参数中恢复?

可以恢复出来


fmt.Fprintf(writer, "\n🧐 Validating Profile: \"%s\" \n", profileContext.PathOrName)
} else {
helmAgent := installer.NewHelmAgent(nil, writer, false)
Copy link
Collaborator

Choose a reason for hiding this comment

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

这里 profile 为 nil, 会 报 nil pointer

@@ -61,29 +65,46 @@ func newUpgradeCmd() *cobra.Command {
}

// upgrade upgrade higress resources from the cluster.
func upgrade(writer io.Writer, iArgs *InstallArgs) error {
func upgrade(writer io.Writer, iArgs *upgradeArgs) error {
Copy link
Collaborator

Choose a reason for hiding this comment

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

升级时候 如果 --from-helm = true, 可以读取helm 安装所有 release 列表,让用户选择升级那个 release, 如果用户选择某个RELEASE 后,要检查这个 RELEASE 是否已经升级( 是否已经有 install profile),如果没有让用户升级。

@johnlanni johnlanni marked this pull request as draft April 23, 2024 08:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

hgctl install/upgrade 子命令优化
4 participants