Skip to content

Refactor plugin reconciliation to ensure only one update on plugin #5148

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Jan 14, 2024
Merged

Refactor plugin reconciliation to ensure only one update on plugin #5148

merged 1 commit into from
Jan 14, 2024

Conversation

JohnNiang
Copy link
Member

@JohnNiang JohnNiang commented Jan 3, 2024 •
edited
Loading

What type of PR is this?

/kind improvement
/area core

What this PR does / why we need it:

This PR mainly refactors plugin reconciliation to ensure only one update on plugin and make the logic clearer.

The next step is to refactor plugin manager to make plugin resources can receive the state changes of the plugin in plugin manager.

It works very well in Windows environment.

See #5129 for more.

Which issue(s) this PR fixes:

Fixes #5129

Special notes for your reviewer:

  • Refine unit tests against plugin reconciler.

Does this PR introduce a user-facing change?

None

@f2c-ci-robot f2c-ci-robot bot added kind/improvement Categorizes issue or PR as related to a improvement. release-note-none Denotes a PR that doesn't merit a release note. do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. labels Jan 3, 2024
@f2c-ci-robot f2c-ci-robot bot requested review from guqing and minliacom January 3, 2024 09:17
@f2c-ci-robot f2c-ci-robot bot added the area/core Issues or PRs related to the Halo Core label Jan 3, 2024
@JohnNiang JohnNiang force-pushed the refactor/plugin-reconciler branch from e50ba7e to 17c5369 Compare January 8, 2024 15:41
@JohnNiang JohnNiang marked this pull request as ready for review January 8, 2024 15:41
@JohnNiang JohnNiang changed the title WIP: Refactor plugin reconciliation to ensure only one update on plugin Refactor plugin reconciliation to ensure only one update on plugin Jan 8, 2024
@f2c-ci-robot f2c-ci-robot bot requested a review from lan-yonghui January 8, 2024 15:41
@f2c-ci-robot f2c-ci-robot bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jan 8, 2024
@JohnNiang
Copy link
Member Author

/ping @halo-dev/sig-halo

@guqing
Copy link
Member

guqing commented Jan 9, 2024

应该是要继续限制一下生产模式下如果运行的插件不在 plugins-root 中不允许运行吧，也就是要同时使用开发模式和生产模式的插件配置必须要是开发模式

@JohnNiang JohnNiang force-pushed the refactor/plugin-reconciler branch from 17c5369 to d454eca Compare January 9, 2024 07:26
@codecov Codecov
Copy link

codecov bot commented Jan 9, 2024 •
edited
Loading

Codecov Report

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

Comparison is base (57e3394) 55.91% compared to head (af46ca0) 56.41%.
Report is 15 commits behind head on main.

❗ Current head af46ca0 differs from pull request most recent head 87a8d64. Consider uploading reports for the commit 87a8d64 to get more accurate results

Files Patch % Lines
...pp/core/extension/reconciler/PluginReconciler.java 83.18% 20 Missing and 17 partials ⚠️
.../main/java/run/halo/app/core/extension/Plugin.java 0.00% 12 Missing ⚠️
...va/run/halo/app/plugin/SpringExtensionFactory.java 0.00% 9 Missing ⚠️
...app/core/extension/service/DefaultRoleService.java 0.00% 3 Missing and 1 partial ⚠️
.../halo/app/plugin/PluginDevelopmentInitializer.java 0.00% 4 Missing ⚠️
...lo/app/core/extension/endpoint/PluginEndpoint.java 0.00% 3 Missing ⚠️
...src/main/java/run/halo/app/plugin/PluginUtils.java 66.66% 0 Missing and 1 partial ⚠️
Additional details and impacted files 
@@             Coverage Diff              @@
##               main    #5148      +/-   ##
============================================
+ Coverage     55.91%   56.41%   +0.49%     
+ Complexity     3032     3006      -26     
============================================
  Files           524      525       +1     
  Lines         17826    17606     -220     
  Branches       1329     1303      -26     
============================================
- Hits           9968     9933      -35     
+ Misses         7309     7134     -175     
+ Partials        549      539      -10

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@JohnNiang
Copy link
Member Author

