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: custom listening port for gateway pod in helm #829

Merged
merged 8 commits into from
Feb 23, 2024

Conversation

Uncle-Justice
Copy link
Contributor

@Uncle-Justice Uncle-Justice commented Feb 20, 2024

Ⅰ. Describe what this PR did

使用helm安装higress时,在values.yaml中自定义gateway pod配置中的监听端口。

产生的变化:

  1. describe gateway pod配置,可观察到自定义监听端口
  2. controller也会根据helm中注入的命令行参数,生成监听对应的自定义端口的gateway

Ⅱ. Does this pull request fix one issue?

fixes #734

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

Ⅳ. Describe how to verify it

修改values.yaml中监听的http端口后,可在集群内部直接通过对应端口访问到服务
image

在hostNetwork模式下,可在集群内部观察到envoy监听的端口也发生了对应的变化
image

Ⅴ. Special notes for reviews

  1. 目前如果自定义非80端口,外部curl集群是无法访问到对应服务的,不过看之前的issue要求的就是在没有外部lb且hostNetwork模式下,因此应该可以暂不考虑这个问题。
  2. 在controller中考虑了如果yaml中未设置对应字段的情况,该情况下默认80&443(因为测试代码无法使用yaml引入环境变量),如果字段对应的值是非法值,集群会报错。

@johnlanni
Copy link
Collaborator

@Uncle-Justice higress controller的代码是不是漏提交了,即环境变量控制转换的逻辑

@codecov-commenter
Copy link

codecov-commenter commented Feb 20, 2024

Codecov Report

Attention: 4 lines in your changes are missing coverage. Please review.

Comparison is base (f537a1c) 38.04% compared to head (7e27b50) 38.07%.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #829      +/-   ##
==========================================
+ Coverage   38.04%   38.07%   +0.03%     
==========================================
  Files          61       61              
  Lines       10428    10434       +6     
==========================================
+ Hits         3967     3973       +6     
  Misses       6161     6161              
  Partials      300      300              
Files Coverage Δ
pkg/bootstrap/server.go 64.66% <100.00%> (+0.26%) ⬆️
pkg/cmd/server.go 85.00% <100.00%> (+0.78%) ⬆️
pkg/ingress/kube/common/model.go 2.60% <ø> (ø)
pkg/ingress/kube/ingressv1/controller.go 5.41% <0.00%> (ø)

@@ -357,6 +359,26 @@ func (c *controller) ConvertGateway(convertOptions *common.ConvertOptions, wrapp
return fmt.Errorf("invalid ingress rule %s:%s in cluster %s, either `defaultBackend` or `rules` must be specified", cfg.Namespace, cfg.Name, c.options.ClusterId)
}

// If no ports setting in yaml env(port is parsed as 0), take 80&443 as default.
gatewayHttpPort, err := strconv.ParseUint(os.Getenv("GATEWAY_HTTP_PORT"), 10, 32)
Copy link
Collaborator

Choose a reason for hiding this comment

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

每次都取环境变量不太好,可能作为启动参数,实现更简洁一些:

higress/pkg/cmd/server.go

Lines 110 to 115 in b652f3e

serveCmd.PersistentFlags().Float32Var(&serverArgs.RegistryOptions.KubeOptions.KubernetesAPIQPS, "kubernetesApiQPS", 80.0,
"Maximum QPS when communicating with the kubernetes API")
serveCmd.PersistentFlags().IntVar(&serverArgs.RegistryOptions.KubeOptions.KubernetesAPIBurst, "kubernetesApiBurst", 160,
"Maximum burst for throttle when communicating with the kubernetes API")

Copy link
Collaborator

@johnlanni johnlanni left a comment

Choose a reason for hiding this comment

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

LGTM

@johnlanni johnlanni merged commit e55a3c0 into alibaba:main Feb 23, 2024
8 checks passed
Renz7 pushed a commit to Renz7/higress that referenced this pull request Mar 4, 2024
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.

K8s安装,hostNetwork模式部署时,支持配置监听端口,以避免冲突
3 participants