feat: unify and enhance the lifecycle of an instance#1440
feat: unify and enhance the lifecycle of an instance#1440Saramanda9988 wants to merge 13 commits intoapache:developfrom
Conversation
…ration in informers
…tion in informers
… and fallback handling
…ging and filtering
…rvice configuration
|
robocanic
left a comment
robocanic
left a comment
There was a problem hiding this comment.
Great work! I've reviewd the whole code change and the quality is really high. I left some comments and hope you can discuss them with me.
| } | ||
| } | ||
|
|
||
| func (s *RuntimeInstanceEventSubscriber) getRelatedKubernetesInstance( |
There was a problem hiding this comment.
Discussion: 这里用两种方式来获取runtime instance对应的instance实例，但可能都获取不到，因为用户如果不在pod上打上对应的label/annotions，我们就不知道这个pod是对应哪个dubbo实例。但在k8s中，ip是唯一的，所以可以通过ip的唯一来关联到rpc instance和k8s pod。所以在之前的代码逻辑里，在确定engine type是k8s之后，就会通过runtime instance的ip来反查instance，这是没问题的。现在先用reskey，再用resName，但可以都获取不到。建议是加个通过ip查询来兜底或者是还是用之前的逻辑
There was a problem hiding this comment.
使用了label主要是因为
-
这里拿到的是 podIP，不是 稳定的 serviceIP或者clusterIP，存在旧 rpc instance 残留和 podIP 复用窗口的时候，ip-only 容易把新 pod 的 runtime 信息合并到旧实例上，出现误关联。
-
starting 状态依赖 runtime 侧先创建/融合实例；如果不使用 label/annotation identifier 提供的 mesh/appName/rpcPort，rpc runtime 侧无法可靠识别实例（因为有attributeCheck，或许我们假定rpc一定会去注册？但是示例中的前端服务似乎不会注册到注册中心上），只能等注册中心返回 rpc instance 后再展示，starting 状态会无法显示。
我建议保留现在这样的逻辑，ip可以作为fallback，但是能否在文档中推荐用户标注相关label以获取更准确的生命周期显示？或者有其他更好的方案，比如这里的podIP使用clusterIP？
There was a problem hiding this comment.
- 这种情况是不是不太可能？rpc instance的生命周期长度是比runtime instance 短的。podIP存在的时候rpc instance 的ip不一定存在；但反过来，rpc instance的ip存在，一定能找到一个唯一的runtime instance。
2. 用户如果不改造确实会有这个问题，但这个改造是需要用户去改造他们的CI/CD的，会有一定成本，这里确实需要在文档中说明并推荐用户进行标记
ip可以作为一个fallback，就通过resKey，resName都找不着再用ip。
There was a problem hiding this comment.
对于第一个我可能说的不太准确，k8s在滚动更新的时候，pod ip是完全可以复用的，即旧的pod还没有完全销毁（发出delete事件），新的pod ip相同的pod已经上线，在下面这种情况，会出现实例生命周期混乱：
- 旧 pod 的 RPC instance 还没从注册中心删除。
- 新 pod 复用了旧 pod 的 IP。
- 新 pod 的 runtime 事件先到。
用户没有配置 appName / rpcPort / mesh 这些 identifier，导致 runtime 侧只能走 IP fallback，这时 runtime 侧会把新 pod 的 K8s 状态 merge 到仍然挂着旧 pod RPC 信息的 InstanceResource 上，出现“旧 RPC 信息 + 新 runtime 状态”的混合态，旧实例的rpc注册一消失新实例变Draining或者Starting，之前复现的时候出现过一次，但是这种情况挺苛刻的，这样的时间窗口能否接受？
There was a problem hiding this comment.
即旧的pod还没有完全销毁（发出delete事件），新的pod ip相同的pod已经上线
这在K8s官方文档里有说到吗，我印象中是旧Pod必须完全销毁之后，IP才能被其它新pod复用。
| } | ||
|
|
||
| func (s *RPCInstanceEventSubscriber) getRuntimeInstanceByIp(ip string) *meshresource.RuntimeInstanceResource { | ||
| func (s *RPCInstanceEventSubscriber) getRuntimeInstanceForInstance(instanceRes *meshresource.InstanceResource) *meshresource.RuntimeInstanceResource { |
There was a problem hiding this comment.
Discussion: 这里用ip查出来之后，再用mesh，appName，rpcPort来做一个过滤选择，但可能都会被过滤掉，因为mesh，appName，rpcPort这些属性是需要用户在k8s pod部署时手动打上的。在之前的代码逻辑里面，确定engine type是k8s后用ip来查询，这是因为k8s中ip是唯一的，ip能够将pod和注册中心上的rpc instance一一关联起来，是没有问题的。所以这里建议是加一个兜底返回，或者还是用之前的逻辑
| // sorter: true, | ||
| width: 150 | ||
| }, | ||
| { |
There was a problem hiding this comment.
Discussion：生命周期状态是registerState和DeployState共同决定的，如果把这个生命周期的状态给放到顶部，挨着instanceName，会不会好一点？
| if instance == nil || strutil.IsBlank(instance.DeployState) { | ||
| return "Unknown" | ||
| } | ||
| switch instance.DeployState { |
There was a problem hiding this comment.
Suggestion: 看能不能把DeployState，RegisterState，LifecycleState统统定义成枚举/自定义类型，定义在一个地方，再写上对应的注释，就能清楚每个state的具体含义
2 participants
Please provide a description of this PR:
please refer to: #1399
What changed
To help us figure out who should review this PR, please put an X in all the areas that this PR affects.
Please check any characteristics that apply to this pull request.
screenshot