应该是要继续限制一下生产模式下如果运行的插件不在 plugins-root 中不允许运行吧，也就是要同时使用开发模式和生产模式的插件配置必须要是开发模式

如果配置了 halo.plugin.runtime-mode=deployment 的话， 就默认不允许运行开发模式的插件，也就意味着即使插件配置了 plugin-path annotations 也不能正常运行。我这样理解对么？

@guqing
Copy link
Member

guqing commented Jan 9, 2024 •
edited
Loading

应该是要继续限制一下生产模式下如果运行的插件不在 plugins-root 中不允许运行吧，也就是要同时使用开发模式和生产模式的插件配置必须要是开发模式

如果配置了 halo.plugin.runtime-mode=deployment 的话， 就默认不允许运行开发模式的插件，也就意味着即使插件配置了 plugin-path annotations 也不能正常运行。我这样理解对么？

是的，之前是这样

@JohnNiang JohnNiang force-pushed the refactor/plugin-reconciler branch from d454eca to af46ca0 Compare January 9, 2024 08:01
@JohnNiang
Copy link
Member Author

JohnNiang commented Jan 9, 2024 •
edited
Loading

是的，之前是这样

Hi @guqing ，已修改，请看测试用例：PluginReconcilerTest#shouldNotStartPluginWithDevModeInNonDevEnv。

guqing
guqing approved these changes Jan 9, 2024
View reviewed changes
Copy link
Member

@guqing guqing left a comment

Choose a reason for hiding this comment

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

经测试在 mac 目前没有发现问题，兼容性 ok，windows 没有试过没环境

@f2c-ci-robot f2c-ci-robot bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jan 9, 2024
@JohnNiang
Copy link
Member Author

/milestone 2.12.x

@f2c-ci-robot f2c-ci-robot bot added this to the 2.12.x milestone Jan 9, 2024
@guqing
Copy link
Member

guqing commented Jan 10, 2024 •
edited
Loading

/remove-approve

忽略了一点之前没发现，需要在插件停止时也能获取到文章的设置项，且可以保存配置，所以文章的 setting 不应该在停止时被删除而是卸载时删除

当停止时：
image
当启动时：
image

@f2c-ci-robot f2c-ci-robot
Copy link

f2c-ci-robot bot commented Jan 10, 2024

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please ask for approval from johnniang. For more information see the Kubernetes Code Review Process.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@f2c-ci-robot f2c-ci-robot bot removed the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jan 10, 2024
@JohnNiang
Copy link
Member Author

Hi @guqing ，我这里有一个不确定的小问题：在 Plugin Reconciler 中创建/更新的 setting，将会在插件启动的时候又会初始化一遍，而且是强制更新。即使我为 setting 添加 annotation：delete-stage=uninstall 似乎也会被覆盖掉。不知道之前是如何实现的呢？

@JohnNiang
Copy link
Member Author

/unhold

The latest issue was resolved in e05640f.

/ping @halo-dev/sig-halo

@f2c-ci-robot f2c-ci-robot bot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jan 11, 2024
@JohnNiang JohnNiang requested a review from guqing January 12, 2024 02:29
@guqing
Copy link
Member

guqing commented Jan 12, 2024

image 开发模式下启动瞬间插件出现资源无法加载

@JohnNiang JohnNiang force-pushed the refactor/plugin-reconciler branch from e05640f to fa8319f Compare January 12, 2024 07:22
@JohnNiang
Copy link
Member Author

Hi @guqing ，已彻底修复无法找到 extension/ 目录问题。

@guqing
Copy link
Member

guqing commented Jan 12, 2024

Hi @guqing ，已彻底修复无法找到 extension/ 目录问题。

好的 我试试

@guqing
Copy link
Member

guqing commented Jan 12, 2024

@JohnNiang 我在开发模式修改了 plugin.yaml 的一些信息比如 description 后 reloadPlugin，插件的信息没有变更还是旧的

@JohnNiang
Copy link
Member Author

@JohnNiang 我在开发模式修改了 plugin.yaml 的一些信息比如 description 后 reloadPlugin，插件的信息没有变更还是旧的

目前似乎不只是开发环境修改 plugin.yaml 失效，升级后似乎也没有正常更新 plugin 资源。

@guqing
Copy link
Member

guqing commented Jan 12, 2024

