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

feature: add registry watcherStatus endpoint (#913) #915

Merged
merged 3 commits into from
Apr 24, 2024

Conversation

hanxiantao
Copy link
Collaborator

@hanxiantao hanxiantao commented Apr 20, 2024

Ⅰ. Describe what this PR did

controller中对外暴漏registry watcher的状态

Ⅱ. Does this pull request fix one issue?

fixes #913

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

Ⅳ. Describe how to verify it

控制台中新增了两个Eureka的类型的服务来源
image
名称为error的healthyStatus为false,一直访问eureka server失败
image
请求controller的/registry/watcherStatus接口,效果如下:
image

Ⅴ. Special notes for reviews

@CLAassistant
Copy link

CLAassistant commented Apr 20, 2024

CLA assistant check
All committers have signed the CLA.

registry/reconcile/reconcile.go Outdated Show resolved Hide resolved
registry/reconcile/reconcile.go Outdated Show resolved Hide resolved
@hanxiantao hanxiantao requested a review from CH3CHO April 21, 2024 07:17
@hanxiantao
Copy link
Collaborator Author

@CH3CHO 相关问题已修复,有时间麻烦再帮忙review下

@CH3CHO
Copy link
Collaborator

CH3CHO commented Apr 21, 2024

纯代码层面的问题见行内注释。以下为功能层面的问题:

  1. 不同的 registry 的内部实现逻辑不同。两个 nacos 类型的服务来源在配置的地址完全连不上的情况下时,是无法生成对应的 watcher 的,导致接口中不会出现它们的记录;
  2. static 和 dns 两种类型的服务来源在返回的数据中 type 为空。

watcherStatusList := registryReconciler.GetRegistryWatcherStatusList()
writeJSON(w, watcherStatusList)

w.WriteHeader(http.StatusOK)
Copy link
Collaborator

Choose a reason for hiding this comment

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

如果 writeJSON 执行出错了,下面再是写个 StatusOK 会不会有问题啊?

@hanxiantao
Copy link
Collaborator Author

纯代码层面的问题见行内注释。以下为功能层面的问题:

  1. 不同的 registry 的内部实现逻辑不同。两个 nacos 类型的服务来源在配置的地址完全连不上的情况下时,是无法生成对应的 watcher 的,导致接口中不会出现它们的记录;
  2. static 和 dns 两种类型的服务来源在返回的数据中 type 为空。

代码相关问题已修复
关于问题1:配置一个错误的nacos地址,接口是可以返回nacos信息的
image
image
一开始我遇到的问题是本地/var/log/nacos目录没有权限创建所以导致watcher创建失败,这个如果有问题的话可能会导致httpserver启动失败
关于问题2:registry/direct/watcher.go中实现了GetRegistryType()方法,返回了type
image

@hanxiantao hanxiantao requested a review from CH3CHO April 22, 2024 01:08
@CH3CHO
Copy link
Collaborator

CH3CHO commented Apr 22, 2024

纯代码层面的问题见行内注释。以下为功能层面的问题:

  1. 不同的 registry 的内部实现逻辑不同。两个 nacos 类型的服务来源在配置的地址完全连不上的情况下时,是无法生成对应的 watcher 的,导致接口中不会出现它们的记录;
  2. static 和 dns 两种类型的服务来源在返回的数据中 type 为空。

关于上面的两个问题:

  1. 你试一下 Nacos 2.x;
  2. 这个 OK 了。

@hanxiantao
Copy link
Collaborator Author

hanxiantao commented Apr 23, 2024

纯代码层面的问题见行内注释。以下为功能层面的问题:

  1. 不同的 registry 的内部实现逻辑不同。两个 nacos 类型的服务来源在配置的地址完全连不上的情况下时,是无法生成对应的 watcher 的,导致接口中不会出现它们的记录;
  2. static 和 dns 两种类型的服务来源在返回的数据中 type 为空。

关于上面的两个问题:

  1. 你试一下 Nacos 2.x;
  2. 这个 OK 了。

我这边验证了下,在Nacos 2.x且开启了认证的情况下会出现watcher创建失败的情况
如果watcher创建失败,目前Reconciler中的registries和watchers都不包含该RegistryConfig的信息(registries和watchers的内容应该保持一致),所以会在Reconciler中新增一个failedRegistries字段来保存创建watcher失败的RegistryConfig
在每次Reconcile的时候会清理failedRegistries,如果本次Reconcile时创建watcher失败,会添加到failedRegistries中

Copy link
Collaborator

@CH3CHO CH3CHO left a comment

Choose a reason for hiding this comment

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

For the API itself, LGTM

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
Copy link
Collaborator

纯代码层面的问题见行内注释。以下为功能层面的问题:

  1. 不同的 registry 的内部实现逻辑不同。两个 nacos 类型的服务来源在配置的地址完全连不上的情况下时,是无法生成对应的 watcher 的,导致接口中不会出现它们的记录;
  2. static 和 dns 两种类型的服务来源在返回的数据中 type 为空。

关于上面的两个问题:

  1. 你试一下 Nacos 2.x;
  2. 这个 OK 了。

我这边验证了下,在Nacos 2.x且开启了认证的情况下会出现watcher创建失败的情况 如果watcher创建失败,目前Reconciler中的registries和watchers都不包含该RegistryConfig的信息(registries和watchers的内容应该保持一致),所以会在Reconciler中新增一个failedRegistries字段来保存创建watcher失败的RegistryConfig 在每次Reconcile的时候会清理failedRegistries,如果本次Reconcile时创建watcher失败,会添加到failedRegistries中

@CH3CHO @hanxiantao 是否watchers中找不到对应信息,就认为not ready就可以?

@CH3CHO
Copy link
Collaborator

CH3CHO commented Apr 23, 2024

@CH3CHO @hanxiantao 是否watchers中找不到对应信息,就认为not ready就可以?

也可以的。

@hanxiantao
Copy link
Collaborator Author

@CH3CHO @hanxiantao 是否watchers中找不到对应信息,就认为not ready就可以?

也可以的。

如果是这样的话,目前的代码可以合并的,新增一个failedRegistries来维护创建失败的,其实也有点多余,而且只在这个接口中使用到

@CH3CHO CH3CHO merged commit a787088 into alibaba:main Apr 24, 2024
8 checks passed
@hanxiantao hanxiantao deleted the controller-registry-debug branch June 7, 2024 00:55
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.

【建议】服务来源配置Nacos等增加通讯检测和手动同步
4 participants