From 9bea5ef1c9ba23e66c0e3a7576a4110965234f52 Mon Sep 17 00:00:00 2001 From: guqing <38999863+guqing@users.noreply.github.com> Date: Mon, 24 Jul 2023 16:22:42 +0800 Subject: [PATCH] fix: inconsistency status occurred during plugin startup due to optimistic locking conflict (#4275) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit #### What type of PR is this? /kind improvement /area core /area plugin /milestone 2.7.x #### What this PR does / why we need it: 修复插件启动成功但更新数据失败而导致插件状态不符合预期的问题 how to test it? 1. 安装一个带 console 页面的插件并停用它 2. 使用 IDEA 在 doStart 方法最后更新数据的地方也就是 https://github.com/halo-dev/halo/blob/834e37cf130f2487fab268c9bc7198555933f0dc/application/src/main/java/run/halo/app/core/extension/reconciler/PluginReconciler.java#L447 处打断点，suspend 勾选为 Thread image 3. 启用插件,会执行到断点处 4. 使用如下命令更新数据将 status 删除以模拟乐观锁冲突并清除 status 状态排除干扰 ```shell curl -u admin:admin -X PUT http://localhost:8090/apis/plugin.halo.run/v1alpha1/plugins/{name} --data '替换为 plugin 的 json ' ``` 5. 放行端点 根据上述步骤先在 main 分支浮现然后在切换到此 PR 对比结果，期望插件的状态为启动成功且 status 数据示例如下： conditions 有两条会因为乐观锁更新失败一次且entry和stylesheet都有值 ```json { "phase": "STARTED", "conditions": [ { "type": "STARTED", "status": "TRUE", "lastTransitionTime": "2023-07-21T07:46:01.274211Z", "message": "Started successfully", "reason": "STARTED" }, { "type": "FAILED", "status": "FALSE", "lastTransitionTime": "2023-07-21T07:46:01.248001Z", "message": "Failed to update table [extensions]; Version does not match for row with Id [/registry/plugin.halo.run/plugins/PluginBytemd]", "reason": "UnexpectedState" } ], "lastStartTime": "2023-07-21T07:46:01.273625Z", "entry": "/plugins/PluginBytemd/assets/console/main.js?version=1.1.0-SNAPSHOT", "stylesheet": "/plugins/PluginBytemd/assets/console/style.css?version=1.1.0-SNAPSHOT", "logo": "/plugins/PluginBytemd/assets/logo.png?version=1.1.0-SNAPSHOT", "loadLocation": "file:///Users/guqing/Development/halo-sigs/plugin-bytemd/" } ``` #### Which issue(s) this PR fixes: Fixes #4273 #### Does this PR introduce a user-facing change? ```release-note 修复插件启动成功但更新数据失败而导致插件状态不符合预期的问题 ``` --- .../reconciler/PluginReconciler.java | 9 +++--- .../reconciler/PluginReconcilerTest.java | 29 +++++++++++++++---- 2 files changed, 29 insertions(+), 9 deletions(-) diff --git a/application/src/main/java/run/halo/app/core/extension/reconciler/PluginReconciler.java b/application/src/main/java/run/halo/app/core/extension/reconciler/PluginReconciler.java index 441e82a53..cb42c5f06 100644 --- a/application/src/main/java/run/halo/app/core/extension/reconciler/PluginReconciler.java +++ b/application/src/main/java/run/halo/app/core/extension/reconciler/PluginReconciler.java @@ -320,11 +320,12 @@ public class PluginReconciler implements Reconciler { Plugin.PluginStatus status = plugin.statusNonNull(); PluginWrapper pluginWrapper = haloPluginManager.getPlugin(pluginName); - PluginState pluginState = Optional.ofNullable(pluginWrapper) - .map(PluginWrapper::getPluginState) - .orElse(PluginState.FAILED); + if (pluginWrapper != null) { + pluginWrapper.setPluginState(PluginState.FAILED); + pluginWrapper.setFailedException(e); + } - status.setPhase(pluginState); + status.setPhase(PluginState.FAILED); Plugin.PluginStatus oldStatus = JsonUtils.deepCopy(status); Condition condition = Condition.builder() diff --git a/application/src/test/java/run/halo/app/core/extension/reconciler/PluginReconcilerTest.java b/application/src/test/java/run/halo/app/core/extension/reconciler/PluginReconcilerTest.java index 9d6d72094..714cf9b60 100644 --- a/application/src/test/java/run/halo/app/core/extension/reconciler/PluginReconcilerTest.java +++ b/application/src/test/java/run/halo/app/core/extension/reconciler/PluginReconcilerTest.java @@ -115,11 +115,7 @@ class PluginReconcilerTest { // mock start plugin failed when(extensionClient.fetch(eq(Plugin.class), eq("apples"))).thenReturn(Optional.of(plugin)); - when(haloPluginManager.startPlugin(any())).thenAnswer((Answer) invocation -> { - // mock plugin real state is started - when(pluginWrapper.getPluginState()).thenReturn(PluginState.FAILED); - return PluginState.FAILED; - }); + when(haloPluginManager.startPlugin(any())).thenReturn(PluginState.FAILED); // mock plugin real state is started when(pluginWrapper.getPluginState()).thenReturn(PluginState.STOPPED); @@ -497,6 +493,29 @@ class PluginReconcilerTest { } } + @Test + void persistenceFailureStatus() { + String name = "fake-plugin"; + Plugin plugin = new Plugin(); + Plugin.PluginStatus status = new Plugin.PluginStatus(); + plugin.setStatus(status); + when(extensionClient.fetch(eq(Plugin.class), eq(name))) + .thenReturn(Optional.of(plugin)); + PluginWrapper pluginWrapper = mock(PluginWrapper.class); + when(haloPluginManager.getPlugin(eq(name))) + .thenReturn(pluginWrapper); + Throwable error = mock(Throwable.class); + pluginReconciler.persistenceFailureStatus(name, error); + + assertThat(status.getPhase()).isEqualTo(PluginState.FAILED); + assertThat(status.getConditions()).hasSize(1); + assertThat(status.getConditions().peek().getType()) + .isEqualTo(PluginState.FAILED.toString()); + + verify(pluginWrapper).setPluginState(eq(PluginState.FAILED)); + verify(pluginWrapper).setFailedException(eq(error)); + } + private ArgumentCaptor doReconcileNeedRequeue() { ArgumentCaptor pluginCaptor = ArgumentCaptor.forClass(Plugin.class); doNothing().when(extensionClient).update(pluginCaptor.capture());