@JohnNiang 我在开发模式修改了 plugin.yaml 的一些信息比如 description 后 reloadPlugin，插件的信息没有变更还是旧的

目前似乎不只是开发环境修改 plugin.yaml 失效，升级后似乎也没有正常更新 plugin 资源。

我是尝试的开发环境发现的问题，然后切到 main 是正确的

@JohnNiang JohnNiang force-pushed the refactor/plugin-reconciler branch from fa8319f to dbc6738 Compare January 12, 2024 17:38
@JohnNiang
Copy link
Member Author

@JohnNiang 我在开发模式修改了 plugin.yaml 的一些信息比如 description 后 reloadPlugin，插件的信息没有变更还是旧的

目前似乎不只是开发环境修改 plugin.yaml 失效，升级后似乎也没有正常更新 plugin 资源。

我是尝试的开发环境发现的问题，然后切到 main 是正确的

确实是因为当前 PR 未考虑到升级和重载问题，现在已修复。

@JohnNiang
Copy link
Member Author

/ping @halo-dev/sig-halo

@ruibaby
Copy link
Member

ruibaby commented Jan 14, 2024 •
edited
Loading

在开发环境下，无法正常使用 ./gradlew reloadPlugin，日志：

> Task :reloadPlugin FAILED

FAILURE: Build failed with an exception.

* What went wrong:
Execution failed for task ':reloadPlugin'.
> Reload plugin failed, Bad Request

* Try:
> Run with --stacktrace option to get the stack trace.
> Run with --info or --debug option to get more log output.
> Run with --scan to get full insights.

* Get more help at https://help.gradle.org

BUILD FAILED in 8s
14 actionable tasks: 8 executed, 6 up-to-date

2024-01-14T12:05:23.419+08:00  INFO 7 --- [nReconciler-t-1] run.halo.app.plugin.HaloPluginManager    : Start plugin 'PluginSearchWidget@1.2.0'
2024-01-14T12:05:23.423+08:00  INFO 7 --- [nReconciler-t-1] run.halo.app.plugin.BasePlugin           : Initialized plugin PluginSearchWidget
2024-01-14T12:05:23.449+08:00  INFO 7 --- [nReconciler-t-1] r.h.a.c.e.reconciler.PluginReconciler    : Loading plugin PluginFeed
2024-01-14T12:05:23.455+08:00  INFO 7 --- [nReconciler-t-1] org.pf4j.AbstractPluginManager           : Plugin 'PluginFeed@1.2.0' resolved
2024-01-14T12:05:23.456+08:00  INFO 7 --- [nReconciler-t-1] r.h.a.c.e.reconciler.PluginReconciler    : Loaded plugin PluginFeed
2024-01-14T12:05:23.456+08:00  INFO 7 --- [nReconciler-t-1] r.h.a.c.e.reconciler.PluginReconciler    : Initializing setting and config map for plugin PluginFeed
2024-01-14T12:05:23.483+08:00  INFO 7 --- [nReconciler-t-1] r.h.a.c.e.reconciler.PluginReconciler    : Initialized setting plugin-feed-setting for plugin PluginFeed
2024-01-14T12:05:23.489+08:00  INFO 7 --- [nReconciler-t-1] r.h.a.c.e.reconciler.PluginReconciler    : Initialized config map plugin-feed-config for plugin PluginFeed
2024-01-14T12:05:23.505+08:00  INFO 7 --- [nReconciler-t-1] r.h.a.c.e.reconciler.PluginReconciler    : Resolving logo resource for plugin PluginFeed
2024-01-14T12:05:23.505+08:00  INFO 7 --- [nReconciler-t-1] r.h.a.c.e.reconciler.PluginReconciler    : Resolving main.js and style.css for plugin PluginFeed
2024-01-14T12:05:23.506+08:00  INFO 7 --- [nReconciler-t-1] run.halo.app.plugin.HaloPluginManager    : Start plugin 'PluginFeed@1.2.0'
2024-01-14T12:05:23.513+08:00  INFO 7 --- [nReconciler-t-1] run.halo.app.plugin.BasePlugin           : Initialized plugin PluginFeed
2024-01-14T12:05:23.549+08:00  INFO 7 --- [nReconciler-t-1] r.h.a.c.e.reconciler.PluginReconciler    : Initializing setting and config map for plugin plugin-docsme
2024-01-14T12:05:23.570+08:00  INFO 7 --- [nReconciler-t-1] r.h.a.c.e.reconciler.PluginReconciler    : Resolving logo resource for plugin plugin-docsme
2024-01-14T12:05:23.570+08:00  INFO 7 --- [nReconciler-t-1] r.h.a.c.e.reconciler.PluginReconciler    : Resolving main.js and style.css for plugin plugin-docsme
Initialize system failed: {"type":"about:blank","title":"Conflict","status":409,"detail":"System has been initialized","instance":"http://localhost:8090/apis/api.console.halo.run/v1alpha1/system/initialize","requestId":"5201569f-3","timestamp":"2024-01-14T04:05:25.726083565Z"}
Halo 初始化成功，访问： http://localhost:8090/console
用户名：admin
密码：admin

