diff options
91 files changed, 1203 insertions, 636 deletions
diff --git a/config-provisioning/src/main/java/com/yahoo/config/provision/EndpointsChecker.java b/config-provisioning/src/main/java/com/yahoo/config/provision/EndpointsChecker.java index d9ced0177e5..c33a575e9b7 100644 --- a/config-provisioning/src/main/java/com/yahoo/config/provision/EndpointsChecker.java +++ b/config-provisioning/src/main/java/com/yahoo/config/provision/EndpointsChecker.java @@ -19,29 +19,37 @@ import java.util.Optional; */ public interface EndpointsChecker { - record Endpoint(ClusterSpec.Id clusterName, + record Endpoint(ApplicationId applicationId, + ClusterSpec.Id clusterName, HttpURL url, Optional<InetAddress> ipAddress, Optional<DomainName> canonicalName, - boolean isPublic) { } + boolean isPublic, + CloudAccount account) { } /** Status sorted by increasing readiness. */ enum Status { endpointsUnavailable, containersUnhealthy, available } - record Availability(Status status, String message) { } + record Availability(Status status, String message) { + public static final Availability ready = new Availability(Status.available, "Endpoints are ready."); + } interface HostNameResolver { Optional<InetAddress> resolve(DomainName hostName); } interface CNameResolver { Optional<DomainName> resolve(DomainName hostName); } - interface ContainerHealthChecker { boolean healthy(Endpoint endpoint); } + interface HealthChecker { Availability healthy(Endpoint endpoint); } + + interface HealthCheckerProvider { + default HealthChecker getHealthChecker() { return __ -> Availability.ready; } + } - static EndpointsChecker of(ContainerHealthChecker containerHealthChecker) { - return zoneEndpoints -> endpointsAvailable(zoneEndpoints, EndpointsChecker::resolveHostName, EndpointsChecker::resolveCname, containerHealthChecker); + static EndpointsChecker of(HealthChecker healthChecker) { + return zoneEndpoints -> endpointsAvailable(zoneEndpoints, EndpointsChecker::resolveHostName, EndpointsChecker::resolveCname, healthChecker); } - static EndpointsChecker mock(HostNameResolver hostNameResolver, CNameResolver cNameResolver, ContainerHealthChecker containerHealthChecker) { - return zoneEndpoints -> endpointsAvailable(zoneEndpoints, hostNameResolver, cNameResolver, containerHealthChecker); + static EndpointsChecker mock(HostNameResolver hostNameResolver, CNameResolver cNameResolver, HealthChecker healthChecker) { + return zoneEndpoints -> endpointsAvailable(zoneEndpoints, hostNameResolver, cNameResolver, healthChecker); } Availability endpointsAvailable(List<Endpoint> zoneEndpoints); @@ -49,7 +57,7 @@ public interface EndpointsChecker { private static Availability endpointsAvailable(List<Endpoint> zoneEndpoints, HostNameResolver hostNameResolver, CNameResolver cNameResolver, - ContainerHealthChecker containerHealthChecker) { + HealthChecker healthChecker) { if (zoneEndpoints.isEmpty()) return new Availability(Status.endpointsUnavailable, "Endpoints not yet ready."); @@ -89,11 +97,13 @@ public interface EndpointsChecker { } } - for (Endpoint endpoint : zoneEndpoints) - if ( ! containerHealthChecker.healthy(endpoint)) - return new Availability(Status.containersUnhealthy, "Failed to get enough healthy responses from " + endpoint.url()); - - return new Availability(Status.available, "Endpoints are ready"); + Availability availability = Availability.ready; + for (Endpoint endpoint : zoneEndpoints) { + Availability candidate = healthChecker.healthy(endpoint); + if (candidate.status.compareTo(availability.status) < 0) + availability = candidate; + } + return availability; } /** Returns the IP address of the given host name, if any. */ diff --git a/config-provisioning/src/main/java/com/yahoo/config/provision/NodeResources.java b/config-provisioning/src/main/java/com/yahoo/config/provision/NodeResources.java index 8004d4dc951..9ca10091129 100644 --- a/config-provisioning/src/main/java/com/yahoo/config/provision/NodeResources.java +++ b/config-provisioning/src/main/java/com/yahoo/config/provision/NodeResources.java @@ -117,7 +117,7 @@ public class NodeResources { } public boolean isDefault() { return this == getDefault(); } - public static Architecture getDefault() { return x86_64; } + public static Architecture getDefault() { return any; } } @@ -498,7 +498,7 @@ public class NodeResources { if (cpu == 0) cpu = 0.5; if (cpu == 2 && mem == 8 ) cpu = 1.5; if (cpu == 2 && mem == 12 ) cpu = 2.3; - return new NodeResources(cpu, mem, dsk, 0.3, DiskSpeed.getDefault(), StorageType.getDefault(), Architecture.x86_64); + return new NodeResources(cpu, mem, dsk, 0.3, DiskSpeed.getDefault(), StorageType.getDefault(), Architecture.any); } private static double validate(double value, String valueName) { diff --git a/config-provisioning/src/test/java/com/yahoo/config/provision/NodeResourcesTest.java b/config-provisioning/src/test/java/com/yahoo/config/provision/NodeResourcesTest.java index 9351254034e..ae052c03556 100644 --- a/config-provisioning/src/test/java/com/yahoo/config/provision/NodeResourcesTest.java +++ b/config-provisioning/src/test/java/com/yahoo/config/provision/NodeResourcesTest.java @@ -20,11 +20,11 @@ public class NodeResourcesTest { @Test void testToString() { - assertEquals("[vcpu: 1.0, memory: 10.0 Gb, disk 100.0 Gb, architecture: x86_64]", + assertEquals("[vcpu: 1.0, memory: 10.0 Gb, disk 100.0 Gb, architecture: any]", new NodeResources(1., 10., 100., 0).toString()); - assertEquals("[vcpu: 0.3, memory: 3.3 Gb, disk 33.3 Gb, bandwidth: 0.3 Gbps, architecture: x86_64]", + assertEquals("[vcpu: 0.3, memory: 3.3 Gb, disk 33.3 Gb, bandwidth: 0.3 Gbps, architecture: any]", new NodeResources(1 / 3., 10 / 3., 100 / 3., 0.3).toString()); - assertEquals("[vcpu: 0.7, memory: 9.0 Gb, disk 66.7 Gb, bandwidth: 0.7 Gbps, architecture: x86_64]", + assertEquals("[vcpu: 0.7, memory: 9.0 Gb, disk 66.7 Gb, bandwidth: 0.7 Gbps, architecture: any]", new NodeResources(2 / 3., 8.97, 200 / 3., 0.67).toString()); } diff --git a/configdefinitions/src/vespa/configserver.def b/configdefinitions/src/vespa/configserver.def index 5b8653d1a19..0f18298b434 100644 --- a/configdefinitions/src/vespa/configserver.def +++ b/configdefinitions/src/vespa/configserver.def @@ -15,7 +15,7 @@ zookeeper.barrierTimeout long default=360 zookeeperLocalhostAffinity bool default=true sessionLifetime long default=3600 # in seconds # How long to wait for all ZooKeeper servers to reach barrier after quorum has reached barrier. In seconds -barrierWaitForAllTimeout long default=1 +barrierWaitForAllTimeout long default=5 # Directories configModelPluginDir[] string diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/ApplicationRepository.java b/configserver/src/main/java/com/yahoo/vespa/config/server/ApplicationRepository.java index 955b1bc8f4f..81de2e06b6c 100644 --- a/configserver/src/main/java/com/yahoo/vespa/config/server/ApplicationRepository.java +++ b/configserver/src/main/java/com/yahoo/vespa/config/server/ApplicationRepository.java @@ -21,7 +21,10 @@ import com.yahoo.config.provision.ApplicationTransaction; import com.yahoo.config.provision.Capacity; import com.yahoo.config.provision.EndpointsChecker; import com.yahoo.config.provision.EndpointsChecker.Availability; +import com.yahoo.config.provision.EndpointsChecker.HealthCheckerProvider; +import com.yahoo.config.provision.EndpointsChecker.HealthChecker; import com.yahoo.config.provision.EndpointsChecker.Endpoint; +import com.yahoo.config.provision.EndpointsChecker.Status; import com.yahoo.config.provision.Environment; import com.yahoo.config.provision.HostFilter; import com.yahoo.config.provision.InfraDeployer; @@ -172,6 +175,8 @@ public class ApplicationRepository implements com.yahoo.config.provision.Deploye ConfigserverConfig configserverConfig, Orchestrator orchestrator, TesterClient testerClient, + Zone zone, + HealthCheckerProvider healthCheckers, Metric metric, SecretStore secretStore, FlagSource flagSource) { @@ -180,7 +185,7 @@ public class ApplicationRepository implements com.yahoo.config.provision.Deploye infraDeployerProvider.getInfraDeployer(), configConvergenceChecker, httpProxy, - createEndpointsChecker(configserverConfig), + createEndpointsChecker(configserverConfig, zone, healthCheckers.getHealthChecker()), configserverConfig, orchestrator, new LogRetriever(), @@ -1222,28 +1227,36 @@ public class ApplicationRepository implements com.yahoo.config.provision.Deploye } - private static EndpointsChecker createEndpointsChecker(ConfigserverConfig config) { + private static EndpointsChecker createEndpointsChecker(ConfigserverConfig config, Zone zone, HealthChecker healthChecker) { CloseableHttpClient client = (SystemName.from(config.system()).isPublic() ? DefaultHttpClientBuilder.create(() -> null, "hosted-vespa-convergence-health-checker") : VespaHttpClientBuilder.custom().apacheBuilder().setUserAgent("hosted-vespa-convergence-health-checker")) .setDefaultHeaders(List.of(new BasicHeader(HttpHeaders.CONNECTION, "close"))) .build(); return EndpointsChecker.of(endpoint -> { + Availability health = healthChecker.healthy(endpoint); + if ( health.status() != Status.available // Unhealthy targets is the root cause, so return those details. + || endpoint.isPublic() // Controller checks /status.html on its own. + || endpoint.account().isEnclave(zone)) // Private endpoints in enclave are not reachable by us. + return health; + int remainingFailures = 3; - int remainingSuccesses = 100; + int remainingSuccesses = 10; while (remainingSuccesses > 0 && remainingFailures > 0) { try { if (client.execute(new HttpGet(endpoint.url().withPath(parse("/status.html")).asURI()), response -> response.getCode() == 200)) remainingSuccesses--; - else remainingFailures--; + else + throw new IOException("got non-200 status code"); } catch (Exception e) { log.log(Level.FINE, e, () -> "Failed to check " + endpoint + "status.html: " + e.getMessage()); - remainingFailures--; + if (--remainingFailures == 0) + return new Availability(Status.containersUnhealthy, "Failed to get enough healthy responses from " + endpoint.url()); } } - return remainingSuccesses == 0; + return Availability.ready; }); } diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/HealthCheckerProviderProvider.java b/configserver/src/main/java/com/yahoo/vespa/config/server/HealthCheckerProviderProvider.java new file mode 100644 index 00000000000..2d54f256a05 --- /dev/null +++ b/configserver/src/main/java/com/yahoo/vespa/config/server/HealthCheckerProviderProvider.java @@ -0,0 +1,19 @@ +package com.yahoo.vespa.config.server; + +import com.yahoo.config.provision.EndpointsChecker.HealthCheckerProvider; +import com.yahoo.container.di.componentgraph.Provider; + +/** + * Default stub for container health checker, overridden by node-repository when that is present. + * + * @author jonmv + */ +public class HealthCheckerProviderProvider implements Provider<HealthCheckerProvider> { + + @Override + public HealthCheckerProvider get() { return new HealthCheckerProvider() { }; } + + @Override + public void deconstruct() { } + +} diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/deploy/Deployment.java b/configserver/src/main/java/com/yahoo/vespa/config/server/deploy/Deployment.java index 66a5bc5a023..e135bc7f0e2 100644 --- a/configserver/src/main/java/com/yahoo/vespa/config/server/deploy/Deployment.java +++ b/configserver/src/main/java/com/yahoo/vespa/config/server/deploy/Deployment.java @@ -18,8 +18,6 @@ import com.yahoo.vespa.config.server.ApplicationRepository; import com.yahoo.vespa.config.server.ApplicationRepository.ActionTimer; import com.yahoo.vespa.config.server.ApplicationRepository.Activation; import com.yahoo.vespa.config.server.TimeoutBudget; -import com.yahoo.vespa.config.server.application.Application; -import com.yahoo.vespa.config.server.application.ConfigConvergenceChecker; import com.yahoo.vespa.config.server.application.ConfigNotConvergedException; import com.yahoo.vespa.config.server.configchange.ConfigChangeActions; import com.yahoo.vespa.config.server.configchange.ReindexActions; @@ -188,28 +186,44 @@ public class Deployment implements com.yahoo.config.provision.Deployment { private void waitForConfigToConverge(ApplicationId applicationId) { deployLogger.log(Level.INFO, "Wait for all services to use new config generation before restarting"); - while (true) { - try { - params.get().getTimeoutBudget().assertNotTimedOut( - () -> "Timeout exceeded while waiting for config convergence for " + applicationId); - } catch (UncheckedTimeoutException e) { - throw new ConfigNotConvergedException(e); - } + var convergenceChecker = applicationRepository.configConvergenceChecker(); + var app = applicationRepository.getActiveApplication(applicationId); - ConfigConvergenceChecker convergenceChecker = applicationRepository.configConvergenceChecker(); - Application app = applicationRepository.getActiveApplication(applicationId); - ServiceListResponse response = convergenceChecker.checkConvergenceUnlessDeferringChangesUntilRestart(app); + ServiceListResponse response = null; + while (timeLeft(applicationId, response)) { + response = convergenceChecker.checkConvergenceUnlessDeferringChangesUntilRestart(app); if (response.converged) { deployLogger.log(Level.INFO, "Services converged on new config generation " + response.currentGeneration); return; } else { - deployLogger.log(Level.INFO, "Services did not converge on new config generation " + - response.wantedGeneration + ", current generation: " + response.currentGeneration + ", will retry"); + deployLogger.log(Level.INFO, "Services that did not converge on new config generation " + + response.wantedGeneration + ": " + + servicesNotConvergedFormatted(response) + ". Will retry"); try { Thread.sleep(5_000); } catch (InterruptedException e) { /* ignore */ } } } } + private boolean timeLeft(ApplicationId applicationId, ServiceListResponse response) { + try { + params.get().getTimeoutBudget().assertNotTimedOut( + () -> "Timeout exceeded while waiting for config convergence for " + applicationId + + ", wanted generation " + response.wantedGeneration + ", these services had another generation: " + + servicesNotConvergedFormatted(response)); + } catch (UncheckedTimeoutException e) { + throw new ConfigNotConvergedException(e); + } + return true; + } + + private String servicesNotConvergedFormatted(ServiceListResponse response) { + return response.services().stream() + .filter(service -> service.currentGeneration != response.wantedGeneration) + .map(service -> service.serviceInfo.getHostName() + ":" + service.serviceInfo.getServiceName() + + " on generation " + service.currentGeneration) + .collect(Collectors.joining(", ")); + } + private void storeReindexing(ApplicationId applicationId, long requiredSession) { applicationRepository.modifyReindexing(applicationId, reindexing -> { if (configChangeActions != null) diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/http/v2/ApplicationHandler.java b/configserver/src/main/java/com/yahoo/vespa/config/server/http/v2/ApplicationHandler.java index 62a1704b350..9a6e4632071 100644 --- a/configserver/src/main/java/com/yahoo/vespa/config/server/http/v2/ApplicationHandler.java +++ b/configserver/src/main/java/com/yahoo/vespa/config/server/http/v2/ApplicationHandler.java @@ -10,6 +10,7 @@ import com.yahoo.config.application.api.ApplicationFile; import com.yahoo.config.model.api.Model; import com.yahoo.config.model.api.ServiceInfo; import com.yahoo.config.provision.ApplicationId; +import com.yahoo.config.provision.CloudAccount; import com.yahoo.config.provision.ClusterSpec; import com.yahoo.config.provision.EndpointsChecker.Availability; import com.yahoo.config.provision.EndpointsChecker.Endpoint; @@ -113,7 +114,7 @@ public class ApplicationHandler extends HttpHandler { public HttpResponse handlePOST(HttpRequest request) { Path path = new Path(request.getUri()); - if (path.matches("/application/v2/tenant/{tenant}/application/{application}/environment/{ignore}/region/{ignore}/instance/{instance}/verify-endpoints")) return verifyEndpoints(request); + if (path.matches("/application/v2/tenant/{tenant}/application/{application}/environment/{ignore}/region/{ignore}/instance/{instance}/verify-endpoints")) return verifyEndpoints(applicationId(path), request); if (path.matches("/application/v2/tenant/{tenant}/application/{application}/environment/{ignore}/region/{ignore}/instance/{instance}/reindex")) return triggerReindexing(applicationId(path), request); if (path.matches("/application/v2/tenant/{tenant}/application/{application}/environment/{ignore}/region/{ignore}/instance/{instance}/reindexing")) return enableReindexing(applicationId(path)); if (path.matches("/application/v2/tenant/{tenant}/application/{application}/environment/{ignore}/region/{ignore}/instance/{instance}/restart")) return restart(applicationId(path), request); @@ -332,17 +333,19 @@ public class ApplicationHandler extends HttpHandler { return new MessageResponse("Success"); } - private HttpResponse verifyEndpoints(HttpRequest request) { + private HttpResponse verifyEndpoints(ApplicationId applicationId, HttpRequest request) { byte[] data = uncheck(() -> request.getData().readAllBytes()); List<Endpoint> endpoints = new ArrayList<>(); SlimeUtils.jsonToSlime(data).get() .field("endpoints") .traverse((ArrayTraverser) (__, endpointObject) -> { - endpoints.add(new Endpoint(ClusterSpec.Id.from(endpointObject.field("clusterName").asString()), + endpoints.add(new Endpoint(applicationId, + ClusterSpec.Id.from(endpointObject.field("clusterName").asString()), HttpURL.from(URI.create(endpointObject.field("url").asString())), SlimeUtils.optionalString(endpointObject.field("ipAddress")).map(uncheck(InetAddress::getByName)), SlimeUtils.optionalString(endpointObject.field("canonicalName")).map(DomainName::of), - endpointObject.field("public").asBool())); + endpointObject.field("public").asBool(), + CloudAccount.from(endpointObject.field("account").asString()))); }); if (endpoints.isEmpty()) throw new IllegalArgumentException("No endpoints in request " + request); diff --git a/configserver/src/main/resources/configserver-app/services.xml b/configserver/src/main/resources/configserver-app/services.xml index bba7d9627dd..b6904467893 100644 --- a/configserver/src/main/resources/configserver-app/services.xml +++ b/configserver/src/main/resources/configserver-app/services.xml @@ -26,6 +26,7 @@ <component id="com.yahoo.vespa.config.server.tenant.TenantRepository" bundle="configserver" /> <component id="com.yahoo.vespa.config.server.host.HostRegistry" bundle="configserver" /> <component id="com.yahoo.vespa.config.server.ApplicationRepository" bundle="configserver" /> + <component id="com.yahoo.vespa.config.server.HealthCheckerProviderProvider" bundle="configserver" /> <component id="com.yahoo.vespa.config.server.version.VersionState" bundle="configserver" /> <component id="com.yahoo.config.provision.Zone" bundle="config-provisioning" /> <component id="com.yahoo.vespa.config.server.application.ConfigConvergenceChecker" bundle="configserver" /> diff --git a/configserver/src/test/java/com/yahoo/vespa/config/server/MockConfigConvergenceChecker.java b/configserver/src/test/java/com/yahoo/vespa/config/server/MockConfigConvergenceChecker.java index b4892caa05f..f335e978235 100644 --- a/configserver/src/test/java/com/yahoo/vespa/config/server/MockConfigConvergenceChecker.java +++ b/configserver/src/test/java/com/yahoo/vespa/config/server/MockConfigConvergenceChecker.java @@ -5,14 +5,26 @@ import com.yahoo.vespa.config.server.application.Application; import com.yahoo.vespa.config.server.application.ConfigConvergenceChecker; import java.time.Duration; +import java.util.HashMap; +import java.util.List; import java.util.Map; +import java.util.function.Function; +import java.util.stream.Collectors; public class MockConfigConvergenceChecker extends ConfigConvergenceChecker { private final long wantedGeneration; + private final List<ServiceInfo> servicesThatFailFirstIteration; + + private int iteration = 0; public MockConfigConvergenceChecker(long wantedGeneration) { + this(wantedGeneration, List.of()); + } + + public MockConfigConvergenceChecker(long wantedGeneration, List<ServiceInfo> servicesThatFailFirstIteration) { this.wantedGeneration = wantedGeneration; + this.servicesThatFailFirstIteration = servicesThatFailFirstIteration; } @Override @@ -32,7 +44,15 @@ public class MockConfigConvergenceChecker extends ConfigConvergenceChecker { @Override public ServiceListResponse checkConvergenceUnlessDeferringChangesUntilRestart(Application application) { - return new ServiceListResponse(Map.of(), wantedGeneration, wantedGeneration); + iteration++; + if (servicesThatFailFirstIteration.isEmpty() || iteration > 1) + return new ServiceListResponse(Map.of(), wantedGeneration, wantedGeneration); + + Map<ServiceInfo, Long> services = new HashMap<>(); + for (var service : servicesThatFailFirstIteration) { + services.put(service, wantedGeneration - 1); + } + return new ServiceListResponse(services, wantedGeneration, wantedGeneration - 1); } } diff --git a/configserver/src/test/java/com/yahoo/vespa/config/server/deploy/HostedDeployTest.java b/configserver/src/test/java/com/yahoo/vespa/config/server/deploy/HostedDeployTest.java index 842416615e2..58e76d5dc0a 100644 --- a/configserver/src/test/java/com/yahoo/vespa/config/server/deploy/HostedDeployTest.java +++ b/configserver/src/test/java/com/yahoo/vespa/config/server/deploy/HostedDeployTest.java @@ -22,9 +22,11 @@ import com.yahoo.config.provision.DockerImage; import com.yahoo.config.provision.Environment; import com.yahoo.config.provision.RegionName; import com.yahoo.config.provision.Zone; +import com.yahoo.slime.SlimeUtils; import com.yahoo.test.ManualClock; import com.yahoo.vespa.config.server.MockConfigConvergenceChecker; import com.yahoo.vespa.config.server.application.ApplicationReindexing; +import com.yahoo.vespa.config.server.application.ConfigConvergenceChecker; import com.yahoo.vespa.config.server.http.InternalServerException; import com.yahoo.vespa.config.server.http.InvalidApplicationException; import com.yahoo.vespa.config.server.http.UnknownVespaVersionException; @@ -42,7 +44,6 @@ import java.time.Clock; import java.time.Duration; import java.time.Instant; import java.util.Collections; -import java.util.HashMap; import java.util.List; import java.util.Map; import java.util.Optional; @@ -445,8 +446,7 @@ public class HostedDeployTest { @Test public void testThatConfigChangeActionsAreCollectedFromAllModels() { List<Host> hosts = createHosts(9, "6.1.0", "6.2.0"); - List<ServiceInfo> services = List.of( - new ServiceInfo("serviceName", "serviceType", null, new HashMap<>(), "configId", "hostName")); + List<ServiceInfo> services = createServices(); List<ModelFactory> modelFactories = List.of( new ConfigChangeActionsModelFactory(Version.fromString("6.1.0"), @@ -461,10 +461,41 @@ public class HostedDeployTest { } @Test + public void testConfigConvergenceBeforeRestart() { + List<Host> hosts = createHosts(9, "6.1.0", "6.2.0"); + List<ServiceInfo> services = createServices(); + + List<ModelFactory> modelFactories = List.of( + new ConfigChangeActionsModelFactory(Version.fromString("6.1.0"), + new VespaRestartAction(ClusterSpec.Id.from("test"), "change", services)), + new ConfigChangeActionsModelFactory(Version.fromString("6.2.0"), + new VespaRestartAction(ClusterSpec.Id.from("test"), "other change", services))); + + DeployTester tester = createTester(hosts, + modelFactories, + prodZone, + Clock.systemUTC(), + new MockConfigConvergenceChecker(2L, services)); + var result = tester.deployApp("src/test/apps/hosted/", "6.2.0"); + DeployHandlerLogger deployLogger = result.deployLogger(); + + System.out.println(SlimeUtils.toJson(deployLogger.slime().get())); + assertLogContainsMessage(deployLogger, "Wait for all services to use new config generation before restarting"); + assertLogContainsMessage(deployLogger, "Services that did not converge on new config generation 2: hostName:serviceName on generation 1, hostName2:serviceName2 on generation 1. Will retry"); + assertLogContainsMessage(deployLogger, "Services converged on new config generation 2"); + } + + private void assertLogContainsMessage(DeployHandlerLogger log, String message) { + assertEquals(1, SlimeUtils.entriesStream(log.slime().get().field("log")) + .map(entry -> entry.field("message").asString()) + .filter(m -> m.contains(message)) + .count()); + } + + @Test public void testThatAllowedConfigChangeActionsAreActedUpon() { List<Host> hosts = createHosts(9, "6.1.0"); - List<ServiceInfo> services = List.of( - new ServiceInfo("serviceName", "serviceType", null, Map.of("clustername", "cluster"), "configId", "hostName")); + List<ServiceInfo> services = createServices(); ManualClock clock = new ManualClock(Instant.EPOCH); List<ModelFactory> modelFactories = List.of( @@ -539,18 +570,40 @@ public class HostedDeployTest { return createTester(hosts, modelFactories, zone, Clock.systemUTC()); } - private DeployTester createTester(List<Host> hosts, List<ModelFactory> modelFactories, - Zone prodZone, Clock clock) { + private DeployTester createTester(List<Host> hosts, List<ModelFactory> modelFactories, Zone zone, Clock clock) { + return createTester(hosts, modelFactories, zone, clock, new MockConfigConvergenceChecker(2)); + } + + private DeployTester createTester(List<Host> hosts, + List<ModelFactory> modelFactories, + Zone zone, + Clock clock, + ConfigConvergenceChecker configConvergenceChecker) { return new DeployTester.Builder(temporaryFolder) .modelFactories(modelFactories) .clock(clock) - .zone(prodZone) + .zone(zone) .hostProvisioner(new InMemoryProvisioner(new Hosts(hosts), true, false)) - .configConvergenceChecker(new MockConfigConvergenceChecker(2)) - .hostedConfigserverConfig(prodZone) + .configConvergenceChecker(configConvergenceChecker) + .hostedConfigserverConfig(zone) .build(); } + private static List<ServiceInfo> createServices() { + return List.of(new ServiceInfo("serviceName", + "serviceType", + null, + Map.of("clustername", "cluster"), + "configId", + "hostName"), + new ServiceInfo("serviceName2", + "serviceType2", + null, + Map.of("clustername", "cluster"), + "configId", + "hostName2")); + } + private static class ConfigChangeActionsModelFactory extends TestModelFactory { private final List<ConfigChangeAction> actions; diff --git a/configserver/src/test/java/com/yahoo/vespa/config/server/http/v2/ApplicationHandlerTest.java b/configserver/src/test/java/com/yahoo/vespa/config/server/http/v2/ApplicationHandlerTest.java index e8c4d819c31..306ba6da6f9 100644 --- a/configserver/src/test/java/com/yahoo/vespa/config/server/http/v2/ApplicationHandlerTest.java +++ b/configserver/src/test/java/com/yahoo/vespa/config/server/http/v2/ApplicationHandlerTest.java @@ -10,6 +10,7 @@ import com.yahoo.config.model.api.PortInfo; import com.yahoo.config.model.api.ServiceInfo; import com.yahoo.config.provision.ApplicationId; import com.yahoo.config.provision.ApplicationName; +import com.yahoo.config.provision.CloudAccount; import com.yahoo.config.provision.ClusterSpec; import com.yahoo.config.provision.EndpointsChecker; import com.yahoo.config.provision.EndpointsChecker.Availability; @@ -511,11 +512,13 @@ public class ApplicationHandlerTest { @Test public void testVerifyEndpoints() { - expectedEndpoints = List.of(new Endpoint(ClusterSpec.Id.from("bluster"), + expectedEndpoints = List.of(new Endpoint(ApplicationId.defaultId(), + ClusterSpec.Id.from("bluster"), HttpURL.from(URI.create("https://bluster.tld:1234")), Optional.of(uncheck(() -> InetAddress.getByName("4.3.2.1"))), Optional.of(DomainName.of("fluster.tld")), - false)); + false, + CloudAccount.empty)); availability = new Availability(EndpointsChecker.Status.available, "Endpoints are ready"); ApplicationHandler handler = createApplicationHandler(); HttpRequest request = createTestRequest(toUrlPath(applicationId, Zone.defaultZone(), true) + "/verify-endpoints", diff --git a/container-search/src/main/java/com/yahoo/prelude/hitfield/RawBase64.java b/container-search/src/main/java/com/yahoo/prelude/hitfield/RawBase64.java index 485e2c9a8c3..6660f1bf712 100644 --- a/container-search/src/main/java/com/yahoo/prelude/hitfield/RawBase64.java +++ b/container-search/src/main/java/com/yahoo/prelude/hitfield/RawBase64.java @@ -10,7 +10,7 @@ import java.util.Objects; * @author baldersheim */ public class RawBase64 implements Comparable<RawBase64> { - private final static Base64.Encoder encoder = Base64.getEncoder().withoutPadding(); + private final static Base64.Encoder encoder = Base64.getEncoder(); private final byte[] content; public RawBase64(byte[] content) { Objects.requireNonNull(content); diff --git a/container-search/src/test/java/com/yahoo/search/grouping/result/GroupIdTestCase.java b/container-search/src/test/java/com/yahoo/search/grouping/result/GroupIdTestCase.java index 77ed858b14b..0443f507848 100644 --- a/container-search/src/test/java/com/yahoo/search/grouping/result/GroupIdTestCase.java +++ b/container-search/src/test/java/com/yahoo/search/grouping/result/GroupIdTestCase.java @@ -48,8 +48,8 @@ public class GroupIdTestCase { assertEquals("group:long:69", new LongId(69L).toString()); assertEquals("group:long_bucket:6:9", new LongBucketId(6L, 9L).toString()); assertEquals("group:null", new NullId().toString()); - assertEquals("group:raw:Bgk", new RawId(new byte[]{6, 9}).toString()); - assertEquals("group:raw_bucket:Bgk:CQY", new RawBucketId(new byte[]{6, 9}, new byte[]{9, 6}).toString()); + assertEquals("group:raw:Bgk=", new RawId(new byte[]{6, 9}).toString()); + assertEquals("group:raw_bucket:Bgk=:CQY=", new RawBucketId(new byte[]{6, 9}, new byte[]{9, 6}).toString()); assertTrue(new RootId(0).toString().startsWith("group:root:")); assertEquals("group:string:69", new StringId("69").toString()); assertEquals("group:string_bucket:6:9", new StringBucketId("6", "9").toString()); diff --git a/container-search/src/test/java/com/yahoo/search/grouping/vespa/ResultBuilderTestCase.java b/container-search/src/test/java/com/yahoo/search/grouping/vespa/ResultBuilderTestCase.java index b0b48bb8731..4d52f37c751 100644 --- a/container-search/src/test/java/com/yahoo/search/grouping/vespa/ResultBuilderTestCase.java +++ b/container-search/src/test/java/com/yahoo/search/grouping/vespa/ResultBuilderTestCase.java @@ -60,12 +60,12 @@ public class ResultBuilderTestCase { assertGroupId("group:6.9", new FloatResultNode(6.9)); assertGroupId("group:69", new IntegerResultNode(69)); assertGroupId("group:null", new NullResultNode()); - assertGroupId("group:Bgk", new RawResultNode(new byte[]{6, 9})); + assertGroupId("group:Bgk=", new RawResultNode(new byte[]{6, 9})); assertGroupId("group:a", new StringResultNode("a")); assertGroupId("group:6.9:9.6", new FloatBucketResultNode(6.9, 9.6)); assertGroupId("group:6:9", new IntegerBucketResultNode(6, 9)); assertGroupId("group:a:b", new StringBucketResultNode("a", "b")); - assertGroupId("group:Bgk:CQY", new RawBucketResultNode(new RawResultNode(new byte[]{6, 9}), + assertGroupId("group:Bgk=:CQY=", new RawBucketResultNode(new RawResultNode(new byte[]{6, 9}), new RawResultNode(new byte[]{9, 6}))); } @@ -92,7 +92,7 @@ public class ResultBuilderTestCase { assertResult("69", new MinAggregationResult(new IntegerResultNode(69))); assertResult("69.3", new MinAggregationResult(new FloatResultNode(69.3))); assertResult("69.6", new MinAggregationResult(new StringResultNode("69.6"))); - assertResult("Bgk", new MinAggregationResult(new RawResultNode(new byte[]{6,9}))); + assertResult("Bgk=", new MinAggregationResult(new RawResultNode(new byte[]{6,9}))); } @Test diff --git a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/stubs/MockTesterCloud.java b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/stubs/MockTesterCloud.java index e29e8086c80..c148afb9190 100644 --- a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/stubs/MockTesterCloud.java +++ b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/stubs/MockTesterCloud.java @@ -26,7 +26,7 @@ import static com.yahoo.vespa.hosted.controller.api.integration.deployment.Teste public class MockTesterCloud implements TesterCloud { private final NameService nameService; - private final EndpointsChecker endpointsChecker = EndpointsChecker.mock(this::resolveHostName, this::resolveCname, __ -> true); + private final EndpointsChecker endpointsChecker = EndpointsChecker.mock(this::resolveHostName, this::resolveCname, __ -> Availability.ready); private List<LogEntry> log = new ArrayList<>(); private Status status = NOT_STARTED; diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/Controller.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/Controller.java index c8ec38ec73b..fb437c3258b 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/Controller.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/Controller.java @@ -40,6 +40,7 @@ import com.yahoo.yolean.concurrent.Sleeper; import java.time.Clock; import java.time.Instant; import java.util.LinkedHashMap; +import java.util.List; import java.util.Map; import java.util.Objects; import java.util.Optional; @@ -50,6 +51,9 @@ import java.util.function.Predicate; import java.util.logging.Logger; import java.util.stream.Collectors; +import static java.util.stream.Collectors.toList; +import static java.util.stream.Collectors.toSet; + /** * API to the controller. This contains the object model of everything the controller cares about, mainly tenants and * applications. The object model is persisted to curator. @@ -179,13 +183,23 @@ public class Controller extends AbstractComponent { log.info("Changing system version from " + printableVersion(currentStatus.systemVersion()) + " to " + printableVersion(newStatus.systemVersion())); } + Set<Version> obsoleteVersions = currentStatus.versions().stream().map(VespaVersion::versionNumber).collect(toSet()); + for (VespaVersion version : newStatus.versions()) { + obsoleteVersions.remove(version.versionNumber()); + VespaVersion current = currentStatus.version(version.versionNumber()); + if (current == null) + log.info("New version " + version.versionNumber().toFullString() + " added"); + else if ( ! current.confidence().equals(version.confidence())) + log.info("Confidence for version " + version.versionNumber().toFullString() + + " changed from " + current.confidence() + " to " + version.confidence()); + } + for (Version version : obsoleteVersions) + log.info("Version " + version.toFullString() + " is obsolete, and will be forgotten"); + curator.writeVersionStatus(newStatus); - // Removes confidence overrides for versions that no longer exist in the system - removeConfidenceOverride(version -> newStatus.versions().stream() - .noneMatch(vespaVersion -> vespaVersion.versionNumber() - .equals(version))); + removeConfidenceOverride(obsoleteVersions::contains); } - + /** Returns the latest known version status. Calling this is free but the status may be slightly out of date. */ public VersionStatus readVersionStatus() { return curator.readVersionStatus(); } diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/InternalStepRunner.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/InternalStepRunner.java index 7aacd93813c..71ab1c4d7da 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/InternalStepRunner.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/InternalStepRunner.java @@ -356,7 +356,7 @@ public class InternalStepRunner implements StepRunner { } if (summary.converged()) { controller.jobController().locked(id, lockedRun -> lockedRun.withSummary(null)); - Availability availability = endpointsAvailable(id.application(), id.type().zone(), logger); + Availability availability = endpointsAvailable(id.application(), id.type().zone(), deployment.get(), logger); if (availability.status() == Status.available) { if (controller.routing().policies().processDnsChallenges(new DeploymentId(id.application(), id.type().zone()))) { logger.log("Installation succeeded!"); @@ -496,24 +496,26 @@ public class InternalStepRunner implements StepRunner { } } - private Availability endpointsAvailable(ApplicationId id, ZoneId zone, DualLogger logger) { - DeploymentId deployment = new DeploymentId(id, zone); - Map<ZoneId, List<Endpoint>> endpoints = controller.routing().readTestRunnerEndpointsOf(Set.of(deployment)); + private Availability endpointsAvailable(ApplicationId id, ZoneId zone, Deployment deployment, DualLogger logger) { + DeploymentId deploymentId = new DeploymentId(id, zone); + Map<ZoneId, List<Endpoint>> endpoints = controller.routing().readTestRunnerEndpointsOf(Set.of(deploymentId)); logEndpoints(endpoints, logger); - DeploymentRoutingContext context = controller.routing().of(deployment); + DeploymentRoutingContext context = controller.routing().of(deploymentId); boolean resolveEndpoints = context.routingMethod() == RoutingMethod.exclusive; return controller.serviceRegistry().testerCloud().verifyEndpoints( - deployment, + deploymentId, endpoints.getOrDefault(zone, List.of()) .stream() .map(endpoint -> { ClusterSpec.Id cluster = ClusterSpec.Id.from(endpoint.name()); RoutingPolicy policy = context.routingPolicy(cluster).get(); - return new EndpointsChecker.Endpoint(cluster, + return new EndpointsChecker.Endpoint(id, + cluster, HttpURL.from(endpoint.url()), policy.ipAddress().filter(__ -> resolveEndpoints).map(uncheck(InetAddress::getByName)), policy.canonicalName().filter(__ -> resolveEndpoints), - policy.isPublic()); + policy.isPublic(), + deployment.cloudAccount()); }).toList()); } diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiHandler.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiHandler.java index 49186be2089..2a6d62672f3 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiHandler.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiHandler.java @@ -52,8 +52,6 @@ import com.yahoo.slime.SlimeUtils; import com.yahoo.text.Text; import com.yahoo.vespa.athenz.api.OAuthCredentials; import com.yahoo.vespa.athenz.client.zms.ZmsClientException; -import com.yahoo.vespa.flags.BooleanFlag; -import com.yahoo.vespa.flags.Flags; import com.yahoo.vespa.hosted.controller.Application; import com.yahoo.vespa.hosted.controller.Controller; import com.yahoo.vespa.hosted.controller.Instance; @@ -167,7 +165,8 @@ import java.util.stream.Stream; import static com.yahoo.jdisc.Response.Status.BAD_REQUEST; import static com.yahoo.jdisc.Response.Status.CONFLICT; -import static com.yahoo.vespa.flags.FetchVector.Dimension.APPLICATION_ID; +import static com.yahoo.vespa.hosted.controller.api.application.v4.EnvironmentResource.APPLICATION_TEST_ZIP; +import static com.yahoo.vespa.hosted.controller.api.application.v4.EnvironmentResource.APPLICATION_ZIP; import static com.yahoo.yolean.Exceptions.uncheck; import static java.util.Comparator.comparingInt; import static java.util.Map.Entry.comparingByKey; @@ -189,7 +188,6 @@ public class ApplicationApiHandler extends AuditLoggingRequestHandler { private final Controller controller; private final AccessControlRequests accessControlRequests; private final TestConfigSerializer testConfigSerializer; - private final BooleanFlag failDeploymentOnMissingCertificateFile; @Inject public ApplicationApiHandler(ThreadedHttpRequestHandler.Context parentCtx, @@ -199,7 +197,6 @@ public class ApplicationApiHandler extends AuditLoggingRequestHandler { this.controller = controller; this.accessControlRequests = accessControlRequests; this.testConfigSerializer = new TestConfigSerializer(controller.system()); - this.failDeploymentOnMissingCertificateFile = Flags.FAIL_DEPLOYMENT_ON_MISSING_CERTIFICATE_FILE.bindTo(controller.flagSource()); } @Override @@ -2435,7 +2432,7 @@ public class ApplicationApiHandler extends AuditLoggingRequestHandler { if ( ! dataParts.containsKey("applicationZip")) throw new IllegalArgumentException("Missing required form part 'applicationZip'"); - ApplicationPackage applicationPackage = new ApplicationPackage(dataParts.get(EnvironmentResource.APPLICATION_ZIP)); + ApplicationPackage applicationPackage = new ApplicationPackage(dataParts.get(APPLICATION_ZIP)); controller.applications().verifyApplicationIdentityConfiguration(id.tenant(), Optional.of(id.instance()), Optional.of(type.zone()), @@ -3071,14 +3068,8 @@ public class ApplicationApiHandler extends AuditLoggingRequestHandler { }); ApplicationPackage applicationPackage = - new ApplicationPackage(dataParts.get(EnvironmentResource.APPLICATION_ZIP), - true, - controller.system().isPublic() && - failDeploymentOnMissingCertificateFile - .with(APPLICATION_ID, ApplicationId.from(tenant, application, "default").serializedForm()) - .value()); - - byte[] testPackage = dataParts.getOrDefault(EnvironmentResource.APPLICATION_TEST_ZIP, new byte[0]); + new ApplicationPackage(dataParts.get(APPLICATION_ZIP), true, controller.system().isPublic()); + byte[] testPackage = dataParts.getOrDefault(APPLICATION_TEST_ZIP, new byte[0]); Submission submission = new Submission(applicationPackage, testPackage, sourceUrl, sourceRevision, authorEmail, description, risk); TenantName tenantName = TenantName.from(tenant); diff --git a/document/src/main/java/com/yahoo/document/json/JsonSerializationHelper.java b/document/src/main/java/com/yahoo/document/json/JsonSerializationHelper.java index 795f8e93187..7a4f178b8ef 100644 --- a/document/src/main/java/com/yahoo/document/json/JsonSerializationHelper.java +++ b/document/src/main/java/com/yahoo/document/json/JsonSerializationHelper.java @@ -47,7 +47,7 @@ import java.util.Map; */ public class JsonSerializationHelper { - private final static Base64.Encoder base64Encoder = Base64.getEncoder().withoutPadding(); // Important: _basic_ format + private final static Base64.Encoder base64Encoder = Base64.getEncoder(); // Important: _basic_ format static class JsonSerializationException extends RuntimeException { public JsonSerializationException(Exception base) { diff --git a/document/src/main/java/com/yahoo/document/serialization/XmlSerializationHelper.java b/document/src/main/java/com/yahoo/document/serialization/XmlSerializationHelper.java index d35693f785f..b2c92e6bb0b 100644 --- a/document/src/main/java/com/yahoo/document/serialization/XmlSerializationHelper.java +++ b/document/src/main/java/com/yahoo/document/serialization/XmlSerializationHelper.java @@ -31,10 +31,9 @@ import java.util.Map; * @author <a href="mailto:humbe@yahoo-inc.com">Håkon Humberset</a> */ @Deprecated -@SuppressWarnings("removal") public class XmlSerializationHelper { - private final static Base64.Encoder base64Encoder = Base64.getEncoder().withoutPadding(); + private final static Base64.Encoder base64Encoder = Base64.getEncoder(); public static void printArrayXml(Array array, XmlStream xml) { List<FieldValue> lst = array.getValues(); diff --git a/document/src/test/java/com/yahoo/document/DocumentTestCase.java b/document/src/test/java/com/yahoo/document/DocumentTestCase.java index 4470865b636..33b77cb1878 100644 --- a/document/src/test/java/com/yahoo/document/DocumentTestCase.java +++ b/document/src/test/java/com/yahoo/document/DocumentTestCase.java @@ -52,7 +52,7 @@ public class DocumentTestCase extends DocumentTestCaseBase { " <mailid>emailfromalicetobob&someone</mailid>\n" + " <date>-2013512400</date>\n" + " <attachmentcount>2</attachmentcount>\n" + - " <rawfield binaryencoding=\"base64\">AAECAwQFBgcICQoLDA0ODxAREhMUFRYXGBkaGxwdHh8gISIjJCUmJygpKissLS4vMDEyMzQ1Njc4OTo7PD0+P0BBQkNERUZHSElKS0xNTk9QUVJTVFVWV1hZWltcXV5fYGFiYw</rawfield>\n"; + " <rawfield binaryencoding=\"base64\">AAECAwQFBgcICQoLDA0ODxAREhMUFRYXGBkaGxwdHh8gISIjJCUmJygpKissLS4vMDEyMzQ1Njc4OTo7PD0+P0BBQkNERUZHSElKS0xNTk9QUVJTVFVWV1hZWltcXV5fYGFiYw==</rawfield>\n"; private static final String SERTEST_DOC_AS_XML_WEIGHT1 = " <weightedfield>\n" + diff --git a/document/src/test/java/com/yahoo/document/json/DocumentUpdateJsonSerializerTest.java b/document/src/test/java/com/yahoo/document/json/DocumentUpdateJsonSerializerTest.java index af7469de31b..08a5c9a124c 100644 --- a/document/src/test/java/com/yahoo/document/json/DocumentUpdateJsonSerializerTest.java +++ b/document/src/test/java/com/yahoo/document/json/DocumentUpdateJsonSerializerTest.java @@ -504,7 +504,7 @@ public class DocumentUpdateJsonSerializerTest { " 'update': 'DOCUMENT_ID',", " 'fields': {", " 'raw_field': {", - " 'assign': 'RG9uJ3QgYmVsaWV2ZSBoaXMgbGllcw'", + " 'assign': 'RG9uJ3QgYmVsaWV2ZSBoaXMgbGllcw=='", " }", " }", "}" diff --git a/document/src/test/java/com/yahoo/document/json/JsonWriterTestCase.java b/document/src/test/java/com/yahoo/document/json/JsonWriterTestCase.java index 4f15a2fe368..eab33afc3e4 100644 --- a/document/src/test/java/com/yahoo/document/json/JsonWriterTestCase.java +++ b/document/src/test/java/com/yahoo/document/json/JsonWriterTestCase.java @@ -291,7 +291,7 @@ public class JsonWriterTestCase { String payload = new String( new JsonStringEncoder().quoteAsString( "c3RyaW5nIGxvbmcgZW5vdWdoIHRvIGVtaXQgbW9yZSB0aGFuIDc2IGJhc2U2NCBjaGFyYWN0ZXJzIGFuZC" + - "B3aGljaCBzaG91bGQgY2VydGFpbmx5IG5vdCBiZSBuZXdsaW5lLWRlbGltaXRlZCE")); + "B3aGljaCBzaG91bGQgY2VydGFpbmx5IG5vdCBiZSBuZXdsaW5lLWRlbGltaXRlZCE=")); String docId = "id:unittest:testraw::whee"; diff --git a/documentapi/src/test/java/com/yahoo/documentapi/messagebus/protocol/test/Messages60TestCase.java b/documentapi/src/test/java/com/yahoo/documentapi/messagebus/protocol/test/Messages60TestCase.java index 22650fcdbf8..940217aa2b4 100644 --- a/documentapi/src/test/java/com/yahoo/documentapi/messagebus/protocol/test/Messages60TestCase.java +++ b/documentapi/src/test/java/com/yahoo/documentapi/messagebus/protocol/test/Messages60TestCase.java @@ -633,6 +633,8 @@ public class Messages60TestCase extends MessagesTestBase { @Override public void run() throws Exception { + test_result_with_match_features(); + Routable routable = deserialize("QueryResultMessage-1", DocumentProtocol.MESSAGE_QUERYRESULT, Language.CPP); assertTrue(routable instanceof QueryResultMessage); @@ -647,9 +649,11 @@ public class Messages60TestCase extends MessagesTestBase { com.yahoo.vdslib.SearchResult.Hit h = msg.getResult().getHit(0); assertEquals(89.0, h.getRank(), 1E-6); assertEquals("doc1", h.getDocId()); + assertFalse(h.getMatchFeatures().isPresent()); h = msg.getResult().getHit(1); assertEquals(109.0, h.getRank(), 1E-6); assertEquals("doc17", h.getDocId()); + assertFalse(h.getMatchFeatures().isPresent()); routable = deserialize("QueryResultMessage-3", DocumentProtocol.MESSAGE_QUERYRESULT, Language.CPP); assertTrue(routable instanceof QueryResultMessage); @@ -659,9 +663,11 @@ public class Messages60TestCase extends MessagesTestBase { h = msg.getResult().getHit(0); assertEquals(109.0, h.getRank(), 1E-6); assertEquals("doc17", h.getDocId()); + assertFalse(h.getMatchFeatures().isPresent()); h = msg.getResult().getHit(1); assertEquals(89.0, h.getRank(), 1E-6); assertEquals("doc1", h.getDocId()); + assertFalse(h.getMatchFeatures().isPresent()); routable = deserialize("QueryResultMessage-4", DocumentProtocol.MESSAGE_QUERYRESULT, Language.CPP); assertTrue(routable instanceof QueryResultMessage); @@ -673,32 +679,55 @@ public class Messages60TestCase extends MessagesTestBase { assertEquals(89.0, h.getRank(), 1E-6); assertEquals("doc1", h.getDocId()); byte[] b = ((SearchResult.HitWithSortBlob)h).getSortBlob(); - assertEquals(9, b.length); - byte[] e = { 's', 'o', 'r', 't', 'd', 'a', 't', 'a', '2' }; - for (int i = 0; i < b.length; i++) { - assertEquals(e[i], b[i]); - } + assertEqualsData(new byte[] { 's', 'o', 'r', 't', 'd', 'a', 't', 'a', '2' }, b); + h = msg.getResult().getHit(1); assertTrue(h instanceof SearchResult.HitWithSortBlob); assertEquals(109.0, h.getRank(), 1E-6); assertEquals("doc17", h.getDocId()); b = ((SearchResult.HitWithSortBlob)h).getSortBlob(); - assertEquals(9, b.length); - byte[] d = { 's', 'o', 'r', 't', 'd', 'a', 't', 'a', '1' }; - for (int i = 0; i < b.length; i++) { - assertEquals(d[i], b[i]); - } + assertEqualsData(new byte[] { 's', 'o', 'r', 't', 'd', 'a', 't', 'a', '1' }, b); + h = msg.getResult().getHit(2); assertTrue(h instanceof SearchResult.HitWithSortBlob); assertEquals(90.0, h.getRank(), 1E-6); assertEquals("doc18", h.getDocId()); b = ((SearchResult.HitWithSortBlob)h).getSortBlob(); - assertEquals(9, b.length); - byte[] c = { 's', 'o', 'r', 't', 'd', 'a', 't', 'a', '3' }; - for (int i = 0; i < b.length; i++) { - assertEquals(c[i], b[i]); + assertEqualsData(new byte[] { 's', 'o', 'r', 't', 'd', 'a', 't', 'a', '3' }, b); + } + + void assertEqualsData(byte[] exp, byte[] act) { + assertEquals(exp.length, act.length); + for (int i = 0; i < exp.length; ++i) { + assertEquals(exp[i], act[i]); } } + + void test_result_with_match_features() { + Routable routable = deserialize("QueryResultMessage-6", DocumentProtocol.MESSAGE_QUERYRESULT, Language.CPP); + assertTrue(routable instanceof QueryResultMessage); + + var msg = (QueryResultMessage)routable; + assertEquals(2, msg.getResult().getHitCount()); + + var h = msg.getResult().getHit(0); + assertTrue(h instanceof SearchResult.Hit); + assertEquals(7.0, h.getRank(), 1E-6); + assertEquals("doc2", h.getDocId()); + assertTrue(h.getMatchFeatures().isPresent()); + var mf = h.getMatchFeatures().get(); + assertEquals(12.0, mf.field("foo").asDouble(), 1E-6); + assertEqualsData(new byte[] { 'T', 'h', 'e', 'r', 'e' }, mf.field("bar").asData()); + + h = msg.getResult().getHit(1); + assertTrue(h instanceof SearchResult.Hit); + assertEquals(5.0, h.getRank(), 1E-6); + assertEquals("doc1", h.getDocId()); + assertTrue(h.getMatchFeatures().isPresent()); + mf = h.getMatchFeatures().get(); + assertEquals(1.0, mf.field("foo").asDouble(), 1E-6); + assertEqualsData(new byte[] { 'H', 'i' }, mf.field("bar").asData()); + } } public class testGetBucketListReply implements RunnableTest { diff --git a/documentapi/src/tests/messages/messages60test.cpp b/documentapi/src/tests/messages/messages60test.cpp index 89ab20373e7..58295ae2395 100644 --- a/documentapi/src/tests/messages/messages60test.cpp +++ b/documentapi/src/tests/messages/messages60test.cpp @@ -687,7 +687,7 @@ Messages60Test::testQueryResultMessage() sr3.set_match_features(FeatureValues(mf)); sr3.sort(); - EXPECT_EQUAL(MESSAGE_BASE_LENGTH + 123u, serialize("QueryResultMessage-6", qrm3)); + EXPECT_EQUAL(MESSAGE_BASE_LENGTH + 125u, serialize("QueryResultMessage-6", qrm3)); routable = deserialize("QueryResultMessage-6", DocumentProtocol::MESSAGE_QUERYRESULT, LANG_CPP); if (!EXPECT_TRUE(routable)) { return false; @@ -709,6 +709,10 @@ Messages60Test::testQueryResultMessage() EXPECT_EQUAL(2u, mfv.size()); EXPECT_EQUAL(1.0, mfv[0].as_double()); EXPECT_EQUAL("Hi", mfv[1].as_data().make_string()); + const auto& mf_names = dr->get_match_features().names; + EXPECT_EQUAL(2u, mf_names.size()); + EXPECT_EQUAL("foo", mf_names[0]); + EXPECT_EQUAL("bar", mf_names[1]); return true; } diff --git a/documentapi/test/crosslanguagefiles/6.221-cpp-QueryResultMessage-6.dat b/documentapi/test/crosslanguagefiles/6.221-cpp-QueryResultMessage-6.dat Binary files differindex 229441aa9ba..efe7f8546a9 100644 --- a/documentapi/test/crosslanguagefiles/6.221-cpp-QueryResultMessage-6.dat +++ b/documentapi/test/crosslanguagefiles/6.221-cpp-QueryResultMessage-6.dat diff --git a/flags/src/main/java/com/yahoo/vespa/flags/Flags.java b/flags/src/main/java/com/yahoo/vespa/flags/Flags.java index 08a3285c9b4..a600edc031f 100644 --- a/flags/src/main/java/com/yahoo/vespa/flags/Flags.java +++ b/flags/src/main/java/com/yahoo/vespa/flags/Flags.java @@ -216,7 +216,7 @@ public class Flags { public static final UnboundDoubleFlag CONTAINER_SHUTDOWN_TIMEOUT = defineDoubleFlag( "container-shutdown-timeout", 50.0, - List.of("baldersheim"), "2021-09-25", "2023-05-01", + List.of("baldersheim"), "2021-09-25", "2023-12-31", "Timeout for shutdown of a jdisc container", "Takes effect at redeployment", ZONE_ID, APPLICATION_ID); @@ -224,13 +224,13 @@ public class Flags { // TODO: Move to a permanent flag public static final UnboundListFlag<String> ALLOWED_ATHENZ_PROXY_IDENTITIES = defineListFlag( "allowed-athenz-proxy-identities", List.of(), String.class, - List.of("bjorncs", "tokle"), "2021-02-10", "2023-05-01", + List.of("bjorncs", "tokle"), "2021-02-10", "2023-08-01", "Allowed Athenz proxy identities", "takes effect at redeployment"); public static final UnboundIntFlag MAX_ACTIVATION_INHIBITED_OUT_OF_SYNC_GROUPS = defineIntFlag( "max-activation-inhibited-out-of-sync-groups", 0, - List.of("vekterli"), "2021-02-19", "2023-05-01", + List.of("vekterli"), "2021-02-19", "2023-09-01", "Allows replicas in up to N content groups to not be activated " + "for query visibility if they are out of sync with a majority of other replicas", "Takes effect at redeployment", @@ -238,7 +238,7 @@ public class Flags { public static final UnboundDoubleFlag MIN_NODE_RATIO_PER_GROUP = defineDoubleFlag( "min-node-ratio-per-group", 0.0, - List.of("geirst", "vekterli"), "2021-07-16", "2023-05-01", + List.of("geirst", "vekterli"), "2021-07-16", "2023-09-01", "Minimum ratio of nodes that have to be available (i.e. not Down) in any hierarchic content cluster group for the group to be Up", "Takes effect at redeployment", ZONE_ID, APPLICATION_ID); @@ -302,7 +302,7 @@ public class Flags { public static final UnboundBooleanFlag ENABLE_PROXY_PROTOCOL_MIXED_MODE = defineFeatureFlag( "enable-proxy-protocol-mixed-mode", true, - List.of("tokle"), "2022-05-09", "2023-04-30", + List.of("tokle"), "2022-05-09", "2023-08-01", "Enable or disable proxy protocol mixed mode", "Takes effect on redeployment", APPLICATION_ID); @@ -331,7 +331,7 @@ public class Flags { public static final UnboundBooleanFlag RESTRICT_DATA_PLANE_BINDINGS = defineFeatureFlag( "restrict-data-plane-bindings", false, - List.of("mortent"), "2022-09-08", "2023-05-01", + List.of("mortent"), "2022-09-08", "2023-08-01", "Use restricted data plane bindings", "Takes effect at redeployment", APPLICATION_ID); @@ -352,7 +352,7 @@ public class Flags { public static final UnboundStringFlag CORE_ENCRYPTION_PUBLIC_KEY_ID = defineStringFlag( "core-encryption-public-key-id", "", - List.of("vekterli"), "2022-11-03", "2023-05-01", + List.of("vekterli"), "2022-11-03", "2023-10-01", "Specifies which public key to use for core dump encryption.", "Takes effect on the next tick.", ZONE_ID, NODE_TYPE, HOSTNAME); @@ -366,18 +366,11 @@ public class Flags { public static final UnboundBooleanFlag VESPA_ATHENZ_PROVIDER = defineFeatureFlag( "vespa-athenz-provider", false, - List.of("mortent"), "2023-02-22", "2023-05-01", + List.of("mortent"), "2023-02-22", "2023-08-01", "Enable athenz provider in public systems", "Takes effect on next config server container start", ZONE_ID); - public static final UnboundLongFlag ZOOKEEPER_BARRIER_WAIT_FOR_ALL_TIMEOUT = defineLongFlag( - "zookeeper-barrier-wait-for-all-timeout", 5, - List.of("hmusum"), "2023-03-28", "2023-05-28", - "Time to wait for all barrier members after getting response from quorum number of member", - "Takes effect on next config server container start", - ZONE_ID); - public static final UnboundBooleanFlag NODE_ADMIN_TENANT_SERVICE_REGISTRY = defineFeatureFlag( "node-admin-tenant-service-registry", false, List.of("olaa"), "2023-04-12", "2023-06-12", @@ -398,7 +391,7 @@ public class Flags { ZONE_ID, APPLICATION_ID); public static final UnboundBooleanFlag FAIL_DEPLOYMENT_ON_MISSING_CERTIFICATE_FILE = defineFeatureFlag( - "fail-on-missing-certificate-file", false, List.of("hmusum"), "2023-04-21", "2023-05-21", + "fail-on-missing-certificate-file", true, List.of("hmusum"), "2023-04-21", "2023-05-21", "Whether to fail in controller when a submitted application package has no certificate files", "Takes effect at redeployment", ZONE_ID); diff --git a/jdisc_core/src/main/java/com/yahoo/jdisc/core/ScheduledQueue.java b/jdisc_core/src/main/java/com/yahoo/jdisc/core/ScheduledQueue.java index 09483a7c7e5..355d6aee43a 100644 --- a/jdisc_core/src/main/java/com/yahoo/jdisc/core/ScheduledQueue.java +++ b/jdisc_core/src/main/java/com/yahoo/jdisc/core/ScheduledQueue.java @@ -10,11 +10,10 @@ import java.util.Queue; */ class ScheduledQueue { - public static final int MILLIS_PER_SLOT = 100; - public static final int NUM_SLOTS = 512; - public static final int NUM_SLOTS_UNDILATED = 3; - public static final int SLOT_MASK = 511; // bitmask to modulo NUM_SLOTS - public static final int ITER_SHIFT = 9; // number of bits to shift off SLOT_MASK + public static final int MILLIS_PER_SLOT = 10; + public static final int NUM_SLOTS = 1024; + public static final int SLOT_MASK = NUM_SLOTS - 1; // bitmask to modulo NUM_SLOTS + public static final int ITER_SHIFT = Integer.numberOfTrailingZeros(NUM_SLOTS); // number of bits to shift off SLOT_MASK private final Entry[] slots = new Entry[NUM_SLOTS + 1]; private final int[] counts = new int[NUM_SLOTS + 1]; @@ -35,17 +34,16 @@ class ScheduledQueue { if (slots[NUM_SLOTS] == null && currentTimeMillis < nextTick) { return; } - int queueSize = queueSize() + out.size(); drainTo(NUM_SLOTS, 0, out); - for (int i = 0; currentTimeMillis >= nextTick && (queueSize > out.size()); i++, nextTick += MILLIS_PER_SLOT) { - if (i < NUM_SLOTS_UNDILATED) { - if (++currSlot >= NUM_SLOTS) { - currSlot = 0; - currIter++; - } - drainTo(currSlot, currIter, out); + while (currentTimeMillis >= nextTick) { + if (++currSlot >= NUM_SLOTS) { + currSlot = 0; + currIter++; } + drainTo(currSlot, currIter, out); + nextTick += MILLIS_PER_SLOT; } + } private void drainTo(int slot, int iter, Queue<Object> out) { diff --git a/jdisc_core/src/main/java/com/yahoo/jdisc/core/TimeoutManagerImpl.java b/jdisc_core/src/main/java/com/yahoo/jdisc/core/TimeoutManagerImpl.java index 7a3898b2946..4e651cb9013 100644 --- a/jdisc_core/src/main/java/com/yahoo/jdisc/core/TimeoutManagerImpl.java +++ b/jdisc_core/src/main/java/com/yahoo/jdisc/core/TimeoutManagerImpl.java @@ -2,6 +2,7 @@ package com.yahoo.jdisc.core; import com.google.inject.Inject; +import com.yahoo.concurrent.SystemTimer; import com.yahoo.jdisc.Request; import com.yahoo.jdisc.ResourceReference; import com.yahoo.jdisc.Response; @@ -14,6 +15,7 @@ import com.yahoo.jdisc.handler.RequestHandler; import com.yahoo.jdisc.handler.ResponseHandler; import java.nio.ByteBuffer; +import java.time.Duration; import java.util.LinkedList; import java.util.Queue; import java.util.concurrent.ThreadFactory; @@ -60,7 +62,7 @@ public class TimeoutManagerImpl { return new ManagedRequestHandler(handler); } - synchronized int queueSize() { return scheduler.queueSize(); } + int queueSize() { return scheduler.queueSize(); } Timer timer() { return timer; @@ -91,7 +93,7 @@ public class TimeoutManagerImpl { private class ManagerTask implements Runnable { - boolean oneMoreCheck(int timeoutMS) { + boolean oneMoreCheck(long timeoutMS) { synchronized (done) { if (!done.get()) { try { @@ -106,7 +108,9 @@ public class TimeoutManagerImpl { @Override public void run() { - while (oneMoreCheck(ScheduledQueue.MILLIS_PER_SLOT)) { + Duration desiredTimeout = Duration.ofMillis(ScheduledQueue.MILLIS_PER_SLOT); + Duration actualTimeout = SystemTimer.adjustTimeoutByDetectedHz(desiredTimeout); + while (oneMoreCheck(actualTimeout.toMillis())) { checkTasks(timer.currentTimeMillis()); } } diff --git a/jdisc_core/src/test/java/com/yahoo/jdisc/core/ScheduledQueueTestCase.java b/jdisc_core/src/test/java/com/yahoo/jdisc/core/ScheduledQueueTestCase.java index cd963caa8d2..b5eef973d5b 100644 --- a/jdisc_core/src/test/java/com/yahoo/jdisc/core/ScheduledQueueTestCase.java +++ b/jdisc_core/src/test/java/com/yahoo/jdisc/core/ScheduledQueueTestCase.java @@ -5,12 +5,10 @@ import org.junit.jupiter.api.Test; import java.util.Arrays; import java.util.LinkedList; -import java.util.List; import java.util.Queue; import static com.yahoo.jdisc.core.ScheduledQueue.MILLIS_PER_SLOT; import static com.yahoo.jdisc.core.ScheduledQueue.NUM_SLOTS; -import static com.yahoo.jdisc.core.ScheduledQueue.NUM_SLOTS_UNDILATED; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertTrue; import static org.junit.jupiter.api.Assertions.fail; @@ -95,7 +93,7 @@ public class ScheduledQueueTestCase { @Test void requireThatEntriesDoNotExpireMoreThanOnce() { ScheduledQueue queue = new ScheduledQueue(0); - Object foo = scheduleAt(queue, NUM_SLOTS * MILLIS_PER_SLOT + 50); + Object foo = scheduleAt(queue, NUM_SLOTS * MILLIS_PER_SLOT + MILLIS_PER_SLOT/2); long now = 0; for (int i = 0; i < NUM_SLOTS; ++i, now += MILLIS_PER_SLOT) { @@ -115,25 +113,6 @@ public class ScheduledQueueTestCase { assertDrainTo(queue, 0, foo); } - @Test - void requireThatDrainToPerformsTimeDilationWhenOverloaded() { - ScheduledQueue queue = new ScheduledQueue(0); - List<Object> payloads = new LinkedList<>(); - for (int i = 1; i <= NUM_SLOTS_UNDILATED + 1; ++i) { - payloads.add(scheduleAt(queue, i * MILLIS_PER_SLOT)); - } - - Queue<Object> expired = new LinkedList<>(); - long currentTimeMillis = payloads.size() * MILLIS_PER_SLOT; - queue.drainTo(currentTimeMillis, expired); - assertEquals(NUM_SLOTS_UNDILATED, expired.size()); - - expired = new LinkedList<>(); - currentTimeMillis += MILLIS_PER_SLOT; - queue.drainTo(currentTimeMillis, expired); - assertEquals(1, expired.size()); - } - private static Object scheduleAt(ScheduledQueue queue, long expireAtMillis) { Object obj = new Object(); queue.newEntry(obj).scheduleAt(expireAtMillis); diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/cgroup/Cgroup.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/cgroup/Cgroup.java new file mode 100644 index 00000000000..e40e3c6c003 --- /dev/null +++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/cgroup/Cgroup.java @@ -0,0 +1,163 @@ +// 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.cgroup; + +import com.yahoo.vespa.defaults.Defaults; +import com.yahoo.vespa.hosted.node.admin.component.TaskContext; +import com.yahoo.vespa.hosted.node.admin.container.ContainerId; +import com.yahoo.vespa.hosted.node.admin.task.util.file.UnixPath; + +import java.nio.file.FileSystem; +import java.nio.file.Path; +import java.util.HashMap; +import java.util.List; +import java.util.Map; +import java.util.Optional; +import java.util.function.Consumer; +import java.util.logging.Logger; + +/** + * Represents a cgroup in the control group v2 hierarchy, see + * <a href="https://www.kernel.org/doc/html/latest/admin-guide/cgroup-v2.html">Control Group v2</a>. + * + * @author hakonhall + */ +public class Cgroup { + private static final Logger logger = Logger.getLogger(Cgroup.class.getName()); + + private static Map<String, Consumer<UnixPath>> cgroupDirectoryCallbacks = new HashMap<>(); + + private final Path root; + private final Path relativePath; + + public static Cgroup root(FileSystem fileSystem) { + return new Cgroup(fileSystem.getPath("/sys/fs/cgroup"), fileSystem.getPath("")); + } + + private Cgroup(Path root, Path relativePath) { + this.root = root.normalize(); + this.relativePath = this.root.relativize(this.root.resolve(relativePath).normalize()); + if (this.relativePath.toString().equals("..") || this.relativePath.toString().startsWith("../")) { + throw new IllegalArgumentException("Invalid cgroup relative path: " + relativePath); + } + } + + /** Whether this cgroup actually exists in the kernel / on the file system. */ + public boolean exists() { return unixPath().resolve("cgroup.controllers").exists(); } + + /** Creates this cgroup if it does not already exist, and return this. */ + public Cgroup create() { + if (unixPath().createDirectory()) { + // cgroup automatically creates various files in a newly created cgroup directory. A unit test may simulate + // this by registering consumers before the test is run. + Consumer<UnixPath> callback = cgroupDirectoryCallbacks.get(relativePath.toString()); + if (callback != null) + callback.accept(unixPath()); + } + return this; + } + + /** Whether v2 cgroup is enabled on this host. */ + public boolean v2CgroupIsEnabled() { return resolveRoot().exists(); } + + /** + * Resolve the given path against the path of this cgroup, and return the resulting cgroup. + * If the given path is absolute, it is resolved against the root of the cgroup hierarchy. + */ + public Cgroup resolve(String path) { + Path effectivePath = fileSystem().getPath(path); + if (effectivePath.isAbsolute()) { + return new Cgroup(root, fileSystem().getPath("/").relativize(effectivePath)); + } else { + return new Cgroup(root, relativePath.resolve(path)); + } + } + + /** Returns the root cgroup, possibly this. */ + public Cgroup resolveRoot() { return isRoot() ? this : new Cgroup(root, fileSystem().getPath("")); } + + /** Returns the cgroup of a system service assuming this is the root, e.g. vespa-host-admin -> system.slice/vespa-host-admin.service. */ + public Cgroup resolveSystemService(String name) { return resolve("system.slice").resolve(serviceNameOf(name)); } + + /** Returns the root cgroup of the given Podman container. */ + public Cgroup resolveContainer(ContainerId containerId) { return resolve("/machine.slice/libpod-" + containerId + ".scope/container"); } + + /** Returns the root cgroup of the container, or otherwise the root cgroup. */ + public Cgroup resolveRoot(Optional<ContainerId> containerId) { return containerId.map(this::resolveContainer).orElseGet(this::resolveRoot); } + + /** Returns the absolute path to this cgroup. */ + public Path path() { return root.resolve(relativePath); } + + /** Returns the UnixPath of {@link #path()}. */ + public UnixPath unixPath() { return new UnixPath(path()); } + + public String read(String filename) { + return unixPath().resolve(filename).readUtf8File(); + } + + public Optional<String> readIfExists(String filename) { + return unixPath().resolve(filename).readUtf8FileIfExists().map(String::strip); + } + + public List<String> readLines(String filename) { + return unixPath().resolve(filename).readUtf8File().lines().toList(); + } + + public Optional<Integer> readIntIfExists(String filename) { + return unixPath().resolve(filename).readUtf8FileIfExists().map(String::strip).map(Integer::parseInt); + } + + public Size readSize(String filename) { return Size.from(read(filename).stripTrailing()); } + + public boolean convergeFileContent(TaskContext context, String filename, String content, boolean apply) { + UnixPath path = unixPath().resolve(filename); + String currentContent = path.readUtf8File(); + if (ensureSuffixNewline(currentContent).equals(ensureSuffixNewline(content))) return false; + + if (apply) { + context.recordSystemModification(logger, "Updating " + path + " from '" + currentContent.stripTrailing() + + "' to '" + content.stripTrailing() + "'"); + path.writeUtf8File(content); + } + return true; + } + + /** The kernel appears to append a newline if none exist, when writing to files in cgroupfs. */ + private static String ensureSuffixNewline(String content) { + return content.endsWith("\n") ? content : content + "\n"; + } + + /** Returns an instance representing core interface files (cgroup.* files). */ + public CgroupCore core() { return new CgroupCore(this); } + + /** Returns the CPU controller of this cgroup (cpu.* files). */ + public CpuController cpu() { return new CpuController(this); } + + /** Returns the memory controller of this cgroup (memory.* files). */ + public MemoryController memory() { return new MemoryController(this); } + + /** + * Wraps {@code command} to ensure it is executed in this cgroup. + * + * <p>WARNING: This method must be called only after vespa-cgexec has been installed.</p> + */ + public String[] wrapCommandForExecutionInCgroup(String... command) { + String[] fullCommand = new String[3 + command.length]; + fullCommand[0] = Defaults.getDefaults().vespaHome() + "/bin/vespa-cgexec"; + fullCommand[1] = "-g"; + fullCommand[2] = relativePath.toString(); + System.arraycopy(command, 0, fullCommand, 3, command.length); + return fullCommand; + } + + public static void unitTesting_atCgroupCreation(String relativePath, Consumer<UnixPath> callback) { + cgroupDirectoryCallbacks.put(relativePath, callback); + } + + private boolean isRoot() { return relativePath.toString().isEmpty(); } + + private static String serviceNameOf(String name) { + return name.indexOf('.') == -1 ? name + ".service" : name; + } + + private FileSystem fileSystem() { return root.getFileSystem(); } +} diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/cgroup/CgroupCore.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/cgroup/CgroupCore.java new file mode 100644 index 00000000000..68f27549e1b --- /dev/null +++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/cgroup/CgroupCore.java @@ -0,0 +1,34 @@ +// 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.cgroup; + +import java.util.List; + +/** + * Utility methods for accessing the cgroup core interface files, i.e. all cgroup.* files. + * + * @author hakonhall + */ +public class CgroupCore { + private final Cgroup cgroup; + + CgroupCore(Cgroup cgroup) { this.cgroup = cgroup; } + + public List<Integer> getPidsInCgroup() { + return cgroup.readLines("cgroup.procs") + .stream() + .map(Integer::parseInt) + .toList(); + } + + /** Whether the given PID is a member of this cgroup. */ + public boolean isMember(int pid) { + return getPidsInCgroup().contains(pid); + } + + /** Move the given PID to this cgroup, but return false if it was already a member. */ + public boolean addMember(int pid) { + if (isMember(pid)) return false; + cgroup.unixPath().resolve("cgroup.procs").writeUtf8File(Integer.toString(pid)); + return true; + } +} diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/cgroup/CpuController.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/cgroup/CpuController.java new file mode 100644 index 00000000000..c273a6237b4 --- /dev/null +++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/cgroup/CpuController.java @@ -0,0 +1,111 @@ +// 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.cgroup; + +import com.yahoo.collections.Pair; +import com.yahoo.vespa.hosted.node.admin.component.TaskContext; + +import java.util.Arrays; +import java.util.Map; +import java.util.Optional; +import java.util.stream.Collectors; + +import static java.lang.Integer.parseInt; + +/** + * Represents a cgroup v2 CPU controller, i.e. all cpu.* files. + * + * @author hakonhall + */ +public class CpuController { + private final Cgroup cgroup; + + CpuController(Cgroup cgroup) { + this.cgroup = cgroup; + } + + /** + * The maximum bandwidth limit of the format "QUOTA PERIOD", which indicates that the cgroup may consume + * up to QUOTA in each PERIOD duration. A quota of "max" indicates no limit. + */ + public record Max(Size quota, int period) { + public String toFileContent() { return quota + " " + period + '\n'; } + } + + /** + * Returns the maximum CPU usage, or empty if cgroup is not found. + * + * @see Max + */ + public Optional<Max> readMax() { + return cgroup.readIfExists("cpu.max") + .map(content -> { + String[] parts = content.strip().split(" "); + return new Max(Size.from(parts[0]), parseInt(parts[1])); + }); + } + + /** + * Update CPU quota and period for the given container ID. Set quota to -1 value for unlimited. + * + * @see #readMax() + * @see Max + */ + public boolean updateMax(TaskContext context, int quota, int period) { + Max max = new Max(quota < 0 ? Size.max() : Size.from(quota), period); + return cgroup.convergeFileContent(context, "cpu.max", max.toFileContent(), true); + } + + /** @return The weight in the range [1, 10000], or empty if not found. */ + private Optional<Integer> readWeight() { + return cgroup.readIntIfExists("cpu.weight"); + } + + /** @return The number of shares allocated to this cgroup for purposes of CPU time scheduling, or empty if not found. */ + public Optional<Integer> readShares() { + return readWeight().map(CpuController::weightToShares); + } + + public boolean updateShares(TaskContext context, int shares) { + return cgroup.convergeFileContent(context, "cpu.weight", sharesToWeight(shares) + "\n", true); + } + + // Must be same as in crun: https://github.com/containers/crun/blob/72c6e60ade0e4716fe2d8353f0d97d72cc8d1510/src/libcrun/cgroup.c#L3061 + // TODO: Migrate to weights + public static int sharesToWeight(int shares) { return (int) (1 + ((shares - 2L) * 9999) / 262142); } + public static int weightToShares(int weight) { return (int) (2 + ((weight - 1L) * 262142) / 9999); } + + public enum StatField { + TOTAL_USAGE_USEC("usage_usec"), + USER_USAGE_USEC("user_usec"), + SYSTEM_USAGE_USEC("system_usec"), + TOTAL_PERIODS("nr_periods"), + THROTTLED_PERIODS("nr_throttled"), + THROTTLED_TIME_USEC("throttled_usec"); + + private final String name; + + StatField(String name) { + this.name = name; + } + + long parseValue(String value) { + return Long.parseLong(value); + } + + static Optional<StatField> fromField(String fieldName) { + return Arrays.stream(values()) + .filter(field -> fieldName.equals(field.name)) + .findFirst(); + } + } + + public Map<StatField, Long> readStats() { + return cgroup.readLines("cpu.stat") + .stream() + .map(line -> line.split("\\s+")) + .filter(parts -> parts.length == 2) + .flatMap(parts -> StatField.fromField(parts[0]).stream().map(field -> new Pair<>(field, field.parseValue(parts[1])))) + .collect(Collectors.toMap(Pair::getFirst, Pair::getSecond)); + } + +} diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/cgroup/MemoryController.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/cgroup/MemoryController.java new file mode 100644 index 00000000000..840cd025917 --- /dev/null +++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/cgroup/MemoryController.java @@ -0,0 +1,48 @@ +// 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.cgroup; + +import java.util.List; +import java.util.Optional; + +/** + * Represents a cgroup v2 memory controller, i.e. all memory.* files. + * + * @author hakonhall + */ +public class MemoryController { + private final Cgroup cgroup; + + MemoryController(Cgroup cgroup) { + this.cgroup = cgroup; + } + + /** @return Maximum amount of memory that can be used by the cgroup and its descendants. */ + public Size readMax() { + return cgroup.readSize("memory.max"); + } + + /** @return The total amount of memory currently being used by the cgroup and its descendants, in bytes. */ + public Size readCurrent() { + return cgroup.readSize("memory.current"); + } + + /** @return The total amount of memory currently being used by the cgroup and its descendants, in bytes. */ + public Optional<Size> readCurrentIfExists() { + return cgroup.readIfExists("memory.current").map(Size::from); + } + + /** @return Number of bytes used to cache filesystem data, including tmpfs and shared memory. */ + public Size readFileSystemCache() { + return Size.from(readField(cgroup.readLines("memory.stat"), "file")); + } + + private static String readField(List<String> lines, String fieldName) { + return lines.stream() + .map(line -> line.split("\\s+")) + .filter(fields -> fields.length == 2) + .filter(fields -> fieldName.equals(fields[0])) + .map(fields -> fields[1]) + .findFirst() + .orElseThrow(() -> new IllegalArgumentException("No such field: " + fieldName)); + } +} diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/cgroup/Size.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/cgroup/Size.java new file mode 100644 index 00000000000..5e6ca7de8bd --- /dev/null +++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/cgroup/Size.java @@ -0,0 +1,67 @@ +// 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.cgroup; + +import java.util.Objects; + +/** + * Represents a number of bytes or possibly "max". + * + * @author hakonhall + */ +public class Size { + private static final String MAX = "max"; + + private final boolean max; + private final long value; + + public static Size max() { + return new Size(true, 0); + } + + public static Size from(long value) { + return new Size(false, value); + } + + public static Size from(String value) { + return value.equals(MAX) ? new Size(true, 0) : new Size(false, Long.parseLong(value)); + } + + private Size(boolean max, long value) { + this.max = max; + this.value = value; + } + + public boolean isMax() { + return max; + } + + /** Returns the value, i.e. the number of "bytes" if applicable. Throws if this is max. */ + public long value() { + if (max) throw new IllegalStateException("Value is max"); + return value; + } + + public String toFileContent() { return toString() + '\n'; } + + @Override + public String toString() { return max ? MAX : Long.toString(value); } + + public boolean isGreaterThan(Size that) { + if (that.max) return false; + if (this.max) return true; + return this.value > that.value; + } + + @Override + public boolean equals(Object o) { + if (this == o) return true; + if (o == null || getClass() != o.getClass()) return false; + Size size = (Size) o; + return max == size.max && value == size.value; + } + + @Override + public int hashCode() { + return Objects.hash(max, value); + } +} diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/cgroup/package-info.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/cgroup/package-info.java new file mode 100644 index 00000000000..c0a23166ccc --- /dev/null +++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/cgroup/package-info.java @@ -0,0 +1,9 @@ +// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. + +/** + * @author hakonhall + */ +@ExportPackage +package com.yahoo.vespa.hosted.node.admin.cgroup; + +import com.yahoo.osgi.annotation.ExportPackage;
\ No newline at end of file diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/container/CGroupV2.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/container/CGroupV2.java deleted file mode 100644 index 3cb34e066ff..00000000000 --- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/container/CGroupV2.java +++ /dev/null @@ -1,188 +0,0 @@ -// 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.container; - -import com.yahoo.collections.Pair; -import com.yahoo.vespa.defaults.Defaults; -import com.yahoo.vespa.hosted.node.admin.nodeagent.NodeAgentContext; -import com.yahoo.vespa.hosted.node.admin.task.util.file.UnixPath; - -import java.io.IOException; -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.Map; -import java.util.Optional; -import java.util.OptionalInt; -import java.util.logging.Logger; -import java.util.stream.Collectors; - -/** - * Read and write interface to the cgroup v2 of a Podman container. - * - * @see <a href="https://www.kernel.org/doc/html/latest/admin-guide/cgroup-v2.html">CGroups V2</a> - * @author freva - */ -public class CGroupV2 { - - private static final Logger logger = Logger.getLogger(CGroupV2.class.getName()); - private static final String MAX = "max"; - - private final Path rootCgroupPath; - - public CGroupV2(FileSystem fileSystem) { - this.rootCgroupPath = fileSystem.getPath("/sys/fs/cgroup"); - } - - /** - * Wraps {@code command} to ensure it is executed in the given cgroup. - * - * <p>WARNING: This method must be called only after vespa-cgexec has been installed.</p> - * - * @param cgroup The cgroup to execute the command in, e.g. /sys/fs/cgroup/system.slice/wireguard.scope. - * @param command The command to execute in the cgroup. - * @see #cgroupRootPath() - * @see #cgroupPath(ContainerId) - */ - public String[] wrapForExecutionIn(Path cgroup, String... command) { - String[] fullCommand = new String[3 + command.length]; - fullCommand[0] = Defaults.getDefaults().vespaHome() + "/bin/vespa-cgexec"; - fullCommand[1] = "-g"; - fullCommand[2] = cgroup.toString(); - System.arraycopy(command, 0, fullCommand, 3, command.length); - return fullCommand; - } - - /** - * Returns quota and period values used for CPU scheduling. This serves as hard cap on CPU usage by allowing - * the CGroupV2 to use up to {@code quota} each {@code period}. If uncapped, quota will be negative. - * - * @param containerId full container ID. - * @return CPU quota and period for the given container. Empty if CGroupV2 for this container is not found. - */ - public Optional<Pair<Integer, Integer>> cpuQuotaPeriod(ContainerId containerId) { - return cpuMaxPath(containerId).readUtf8FileIfExists() - .map(s -> { - String[] parts = s.strip().split(" "); - return new Pair<>(MAX.equals(parts[0]) ? -1 : Integer.parseInt(parts[0]), Integer.parseInt(parts[1])); - }); - } - - /** @return number of shares allocated to this CGroupV2 for purposes of CPU time scheduling, empty if CGroupV2 not found */ - public OptionalInt cpuShares(ContainerId containerId) { - return cpuWeightPath(containerId).readUtf8FileIfExists() - .map(s -> OptionalInt.of(weightToShares(Integer.parseInt(s.strip())))) - .orElseGet(OptionalInt::empty); - } - - /** Update CPU quota and period for the given container ID, set quota to -1 value for unlimited */ - public boolean updateCpuQuotaPeriod(NodeAgentContext context, ContainerId containerId, int cpuQuotaUs, int periodUs) { - String wanted = String.format("%s %d", cpuQuotaUs < 0 ? MAX : cpuQuotaUs, periodUs); - return writeCGroupsValue(context, cpuMaxPath(containerId), wanted); - } - - public boolean updateCpuShares(NodeAgentContext context, ContainerId containerId, int shares) { - return writeCGroupsValue(context, cpuWeightPath(containerId), Integer.toString(sharesToWeight(shares))); - } - - enum CpuStatField { - TOTAL_USAGE_USEC("usage_usec"), - USER_USAGE_USEC("user_usec"), - SYSTEM_USAGE_USEC("system_usec"), - TOTAL_PERIODS("nr_periods"), - THROTTLED_PERIODS("nr_throttled"), - THROTTLED_TIME_USEC("throttled_usec"); - - private final String name; - - CpuStatField(String name) { - this.name = name; - } - - long parseValue(String value) { - return Long.parseLong(value); - } - - static Optional<CpuStatField> fromField(String fieldName) { - return Arrays.stream(values()) - .filter(field -> fieldName.equals(field.name)) - .findFirst(); - } - } - - public Map<CpuStatField, Long> cpuStats(ContainerId containerId) throws IOException { - return Files.readAllLines(cgroupPath(containerId).resolve("cpu.stat")).stream() - .map(line -> line.split("\\s+")) - .filter(parts -> parts.length == 2) - .flatMap(parts -> CpuStatField.fromField(parts[0]).stream().map(field -> new Pair<>(field, field.parseValue(parts[1])))) - .collect(Collectors.toMap(Pair::getFirst, Pair::getSecond)); - } - - /** @return Maximum amount of memory that can be used by the cgroup and its descendants. */ - public long memoryLimitInBytes(ContainerId containerId) throws IOException { - String limit = Files.readString(cgroupPath(containerId).resolve("memory.max")).strip(); - return MAX.equals(limit) ? -1L : Long.parseLong(limit); - } - - /** @return The total amount of memory currently being used by the cgroup and its descendants. */ - public long memoryUsageInBytes(ContainerId containerId) throws IOException { - return parseLong(cgroupPath(containerId).resolve("memory.current")); - } - - /** @return Number of bytes used to cache filesystem data, including tmpfs and shared memory. */ - public long memoryCacheInBytes(ContainerId containerId) throws IOException { - return parseLong(cgroupPath(containerId).resolve("memory.stat"), "file"); - } - - /** Returns the cgroup v2 mount point path (/sys/fs/cgroup). */ - public Path cgroupRootPath() { - return rootCgroupPath; - } - - /** Returns the cgroup directory of the Podman container, and which appears as the root cgroup within the container. */ - public Path cgroupPath(ContainerId containerId) { - // crun path, runc path is without the 'container' directory - return rootCgroupPath.resolve("machine.slice/libpod-" + containerId + ".scope/container"); - } - - private UnixPath cpuMaxPath(ContainerId containerId) { - return new UnixPath(cgroupPath(containerId).resolve("cpu.max")); - } - - private UnixPath cpuWeightPath(ContainerId containerId) { - return new UnixPath(cgroupPath(containerId).resolve("cpu.weight")); - } - - private static boolean writeCGroupsValue(NodeAgentContext context, UnixPath unixPath, String value) { - String currentValue = unixPath.readUtf8File().strip(); - if (currentValue.equals(value)) return false; - - context.recordSystemModification(logger, "Updating " + unixPath + " from " + currentValue + " to " + value); - unixPath.writeUtf8File(value); - return true; - } - - // Must be same as in crun: https://github.com/containers/crun/blob/72c6e60ade0e4716fe2d8353f0d97d72cc8d1510/src/libcrun/cgroup.c#L3061 - static int sharesToWeight(int shares) { return (int) (1 + ((shares - 2L) * 9999) / 262142); } - static int weightToShares(int weight) { return (int) (2 + ((weight - 1L) * 262142) / 9999); } - - static long parseLong(Path path) throws IOException { - return Long.parseLong(Files.readString(path).trim()); - } - - static long parseLong(Path path, String fieldName) throws IOException { - return parseLong(Files.readAllLines(path), fieldName); - } - - static long parseLong(List<String> lines, String fieldName) { - for (String line : lines) { - String[] fields = line.split("\\s+"); - if (fields.length != 2) - throw new IllegalArgumentException("Expected line on the format 'key value', got: '" + line + "'"); - - if (fieldName.equals(fields[0])) return Long.parseLong(fields[1]); - } - throw new IllegalArgumentException("No such field: " + fieldName); - } -} diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/container/ContainerOperations.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/container/ContainerOperations.java index ce2a6bb22ac..264035b86a1 100644 --- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/container/ContainerOperations.java +++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/container/ContainerOperations.java @@ -2,6 +2,8 @@ package com.yahoo.vespa.hosted.node.admin.container; import com.yahoo.config.provision.DockerImage; +import com.yahoo.jdisc.Timer; +import com.yahoo.vespa.hosted.node.admin.cgroup.Cgroup; import com.yahoo.vespa.hosted.node.admin.component.TaskContext; import com.yahoo.vespa.hosted.node.admin.container.image.ContainerImageDownloader; import com.yahoo.vespa.hosted.node.admin.container.image.ContainerImagePruner; @@ -12,7 +14,6 @@ import com.yahoo.vespa.hosted.node.admin.task.util.process.CommandLine; import com.yahoo.vespa.hosted.node.admin.task.util.process.CommandResult; import java.nio.file.FileSystem; -import java.time.Clock; import java.time.Duration; import java.util.List; import java.util.Objects; @@ -33,10 +34,10 @@ public class ContainerOperations { private final ContainerImagePruner imagePruner; private final ContainerStatsCollector containerStatsCollector; - public ContainerOperations(ContainerEngine containerEngine, CGroupV2 cgroup, FileSystem fileSystem) { + public ContainerOperations(ContainerEngine containerEngine, Cgroup cgroup, FileSystem fileSystem, Timer timer) { this.containerEngine = Objects.requireNonNull(containerEngine); this.imageDownloader = new ContainerImageDownloader(containerEngine); - this.imagePruner = new ContainerImagePruner(containerEngine, Clock.systemUTC()); + this.imagePruner = new ContainerImagePruner(containerEngine, timer); this.containerStatsCollector = new ContainerStatsCollector(containerEngine, cgroup, fileSystem); } diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/container/ContainerStatsCollector.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/container/ContainerStatsCollector.java index 870809123a9..8244666f9e0 100644 --- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/container/ContainerStatsCollector.java +++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/container/ContainerStatsCollector.java @@ -1,6 +1,10 @@ // 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.container; +import com.yahoo.vespa.hosted.node.admin.cgroup.Cgroup; +import com.yahoo.vespa.hosted.node.admin.cgroup.CpuController; +import com.yahoo.vespa.hosted.node.admin.cgroup.Size; +import com.yahoo.vespa.hosted.node.admin.cgroup.MemoryController; import com.yahoo.vespa.hosted.node.admin.nodeagent.NodeAgentContext; import com.yahoo.vespa.hosted.node.admin.task.util.file.UnixUser; @@ -27,18 +31,18 @@ import java.util.stream.Stream; class ContainerStatsCollector { private final ContainerEngine containerEngine; - private final CGroupV2 cgroup; private final FileSystem fileSystem; + private final Cgroup rootCgroup; private final int onlineCpus; - ContainerStatsCollector(ContainerEngine containerEngine, CGroupV2 cgroup, FileSystem fileSystem) { - this(containerEngine, cgroup, fileSystem, Runtime.getRuntime().availableProcessors()); + ContainerStatsCollector(ContainerEngine containerEngine, Cgroup rootCgroup, FileSystem fileSystem) { + this(containerEngine, rootCgroup, fileSystem, Runtime.getRuntime().availableProcessors()); } - ContainerStatsCollector(ContainerEngine containerEngine, CGroupV2 cgroup, FileSystem fileSystem, int onlineCpus) { + ContainerStatsCollector(ContainerEngine containerEngine, Cgroup rootCgroup, FileSystem fileSystem, int onlineCpus) { this.containerEngine = Objects.requireNonNull(containerEngine); - this.cgroup = Objects.requireNonNull(cgroup); this.fileSystem = Objects.requireNonNull(fileSystem); + this.rootCgroup = Objects.requireNonNull(rootCgroup); this.onlineCpus = onlineCpus; } @@ -52,6 +56,10 @@ class ContainerStatsCollector { return Optional.of(new ContainerStats(networkStats, memoryStats, cpuStats, gpuStats)); } catch (NoSuchFileException ignored) { return Optional.empty(); // Container disappeared while we collected stats + } catch (UncheckedIOException e) { + if (e.getCause() != null && e.getCause() instanceof NoSuchFileException) + return Optional.empty(); + throw e; } catch (IOException e) { throw new UncheckedIOException(e); } @@ -83,21 +91,22 @@ class ContainerStatsCollector { } private ContainerStats.CpuStats collectCpuStats(ContainerId containerId) throws IOException { - Map<CGroupV2.CpuStatField, Long> cpuStats = cgroup.cpuStats(containerId); + Map<CpuController.StatField, Long> cpuStats = rootCgroup.resolveContainer(containerId).cpu().readStats(); return new ContainerStats.CpuStats(onlineCpus, systemCpuUsage(), - cpuStats.get(CGroupV2.CpuStatField.TOTAL_USAGE_USEC), - cpuStats.get(CGroupV2.CpuStatField.SYSTEM_USAGE_USEC), - cpuStats.get(CGroupV2.CpuStatField.THROTTLED_TIME_USEC), - cpuStats.get(CGroupV2.CpuStatField.TOTAL_PERIODS), - cpuStats.get(CGroupV2.CpuStatField.THROTTLED_PERIODS)); + cpuStats.get(CpuController.StatField.TOTAL_USAGE_USEC), + cpuStats.get(CpuController.StatField.SYSTEM_USAGE_USEC), + cpuStats.get(CpuController.StatField.THROTTLED_TIME_USEC), + cpuStats.get(CpuController.StatField.TOTAL_PERIODS), + cpuStats.get(CpuController.StatField.THROTTLED_PERIODS)); } private ContainerStats.MemoryStats collectMemoryStats(ContainerId containerId) throws IOException { - long memoryLimitInBytes = cgroup.memoryLimitInBytes(containerId); - long memoryUsageInBytes = cgroup.memoryUsageInBytes(containerId); - long cachedInBytes = cgroup.memoryCacheInBytes(containerId); - return new ContainerStats.MemoryStats(cachedInBytes, memoryUsageInBytes, memoryLimitInBytes); + MemoryController memoryController = rootCgroup.resolveContainer(containerId).memory(); + Size max = memoryController.readMax(); + long memoryUsageInBytes = memoryController.readCurrent().value(); + long cachedInBytes = memoryController.readFileSystemCache().value(); + return new ContainerStats.MemoryStats(cachedInBytes, memoryUsageInBytes, max.isMax() ? -1 : max.value()); } private ContainerStats.NetworkStats collectNetworkStats(String iface, int containerPid) throws IOException { diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/container/image/ContainerImagePruner.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/container/image/ContainerImagePruner.java index 8fcd7893f64..372b8522b0c 100644 --- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/container/image/ContainerImagePruner.java +++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/container/image/ContainerImagePruner.java @@ -2,11 +2,11 @@ package com.yahoo.vespa.hosted.node.admin.container.image; import com.yahoo.collections.Pair; +import com.yahoo.jdisc.Timer; import com.yahoo.vespa.hosted.node.admin.component.TaskContext; import com.yahoo.vespa.hosted.node.admin.container.ContainerEngine; import com.yahoo.vespa.hosted.node.admin.container.PartialContainer; -import java.time.Clock; import java.time.Duration; import java.time.Instant; import java.util.ArrayList; @@ -51,14 +51,14 @@ public class ContainerImagePruner { private static final Logger LOG = Logger.getLogger(ContainerImagePruner.class.getName()); - private final Clock clock; + private final Timer timer; private final ContainerEngine containerEngine; private final Map<String, Instant> lastTimeUsedByImageId = new ConcurrentHashMap<>(); - public ContainerImagePruner(ContainerEngine containerEngine, Clock clock) { + public ContainerImagePruner(ContainerEngine containerEngine, Timer timer) { this.containerEngine = Objects.requireNonNull(containerEngine); - this.clock = Objects.requireNonNull(clock); + this.timer = Objects.requireNonNull(timer); } /** @@ -112,7 +112,7 @@ public class ContainerImagePruner { } private Set<String> updateRecentlyUsedImageIds(List<Image> images, List<PartialContainer> containers, Duration minImageAgeToDelete) { - final Instant now = clock.instant(); + final Instant now = timer.currentTime(); // Add any already downloaded image to the list once images.forEach(image -> lastTimeUsedByImageId.putIfAbsent(image.id(), now)); diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/maintenance/StorageMaintainer.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/maintenance/StorageMaintainer.java index f0182ae36e4..9a548fba431 100644 --- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/maintenance/StorageMaintainer.java +++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/maintenance/StorageMaintainer.java @@ -5,6 +5,7 @@ import com.google.common.cache.Cache; import com.google.common.cache.CacheBuilder; import com.yahoo.config.provision.ApplicationId; import com.yahoo.config.provision.NodeType; +import com.yahoo.jdisc.Timer; import com.yahoo.vespa.hosted.node.admin.component.TaskContext; import com.yahoo.vespa.hosted.node.admin.container.Container; import com.yahoo.vespa.hosted.node.admin.container.ContainerName; @@ -27,7 +28,6 @@ import com.yahoo.vespa.hosted.node.admin.task.util.process.Terminal; import java.net.URI; import java.nio.file.Files; import java.nio.file.Path; -import java.time.Clock; import java.time.Duration; import java.time.Instant; import java.time.ZoneOffset; @@ -55,7 +55,7 @@ public class StorageMaintainer { private final CoredumpHandler coredumpHandler; private final DiskCleanup diskCleanup; private final SyncClient syncClient; - private final Clock clock; + private final Timer timer; private final Path archiveContainerStoragePath; // We cache disk usage to avoid doing expensive disk operations so often @@ -65,12 +65,12 @@ public class StorageMaintainer { .build(); public StorageMaintainer(Terminal terminal, CoredumpHandler coredumpHandler, DiskCleanup diskCleanup, - SyncClient syncClient, Clock clock, Path archiveContainerStoragePath) { + SyncClient syncClient, Timer timer, Path archiveContainerStoragePath) { this.terminal = terminal; this.coredumpHandler = coredumpHandler; this.diskCleanup = diskCleanup; this.syncClient = syncClient; - this.clock = clock; + this.timer = timer; this.archiveContainerStoragePath = archiveContainerStoragePath; } @@ -138,7 +138,7 @@ public class StorageMaintainer { } private List<DiskCleanupRule> createCleanupRules(NodeAgentContext context) { - Instant start = clock.instant(); + Instant start = timer.currentTime(); double oneMonthSeconds = Duration.ofDays(30).getSeconds(); Function<Instant, Double> monthNormalizer = instant -> Duration.between(instant, start).getSeconds() / oneMonthSeconds; List<DiskCleanupRule> rules = new ArrayList<>(); @@ -176,7 +176,7 @@ public class StorageMaintainer { public void archiveNodeStorage(NodeAgentContext context) { ContainerPath logsDirInContainer = context.paths().underVespaHome("logs"); Path containerLogsInArchiveDir = archiveContainerStoragePath - .resolve(context.containerName().asString() + "_" + DATE_TIME_FORMATTER.format(clock.instant()) + logsDirInContainer.pathInContainer()); + .resolve(context.containerName().asString() + "_" + DATE_TIME_FORMATTER.format(timer.currentTime()) + logsDirInContainer.pathInContainer()); // Files.move() does not support moving non-empty directories across providers, move using host paths UnixPath containerLogsOnHost = new UnixPath(logsDirInContainer.pathOnHost()); diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/maintenance/coredump/CoredumpHandler.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/maintenance/coredump/CoredumpHandler.java index 8c293fb40c9..af47f00a21e 100644 --- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/maintenance/coredump/CoredumpHandler.java +++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/maintenance/coredump/CoredumpHandler.java @@ -2,6 +2,7 @@ package com.yahoo.vespa.hosted.node.admin.maintenance.coredump; import com.yahoo.config.provision.DockerImage; +import com.yahoo.jdisc.Timer; import com.yahoo.security.KeyId; import com.yahoo.security.SecretSharedKey; import com.yahoo.vespa.flags.FetchVector; @@ -28,7 +29,6 @@ import java.io.OutputStream; import java.io.UncheckedIOException; import java.nio.file.Files; import java.nio.file.Path; -import java.time.Clock; import java.util.Comparator; import java.util.List; import java.util.Optional; @@ -65,7 +65,7 @@ public class CoredumpHandler { private final String crashPatchInContainer; private final Path doneCoredumpsPath; private final Metrics metrics; - private final Clock clock; + private final Timer timer; private final Supplier<String> coredumpIdSupplier; private final SecretSharedKeySupplier secretSharedKeySupplier; private final StringFlag coreEncryptionPublicKeyIdFlag; @@ -75,23 +75,23 @@ public class CoredumpHandler { * @param doneCoredumpsPath path on host where processed core dumps are stored */ public CoredumpHandler(CoreCollector coreCollector, Cores cores, - String crashPathInContainer, Path doneCoredumpsPath, Metrics metrics, + String crashPathInContainer, Path doneCoredumpsPath, Metrics metrics, Timer timer, SecretSharedKeySupplier secretSharedKeySupplier, FlagSource flagSource) { this(coreCollector, cores, crashPathInContainer, doneCoredumpsPath, - metrics, Clock.systemUTC(), () -> UUID.randomUUID().toString(), secretSharedKeySupplier, + metrics, timer, () -> UUID.randomUUID().toString(), secretSharedKeySupplier, flagSource); } CoredumpHandler(CoreCollector coreCollector, Cores cores, String crashPathInContainer, Path doneCoredumpsPath, Metrics metrics, - Clock clock, Supplier<String> coredumpIdSupplier, + Timer timer, Supplier<String> coredumpIdSupplier, SecretSharedKeySupplier secretSharedKeySupplier, FlagSource flagSource) { this.coreCollector = coreCollector; this.cores = cores; this.crashPatchInContainer = crashPathInContainer; this.doneCoredumpsPath = doneCoredumpsPath; this.metrics = metrics; - this.clock = clock; + this.timer = timer; this.coredumpIdSupplier = coredumpIdSupplier; this.secretSharedKeySupplier = secretSharedKeySupplier; this.coreEncryptionPublicKeyIdFlag = Flags.CORE_ENCRYPTION_PUBLIC_KEY_ID.bindTo(flagSource); @@ -276,7 +276,7 @@ public class CoredumpHandler { private boolean isReadyForProcessing(FileFinder.FileAttributes fileAttributes) { // Wait at least a minute until we start processing a core/heap dump to ensure that // kernel/JVM has finished writing it - return clock.instant().minusSeconds(60).isAfter(fileAttributes.lastModifiedTime()); + return timer.currentTime().minusSeconds(60).isAfter(fileAttributes.lastModifiedTime()); } void processAndReportSingleCoreDump(NodeAgentContext context, ContainerPath coreDumpDirectory, diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/maintenance/identity/AthenzCredentialsMaintainer.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/maintenance/identity/AthenzCredentialsMaintainer.java index 57a6ceb68aa..6119c77242c 100644 --- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/maintenance/identity/AthenzCredentialsMaintainer.java +++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/maintenance/identity/AthenzCredentialsMaintainer.java @@ -3,6 +3,7 @@ package com.yahoo.vespa.hosted.node.admin.maintenance.identity; import com.yahoo.component.Version; import com.yahoo.config.provision.ApplicationId; +import com.yahoo.jdisc.Timer; import com.yahoo.security.KeyAlgorithm; import com.yahoo.security.KeyUtils; import com.yahoo.security.Pkcs10Csr; @@ -45,7 +46,6 @@ import java.nio.file.Path; import java.security.KeyPair; import java.security.PrivateKey; import java.security.cert.X509Certificate; -import java.time.Clock; import java.time.Duration; import java.time.Instant; import java.util.List; @@ -76,7 +76,7 @@ public class AthenzCredentialsMaintainer implements CredentialsMaintainer { private final URI ztsEndpoint; private final Path ztsTrustStorePath; - private final Clock clock; + private final Timer timer; private final String certificateDnsSuffix; private final ServiceIdentityProvider hostIdentityProvider; private final IdentityDocumentClient identityDocumentClient; @@ -92,7 +92,7 @@ public class AthenzCredentialsMaintainer implements CredentialsMaintainer { String certificateDnsSuffix, ServiceIdentityProvider hostIdentityProvider, FlagSource flagSource, - Clock clock) { + Timer timer) { this.ztsEndpoint = ztsEndpoint; this.ztsTrustStorePath = ztsTrustStorePath; this.certificateDnsSuffix = certificateDnsSuffix; @@ -101,7 +101,7 @@ public class AthenzCredentialsMaintainer implements CredentialsMaintainer { configServerInfo.getLoadBalancerEndpoint(), hostIdentityProvider, new AthenzIdentityVerifier(Set.of(configServerInfo.getConfigServerIdentity()))); - this.clock = clock; + this.timer = timer; this.tenantServiceIdentityFlag = Flags.NODE_ADMIN_TENANT_SERVICE_REGISTRY.bindTo(flagSource); this.useNewIdentityDocumentLayout = Flags.NEW_IDDOC_LAYOUT.bindTo(flagSource); } @@ -144,7 +144,7 @@ public class AthenzCredentialsMaintainer implements CredentialsMaintainer { } X509Certificate certificate = readCertificateFromFile(certificateFile); - Instant now = clock.instant(); + Instant now = timer.currentTime(); Instant expiry = certificate.getNotAfter().toInstant(); var doc = EntityBindingsMapper.readSignedIdentityDocumentFromFile(identityDocumentFile); if (refreshIdentityDocument(doc, context)) { @@ -192,11 +192,13 @@ public class AthenzCredentialsMaintainer implements CredentialsMaintainer { try { var roleCertificatePath = siaDirectory.resolve("certs") .resolve(String.format("%s.cert.pem", role)); + var roleKeyPath = siaDirectory.resolve("keys") + .resolve(String.format("%s.key.pem", role)); if (!Files.exists(roleCertificatePath)) { - writeRoleCertificate(context, privateKeyFile, certificateFile, roleCertificatePath, identity, identityDocument, role); + writeRoleCredentials(context, privateKeyFile, certificateFile, roleCertificatePath, roleKeyPath, identity, identityDocument, role); modified = true; } else if (shouldRefreshCertificate(context, roleCertificatePath)) { - writeRoleCertificate(context, privateKeyFile, certificateFile, roleCertificatePath, identity, identityDocument, role); + writeRoleCredentials(context, privateKeyFile, certificateFile, roleCertificatePath, roleKeyPath, identity, identityDocument, role); modified = true; } } catch (IOException e) { @@ -208,33 +210,38 @@ public class AthenzCredentialsMaintainer implements CredentialsMaintainer { private boolean shouldRefreshCertificate(NodeAgentContext context, ContainerPath certificatePath) throws IOException { var certificate = readCertificateFromFile(certificatePath); - var now = clock.instant(); + var now = timer.currentTime(); var shouldRefresh = now.isAfter(certificate.getNotAfter().toInstant()) || now.isBefore(certificate.getNotBefore().toInstant().plus(REFRESH_PERIOD)); return !shouldThrottleRefreshAttempts(context.containerName(), now) && shouldRefresh; } - private void writeRoleCertificate(NodeAgentContext context, + private void writeRoleCredentials(NodeAgentContext context, ContainerPath privateKeyFile, ContainerPath certificateFile, ContainerPath roleCertificatePath, + ContainerPath roleKeyPath, AthenzIdentity identity, IdentityDocument identityDocument, String role) throws IOException { HostnameVerifier ztsHostNameVerifier = (hostname, sslSession) -> true; + var keyPair = KeyUtils.generateKeypair(KeyAlgorithm.RSA); var athenzRole = AthenzRole.fromResourceNameString(role); - var privateKey = KeyUtils.fromPemEncodedPrivateKey(new String(Files.readAllBytes(privateKeyFile))); - var containerIdentitySslContext = new SslContextBuilder().withKeyStore(privateKeyFile, certificateFile) + var containerIdentitySslContext = new SslContextBuilder() + .withKeyStore(privateKeyFile, certificateFile) .withTrustStore(ztsTrustStorePath) .build(); - try (ZtsClient ztsClient = new DefaultZtsClient.Builder(ztsEndpoint(identityDocument)).withSslContext(containerIdentitySslContext).withHostnameVerifier(ztsHostNameVerifier).build()) { + try (ZtsClient ztsClient = new DefaultZtsClient.Builder(ztsEndpoint(identityDocument)) + .withSslContext(containerIdentitySslContext) + .withHostnameVerifier(ztsHostNameVerifier) + .build()) { var csrGenerator = new CsrGenerator(certificateDnsSuffix, identityDocument.providerService().getFullName()); var csr = csrGenerator.generateRoleCsr( - identity, athenzRole, identityDocument.providerUniqueId(), identityDocument.clusterType(), KeyUtils.toKeyPair(privateKey)); + identity, athenzRole, identityDocument.providerUniqueId(), identityDocument.clusterType(), keyPair); var roleCertificate = ztsClient.getRoleCertificate(athenzRole, csr); - writeFile(roleCertificatePath, X509CertificateUtils.toPem(roleCertificate)); + writePrivateKeyAndCertificate(roleKeyPath, keyPair.getPrivate(), roleCertificatePath, roleCertificate); context.log(logger, "Role certificate successfully retrieved written to file " + roleCertificatePath.pathInContainer()); } } @@ -256,7 +263,7 @@ public class AthenzCredentialsMaintainer implements CredentialsMaintainer { ContainerPath certificateFile = (ContainerPath) SiaUtils.getCertificateFile(containerSiaDirectory, context.identity()); try { X509Certificate certificate = readCertificateFromFile(certificateFile); - Instant now = clock.instant(); + Instant now = timer.currentTime(); Instant expiry = certificate.getNotAfter().toInstant(); return Duration.between(now, expiry); } catch (IOException e) { @@ -278,9 +285,10 @@ public class AthenzCredentialsMaintainer implements CredentialsMaintainer { var privateKeyFile = (ContainerPath) SiaUtils.getPrivateKeyFile(siaDirectory, athenzIdentity); var certificateFile = (ContainerPath) SiaUtils.getCertificateFile(siaDirectory, athenzIdentity); try { - return Files.deleteIfExists(identityDocumentFile) || - Files.deleteIfExists(privateKeyFile) || - Files.deleteIfExists(certificateFile); + var modified = Files.deleteIfExists(identityDocumentFile); + modified |= Files.deleteIfExists(privateKeyFile); + modified |= Files.deleteIfExists(certificateFile); + return modified; } catch (IOException e) { throw new UncheckedIOException(e); } 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 ea393979bf6..716e2f92e74 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 @@ -3,6 +3,7 @@ package com.yahoo.vespa.hosted.node.admin.maintenance.servicedump; import com.yahoo.config.provision.ApplicationId; import com.yahoo.config.provision.CloudName; +import com.yahoo.jdisc.Timer; import com.yahoo.text.Lowercase; import com.yahoo.vespa.hosted.node.admin.configserver.noderepository.NodeAttributes; import com.yahoo.vespa.hosted.node.admin.configserver.noderepository.NodeRepository; @@ -19,7 +20,6 @@ import com.yahoo.yolean.concurrent.Sleeper; import java.io.UncheckedIOException; import java.net.URI; -import java.time.Clock; import java.time.Instant; import java.time.temporal.ChronoUnit; import java.util.ArrayList; @@ -44,20 +44,20 @@ public class VespaServiceDumperImpl implements VespaServiceDumper { private final ContainerOperations container; private final SyncClient syncClient; private final NodeRepository nodeRepository; - private final Clock clock; + private final Timer timer; private final ArtifactProducers artifactProducers; - public VespaServiceDumperImpl(ContainerOperations container, SyncClient syncClient, NodeRepository nodeRepository) { - this(ArtifactProducers.createDefault(Sleeper.DEFAULT), container, syncClient, nodeRepository, Clock.systemUTC()); + public VespaServiceDumperImpl(ContainerOperations container, SyncClient syncClient, NodeRepository nodeRepository, Timer timer) { + this(ArtifactProducers.createDefault(Sleeper.DEFAULT), container, syncClient, nodeRepository, timer); } // For unit testing VespaServiceDumperImpl(ArtifactProducers producers, ContainerOperations container, SyncClient syncClient, - NodeRepository nodeRepository, Clock clock) { + NodeRepository nodeRepository, Timer timer) { this.container = container; this.syncClient = syncClient; this.nodeRepository = nodeRepository; - this.clock = clock; + this.timer = timer; this.artifactProducers = producers; } @@ -65,7 +65,7 @@ public class VespaServiceDumperImpl implements VespaServiceDumper { public void processServiceDumpRequest(NodeAgentContext context) { if (context.zone().getCloudName().equals(CloudName.GCP)) return; - Instant startedAt = clock.instant(); + Instant startedAt = timer.currentTime(); NodeSpec nodeSpec = context.node(); ServiceDumpReport request; try { @@ -119,7 +119,7 @@ public class VespaServiceDumperImpl implements VespaServiceDumper { producedArtifacts.addAll(producer.produceArtifacts(producerCtx)); } uploadArtifacts(context, destination, producedArtifacts); - storeReport(context, ServiceDumpReport.createSuccessReport(request, startedAt, clock.instant(), destination)); + storeReport(context, ServiceDumpReport.createSuccessReport(request, startedAt, timer.currentTime(), destination)); } catch (Exception e) { handleFailure(context, request, startedAt, e, e.getMessage()); } finally { @@ -157,13 +157,13 @@ public class VespaServiceDumperImpl implements VespaServiceDumper { 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(requestOrNull, startedAt, clock.instant(), message); + ServiceDumpReport report = ServiceDumpReport.createErrorReport(requestOrNull, startedAt, timer.currentTime(), message); storeReport(context, report); } private void handleFailure(NodeAgentContext context, ServiceDumpReport requestOrNull, Instant startedAt, String message) { context.log(log, Level.WARNING, message); - ServiceDumpReport report = ServiceDumpReport.createErrorReport(requestOrNull, startedAt, clock.instant(), message); + ServiceDumpReport report = ServiceDumpReport.createErrorReport(requestOrNull, startedAt, timer.currentTime(), message); storeReport(context, report); } diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeadmin/NodeAdminImpl.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeadmin/NodeAdminImpl.java index f168523a1ef..cd7eee8ba50 100644 --- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeadmin/NodeAdminImpl.java +++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeadmin/NodeAdminImpl.java @@ -1,6 +1,7 @@ // 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.nodeadmin; +import com.yahoo.jdisc.Timer; import com.yahoo.vespa.hosted.node.admin.container.ContainerStats; import com.yahoo.vespa.hosted.node.admin.container.metrics.Counter; import com.yahoo.vespa.hosted.node.admin.container.metrics.Dimensions; @@ -13,7 +14,6 @@ import com.yahoo.vespa.hosted.node.admin.nodeagent.NodeAgentFactory; import com.yahoo.vespa.hosted.node.admin.nodeagent.NodeAgentScheduler; import java.nio.file.FileSystem; -import java.time.Clock; import java.time.Duration; import java.time.Instant; import java.util.HashSet; @@ -37,7 +37,7 @@ public class NodeAdminImpl implements NodeAdmin { private final NodeAgentWithSchedulerFactory nodeAgentWithSchedulerFactory; - private final Clock clock; + private final Timer timer; private final Duration freezeTimeout; private final Duration spread; private boolean previousWantFrozen; @@ -54,27 +54,27 @@ public class NodeAdminImpl implements NodeAdmin { private final Metrics metrics; private Dimensions previousMemoryOverheadDimensions = null; - public NodeAdminImpl(NodeAgentFactory nodeAgentFactory, Metrics metrics, Clock clock, FileSystem fileSystem) { - this(nodeAgentContext -> create(clock, nodeAgentFactory, nodeAgentContext), - metrics, clock, NODE_AGENT_FREEZE_TIMEOUT, NODE_AGENT_SPREAD, new ProcMeminfoReader(fileSystem)); + public NodeAdminImpl(NodeAgentFactory nodeAgentFactory, Metrics metrics, Timer timer, FileSystem fileSystem) { + this(nodeAgentContext -> create(timer, nodeAgentFactory, nodeAgentContext), + metrics, timer, NODE_AGENT_FREEZE_TIMEOUT, NODE_AGENT_SPREAD, new ProcMeminfoReader(fileSystem)); } public NodeAdminImpl(NodeAgentFactory nodeAgentFactory, Metrics metrics, - Clock clock, Duration freezeTimeout, Duration spread, ProcMeminfoReader procMeminfoReader) { - this(nodeAgentContext -> create(clock, nodeAgentFactory, nodeAgentContext), - metrics, clock, freezeTimeout, spread, procMeminfoReader); + Timer timer, Duration freezeTimeout, Duration spread, ProcMeminfoReader procMeminfoReader) { + this(nodeAgentContext -> create(timer, nodeAgentFactory, nodeAgentContext), + metrics, timer, freezeTimeout, spread, procMeminfoReader); } NodeAdminImpl(NodeAgentWithSchedulerFactory nodeAgentWithSchedulerFactory, - Metrics metrics, Clock clock, Duration freezeTimeout, Duration spread, + Metrics metrics, Timer timer, Duration freezeTimeout, Duration spread, ProcMeminfoReader procMeminfoReader) { this.nodeAgentWithSchedulerFactory = nodeAgentWithSchedulerFactory; - this.clock = clock; + this.timer = timer; this.freezeTimeout = freezeTimeout; this.spread = spread; this.previousWantFrozen = true; this.isFrozen = true; - this.startOfFreezeConvergence = clock.instant(); + this.startOfFreezeConvergence = timer.currentTime(); this.numberOfUnhandledExceptions = metrics.declareCounter("unhandled_exceptions", new Dimensions(Map.of("src", "node-agents"))); @@ -104,7 +104,7 @@ public class NodeAdminImpl implements NodeAdmin { }); Duration timeBetweenNodeAgents = spread.dividedBy(Math.max(nodeAgentContextsByHostname.size() - 1, 1)); - Instant nextAgentStart = clock.instant(); + Instant nextAgentStart = timer.currentTime(); // At this point, nodeAgentContextsByHostname and nodeAgentWithSchedulerByHostname should have the same keys for (Map.Entry<String, NodeAgentContext> entry : nodeAgentContextsByHostname.entrySet()) { nodeAgentWithSchedulerByHostname.get(entry.getKey()).scheduleTickWith(entry.getValue(), nextAgentStart); @@ -158,7 +158,7 @@ public class NodeAdminImpl implements NodeAdmin { public boolean setFrozen(boolean wantFrozen) { if (wantFrozen != previousWantFrozen) { if (wantFrozen) { - this.startOfFreezeConvergence = clock.instant(); + this.startOfFreezeConvergence = timer.currentTime(); } else { this.startOfFreezeConvergence = null; } @@ -188,7 +188,7 @@ public class NodeAdminImpl implements NodeAdmin { if (startOfFreezeConvergence == null) { return Duration.ZERO; } else { - return Duration.between(startOfFreezeConvergence, clock.instant()); + return Duration.between(startOfFreezeConvergence, timer.currentTime()); } } @@ -252,8 +252,8 @@ public class NodeAdminImpl implements NodeAdmin { NodeAgentWithScheduler create(NodeAgentContext context); } - private static NodeAgentWithScheduler create(Clock clock, NodeAgentFactory nodeAgentFactory, NodeAgentContext context) { - NodeAgentContextManager contextManager = new NodeAgentContextManager(clock, context); + private static NodeAgentWithScheduler create(Timer timer, NodeAgentFactory nodeAgentFactory, NodeAgentContext context) { + NodeAgentContextManager contextManager = new NodeAgentContextManager(timer, context); NodeAgent nodeAgent = nodeAgentFactory.create(contextManager, context); return new NodeAgentWithScheduler(nodeAgent, contextManager); } diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeagent/NodeAgentContextManager.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeagent/NodeAgentContextManager.java index 00fe7198667..54d59aac3c8 100644 --- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeagent/NodeAgentContextManager.java +++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeagent/NodeAgentContextManager.java @@ -1,7 +1,8 @@ // 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.nodeagent; -import java.time.Clock; +import com.yahoo.jdisc.Timer; + import java.time.Duration; import java.time.Instant; import java.util.Objects; @@ -14,7 +15,7 @@ import java.util.Objects; public class NodeAgentContextManager implements NodeAgentContextSupplier, NodeAgentScheduler { private final Object monitor = new Object(); - private final Clock clock; + private final Timer timer; private NodeAgentContext currentContext; private NodeAgentContext nextContext; @@ -24,8 +25,8 @@ public class NodeAgentContextManager implements NodeAgentContextSupplier, NodeAg private boolean interrupted = false; private boolean isWaitingForNextContext = false; - public NodeAgentContextManager(Clock clock, NodeAgentContext context) { - this.clock = clock; + public NodeAgentContextManager(Timer timer, NodeAgentContext context) { + this.timer = timer; this.currentContext = context; } @@ -48,8 +49,8 @@ public class NodeAgentContextManager implements NodeAgentContextSupplier, NodeAg boolean successful; long remainder; - long end = clock.instant().plus(timeout).toEpochMilli(); - while (!(successful = isFrozen == frozen) && (remainder = end - clock.millis()) > 0) { + long end = timer.currentTime().plus(timeout).toEpochMilli(); + while (!(successful = isFrozen == frozen) && (remainder = end - timer.currentTimeMillis()) > 0) { try { monitor.wait(remainder); // Wait with timeout until the supplier is has reached wanted frozen state } catch (InterruptedException ignored) { } diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeagent/NodeAgentImpl.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeagent/NodeAgentImpl.java index 025a04a15d6..64efeb85e63 100644 --- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeagent/NodeAgentImpl.java +++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeagent/NodeAgentImpl.java @@ -7,6 +7,7 @@ import com.yahoo.config.provision.DockerImage; import com.yahoo.config.provision.Environment; import com.yahoo.config.provision.NodeType; import com.yahoo.config.provision.zone.ZoneApi; +import com.yahoo.jdisc.Timer; import com.yahoo.vespa.flags.DoubleFlag; import com.yahoo.vespa.flags.FetchVector; import com.yahoo.vespa.flags.FlagSource; @@ -32,7 +33,6 @@ import com.yahoo.vespa.hosted.node.admin.maintenance.servicedump.VespaServiceDum import com.yahoo.vespa.hosted.node.admin.nodeadmin.ConvergenceException; import com.yahoo.vespa.hosted.node.admin.task.util.file.FileFinder; -import java.time.Clock; import java.time.Duration; import java.time.Instant; import java.util.ArrayList; @@ -70,7 +70,7 @@ public class NodeAgentImpl implements NodeAgent { private final List<CredentialsMaintainer> credentialsMaintainers; private final Optional<AclMaintainer> aclMaintainer; private final Optional<HealthChecker> healthChecker; - private final Clock clock; + private final Timer timer; private final Duration warmUpDuration; private final DoubleFlag containerCpuCap; private final VespaServiceDumper serviceDumper; @@ -109,10 +109,10 @@ public class NodeAgentImpl implements NodeAgent { Orchestrator orchestrator, ContainerOperations containerOperations, RegistryCredentialsProvider registryCredentialsProvider, StorageMaintainer storageMaintainer, FlagSource flagSource, List<CredentialsMaintainer> credentialsMaintainers, - Optional<AclMaintainer> aclMaintainer, Optional<HealthChecker> healthChecker, Clock clock, + Optional<AclMaintainer> aclMaintainer, Optional<HealthChecker> healthChecker, Timer timer, VespaServiceDumper serviceDumper, List<ContainerWireguardTask> wireguardTasks) { this(contextSupplier, nodeRepository, orchestrator, containerOperations, registryCredentialsProvider, - storageMaintainer, flagSource, credentialsMaintainers, aclMaintainer, healthChecker, clock, + storageMaintainer, flagSource, credentialsMaintainers, aclMaintainer, healthChecker, timer, DEFAULT_WARM_UP_DURATION, serviceDumper, wireguardTasks); } @@ -120,7 +120,7 @@ public class NodeAgentImpl implements NodeAgent { Orchestrator orchestrator, ContainerOperations containerOperations, RegistryCredentialsProvider registryCredentialsProvider, StorageMaintainer storageMaintainer, FlagSource flagSource, List<CredentialsMaintainer> credentialsMaintainers, - Optional<AclMaintainer> aclMaintainer, Optional<HealthChecker> healthChecker, Clock clock, + Optional<AclMaintainer> aclMaintainer, Optional<HealthChecker> healthChecker, Timer timer, Duration warmUpDuration, VespaServiceDumper serviceDumper, List<ContainerWireguardTask> wireguardTasks) { this.contextSupplier = contextSupplier; @@ -132,7 +132,7 @@ public class NodeAgentImpl implements NodeAgent { this.credentialsMaintainers = credentialsMaintainers; this.aclMaintainer = aclMaintainer; this.healthChecker = healthChecker; - this.clock = clock; + this.timer = timer; this.warmUpDuration = warmUpDuration; this.containerCpuCap = PermanentFlags.CONTAINER_CPU_CAP.bindTo(flagSource); this.serviceDumper = serviceDumper; @@ -232,7 +232,7 @@ public class NodeAgentImpl implements NodeAgent { Optional<DropDocumentsReport> report = context.node().reports().getReport(DropDocumentsReport.reportId(), DropDocumentsReport.class); if (report.isPresent() && report.get().startedAt() == null && report.get().readiedAt() != null) { - newNodeAttributes.withReport(DropDocumentsReport.reportId(), report.get().withStartedAt(clock.millis()).toJsonNode()); + newNodeAttributes.withReport(DropDocumentsReport.reportId(), report.get().withStartedAt(timer.currentTimeMillis()).toJsonNode()); changed = true; } @@ -400,7 +400,7 @@ public class NodeAgentImpl implements NodeAgent { ContainerResources wantedContainerResources = getContainerResources(context); if (healthChecker.isPresent() && firstSuccessfulHealthCheckInstant - .map(clock.instant().minus(warmUpDuration(context))::isBefore) + .map(timer.currentTime().minus(warmUpDuration(context))::isBefore) .orElse(true)) return existingContainer; @@ -450,7 +450,7 @@ public class NodeAgentImpl implements NodeAgent { container.ifPresent(c -> removeContainer(context, c, List.of("Dropping documents"), true)); FileFinder.from(context.paths().underVespaHome("var/db/vespa/search")).deleteRecursively(context); nodeRepository.updateNodeAttributes(context.node().hostname(), - new NodeAttributes().withReport(DropDocumentsReport.reportId(), report.get().withDroppedAt(clock.millis()).toJsonNode())); + new NodeAttributes().withReport(DropDocumentsReport.reportId(), report.get().withDroppedAt(timer.currentTimeMillis()).toJsonNode())); } throw ConvergenceException.ofTransient("Documents already dropped, waiting for signal to start the container"); @@ -529,9 +529,9 @@ public class NodeAgentImpl implements NodeAgent { if (healthChecker.isPresent()) { healthChecker.get().verifyHealth(context); if (firstSuccessfulHealthCheckInstant.isEmpty()) - firstSuccessfulHealthCheckInstant = Optional.of(clock.instant()); + firstSuccessfulHealthCheckInstant = Optional.of(timer.currentTime()); - Duration timeLeft = Duration.between(clock.instant(), firstSuccessfulHealthCheckInstant.get().plus(warmUpDuration(context))); + Duration timeLeft = Duration.between(timer.currentTime(), firstSuccessfulHealthCheckInstant.get().plus(warmUpDuration(context))); if (!container.get().resources().equalsCpu(getContainerResources(context))) throw ConvergenceException.ofTransient("Refusing to resume until warm up period ends (" + (timeLeft.isNegative() ? "next tick" : "in " + timeLeft) + ")"); diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/file/UnixPath.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/file/UnixPath.java index fbef3def446..94c2df1a8b8 100644 --- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/file/UnixPath.java +++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/file/UnixPath.java @@ -23,6 +23,7 @@ import java.nio.file.attribute.PosixFilePermissions; import java.nio.file.attribute.UserPrincipal; import java.nio.file.attribute.UserPrincipalLookupService; import java.time.Instant; +import java.util.List; import java.util.Optional; import java.util.Set; import java.util.stream.Stream; @@ -99,6 +100,22 @@ public class UnixPath { } } + public List<String> readLines() { + return uncheck(() -> Files.readAllLines(path)); + } + + /** Create an empty file and return true, or false if the file already exists (the file may not be regular). */ + public boolean create() { + try { + Files.createFile(path); + return true; + } catch (FileAlreadyExistsException ignored) { + return false; + } catch (IOException e) { + throw new UncheckedIOException(e); + } + } + public UnixPath writeUtf8File(String content, OpenOption... options) { return writeBytes(content.getBytes(StandardCharsets.UTF_8), options); } @@ -209,15 +226,16 @@ public class UnixPath { return this; } - /** Create directory with given permissions, unless it already exists, and return this. */ - public UnixPath createDirectory(String... permissions) { + /** Create directory with given permissions and return true, or false if it already exists. */ + public boolean createDirectory(String... permissions) { try { Files.createDirectory(path, permissionsAsFileAttributes(permissions)); } catch (FileAlreadyExistsException ignore) { + return false; } catch (IOException e) { throw new UncheckedIOException(e); } - return this; + return true; } public UnixPath createDirectories(String... permissions) { diff --git a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/container/CGroupV2Test.java b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/cgroup/CgroupTest.java index 789f31f75c6..27580082020 100644 --- a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/container/CGroupV2Test.java +++ b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/cgroup/CgroupTest.java @@ -1,7 +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.container; -import com.yahoo.collections.Pair; +// 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.cgroup; + +import com.yahoo.vespa.hosted.node.admin.container.ContainerId; import com.yahoo.vespa.hosted.node.admin.nodeagent.NodeAgentContext; import com.yahoo.vespa.hosted.node.admin.nodeagent.NodeAgentContextImpl; import com.yahoo.vespa.hosted.node.admin.task.util.file.UnixPath; @@ -12,16 +14,15 @@ import java.io.IOException; import java.nio.file.FileSystem; import java.util.Map; import java.util.Optional; -import java.util.OptionalInt; - -import static com.yahoo.vespa.hosted.node.admin.container.CGroupV2.CpuStatField.SYSTEM_USAGE_USEC; -import static com.yahoo.vespa.hosted.node.admin.container.CGroupV2.CpuStatField.THROTTLED_PERIODS; -import static com.yahoo.vespa.hosted.node.admin.container.CGroupV2.CpuStatField.THROTTLED_TIME_USEC; -import static com.yahoo.vespa.hosted.node.admin.container.CGroupV2.CpuStatField.TOTAL_PERIODS; -import static com.yahoo.vespa.hosted.node.admin.container.CGroupV2.CpuStatField.TOTAL_USAGE_USEC; -import static com.yahoo.vespa.hosted.node.admin.container.CGroupV2.CpuStatField.USER_USAGE_USEC; -import static com.yahoo.vespa.hosted.node.admin.container.CGroupV2.sharesToWeight; -import static com.yahoo.vespa.hosted.node.admin.container.CGroupV2.weightToShares; + +import static com.yahoo.vespa.hosted.node.admin.cgroup.CpuController.StatField.SYSTEM_USAGE_USEC; +import static com.yahoo.vespa.hosted.node.admin.cgroup.CpuController.StatField.THROTTLED_PERIODS; +import static com.yahoo.vespa.hosted.node.admin.cgroup.CpuController.StatField.THROTTLED_TIME_USEC; +import static com.yahoo.vespa.hosted.node.admin.cgroup.CpuController.StatField.TOTAL_PERIODS; +import static com.yahoo.vespa.hosted.node.admin.cgroup.CpuController.StatField.TOTAL_USAGE_USEC; +import static com.yahoo.vespa.hosted.node.admin.cgroup.CpuController.StatField.USER_USAGE_USEC; +import static com.yahoo.vespa.hosted.node.admin.cgroup.CpuController.sharesToWeight; +import static com.yahoo.vespa.hosted.node.admin.cgroup.CpuController.weightToShares; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertTrue; @@ -29,47 +30,48 @@ import static org.junit.jupiter.api.Assertions.assertTrue; /** * @author freva */ -public class CGroupV2Test { +public class CgroupTest { private static final ContainerId containerId = new ContainerId("4aec78cc"); private final FileSystem fileSystem = TestFileSystem.create(); - private final CGroupV2 cgroup = new CGroupV2(fileSystem); + private final Cgroup containerCgroup = Cgroup.root(fileSystem).resolveContainer(containerId); + private final CpuController containerCpu = containerCgroup.cpu(); private final NodeAgentContext context = NodeAgentContextImpl.builder("node123.yahoo.com").fileSystem(fileSystem).build(); private final UnixPath cgroupRoot = new UnixPath(fileSystem.getPath("/sys/fs/cgroup/machine.slice/libpod-4aec78cc.scope/container")).createDirectories(); @Test public void updates_cpu_quota_and_period() { - assertEquals(Optional.empty(), cgroup.cpuQuotaPeriod(containerId)); + assertEquals(Optional.empty(), containerCgroup.cpu().readMax()); cgroupRoot.resolve("cpu.max").writeUtf8File("max 100000\n"); - assertEquals(Optional.of(new Pair<>(-1, 100000)), cgroup.cpuQuotaPeriod(containerId)); + assertEquals(Optional.of(new CpuController.Max(Size.max(), 100000)), containerCpu.readMax()); cgroupRoot.resolve("cpu.max").writeUtf8File("456 123456\n"); - assertEquals(Optional.of(new Pair<>(456, 123456)), cgroup.cpuQuotaPeriod(containerId)); + assertEquals(Optional.of(new CpuController.Max(Size.from(456), 123456)), containerCpu.readMax()); - assertFalse(cgroup.updateCpuQuotaPeriod(context, containerId, 456, 123456)); + containerCgroup.cpu().updateMax(context, 456, 123456); - assertTrue(cgroup.updateCpuQuotaPeriod(context, containerId, 654, 123456)); - assertEquals(Optional.of(new Pair<>(654, 123456)), cgroup.cpuQuotaPeriod(containerId)); - assertEquals("654 123456", cgroupRoot.resolve("cpu.max").readUtf8File()); + assertTrue(containerCgroup.cpu().updateMax(context, 654, 123456)); + assertEquals(Optional.of(new CpuController.Max(Size.from(654), 123456)), containerCpu.readMax()); + assertEquals("654 123456\n", cgroupRoot.resolve("cpu.max").readUtf8File()); - assertTrue(cgroup.updateCpuQuotaPeriod(context, containerId, -1, 123456)); - assertEquals(Optional.of(new Pair<>(-1, 123456)), cgroup.cpuQuotaPeriod(containerId)); - assertEquals("max 123456", cgroupRoot.resolve("cpu.max").readUtf8File()); + assertTrue(containerCgroup.cpu().updateMax(context, -1, 123456)); + assertEquals(Optional.of(new CpuController.Max(Size.max(), 123456)), containerCpu.readMax()); + assertEquals("max 123456\n", cgroupRoot.resolve("cpu.max").readUtf8File()); } @Test public void updates_cpu_shares() { - assertEquals(OptionalInt.empty(), cgroup.cpuShares(containerId)); + assertEquals(Optional.empty(), containerCgroup.cpu().readShares()); cgroupRoot.resolve("cpu.weight").writeUtf8File("1\n"); - assertEquals(OptionalInt.of(2), cgroup.cpuShares(containerId)); + assertEquals(Optional.of(2), containerCgroup.cpu().readShares()); - assertFalse(cgroup.updateCpuShares(context, containerId, 2)); + assertFalse(containerCgroup.cpu().updateShares(context, 2)); - assertTrue(cgroup.updateCpuShares(context, containerId, 12345)); - assertEquals(OptionalInt.of(12323), cgroup.cpuShares(containerId)); + assertTrue(containerCgroup.cpu().updateShares(context, 12345)); + assertEquals(Optional.of(12323), containerCgroup.cpu().readShares()); } @Test @@ -82,16 +84,16 @@ public class CGroupV2Test { "throttled_usec 14256\n"); assertEquals(Map.of(TOTAL_USAGE_USEC, 17794243L, USER_USAGE_USEC, 16099205L, SYSTEM_USAGE_USEC, 1695038L, - TOTAL_PERIODS, 12465L, THROTTLED_PERIODS, 25L, THROTTLED_TIME_USEC, 14256L), cgroup.cpuStats(containerId)); + TOTAL_PERIODS, 12465L, THROTTLED_PERIODS, 25L, THROTTLED_TIME_USEC, 14256L), containerCgroup.cpu().readStats()); } @Test public void reads_memory_metrics() throws IOException { cgroupRoot.resolve("memory.current").writeUtf8File("2525093888\n"); - assertEquals(2525093888L, cgroup.memoryUsageInBytes(containerId)); + assertEquals(2525093888L, containerCgroup.memory().readCurrent().value()); cgroupRoot.resolve("memory.max").writeUtf8File("4322885632\n"); - assertEquals(4322885632L, cgroup.memoryLimitInBytes(containerId)); + assertEquals(4322885632L, containerCgroup.memory().readMax().value()); cgroupRoot.resolve("memory.stat").writeUtf8File("anon 3481600\n" + "file 69206016\n" + @@ -102,7 +104,7 @@ public class CGroupV2Test { "shmem 8380416\n" + "file_mapped 1081344\n" + "file_dirty 135168\n"); - assertEquals(69206016L, cgroup.memoryCacheInBytes(containerId)); + assertEquals(69206016L, containerCgroup.memory().readFileSystemCache().value()); } @Test diff --git a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/container/ContainerOperationsTest.java b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/container/ContainerOperationsTest.java index 701dd33cf55..567a23ed09d 100644 --- a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/container/ContainerOperationsTest.java +++ b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/container/ContainerOperationsTest.java @@ -2,6 +2,8 @@ package com.yahoo.vespa.hosted.node.admin.container; import com.yahoo.config.provision.DockerImage; +import com.yahoo.jdisc.test.TestTimer; +import com.yahoo.vespa.hosted.node.admin.cgroup.Cgroup; import com.yahoo.vespa.hosted.node.admin.component.TestTaskContext; import com.yahoo.vespa.test.file.TestFileSystem; import org.junit.jupiter.api.Test; @@ -11,9 +13,11 @@ import java.time.Instant; import java.util.List; import java.util.Map; import java.util.Set; -import java.util.stream.Collectors; -import static org.junit.jupiter.api.Assertions.*; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.mockito.Mockito.mock; /** * @author mpolden @@ -23,7 +27,8 @@ public class ContainerOperationsTest { private final TestTaskContext context = new TestTaskContext(); private final ContainerEngineMock containerEngine = new ContainerEngineMock(); private final FileSystem fileSystem = TestFileSystem.create(); - private final ContainerOperations containerOperations = new ContainerOperations(containerEngine, new CGroupV2(fileSystem), fileSystem); + private final TestTimer timer = new TestTimer(); + private final ContainerOperations containerOperations = new ContainerOperations(containerEngine, mock(Cgroup.class), fileSystem, timer); @Test void no_managed_containers_running() { diff --git a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/container/ContainerStatsCollectorTest.java b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/container/ContainerStatsCollectorTest.java index 72c5d016a47..d4598c8923f 100644 --- a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/container/ContainerStatsCollectorTest.java +++ b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/container/ContainerStatsCollectorTest.java @@ -1,6 +1,8 @@ // 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.container; +import com.yahoo.vespa.hosted.node.admin.cgroup.Cgroup; +import com.yahoo.vespa.hosted.node.admin.cgroup.Size; import com.yahoo.vespa.hosted.node.admin.configserver.noderepository.NodeSpec; import com.yahoo.vespa.hosted.node.admin.nodeagent.NodeAgentContext; import com.yahoo.vespa.hosted.node.admin.nodeagent.NodeAgentContextImpl; @@ -8,6 +10,7 @@ import com.yahoo.vespa.hosted.node.admin.task.util.file.UnixPath; import com.yahoo.vespa.hosted.node.admin.task.util.process.TestTerminal; import com.yahoo.vespa.test.file.TestFileSystem; import org.junit.jupiter.api.Test; +import org.mockito.Answers; import java.io.IOException; import java.nio.file.FileSystem; @@ -17,12 +20,12 @@ import java.util.List; import java.util.Map; import java.util.Optional; -import static com.yahoo.vespa.hosted.node.admin.container.CGroupV2.CpuStatField.SYSTEM_USAGE_USEC; -import static com.yahoo.vespa.hosted.node.admin.container.CGroupV2.CpuStatField.THROTTLED_PERIODS; -import static com.yahoo.vespa.hosted.node.admin.container.CGroupV2.CpuStatField.THROTTLED_TIME_USEC; -import static com.yahoo.vespa.hosted.node.admin.container.CGroupV2.CpuStatField.TOTAL_PERIODS; -import static com.yahoo.vespa.hosted.node.admin.container.CGroupV2.CpuStatField.TOTAL_USAGE_USEC; -import static com.yahoo.vespa.hosted.node.admin.container.CGroupV2.CpuStatField.USER_USAGE_USEC; +import static com.yahoo.vespa.hosted.node.admin.cgroup.CpuController.StatField.SYSTEM_USAGE_USEC; +import static com.yahoo.vespa.hosted.node.admin.cgroup.CpuController.StatField.THROTTLED_PERIODS; +import static com.yahoo.vespa.hosted.node.admin.cgroup.CpuController.StatField.THROTTLED_TIME_USEC; +import static com.yahoo.vespa.hosted.node.admin.cgroup.CpuController.StatField.TOTAL_PERIODS; +import static com.yahoo.vespa.hosted.node.admin.cgroup.CpuController.StatField.TOTAL_USAGE_USEC; +import static com.yahoo.vespa.hosted.node.admin.cgroup.CpuController.StatField.USER_USAGE_USEC; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertTrue; import static org.mockito.ArgumentMatchers.eq; @@ -37,11 +40,10 @@ public class ContainerStatsCollectorTest { private final TestTerminal testTerminal = new TestTerminal(); private final ContainerEngineMock containerEngine = new ContainerEngineMock(testTerminal); private final FileSystem fileSystem = TestFileSystem.create(); - private final CGroupV2 cgroup = mock(CGroupV2.class); + private final Cgroup cgroup = mock(Cgroup.class, Answers.RETURNS_DEEP_STUBS); private final NodeAgentContext context = NodeAgentContextImpl.builder(NodeSpec.Builder.testSpec("c1").build()) .fileSystem(TestFileSystem.create()) .build(); - @Test void collect() throws Exception { ContainerStatsCollector collector = new ContainerStatsCollector(containerEngine, cgroup, fileSystem, 24); @@ -92,17 +94,17 @@ public class ContainerStatsCollectorTest { " eth0: 22280813 118083 3 4 0 0 0 0 19859383 115415 5 6 0 0 0 0\n"); } - private void mockMemoryStats(ContainerId containerId) throws IOException { - when(cgroup.memoryUsageInBytes(eq(containerId))).thenReturn(1228017664L); - when(cgroup.memoryLimitInBytes(eq(containerId))).thenReturn(2147483648L); - when(cgroup.memoryCacheInBytes(eq(containerId))).thenReturn(470790144L); + private void mockMemoryStats(ContainerId containerId) { + when(cgroup.resolveContainer(eq(containerId)).memory().readCurrent()).thenReturn(Size.from(1228017664L)); + when(cgroup.resolveContainer(eq(containerId)).memory().readMax()).thenReturn(Size.from(2147483648L)); + when(cgroup.resolveContainer(eq(containerId)).memory().readFileSystemCache()).thenReturn(Size.from(470790144L)); } private void mockCpuStats(ContainerId containerId) throws IOException { UnixPath proc = new UnixPath(fileSystem.getPath("/proc")); proc.createDirectories(); - when(cgroup.cpuStats(eq(containerId))).thenReturn(Map.of( + when(cgroup.resolveContainer(eq(containerId)).cpu().readStats()).thenReturn(Map.of( TOTAL_USAGE_USEC, 691675615472L, SYSTEM_USAGE_USEC, 262190000000L, USER_USAGE_USEC, 40900L, TOTAL_PERIODS, 1L, THROTTLED_PERIODS, 2L, THROTTLED_TIME_USEC, 3L)); diff --git a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/container/image/ContainerImagePrunerTest.java b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/container/image/ContainerImagePrunerTest.java index 2ef6780dff6..79701f59994 100644 --- a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/container/image/ContainerImagePrunerTest.java +++ b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/container/image/ContainerImagePrunerTest.java @@ -2,7 +2,7 @@ package com.yahoo.vespa.hosted.node.admin.container.image; import com.yahoo.config.provision.DockerImage; -import com.yahoo.test.ManualClock; +import com.yahoo.jdisc.test.TestTimer; import com.yahoo.vespa.hosted.node.admin.component.TaskContext; import com.yahoo.vespa.hosted.node.admin.component.TestTaskContext; import com.yahoo.vespa.hosted.node.admin.container.Container; @@ -130,8 +130,8 @@ public class ContainerImagePrunerTest { private final ContainerEngineMock containerEngine = new ContainerEngineMock(); private final TaskContext context = new TestTaskContext(); - private final ManualClock clock = new ManualClock(); - private final ContainerImagePruner pruner = new ContainerImagePruner(containerEngine, clock); + private final TestTimer timer = new TestTimer(); + private final ContainerImagePruner pruner = new ContainerImagePruner(containerEngine, timer); private final Map<String, Integer> removalCountByImageId = new HashMap<>(); private boolean initialized = false; @@ -165,7 +165,7 @@ public class ContainerImagePrunerTest { initialized = true; } - clock.advance(Duration.ofMinutes(minutesAfter)); + timer.advance(Duration.ofMinutes(minutesAfter)); pruner.removeUnusedImages(context, excludedRefs, Duration.ofHours(1).minusSeconds(1)); diff --git a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/integration/ContainerTester.java b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/integration/ContainerTester.java index 7676d0e1790..b0bcea01f79 100644 --- a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/integration/ContainerTester.java +++ b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/integration/ContainerTester.java @@ -4,10 +4,11 @@ package com.yahoo.vespa.hosted.node.admin.integration; import com.yahoo.config.provision.DockerImage; import com.yahoo.config.provision.HostName; import com.yahoo.config.provision.NodeType; +import com.yahoo.jdisc.test.TestTimer; import com.yahoo.vespa.flags.InMemoryFlagSource; +import com.yahoo.vespa.hosted.node.admin.cgroup.Cgroup; import com.yahoo.vespa.hosted.node.admin.configserver.noderepository.NodeSpec; import com.yahoo.vespa.hosted.node.admin.configserver.orchestrator.Orchestrator; -import com.yahoo.vespa.hosted.node.admin.container.CGroupV2; import com.yahoo.vespa.hosted.node.admin.container.ContainerEngineMock; import com.yahoo.vespa.hosted.node.admin.container.ContainerName; import com.yahoo.vespa.hosted.node.admin.container.ContainerOperations; @@ -30,9 +31,7 @@ import org.mockito.InOrder; import org.mockito.Mockito; import java.nio.file.FileSystem; -import java.time.Clock; import java.time.Duration; -import java.util.Collections; import java.util.List; import java.util.Optional; import java.util.concurrent.Phaser; @@ -60,7 +59,8 @@ public class ContainerTester implements AutoCloseable { private final ContainerEngineMock containerEngine = new ContainerEngineMock(); private final FileSystem fileSystem = TestFileSystem.create(); - final ContainerOperations containerOperations = spy(new ContainerOperations(containerEngine, new CGroupV2(fileSystem), fileSystem)); + private final TestTimer timer = new TestTimer(); + final ContainerOperations containerOperations = spy(new ContainerOperations(containerEngine, mock(Cgroup.class), fileSystem, timer)); final NodeRepoMock nodeRepository = spy(new NodeRepoMock()); final Orchestrator orchestrator = mock(Orchestrator.class); final StorageMaintainer storageMaintainer = mock(StorageMaintainer.class); @@ -85,7 +85,6 @@ public class ContainerTester implements AutoCloseable { NodeSpec hostSpec = NodeSpec.Builder.testSpec(HOST_HOSTNAME.value()).type(NodeType.host).build(); nodeRepository.updateNodeSpec(hostSpec); - Clock clock = Clock.systemUTC(); Metrics metrics = new Metrics(); FileSystem fileSystem = TestFileSystem.create(); ProcMeminfoReader procMeminfoReader = mock(ProcMeminfoReader.class); @@ -94,7 +93,7 @@ public class ContainerTester implements AutoCloseable { NodeAgentFactory nodeAgentFactory = (contextSupplier, nodeContext) -> new NodeAgentImpl(contextSupplier, nodeRepository, orchestrator, containerOperations, () -> RegistryCredentials.none, storageMaintainer, flagSource, - Collections.emptyList(), Optional.empty(), Optional.empty(), clock, Duration.ofSeconds(-1), + List.of(), Optional.empty(), Optional.empty(), timer, Duration.ofSeconds(-1), VespaServiceDumper.DUMMY_INSTANCE, List.of()) { @Override public void converge(NodeAgentContext context) { super.converge(context); @@ -109,7 +108,7 @@ public class ContainerTester implements AutoCloseable { phaser.arriveAndDeregister(); } }; - nodeAdmin = new NodeAdminImpl(nodeAgentFactory, metrics, clock, Duration.ofMillis(10), Duration.ZERO, procMeminfoReader); + nodeAdmin = new NodeAdminImpl(nodeAgentFactory, metrics, timer, Duration.ofMillis(10), Duration.ZERO, procMeminfoReader); NodeAgentContextFactory nodeAgentContextFactory = (nodeSpec, acl) -> NodeAgentContextImpl.builder(nodeSpec).acl(acl).fileSystem(fileSystem).build(); nodeAdminStateUpdater = new NodeAdminStateUpdater(nodeAgentContextFactory, nodeRepository, orchestrator, diff --git a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/maintenance/StorageMaintainerTest.java b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/maintenance/StorageMaintainerTest.java index daae19478ed..1b770788995 100644 --- a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/maintenance/StorageMaintainerTest.java +++ b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/maintenance/StorageMaintainerTest.java @@ -2,7 +2,7 @@ package com.yahoo.vespa.hosted.node.admin.maintenance; import com.yahoo.config.provision.NodeResources; -import com.yahoo.test.ManualClock; +import com.yahoo.jdisc.test.TestTimer; import com.yahoo.vespa.hosted.node.admin.configserver.noderepository.NodeSpec; import com.yahoo.vespa.hosted.node.admin.maintenance.coredump.CoredumpHandler; import com.yahoo.vespa.hosted.node.admin.maintenance.disk.DiskCleanup; @@ -44,9 +44,9 @@ public class StorageMaintainerTest { private final CoredumpHandler coredumpHandler = mock(CoredumpHandler.class); private final DiskCleanup diskCleanup = mock(DiskCleanup.class); private final SyncClient syncClient = mock(SyncClient.class); - private final ManualClock clock = new ManualClock(Instant.ofEpochSecond(1234567890)); + private final TestTimer timer = new TestTimer(Instant.ofEpochSecond(1234567890)); private final FileSystem fileSystem = TestFileSystem.create(); - private final StorageMaintainer storageMaintainer = new StorageMaintainer(terminal, coredumpHandler, diskCleanup, syncClient, clock, + private final StorageMaintainer storageMaintainer = new StorageMaintainer(terminal, coredumpHandler, diskCleanup, syncClient, timer, fileSystem.getPath("/data/vespa/storage/container-archive")); @Test @@ -87,7 +87,7 @@ public class StorageMaintainerTest { // Archive container-1 storageMaintainer.archiveNodeStorage(context1); - clock.advance(Duration.ofSeconds(3)); + timer.advance(Duration.ofSeconds(3)); storageMaintainer.archiveNodeStorage(context1); // container-1 should be gone from container-storage diff --git a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/maintenance/coredump/CoredumpHandlerTest.java b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/maintenance/coredump/CoredumpHandlerTest.java index b0bd1b7b68f..061eef94c2b 100644 --- a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/maintenance/coredump/CoredumpHandlerTest.java +++ b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/maintenance/coredump/CoredumpHandlerTest.java @@ -2,10 +2,10 @@ package com.yahoo.vespa.hosted.node.admin.maintenance.coredump; import com.yahoo.config.provision.DockerImage; +import com.yahoo.jdisc.test.TestTimer; import com.yahoo.security.KeyId; import com.yahoo.security.SealedSharedKey; import com.yahoo.security.SecretSharedKey; -import com.yahoo.test.ManualClock; import com.yahoo.vespa.flags.Flags; import com.yahoo.vespa.flags.InMemoryFlagSource; import com.yahoo.vespa.hosted.node.admin.configserver.cores.CoreDumpMetadata; @@ -63,14 +63,14 @@ public class CoredumpHandlerTest { private final CoreCollector coreCollector = mock(CoreCollector.class); private final Cores cores = mock(Cores.class); private final Metrics metrics = new Metrics(); - private final ManualClock clock = new ManualClock(); + private final TestTimer timer = new TestTimer(); @SuppressWarnings("unchecked") private final Supplier<String> coredumpIdSupplier = mock(Supplier.class); private final SecretSharedKeySupplier secretSharedKeySupplier = mock(SecretSharedKeySupplier.class); private final InMemoryFlagSource flagSource = new InMemoryFlagSource(); private final CoredumpHandler coredumpHandler = new CoredumpHandler(coreCollector, cores, containerCrashPath.pathInContainer(), - doneCoredumpsPath, metrics, clock, coredumpIdSupplier, secretSharedKeySupplier, + doneCoredumpsPath, metrics, timer, coredumpIdSupplier, secretSharedKeySupplier, flagSource); @Test @@ -86,7 +86,7 @@ public class CoredumpHandlerTest { assertEquals(Optional.empty(), enqueuedPath); // bash.core.431 finished writing... and 2 more have since been written - clock.advance(Duration.ofMinutes(3)); + timer.advance(Duration.ofMinutes(3)); createFileAged(crashPath.resolve("vespa-proton.core.119"), Duration.ofMinutes(10)); createFileAged(crashPath.resolve("vespa-slobrok.core.673"), Duration.ofMinutes(5)); @@ -280,7 +280,7 @@ public class CoredumpHandlerTest { private Path createFileAged(Path path, Duration age) { return uncheck(() -> Files.setLastModifiedTime( Files.createFile(path), - FileTime.from(clock.instant().minus(age)))); + FileTime.from(timer.currentTime().minus(age)))); } private static byte[] bytesOf(String str) { 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 554a319f08b..cbe42c90a20 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 @@ -4,7 +4,7 @@ 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.jdisc.test.TestTimer; import com.yahoo.vespa.hosted.node.admin.configserver.noderepository.NodeSpec; import com.yahoo.vespa.hosted.node.admin.configserver.noderepository.NodeState; import com.yahoo.vespa.hosted.node.admin.container.ContainerOperations; @@ -26,7 +26,6 @@ import java.nio.file.Files; import java.nio.file.Path; import java.time.Instant; import java.util.List; -import java.util.stream.Collectors; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.mockito.ArgumentMatchers.any; @@ -71,11 +70,11 @@ class VespaServiceDumperImplTest { .thenReturn(new CommandResult(null, 0, "")); SyncClient syncClient = createSyncClientMock(); NodeRepoMock nodeRepository = new NodeRepoMock(); - ManualClock clock = new ManualClock(Instant.ofEpochMilli(1600001000000L)); + TestTimer timer = new TestTimer(Instant.ofEpochMilli(1600001000000L)); NodeSpec nodeSpec = createNodeSpecWithDumpRequest(nodeRepository, List.of("perf-report"), new ServiceDumpReport.DumpOptions(true, 45.0, null)); VespaServiceDumper reporter = new VespaServiceDumperImpl( - ArtifactProducers.createDefault(Sleeper.NOOP), operations, syncClient, nodeRepository, clock); + ArtifactProducers.createDefault(Sleeper.NOOP), operations, syncClient, nodeRepository, timer); NodeAgentContextImpl context = NodeAgentContextImpl.builder(nodeSpec) .fileSystem(fileSystem) .build(); @@ -112,12 +111,12 @@ class VespaServiceDumperImplTest { .thenReturn(new CommandResult(null, 0, "name=host-admin success")); SyncClient syncClient = createSyncClientMock(); NodeRepoMock nodeRepository = new NodeRepoMock(); - ManualClock clock = new ManualClock(Instant.ofEpochMilli(1600001000000L)); + TestTimer timer = new TestTimer(Instant.ofEpochMilli(1600001000000L)); NodeSpec nodeSpec = createNodeSpecWithDumpRequest( nodeRepository, List.of("jvm-jfr"), new ServiceDumpReport.DumpOptions(null, null, null)); VespaServiceDumper reporter = new VespaServiceDumperImpl( - ArtifactProducers.createDefault(Sleeper.NOOP), operations, syncClient, nodeRepository, clock); + ArtifactProducers.createDefault(Sleeper.NOOP), operations, syncClient, nodeRepository, timer); NodeAgentContextImpl context = NodeAgentContextImpl.builder(nodeSpec) .fileSystem(fileSystem) .build(); @@ -156,11 +155,11 @@ class VespaServiceDumperImplTest { .thenReturn(new CommandResult(null, 0, "name=host-admin success")); SyncClient syncClient = createSyncClientMock(); NodeRepoMock nodeRepository = new NodeRepoMock(); - ManualClock clock = new ManualClock(Instant.ofEpochMilli(1600001000000L)); + TestTimer timer = new TestTimer(Instant.ofEpochMilli(1600001000000L)); NodeSpec nodeSpec = createNodeSpecWithDumpRequest(nodeRepository, List.of("perf-report", "jvm-jfr"), new ServiceDumpReport.DumpOptions(true, 20.0, null)); VespaServiceDumper reporter = new VespaServiceDumperImpl( - ArtifactProducers.createDefault(Sleeper.NOOP), operations, syncClient, nodeRepository, clock); + ArtifactProducers.createDefault(Sleeper.NOOP), operations, syncClient, nodeRepository, timer); NodeAgentContextImpl context = NodeAgentContextImpl.builder(nodeSpec) .fileSystem(fileSystem) .build(); @@ -179,7 +178,7 @@ class VespaServiceDumperImplTest { ContainerOperations operations = mock(ContainerOperations.class); SyncClient syncClient = createSyncClientMock(); NodeRepoMock nodeRepository = new NodeRepoMock(); - ManualClock clock = new ManualClock(Instant.ofEpochMilli(1600001000000L)); + TestTimer timer = new TestTimer(Instant.ofEpochMilli(1600001000000L)); JsonNodeFactory fac = new ObjectMapper().getNodeFactory(); ObjectNode invalidRequest = new ObjectNode(fac) .set("dumpOptions", new ObjectNode(fac).put("duration", "invalidDurationDataType")); @@ -189,7 +188,7 @@ class VespaServiceDumperImplTest { .build(); nodeRepository.updateNodeSpec(spec); VespaServiceDumper reporter = new VespaServiceDumperImpl( - ArtifactProducers.createDefault(Sleeper.NOOP), operations, syncClient, nodeRepository, clock); + ArtifactProducers.createDefault(Sleeper.NOOP), operations, syncClient, nodeRepository, timer); NodeAgentContextImpl context = NodeAgentContextImpl.builder(spec) .fileSystem(fileSystem) .build(); diff --git a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/nodeadmin/NodeAdminImplTest.java b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/nodeadmin/NodeAdminImplTest.java index 96c18517bfe..7c58007a91a 100644 --- a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/nodeadmin/NodeAdminImplTest.java +++ b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/nodeadmin/NodeAdminImplTest.java @@ -1,7 +1,7 @@ // 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.nodeadmin; -import com.yahoo.test.ManualClock; +import com.yahoo.jdisc.test.TestTimer; import com.yahoo.vespa.hosted.node.admin.container.metrics.Metrics; import com.yahoo.vespa.hosted.node.admin.nodeagent.NodeAgentContext; import com.yahoo.vespa.hosted.node.admin.nodeagent.NodeAgentContextImpl; @@ -17,7 +17,9 @@ import java.util.Set; import static com.yahoo.vespa.hosted.node.admin.nodeadmin.NodeAdminImpl.NodeAgentWithScheduler; import static com.yahoo.vespa.hosted.node.admin.nodeadmin.NodeAdminImpl.NodeAgentWithSchedulerFactory; -import static org.junit.jupiter.api.Assertions.*; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertTrue; import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.anyBoolean; import static org.mockito.ArgumentMatchers.eq; @@ -34,10 +36,10 @@ import static org.mockito.Mockito.when; public class NodeAdminImplTest { private final NodeAgentWithSchedulerFactory nodeAgentWithSchedulerFactory = mock(NodeAgentWithSchedulerFactory.class); - private final ManualClock clock = new ManualClock(); + private final TestTimer timer = new TestTimer(); private final ProcMeminfoReader procMeminfoReader = mock(ProcMeminfoReader.class); private final NodeAdminImpl nodeAdmin = new NodeAdminImpl(nodeAgentWithSchedulerFactory, - new Metrics(), clock, Duration.ZERO, Duration.ZERO, procMeminfoReader); + new Metrics(), timer, Duration.ZERO, Duration.ZERO, procMeminfoReader); @Test void nodeAgentsAreProperlyLifeCycleManaged() { @@ -129,19 +131,19 @@ public class NodeAdminImplTest { // Initially everything is frozen to force convergence assertTrue(nodeAdmin.isFrozen()); assertTrue(nodeAdmin.subsystemFreezeDuration().isZero()); - clock.advance(Duration.ofSeconds(1)); + timer.advance(Duration.ofSeconds(1)); assertEquals(Duration.ofSeconds(1), nodeAdmin.subsystemFreezeDuration()); // Unfreezing floors freeze duration assertTrue(nodeAdmin.setFrozen(false)); // Unfreeze everything assertTrue(nodeAdmin.subsystemFreezeDuration().isZero()); - clock.advance(Duration.ofSeconds(1)); + timer.advance(Duration.ofSeconds(1)); assertTrue(nodeAdmin.subsystemFreezeDuration().isZero()); // Advancing time now will make freeze duration proceed according to clock assertTrue(nodeAdmin.setFrozen(true)); assertTrue(nodeAdmin.subsystemFreezeDuration().isZero()); - clock.advance(Duration.ofSeconds(1)); + timer.advance(Duration.ofSeconds(1)); assertEquals(Duration.ofSeconds(1), nodeAdmin.subsystemFreezeDuration()); } diff --git a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/nodeagent/NodeAgentContextManagerTest.java b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/nodeagent/NodeAgentContextManagerTest.java index 82c3f79f39f..a0a06f0fb1a 100644 --- a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/nodeagent/NodeAgentContextManagerTest.java +++ b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/nodeagent/NodeAgentContextManagerTest.java @@ -1,11 +1,11 @@ // 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.nodeagent; +import com.yahoo.jdisc.core.SystemTimer; import com.yahoo.vespa.test.file.TestFileSystem; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.Timeout; -import java.time.Clock; import java.time.Duration; import java.time.Instant; import java.util.Optional; @@ -13,7 +13,10 @@ import java.util.concurrent.Callable; import java.util.concurrent.CountDownLatch; import static com.yahoo.vespa.hosted.node.admin.nodeagent.NodeAgentContextSupplier.ContextSupplierInterruptedException; -import static org.junit.jupiter.api.Assertions.*; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertSame; +import static org.junit.jupiter.api.Assertions.assertTrue; /** * @author freva @@ -22,15 +25,15 @@ public class NodeAgentContextManagerTest { private static final int TIMEOUT = 10_000; - private final Clock clock = Clock.systemUTC(); + private final SystemTimer timer = new SystemTimer(); private final NodeAgentContext initialContext = generateContext(); - private final NodeAgentContextManager manager = new NodeAgentContextManager(clock, initialContext); + private final NodeAgentContextManager manager = new NodeAgentContextManager(timer, initialContext); @Test @Timeout(TIMEOUT) void context_is_ignored_unless_scheduled_while_waiting() { NodeAgentContext context1 = generateContext(); - manager.scheduleTickWith(context1, clock.instant()); + manager.scheduleTickWith(context1, timer.currentTime()); assertSame(initialContext, manager.currentContext()); AsyncExecutor<NodeAgentContext> async = new AsyncExecutor<>(manager::nextContext); @@ -38,7 +41,7 @@ public class NodeAgentContextManagerTest { assertFalse(async.isCompleted()); NodeAgentContext context2 = generateContext(); - manager.scheduleTickWith(context2, clock.instant()); + manager.scheduleTickWith(context2, timer.currentTime()); assertSame(context2, async.awaitResult().response.get()); assertSame(context2, manager.currentContext()); @@ -51,13 +54,13 @@ public class NodeAgentContextManagerTest { manager.waitUntilWaitingForNextContext(); NodeAgentContext context1 = generateContext(); - Instant returnAt = clock.instant().plusMillis(500); + Instant returnAt = timer.currentTime().plusMillis(500); manager.scheduleTickWith(context1, returnAt); assertSame(context1, async.awaitResult().response.get()); assertSame(context1, manager.currentContext()); // Is accurate to a millisecond - assertFalse(clock.instant().plusMillis(1).isBefore(returnAt)); + assertFalse(timer.currentTime().plusMillis(1).isBefore(returnAt)); } @Test @@ -68,7 +71,7 @@ public class NodeAgentContextManagerTest { assertFalse(async.isCompleted()); NodeAgentContext context1 = generateContext(); - manager.scheduleTickWith(context1, clock.instant()); + manager.scheduleTickWith(context1, timer.currentTime()); async.awaitResult(); assertEquals(Optional.of(context1), async.response); @@ -98,7 +101,7 @@ public class NodeAgentContextManagerTest { NodeAgentContext context1 = generateContext(); AsyncExecutor<NodeAgentContext> async = new AsyncExecutor<>(manager::nextContext); manager.waitUntilWaitingForNextContext(); - manager.scheduleTickWith(context1, clock.instant()); + manager.scheduleTickWith(context1, timer.currentTime()); assertSame(context1, async.awaitResult().response.get()); assertTrue(manager.setFrozen(false, Duration.ZERO)); @@ -108,9 +111,9 @@ public class NodeAgentContextManagerTest { @Timeout(TIMEOUT) void setFrozen_blocks_at_least_for_duration_of_timeout() { long wantedDurationMillis = 100; - long start = clock.millis(); + long start = timer.currentTimeMillis(); assertFalse(manager.setFrozen(false, Duration.ofMillis(wantedDurationMillis))); - long actualDurationMillis = clock.millis() - start; + long actualDurationMillis = timer.currentTimeMillis() - start; assertTrue(actualDurationMillis >= wantedDurationMillis); } diff --git a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/nodeagent/NodeAgentImplTest.java b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/nodeagent/NodeAgentImplTest.java index 2db5314dbf2..0913e1d040a 100644 --- a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/nodeagent/NodeAgentImplTest.java +++ b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/nodeagent/NodeAgentImplTest.java @@ -6,7 +6,7 @@ import com.yahoo.config.provision.ApplicationId; import com.yahoo.config.provision.DockerImage; import com.yahoo.config.provision.NodeResources; import com.yahoo.config.provision.NodeType; -import com.yahoo.test.ManualClock; +import com.yahoo.jdisc.test.TestTimer; import com.yahoo.vespa.flags.InMemoryFlagSource; import com.yahoo.vespa.flags.PermanentFlags; import com.yahoo.vespa.hosted.node.admin.configserver.noderepository.NodeAttributes; @@ -78,7 +78,7 @@ public class NodeAgentImplTest { private final HealthChecker healthChecker = mock(HealthChecker.class); private final CredentialsMaintainer credentialsMaintainer = mock(CredentialsMaintainer.class); private final InMemoryFlagSource flagSource = new InMemoryFlagSource(); - private final ManualClock clock = new ManualClock(Instant.now()); + private final TestTimer timer = new TestTimer(Instant.now()); private final FileSystem fileSystem = TestFileSystem.create(); @BeforeEach @@ -654,7 +654,7 @@ public class NodeAgentImplTest { } catch (ConvergenceException e) { assertEquals(healthCheckException, e); } - clock.advance(Duration.ofSeconds(30)); + timer.advance(Duration.ofSeconds(30)); } doNothing().when(healthChecker).verifyHealth(any()); @@ -669,7 +669,7 @@ public class NodeAgentImplTest { inOrder.verify(containerOperations, never()).updateContainer(any(), any(), any()); - clock.advance(Duration.ofSeconds(31)); + timer.advance(Duration.ofSeconds(31)); nodeAgent.doConverge(context); inOrder.verify(orchestrator, never()).suspend(any()); @@ -739,7 +739,7 @@ public class NodeAgentImplTest { inOrder.verify(containerOperations, never()).updateContainer(any(), any(), any()); - clock.advance(Duration.ofSeconds(31)); + timer.advance(Duration.ofSeconds(31)); nodeAgent.doConverge(context); inOrder.verify(orchestrator, times(1)).resume(eq(hostName)); } @@ -767,7 +767,7 @@ public class NodeAgentImplTest { nodeAgent.converge(context); verify(containerOperations).removeContainer(eq(context), any()); assertFalse(indexPath.exists()); - inOrder.verify(nodeRepository).updateNodeAttributes(eq(hostName), eq(new NodeAttributes().withReport(DropDocumentsReport.reportId(), new DropDocumentsReport(1L, clock.millis(), null, null).toJsonNode()))); + inOrder.verify(nodeRepository).updateNodeAttributes(eq(hostName), eq(new NodeAttributes().withReport(DropDocumentsReport.reportId(), new DropDocumentsReport(1L, timer.currentTimeMillis(), null, null).toJsonNode()))); inOrder.verifyNoMoreInteractions(); // After droppedAt and before readiedAt are set, we cannot proceed @@ -784,13 +784,13 @@ public class NodeAgentImplTest { inOrder.verifyNoMoreInteractions(); mockGetContainer(dockerImage, ContainerResources.from(0, 2, 16), true); - clock.advance(Duration.ofSeconds(31)); + timer.advance(Duration.ofSeconds(31)); nodeAgent.converge(context); verify(containerOperations, times(1)).startContainer(eq(context)); verify(containerOperations, never()).removeContainer(eq(context), any()); inOrder.verify(nodeRepository).updateNodeAttributes(eq(hostName), eq(new NodeAttributes() .withRebootGeneration(0) - .withReport(DropDocumentsReport.reportId(), new DropDocumentsReport(1L, 2L, 3L, clock.millis()).toJsonNode()))); + .withReport(DropDocumentsReport.reportId(), new DropDocumentsReport(1L, 2L, 3L, timer.currentTimeMillis()).toJsonNode()))); inOrder.verifyNoMoreInteractions(); } @@ -844,7 +844,7 @@ public class NodeAgentImplTest { return new NodeAgentImpl(contextSupplier, nodeRepository, orchestrator, containerOperations, () -> RegistryCredentials.none, storageMaintainer, flagSource, List.of(credentialsMaintainer), Optional.of(aclMaintainer), Optional.of(healthChecker), - clock, warmUpDuration, VespaServiceDumper.DUMMY_INSTANCE, List.of()); + timer, warmUpDuration, VespaServiceDumper.DUMMY_INSTANCE, List.of()); } private void mockGetContainer(DockerImage dockerImage, boolean isRunning) { @@ -860,7 +860,7 @@ public class NodeAgentImplTest { Optional.of(new Container( containerId, ContainerName.fromHostname(hostName), - clock.instant(), + timer.currentTime(), isRunning ? Container.State.running : Container.State.exited, "image-id-1", dockerImage, diff --git a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/task/util/file/UnixPathTest.java b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/task/util/file/UnixPathTest.java index ad8c6ea3a35..bbe96272b4b 100644 --- a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/task/util/file/UnixPathTest.java +++ b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/task/util/file/UnixPathTest.java @@ -10,8 +10,14 @@ import java.nio.charset.StandardCharsets; import java.nio.file.FileSystem; import java.nio.file.Files; import java.nio.file.Path; +import java.util.List; -import static org.junit.jupiter.api.Assertions.*; +import static org.junit.jupiter.api.Assertions.assertArrayEquals; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.junit.jupiter.api.Assertions.fail; /** * @author hakonhall @@ -38,6 +44,15 @@ public class UnixPathTest { path.writeUtf8File(original); String fromFile = path.readUtf8File(); assertEquals(original, fromFile); + assertEquals(List.of("foo", "bar"), path.readLines()); + } + + @Test + void touch() { + UnixPath path = new UnixPath(fs.getPath("example.txt")); + assertTrue(path.create()); + assertEquals("", path.readUtf8File()); + assertFalse(path.create()); } @Test @@ -74,9 +89,10 @@ public class UnixPathTest { Path path = fs.getPath("dir"); UnixPath unixPath = new UnixPath(path); String permissions = "rwxr-xr--"; - unixPath.createDirectory(permissions); + assertTrue(unixPath.createDirectory(permissions)); assertTrue(unixPath.isDirectory()); assertEquals(permissions, unixPath.getPermissions()); + assertFalse(unixPath.createDirectory(permissions)); } @Test diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/lb/LoadBalancerService.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/lb/LoadBalancerService.java index 313cf45e1ee..4f33e079d8f 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/lb/LoadBalancerService.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/lb/LoadBalancerService.java @@ -2,6 +2,7 @@ package com.yahoo.vespa.hosted.provision.lb; import com.yahoo.config.provision.ClusterSpec; +import com.yahoo.config.provision.EndpointsChecker.HealthChecker; import com.yahoo.config.provision.NodeType; /** @@ -9,7 +10,7 @@ import com.yahoo.config.provision.NodeType; * * @author mpolden */ -public interface LoadBalancerService { +public interface LoadBalancerService extends HealthChecker { /** * Provisions load balancers from the given specification. Implementations are expected to be idempotent diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/lb/LoadBalancerServiceMock.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/lb/LoadBalancerServiceMock.java index 751f3d46059..a79766a577d 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/lb/LoadBalancerServiceMock.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/lb/LoadBalancerServiceMock.java @@ -4,6 +4,8 @@ package com.yahoo.vespa.hosted.provision.lb; import ai.vespa.http.DomainName; import com.google.common.collect.ImmutableSet; import com.yahoo.config.provision.ClusterSpec; +import com.yahoo.config.provision.EndpointsChecker.Availability; +import com.yahoo.config.provision.EndpointsChecker.Endpoint; import com.yahoo.config.provision.NodeType; import java.util.Collections; @@ -85,4 +87,9 @@ public class LoadBalancerServiceMock implements LoadBalancerService { instances.remove(loadBalancer.id()); } + @Override + public Availability healthy(Endpoint endpoint) { + return Availability.ready; + } + } diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/lb/SharedLoadBalancerService.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/lb/SharedLoadBalancerService.java index f9f26852b0d..e49d1b302cf 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/lb/SharedLoadBalancerService.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/lb/SharedLoadBalancerService.java @@ -3,6 +3,8 @@ package com.yahoo.vespa.hosted.provision.lb; import ai.vespa.http.DomainName; import com.yahoo.config.provision.ClusterSpec; +import com.yahoo.config.provision.EndpointsChecker.Availability; +import com.yahoo.config.provision.EndpointsChecker.Endpoint; import com.yahoo.config.provision.NodeType; import java.util.List; @@ -68,4 +70,9 @@ public class SharedLoadBalancerService implements LoadBalancerService { return nodeType == NodeType.tenant && clusterType.isContainer(); } + @Override + public Availability healthy(Endpoint endpoint) { + return Availability.ready; + } + } diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/ProvisionServiceProvider.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/ProvisionServiceProvider.java index 6e301b7724c..65039aaca77 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/ProvisionServiceProvider.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/ProvisionServiceProvider.java @@ -1,6 +1,7 @@ // Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.vespa.hosted.provision.provisioning; +import com.yahoo.config.provision.EndpointsChecker.HealthCheckerProvider; import com.yahoo.vespa.hosted.provision.lb.LoadBalancerService; import java.util.Optional; @@ -10,11 +11,12 @@ import java.util.Optional; * * @author freva */ -public interface ProvisionServiceProvider { +public interface ProvisionServiceProvider extends HealthCheckerProvider { Optional<LoadBalancerService> getLoadBalancerService(); Optional<HostProvisioner> getHostProvisioner(); HostResourcesCalculator getHostResourcesCalculator(); + } diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/AutoscalingMaintainerTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/AutoscalingMaintainerTest.java index 40ca30d758e..d75f51680d7 100644 --- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/AutoscalingMaintainerTest.java +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/AutoscalingMaintainerTest.java @@ -154,10 +154,10 @@ public class AutoscalingMaintainerTest { @Test public void test_toString() { - assertEquals("4 nodes with [vcpu: 1.0, memory: 2.0 Gb, disk 4.0 Gb, bandwidth: 1.0 Gbps, architecture: x86_64] (total: [vcpu: 4.0, memory: 8.0 Gb, disk 16.0 Gb, bandwidth: 4.0 Gbps, architecture: x86_64])", + assertEquals("4 nodes with [vcpu: 1.0, memory: 2.0 Gb, disk 4.0 Gb, bandwidth: 1.0 Gbps, architecture: any] (total: [vcpu: 4.0, memory: 8.0 Gb, disk 16.0 Gb, bandwidth: 4.0 Gbps, architecture: any])", AutoscalingMaintainer.toString(new ClusterResources(4, 1, new NodeResources(1, 2, 4, 1)))); - assertEquals("4 nodes (in 2 groups) with [vcpu: 1.0, memory: 2.0 Gb, disk 4.0 Gb, bandwidth: 1.0 Gbps, architecture: x86_64] (total: [vcpu: 4.0, memory: 8.0 Gb, disk 16.0 Gb, bandwidth: 4.0 Gbps, architecture: x86_64])", + assertEquals("4 nodes (in 2 groups) with [vcpu: 1.0, memory: 2.0 Gb, disk 4.0 Gb, bandwidth: 1.0 Gbps, architecture: any] (total: [vcpu: 4.0, memory: 8.0 Gb, disk 16.0 Gb, bandwidth: 4.0 Gbps, architecture: any])", AutoscalingMaintainer.toString(new ClusterResources(4, 2, new NodeResources(1, 2, 4, 1)))); } diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/ScalingSuggestionsMaintainerTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/ScalingSuggestionsMaintainerTest.java index e6c183d02ce..f73d6f2ce01 100644 --- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/ScalingSuggestionsMaintainerTest.java +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/ScalingSuggestionsMaintainerTest.java @@ -73,9 +73,9 @@ public class ScalingSuggestionsMaintainerTest { new TestMetric()); maintainer.maintain(); - assertEquals("8 nodes with [vcpu: 3.2, memory: 4.5 Gb, disk 10.0 Gb, bandwidth: 0.1 Gbps, architecture: x86_64]", + assertEquals("8 nodes with [vcpu: 3.2, memory: 4.5 Gb, disk 10.0 Gb, bandwidth: 0.1 Gbps, architecture: any]", suggestionOf(app1, cluster1, tester).resources().get().toString()); - assertEquals("8 nodes with [vcpu: 3.6, memory: 4.4 Gb, disk 11.8 Gb, bandwidth: 0.1 Gbps, architecture: x86_64]", + assertEquals("8 nodes with [vcpu: 3.6, memory: 4.4 Gb, disk 11.8 Gb, bandwidth: 0.1 Gbps, architecture: any]", suggestionOf(app2, cluster2, tester).resources().get().toString()); // Utilization goes way down @@ -83,14 +83,14 @@ public class ScalingSuggestionsMaintainerTest { addMeasurements(0.10f, 0.10f, 0.10f, 0, 500, app1, tester.nodeRepository()); maintainer.maintain(); assertEquals("Suggestion stays at the peak value observed", - "8 nodes with [vcpu: 3.2, memory: 4.5 Gb, disk 10.0 Gb, bandwidth: 0.1 Gbps, architecture: x86_64]", + "8 nodes with [vcpu: 3.2, memory: 4.5 Gb, disk 10.0 Gb, bandwidth: 0.1 Gbps, architecture: any]", suggestionOf(app1, cluster1, tester).resources().get().toString()); // Utilization is still way down and a week has passed tester.clock().advance(Duration.ofDays(7)); addMeasurements(0.10f, 0.10f, 0.10f, 0, 500, app1, tester.nodeRepository()); maintainer.maintain(); assertEquals("Peak suggestion has been outdated", - "3 nodes with [vcpu: 1.2, memory: 4.0 Gb, disk 10.0 Gb, bandwidth: 0.1 Gbps, architecture: x86_64]", + "3 nodes with [vcpu: 1.2, memory: 4.0 Gb, disk 10.0 Gb, bandwidth: 0.1 Gbps, architecture: any]", suggestionOf(app1, cluster1, tester).resources().get().toString()); assertTrue(shouldSuggest(app1, cluster1, tester)); diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/os/OsVersionsTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/os/OsVersionsTest.java index 98c17eb4d5e..40b035968bd 100644 --- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/os/OsVersionsTest.java +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/os/OsVersionsTest.java @@ -326,7 +326,7 @@ public class OsVersionsTest { .account(CloudAccount.from("000000000000")) .build()); - provisionInfraApplication(hostCount, infraApplication, NodeType.host, NodeResources.StorageType.remote); + provisionInfraApplication(hostCount, infraApplication, NodeType.host, NodeResources.StorageType.remote, NodeResources.Architecture.x86_64); Supplier<NodeList> hostNodes = () -> tester.nodeRepository().nodes().list().nodeType(NodeType.host); // New target is set @@ -535,13 +535,20 @@ public class OsVersionsTest { return provisionInfraApplication(nodeCount, infraApplication, NodeType.host); } - private List<Node> provisionInfraApplication(int nodeCount, ApplicationId application, NodeType nodeType) { + private List<Node> provisionInfraApplication(int nodeCount, ApplicationId application, + NodeType nodeType) { return provisionInfraApplication(nodeCount, application, nodeType, NodeResources.StorageType.local); } - private List<Node> provisionInfraApplication(int nodeCount, ApplicationId application, NodeType nodeType, NodeResources.StorageType storageType) { + private List<Node> provisionInfraApplication(int nodeCount, ApplicationId application, + NodeType nodeType, NodeResources.StorageType storageType) { + return provisionInfraApplication(nodeCount, application, nodeType, storageType, NodeResources.Architecture.x86_64); + } + + private List<Node> provisionInfraApplication(int nodeCount, ApplicationId application, NodeType nodeType, + NodeResources.StorageType storageType, NodeResources.Architecture architecture) { var nodes = tester.makeReadyNodes(nodeCount, new NodeResources(48, 128, 2000, 10, - NodeResources.DiskSpeed.fast, storageType), + NodeResources.DiskSpeed.fast, storageType, architecture), nodeType, 10); tester.prepareAndActivateInfraApplication(application, nodeType); tester.clock().advance(Duration.ofDays(1).plusSeconds(1)); // Let grace period pass diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/HostCapacityTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/HostCapacityTest.java index 12a8e4d9386..ec8a8f637c8 100644 --- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/HostCapacityTest.java +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/HostCapacityTest.java @@ -98,11 +98,14 @@ public class HostCapacityTest { @Test public void unusedCapacityOf() { - assertEquals(new NodeResources(5, 40, 80, 2, NodeResources.DiskSpeed.fast, NodeResources.StorageType.remote), + assertEquals(new NodeResources(5, 40, 80, 2, + NodeResources.DiskSpeed.fast, NodeResources.StorageType.remote, NodeResources.Architecture.x86_64), capacity.unusedCapacityOf(host1)); - assertEquals(new NodeResources(5, 60, 80, 4.5, NodeResources.DiskSpeed.fast, NodeResources.StorageType.remote), + assertEquals(new NodeResources(5, 60, 80, 4.5, + NodeResources.DiskSpeed.fast, NodeResources.StorageType.remote, NodeResources.Architecture.x86_64), capacity.unusedCapacityOf(host3)); - assertEquals(new NodeResources(7, 100, 120, 5, NodeResources.DiskSpeed.fast, NodeResources.StorageType.remote), + assertEquals(new NodeResources(7, 100, 120, 5, + NodeResources.DiskSpeed.fast, NodeResources.StorageType.remote, NodeResources.Architecture.x86_64), capacity.unusedCapacityOf(host4)); doAnswer(invocation -> { @@ -110,17 +113,19 @@ public class HostCapacityTest { return totalHostResources.subtract(new NodeResources(1, 2, 3, 0.5, NodeResources.DiskSpeed.any)); }).when(hostResourcesCalculator).advertisedResourcesOf(any()); - assertEquals(new NodeResources(4, 38, 77, 1.5, NodeResources.DiskSpeed.fast, NodeResources.StorageType.remote), + assertEquals(new NodeResources(4, 38, 77, 1.5, NodeResources.DiskSpeed.fast, NodeResources.StorageType.remote, NodeResources.Architecture.x86_64), capacity.unusedCapacityOf(host1)); - assertEquals(new NodeResources(4, 58, 77, 4, NodeResources.DiskSpeed.fast, NodeResources.StorageType.remote), + assertEquals(new NodeResources(4, 58, 77, 4, NodeResources.DiskSpeed.fast, NodeResources.StorageType.remote, NodeResources.Architecture.x86_64), capacity.unusedCapacityOf(host3)); } @Test public void availableCapacityOf() { - assertEquals(new NodeResources(5, 40, 80, 2, NodeResources.DiskSpeed.fast, NodeResources.StorageType.remote), + assertEquals(new NodeResources(5, 40, 80, 2, + NodeResources.DiskSpeed.fast, NodeResources.StorageType.remote, NodeResources.Architecture.x86_64), capacity.availableCapacityOf(host1)); - assertEquals(new NodeResources(5, 60, 80, 4.5, NodeResources.DiskSpeed.fast, NodeResources.StorageType.remote), + assertEquals(new NodeResources(5, 60, 80, 4.5, + NodeResources.DiskSpeed.fast, NodeResources.StorageType.remote, NodeResources.Architecture.x86_64), capacity.availableCapacityOf(host3)); assertEquals(NodeResources.zero(), capacity.availableCapacityOf(host4)); diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/VirtualNodeProvisioningTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/VirtualNodeProvisioningTest.java index 0744d82c85b..f40c8037f41 100644 --- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/VirtualNodeProvisioningTest.java +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/VirtualNodeProvisioningTest.java @@ -440,7 +440,7 @@ public class VirtualNodeProvisioningTest { catch (Exception e) { assertEquals("No room for 3 nodes as 2 of 4 hosts are exclusive", "Could not satisfy request for 3 nodes with " + - "[vcpu: 2.0, memory: 4.0 Gb, disk 100.0 Gb, bandwidth: 1.0 Gbps, architecture: x86_64] " + + "[vcpu: 2.0, memory: 4.0 Gb, disk 100.0 Gb, bandwidth: 1.0 Gbps, architecture: any] " + "in tenant2.app2 container cluster 'my-container' 6.39: " + "Node allocation failure on group 0: " + "Not enough suitable nodes available due to host exclusivity constraints", @@ -467,7 +467,7 @@ public class VirtualNodeProvisioningTest { } catch (NodeAllocationException e) { assertEquals("Could not satisfy request for 2 nodes with " + - "[vcpu: 1.0, memory: 4.0 Gb, disk 100.0 Gb, bandwidth: 1.0 Gbps, storage type: remote, architecture: x86_64] " + + "[vcpu: 1.0, memory: 4.0 Gb, disk 100.0 Gb, bandwidth: 1.0 Gbps, storage type: remote, architecture: any] " + "in tenant.app1 content cluster 'my-content'" + " 6.42: Node allocation failure on group 0", e.getMessage()); @@ -549,8 +549,8 @@ public class VirtualNodeProvisioningTest { } catch (IllegalArgumentException e) { assertEquals("No allocation possible within limits: " + - "from 2 nodes with [vcpu: 1.0, memory: 5.0 Gb, disk 10.0 Gb, bandwidth: 1.0 Gbps, architecture: x86_64] " + - "to 4 nodes with [vcpu: 1.0, memory: 5.0 Gb, disk 10.0 Gb, bandwidth: 1.0 Gbps, architecture: x86_64]", + "from 2 nodes with [vcpu: 1.0, memory: 5.0 Gb, disk 10.0 Gb, bandwidth: 1.0 Gbps, architecture: any] " + + "to 4 nodes with [vcpu: 1.0, memory: 5.0 Gb, disk 10.0 Gb, bandwidth: 1.0 Gbps, architecture: any]", e.getMessage()); } } @@ -573,9 +573,9 @@ public class VirtualNodeProvisioningTest { } catch (IllegalArgumentException e) { assertEquals("No allocation possible within limits: " + - "from 2 nodes with [vcpu: 20.0, memory: 37.0 Gb, disk 100.0 Gb, bandwidth: 1.0 Gbps, architecture: x86_64] " + - "to 4 nodes with [vcpu: 20.0, memory: 37.0 Gb, disk 100.0 Gb, bandwidth: 1.0 Gbps, architecture: x86_64]. " + - "Nearest allowed node resources: [vcpu: 20.0, memory: 40.0 Gb, disk 100.0 Gb, bandwidth: 1.0 Gbps, storage type: remote, architecture: x86_64]", + "from 2 nodes with [vcpu: 20.0, memory: 37.0 Gb, disk 100.0 Gb, bandwidth: 1.0 Gbps, architecture: any] " + + "to 4 nodes with [vcpu: 20.0, memory: 37.0 Gb, disk 100.0 Gb, bandwidth: 1.0 Gbps, architecture: any]. " + + "Nearest allowed node resources: [vcpu: 20.0, memory: 40.0 Gb, disk 100.0 Gb, bandwidth: 1.0 Gbps, storage type: remote, architecture: any]", e.getMessage()); } } diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/responses/application1.json b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/responses/application1.json index 4c18b614075..92e5425e84e 100644 --- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/responses/application1.json +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/responses/application1.json @@ -14,7 +14,7 @@ "bandwidthGbps" : 1.0, "diskSpeed" : "fast", "storageType" : "any", - "architecture":"x86_64" + "architecture":"any" } }, "max" : { @@ -27,7 +27,7 @@ "bandwidthGbps" : 1.0, "diskSpeed" : "fast", "storageType" : "any", - "architecture":"x86_64" + "architecture":"any" } }, "current" : { @@ -56,7 +56,7 @@ "bandwidthGbps": 1.0, "diskSpeed": "fast", "storageType": "any", - "architecture": "x86_64" + "architecture": "any" } }, "at" : 123, @@ -89,7 +89,7 @@ "bandwidthGbps": 1.0, "diskSpeed": "fast", "storageType": "any", - "architecture": "x86_64" + "architecture": "any" } }, "at" : 123, @@ -121,7 +121,7 @@ "bandwidthGbps": 0.0, "diskSpeed": "fast", "storageType": "any", - "architecture":"x86_64" + "architecture":"any" } }, "to": { diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/responses/application2.json b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/responses/application2.json index ea4d5e01ebc..cba56e1c51e 100644 --- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/responses/application2.json +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/responses/application2.json @@ -14,7 +14,7 @@ "bandwidthGbps": 1.0, "diskSpeed": "fast", "storageType": "any", - "architecture":"x86_64" + "architecture":"any" } }, "max": { @@ -27,7 +27,7 @@ "bandwidthGbps": 1.0, "diskSpeed": "fast", "storageType": "any", - "architecture":"x86_64" + "architecture":"any" } }, "current": { @@ -75,7 +75,7 @@ "bandwidthGbps": 0.0, "diskSpeed": "fast", "storageType": "any", - "architecture":"x86_64" + "architecture":"any" } }, "to": { diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/responses/docker-container1.json b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/responses/docker-container1.json index a77e79170e7..8ef88eae97d 100644 --- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/responses/docker-container1.json +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/responses/docker-container1.json @@ -25,7 +25,7 @@ "currentRestartGeneration": 0, "wantedDockerImage": "docker-registry.domain.tld:8080/dist/vespa:6.42.0", "wantedVespaVersion": "6.42.0", - "requestedResources": { "vcpu":1.0, "memoryGb":4.0, "diskGb":100.0, "bandwidthGbps":1.0,"diskSpeed":"fast", "storageType":"any","architecture":"x86_64" }, + "requestedResources": { "vcpu":1.0, "memoryGb":4.0, "diskGb":100.0, "bandwidthGbps":1.0,"diskSpeed":"fast", "storageType":"any","architecture":"any" }, "rebootGeneration": 0, "currentRebootGeneration": 0, "failCount": 0, diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/responses/node1.json b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/responses/node1.json index 0b14c5769f6..d90ed692f1c 100644 --- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/responses/node1.json +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/responses/node1.json @@ -24,7 +24,7 @@ "currentRestartGeneration": 0, "wantedDockerImage": "docker-registry.domain.tld:8080/dist/vespa:6.42.0", "wantedVespaVersion": "6.42.0", - "requestedResources": { "vcpu":2.0, "memoryGb":8.0, "diskGb":50.0, "bandwidthGbps":1.0, "diskSpeed":"fast", "storageType":"any","architecture":"x86_64" }, + "requestedResources": { "vcpu":2.0, "memoryGb":8.0, "diskGb":50.0, "bandwidthGbps":1.0, "diskSpeed":"fast", "storageType":"any","architecture":"any" }, "rebootGeneration": 0, "currentRebootGeneration": 0, "failCount": 0, diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/responses/node10.json b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/responses/node10.json index 25f7b2a9c2d..bec194ea325 100644 --- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/responses/node10.json +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/responses/node10.json @@ -25,7 +25,7 @@ "currentRestartGeneration": 0, "wantedDockerImage": "docker-registry.domain.tld:8080/dist/vespa:6.42.0", "wantedVespaVersion": "6.42.0", - "requestedResources": { "vcpu":2.0, "memoryGb":8.0, "diskGb":50.0, "bandwidthGbps":1.0, "diskSpeed":"fast", "storageType":"any","architecture":"x86_64" }, + "requestedResources": { "vcpu":2.0, "memoryGb":8.0, "diskGb":50.0, "bandwidthGbps":1.0, "diskSpeed":"fast", "storageType":"any","architecture":"any" }, "rebootGeneration": 0, "currentRebootGeneration": 0, "vespaVersion": "5.104.142", diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/responses/node11.json b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/responses/node11.json index 72131465f29..d7e07f02f3a 100644 --- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/responses/node11.json +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/responses/node11.json @@ -5,9 +5,9 @@ "type": "tenant", "hostname": "host11.yahoo.com", "parentHostname": "parent.host.yahoo.com", - "flavor": "[vcpu: 1.0, memory: 1.0 Gb, disk 100.0 Gb, bandwidth: 0.3 Gbps, architecture: x86_64]", - "resources":{"vcpu":1.0,"memoryGb":1.0,"diskGb":100.0,"bandwidthGbps":0.3,"diskSpeed":"fast","storageType":"any","architecture":"x86_64"}, - "realResources":{"vcpu":1.0,"memoryGb":1.0,"diskGb":100.0,"bandwidthGbps":0.3,"diskSpeed":"fast","storageType":"any","architecture":"x86_64"}, + "flavor": "[vcpu: 1.0, memory: 1.0 Gb, disk 100.0 Gb, bandwidth: 0.3 Gbps, architecture: any]", + "resources":{"vcpu":1.0,"memoryGb":1.0,"diskGb":100.0,"bandwidthGbps":0.3,"diskSpeed":"fast","storageType":"any","architecture":"any"}, + "realResources":{"vcpu":1.0,"memoryGb":1.0,"diskGb":100.0,"bandwidthGbps":0.3,"diskSpeed":"fast","storageType":"any","architecture":"any"}, "environment": "DOCKER_CONTAINER", "rebootGeneration": 0, "currentRebootGeneration": 0, diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/responses/node13.json b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/responses/node13.json index f1fb2def612..73c34a7fa9e 100644 --- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/responses/node13.json +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/responses/node13.json @@ -24,7 +24,7 @@ "currentRestartGeneration": 0, "wantedDockerImage": "docker-registry.domain.tld:8080/dist/vespa:6.42.0", "wantedVespaVersion": "6.42.0", - "requestedResources": { "vcpu":10.0, "memoryGb":48.0, "diskGb":500.0, "bandwidthGbps":1.0, "diskSpeed":"fast", "storageType":"any","architecture":"x86_64" }, + "requestedResources": { "vcpu":10.0, "memoryGb":48.0, "diskGb":500.0, "bandwidthGbps":1.0, "diskSpeed":"fast", "storageType":"any","architecture":"any" }, "rebootGeneration": 0, "currentRebootGeneration": 0, "failCount": 0, diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/responses/node14.json b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/responses/node14.json index c19ca13066d..abb0ba57e49 100644 --- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/responses/node14.json +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/responses/node14.json @@ -24,7 +24,7 @@ "currentRestartGeneration": 0, "wantedDockerImage": "docker-registry.domain.tld:8080/dist/vespa:6.42.0", "wantedVespaVersion": "6.42.0", - "requestedResources": { "vcpu":10.0, "memoryGb":48.0, "diskGb":500.0, "bandwidthGbps":1.0, "diskSpeed":"fast", "storageType":"any","architecture":"x86_64" }, + "requestedResources": { "vcpu":10.0, "memoryGb":48.0, "diskGb":500.0, "bandwidthGbps":1.0, "diskSpeed":"fast", "storageType":"any","architecture":"any" }, "rebootGeneration": 0, "currentRebootGeneration": 0, "failCount": 0, diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/responses/node2.json b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/responses/node2.json index 40e32f5113e..9cd675163f0 100644 --- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/responses/node2.json +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/responses/node2.json @@ -24,7 +24,7 @@ "currentRestartGeneration": 0, "wantedDockerImage": "docker-registry.domain.tld:8080/dist/vespa:6.42.0", "wantedVespaVersion": "6.42.0", - "requestedResources": { "vcpu":2.0, "memoryGb":8.0, "diskGb":50.0, "bandwidthGbps":1.0, "diskSpeed":"fast", "storageType":"any","architecture":"x86_64" }, + "requestedResources": { "vcpu":2.0, "memoryGb":8.0, "diskGb":50.0, "bandwidthGbps":1.0, "diskSpeed":"fast", "storageType":"any","architecture":"any" }, "rebootGeneration": 0, "currentRebootGeneration": 0, "failCount": 0, diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/responses/node4-after-changes.json b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/responses/node4-after-changes.json index 6354fb6a894..03621c40f67 100644 --- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/responses/node4-after-changes.json +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/responses/node4-after-changes.json @@ -26,7 +26,7 @@ "currentRestartGeneration": 1, "wantedDockerImage": "docker-registry.domain.tld:8080/dist/vespa:6.42.0", "wantedVespaVersion": "6.42.0", - "requestedResources": { "vcpu":1.0, "memoryGb":4.0, "diskGb":100.0, "bandwidthGbps":1.0, "diskSpeed":"fast", "storageType":"any","architecture":"x86_64" }, + "requestedResources": { "vcpu":1.0, "memoryGb":4.0, "diskGb":100.0, "bandwidthGbps":1.0, "diskSpeed":"fast", "storageType":"any","architecture":"any" }, "orchestratorStatus": "ALLOWED_TO_BE_DOWN", "suspendedSinceMillis": 0, "rebootGeneration": 2, diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/responses/node4-wg.json b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/responses/node4-wg.json index 6510bdde0c8..a1883ba4b25 100644 --- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/responses/node4-wg.json +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/responses/node4-wg.json @@ -25,7 +25,7 @@ "currentRestartGeneration": 0, "wantedDockerImage": "docker-registry.domain.tld:8080/dist/vespa:6.42.0", "wantedVespaVersion": "6.42.0", - "requestedResources": { "vcpu":1.0, "memoryGb":4.0, "diskGb":100.0, "bandwidthGbps":1.0, "diskSpeed":"fast", "storageType":"any","architecture":"x86_64" }, + "requestedResources": { "vcpu":1.0, "memoryGb":4.0, "diskGb":100.0, "bandwidthGbps":1.0, "diskSpeed":"fast", "storageType":"any","architecture":"any" }, "rebootGeneration": 0, "currentRebootGeneration": 0, "vespaVersion": "6.41.0", diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/responses/node4-with-hostnames.json b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/responses/node4-with-hostnames.json index 6eae33eda15..50007fd6610 100644 --- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/responses/node4-with-hostnames.json +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/responses/node4-with-hostnames.json @@ -25,7 +25,7 @@ "currentRestartGeneration": 0, "wantedDockerImage": "docker-registry.domain.tld:8080/dist/vespa:6.42.0", "wantedVespaVersion": "6.42.0", - "requestedResources": { "vcpu":1.0, "memoryGb":4.0, "diskGb":100.0, "bandwidthGbps":1.0, "diskSpeed":"fast", "storageType":"any","architecture":"x86_64" }, + "requestedResources": { "vcpu":1.0, "memoryGb":4.0, "diskGb":100.0, "bandwidthGbps":1.0, "diskSpeed":"fast", "storageType":"any","architecture":"any" }, "rebootGeneration": 0, "currentRebootGeneration": 0, "vespaVersion": "6.41.0", diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/responses/node4.json b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/responses/node4.json index 961c03570e6..f206adf4366 100644 --- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/responses/node4.json +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/responses/node4.json @@ -25,7 +25,7 @@ "currentRestartGeneration": 0, "wantedDockerImage": "docker-registry.domain.tld:8080/dist/vespa:6.42.0", "wantedVespaVersion": "6.42.0", - "requestedResources": { "vcpu":1.0, "memoryGb":4.0, "diskGb":100.0, "bandwidthGbps":1.0, "diskSpeed":"fast", "storageType":"any","architecture":"x86_64" }, + "requestedResources": { "vcpu":1.0, "memoryGb":4.0, "diskGb":100.0, "bandwidthGbps":1.0, "diskSpeed":"fast", "storageType":"any","architecture":"any" }, "rebootGeneration": 0, "currentRebootGeneration": 0, "vespaVersion": "6.41.0", diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/responses/node6.json b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/responses/node6.json index de286d1f55f..69316b1ca7f 100644 --- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/responses/node6.json +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/responses/node6.json @@ -24,7 +24,7 @@ "currentRestartGeneration": 0, "wantedDockerImage": "docker-registry.domain.tld:8080/dist/vespa:6.42.0", "wantedVespaVersion": "6.42.0", - "requestedResources": { "vcpu":2.0, "memoryGb":8.0, "diskGb":50.0, "bandwidthGbps":1.0, "diskSpeed":"fast", "storageType":"any","architecture":"x86_64" }, + "requestedResources": { "vcpu":2.0, "memoryGb":8.0, "diskGb":50.0, "bandwidthGbps":1.0, "diskSpeed":"fast", "storageType":"any","architecture":"any" }, "rebootGeneration": 0, "currentRebootGeneration": 0, "failCount": 0, diff --git a/vdslib/src/main/java/com/yahoo/vdslib/SearchResult.java b/vdslib/src/main/java/com/yahoo/vdslib/SearchResult.java index e263c292bc8..b7c9b1b71b5 100644 --- a/vdslib/src/main/java/com/yahoo/vdslib/SearchResult.java +++ b/vdslib/src/main/java/com/yahoo/vdslib/SearchResult.java @@ -1,28 +1,40 @@ // Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.vdslib; +import com.yahoo.data.access.helpers.MatchFeatureData; import com.yahoo.vespa.objects.BufferSerializer; import com.yahoo.vespa.objects.Deserializer; import java.io.UnsupportedEncodingException; import java.nio.ByteOrder; +import java.util.ArrayList; import java.util.Map; +import java.util.Optional; import java.util.TreeMap; public class SearchResult { public static class Hit implements Comparable<Hit> { private String docId; - private double rank; + private double rank; + private MatchFeatureData.HitValue matchFeatures; public Hit(Hit h) { docId = h.docId; rank = h.rank; + matchFeatures = h.matchFeatures; } public Hit(String docId, double rank) { this.rank = rank; this.docId = docId; + this.matchFeatures = null; + } + final public String getDocId() { return docId; } + final public double getRank() { return rank; } + final public Optional<MatchFeatureData.HitValue> getMatchFeatures() { + return Optional.ofNullable(matchFeatures); } - final public String getDocId() { return docId; } - final public double getRank() { return rank; } final public void setRank(double rank) { this.rank = rank; } + final public void setMatchFeatures(MatchFeatureData.HitValue matchFeatures) { + this.matchFeatures = matchFeatures; + } public int compareTo(Hit h) { return (h.rank < rank) ? -1 : (h.rank > rank) ? 1 : 0; // Sort order: descending } @@ -49,12 +61,19 @@ public class SearchResult { private Hit[] hits; private TreeMap<Integer, byte []> aggregatorList; private TreeMap<Integer, byte []> groupingList; + private static int EXTENSION_FLAGS_PRESENT = -1; + private static int MATCH_FEATURES_PRESENT_MASK = 1; public SearchResult(Deserializer buf) { BufferSerializer bser = (BufferSerializer) buf; // TODO: dirty cast. must do this differently bser.order(ByteOrder.BIG_ENDIAN); this.totalHits = buf.getInt(null); int numHits = buf.getInt(null); + int extensionFlags = 0; + if (hasExtensionFlags(numHits)) { + extensionFlags = buf.getInt(null); + numHits = buf.getInt(null); + } hits = new Hit[numHits]; if (numHits != 0) { int docIdBufferLength = buf.getInt(null); @@ -101,7 +120,45 @@ public class SearchResult { groupingList.put(aggrId, buf.getBytes(null, aggrLength)); } + if (hasMatchFeatures(extensionFlags)) { + deserializeMatchFeatures(buf, numHits); + } } + + private void deserializeMatchFeatures(Deserializer buf, int numHits) { + var featureNames = new ArrayList<String>(); + int numFeatures = buf.getInt(null); + for (int i = 0; i < numFeatures; ++i) { + featureNames.add(buf.getString(null)); + } + var factory = new MatchFeatureData(featureNames); + for (int i = 0; i < numHits; ++i) { + var matchFeatures = factory.addHit(); + for (int j = 0; j < numFeatures; ++j) { + byte featureType = buf.getByte(null); + if (isDoubleFeature(featureType)) { + matchFeatures.set(j, buf.getDouble(null)); + } else { + int bufLength = buf.getInt(null); + matchFeatures.set(j, buf.getBytes(null, bufLength)); + } + } + hits[i].setMatchFeatures(matchFeatures); + } + } + + private static boolean hasExtensionFlags(int numHits) { + return numHits == EXTENSION_FLAGS_PRESENT; + } + + private static boolean hasMatchFeatures(int extensionFlags) { + return (extensionFlags & MATCH_FEATURES_PRESENT_MASK) != 0; + } + + private static boolean isDoubleFeature(byte featureType) { + return featureType == 0; + } + /** * Constructs a new message from a byte buffer. * diff --git a/vdslib/src/vespa/vdslib/container/searchresult.cpp b/vdslib/src/vespa/vdslib/container/searchresult.cpp index c8bc331d1a8..e348c9d9e13 100644 --- a/vdslib/src/vespa/vdslib/container/searchresult.cpp +++ b/vdslib/src/vespa/vdslib/container/searchresult.cpp @@ -11,21 +11,21 @@ namespace vdslib { namespace { // Magic value for hit count to enable extension flags -constexpr uint32_t enable_extension_flags_magic = 0xffffffffu; +constexpr uint32_t extension_flags_present = 0xffffffffu; // Extension flag values -constexpr uint32_t match_features_present = 1; +constexpr uint32_t match_features_present_mask = 1; // Selector values for feature value constexpr uint8_t feature_value_is_double = 0; constexpr uint8_t feature_value_is_data = 1; inline bool has_match_features(uint32_t extension_flags) { - return ((extension_flags & match_features_present) != 0); + return ((extension_flags & match_features_present_mask) != 0); } inline bool must_serialize_extension_flags(uint32_t extension_flags, uint32_t hit_count) { - return ((extension_flags != 0) || (hit_count == enable_extension_flags_magic)); + return ((extension_flags != 0) || (hit_count == extension_flags_present)); } } @@ -155,7 +155,7 @@ SearchResult::deserialize(document::ByteBuffer & buf) uint32_t numResults(0), bufSize(0); buf.getIntNetwork(tmp); numResults = tmp; uint32_t extension_flags = 0u; - if (numResults == enable_extension_flags_magic) { + if (numResults == extension_flags_present) { buf.getIntNetwork(tmp); extension_flags = tmp; buf.getIntNetwork(tmp); @@ -189,7 +189,7 @@ void SearchResult::serialize(vespalib::GrowableByteBuffer & buf) const uint32_t hitCount = std::min(_hits.size(), _wantedHits); uint32_t extension_flags = calc_extension_flags(hitCount); if (must_serialize_extension_flags(extension_flags, hitCount)) { - buf.putInt(enable_extension_flags_magic); + buf.putInt(extension_flags_present); buf.putInt(extension_flags); } buf.putInt(hitCount); @@ -241,7 +241,7 @@ SearchResult::calc_extension_flags(uint32_t hit_count) const noexcept { uint32_t extension_flags = 0u; if (!_match_features.names.empty() && hit_count != 0) { - extension_flags |= match_features_present; + extension_flags |= match_features_present_mask; } return extension_flags; } @@ -251,7 +251,7 @@ SearchResult::get_match_features_serialized_size(uint32_t hit_count) const noexc { uint32_t size = sizeof(uint32_t); for (auto& name : _match_features.names) { - size += sizeof(uint32_t) + name.size(); + size += sizeof(uint32_t) + name.size() + 1; } for (uint32_t i = 0; i < hit_count; ++i) { auto mfv = get_match_feature_values(i); @@ -271,7 +271,7 @@ SearchResult::serialize_match_features(vespalib::GrowableByteBuffer& buf, uint32 { buf.putInt(_match_features.names.size()); for (auto& name : _match_features.names) { - buf.putString(name); + buf.put_c_string(name); } for (uint32_t i = 0; i < hit_count; ++i) { auto mfv = get_match_feature_values(i); @@ -301,10 +301,11 @@ SearchResult::deserialize_match_features(document::ByteBuffer& buf) _match_features.names.resize(num_features); for (auto& name : _match_features.names) { buf.getIntNetwork(tmp); - name.resize(tmp); - if (tmp != 0) { - buf.getBytes(&name[0], tmp); + if (tmp > 1) { + name.resize(tmp - 1); + buf.getBytes(&name[0], tmp - 1); } + buf.getByte(selector); // Read and ignore the nul-termination. } uint32_t hit_count = _hits.size(); uint32_t num_values = num_features * hit_count; diff --git a/vespa-feed-client/src/main/java/ai/vespa/feed/client/impl/HttpRequestStrategy.java b/vespa-feed-client/src/main/java/ai/vespa/feed/client/impl/HttpRequestStrategy.java index 4e2256f40b4..ce86ad59ffe 100644 --- a/vespa-feed-client/src/main/java/ai/vespa/feed/client/impl/HttpRequestStrategy.java +++ b/vespa-feed-client/src/main/java/ai/vespa/feed/client/impl/HttpRequestStrategy.java @@ -9,7 +9,6 @@ import ai.vespa.feed.client.FeedException; import ai.vespa.feed.client.HttpResponse ; import ai.vespa.feed.client.OperationStats; -import javax.net.ssl.SSLException; import java.io.IOException; import java.nio.channels.CancelledKeyException; import java.util.Map; @@ -139,9 +138,9 @@ class HttpRequestStrategy implements RequestStrategy { */ private boolean retry(HttpRequest request, Throwable thrown, int attempt) { breaker.failure(thrown); - if ( (thrown instanceof IOException && ! (thrown instanceof SSLException)) // General IO problems, but not SSL, which is irrecoverable. - || (thrown instanceof CancellationException) // TLS session disconnect. - || (thrown instanceof CancelledKeyException)) { // Selection cancelled. + if ( (thrown instanceof IOException) // General IO problems. + || (thrown instanceof CancellationException) // TLS session disconnect. + || (thrown instanceof CancelledKeyException)) { // Selection cancelled. log.log(FINER, thrown, () -> "Failed attempt " + attempt + " at " + request); return retry(request, attempt); } diff --git a/vespaclient-java/src/main/java/com/yahoo/vespa/security/tool/CliUtils.java b/vespaclient-java/src/main/java/com/yahoo/vespa/security/tool/CliUtils.java index a60c3647b41..b09ae17cd77 100644 --- a/vespaclient-java/src/main/java/com/yahoo/vespa/security/tool/CliUtils.java +++ b/vespaclient-java/src/main/java/com/yahoo/vespa/security/tool/CliUtils.java @@ -8,6 +8,7 @@ import java.io.InputStream; import java.io.OutputStream; import java.nio.file.Files; import java.nio.file.Paths; +import java.nio.file.attribute.PosixFilePermissions; /** * @author vekterli @@ -43,7 +44,10 @@ public class CliUtils { return stdOut; } else { // TODO fail if file already exists? - return Files.newOutputStream(Paths.get(pathOrDash)); + var privFilePerms = PosixFilePermissions.fromString("rw-------"); + var outPath = Paths.get(pathOrDash); + Files.createFile(outPath, PosixFilePermissions.asFileAttribute(privFilePerms)); + return Files.newOutputStream(outPath); } } diff --git a/vespaclient-java/src/test/java/com/yahoo/vespa/security/tool/CryptoToolsTest.java b/vespaclient-java/src/test/java/com/yahoo/vespa/security/tool/CryptoToolsTest.java index f55278342e1..05d7e8c9511 100644 --- a/vespaclient-java/src/test/java/com/yahoo/vespa/security/tool/CryptoToolsTest.java +++ b/vespaclient-java/src/test/java/com/yahoo/vespa/security/tool/CryptoToolsTest.java @@ -64,6 +64,12 @@ public class CryptoToolsTest { Files.writeString(keyPath, contents); } + private static void assertOnlyFileOwnerHasAccessRights(Path file) throws IOException { + var actualFilePerms = Files.getPosixFilePermissions(file); + var expectedPerms = PosixFilePermissions.fromString("rw-------"); + assertEquals(expectedPerms, actualFilePerms); + } + @Test void top_level_help_page_printed_if_help_option_given() throws IOException { verifyStdoutMatchesFile(List.of("--help"), "expected-help-output.txt"); @@ -180,9 +186,7 @@ public class CryptoToolsTest { "--private-out-file", absPathOf(privKeyFile), "--public-out-file", absPathOf(pubKeyFile))); assertEquals(0, procOut.exitCode()); - var privKeyPerms = Files.getPosixFilePermissions(privKeyFile); - var expectedPerms = PosixFilePermissions.fromString("rw-------"); - assertEquals(expectedPerms, privKeyPerms); + assertOnlyFileOwnerHasAccessRights(privKeyFile); } private static final String TEST_PRIV_KEY = "GFg54SaGNCmcSGufZCx68SKLGuAFrASoDeMk3t5AjU6L"; @@ -381,6 +385,7 @@ public class CryptoToolsTest { assertEquals("", procOut.stdErr()); assertEquals(greatSecret, Files.readString(decryptedPath)); + assertOnlyFileOwnerHasAccessRights(decryptedPath); } @Test diff --git a/vespalib/src/vespa/vespalib/util/growablebytebuffer.cpp b/vespalib/src/vespa/vespalib/util/growablebytebuffer.cpp index 424ad7ba470..fa0dc9bf99e 100644 --- a/vespalib/src/vespa/vespalib/util/growablebytebuffer.cpp +++ b/vespalib/src/vespa/vespalib/util/growablebytebuffer.cpp @@ -76,6 +76,14 @@ GrowableByteBuffer::putString(vespalib::stringref v) } void +GrowableByteBuffer::put_c_string(vespalib::stringref v) +{ + putInt(v.size() + 1); + putBytes(v.data(), v.size()); + putByte(0); +} + +void GrowableByteBuffer::putByte(uint8_t v) { putBytes(reinterpret_cast<const char*>(&v), sizeof(v)); diff --git a/vespalib/src/vespa/vespalib/util/growablebytebuffer.h b/vespalib/src/vespa/vespalib/util/growablebytebuffer.h index b0fb30606d4..61698868dba 100644 --- a/vespalib/src/vespa/vespalib/util/growablebytebuffer.h +++ b/vespalib/src/vespa/vespalib/util/growablebytebuffer.h @@ -68,11 +68,17 @@ public: void putDouble(double v); /** - Adds a string to the buffer. + Adds a string to the buffer (without nul-termination). */ void putString(vespalib::stringref v); /** + * Adds a string to the buffer (including nul-termination). + * This matches com.yahoo.vespa.objects.Deserializer.getString. + */ + void put_c_string(vespalib::stringref v); + + /** Adds a single byte to the buffer. */ void putByte(uint8_t v); |