diff --git a/spring-boot-admin-server-ui/src/main/frontend/store.spec.ts b/spring-boot-admin-server-ui/src/main/frontend/store.spec.ts index a51f389f46a..bcc27dbe624 100644 --- a/spring-boot-admin-server-ui/src/main/frontend/store.spec.ts +++ b/spring-boot-admin-server-ui/src/main/frontend/store.spec.ts @@ -95,6 +95,97 @@ describe('store', () => { expect(removedListener).not.toHaveBeenCalled(); }); + it('handles instance rename: migrates instance to new application and removes the empty old one', async () => { + // The backend SSE stream re-publishes the new application (with the migrated + // instance) and then the previous application with an empty instance list so + // the store can drop it. The instance's registration.name changes while the + // id/healthUrl stay the same. + const instance = { ...registerWithOneInstance.instances[0] }; + instance.registration = { + ...instance.registration, + name: 'new-service', + }; + const newService = { ...registerWithOneInstance, name: 'new-service', instances: [instance] }; + const oldServiceEmpty = { ...registerWithOneInstance, name: 'old-service', instances: [] }; + + // Seed the store with the application under its previous name. + const oldService = { ...registerWithOneInstance, name: 'old-service' }; + mockSubject.next({ data: oldService }); + + await waitFor(() => { + expect(applicationStore.applications).toHaveLength(1); + expect(applicationStore.applications[0].name).toBe('old-service'); + }); + + // Simulate the rename update sequence emitted by the backend: new-service + // gains the instance, old-service ends up empty and must be removed. + mockSubject.next({ data: newService }); + mockSubject.next({ data: oldServiceEmpty }); + + await waitFor(() => { + expect(applicationStore.applications).toHaveLength(1); + const app = applicationStore.applications[0]; + expect(app.name).toBe('new-service'); + expect(app.instances).toHaveLength(1); + expect(app.instances[0].id).toBe(instance.id); + }); + + expect(removedListener).toHaveBeenCalled(); + const removedName = removedListener.mock.calls[removedListener.mock.calls.length - 1][0].name; + expect(removedName).toBe('old-service'); + }); + + it('handles rename when old application still has other instances: updates both groups, removes none', async () => { + const migratedInstance = { + ...registerWithOneInstance.instances[0], + id: 'instance-a', + registration: { + ...registerWithOneInstance.instances[0].registration, + name: 'new-service', + healthUrl: 'http://localhost:8080/actuator/health', + }, + }; + const remainingInstance = { + ...registerWithOneInstance.instances[0], + id: 'instance-b', + registration: { + ...registerWithOneInstance.instances[0].registration, + name: 'old-service', + healthUrl: 'http://localhost:8081/actuator/health', + }, + }; + + const oldServiceWithTwo = { + ...registerWithOneInstance, + name: 'old-service', + instances: [ + { ...migratedInstance, registration: { ...migratedInstance.registration, name: 'old-service' } }, + remainingInstance, + ], + }; + const newService = { ...registerWithOneInstance, name: 'new-service', instances: [migratedInstance] }; + const oldServiceWithOne = { ...registerWithOneInstance, name: 'old-service', instances: [remainingInstance] }; + + mockSubject.next({ data: oldServiceWithTwo }); + await waitFor(() => { + expect(applicationStore.applications).toHaveLength(1); + expect(applicationStore.applications[0].instances).toHaveLength(2); + }); + + mockSubject.next({ data: newService }); + mockSubject.next({ data: oldServiceWithOne }); + + await waitFor(() => { + const names = applicationStore.applications.map((a) => a.name).sort(); + expect(names).toEqual(['new-service', 'old-service']); + const oldApp = applicationStore.applications.find((a) => a.name === 'old-service'); + expect(oldApp.instances).toHaveLength(1); + expect(oldApp.instances[0].id).toBe('instance-b'); + }); + + expect(removedListener).not.toHaveBeenCalled(); + }); + it('removes an application', async () => { mockSubject.next({ data: registerWithOneInstance }); diff --git a/spring-boot-admin-server/src/main/java/de/codecentric/boot/admin/server/domain/entities/Instance.java b/spring-boot-admin-server/src/main/java/de/codecentric/boot/admin/server/domain/entities/Instance.java index e315c250a48..d68d05b52a8 100644 --- a/spring-boot-admin-server/src/main/java/de/codecentric/boot/admin/server/domain/entities/Instance.java +++ b/spring-boot-admin-server/src/main/java/de/codecentric/boot/admin/server/domain/entities/Instance.java @@ -123,7 +123,9 @@ public Instance register(Registration registration) { } if (!Objects.equals(this.registration, registration)) { - return this.apply(new InstanceRegistrationUpdatedEvent(this.id, this.nextVersion(), registration), true); + return this.apply( + new InstanceRegistrationUpdatedEvent(this.id, this.nextVersion(), registration, this.registration), + true); } return this; diff --git a/spring-boot-admin-server/src/main/java/de/codecentric/boot/admin/server/domain/events/InstanceRegistrationUpdatedEvent.java b/spring-boot-admin-server/src/main/java/de/codecentric/boot/admin/server/domain/events/InstanceRegistrationUpdatedEvent.java index 88be2256487..b7f912d8b70 100644 --- a/spring-boot-admin-server/src/main/java/de/codecentric/boot/admin/server/domain/events/InstanceRegistrationUpdatedEvent.java +++ b/spring-boot-admin-server/src/main/java/de/codecentric/boot/admin/server/domain/events/InstanceRegistrationUpdatedEvent.java @@ -19,11 +19,18 @@ import java.io.Serial; import java.time.Instant; +import org.jspecify.annotations.Nullable; + import de.codecentric.boot.admin.server.domain.values.InstanceId; import de.codecentric.boot.admin.server.domain.values.Registration; /** * This event gets emitted when an instance updates it's registration. + *

+ * The optional {@link #getPrevious() previous} registration holds the registration as it + * was before the update. This allows listeners to detect an application rename + * (i.e. when the instance id stays the same but {@link Registration#getName()} changes). + * When no previous registration is known the field is {@code null}. * * @author Johannes Edmeier */ @@ -39,14 +46,27 @@ public class InstanceRegistrationUpdatedEvent extends InstanceEvent { Registration registration; + @Nullable Registration previous; + public InstanceRegistrationUpdatedEvent(InstanceId instance, long version, Registration registration) { - this(instance, version, Instant.now(), registration); + this(instance, version, Instant.now(), registration, null); } public InstanceRegistrationUpdatedEvent(InstanceId instance, long version, Instant timestamp, Registration registration) { + this(instance, version, timestamp, registration, null); + } + + public InstanceRegistrationUpdatedEvent(InstanceId instance, long version, Registration registration, + @Nullable Registration previous) { + this(instance, version, Instant.now(), registration, previous); + } + + public InstanceRegistrationUpdatedEvent(InstanceId instance, long version, Instant timestamp, + Registration registration, @Nullable Registration previous) { super(instance, version, TYPE, timestamp); this.registration = registration; + this.previous = previous; } -} +} \ No newline at end of file diff --git a/spring-boot-admin-server/src/main/java/de/codecentric/boot/admin/server/services/ApplicationRegistry.java b/spring-boot-admin-server/src/main/java/de/codecentric/boot/admin/server/services/ApplicationRegistry.java index b95946278a8..f6d134f2fd6 100644 --- a/spring-boot-admin-server/src/main/java/de/codecentric/boot/admin/server/services/ApplicationRegistry.java +++ b/spring-boot-admin-server/src/main/java/de/codecentric/boot/admin/server/services/ApplicationRegistry.java @@ -29,8 +29,11 @@ import de.codecentric.boot.admin.server.domain.entities.Application; import de.codecentric.boot.admin.server.domain.entities.Instance; +import de.codecentric.boot.admin.server.domain.events.InstanceEvent; +import de.codecentric.boot.admin.server.domain.events.InstanceRegistrationUpdatedEvent; import de.codecentric.boot.admin.server.domain.values.BuildVersion; import de.codecentric.boot.admin.server.domain.values.InstanceId; +import de.codecentric.boot.admin.server.domain.values.Registration; import de.codecentric.boot.admin.server.domain.values.StatusInfo; import de.codecentric.boot.admin.server.eventstore.InstanceEventPublisher; @@ -79,11 +82,38 @@ public Mono getApplication(String name) { public Flux getApplicationStream() { return Flux.from(this.instanceEventPublisher) - .flatMap((event) -> this.instanceRegistry.getInstance(event.getInstance())) - .map(this::getApplicationForInstance) + .flatMap(this::resolveAffectedApplicationGroups) .flatMap((group) -> toApplication(group.getT1(), group.getT2())); } + /** + * Resolves the application groups that are affected by the given event. + *

+ * For most events this is only the application the instance currently belongs to. + * When an {@link InstanceRegistrationUpdatedEvent} indicates that an instance was + * renamed (i.e. the previous registration carries a different name) the previously + * associated application is included as well so that it gets re-aggregated. If no + * instances remain under the previous name the resulting {@link Application} will + * have an empty instance list which is the signal for clients to remove it. + * @param event the event that was published + * @return a flux of name / instances tuples that need to be (re-)published + */ + protected Flux>> resolveAffectedApplicationGroups(InstanceEvent event) { + return this.instanceRegistry.getInstance(event.getInstance()).flatMapMany((instance) -> { + Flux>> groups = Flux.just(getApplicationForInstance(instance)); + + if (event instanceof InstanceRegistrationUpdatedEvent updatedEvent) { + Registration previous = updatedEvent.getPrevious(); + String currentName = instance.getRegistration().getName(); + if (previous != null && !Objects.equals(previous.getName(), currentName)) { + groups = Flux.merge(groups, Flux.just(getApplicationForName(previous.getName()))); + } + } + + return groups; + }); + } + public Flux deregister(String name) { return this.instanceRegistry.getInstances(name) .flatMap((instance) -> this.instanceRegistry.deregister(instance.getId())); @@ -91,6 +121,10 @@ public Flux deregister(String name) { protected Tuple2> getApplicationForInstance(Instance instance) { String name = instance.getRegistration().getName(); + return getApplicationForName(name); + } + + protected Tuple2> getApplicationForName(String name) { return Tuples.of(name, this.instanceRegistry.getInstances(name).filter(Instance::isRegistered)); } diff --git a/spring-boot-admin-server/src/main/java/de/codecentric/boot/admin/server/utils/jackson/InstanceRegistrationUpdatedEventMixin.java b/spring-boot-admin-server/src/main/java/de/codecentric/boot/admin/server/utils/jackson/InstanceRegistrationUpdatedEventMixin.java index 67c36db3674..64e663a58f1 100644 --- a/spring-boot-admin-server/src/main/java/de/codecentric/boot/admin/server/utils/jackson/InstanceRegistrationUpdatedEventMixin.java +++ b/spring-boot-admin-server/src/main/java/de/codecentric/boot/admin/server/utils/jackson/InstanceRegistrationUpdatedEventMixin.java @@ -20,6 +20,7 @@ import com.fasterxml.jackson.annotation.JsonCreator; import com.fasterxml.jackson.annotation.JsonProperty; +import org.jspecify.annotations.Nullable; import de.codecentric.boot.admin.server.domain.events.InstanceRegistrationUpdatedEvent; import de.codecentric.boot.admin.server.domain.values.InstanceId; @@ -36,7 +37,8 @@ public abstract class InstanceRegistrationUpdatedEventMixin { @JsonCreator public InstanceRegistrationUpdatedEventMixin(@JsonProperty("instance") InstanceId instance, @JsonProperty("version") long version, @JsonProperty("timestamp") Instant timestamp, - @JsonProperty("registration") Registration registration) { + @JsonProperty("registration") Registration registration, + @JsonProperty("previous") @Nullable Registration previous) { } -} +} \ No newline at end of file diff --git a/spring-boot-admin-server/src/test/java/de/codecentric/boot/admin/server/services/ApplicationRegistryTest.java b/spring-boot-admin-server/src/test/java/de/codecentric/boot/admin/server/services/ApplicationRegistryTest.java index 02a7f418f67..1323a2cd24c 100644 --- a/spring-boot-admin-server/src/test/java/de/codecentric/boot/admin/server/services/ApplicationRegistryTest.java +++ b/spring-boot-admin-server/src/test/java/de/codecentric/boot/admin/server/services/ApplicationRegistryTest.java @@ -16,7 +16,9 @@ package de.codecentric.boot.admin.server.services; +import java.time.Duration; import java.util.ArrayList; +import java.util.List; import java.util.Arrays; import java.util.Collections; @@ -30,6 +32,8 @@ import de.codecentric.boot.admin.server.domain.entities.Application; import de.codecentric.boot.admin.server.domain.entities.Instance; +import de.codecentric.boot.admin.server.domain.events.InstanceEvent; +import de.codecentric.boot.admin.server.domain.events.InstanceRegistrationUpdatedEvent; import de.codecentric.boot.admin.server.domain.values.BuildVersion; import de.codecentric.boot.admin.server.domain.values.InstanceId; import de.codecentric.boot.admin.server.domain.values.Registration; @@ -48,11 +52,13 @@ class ApplicationRegistryTest { private ApplicationRegistry applicationRegistry; + private TestInstanceEventPublisher eventPublisher; + @BeforeEach void setUp() { this.instanceRegistry = mock(InstanceRegistry.class); - InstanceEventPublisher instanceEventPublisher = mock(InstanceEventPublisher.class); - this.applicationRegistry = new ApplicationRegistry(this.instanceRegistry, instanceEventPublisher); + this.eventPublisher = new TestInstanceEventPublisher(); + this.applicationRegistry = new ApplicationRegistry(this.instanceRegistry, this.eventPublisher); } @Test @@ -172,6 +178,124 @@ void getStatus(String instance1Status, String instance2Status, String expectedAp .verifyComplete(); } + @Test + void getApplicationStream_emitsNewApplicationAndRemovesOldOneWhenInstanceRenamed() { + // The instance keeps a stable id (based on healthUrl) but changes its name from + // old to new. + Registration oldRegistration = Registration.create("old-service", "http://localhost:8080/health").build(); + Registration newRegistration = Registration.create("new-service", "http://localhost:8080/health").build(); + InstanceId id = InstanceId.of("renamed-instance"); + Instance renamedInstance = Instance.create(id).register(oldRegistration).register(newRegistration); + + InstanceRegistrationUpdatedEvent event = new InstanceRegistrationUpdatedEvent(id, renamedInstance.getVersion(), + newRegistration, oldRegistration); + + when(this.instanceRegistry.getInstance(id)).thenReturn(Mono.just(renamedInstance)); + when(this.instanceRegistry.getInstances("new-service")) + .thenReturn(Flux.just(renamedInstance).filter(Instance::isRegistered)); + when(this.instanceRegistry.getInstances("old-service")).thenReturn(Flux.empty()); + + List applications = Flux.from(this.applicationRegistry.getApplicationStream()) + .doOnSubscribe((sub) -> this.eventPublisher.emit(event)) + .filter((application) -> application.getName().equals("new-service") + || application.getName().equals("old-service")) + .take(2) + .collectList() + .block(Duration.ofSeconds(5)); + + assertThat(applications).extracting(Application::getName) + .containsExactlyInAnyOrder("new-service", "old-service"); + Application renamed = applications.stream() + .filter((a) -> a.getName().equals("new-service")) + .findFirst() + .orElseThrow(); + assertThat(renamed.getInstances()).extracting(Instance::getId).containsExactly(id); + + Application previous = applications.stream() + .filter((a) -> a.getName().equals("old-service")) + .findFirst() + .orElseThrow(); + assertThat(previous.getInstances()).isEmpty(); + } + + @Test + void getApplicationStream_recomputesOldApplicationWhenSiblingInstanceRemainsAfterRename() { + // old-service still has instance-2; instance-1 moves to new-service. + Registration oldRegistration1 = Registration.create("old-service", "http://localhost:8080/health").build(); + Registration newRegistration1 = Registration.create("new-service", "http://localhost:8080/health").build(); + InstanceId id1 = InstanceId.of("instance-1"); + Instance renamedInstance = Instance.create(id1).register(oldRegistration1).register(newRegistration1); + + Registration oldRegistration2 = Registration.create("old-service", "http://localhost:8081/health").build(); + Instance remainingInstance = Instance.create(InstanceId.of("instance-2")).register(oldRegistration2); + + InstanceRegistrationUpdatedEvent event = new InstanceRegistrationUpdatedEvent(id1, renamedInstance.getVersion(), + newRegistration1, oldRegistration1); + + when(this.instanceRegistry.getInstance(id1)).thenReturn(Mono.just(renamedInstance)); + when(this.instanceRegistry.getInstances("new-service")) + .thenReturn(Flux.just(renamedInstance).filter(Instance::isRegistered)); + when(this.instanceRegistry.getInstances("old-service")) + .thenReturn(Flux.just(remainingInstance).filter(Instance::isRegistered)); + + List applications = Flux.from(this.applicationRegistry.getApplicationStream()) + .doOnSubscribe((sub) -> this.eventPublisher.emit(event)) + .filter((application) -> application.getName().equals("new-service") + || application.getName().equals("old-service")) + .take(2) + .collectList() + .block(Duration.ofSeconds(5)); + + assertThat(applications).extracting(Application::getName) + .containsExactlyInAnyOrder("new-service", "old-service"); + Application renamed = applications.stream() + .filter((a) -> a.getName().equals("new-service")) + .findFirst() + .orElseThrow(); + assertThat(renamed.getInstances()).extracting(Instance::getId).containsExactly(id1); + + Application previous = applications.stream() + .filter((a) -> a.getName().equals("old-service")) + .findFirst() + .orElseThrow(); + assertThat(previous.getInstances()).extracting(Instance::getId).containsExactly(remainingInstance.getId()); + } + + @Test + void getApplicationStream_doesNotEmitExtraApplicationWhenOnlyNonNameFieldsChange() { + // Same name, only the managementUrl differs - the event should result in a single + // update for the same application and not a spurious delete/empty event. + Registration oldRegistration = Registration.create("service", "http://localhost:8080/health") + .managementUrl("http://localhost:8080/actuator") + .build(); + Registration newRegistration = Registration.create("service", "http://localhost:8080/health") + .managementUrl("http://localhost:9090/actuator") + .build(); + InstanceId id = InstanceId.of("stable-instance"); + Instance updatedInstance = Instance.create(id).register(oldRegistration).register(newRegistration); + + InstanceRegistrationUpdatedEvent event = new InstanceRegistrationUpdatedEvent(id, updatedInstance.getVersion(), + newRegistration, oldRegistration); + + when(this.instanceRegistry.getInstance(id)).thenReturn(Mono.just(updatedInstance)); + when(this.instanceRegistry.getInstances("service")) + .thenReturn(Flux.just(updatedInstance).filter(Instance::isRegistered)); + + // Emit the event right after the stream is subscribed; expect exactly one + // application named "service" to be re-published (no empty/delete event). + List applications = Flux.from(this.applicationRegistry.getApplicationStream()) + .doOnSubscribe((sub) -> this.eventPublisher.emit(event)) + .filter((application) -> application.getName().equals("service")) + .take(1) + .collectList() + .block(Duration.ofSeconds(5)); + + assertThat(applications).hasSize(1); + Application application = applications.iterator().next(); + assertThat(application.getName()).isEqualTo("service"); + assertThat(application.getInstances()).extracting(Instance::getId).containsExactly(id); + } + private Instance getInstance(String applicationName, String version) { Registration registration = Registration.create(applicationName, "http://localhost:8080/health") .metadata("version", version) @@ -184,4 +308,26 @@ private Instance getInstance(String applicationName) { return getInstance(applicationName, "FooBarVersion"); } + /** + * Test helper that exposes the protected publish method so tests can push events. + */ + static class TestInstanceEventPublisher extends InstanceEventPublisher { + + private final reactor.core.publisher.Sinks.Many sink = reactor.core.publisher.Sinks.many() + .multicast() + .onBackpressureBuffer(); + + @Override + public void subscribe(org.reactivestreams.Subscriber subscriber) { + this.sink.asFlux().subscribe(subscriber); + } + + void emit(InstanceEvent... events) { + for (InstanceEvent event : events) { + this.sink.emitNext(event, reactor.core.publisher.Sinks.EmitFailureHandler.FAIL_FAST); + } + } + + } + } diff --git a/spring-boot-admin-server/src/test/java/de/codecentric/boot/admin/server/utils/jackson/InstanceRegistrationUpdatedEventMixinTest.java b/spring-boot-admin-server/src/test/java/de/codecentric/boot/admin/server/utils/jackson/InstanceRegistrationUpdatedEventMixinTest.java index 3ccb488bc7e..1c10e9114e7 100644 --- a/spring-boot-admin-server/src/test/java/de/codecentric/boot/admin/server/utils/jackson/InstanceRegistrationUpdatedEventMixinTest.java +++ b/spring-boot-admin-server/src/test/java/de/codecentric/boot/admin/server/utils/jackson/InstanceRegistrationUpdatedEventMixinTest.java @@ -217,4 +217,57 @@ void verifySerializeWithoutRegistration() throws IOException { assertThat(jsonContent).extractingJsonPathMapValue("$.registration").isNull(); } + @Test + void verifySerializeWithPrevious() throws IOException { + InstanceId id = InstanceId.of("test123"); + Instant timestamp = Instant.ofEpochSecond(1587751031).truncatedTo(ChronoUnit.SECONDS); + Registration registration = Registration.create("new-service", "http://localhost:9080/health").build(); + Registration previous = Registration.create("old-service", "http://localhost:9080/health").build(); + + InstanceRegistrationUpdatedEvent event = new InstanceRegistrationUpdatedEvent(id, 12345678L, timestamp, + registration, previous); + + JsonContent jsonContent = jsonTester.write(event); + assertThat(jsonContent).extractingJsonPathStringValue("$.registration.name").isEqualTo("new-service"); + assertThat(jsonContent).extractingJsonPathStringValue("$.previous.name").isEqualTo("old-service"); + assertThat(jsonContent).extractingJsonPathStringValue("$.previous.healthUrl") + .isEqualTo("http://localhost:9080/health"); + } + + @Test + void verifyDeserializeWithoutPreviousRemainsBackwardCompatible() throws JSONException, JacksonException { + // Legacy JSON payload without the previous field must still deserialize (previous + // is null). + String json = new JSONObject().put("instance", "test123") + .put("version", 12345678L) + .put("timestamp", 1587751031.000000000) + .put("type", "REGISTRATION_UPDATED") + .put("registration", new JSONObject().put("name", "test").put("healthUrl", "http://localhost:9080/health")) + .toString(); + + InstanceRegistrationUpdatedEvent event = jsonMapper.readValue(json, InstanceRegistrationUpdatedEvent.class); + assertThat(event).isNotNull(); + assertThat(event.getRegistration().getName()).isEqualTo("test"); + assertThat(event.getPrevious()).isNull(); + } + + @Test + void verifyDeserializeWithPrevious() throws JSONException, JacksonException { + String json = new JSONObject().put("instance", "test123") + .put("version", 12345678L) + .put("timestamp", 1587751031.000000000) + .put("type", "REGISTRATION_UPDATED") + .put("registration", + new JSONObject().put("name", "new-service").put("healthUrl", "http://localhost:9080/health")) + .put("previous", + new JSONObject().put("name", "old-service").put("healthUrl", "http://localhost:9080/health")) + .toString(); + + InstanceRegistrationUpdatedEvent event = jsonMapper.readValue(json, InstanceRegistrationUpdatedEvent.class); + assertThat(event).isNotNull(); + assertThat(event.getRegistration().getName()).isEqualTo("new-service"); + assertThat(event.getPrevious()).isNotNull(); + assertThat(event.getPrevious().getName()).isEqualTo("old-service"); + } + }