mirror of https://github.com/halo-dev/halo
refactor: remove finalizer when resources deleted completely (#3065)
#### What type of PR is this? /kind improvement /area core #### What this PR does / why we need it: 主题卸载时等待删除关联资源后再清除 Finalizer see #2967 for more detail. https://github.com/halo-dev/halo/issues/2967#issuecomment-1354384978 #### Which issue(s) this PR fixes: how to test it? - 在主题中多添加一个 AnnotationSetting 资源 yaml,测试主题删除是否正常。 - 对包 `org.springframework.retry` 开启 debug 日志后能在删除主题时看到类似如下日志: ``` 16:33:02.822 [Test worker] DEBUG org.springframework.retry.support.RetryTemplate - Retry: count=0 16:33:03.128 [Test worker] DEBUG org.springframework.retry.support.RetryTemplate - Checking for rethrow: count=1 ``` Fixes #2967 #### Special notes for your reviewer: /cc @halo-dev/sig-halo #### Does this PR introduce a user-facing change? ```release-note 主题卸载时等待删除关联资源后再清除 Finalizer ```pull/3076/head^2
parent
b667e988df
commit
9d0ad5de26
|
@ -8,6 +8,7 @@ import java.util.Map;
|
|||
import java.util.Set;
|
||||
import java.util.UUID;
|
||||
import org.apache.commons.lang3.StringUtils;
|
||||
import org.springframework.retry.support.RetryTemplate;
|
||||
import org.springframework.stereotype.Component;
|
||||
import org.springframework.util.CollectionUtils;
|
||||
import org.springframework.util.FileSystemUtils;
|
||||
|
@ -41,6 +42,12 @@ public class ThemeReconciler implements Reconciler<Request> {
|
|||
private final ExtensionClient client;
|
||||
private final ThemePathPolicy themePathPolicy;
|
||||
|
||||
private final RetryTemplate retryTemplate = RetryTemplate.builder()
|
||||
.maxAttempts(20)
|
||||
.fixedBackoff(300)
|
||||
.retryOn(IllegalStateException.class)
|
||||
.build();
|
||||
|
||||
public ThemeReconciler(ExtensionClient client, HaloProperties haloProperties) {
|
||||
this.client = client;
|
||||
themePathPolicy = new ThemePathPolicy(haloProperties.getWorkDir());
|
||||
|
@ -157,20 +164,39 @@ public class ThemeReconciler implements Reconciler<Request> {
|
|||
if (StringUtils.isNotBlank(settingName)) {
|
||||
client.fetch(Setting.class, settingName)
|
||||
.ifPresent(client::delete);
|
||||
retryTemplate.execute(callback -> {
|
||||
client.fetch(Setting.class, settingName).ifPresent(setting -> {
|
||||
throw new IllegalStateException("Waiting for setting to be deleted.");
|
||||
});
|
||||
return null;
|
||||
});
|
||||
}
|
||||
// delete annotation setting
|
||||
deleteAnnotationSettings(theme.getMetadata().getName());
|
||||
}
|
||||
|
||||
private void deleteAnnotationSettings(String themeName) {
|
||||
List<AnnotationSetting> result = client.list(AnnotationSetting.class, annotationSetting -> {
|
||||
Map<String, String> labels = ExtensionUtil.nullSafeLabels(annotationSetting);
|
||||
return themeName.equals(labels.get(Theme.THEME_NAME_LABEL));
|
||||
}, null);
|
||||
List<AnnotationSetting> result = listAnnotationSettingsByThemeName(themeName);
|
||||
|
||||
for (AnnotationSetting annotationSetting : result) {
|
||||
client.delete(annotationSetting);
|
||||
}
|
||||
|
||||
retryTemplate.execute(callback -> {
|
||||
List<AnnotationSetting> annotationSettings =
|
||||
listAnnotationSettingsByThemeName(themeName);
|
||||
if (annotationSettings.isEmpty()) {
|
||||
return null;
|
||||
}
|
||||
throw new IllegalStateException("Waiting for annotation settings to be deleted.");
|
||||
});
|
||||
}
|
||||
|
||||
private List<AnnotationSetting> listAnnotationSettingsByThemeName(String themeName) {
|
||||
return client.list(AnnotationSetting.class, annotationSetting -> {
|
||||
Map<String, String> labels = ExtensionUtil.nullSafeLabels(annotationSetting);
|
||||
return themeName.equals(labels.get(Theme.THEME_NAME_LABEL));
|
||||
}, null);
|
||||
}
|
||||
|
||||
private void deleteThemeFiles(Theme theme) {
|
||||
|
|
|
@ -1,6 +1,7 @@
|
|||
package run.halo.app.core.extension.reconciler;
|
||||
|
||||
import static org.assertj.core.api.Assertions.assertThat;
|
||||
import static org.assertj.core.api.AssertionsForClassTypes.assertThatThrownBy;
|
||||
import static org.mockito.ArgumentMatchers.any;
|
||||
import static org.mockito.ArgumentMatchers.eq;
|
||||
import static org.mockito.Mockito.times;
|
||||
|
@ -12,6 +13,7 @@ import java.io.IOException;
|
|||
import java.nio.file.Files;
|
||||
import java.nio.file.Path;
|
||||
import java.time.Instant;
|
||||
import java.util.List;
|
||||
import java.util.Map;
|
||||
import java.util.Optional;
|
||||
import org.json.JSONException;
|
||||
|
@ -23,7 +25,9 @@ import org.mockito.ArgumentCaptor;
|
|||
import org.mockito.Mock;
|
||||
import org.mockito.Mockito;
|
||||
import org.mockito.junit.jupiter.MockitoExtension;
|
||||
import org.mockito.stubbing.Answer;
|
||||
import org.skyscreamer.jsonassert.JSONAssert;
|
||||
import org.springframework.retry.RetryException;
|
||||
import org.springframework.util.FileSystemUtils;
|
||||
import org.springframework.util.ResourceUtils;
|
||||
import run.halo.app.core.extension.AnnotationSetting;
|
||||
|
@ -32,6 +36,7 @@ import run.halo.app.core.extension.Theme;
|
|||
import run.halo.app.extension.ConfigMap;
|
||||
import run.halo.app.extension.ExtensionClient;
|
||||
import run.halo.app.extension.Metadata;
|
||||
import run.halo.app.extension.MetadataOperator;
|
||||
import run.halo.app.extension.controller.Reconciler;
|
||||
import run.halo.app.infra.properties.HaloProperties;
|
||||
import run.halo.app.infra.utils.JsonUtils;
|
||||
|
@ -105,14 +110,100 @@ class ThemeReconcilerTest {
|
|||
themeReconciler.reconcile(new Reconciler.Request(metadata.getName()));
|
||||
|
||||
verify(extensionClient, times(2)).fetch(eq(Theme.class), eq(metadata.getName()));
|
||||
verify(extensionClient, times(1)).fetch(eq(Setting.class), eq(themeSpec.getSettingName()));
|
||||
verify(extensionClient, times(2)).fetch(eq(Setting.class), eq(themeSpec.getSettingName()));
|
||||
|
||||
verify(extensionClient, times(1)).list(eq(AnnotationSetting.class), any(), any());
|
||||
verify(extensionClient, times(2)).list(eq(AnnotationSetting.class), any(), any());
|
||||
|
||||
assertThat(Files.exists(testWorkDir)).isTrue();
|
||||
assertThat(Files.exists(defaultThemePath)).isFalse();
|
||||
}
|
||||
|
||||
@Test
|
||||
void reconcileDeleteRetry() {
|
||||
Theme theme = fakeTheme();
|
||||
final MetadataOperator metadata = theme.getMetadata();
|
||||
|
||||
Path testWorkDir = tempDirectory.resolve("reconcile-delete");
|
||||
when(haloProperties.getWorkDir()).thenReturn(testWorkDir);
|
||||
|
||||
final ThemeReconciler themeReconciler =
|
||||
new ThemeReconciler(extensionClient, haloProperties);
|
||||
|
||||
final int[] retryFlags = {0, 0};
|
||||
when(extensionClient.fetch(eq(Setting.class), eq("theme-test-setting")))
|
||||
.thenAnswer((Answer<Optional<Setting>>) invocation -> {
|
||||
retryFlags[0]++;
|
||||
// retry 2 times
|
||||
if (retryFlags[0] < 3) {
|
||||
return Optional.of(new Setting());
|
||||
}
|
||||
return Optional.empty();
|
||||
});
|
||||
|
||||
when(extensionClient.list(eq(AnnotationSetting.class), any(), eq(null)))
|
||||
.thenAnswer((Answer<List<AnnotationSetting>>) invocation -> {
|
||||
retryFlags[1]++;
|
||||
// retry 2 times
|
||||
if (retryFlags[1] < 3) {
|
||||
return List.of(new AnnotationSetting());
|
||||
}
|
||||
return List.of();
|
||||
});
|
||||
|
||||
themeReconciler.reconcile(new Reconciler.Request(metadata.getName()));
|
||||
|
||||
String settingName = theme.getSpec().getSettingName();
|
||||
verify(extensionClient, times(2)).fetch(eq(Theme.class), eq(metadata.getName()));
|
||||
verify(extensionClient, times(3)).fetch(eq(Setting.class), eq(settingName));
|
||||
verify(extensionClient, times(3)).list(eq(AnnotationSetting.class), any(), eq(null));
|
||||
}
|
||||
|
||||
@Test
|
||||
void reconcileDeleteRetryWhenThrowException() {
|
||||
Theme theme = fakeTheme();
|
||||
|
||||
Path testWorkDir = tempDirectory.resolve("reconcile-delete");
|
||||
when(haloProperties.getWorkDir()).thenReturn(testWorkDir);
|
||||
|
||||
final ThemeReconciler themeReconciler =
|
||||
new ThemeReconciler(extensionClient, haloProperties);
|
||||
|
||||
final int[] retryFlags = {0};
|
||||
when(extensionClient.fetch(eq(Setting.class), eq("theme-test-setting")))
|
||||
.thenAnswer((Answer<Optional<Setting>>) invocation -> {
|
||||
retryFlags[0]++;
|
||||
// retry 2 times
|
||||
if (retryFlags[0] < 2) {
|
||||
return Optional.of(new Setting());
|
||||
}
|
||||
throw new RetryException("retry exception.");
|
||||
});
|
||||
|
||||
String settingName = theme.getSpec().getSettingName();
|
||||
assertThatThrownBy(
|
||||
() -> themeReconciler.reconcile(new Reconciler.Request(theme.getMetadata().getName())))
|
||||
.isInstanceOf(RetryException.class)
|
||||
.hasMessage("retry exception.");
|
||||
|
||||
verify(extensionClient, times(2)).fetch(eq(Setting.class), eq(settingName));
|
||||
}
|
||||
|
||||
private Theme fakeTheme() {
|
||||
Theme theme = new Theme();
|
||||
Metadata metadata = new Metadata();
|
||||
metadata.setName("theme-test");
|
||||
metadata.setDeletionTimestamp(Instant.now());
|
||||
theme.setMetadata(metadata);
|
||||
theme.setKind(Theme.KIND);
|
||||
theme.setApiVersion("theme.halo.run/v1alpha1");
|
||||
Theme.ThemeSpec themeSpec = new Theme.ThemeSpec();
|
||||
themeSpec.setSettingName("theme-test-setting");
|
||||
theme.setSpec(themeSpec);
|
||||
when(extensionClient.fetch(eq(Theme.class), eq(metadata.getName())))
|
||||
.thenReturn(Optional.of(theme));
|
||||
return theme;
|
||||
}
|
||||
|
||||
@Test
|
||||
void themeSettingDefaultValue() throws IOException, JSONException {
|
||||
Path testWorkDir = tempDirectory.resolve("reconcile-setting-value");
|
||||
|
@ -169,10 +260,10 @@ class ThemeReconcilerTest {
|
|||
ConfigMap defaultValueConfigMap = configMapCaptor.getValue();
|
||||
Map<String, String> data = defaultValueConfigMap.getData();
|
||||
JSONAssert.assertEquals("""
|
||||
{
|
||||
"sns": "{\\"email\\":\\"example@exmple.com\\"}"
|
||||
}
|
||||
""",
|
||||
{
|
||||
"sns": "{\\"email\\":\\"example@exmple.com\\"}"
|
||||
}
|
||||
""",
|
||||
JsonUtils.objectToJson(data),
|
||||
true);
|
||||
}
|
||||
|
|
Loading…
Reference in New Issue