2024-01-14T12:06:32.258+08:00  INFO 7 --- [nReconciler-t-1] r.h.a.c.e.reconciler.PluginReconciler    : Deleting old plugin file file:/data/plugins/plugin-docsme/ for plugin plugin-docsme, and new load location is file:///root/.halo2/plugins/plugin-docsme-1.0.0-SNAPSHOT.jar.
2024-01-14T12:06:32.259+08:00  WARN 7 --- [nReconciler-t-1] r.h.a.c.e.reconciler.PluginReconciler    : Failed to delete old plugin file file:/data/plugins/plugin-docsme/ for plugin plugin-docsme

java.nio.file.DirectoryNotEmptyException: /data/plugins/plugin-docsme
        at java.base/sun.nio.fs.UnixFileSystemProvider.implDelete(Unknown Source) ~[na:na]
        at java.base/sun.nio.fs.AbstractFileSystemProvider.deleteIfExists(Unknown Source) ~[na:na]
        at java.base/java.nio.file.Files.deleteIfExists(Unknown Source) ~[na:na]
        at run.halo.app.core.extension.reconciler.PluginReconciler.resolveLoadLocation(PluginReconciler.java:418) ~[classes/:2.12.0-SNAPSHOT]
        at run.halo.app.core.extension.reconciler.PluginReconciler.lambda$reconcile$0(PluginReconciler.java:111) ~[classes/:2.12.0-SNAPSHOT]
        at java.base/java.util.Optional.map(Unknown Source) ~[na:na]
        at run.halo.app.core.extension.reconciler.PluginReconciler.reconcile(PluginReconciler.java:96) ~[classes/:2.12.0-SNAPSHOT]
        at run.halo.app.core.extension.reconciler.PluginReco
nciler.reconcile(PluginReconciler.java:65) ~[classes/:2.12.0-SNAPSHOT]
        at run.halo.app.extension.controller.DefaultController$Worker.run(DefaultController.java:163) ~[api-2.12.0-SNAPSHOT.jar:na]
        at java.base/java.util.concurrent.Executors$RunnableAdapter.call(Unknown Source) ~[na:na]
        at java.base/java.util.concurrent.FutureTask.run(Unknown Source) ~[na:na]
        at java.base/java.util.concurrent.ThreadPoolExecutor.runWorker(Unknown Source) ~[na:na]
        at java.base/java.util.concurrent.ThreadPoolExecutor$Worker.run(Unknown Source) ~[na:na]
        at java.base/java.lang.Thread.run(Unknown Source) ~[na:na]
2024-01-14T12:06:32.262+08:00  INFO 7 --- [nReconciler-t-1] r.h.a.c.e.reconciler.PluginReconciler    : Initializing setting and config map for plugin plugin-docsme
2024-01-14T12:06:32.266+08:00  INFO 7 --- [nReconciler-t-1] r.h.a.c.e.reconciler.PluginReconciler    : Resolving logo resource for plugin plugin-docsme
2024-01-14T12:06:32.266+08:00  INFO 7 --- [nReconciler-t-1] r.h.a.c.e.reconciler.PluginReconciler    : Resolving main.js and style.css for plugin plugin-docsme
2024-01-14T12:06:32.278+08:00  INFO 7 --- [nReconciler-t-1] r.h.a.c.e.reconciler.PluginReconciler    : Initializing setting and config map for plugin plugin-docsme
2024-01-14T12:06:32.282+08:00  INFO 7 --- [nReconciler-t-1] r.h.a.c.e.reconciler.PluginReconciler    : Resolving logo resource for plugin plugin-docsme
2024-01-14T12:06:32.283+08:00  INFO 7 --- [nReconciler-t-1] r.h.a.c.e.reconciler.PluginReconciler    : Resolving main.js and style.css for plugin plugin-docsme

