refactor: plugin reconciler to fix entry path when optimistic lock occurred (#2625)

#### What type of PR is this?
/kind improvement
/area core
/milestone 2.0
#### What this PR does / why we need it:
重构 PluginReconciler,避免因为每一段逻辑的执行时间太长而增加乐观锁错误的发生概率
修复判断逻辑问题导致启动时因为发生乐关锁错误造成没有填充 entry 和 stylesheet 字段的问题
#### Which issue(s) this PR fixes:
Fixes #2616

#### Special notes for your reviewer:
/cc @halo-dev/sig-halo 
#### Does this PR introduce a user-facing change?

```release-note
None
```
pull/2629/head
guqing 2022-10-26 11:10:13 +08:00 committed by GitHub
parent 160dd909cc
commit 7c3fc3ac02
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
5 changed files with 105 additions and 93 deletions

View File

@ -189,6 +189,8 @@ public class PluginEndpoint implements CustomEndpoint {
.flatMap(this::transferToTemp) .flatMap(this::transferToTemp)
.flatMap(tempJarFilePath -> { .flatMap(tempJarFilePath -> {
var plugin = new YamlPluginFinder().find(tempJarFilePath); var plugin = new YamlPluginFinder().find(tempJarFilePath);
// Disable auto enable during installation
plugin.getSpec().setEnabled(false);
return client.fetch(Plugin.class, plugin.getMetadata().getName()) return client.fetch(Plugin.class, plugin.getMetadata().getName())
.switchIfEmpty(Mono.defer(() -> client.create(plugin))) .switchIfEmpty(Mono.defer(() -> client.create(plugin)))
.publishOn(Schedulers.boundedElastic()) .publishOn(Schedulers.boundedElastic())
@ -204,12 +206,11 @@ public class PluginEndpoint implements CustomEndpoint {
FileUtils.copy(tempJarFilePath, pluginFilePath); FileUtils.copy(tempJarFilePath, pluginFilePath);
return created; return created;
}) })
.doOnError(error -> { .onErrorResume(
log.error("Failed to install plugin", error); error -> client.fetch(Plugin.class, plugin.getMetadata().getName())
client.fetch(Plugin.class, plugin.getMetadata().getName()) .flatMap(client::delete)
.map(client::delete) .then(Mono.error(error))
.subscribe(); )
})
.doFinally(signalType -> { .doFinally(signalType -> {
try { try {
Files.deleteIfExists(tempJarFilePath); Files.deleteIfExists(tempJarFilePath);

View File

@ -7,7 +7,6 @@ import java.time.Instant;
import java.util.HashSet; import java.util.HashSet;
import java.util.List; import java.util.List;
import java.util.Map; import java.util.Map;
import java.util.Objects;
import java.util.Set; import java.util.Set;
import java.util.stream.Collectors; import java.util.stream.Collectors;
import lombok.extern.slf4j.Slf4j; import lombok.extern.slf4j.Slf4j;
@ -44,67 +43,49 @@ public class PluginReconciler implements Reconciler<Request> {
@Override @Override
public Result reconcile(Request request) { public Result reconcile(Request request) {
return client.fetch(Plugin.class, request.name()) client.fetch(Plugin.class, request.name())
.map(plugin -> { .ifPresent(plugin -> {
if (plugin.getMetadata().getDeletionTimestamp() != null) { if (plugin.getMetadata().getDeletionTimestamp() != null) {
cleanUpResourcesAndRemoveFinalizer(request.name()); cleanUpResourcesAndRemoveFinalizer(request.name());
return new Result(false, null); return;
} }
addFinalizerIfNecessary(plugin); addFinalizerIfNecessary(plugin);
reconcilePluginState(plugin.getMetadata().getName());
final Plugin oldPlugin = JsonUtils.deepCopy(plugin); });
try {
reconcilePluginState(plugin);
// TODO: reconcile other plugin resources
if (!Objects.equals(oldPlugin, plugin)) {
// update plugin when attributes changed
client.update(plugin);
}
} catch (Exception e) {
// update plugin and requeue
client.update(plugin);
log.error(e.getMessage(), e);
return new Result(true, null);
}
return new Result(false, null); return new Result(false, null);
})
.orElse(new Result(false, null));
} }
private void reconcilePluginState(Plugin plugin) { private void reconcilePluginState(String name) {
Plugin.PluginStatus pluginStatus = plugin.statusNonNull(); if (haloPluginManager.getPlugin(name) == null) {
String name = plugin.getMetadata().getName();
PluginWrapper pluginWrapper = haloPluginManager.getPlugin(name);
if (pluginWrapper == null) {
ensurePluginLoaded(); ensurePluginLoaded();
pluginWrapper = haloPluginManager.getPlugin(name);
} }
client.fetch(Plugin.class, name).ifPresent(plugin -> {
Plugin oldPlugin = JsonUtils.deepCopy(plugin);
Plugin.PluginStatus pluginStatus = plugin.statusNonNull();
PluginWrapper pluginWrapper = haloPluginManager.getPlugin(name);
if (pluginWrapper == null) { if (pluginWrapper == null) {
pluginStatus.setPhase(PluginState.FAILED); pluginStatus.setPhase(PluginState.FAILED);
pluginStatus.setReason("PluginNotFound"); pluginStatus.setReason("PluginNotFound");
pluginStatus.setMessage("Plugin " + name + " not found in plugin manager"); pluginStatus.setMessage("Plugin " + name + " not found in plugin manager");
return; } else {
}
if (!Objects.equals(pluginStatus.getPhase(), pluginWrapper.getPluginState())) {
// Set to the correct state // Set to the correct state
pluginStatus.setPhase(pluginWrapper.getPluginState()); pluginStatus.setPhase(pluginWrapper.getPluginState());
}
if (haloPluginManager.getUnresolvedPlugins().contains(pluginWrapper)) { if (haloPluginManager.getUnresolvedPlugins().contains(pluginWrapper)) {
// load and resolve plugin // load and resolve plugin
haloPluginManager.loadPlugin(pluginWrapper.getPluginPath()); haloPluginManager.loadPlugin(pluginWrapper.getPluginPath());
} }
if (shouldReconcileStartState(plugin)) {
startPlugin(plugin);
} }
if (shouldReconcileStopState(plugin)) { if (!plugin.equals(oldPlugin)) {
stopPlugin(plugin); client.update(plugin);
} }
});
startPlugin(name);
stopPlugin(name);
} }
private void ensurePluginLoaded() { private void ensurePluginLoaded() {
@ -126,19 +107,28 @@ public class PluginReconciler implements Reconciler<Request> {
&& plugin.statusNonNull().getPhase() != PluginState.STARTED; && plugin.statusNonNull().getPhase() != PluginState.STARTED;
} }
private void startPlugin(Plugin plugin) { private void startPlugin(String pluginName) {
String pluginName = plugin.getMetadata().getName(); client.fetch(Plugin.class, pluginName).ifPresent(plugin -> {
final Plugin oldPlugin = JsonUtils.deepCopy(plugin);
if (shouldReconcileStartState(plugin)) {
PluginState currentState = haloPluginManager.startPlugin(pluginName); PluginState currentState = haloPluginManager.startPlugin(pluginName);
handleStatus(plugin, currentState, PluginState.STARTED); handleStatus(plugin, currentState, PluginState.STARTED);
Plugin.PluginStatus status = plugin.statusNonNull(); plugin.statusNonNull().setLastStartTime(Instant.now());
}
String jsBundlePath = BundleResourceUtils.getJsBundlePath(haloPluginManager, pluginName); Plugin.PluginStatus status = plugin.statusNonNull();
String jsBundlePath =
BundleResourceUtils.getJsBundlePath(haloPluginManager, pluginName);
status.setEntry(jsBundlePath); status.setEntry(jsBundlePath);
String cssBundlePath = BundleResourceUtils.getCssBundlePath(haloPluginManager, pluginName); String cssBundlePath =
BundleResourceUtils.getCssBundlePath(haloPluginManager, pluginName);
status.setStylesheet(cssBundlePath); status.setStylesheet(cssBundlePath);
status.setLastStartTime(Instant.now()); if (!plugin.equals(oldPlugin)) {
client.update(plugin);
}
});
} }
private boolean shouldReconcileStopState(Plugin plugin) { private boolean shouldReconcileStopState(Plugin plugin) {
@ -146,12 +136,21 @@ public class PluginReconciler implements Reconciler<Request> {
&& plugin.statusNonNull().getPhase() == PluginState.STARTED; && plugin.statusNonNull().getPhase() == PluginState.STARTED;
} }
private void stopPlugin(Plugin plugin) { private void stopPlugin(String pluginName) {
String pluginName = plugin.getMetadata().getName(); client.fetch(Plugin.class, pluginName).ifPresent(plugin -> {
Plugin oldPlugin = JsonUtils.deepCopy(plugin);
if (shouldReconcileStopState(plugin)) {
PluginState currentState = haloPluginManager.stopPlugin(pluginName); PluginState currentState = haloPluginManager.stopPlugin(pluginName);
handleStatus(plugin, currentState, PluginState.STOPPED); handleStatus(plugin, currentState, PluginState.STOPPED);
} }
if (!plugin.equals(oldPlugin)) {
client.update(plugin);
}
});
}
private void handleStatus(Plugin plugin, PluginState currentState, private void handleStatus(Plugin plugin, PluginState currentState,
PluginState desiredState) { PluginState desiredState) {
Plugin.PluginStatus status = plugin.statusNonNull(); Plugin.PluginStatus status = plugin.statusNonNull();

View File

@ -26,9 +26,11 @@ public class PluginBeforeStopSyncListener {
@EventListener @EventListener
public Mono<Void> onApplicationEvent(@NonNull HaloPluginBeforeStopEvent event) { public Mono<Void> onApplicationEvent(@NonNull HaloPluginBeforeStopEvent event) {
var pluginWrapper = event.getPlugin(); var pluginWrapper = event.getPlugin();
var pluginContext = ExtensionContextRegistry.getInstance() ExtensionContextRegistry registry = ExtensionContextRegistry.getInstance();
.getByPluginId(pluginWrapper.getPluginId()); if (!registry.containsContext(pluginWrapper.getPluginId())) {
return Mono.empty();
}
var pluginContext = registry.getByPluginId(pluginWrapper.getPluginId());
return cleanUpPluginExtensionResources(pluginContext); return cleanUpPluginExtensionResources(pluginContext);
} }

View File

@ -59,7 +59,7 @@ public class PermalinkRefreshHandler implements ApplicationListener<PermalinkRul
String oldPermalink = post.getStatusOrDefault().getPermalink(); String oldPermalink = post.getStatusOrDefault().getPermalink();
String permalink = postPermalinkPolicy.permalink(post); String permalink = postPermalinkPolicy.permalink(post);
post.getStatusOrDefault().setPermalink(permalink); post.getStatusOrDefault().setPermalink(permalink);
if (oldPermalink.equals(permalink)) { if (StringUtils.equals(oldPermalink, permalink)) {
return; return;
} }
// update permalink // update permalink

View File

@ -1,6 +1,7 @@
package run.halo.app.core.extension.reconciler; package run.halo.app.core.extension.reconciler;
import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.assertThatThrownBy;
import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.eq; import static org.mockito.ArgumentMatchers.eq;
import static org.mockito.Mockito.doNothing; import static org.mockito.Mockito.doNothing;
@ -17,6 +18,7 @@ import org.junit.jupiter.api.extension.ExtendWith;
import org.mockito.ArgumentCaptor; import org.mockito.ArgumentCaptor;
import org.mockito.Mock; import org.mockito.Mock;
import org.mockito.junit.jupiter.MockitoExtension; import org.mockito.junit.jupiter.MockitoExtension;
import org.pf4j.PluginRuntimeException;
import org.pf4j.PluginState; import org.pf4j.PluginState;
import org.pf4j.PluginWrapper; import org.pf4j.PluginWrapper;
import run.halo.app.core.extension.Plugin; import run.halo.app.core.extension.Plugin;
@ -64,6 +66,7 @@ class PluginReconcilerTest {
when(pluginWrapper.getPluginState()).thenReturn(PluginState.STOPPED); when(pluginWrapper.getPluginState()).thenReturn(PluginState.STOPPED);
ArgumentCaptor<Plugin> pluginCaptor = doReconcileWithoutRequeue(); ArgumentCaptor<Plugin> pluginCaptor = doReconcileWithoutRequeue();
verify(extensionClient, times(2)).update(any());
Plugin updateArgs = pluginCaptor.getValue(); Plugin updateArgs = pluginCaptor.getValue();
assertThat(updateArgs).isNotNull(); assertThat(updateArgs).isNotNull();
@ -87,6 +90,7 @@ class PluginReconcilerTest {
PluginStartingError.of("apples", "error message", "dev message"); PluginStartingError.of("apples", "error message", "dev message");
when(haloPluginManager.getPluginStartingError(any())).thenReturn(pluginStartingError); when(haloPluginManager.getPluginStartingError(any())).thenReturn(pluginStartingError);
assertThatThrownBy(() -> {
ArgumentCaptor<Plugin> pluginCaptor = doReconcileNeedRequeue(); ArgumentCaptor<Plugin> pluginCaptor = doReconcileNeedRequeue();
// Verify the state before the update plugin // Verify the state before the update plugin
@ -99,6 +103,9 @@ class PluginReconcilerTest {
assertThat(status.getReason()).isEqualTo("error message"); assertThat(status.getReason()).isEqualTo("error message");
assertThat(status.getMessage()).isEqualTo("dev message"); assertThat(status.getMessage()).isEqualTo("dev message");
assertThat(status.getLastStartTime()).isNull(); assertThat(status.getLastStartTime()).isNull();
}).isInstanceOf(PluginRuntimeException.class)
.hasMessage("error message");
} }
@Test @Test
@ -111,6 +118,7 @@ class PluginReconcilerTest {
when(pluginWrapper.getPluginState()).thenReturn(PluginState.STARTED); when(pluginWrapper.getPluginState()).thenReturn(PluginState.STARTED);
ArgumentCaptor<Plugin> pluginCaptor = doReconcileWithoutRequeue(); ArgumentCaptor<Plugin> pluginCaptor = doReconcileWithoutRequeue();
verify(extensionClient, times(2)).update(any());
Plugin updateArgs = pluginCaptor.getValue(); Plugin updateArgs = pluginCaptor.getValue();
assertThat(updateArgs).isNotNull(); assertThat(updateArgs).isNotNull();
@ -145,6 +153,7 @@ class PluginReconcilerTest {
when(pluginWrapper.getPluginState()).thenReturn(PluginState.STARTED); when(pluginWrapper.getPluginState()).thenReturn(PluginState.STARTED);
ArgumentCaptor<Plugin> pluginCaptor = doReconcileWithoutRequeue(); ArgumentCaptor<Plugin> pluginCaptor = doReconcileWithoutRequeue();
verify(extensionClient, times(3)).update(any());
Plugin updateArgs = pluginCaptor.getValue(); Plugin updateArgs = pluginCaptor.getValue();
assertThat(updateArgs).isNotNull(); assertThat(updateArgs).isNotNull();
@ -168,6 +177,7 @@ class PluginReconcilerTest {
PluginStartingError.of("apples", "error message", "dev message"); PluginStartingError.of("apples", "error message", "dev message");
when(haloPluginManager.getPluginStartingError(any())).thenReturn(pluginStartingError); when(haloPluginManager.getPluginStartingError(any())).thenReturn(pluginStartingError);
assertThatThrownBy(() -> {
ArgumentCaptor<Plugin> pluginCaptor = doReconcileNeedRequeue(); ArgumentCaptor<Plugin> pluginCaptor = doReconcileNeedRequeue();
Plugin updateArgs = pluginCaptor.getValue(); Plugin updateArgs = pluginCaptor.getValue();
@ -178,6 +188,8 @@ class PluginReconcilerTest {
assertThat(status.getPhase()).isEqualTo(PluginState.FAILED); assertThat(status.getPhase()).isEqualTo(PluginState.FAILED);
assertThat(status.getReason()).isEqualTo("error message"); assertThat(status.getReason()).isEqualTo("error message");
assertThat(status.getMessage()).isEqualTo("dev message"); assertThat(status.getMessage()).isEqualTo("dev message");
}).isInstanceOf(PluginRuntimeException.class)
.hasMessage("error message");
} }
private ArgumentCaptor<Plugin> doReconcileNeedRequeue() { private ArgumentCaptor<Plugin> doReconcileNeedRequeue() {
@ -201,8 +213,6 @@ class PluginReconcilerTest {
Reconciler.Result result = pluginReconciler.reconcile(new Reconciler.Request("apples")); Reconciler.Result result = pluginReconciler.reconcile(new Reconciler.Request("apples"));
assertThat(result).isNotNull(); assertThat(result).isNotNull();
assertThat(result.reEnqueue()).isEqualTo(false); assertThat(result.reEnqueue()).isEqualTo(false);
verify(extensionClient, times(2)).update(any());
return pluginCaptor; return pluginCaptor;
} }