mcpbridge新增vport元素，以修复服务后端端口不一致导致的兼容性问题 || =mcpbridge added a vport element to fix compatibility issues caused by inconsistent ports in the service backend #2859
Conversation
lcfang
requested review from
2456868764,
CH3CHO,
Erica177 and
johnlanni
as code owners
github-actions
bot
changed the title
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #2859 +/- ##
==========================================
+ Coverage 35.91% 44.99% +9.08%
==========================================
Files 69 82 +13
Lines 11576 13347 +1771
==========================================
+ Hits 4157 6006 +1849
+ Misses 7104 6993 -111
- Partials 315 348 +33 🚀 New features to boost your workflow:
|
|
@CH3CHO @johnlanni 这个PR麻烦尽快帮忙合入吧 @CH3CHO @johnlanni Please help me join this PR as soon as possible |
registry/reconcile/reconcile.go
Outdated
| clusterId: clusterId, | ||
| } | ||
| r.Cache = memory.NewCache(func() *v1.McpBridge { | ||
| return r.mcpBridge |
There was a problem hiding this comment.
这个地方有点trick，导致整体结构比较混乱，cache里还依赖CRD的描述。是不是应该在具体registry实现里处理vport相关逻辑呢？你可以只在eureka的实现里加。
There was a problem hiding this comment.
但这样的话每一个 registry 就都要实现一遍，会比较麻烦。最好是找一个地方收口管理。
There was a problem hiding this comment.
这个地方有点trick，导致整体结构比较混乱，cache里还依赖CRD的描述。是不是应该在具体registry实现里处理vport相关逻辑呢？你可以只在eureka的实现里加。
这方案咋又回去了啊，最开始在registry里写的， @CH3CHO 觉得收口比较好，我觉得是OK的，优化了，放在cache里是 你建议的呢
There was a problem hiding this comment.
@johnlanni 本来是收口方案在reconcile.go里单独加了cache
@CH3CHO @johnlanni 方案我们在明确下？这个修改影响到我们的版本升级，想要尽快确定下合入
There was a problem hiding this comment.
但这样的话每一个 registry 就都要实现一遍，会比较麻烦。最好是找一个地方收口管理。
如果只是eureka有这个需求，可以先只在eureka里实现，其他不用管。nacos用户目前也没有这个需求
lcfang
force-pushed
the
main
branch
2 times, most recently
from
489da87 to
9068403
Compare
|
@johnlanni @CH3CHO 之前看到有个issue说nacos也有这样的需求，所以改了eureka和nacos，再帮忙看下吧 @johnlanni @CH3CHO I saw an issue that Nacos also has this demand, so I changed eureka and nacos, so I'll help you check it out |
registry/nacos/watcher.go
Outdated
| return w, nil | ||
| } | ||
|
|
||
| func WithMcpBridgeLister(lister listersv1.McpBridgeLister) WatcherOption { |
There was a problem hiding this comment.
不应该这样搞，你只需要vport，只要传入vport就可以了
There was a problem hiding this comment.
现在vport的设计是放在mcpbridge里注册中心下面的，vport需要通过mcpbridge的配置获取，并且得与注册中心和应用信息对比后再更新到serviceEntry，所以这里加了mcpbridge的监听器，这样配置更新才能及时监听到
…lement to fix compatibility issues caused by inconsistent ports in the service backend (alibaba#2859)
4 participants
Ⅰ. Describe what this PR did
当在eureka或者nacos等注册的服务实例端口不一致的时候，当后端实例端口变化，会导致原来的路由配置失效，新增vport元素，固化serviceEntry中的端口，从而保证路由不失效。该功能应同时修改前端代码适配。
Ⅱ. Does this pull request fix one issue?
fixes higress-group/higress-console#518 & higress-group/higress-console#576 & #2511
Ⅲ. Why don't you add test cases (unit test/integration test)?
Ⅳ. Describe how to verify it
Ⅴ. Special notes for reviews
Ⅰ. Describe what this PR did
When the ports of service instances registered in eureka or nacos are inconsistent, when the ports of the backend instance change, the original routing configuration will be invalidated, and the new vport element will be added to solidify the ports in the serviceEntry to ensure that the routing does not fail. This function should also modify the front-end code adaptation.
Ⅱ. Does this pull request fix one issue?
fixes higress-group/higress-console#518 & higress-group/higress-console#576 & #2511
Ⅲ. Why don't you add test cases (unit test/integration test)?
Ⅳ. Describe how to verify it
V. Special notes for reviews