From c9c43d2dfeb785fbf0baa225cc05723b660a64bf Mon Sep 17 00:00:00 2001 From: Jon Marius Venstad Date: Wed, 16 Jan 2019 11:32:25 +0100 Subject: Forward firmware check requests from controller to zones --- .../integration/configserver/NodeRepository.java | 6 +++ .../noderepository/ProvisionResource.java | 8 +++ .../hosted/controller/restapi/os/OsApiHandler.java | 60 ++++++++++++++++++++++ .../controller/integration/NodeRepositoryMock.java | 12 +++++ .../hosted/controller/restapi/os/OsApiTest.java | 16 ++++++ 5 files changed, 102 insertions(+) diff --git a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/configserver/NodeRepository.java b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/configserver/NodeRepository.java index b0acea188d0..7b46e1f589f 100644 --- a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/configserver/NodeRepository.java +++ b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/configserver/NodeRepository.java @@ -33,4 +33,10 @@ public interface NodeRepository { /** Upgrade OS for all nodes of given type to a new version */ void upgradeOs(ZoneId zone, NodeType type, Version version); + /** Requests firmware checks on all hosts in the given zone. */ + void requestFirmwareCheck(ZoneId zone); + + /** Cancels firmware checks on all hosts in the given zone. */ + void cancelFirmwareCheck(ZoneId zone); + } diff --git a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/noderepository/ProvisionResource.java b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/noderepository/ProvisionResource.java index 84b8a6ac009..fd256873e3e 100644 --- a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/noderepository/ProvisionResource.java +++ b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/noderepository/ProvisionResource.java @@ -82,5 +82,13 @@ public interface ProvisionResource { String upgrade(@PathParam("nodeType") NodeType nodeType, NodeUpgrade nodeUpgrade, @HeaderParam("X-HTTP-Method-Override") String patchOverride); + @POST + @Path("/upgrade/firmware") + String requestFirmwareChecks(); + + @DELETE + @Path("/upgrade/firmware") + String cancelFirmwareChecks(); + } diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/os/OsApiHandler.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/os/OsApiHandler.java index fa1b8699612..76e15c8de5a 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/os/OsApiHandler.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/os/OsApiHandler.java @@ -2,6 +2,8 @@ package com.yahoo.vespa.hosted.controller.restapi.os; import com.yahoo.component.Version; +import com.yahoo.config.provision.Environment; +import com.yahoo.config.provision.RegionName; import com.yahoo.container.jdisc.HttpRequest; import com.yahoo.container.jdisc.HttpResponse; import com.yahoo.container.jdisc.LoggingRequestHandler; @@ -13,7 +15,10 @@ import com.yahoo.slime.Slime; import com.yahoo.vespa.config.SlimeUtils; import com.yahoo.vespa.hosted.controller.Controller; import com.yahoo.vespa.hosted.controller.api.integration.zone.CloudName; +import com.yahoo.vespa.hosted.controller.api.integration.zone.ZoneId; +import com.yahoo.vespa.hosted.controller.api.integration.zone.ZoneList; import com.yahoo.vespa.hosted.controller.restapi.ErrorResponse; +import com.yahoo.vespa.hosted.controller.restapi.MessageResponse; import com.yahoo.vespa.hosted.controller.restapi.SlimeJsonResponse; import com.yahoo.vespa.hosted.controller.versions.OsVersion; import com.yahoo.yolean.Exceptions; @@ -21,8 +26,12 @@ import com.yahoo.yolean.Exceptions; import java.io.IOException; import java.io.InputStream; import java.io.UncheckedIOException; +import java.util.List; +import java.util.Map; import java.util.Set; +import java.util.StringJoiner; import java.util.logging.Level; +import java.util.stream.Collectors; /** * This implements the /os/v1 API which provides operators with information about, and scheduling of OS upgrades for @@ -45,6 +54,8 @@ public class OsApiHandler extends LoggingRequestHandler { try { switch (request.getMethod()) { case GET: return get(request); + case POST: return post(request); + case DELETE: return delete(request); case PATCH: return patch(request); default: return ErrorResponse.methodNotAllowed("Method '" + request.getMethod() + "' is unsupported"); } @@ -68,6 +79,55 @@ public class OsApiHandler extends LoggingRequestHandler { return ErrorResponse.notFoundError("Nothing at " + path); } + private HttpResponse post(HttpRequest request) { + Path path = new Path(request.getUri().getPath()); + if (path.matches("/os/v1/firmware/")) return requestFirmwareCheckResponse(path); + if (path.matches("/os/v1/firmware/{environment}/")) return requestFirmwareCheckResponse(path); + if (path.matches("/os/v1/firmware/{environment}/{region}/")) return requestFirmwareCheckResponse(path); + return ErrorResponse.notFoundError("Nothing at " + path); + } + + private HttpResponse delete(HttpRequest request) { + Path path = new Path(request.getUri().getPath()); + if (path.matches("/os/v1/firmware/")) return cancelFirmwareCheckResponse(path); + if (path.matches("/os/v1/firmware/{environment}/")) return cancelFirmwareCheckResponse(path); + if (path.matches("/os/v1/firmware/{environment}/{region}/")) return cancelFirmwareCheckResponse(path); + return ErrorResponse.notFoundError("Nothing at " + path); + } + + private HttpResponse requestFirmwareCheckResponse(Path path) { + List zones = zonesAt(path); + if (zones.isEmpty()) + return ErrorResponse.notFoundError("No zones at " + path); + + StringJoiner response = new StringJoiner(", ", "Requested firmware checks in ", "."); + for (ZoneId zone : zones) { + controller.configServer().nodeRepository().requestFirmwareCheck(zone); + response.add(zone.value()); + } + return new MessageResponse(response.toString()); + } + + private HttpResponse cancelFirmwareCheckResponse(Path path) { + List zones = zonesAt(path); + if (zones.isEmpty()) + return ErrorResponse.notFoundError("No zones at " + path); + + StringJoiner response = new StringJoiner(", ", "Cancelled firmware checks in ", "."); + for (ZoneId zone : zones) { + controller.configServer().nodeRepository().cancelFirmwareCheck(zone); + response.add(zone.value()); + } + return new MessageResponse(response.toString()); + } + + private List zonesAt(Path path) { + ZoneList zones = controller.zoneRegistry().zones().controllerUpgraded(); + if (path.get("region") != null) zones = zones.in(RegionName.from(path.get("region"))); + if (path.get("environment") != null) zones = zones.in(Environment.from(path.get("environment"))); + return zones.ids(); + } + private Slime setOsVersion(HttpRequest request) { Slime requestData = toSlime(request.getData()); Inspector root = requestData.get(); diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/integration/NodeRepositoryMock.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/integration/NodeRepositoryMock.java index b160b853986..f375d80b28b 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/integration/NodeRepositoryMock.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/integration/NodeRepositoryMock.java @@ -12,11 +12,13 @@ import com.yahoo.vespa.hosted.controller.api.integration.zone.ZoneId; import java.util.Collections; import java.util.HashMap; +import java.util.HashSet; import java.util.List; import java.util.Map; import java.util.NoSuchElementException; import java.util.Objects; import java.util.Optional; +import java.util.Set; import java.util.function.Function; import java.util.function.UnaryOperator; import java.util.stream.Collectors; @@ -93,6 +95,16 @@ public class NodeRepositoryMock implements NodeRepository { .forEach(node -> putByHostname(zone, node)); } + @Override + public void requestFirmwareCheck(ZoneId zone) { + ; + } + + @Override + public void cancelFirmwareCheck(ZoneId zone) { + ; + } + public void doUpgrade(DeploymentId deployment, Optional hostName, Version version) { modifyNodes(deployment, hostName, node -> { assert node.wantedVersion().equals(version); diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/os/OsApiTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/os/OsApiTest.java index a7f98103885..9ec254b59fd 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/os/OsApiTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/os/OsApiTest.java @@ -111,6 +111,22 @@ public class OsApiTest extends ControllerContainerTest { assertResponse(new Request("http://localhost:8080/os/v1/", "{\"version\": \"7.4.1\", \"cloud\": \"cloud1\"}", Request.Method.PATCH), "{\"error-code\":\"BAD_REQUEST\",\"message\":\"Cannot downgrade cloud 'cloud1' to version 7.4.1\"}", 400); + // Request firmware checks in all zones. + assertResponse(new Request("http://localhost:8080/os/v1/firmware/", "", Request.Method.POST), + "{\"message\":\"Requested firmware checks in prod.us-east-3, prod.us-west-1, prod.eu-west-1.\"}", 200); + + // Cancel firmware checks in all prod zones. + assertResponse(new Request("http://localhost:8080/os/v1/firmware/prod/", "", Request.Method.DELETE), + "{\"message\":\"Cancelled firmware checks in prod.us-east-3, prod.us-west-1, prod.eu-west-1.\"}", 200); + + // Request firmware checks in prod.us-east-3. + assertResponse(new Request("http://localhost:8080/os/v1/firmware/prod/us-east-3", "", Request.Method.POST), + "{\"message\":\"Requested firmware checks in prod.us-east-3.\"}", 200); + + // Error: Cancel firmware checks in an empty set of zones. + assertResponse(new Request("http://localhost:8080/os/v1/firmware/dev/", "", Request.Method.DELETE), + "{\"error-code\":\"NOT_FOUND\",\"message\":\"No zones at path '/os/v1/firmware/dev'\"}", 404); + } private void upgradeAndUpdateStatus() { -- cgit v1.2.3 From 1ac1658fed8f5e0c0a77ebed533cfd4e8ad40192 Mon Sep 17 00:00:00 2001 From: Jon Marius Venstad Date: Fri, 18 Jan 2019 13:02:31 +0100 Subject: Made firmware checks visible to host admin --- .../noderepository/NodeAttributes.java | 15 +++- .../configserver/noderepository/NodeSpec.java | 93 +++++++++++++++------- .../noderepository/RealNodeRepository.java | 4 + .../bindings/NodeRepositoryNode.java | 6 ++ 4 files changed, 89 insertions(+), 29 deletions(-) diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/noderepository/NodeAttributes.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/noderepository/NodeAttributes.java index 44bc606f385..3766402defa 100644 --- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/noderepository/NodeAttributes.java +++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/noderepository/NodeAttributes.java @@ -3,6 +3,7 @@ package com.yahoo.vespa.hosted.node.admin.configserver.noderepository; import com.yahoo.vespa.hosted.dockerapi.DockerImage; +import java.time.Instant; import java.util.Objects; import java.util.Optional; import java.util.stream.Collectors; @@ -15,6 +16,7 @@ public class NodeAttributes { private Optional dockerImage = Optional.empty(); private Optional vespaVersion = Optional.empty(); private Optional currentOsVersion = Optional.empty(); + private Optional currentFirmwareCheck = Optional.empty(); private Optional hardwareDivergence = Optional.empty(); private Optional hardwareFailureDescription = Optional.empty(); private Optional wantToDeprovision = Optional.empty(); @@ -50,6 +52,11 @@ public class NodeAttributes { return this; } + public NodeAttributes withCurrentFirmwareCheck(Instant currentFirmwareCheck) { + this.currentFirmwareCheck = Optional.of(currentFirmwareCheck); + return this; + } + public NodeAttributes withHardwareDivergence(String hardwareDivergence) { this.hardwareDivergence = Optional.of(hardwareDivergence); return this; @@ -86,6 +93,10 @@ public class NodeAttributes { return currentOsVersion; } + public Optional getCurrentFirmwareCheck() { + return currentFirmwareCheck; + } + public Optional getHardwareDivergence() { return hardwareDivergence; } @@ -101,7 +112,7 @@ public class NodeAttributes { @Override public int hashCode() { return Objects.hash(restartGeneration, rebootGeneration, dockerImage, vespaVersion, currentOsVersion, - hardwareDivergence, hardwareFailureDescription, wantToDeprovision); + currentFirmwareCheck, hardwareDivergence, hardwareFailureDescription, wantToDeprovision); } @Override @@ -116,6 +127,7 @@ public class NodeAttributes { && Objects.equals(dockerImage, other.dockerImage) && Objects.equals(vespaVersion, other.vespaVersion) && Objects.equals(currentOsVersion, other.currentOsVersion) + && Objects.equals(currentFirmwareCheck, other.currentFirmwareCheck) && Objects.equals(hardwareDivergence, other.hardwareDivergence) && Objects.equals(hardwareFailureDescription, other.hardwareFailureDescription) && Objects.equals(wantToDeprovision, other.wantToDeprovision); @@ -129,6 +141,7 @@ public class NodeAttributes { dockerImage.map(img -> "dockerImage=" + img.asString()), vespaVersion.map(ver -> "vespaVersion=" + ver), currentOsVersion.map(ver -> "currentOsVersion=" + ver), + currentFirmwareCheck.map(at -> "currentFirmwareCheck=" + at), hardwareDivergence.map(hwDivg -> "hardwareDivergence=" + hwDivg), hardwareFailureDescription.map(hwDesc -> "hardwareFailureDescription=" + hwDesc), wantToDeprovision.map(depr -> "wantToDeprovision=" + depr)) diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/noderepository/NodeSpec.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/noderepository/NodeSpec.java index c00caef5587..225929db4bd 100644 --- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/noderepository/NodeSpec.java +++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/noderepository/NodeSpec.java @@ -6,6 +6,7 @@ import com.yahoo.config.provision.NodeType; import com.yahoo.vespa.hosted.dockerapi.DockerImage; import com.yahoo.vespa.hosted.provision.Node; +import java.time.Instant; import java.util.Collections; import java.util.Objects; import java.util.Optional; @@ -36,6 +37,9 @@ public class NodeSpec { private final long wantedRebootGeneration; private final long currentRebootGeneration; + private final Optional wantedFirmwareCheck; + private final Optional currentFirmwareCheck; + private final Optional allowedToBeDown; private final Optional wantToDeprovision; private final Optional owner; @@ -54,34 +58,36 @@ public class NodeSpec { private final Optional parentHostname; public NodeSpec( - final String hostname, - final Optional wantedDockerImage, - final Optional currentDockerImage, - final Node.State state, - final NodeType nodeType, - final String flavor, - final String canonicalFlavor, - final Optional wantedVespaVersion, - final Optional vespaVersion, - final Optional wantedOsVersion, - final Optional currentOsVersion, - final Optional allowedToBeDown, - final Optional wantToDeprovision, - final Optional owner, - final Optional membership, - final Optional wantedRestartGeneration, - final Optional currentRestartGeneration, - final long wantedRebootGeneration, - final long currentRebootGeneration, - final double minCpuCores, - final double minMainMemoryAvailableGb, - final double minDiskAvailableGb, - final boolean fastDisk, - final double bandwidth, - final Set ipAddresses, - final Optional hardwareDivergence, - final Optional hardwareFailureDescription, - final Optional parentHostname) { + String hostname, + Optional wantedDockerImage, + Optional currentDockerImage, + Node.State state, + NodeType nodeType, + String flavor, + String canonicalFlavor, + Optional wantedVespaVersion, + Optional vespaVersion, + Optional wantedOsVersion, + Optional currentOsVersion, + Optional allowedToBeDown, + Optional wantToDeprovision, + Optional owner, + Optional membership, + Optional wantedRestartGeneration, + Optional currentRestartGeneration, + long wantedRebootGeneration, + long currentRebootGeneration, + Optional wantedFirmwareCheck, + Optional currentFirmwareCheck, + double minCpuCores, + double minMainMemoryAvailableGb, + double minDiskAvailableGb, + boolean fastDisk, + double bandwidth, + Set ipAddresses, + Optional hardwareDivergence, + Optional hardwareFailureDescription, + Optional parentHostname) { this.hostname = Objects.requireNonNull(hostname); this.wantedDockerImage = Objects.requireNonNull(wantedDockerImage); this.currentDockerImage = Objects.requireNonNull(currentDockerImage); @@ -101,6 +107,8 @@ public class NodeSpec { this.currentRestartGeneration = currentRestartGeneration; this.wantedRebootGeneration = wantedRebootGeneration; this.currentRebootGeneration = currentRebootGeneration; + this.wantedFirmwareCheck = Objects.requireNonNull(wantedFirmwareCheck); + this.currentFirmwareCheck = Objects.requireNonNull(currentFirmwareCheck); this.minCpuCores = minCpuCores; this.minMainMemoryAvailableGb = minMainMemoryAvailableGb; this.minDiskAvailableGb = minDiskAvailableGb; @@ -172,6 +180,14 @@ public class NodeSpec { return currentRebootGeneration; } + public Optional getWantedFirmwareCheck() { + return wantedFirmwareCheck; + } + + public Optional getCurrentFirmwareCheck() { + return currentFirmwareCheck; + } + public Optional getAllowedToBeDown() { return allowedToBeDown; } @@ -250,6 +266,8 @@ public class NodeSpec { Objects.equals(currentRestartGeneration, that.currentRestartGeneration) && Objects.equals(wantedRebootGeneration, that.wantedRebootGeneration) && Objects.equals(currentRebootGeneration, that.currentRebootGeneration) && + Objects.equals(wantedFirmwareCheck, that.wantedFirmwareCheck) && + Objects.equals(currentFirmwareCheck, that.currentFirmwareCheck) && Objects.equals(minCpuCores, that.minCpuCores) && Objects.equals(minMainMemoryAvailableGb, that.minMainMemoryAvailableGb) && Objects.equals(minDiskAvailableGb, that.minDiskAvailableGb) && @@ -283,6 +301,8 @@ public class NodeSpec { currentRestartGeneration, wantedRebootGeneration, currentRebootGeneration, + wantedFirmwareCheck, + currentFirmwareCheck, minCpuCores, minMainMemoryAvailableGb, minDiskAvailableGb, @@ -317,6 +337,8 @@ public class NodeSpec { + " currentRestartGeneration=" + currentRestartGeneration + " wantedRebootGeneration=" + wantedRebootGeneration + " currentRebootGeneration=" + currentRebootGeneration + + " wantedFirmwareCheck=" + wantedFirmwareCheck + + " currentFirmwareCheck=" + currentFirmwareCheck + " minMainMemoryAvailableGb=" + minMainMemoryAvailableGb + " minDiskAvailableGb=" + minDiskAvailableGb + " fastDisk=" + fastDisk @@ -477,6 +499,8 @@ public class NodeSpec { private Optional currentRestartGeneration = Optional.empty(); private long wantedRebootGeneration; private long currentRebootGeneration; + private Optional wantedFirmwareCheck = Optional.empty(); + private Optional currentFirmwareCheck = Optional.empty(); private double minCpuCores; private double minMainMemoryAvailableGb; private double minDiskAvailableGb; @@ -516,6 +540,8 @@ public class NodeSpec { node.membership.ifPresent(this::membership); node.wantedRestartGeneration.ifPresent(this::wantedRestartGeneration); node.currentRestartGeneration.ifPresent(this::currentRestartGeneration); + node.wantedFirmwareCheck.ifPresent(this::wantedFirmwareCheck); + node.currentFirmwareCheck.ifPresent(this::currentFirmwareCheck); node.hardwareDivergence.ifPresent(this::hardwareDivergence); node.hardwareFailureDescription.ifPresent(this::hardwareFailureDescription); node.parentHostname.ifPresent(this::parentHostname); @@ -616,6 +642,16 @@ public class NodeSpec { return this; } + public Builder wantedFirmwareCheck(Instant wantedFirmwareCheck) { + this.wantedFirmwareCheck = Optional.of(wantedFirmwareCheck); + return this; + } + + public Builder currentFirmwareCheck(Instant currentFirmwareCheck) { + this.currentFirmwareCheck = Optional.of(currentFirmwareCheck); + return this; + } + public Builder minCpuCores(double minCpuCores) { this.minCpuCores = minCpuCores; return this; @@ -791,6 +827,7 @@ public class NodeSpec { owner, membership, wantedRestartGeneration, currentRestartGeneration, wantedRebootGeneration, currentRebootGeneration, + wantedFirmwareCheck, currentFirmwareCheck, minCpuCores, minMainMemoryAvailableGb, minDiskAvailableGb, fastDisk, bandwidth, ipAddresses, hardwareDivergence, hardwareFailureDescription, parentHostname); diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/noderepository/RealNodeRepository.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/noderepository/RealNodeRepository.java index e8efe10e505..9c25687fae8 100644 --- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/noderepository/RealNodeRepository.java +++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/noderepository/RealNodeRepository.java @@ -10,6 +10,7 @@ import com.yahoo.vespa.hosted.node.admin.configserver.noderepository.bindings.*; import com.yahoo.vespa.hosted.node.admin.util.PrefixLogger; import com.yahoo.vespa.hosted.provision.Node; +import java.time.Instant; import java.util.Collections; import java.util.List; import java.util.Map; @@ -190,6 +191,8 @@ public class RealNodeRepository implements NodeRepository { Optional.ofNullable(node.currentRestartGeneration), node.rebootGeneration, node.currentRebootGeneration, + Optional.ofNullable(node.wantedFirmwareCheck).map(Instant::ofEpochMilli), + Optional.ofNullable(node.currentFirmwareCheck).map(Instant::ofEpochMilli), node.minCpuCores, node.minMainMemoryAvailableGb, node.minDiskAvailableGb, @@ -220,6 +223,7 @@ public class RealNodeRepository implements NodeRepository { node.currentRebootGeneration = nodeAttributes.getRebootGeneration().orElse(null); node.vespaVersion = nodeAttributes.getVespaVersion().orElse(null); node.currentOsVersion = nodeAttributes.getCurrentOsVersion().orElse(null); + node.currentFirmwareCheck = nodeAttributes.getCurrentFirmwareCheck().map(Instant::toEpochMilli).orElse(null); node.hardwareDivergence = nodeAttributes.getHardwareDivergence().orElse(null); node.hardwareFailureDescription = nodeAttributes.getHardwareFailureDescription().orElse(null); node.wantToDeprovision = nodeAttributes.getWantToDeprovision().orElse(null); diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/noderepository/bindings/NodeRepositoryNode.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/noderepository/bindings/NodeRepositoryNode.java index fcfd552875a..e6e2b675f5c 100644 --- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/noderepository/bindings/NodeRepositoryNode.java +++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/noderepository/bindings/NodeRepositoryNode.java @@ -48,6 +48,10 @@ public class NodeRepositoryNode { public String currentOsVersion; @JsonProperty("wantedOsVersion") public String wantedOsVersion; + @JsonProperty("currentFirmwareCheck") + public Long currentFirmwareCheck; + @JsonProperty("wantedFirmwareCheck") + public Long wantedFirmwareCheck; @JsonProperty("failCount") public Integer failCount; @JsonProperty("fastDisk") @@ -103,6 +107,8 @@ public class NodeRepositoryNode { ", wantedVespaVersion='" + wantedVespaVersion + '\'' + ", currentOsVersion='" + currentOsVersion + '\'' + ", wantedOsVersion='" + wantedOsVersion + '\'' + + ", currentFirmwareCheck='" + currentFirmwareCheck + '\'' + + ", wantedFirmwareCheck='" + wantedFirmwareCheck + '\'' + ", failCount=" + failCount + ", fastDisk=" + fastDisk + ", bandwidth=" + bandwidth + -- cgit v1.2.3 From 0fc27437269b0346cd362227119cf82548404f0a Mon Sep 17 00:00:00 2001 From: Jon Marius Venstad Date: Thu, 24 Jan 2019 11:45:56 +0100 Subject: Support persistent output file for CommandLine --- .../admin/task/util/process/ChildProcess2Impl.java | 3 +- .../node/admin/task/util/process/CommandLine.java | 14 ++++++++ .../task/util/process/ProcessFactoryImpl.java | 33 +++++++++++------- .../admin/task/util/process/CommandLineTest.java | 8 +++++ .../task/util/process/ProcessFactoryImplTest.java | 40 ++++++++++++++++++++++ 5 files changed, 85 insertions(+), 13 deletions(-) diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/process/ChildProcess2Impl.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/process/ChildProcess2Impl.java index 6c8b15b10a6..2854dc55af8 100644 --- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/process/ChildProcess2Impl.java +++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/process/ChildProcess2Impl.java @@ -89,7 +89,8 @@ public class ChildProcess2Impl implements ChildProcess2 { @Override public void close() { try { - Files.delete(outputPath); + if ( ! commandLine.getOutputFile().isPresent()) + Files.delete(outputPath); } catch (Throwable t) { logger.log(LogLevel.WARNING, "Failed to delete " + outputPath, t); } diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/process/CommandLine.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/process/CommandLine.java index 940b3255766..8f39c2d257b 100644 --- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/process/CommandLine.java +++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/process/CommandLine.java @@ -6,12 +6,14 @@ import com.yahoo.vespa.hosted.node.admin.component.TaskContext; import java.nio.charset.Charset; import java.nio.charset.StandardCharsets; +import java.nio.file.Path; import java.time.Duration; import java.util.ArrayList; import java.util.Arrays; import java.util.Collection; import java.util.Collections; import java.util.List; +import java.util.Optional; import java.util.function.Predicate; import java.util.logging.Logger; import java.util.regex.Pattern; @@ -51,6 +53,7 @@ public class CommandLine { private boolean redirectStderrToStdoutInsteadOfDiscard = true; private boolean executeSilentlyCalled = false; + private Optional outputFile = Optional.empty(); private Charset outputEncoding = StandardCharsets.UTF_8; private Duration timeout = DEFAULT_TIMEOUT; private long maxOutputBytes = DEFAULT_MAX_OUTPUT_BYTES; @@ -197,6 +200,16 @@ public class CommandLine { return this; } + /** + * By default, the output of the command is piped to a temporary file, which is deleted + * when execution ends. This method will cause output to be piped to the given path + * instead, and the file will not be removed. + */ + public CommandLine setOutputFile(Path outputFile) { + this.outputFile = Optional.of(outputFile); + return this; + } + /** * By default, the command will be gracefully killed after DEFAULT_TIMEOUT. This method * overrides that default. @@ -235,6 +248,7 @@ public class CommandLine { // Accessor fields necessary for classes in this package. Could be public if necessary. boolean getRedirectStderrToStdoutInsteadOfDiscard() { return redirectStderrToStdoutInsteadOfDiscard; } Predicate getSuccessfulExitCodePredicate() { return successfulExitCodePredicate; } + Optional getOutputFile() { return outputFile; } Charset getOutputEncoding() { return outputEncoding; } Duration getTimeout() { return timeout; } long getMaxOutputBytes() { return maxOutputBytes; } diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/process/ProcessFactoryImpl.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/process/ProcessFactoryImpl.java index 78c25897e1d..a5f8e667ff2 100644 --- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/process/ProcessFactoryImpl.java +++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/process/ProcessFactoryImpl.java @@ -48,7 +48,7 @@ public class ProcessFactoryImpl implements ProcessFactory { processBuilder.redirectError(ProcessBuilder.Redirect.to(DEV_NULL)); } - // The output is redirected to a temporary file because: + // The output is redirected to a file (temporary or user-defined) because: // - We could read continuously from process.getInputStream, but that may block // indefinitely with a faulty program. // - If we don't read continuously from process.getInputStream, then because @@ -60,27 +60,36 @@ public class ProcessFactoryImpl implements ProcessFactory { // has the benefit of allowing for inspection of the file during execution, and // allowing the inspection of the file if it e.g. gets too large to hold in-memory. - String temporaryFilePrefix = - ProcessFactoryImpl.class.getSimpleName() + "-" + commandLine.programName() + "-"; - FileAttribute> fileAttribute = PosixFilePermissions.asFileAttribute( PosixFilePermissions.fromString("rw-------")); - Path temporaryFile = uncheck(() -> Files.createTempFile( - temporaryFilePrefix, - ".out", - fileAttribute)); + Path outputFile = commandLine.getOutputFile() + .map(file -> { + uncheck(() -> Files.deleteIfExists(file)); + uncheck(() -> Files.createFile(file, fileAttribute)); + return file; + }) + .orElseGet(() -> { + String temporaryFilePrefix = + ProcessFactoryImpl.class.getSimpleName() + "-" + commandLine.programName() + "-"; + + return uncheck(() -> Files.createTempFile( + temporaryFilePrefix, + ".out", + fileAttribute)); + }); try { - processBuilder.redirectOutput(temporaryFile.toFile()); + processBuilder.redirectOutput(outputFile.toFile()); ProcessApi2 process = processStarter.start(processBuilder); - return new ChildProcess2Impl(commandLine, process, temporaryFile, timer); + return new ChildProcess2Impl(commandLine, process, outputFile, timer); } catch (RuntimeException | Error throwable) { try { - Files.delete(temporaryFile); + if ( ! commandLine.getOutputFile().isPresent()) + Files.delete(outputFile); } catch (IOException ioException) { logger.log(LogLevel.WARNING, "Failed to delete temporary file at " + - temporaryFile, ioException); + outputFile, ioException); } throw throwable; } diff --git a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/task/util/process/CommandLineTest.java b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/task/util/process/CommandLineTest.java index a5eb0ab059b..1531b06070e 100644 --- a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/task/util/process/CommandLineTest.java +++ b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/task/util/process/CommandLineTest.java @@ -2,12 +2,19 @@ package com.yahoo.vespa.hosted.node.admin.task.util.process; import com.yahoo.vespa.hosted.node.admin.component.TestTaskContext; +import com.yahoo.vespa.hosted.node.admin.task.util.time.TestTimer; +import com.yahoo.vespa.test.file.TestFileSystem; import org.junit.After; import org.junit.Test; +import java.io.IOException; import java.nio.charset.StandardCharsets; +import java.nio.file.FileSystem; +import java.nio.file.Files; +import java.nio.file.Path; import java.util.Arrays; import java.util.List; +import java.util.Optional; import java.util.function.Predicate; import static org.junit.Assert.assertEquals; @@ -61,6 +68,7 @@ public class CommandLineTest { assertEquals(CommandLine.DEFAULT_SIGTERM_GRACE_PERIOD, commandLine.getSigTermGracePeriod()); assertEquals(CommandLine.DEFAULT_SIGKILL_GRACE_PERIOD, commandLine.getSigKillGracePeriod()); assertEquals(0, commandLine.getArguments().size()); + assertEquals(Optional.empty(), commandLine.getOutputFile()); assertEquals(StandardCharsets.UTF_8, commandLine.getOutputEncoding()); assertTrue(commandLine.getRedirectStderrToStdoutInsteadOfDiscard()); Predicate defaultExitCodePredicate = commandLine.getSuccessfulExitCodePredicate(); diff --git a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/task/util/process/ProcessFactoryImplTest.java b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/task/util/process/ProcessFactoryImplTest.java index 5a32b4c68b1..88d26103777 100644 --- a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/task/util/process/ProcessFactoryImplTest.java +++ b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/task/util/process/ProcessFactoryImplTest.java @@ -1,15 +1,24 @@ // Copyright 2018 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.vespa.hosted.node.admin.task.util.process; +import com.yahoo.vespa.hosted.node.admin.task.util.file.FileAttributes; import com.yahoo.vespa.hosted.node.admin.task.util.file.UnixPath; import com.yahoo.vespa.hosted.node.admin.task.util.time.TestTimer; import org.junit.Test; import org.mockito.ArgumentCaptor; +import java.io.IOException; +import java.nio.file.FileSystems; import java.nio.file.Files; import java.nio.file.Path; +import java.nio.file.attribute.FileAttribute; +import java.nio.file.attribute.PosixFilePermission; +import java.nio.file.attribute.PosixFilePermissions; import java.util.Arrays; +import java.util.Optional; +import java.util.Set; +import static com.yahoo.yolean.Exceptions.uncheck; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertTrue; @@ -45,4 +54,35 @@ public class ProcessFactoryImplTest { assertFalse(Files.exists(outputPath)); } + + @Test + public void testSpawnWithPersistentOutputFile() { + + class TemporaryFile implements AutoCloseable { + Path path; + TemporaryFile() { + String outputFileName = ProcessFactoryImplTest.class.getSimpleName() + "-temporary-test-file.out"; + FileAttribute> fileAttribute = PosixFilePermissions.asFileAttribute( + PosixFilePermissions.fromString("rw-------")); + path = uncheck(() -> Files.createTempFile(outputFileName, ".out", fileAttribute)); + } + @Override public void close() { uncheck(() -> Files.deleteIfExists(path)); } + } + + try (TemporaryFile outputPath = new TemporaryFile()) { + CommandLine commandLine = mock(CommandLine.class); + when(commandLine.getArguments()).thenReturn(Arrays.asList("program")); + when(commandLine.programName()).thenReturn("program"); + when(commandLine.getOutputFile()).thenReturn(Optional.of(outputPath.path)); + try (ChildProcess2Impl child = processFactory.spawn(commandLine)) { + assertEquals(outputPath.path, child.getOutputPath()); + assertTrue(Files.exists(outputPath.path)); + assertEquals("rw-------", new UnixPath(outputPath.path).getPermissions()); + } + + assertTrue(Files.exists(outputPath.path)); + } + + } + } \ No newline at end of file -- cgit v1.2.3