复现步骤：

  1. 基于此 PR 构建一个 Docker 镜像，假设镜像 tag 为 pr-5148

  2. 用任意一个插件，在 build.gralde 中配置 halo.version = "pr-5148"

    halo {
    version = 'index'
    debug = true
}

  3. 使用 ./gradlew haloServer 启动开发容器。

  4. 修改 plugin.yaml 任意字段，然后执行 ./gradlew reloadPlugin。

在 2.11 下没有此问题。

@JohnNiang
Copy link
Member Author

Hi @ruibaby ，目前我通过配置 halo.plugin.fixed-plugin-path 是可以正常 reload 的。关于使用 Gradle 插件启用 Halo 并 reload 我还需要测试一下。

@JohnNiang
Refactor plugin reconciliation to ensure only one update on plugin
87a8d64 
Signed-off-by: John Niang <johnniang@foxmail.com>
@JohnNiang JohnNiang force-pushed the refactor/plugin-reconciler branch from dbc6738 to 87a8d64 Compare January 14, 2024 04:54
@JohnNiang
Copy link
Member Author

Hi @ruibaby ，确实是因为 reload 和 upadte 插件的时候强制覆盖了 annotations 和 labels，导致修改了插件的运行模式（开发模式 -> 生产模式），最终无法正确查找到 plugin.yaml。最新的提交已修复该问题。

ruibaby
ruibaby reviewed Jan 14, 2024
View reviewed changes
Copy link
Member

@ruibaby ruibaby left a comment

Choose a reason for hiding this comment

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

已经测试过：

  1. gradle haloServer task
  2. gradle reload task
  3. 修改配置文件

没有再出现过上述问题。

/lgtm

@f2c-ci-robot f2c-ci-robot bot added the lgtm Indicates that a PR is ready to be merged. label Jan 14, 2024
@ruibaby ruibaby merged commit 6d49047 into halo-dev:main Jan 14, 2024
@JohnNiang JohnNiang deleted the refactor/plugin-reconciler branch January 14, 2024 15:04
@ruibaby ruibaby modified the milestones: 2.12.x, 2.12.0 Jan 15, 2024
This was referenced Jan 22, 2024
Closed
Merged
f2c-ci-robot bot pushed a commit that referenced this pull request Jan 24, 2024
@JohnNiang
Revert: refresh the plugin wrapper when starting the plugin (#5238)
7e8df33 
#### What type of PR is this?

/kind improvement
/area core
/milestone 2.12.x

#### What this PR does / why we need it:

This PR reverts changes in PR <#4023>, mainly thanks to PR <#5148>.

We don't need to refresh the plugin wrapper on every startup, because we entirely disable the plugin in plugin manager when disabling plugin at console.

#### Which issue(s) this PR fixes:

Fixes #4016

#### Does this PR introduce a user-facing change?

```release-note
None
```
@guqing guqing mentioned this pull request Jan 26, 2024
Closed
@guqing guqing mentioned this pull request Feb 22, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ruibaby ruibaby ruibaby left review comments

@minliacom minliacom Awaiting requested review from minliacom

@lan-yonghui lan-yonghui Awaiting requested review from lan-yonghui

@guqing guqing Awaiting requested review from guqing

Assignees

@ruibaby ruibaby

Labels
area/core Issues or PRs related to the Halo Core kind/improvement Categorizes issue or PR as related to a improvement. lgtm Indicates that a PR is ready to be merged. release-note-none Denotes a PR that doesn't merit a release note.
Projects
None yet
Milestone
2.12.0
Development

Successfully merging this pull request may close these issues.

Recommend to refactor plugin reconciler with detached method to interact with plugin manager
3 participants
@JohnNiang @guqing @ruibaby