-
Notifications
You must be signed in to change notification settings - Fork 902
feat: Plugin server supports k8s deployment and configures the default download URL of the plugin(#2232, #2280,#2312) #2389
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
Conversation
添加Higress插件服务器的Kubernetes部署支持及配置变更文件
时序图sequenceDiagram
participant User as 用户
participant Helm as Helm控制器
participant Kubernetes as Kubernetes集群
User->>Helm: 执行helm install启用enablePluginServer
Helm->>Kubernetes: 应用plugin-server-deployment.yaml
Kubernetes->>Kubernetes: 创建Deployment管理Pod
Helm->>Kubernetes: 应用plugin-server-service.yaml
Kubernetes->>Kubernetes: 创建Service暴露80端口
Kubernetes-->>User: 返回部署成功状态
💡 小贴士与 lingma-agents 交流的方式📜 直接回复评论
📜 在代码行处标记
📜 在讨论中提问
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🔍 代码评审报告
🎯 评审意见概览
| 严重度 | 数量 | 说明 |
|---|---|---|
| 🔴 Blocker | 0 | 阻断性问题,需立即修复。例如:系统崩溃、关键功能不可用或严重安全漏洞。 |
| 🟠 Critical | 0 | 严重问题,高优先级修复。例如:核心功能异常或性能瓶颈影响用户体验。 |
| 🟡 Major | 0 | 主要问题,建议修复。例如:非核心功能缺陷或代码维护性较差。 |
| 🟢 Minor | 1 | 次要问题,酌情优化。例如:代码格式不规范或注释缺失。 |
总计: 1 个问题
📋 评审意见详情
💡 单文件建议
以下是文件级别的代码建议,聚焦于代码的可读性、可维护性和潜在问题。
📋 helm/core/values.yaml (1 💬)
- 修正插件服务器镜像仓库配置的冗余路径 (L779-L780)
🚀 跨文件建议
以下是对代码架构和设计的综合分析,聚焦于跨文件交互、系统一致性和潜在优化空间。
🔍 1. 镜像仓库配置的全局冗余可能导致维护困难
在values.yaml中,pluginServer.hub字段与global.hub存在重复配置。当前pluginServer的镜像地址通过hub字段单独定义,但其他组件(如Higress Gateway)可能已使用global.hub作为全局镜像仓库。这种冗余配置增加了未来维护成本,当需要统一更改镜像仓库地址时,需要修改多个地方。建议将镜像仓库配置统一到global.hub下,通过.Values.global.hub优先级继承机制实现统一管理,避免配置分散。
📌 关键代码:
hub: higress-registry.cn-hangzhou.cr.aliyuncs.com/higress
# 应该移除该配置,改为优先使用global.hub🔍 2. Kubernetes资源标签定义存在重复代码风险
在_helpers.tpl中定义的app.kubernetes.io标签结构与其他组件(如Higress Gateway)可能存在重复实现。当前pluginServer的labels模板与常规组件的标签定义逻辑类似,但未复用已有模板。建议将通用标签生成逻辑(如app.kubernetes.io相关的字段)抽象到公共模板函数中,避免不同组件间出现相似但不一致的标签定义,提升代码复用性和维护性。
📌 关键代码:
{{- define "pluginServer.labels" -}}
helm.sh/chart: {{ include "pluginServer.chart" . }}
{{ include "pluginServer.selectorLabels" . }}
{{- if .Chart.AppVersion }}
app.kubernetes.io/version: {{ .Chart.AppVersion | quote }}
{{- end }}
app.kubernetes.io/managed-by: {{ .Release.Service }}
app.kubernetes.io/name: {{ include "pluginServer.name" . }}
{{- end }}
🔍 3. 组件启停配置未与依赖组件联动
global.enablePluginServer配置未与其他组件的启用状态建立关联。例如,若插件服务器需要依赖其他组件(如Higress Gateway)才能正常工作,但当前配置允许单独启用/禁用插件服务器,可能导致部署时出现功能缺失。建议在文档中明确插件服务器的依赖关系,或在模板中添加启用条件验证逻辑(如requirement检查)
📌 关键代码:
{{- if .Values.global.enablePluginServer }}💡 小贴士
与 lingma-agents 交流的方式
📜 直接回复评论
直接回复本条评论,lingma-agents 将自动处理您的请求。例如:
-
在当前代码中添加详细的注释说明。
-
请详细介绍一下你说的 LRU 改造方案,并使用伪代码加以说明。
📜 在代码行处标记
在文件的特定位置创建评论并 @lingma-agents。例如:
-
@Lingma-Agent 分析这个方法的性能瓶颈并提供优化建议。
-
@Lingma-Agent 对这个方法生成优化代码。
📜 在讨论中提问
在任何讨论中 @lingma-agents 来获取帮助。例如:
-
@Lingma-Agent 请总结上述讨论并提出解决方案。
-
@Lingma-Agent 请根据讨论内容生成优化代码。
| hub: higress-registry.cn-hangzhou.cr.aliyuncs.com/higress | ||
| tag: "" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
修正插件服务器镜像仓库配置的冗余路径
🟢 Minor | 🧹 Code Smells
📋 问题详情
hub的默认值包含仓库路径/higress,与镜像名拼接时可能导致重复路径。应将仓库路径移至镜像名中,确保配置清晰。
💡 解决方案
调整镜像仓库路径配置:
-779 + hub: higress-registry.cn-hangzhou.cr.aliyuncs.com/higress
+ hub: higress-registry.cn-hangzhou.cr.aliyuncs.com
-780 + image: plugin-server
+ image: higress/plugin-server您的反馈对我们很重要!(建议右键在新标签页中打开以下链接)
lexburner
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #2389 +/- ##
===========================================
+ Coverage 35.91% 46.05% +10.14%
===========================================
Files 69 81 +12
Lines 11576 13018 +1442
===========================================
+ Hits 4157 5995 +1838
+ Misses 7104 6678 -426
- Partials 315 345 +30 🚀 New features to boost your workflow:
|
|
@NorthernBob 关注下 Helm Docs ci 的问题 @NorthernBob Follow the questions of Helm Docs ci |
johnlanni
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
…t download URL of the plugin(alibaba#2232, alibaba#2280,alibaba#2312) (alibaba#2389) Co-authored-by: xujingfeng <[email protected]> Co-authored-by: 澄潭 <[email protected]>
Ⅰ. Describe what this PR did
Plugin server supports k8s one-click deployment and configures the default download URL of the plugin
Ⅱ. Does this pull request fix one issue?
fixes #2232 #2280 #2312
Ⅲ. Why don't you add test cases (unit test/integration test)?
Ⅳ. Describe how to verify it
Ⅴ. Special notes for reviews