diff options
author | Bjørn Christian Seime <bjorncs@yahooinc.com> | 2022-09-15 15:53:15 +0200 |
---|---|---|
committer | Bjørn Christian Seime <bjorncs@yahooinc.com> | 2022-09-15 15:53:15 +0200 |
commit | 1db6198b2c4b8cb96c4872defd7fd8128cad1d77 (patch) | |
tree | 0c565c0e47e58466d8ae97351da871035a86c54c /node-admin | |
parent | 88ea0524a2af51f997c87d616315fa6e0562b72f (diff) |
Fail service dump request gracefully if JSON is invalid
Diffstat (limited to 'node-admin')
3 files changed, 52 insertions, 10 deletions
diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/maintenance/servicedump/ServiceDumpReport.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/maintenance/servicedump/ServiceDumpReport.java index 452f786301b..7ce287a83f1 100644 --- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/maintenance/servicedump/ServiceDumpReport.java +++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/maintenance/servicedump/ServiceDumpReport.java @@ -89,10 +89,15 @@ class ServiceDumpReport extends BaseReport { } public static ServiceDumpReport createErrorReport( - ServiceDumpReport request, Instant startedAt, Instant failedAt, String message) { + ServiceDumpReport reqOrNull, Instant startedAt, Instant failedAt, String message) { + Long createdAt = reqOrNull != null ? reqOrNull.getCreatedMillisOrNull() : Long.valueOf(startedAt.toEpochMilli()); + String configId = reqOrNull != null ? reqOrNull.configId() : "unknown"; + Long expireAt = reqOrNull != null ? reqOrNull.expireAt() : null; + List<String> artifacts = reqOrNull != null ? reqOrNull.artifacts() : List.of(); + DumpOptions dumpOptions = reqOrNull != null ? reqOrNull.dumpOptions() : null; return new ServiceDumpReport( - request.getCreatedMillisOrNull(), startedAt.toEpochMilli(), null, failedAt.toEpochMilli(), null, - request.configId(), request.expireAt(), message, request.artifacts(), request.dumpOptions()); + createdAt, startedAt.toEpochMilli(), null, failedAt.toEpochMilli(), null, + configId, expireAt, message, artifacts, dumpOptions); } @JsonGetter(STARTED_AT_FIELD) public Long startedAt() { return startedAt; } diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/maintenance/servicedump/VespaServiceDumperImpl.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/maintenance/servicedump/VespaServiceDumperImpl.java index d942e5c9e80..3a0cd412a2e 100644 --- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/maintenance/servicedump/VespaServiceDumperImpl.java +++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/maintenance/servicedump/VespaServiceDumperImpl.java @@ -17,6 +17,7 @@ import com.yahoo.vespa.hosted.node.admin.task.util.fs.ContainerPath; import com.yahoo.vespa.hosted.node.admin.task.util.process.CommandResult; import com.yahoo.yolean.concurrent.Sleeper; +import java.io.UncheckedIOException; import java.net.URI; import java.time.Clock; import java.time.Instant; @@ -66,8 +67,14 @@ public class VespaServiceDumperImpl implements VespaServiceDumper { Instant startedAt = clock.instant(); NodeSpec nodeSpec = context.node(); - ServiceDumpReport request = nodeSpec.reports().getReport(ServiceDumpReport.REPORT_ID, ServiceDumpReport.class) - .orElse(null); + ServiceDumpReport request; + try { + request = nodeSpec.reports().getReport(ServiceDumpReport.REPORT_ID, ServiceDumpReport.class) + .orElse(null); + } catch (IllegalArgumentException | UncheckedIOException e) { + handleFailure(context, null, startedAt, e, "Invalid JSON in service dump request"); + return; + } if (request == null || request.isCompletedOrFailed()) { context.log(log, Level.FINE, "No service dump requested or dump already completed/failed"); return; @@ -114,7 +121,7 @@ public class VespaServiceDumperImpl implements VespaServiceDumper { uploadArtifacts(context, destination, producedArtifacts); storeReport(context, ServiceDumpReport.createSuccessReport(request, startedAt, clock.instant(), destination)); } catch (Exception e) { - handleFailure(context, request, startedAt, e); + handleFailure(context, request, startedAt, e, e.getMessage()); } finally { if (unixPathDirectory.exists()) { context.log(log, Level.INFO, "Deleting directory '" + unixPathDirectory +"'."); @@ -147,15 +154,16 @@ public class VespaServiceDumperImpl implements VespaServiceDumper { : Instant.ofEpochMilli(request.expireAt()); } - private void handleFailure(NodeAgentContext context, ServiceDumpReport request, Instant startedAt, Exception failure) { + private void handleFailure(NodeAgentContext context, ServiceDumpReport requestOrNull, Instant startedAt, + Exception failure, String message) { context.log(log, Level.WARNING, failure.toString(), failure); - ServiceDumpReport report = ServiceDumpReport.createErrorReport(request, startedAt, clock.instant(), failure.toString()); + ServiceDumpReport report = ServiceDumpReport.createErrorReport(requestOrNull, startedAt, clock.instant(), message); storeReport(context, report); } - private void handleFailure(NodeAgentContext context, ServiceDumpReport request, Instant startedAt, String message) { + private void handleFailure(NodeAgentContext context, ServiceDumpReport requestOrNull, Instant startedAt, String message) { context.log(log, Level.WARNING, message); - ServiceDumpReport report = ServiceDumpReport.createErrorReport(request, startedAt, clock.instant(), message); + ServiceDumpReport report = ServiceDumpReport.createErrorReport(requestOrNull, startedAt, clock.instant(), message); storeReport(context, report); } diff --git a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/maintenance/servicedump/VespaServiceDumperImplTest.java b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/maintenance/servicedump/VespaServiceDumperImplTest.java index fcdef83e06f..081f0038e06 100644 --- a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/maintenance/servicedump/VespaServiceDumperImplTest.java +++ b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/maintenance/servicedump/VespaServiceDumperImplTest.java @@ -1,6 +1,9 @@ // Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.vespa.hosted.node.admin.maintenance.servicedump; +import com.fasterxml.jackson.databind.ObjectMapper; +import com.fasterxml.jackson.databind.node.JsonNodeFactory; +import com.fasterxml.jackson.databind.node.ObjectNode; import com.yahoo.test.ManualClock; import com.yahoo.vespa.hosted.node.admin.configserver.noderepository.NodeSpec; import com.yahoo.vespa.hosted.node.admin.configserver.noderepository.NodeState; @@ -170,6 +173,32 @@ class VespaServiceDumperImplTest { assertSyncedFiles(context, syncClient, expectedUris); } + @Test + void fails_gracefully_on_invalid_request_json() { + // Setup mocks + ContainerOperations operations = mock(ContainerOperations.class); + SyncClient syncClient = createSyncClientMock(); + NodeRepoMock nodeRepository = new NodeRepoMock(); + ManualClock clock = new ManualClock(Instant.ofEpochMilli(1600001000000L)); + JsonNodeFactory fac = new ObjectMapper().getNodeFactory(); + ObjectNode invalidRequest = new ObjectNode(fac) + .set("dumpOptions", new ObjectNode(fac).put("duration", "invalidDurationDataType")); + NodeSpec spec = NodeSpec.Builder + .testSpec(HOSTNAME, NodeState.active) + .report(ServiceDumpReport.REPORT_ID, invalidRequest) + .build(); + nodeRepository.updateNodeSpec(spec); + VespaServiceDumper reporter = new VespaServiceDumperImpl( + ArtifactProducers.createDefault(Sleeper.NOOP), operations, syncClient, nodeRepository, clock); + NodeAgentContextImpl context = NodeAgentContextImpl.builder(spec) + .fileSystem(fileSystem) + .build(); + reporter.processServiceDumpRequest(context); + String expectedJson = "{\"createdMillis\":1600001000000,\"startedAt\":1600001000000,\"failedAt\":1600001000000," + + "\"configId\":\"unknown\",\"error\":\"Invalid JSON in service dump request\",\"artifacts\":[]}"; + assertReportEquals(nodeRepository, expectedJson); + } + private static NodeSpec createNodeSpecWithDumpRequest(NodeRepoMock repository, List<String> artifacts, ServiceDumpReport.DumpOptions options) { ServiceDumpReport request = ServiceDumpReport.createRequestReport( |