diff --git a/application/src/main/java/run/halo/app/core/attachment/reconciler/AttachmentReconciler.java b/application/src/main/java/run/halo/app/core/attachment/reconciler/AttachmentReconciler.java index 163627f968..3b71dee300 100644 --- a/application/src/main/java/run/halo/app/core/attachment/reconciler/AttachmentReconciler.java +++ b/application/src/main/java/run/halo/app/core/attachment/reconciler/AttachmentReconciler.java @@ -11,6 +11,7 @@ import lombok.extern.slf4j.Slf4j; import org.springframework.context.ApplicationEventPublisher; import org.springframework.stereotype.Component; +import reactor.core.publisher.Mono; import run.halo.app.core.attachment.AttachmentChangedEvent; import run.halo.app.core.extension.attachment.Attachment; import run.halo.app.core.extension.attachment.Attachment.AttachmentStatus; @@ -23,6 +24,7 @@ import run.halo.app.extension.controller.Reconciler; import run.halo.app.extension.controller.Reconciler.Request; import run.halo.app.extension.controller.RequeueException; +import run.halo.app.extension.exception.ExtensionNotFoundException; @Slf4j @Component @@ -84,6 +86,18 @@ public Controller setupWith(ControllerBuilder builder) { } void cleanUpResources(Attachment attachment) { - attachmentService.delete(attachment).block(Duration.ofSeconds(20)); + attachmentService + .delete(attachment) + // A missing policy or config map is permanent: no handler can resolve the + // attachment any more, so there is nothing left to clean up and requeuing + // would only spin forever. Give up on the resources and let the finalizer go. + .onErrorResume(ExtensionNotFoundException.class, e -> { + log.warn( + "Skipped cleaning up resources of attachment {}: {} Removing the attachment record anyway.", + attachment.getMetadata().getName(), + e.getMessage()); + return Mono.empty(); + }) + .block(Duration.ofSeconds(20)); } } diff --git a/application/src/test/java/run/halo/app/core/attachment/reconciler/AttachmentReconcilerTest.java b/application/src/test/java/run/halo/app/core/attachment/reconciler/AttachmentReconcilerTest.java new file mode 100644 index 0000000000..e0ebc41b76 --- /dev/null +++ b/application/src/test/java/run/halo/app/core/attachment/reconciler/AttachmentReconcilerTest.java @@ -0,0 +1,101 @@ +package run.halo.app.core.attachment.reconciler; + +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.Mockito.never; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +import java.time.Instant; +import java.util.Optional; +import java.util.Set; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; +import org.mockito.InjectMocks; +import org.mockito.Mock; +import org.mockito.junit.jupiter.MockitoExtension; +import org.springframework.context.ApplicationEventPublisher; +import reactor.core.publisher.Mono; +import run.halo.app.core.attachment.AttachmentChangedEvent; +import run.halo.app.core.extension.attachment.Attachment; +import run.halo.app.core.extension.attachment.Constant; +import run.halo.app.core.extension.attachment.Policy; +import run.halo.app.core.extension.service.AttachmentService; +import run.halo.app.extension.ExtensionClient; +import run.halo.app.extension.GroupVersionKind; +import run.halo.app.extension.Metadata; +import run.halo.app.extension.controller.Reconciler.Request; +import run.halo.app.extension.exception.ExtensionNotFoundException; + +/** Tests for {@link AttachmentReconciler}. */ +@ExtendWith(MockitoExtension.class) +class AttachmentReconcilerTest { + + @Mock + private ExtensionClient client; + + @Mock + private AttachmentService attachmentService; + + @Mock + private ApplicationEventPublisher eventPublisher; + + @InjectMocks + private AttachmentReconciler reconciler; + + @Test + void shouldRemoveFinalizerWhenPolicyWasDeleted() { + var attachment = deletedAttachment(); + when(client.fetch(Attachment.class, "fake-attachment")).thenReturn(Optional.of(attachment)); + when(attachmentService.delete(attachment)) + .thenReturn(Mono.error(new ExtensionNotFoundException( + GroupVersionKind.fromExtension(Policy.class), "deleted-policy"))); + + reconciler.reconcile(new Request("fake-attachment")); + + assertThat(attachment.getMetadata().getFinalizers()).doesNotContain(Constant.FINALIZER_NAME); + verify(client).update(attachment); + verify(eventPublisher).publishEvent(any(AttachmentChangedEvent.class)); + } + + @Test + void shouldRemoveFinalizerAfterResourcesAreCleanedUp() { + var attachment = deletedAttachment(); + when(client.fetch(Attachment.class, "fake-attachment")).thenReturn(Optional.of(attachment)); + when(attachmentService.delete(attachment)).thenReturn(Mono.just(attachment)); + + reconciler.reconcile(new Request("fake-attachment")); + + assertThat(attachment.getMetadata().getFinalizers()).doesNotContain(Constant.FINALIZER_NAME); + verify(client).update(attachment); + verify(eventPublisher).publishEvent(any(AttachmentChangedEvent.class)); + } + + @Test + void shouldKeepFinalizerWhenCleaningUpResourcesFailsForOtherReasons() { + var attachment = deletedAttachment(); + when(client.fetch(Attachment.class, "fake-attachment")).thenReturn(Optional.of(attachment)); + when(attachmentService.delete(attachment)).thenReturn(Mono.error(new IllegalStateException("boom"))); + + assertThatThrownBy(() -> reconciler.reconcile(new Request("fake-attachment"))) + .isInstanceOf(IllegalStateException.class) + .hasMessage("boom"); + + verify(client, never()).update(any(Attachment.class)); + verify(eventPublisher, never()).publishEvent(any(AttachmentChangedEvent.class)); + } + + private static Attachment deletedAttachment() { + var attachment = new Attachment(); + var metadata = new Metadata(); + metadata.setName("fake-attachment"); + metadata.setDeletionTimestamp(Instant.now()); + metadata.setFinalizers(Set.of(Constant.FINALIZER_NAME)); + attachment.setMetadata(metadata); + var spec = new Attachment.AttachmentSpec(); + spec.setPolicyName("deleted-policy"); + attachment.setSpec(spec); + return attachment; + } +}