From af9c2d97b1fd396009f39b9da2209df9bc0f5ffc Mon Sep 17 00:00:00 2001 From: yessjun Date: Mon, 10 Aug 2026 22:00:13 +0900 Subject: [PATCH 1/2] fix: surface an unreadable Proxmox body as an api failure A 200 whose body is not the expected envelope raised the parser's own exception straight out of the client: an HTML error page from something standing in front of pveproxy, or a changed envelope during a PVE upgrade, reached callers as a type they do not handle and took the admin dashboard down with a 500. Wrapping the parse inside the client closes every call site at once instead of each one enumerating another exception type. An unreadable body counts as transient, because the same request against a settled host parses. --- .../pusan/pickle/proxmox/ProxmoxClient.java | 16 +++++++++++--- .../pickle/proxmox/ProxmoxClientTest.java | 22 +++++++++++++++++++ 2 files changed, 35 insertions(+), 3 deletions(-) diff --git a/src/main/java/kr/ac/pusan/pickle/proxmox/ProxmoxClient.java b/src/main/java/kr/ac/pusan/pickle/proxmox/ProxmoxClient.java index f2b156b..8933528 100644 --- a/src/main/java/kr/ac/pusan/pickle/proxmox/ProxmoxClient.java +++ b/src/main/java/kr/ac/pusan/pickle/proxmox/ProxmoxClient.java @@ -372,8 +372,13 @@ private String power(String apiHost, String node, int vmid, String action, Map T call(HttpMethod method, URI uri, Map form, TypeReference> responseType) { @@ -396,7 +401,12 @@ private T call(HttpMethod method, URI uri, Map form, throw new ProxmoxApiException(response.getStatusCode().value(), extractErrorMessage(body), description); } - return JSON.readValue(body, responseType).data(); + try { + return JSON.readValue(body, responseType).data(); + } catch (RuntimeException e) { + throw new ProxmoxApiException("Proxmox API returned an unreadable body on " + + description + ": " + e.getMessage(), e); + } }); } catch (ResourceAccessException e) { // connect/read timeout, connection refused/reset … — no HTTP response. diff --git a/src/test/java/kr/ac/pusan/pickle/proxmox/ProxmoxClientTest.java b/src/test/java/kr/ac/pusan/pickle/proxmox/ProxmoxClientTest.java index 2487277..4e0cec7 100644 --- a/src/test/java/kr/ac/pusan/pickle/proxmox/ProxmoxClientTest.java +++ b/src/test/java/kr/ac/pusan/pickle/proxmox/ProxmoxClientTest.java @@ -428,6 +428,28 @@ void missingVmSurfacesPveMessage() { assertThat(e.apiMessage()).contains("does not exist"); } + /** + * A 200 whose body is not the envelope — an HTML error page from something + * standing in front of pveproxy, or a changed envelope mid-upgrade — must + * leave the client as the one exception callers already handle, or every + * caller has to enumerate the parser's types on top of it and the dashboard + * 500s the first time one is missed. + */ + @Test + void anUnparseableOkBodyIsATransientApiFailure() { + wm.server().stubFor(get(urlPathEqualTo("/api2/json/nodes/pve1/status")) + .willReturn(aResponse().withStatus(200) + .withHeader("Content-Type", "text/html") + .withBody("502 Bad Gateway"))); + + ProxmoxApiException e = catchThrowableOfType(ProxmoxApiException.class, + () -> client.nodeStatus(wm.apiHost(), NODE)); + + assertThat(e.statusCode()).isZero(); + assertThat(e.isTransient()).isTrue(); + assertThat(e.getMessage()).contains("/api2/json/nodes/pve1/status"); + } + @Test void transportFailureIsTransientWithoutStatusCode() { wm.server().stubFor(get(urlPathEqualTo("/api2/json/cluster/resources")) From 46af99270554cd148047193320b8c1e5e671e753 Mon Sep 17 00:00:00 2001 From: yessjun Date: Mon, 10 Aug 2026 22:00:26 +0900 Subject: [PATCH 2/2] fix: stop the platform panel reporting a subset as the whole (v0.37.0) Making a failed hypervisor probe non-fatal traded a loud signal for a silent one. A node whose storage read is refused stays reachable with the two storage fields null, and a client that sums the live rows drops it and shows the remaining nodes' total as the platform's, so the operator reads free capacity that is not there. The summary now carries liveCoverage: how many nodes are behind each sum, against the node count the sum should have covered. Three more ways the panel could mislead go with it. An OFFLINE node is probed again: OFFLINE keeps a node out of new placements and leaves its guests running, so skipping it dropped their memory out of the platform total. The guest-storage match now starts from the node column, which is NOT NULL, so a storage entry PVE sent without a name is not a 500. And both halves of the panel are built from one read of the node table, so a status change mid-request can no longer ship a row whose status and reachability contradict. The cost of asking a host that really is dead, one read timeout per dashboard load, is accepted deliberately and belongs to the scale-out round that will probe nodes in parallel under a budget. --- contract/openapi.yaml | 27 +++++- .../pickle/admin/AdminNodeQueryService.java | 15 +++- .../pickle/admin/AdminSummaryService.java | 59 +++++++++---- .../pickle/admin/dto/NodeLiveResponse.java | 14 +-- .../dto/SystemDashboardSummaryResponse.java | 22 ++++- .../pickle/common/openapi/OpenApiConfig.java | 2 +- .../admin/AdminSystemSummaryNodeLiveTest.java | 86 +++++++++++++++---- 7 files changed, 181 insertions(+), 44 deletions(-) diff --git a/contract/openapi.yaml b/contract/openapi.yaml index 548e5ac..be2d96e 100644 --- a/contract/openapi.yaml +++ b/contract/openapi.yaml @@ -1814,6 +1814,25 @@ components: - "id" - "name" type: "object" + LiveCoverage: + properties: + memoryMeasuredNodeCount: + description: "메모리 측정값이 있는 노드 수 — nodeCount보다 작으면 메모리 합계는 부분 측정" + format: "int32" + type: "integer" + nodeCount: + description: "nodesLive 행 수 (플랫폼 전체 노드 수)" + format: "int32" + type: "integer" + storageMeasuredNodeCount: + description: "스토리지 측정값이 있는 노드 수 — nodeCount보다 작으면 스토리지 합계는 부분 측정" + format: "int32" + type: "integer" + required: + - "memoryMeasuredNodeCount" + - "nodeCount" + - "storageMeasuredNodeCount" + type: "object" LoginRequest: properties: email: @@ -1934,7 +1953,8 @@ components: format: "int64" type: "integer" reachable: - description: "false = 이 노드의 Proxmox API가 응답하지 않음 — 나머지 필드는 null" + description: "true = 이 노드가 상태 조회에 응답함. false = 응답하지 않음이며 나머지 필드는 모두 null.\ + \ true여도 스토리지 조회는 별도 권한이라 storage* 필드는 null일 수 있음" type: "boolean" storageTotalBytes: format: "int64" @@ -3618,6 +3638,8 @@ components: items: $ref: "#/components/schemas/IpPoolUsage" type: "array" + liveCoverage: + $ref: "#/components/schemas/LiveCoverage" nodes: items: $ref: "#/components/schemas/NodeRatio" @@ -3645,6 +3667,7 @@ components: required: - "certExpiring30dCount" - "ipPools" + - "liveCoverage" - "nodes" - "nodesLive" - "notificationFailureCount" @@ -4955,7 +4978,7 @@ info: description: "부산대학교 클라우드 플랫폼 Pickle의 REST API. 인증은 JWT Bearer, 오류 응답은 RFC 9457 problem+json(Problem\ \ 스키마)을 따릅니다." title: "Pickle API" - version: "0.36.0" + version: "0.37.0" openapi: "3.1.0" paths: /admin/announcements: diff --git a/src/main/java/kr/ac/pusan/pickle/admin/AdminNodeQueryService.java b/src/main/java/kr/ac/pusan/pickle/admin/AdminNodeQueryService.java index d95ca5c..78663a0 100644 --- a/src/main/java/kr/ac/pusan/pickle/admin/AdminNodeQueryService.java +++ b/src/main/java/kr/ac/pusan/pickle/admin/AdminNodeQueryService.java @@ -53,12 +53,25 @@ public NodeSummaryResponse getNode(long nodeId) { @Transactional(readOnly = true) public List listNodes() { + return listNodes(nodeRepository.findAll(Sort.by("id"))); + } + + /** + * The same summaries over node rows the caller already read. The system + * dashboard builds two halves from one node list — these ratios and the + * live hypervisor probe — and reads the rows once so a status change + * landing mid-request cannot leave the two halves describing different + * rows. Only basic columns are touched, so rows read outside this + * transaction are fine. + */ + @Transactional(readOnly = true) + public List listNodes(List nodes) { double cpuWarn = settingsService.decimal(SettingsService.VCPU_OVERCOMMIT_WARN, DEFAULT_VCPU_OVERCOMMIT_WARN); double memoryWarn = settingsService.decimal(SettingsService.MEMORY_USAGE_WARN, DEFAULT_MEMORY_USAGE_WARN); Map allocations = loadAllocations(); - return nodeRepository.findAll(Sort.by("id")).stream() + return nodes.stream() .map(node -> toSummary(node, allocations.getOrDefault(node.getId(), NodeAllocation.EMPTY), cpuWarn, memoryWarn)) .toList(); diff --git a/src/main/java/kr/ac/pusan/pickle/admin/AdminSummaryService.java b/src/main/java/kr/ac/pusan/pickle/admin/AdminSummaryService.java index e2a0538..609dd93 100644 --- a/src/main/java/kr/ac/pusan/pickle/admin/AdminSummaryService.java +++ b/src/main/java/kr/ac/pusan/pickle/admin/AdminSummaryService.java @@ -17,6 +17,7 @@ import kr.ac.pusan.pickle.admin.dto.OrgDashboardSummaryResponse.TopWorkspace; import kr.ac.pusan.pickle.admin.dto.SystemDashboardSummaryResponse; import kr.ac.pusan.pickle.admin.dto.SystemDashboardSummaryResponse.IpPoolUsage; +import kr.ac.pusan.pickle.admin.dto.SystemDashboardSummaryResponse.LiveCoverage; import kr.ac.pusan.pickle.admin.dto.SystemDashboardSummaryResponse.NodeRatio; import kr.ac.pusan.pickle.admin.dto.SystemDashboardSummaryResponse.Tasks; import kr.ac.pusan.pickle.common.error.ApiException; @@ -24,7 +25,6 @@ import kr.ac.pusan.pickle.config.ClockConfig; import kr.ac.pusan.pickle.inventory.Node; import kr.ac.pusan.pickle.inventory.NodeRepository; -import kr.ac.pusan.pickle.inventory.NodeStatus; import kr.ac.pusan.pickle.ipam.IpPoolRepository; import kr.ac.pusan.pickle.ipam.IpamService; import kr.ac.pusan.pickle.orgs.OrgRepository; @@ -168,9 +168,17 @@ and status not in ('DELETED', 'DELETING') * every tile is an independent counter, each query and each collaborator * carries its own read transaction, and the entities read afterwards are * touched on basic columns only. + * + *

What one transaction did give the two node halves was one reading of + * the node table. Without it, a status change landing mid-request would let + * the ratio list and the live list describe different rows — the panel + * would show a node as ACTIVE and unreachable at the same time, which reads + * as an outage rather than as the parking the operator just did. So the + * rows are read once here and both halves are built from that one list. */ public SystemDashboardSummaryResponse systemSummary() { - List nodes = adminNodeQueryService.listNodes().stream() + List nodeRows = nodeRepository.findAll(org.springframework.data.domain.Sort.by("id")); + List nodes = adminNodeQueryService.listNodes(nodeRows).stream() .map(AdminSummaryService::toNodeRatio) .toList(); Tasks tasks = jdbcTemplate.queryForObject(""" @@ -204,10 +212,30 @@ select count(*) from vms v where s.value = 'true'::jsonb and v.status <> 'DELETED' """); + List live = nodesLive(nodeRows); return new SystemDashboardSummaryResponse(nodes, vmCountsByStatus(null), tasks, notificationFailureCount(), certExpiring30d, driftFindingRepository.countByStatus(DriftFindingStatus.OPEN), - sshPasswordEnabledVms, pools, nodesLive()); + sshPasswordEnabledVms, pools, live, liveCoverage(live)); + } + + /** + * How much of the platform the live numbers actually cover. Each live + * measurement is a per-node sum, and a sum over the nodes that answered is + * not the platform total: one node whose storage read is refused leaves a + * figure smaller than the truth, which an operator reads as free capacity + * that is not there. The counts say how many nodes are behind each sum so a + * client cannot present a subset as the whole — the same reason the org + * headroom figures null a disk number no node measured. + */ + private static LiveCoverage liveCoverage(List live) { + long memory = live.stream() + .filter(node -> node.memTotalBytes() != null && node.memUsedBytes() != null) + .count(); + long storage = live.stream() + .filter(node -> node.storageTotalBytes() != null && node.storageUsedBytes() != null) + .count(); + return new LiveCoverage(live.size(), (int) memory, (int) storage); } /** @@ -228,21 +256,17 @@ select count(*) from vms v * so losing it leaves the two storage fields null on a node that is still * reachable rather than blanking a tile that did answer. * - *

An OFFLINE node is not probed at all — the operator has already said - * the host is down, and asking it only buys a timeout per dashboard load - * (the status poller skips OFFLINE for the same reason). It still gets a - * row, so the node count stays whole, and that row is - * {@code reachable: false}: no live numbers are known either way, and the - * node's own status is already on the panel beside it in the ratio list, - * which is where "the operator parked it" is said. + *

Every node is asked, an OFFLINE one included. OFFLINE excludes a node + * from new placements and leaves its existing guests running, so its RAM is + * still spoken for; skipping the probe would drop that usage out of the + * platform memory tile and show headroom the platform does not have. The + * cost of asking a host that really is dead — one read timeout per + * dashboard load — is accepted deliberately here and belongs to the + * scale-out round, which will probe nodes in parallel under a time budget. */ - private List nodesLive() { + private List nodesLive(List nodes) { List live = new ArrayList<>(); - for (Node node : nodeRepository.findAll(org.springframework.data.domain.Sort.by("id"))) { - if (node.getStatus() == NodeStatus.OFFLINE) { - live.add(NodeLiveResponse.unreachable(node.getId(), node.getName())); - continue; - } + for (Node node : nodes) { NodeStatusInfo status; try { status = proxmoxClient.nodeStatus(node.getApiHost(), node.getName()); @@ -285,7 +309,8 @@ private NodeStorageStatus guestStorage(Node node) { List storages = proxmoxClient.nodeStorage(node.getApiHost(), node.getName()); return storages.stream() - .filter(storage -> storage.storage().equals(node.getStorage())) + // node.storage is NOT NULL; the PVE-supplied name may be absent. + .filter(storage -> node.getStorage().equals(storage.storage())) .findFirst() .orElseGet(() -> storages.stream() .filter(storage -> "lvmthin".equals(storage.type()) && storage.isActive()) diff --git a/src/main/java/kr/ac/pusan/pickle/admin/dto/NodeLiveResponse.java b/src/main/java/kr/ac/pusan/pickle/admin/dto/NodeLiveResponse.java index 142783d..c32e4e8 100644 --- a/src/main/java/kr/ac/pusan/pickle/admin/dto/NodeLiveResponse.java +++ b/src/main/java/kr/ac/pusan/pickle/admin/dto/NodeLiveResponse.java @@ -10,19 +10,21 @@ * measurement is nullable because the answer to "the host is not answering" is * this row with {@code reachable} false — not a missing row and not an error. * - *

{@code reachable} false also covers a node the platform chose not to ask: - * an OFFLINE node is skipped rather than waited on, and the answer is the same - * one either way — no live measurement is known. Why it is unknown is on the - * panel already, in the node's own status beside these numbers. + *

Every node is asked, whatever status the operator gave it: an OFFLINE + * node still runs the guests it had, so its live numbers are still part of the + * platform's. {@code reachable} is therefore exactly "this node answered its + * status probe just now". * *

The measurements come from two independent hypervisor calls, so a * reachable node can still carry null storage: the storage half needs a right - * the status half does not. + * the status half does not. How many nodes are behind each platform sum is on + * the summary itself, in {@code liveCoverage}. */ public record NodeLiveResponse( long nodeId, String name, - @Schema(description = "false = 이 노드의 Proxmox API가 응답하지 않음 — 나머지 필드는 null") + @Schema(description = "true = 이 노드가 상태 조회에 응답함. false = 응답하지 않음이며 나머지 필드는 모두 null." + + " true여도 스토리지 조회는 별도 권한이라 storage* 필드는 null일 수 있음") boolean reachable, @Nullable Long memTotalBytes, @Nullable Long memUsedBytes, diff --git a/src/main/java/kr/ac/pusan/pickle/admin/dto/SystemDashboardSummaryResponse.java b/src/main/java/kr/ac/pusan/pickle/admin/dto/SystemDashboardSummaryResponse.java index ebdf7f0..d7e004c 100644 --- a/src/main/java/kr/ac/pusan/pickle/admin/dto/SystemDashboardSummaryResponse.java +++ b/src/main/java/kr/ac/pusan/pickle/admin/dto/SystemDashboardSummaryResponse.java @@ -1,5 +1,6 @@ package kr.ac.pusan.pickle.admin.dto; +import io.swagger.v3.oas.annotations.media.Schema; import java.util.List; import java.util.Map; import kr.ac.pusan.pickle.inventory.NodeStatus; @@ -14,7 +15,26 @@ public record SystemDashboardSummaryResponse( long openDriftFindingCount, long sshPasswordEnabledVmCount, List ipPools, - List nodesLive) { + List nodesLive, + LiveCoverage liveCoverage) { + + /** + * How many nodes each live measurement in {@code nodesLive} actually + * covers. A platform total summed over {@code nodesLive} is only the + * platform total when every node answered: a node whose storage read was + * refused, or that did not answer at all, leaves a sum smaller than the + * truth, which reads as free capacity that is not there. A client that + * sums must compare these counts with {@code nodeCount} and say so when + * they differ, instead of presenting a subset as the whole. + */ + public record LiveCoverage( + @Schema(description = "nodesLive 행 수 (플랫폼 전체 노드 수)") + int nodeCount, + @Schema(description = "메모리 측정값이 있는 노드 수 — nodeCount보다 작으면 메모리 합계는 부분 측정") + int memoryMeasuredNodeCount, + @Schema(description = "스토리지 측정값이 있는 노드 수 — nodeCount보다 작으면 스토리지 합계는 부분 측정") + int storageMeasuredNodeCount) { + } /** Per-node allocation ratios (same aggregates as {@code GET /admin/nodes}). */ public record NodeRatio( diff --git a/src/main/java/kr/ac/pusan/pickle/common/openapi/OpenApiConfig.java b/src/main/java/kr/ac/pusan/pickle/common/openapi/OpenApiConfig.java index 204e791..e1d2ab4 100644 --- a/src/main/java/kr/ac/pusan/pickle/common/openapi/OpenApiConfig.java +++ b/src/main/java/kr/ac/pusan/pickle/common/openapi/OpenApiConfig.java @@ -41,7 +41,7 @@ public class OpenApiConfig { /** Contract version served in {@code info.version}; bump on any contract change. */ - public static final String CONTRACT_VERSION = "0.36.0"; + public static final String CONTRACT_VERSION = "0.37.0"; /** Name of the bearer-JWT security scheme in the published spec. */ private static final String BEARER_SCHEME = "bearerAuth"; diff --git a/src/test/java/kr/ac/pusan/pickle/admin/AdminSystemSummaryNodeLiveTest.java b/src/test/java/kr/ac/pusan/pickle/admin/AdminSystemSummaryNodeLiveTest.java index 3cfb48c..3f569ce 100644 --- a/src/test/java/kr/ac/pusan/pickle/admin/AdminSystemSummaryNodeLiveTest.java +++ b/src/test/java/kr/ac/pusan/pickle/admin/AdminSystemSummaryNodeLiveTest.java @@ -2,9 +2,9 @@ import static org.hamcrest.Matchers.contains; import static org.hamcrest.Matchers.nullValue; +import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.anyString; import static org.mockito.ArgumentMatchers.eq; -import static org.mockito.Mockito.never; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.get; @@ -14,6 +14,7 @@ import java.util.ArrayList; import java.util.List; import java.util.UUID; +import kr.ac.pusan.pickle.inventory.NodeRepository; import kr.ac.pusan.pickle.proxmox.ProxmoxApiException; import kr.ac.pusan.pickle.proxmox.ProxmoxClient; import kr.ac.pusan.pickle.proxmox.dto.NodeStatusInfo; @@ -29,9 +30,11 @@ import org.springframework.boot.test.context.SpringBootTest; import org.springframework.boot.webmvc.test.autoconfigure.AutoConfigureMockMvc; import org.springframework.context.annotation.Import; +import org.springframework.data.domain.Sort; import org.springframework.jdbc.core.JdbcTemplate; import org.springframework.test.context.ActiveProfiles; import org.springframework.test.context.bean.override.mockito.MockitoBean; +import org.springframework.test.context.bean.override.mockito.MockitoSpyBean; import org.springframework.test.web.servlet.MockMvc; /** @@ -42,10 +45,11 @@ * same thing by accident and stop asserting it the day that changes. * *

The other cases the live block has to keep apart: a node that answered its - * status but not its storage is reachable with the storage half missing, a - * status reply carrying no payload is not an answer at all, and a node the - * operator marked OFFLINE is not asked in the first place while still holding - * its row on the panel. + * status but not its storage is reachable with the storage half missing and + * says so in the coverage counts, a status reply carrying no payload is not an + * answer at all, a storage entry PVE sent without a name is not a crash, and a + * node the operator marked OFFLINE is asked like any other because its guests + * are still running on it. */ @SpringBootTest @AutoConfigureMockMvc @@ -74,6 +78,9 @@ class AdminSystemSummaryNodeLiveTest { @MockitoBean private ProxmoxClient proxmoxClient; + @MockitoSpyBean + private NodeRepository nodeRepository; + private String sysAdminToken; private final List createdNodeIds = new ArrayList<>(); @@ -126,6 +133,33 @@ void aStorageOnlyFailureKeepsTheNodeReachable() throws Exception { .andExpect(jsonPath("$.nodesLive[?(@.name == 'pve1')].storageTotalBytes", contains(nullValue()))) .andExpect(jsonPath("$.nodesLive[?(@.name == 'pve1')].storageUsedBytes", + contains(nullValue()))) + // The node stays on the panel, but the platform storage sum is + // now over fewer nodes than the platform has, and a client that + // sums has to be able to see that. + .andExpect(jsonPath("$.liveCoverage.nodeCount").value(1)) + .andExpect(jsonPath("$.liveCoverage.memoryMeasuredNodeCount").value(1)) + .andExpect(jsonPath("$.liveCoverage.storageMeasuredNodeCount").value(0)); + } + + /** + * The storage list comes back as whatever the PVE envelope held, so an + * entry can arrive without the name the match is made on. The node column + * is NOT NULL and the PVE side is not, so the comparison runs from the + * column — otherwise one nameless entry takes the whole dashboard down. + */ + @Test + void aStorageEntryWithoutANameDoesNotFailThePanel() throws Exception { + when(proxmoxClient.nodeStatus(anyString(), anyString())).thenReturn(STATUS); + when(proxmoxClient.nodeStorage(anyString(), anyString())).thenReturn(List.of( + new NodeStorageStatus(null, "dir", 1, 1, 2_000_000_000_000L, + 500_000_000_000L, 1_500_000_000_000L, 0.25))); + + mockMvc.perform(get("/api/v1/admin/system-summary") + .header("Authorization", "Bearer " + sysAdminToken)) + .andExpect(status().isOk()) + .andExpect(jsonPath("$.nodesLive[?(@.name == 'pve1')].reachable").value(true)) + .andExpect(jsonPath("$.nodesLive[?(@.name == 'pve1')].storageTotalBytes", contains(nullValue()))); } @@ -146,12 +180,13 @@ void aStatusReplyWithNoPayloadIsNotAnAnswer() throws Exception { } /** - * A node the operator marked OFFLINE is known to be down, so probing it only - * buys a timeout per dashboard load. It keeps its row so the node count - * stays whole. + * OFFLINE keeps a node out of new placements and leaves the guests it + * already has running, so its memory is still spoken for. Asking it is the + * only way the platform memory tile can count that usage, and a node that + * really is down still degrades on its own without taking the panel. */ @Test - void anOfflineNodeIsNotProbedButStillHoldsItsRow() throws Exception { + void anOfflineNodeIsStillProbedBecauseItsGuestsKeepRunning() throws Exception { String name = "assnl-offline-" + UUID.randomUUID().toString().substring(0, 8); createOfflineNode(name); when(proxmoxClient.nodeStatus(anyString(), anyString())).thenReturn(STATUS); @@ -162,15 +197,34 @@ void anOfflineNodeIsNotProbedButStillHoldsItsRow() throws Exception { mockMvc.perform(get("/api/v1/admin/system-summary") .header("Authorization", "Bearer " + sysAdminToken)) .andExpect(status().isOk()) - .andExpect(jsonPath("$.nodesLive[?(@.name == '" + name + "')].nodeId").exists()) .andExpect(jsonPath("$.nodesLive[?(@.name == '" + name + "')].reachable") - .value(false)) - // The nodes that are not OFFLINE still answer, so the skip is a - // filter on that one node and not the block giving up. - .andExpect(jsonPath("$.nodesLive[?(@.name == 'pve1')].reachable").value(true)); + .value(true)) + .andExpect(jsonPath("$.nodesLive[?(@.name == '" + name + "')].memUsedBytes") + .value(24_000_000_000L)) + .andExpect(jsonPath("$.nodesLive[?(@.name == 'pve1')].reachable").value(true)) + .andExpect(jsonPath("$.liveCoverage.nodeCount").value(2)) + .andExpect(jsonPath("$.liveCoverage.memoryMeasuredNodeCount").value(2)); + + verify(proxmoxClient).nodeStatus(anyString(), eq(name)); + verify(proxmoxClient).nodeStorage(anyString(), eq(name)); + } + + /** + * The ratio list and the live list are two halves of one panel. Read from + * two separate queries they can straddle a status change and describe + * different rows, and the console reads a node that is ACTIVE in one half + * and unreachable in the other as an outage nobody caused. One read of the + * node table is what keeps them consistent. + */ + @Test + void bothHalvesOfThePanelAreBuiltFromOneReadOfTheNodeTable() throws Exception { + when(proxmoxClient.nodeStatus(anyString(), anyString())).thenReturn(STATUS); + + mockMvc.perform(get("/api/v1/admin/system-summary") + .header("Authorization", "Bearer " + sysAdminToken)) + .andExpect(status().isOk()); - verify(proxmoxClient, never()).nodeStatus(anyString(), eq(name)); - verify(proxmoxClient, never()).nodeStorage(anyString(), eq(name)); + verify(nodeRepository).findAll(any(Sort.class)); } private void createOfflineNode(String name) {