diff options
34 files changed, 817 insertions, 683 deletions
diff --git a/config-model/src/main/java/com/yahoo/vespa/model/container/http/ssl/HostedSslConnectorFactory.java b/config-model/src/main/java/com/yahoo/vespa/model/container/http/ssl/HostedSslConnectorFactory.java index 12db3b87243..0ad9bd9e883 100644 --- a/config-model/src/main/java/com/yahoo/vespa/model/container/http/ssl/HostedSslConnectorFactory.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/container/http/ssl/HostedSslConnectorFactory.java @@ -17,21 +17,26 @@ import java.util.List; public class HostedSslConnectorFactory extends ConnectorFactory { private static final List<String> INSECURE_WHITELISTED_PATHS = List.of("/status.html"); + private static final String DEFAULT_HOSTED_TRUSTSTORE = "/opt/yahoo/share/ssl/certs/athenz_certificate_bundle.pem"; private final boolean enforceClientAuth; /** - * Create connector factory that uses a certificate provided by the config-model / configserver. + * Create connector factory that uses a certificate provided by the config-model / configserver and default hosted Vespa truststore. */ + // TODO Enforce client authentication public static HostedSslConnectorFactory withProvidedCertificate(String serverName, EndpointCertificateSecrets endpointCertificateSecrets) { - return new HostedSslConnectorFactory(createConfiguredDirectSslProvider(serverName, endpointCertificateSecrets, /*tlsCaCertificates*/null), false); + return new HostedSslConnectorFactory( + createConfiguredDirectSslProvider(serverName, endpointCertificateSecrets, DEFAULT_HOSTED_TRUSTSTORE, /*tlsCaCertificates*/null), false); } /** * Create connector factory that uses a certificate provided by the config-model / configserver and a truststore configured by the application. */ - public static HostedSslConnectorFactory withProvidedCertificateAndTruststore(String serverName, EndpointCertificateSecrets endpointCertificateSecrets, String tlsCaCertificates) { - return new HostedSslConnectorFactory(createConfiguredDirectSslProvider(serverName, endpointCertificateSecrets, tlsCaCertificates), true); + public static HostedSslConnectorFactory withProvidedCertificateAndTruststore( + String serverName, EndpointCertificateSecrets endpointCertificateSecrets, String tlsCaCertificates) { + return new HostedSslConnectorFactory( + createConfiguredDirectSslProvider(serverName, endpointCertificateSecrets, /*tlsCaCertificatesPath*/null, tlsCaCertificates), true); } /** @@ -47,12 +52,12 @@ public class HostedSslConnectorFactory extends ConnectorFactory { } private static ConfiguredDirectSslProvider createConfiguredDirectSslProvider( - String serverName, EndpointCertificateSecrets endpointCertificateSecrets, String tlsCaCertificates) { + String serverName, EndpointCertificateSecrets endpointCertificateSecrets, String tlsCaCertificatesPath, String tlsCaCertificates) { return new ConfiguredDirectSslProvider( serverName, endpointCertificateSecrets.key(), endpointCertificateSecrets.certificate(), - /*caCertificatePath*/null, + tlsCaCertificatesPath, tlsCaCertificates, ClientAuth.Enum.WANT_AUTH); } diff --git a/config-model/src/test/java/com/yahoo/vespa/model/container/xml/ContainerModelBuilderTest.java b/config-model/src/test/java/com/yahoo/vespa/model/container/xml/ContainerModelBuilderTest.java index 1bbc4ea2684..edbddcd4804 100644 --- a/config-model/src/test/java/com/yahoo/vespa/model/container/xml/ContainerModelBuilderTest.java +++ b/config-model/src/test/java/com/yahoo/vespa/model/container/xml/ContainerModelBuilderTest.java @@ -39,7 +39,6 @@ import com.yahoo.vespa.defaults.Defaults; import com.yahoo.vespa.model.AbstractService; import com.yahoo.vespa.model.VespaModel; import com.yahoo.vespa.model.container.ApplicationContainer; -import com.yahoo.vespa.model.container.Container; import com.yahoo.vespa.model.container.ContainerCluster; import com.yahoo.vespa.model.container.SecretStore; import com.yahoo.vespa.model.container.component.Component; @@ -71,7 +70,9 @@ import static org.hamcrest.CoreMatchers.not; import static org.hamcrest.CoreMatchers.nullValue; import static org.hamcrest.Matchers.contains; import static org.hamcrest.Matchers.containsString; +import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.hasItem; +import static org.hamcrest.Matchers.isEmptyString; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNotNull; @@ -801,6 +802,10 @@ public class ContainerModelBuilderTest extends ContainerModelBuilderTestBase { assertEquals("CERT", connectorConfig.ssl().certificate()); assertEquals("KEY", connectorConfig.ssl().privateKey()); assertEquals(4443, connectorConfig.listenPort()); + + assertThat("Connector must use Athenz truststore in a non-public system.", + connectorConfig.ssl().caCertificateFile(), equalTo("/opt/yahoo/share/ssl/certs/athenz_certificate_bundle.pem")); + assertThat(connectorConfig.ssl().caCertificate(), isEmptyString()); } diff --git a/container-search/src/main/java/com/yahoo/search/federation/FederationSearcher.java b/container-search/src/main/java/com/yahoo/search/federation/FederationSearcher.java index c3ede4fe20a..d0b3189a79c 100644 --- a/container-search/src/main/java/com/yahoo/search/federation/FederationSearcher.java +++ b/container-search/src/main/java/com/yahoo/search/federation/FederationSearcher.java @@ -110,7 +110,7 @@ public class FederationSearcher extends ForkingSearcher { // for testing public FederationSearcher(ComponentId id, SearchChainResolver searchChainResolver) { - this(searchChainResolver, false, PropagateSourceProperties.ALL, null); + this(searchChainResolver, false, PropagateSourceProperties.EVERY, null); } private FederationSearcher(SearchChainResolver searchChainResolver, @@ -271,7 +271,10 @@ public class FederationSearcher extends ForkingSearcher { outgoing.setTimeout(timeout); switch (propagateSourceProperties) { - case ALL: + case EVERY: + propagatePerSourceQueryProperties(query, outgoing, window, sourceName, providerName, null); + break; + case NATIVE: case ALL: propagatePerSourceQueryProperties(query, outgoing, window, sourceName, providerName, Query.nativeProperties); break; case OFFSET_HITS: @@ -288,10 +291,21 @@ public class FederationSearcher extends ForkingSearcher { private void propagatePerSourceQueryProperties(Query original, Query outgoing, Window window, String sourceName, String providerName, List<CompoundName> queryProperties) { - for (CompoundName key : queryProperties) { - Object value = getSourceOrProviderProperty(original, key, sourceName, providerName, window.get(key)); - if (value != null) - outgoing.properties().set(key, value); + if (queryProperties == null) { + outgoing.setHits(window.hits); + outgoing.setOffset(window.offset); + original.properties().listProperties(CompoundName.fromComponents("provider", providerName)).forEach((k, v) -> + outgoing.properties().set(k, v)); + original.properties().listProperties(CompoundName.fromComponents("source", sourceName)).forEach((k, v) -> + outgoing.properties().set(k, v)); + } + else { + for (CompoundName key : queryProperties) { + Object value = getSourceOrProviderProperty(original, key, sourceName, providerName, window.get(key)); + if (value != null) + outgoing.properties().set(key, value); + if (value != null) System.out.println("Setting " + key + " = " + value); + } } } @@ -319,7 +333,7 @@ public class FederationSearcher extends ForkingSearcher { private ErrorMessage missingSearchChainsErrorMessage(List<UnresolvedSearchChainException> unresolvedSearchChainExceptions) { String message = String.join(" ", getMessagesSet(unresolvedSearchChainExceptions)) + - " Valid source refs are " + String.join(", ", allSourceRefDescriptions()) +'.'; + " Valid source refs are " + String.join(", ", allSourceRefDescriptions()) +'.'; return ErrorMessage.createInvalidQueryParameter(message); } diff --git a/container-search/src/main/resources/configdefinitions/strict-contracts.def b/container-search/src/main/resources/configdefinitions/strict-contracts.def index f9dd788c054..5ceb37db8d1 100644 --- a/container-search/src/main/resources/configdefinitions/strict-contracts.def +++ b/container-search/src/main/resources/configdefinitions/strict-contracts.def @@ -1,6 +1,7 @@ # Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. namespace=search.federation +## DEPRECATED: This config will be removed on Vespa 8 ## A config to control whether to activate strict adherence to public contracts ## in the container. Usually, the container tries to do a best effort of hiding ## some undesirable effects of the the public contracts. Modifying this config @@ -11,11 +12,9 @@ namespace=search.federation ## can be construed to be unnecessary. searchchains bool default=false -# WARNING: Beta feature, might be removed soon. -# Propagate source.(sourceName).{QueryProperties.PER_SOURCE_QUERY_PROPERTIES} and -# provider.(providerName).{QueryProperties.PER_SOURCE_QUERY_PROPERTIES} -# to the outgoing query. -# All means all in QueryProperties.PER_SOURCE_QUERY_PROPERTIES -# OFFSET_HITS means {Query.HITS, Query.OFFSET} -# NONE means {} -propagateSourceProperties enum {ALL, OFFSET_HITS, NONE} default=ALL +# EVERY, // Propagate any property starting by source.[sourceName] and provider.[providerName] +# NATIVE, // Propagate native properties only +# ALL, // Deprecated synonym of NATIVE +# OFFSET_HITS, // Propagate offset ands hits only +# NONE // propagate no properties +propagateSourceProperties enum {EVERY, NATIVE, ALL, OFFSET_HITS, NONE} default=EVERY diff --git a/container-search/src/test/java/com/yahoo/prelude/searcher/test/BlendingSearcherTestCase.java b/container-search/src/test/java/com/yahoo/prelude/searcher/test/BlendingSearcherTestCase.java index 9da1c184505..0086f1b3571 100644 --- a/container-search/src/test/java/com/yahoo/prelude/searcher/test/BlendingSearcherTestCase.java +++ b/container-search/src/test/java/com/yahoo/prelude/searcher/test/BlendingSearcherTestCase.java @@ -108,7 +108,7 @@ public class BlendingSearcherTestCase { entry.getValue())); } - StrictContractsConfig contracts = new StrictContractsConfig(new StrictContractsConfig.Builder()); + StrictContractsConfig contracts = new StrictContractsConfig.Builder().build(); FederationSearcher fedSearcher = new FederationSearcher(new FederationConfig(builder), contracts, new ComponentRegistry<>()); @@ -124,7 +124,6 @@ public class BlendingSearcherTestCase { @Test public void testitTwoPhase() { - DocumentSourceSearcher chain1 = new DocumentSourceSearcher(); DocumentSourceSearcher chain2 = new DocumentSourceSearcher(); DocumentSourceSearcher chain3 = new DocumentSourceSearcher(); diff --git a/container-search/src/test/java/com/yahoo/search/federation/test/FederationSearcherTestCase.java b/container-search/src/test/java/com/yahoo/search/federation/test/FederationSearcherTestCase.java index 111b7a8eb69..65cb4dff1f8 100644 --- a/container-search/src/test/java/com/yahoo/search/federation/test/FederationSearcherTestCase.java +++ b/container-search/src/test/java/com/yahoo/search/federation/test/FederationSearcherTestCase.java @@ -201,13 +201,33 @@ public class FederationSearcherTestCase { } @Test - public void testPropertyPropagation() { - Result result = searchWithPropertyPropagation(PropagateSourceProperties.ALL); + public void testPropertyPropagation_native() { + Result result = searchWithPropertyPropagation(PropagateSourceProperties.NATIVE); assertEquals("source:mySource1", result.hits().get(0).getId().stringValue()); assertEquals("source:mySource2", result.hits().get(1).getId().stringValue()); assertEquals("nalle", result.hits().get(0).getQuery().getPresentation().getSummary()); assertNull(result.hits().get(1).getQuery().getPresentation().getSummary()); + assertEquals(null, result.hits().get(0).getQuery().properties().get("custom")); + } + + @Test + public void testPropertyPropagation_every() { + Result result = searchWithPropertyPropagation(PropagateSourceProperties.EVERY); + + assertEquals("source:mySource1", result.hits().get(0).getId().stringValue()); + assertEquals("source:mySource2", result.hits().get(1).getId().stringValue()); + assertEquals("nalle", result.hits().get(0).getQuery().getPresentation().getSummary()); + assertEquals("foo", result.hits().get(0).getQuery().properties().get("customSourceProperty")); + assertEquals(null, result.hits().get(1).getQuery().properties().get("customSourceProperty")); + assertEquals(null, result.hits().get(0).getQuery().properties().get("custom.source.property")); + assertEquals("bar", result.hits().get(1).getQuery().properties().get("custom.source.property")); + assertEquals(13, result.hits().get(0).getQuery().properties().get("hits")); + assertEquals(1, result.hits().get(0).getQuery().properties().get("offset")); + assertEquals(10, result.hits().get(1).getQuery().properties().get("hits")); + assertEquals(0, result.hits().get(1).getQuery().properties().get("offset")); + + assertNull(result.hits().get(1).getQuery().getPresentation().getSummary()); } private Result searchWithPropertyPropagation(PropagateSourceProperties.Enum propagateSourceProperties) { @@ -215,7 +235,7 @@ public class FederationSearcherTestCase { addChained(new MockSearcher(), "mySource2"); Chain<Searcher> mainChain = new Chain<>("default", createFederationSearcher(propagateSourceProperties)); - Query q = new Query(QueryTestCase.httpEncode("?query=test&source.mySource1.presentation.summary=nalle")); + Query q = new Query(QueryTestCase.httpEncode("?query=test&source.mySource1.presentation.summary=nalle&source.mySource1.customSourceProperty=foo&source.mySource2.custom.source.property=bar&source.mySource1.hits=13&source.mySource1.offset=1")); Result result = new Execution(mainChain, Execution.Context.createContextStub(chainRegistry, null)).search(q); assertNull(result.hits().getError()); diff --git a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/deployment/JobType.java b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/deployment/JobType.java index 7aac0826b2b..1f546989b8d 100644 --- a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/deployment/JobType.java +++ b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/deployment/JobType.java @@ -143,15 +143,15 @@ public enum JobType { private final String jobName; private final Map<SystemName, ZoneId> zones; - private final boolean isTest; + private final boolean isProductionTest; - JobType(String jobName, Map<SystemName, ZoneId> zones, boolean isTest) { + JobType(String jobName, Map<SystemName, ZoneId> zones, boolean isProductionTest) { if (zones.values().stream().map(ZoneId::environment).distinct().count() > 1) throw new IllegalArgumentException("All zones of a job must be in the same environment"); this.jobName = jobName; this.zones = zones; - this.isTest = isTest; + this.isProductionTest = isProductionTest; } JobType(String jobName, Map<SystemName, ZoneId> zones) { @@ -176,10 +176,10 @@ public enum JobType { public boolean isProduction() { return environment() == Environment.prod; } /** Returns whether this job runs tests */ - public boolean isTest() { return isTest; } + public boolean isTest() { return isProductionTest || environment().isTest(); } /** Returns whether this job deploys to a zone */ - public boolean isDeployment() { return ! (isProduction() && isTest); } + public boolean isDeployment() { return ! (isProduction() && isProductionTest); } /** Returns the environment of this job type, or null if it does not have an environment */ public Environment environment() { @@ -206,7 +206,7 @@ public enum JobType { /** Returns the job type for the given zone */ public static Optional<JobType> from(SystemName system, ZoneId zone) { - return from(system, zone, false); + return from(system, zone, zone.environment().isTest()); } /** Returns the production test job type for the given environment and region or null if none */ diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/ApplicationController.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/ApplicationController.java index 66a7acc6ddf..c44c533e995 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/ApplicationController.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/ApplicationController.java @@ -13,6 +13,7 @@ import com.yahoo.config.provision.Environment; import com.yahoo.config.provision.InstanceName; import com.yahoo.config.provision.TenantName; import com.yahoo.config.provision.zone.ZoneId; +import com.yahoo.container.jdisc.secretstore.SecretStore; import com.yahoo.vespa.athenz.api.AthenzDomain; import com.yahoo.vespa.athenz.api.AthenzIdentity; import com.yahoo.vespa.athenz.api.AthenzPrincipal; @@ -27,7 +28,6 @@ import com.yahoo.vespa.hosted.controller.api.identifiers.DeploymentId; import com.yahoo.vespa.hosted.controller.api.identifiers.Hostname; import com.yahoo.vespa.hosted.controller.api.identifiers.InstanceId; import com.yahoo.vespa.hosted.controller.api.identifiers.RevisionId; -import com.yahoo.vespa.hosted.controller.api.integration.certificates.ApplicationCertificate; import com.yahoo.vespa.hosted.controller.api.integration.certificates.EndpointCertificateMetadata; import com.yahoo.vespa.hosted.controller.api.integration.configserver.ConfigServer; import com.yahoo.vespa.hosted.controller.api.integration.configserver.ConfigServerException; @@ -51,7 +51,6 @@ import com.yahoo.vespa.hosted.controller.application.ApplicationPackageValidator import com.yahoo.vespa.hosted.controller.application.Deployment; import com.yahoo.vespa.hosted.controller.application.DeploymentMetrics; import com.yahoo.vespa.hosted.controller.application.Endpoint; -import com.yahoo.vespa.hosted.controller.application.EndpointId; import com.yahoo.vespa.hosted.controller.application.SystemApplication; import com.yahoo.vespa.hosted.controller.application.TenantAndApplicationId; import com.yahoo.vespa.hosted.controller.athenz.impl.AthenzFacade; @@ -60,7 +59,7 @@ import com.yahoo.vespa.hosted.controller.deployment.DeploymentTrigger; import com.yahoo.vespa.hosted.controller.deployment.Run; import com.yahoo.vespa.hosted.controller.deployment.Versions; import com.yahoo.vespa.hosted.controller.dns.NameServiceQueue.Priority; -import com.yahoo.vespa.hosted.controller.persistence.EndpointCertificateMetadataSerializer; +import com.yahoo.vespa.hosted.controller.endpointcertificates.EndpointCertificateManager; import com.yahoo.vespa.hosted.controller.routing.RoutingPolicies; import com.yahoo.vespa.hosted.controller.persistence.CuratorDb; import com.yahoo.vespa.hosted.controller.rotation.RotationLock; @@ -95,7 +94,6 @@ import java.util.function.Consumer; import java.util.logging.Level; import java.util.logging.Logger; import java.util.stream.Collectors; -import java.util.stream.Stream; import static com.yahoo.vespa.hosted.controller.api.integration.configserver.Node.State.active; import static com.yahoo.vespa.hosted.controller.api.integration.configserver.Node.State.reserved; @@ -129,10 +127,11 @@ public class ApplicationController { private final Clock clock; private final DeploymentTrigger deploymentTrigger; private final ApplicationPackageValidator applicationPackageValidator; + private final EndpointCertificateManager endpointCertificateManager; ApplicationController(Controller controller, CuratorDb curator, AccessControl accessControl, RotationsConfig rotationsConfig, - Clock clock) { + Clock clock, SecretStore secretStore) { this.controller = controller; this.curator = curator; this.accessControl = accessControl; @@ -146,6 +145,8 @@ public class ApplicationController { rotationRepository = new RotationRepository(rotationsConfig, this, curator); deploymentTrigger = new DeploymentTrigger(controller, clock); applicationPackageValidator = new ApplicationPackageValidator(controller); + endpointCertificateManager = new EndpointCertificateManager(controller.zoneRegistry(), curator, secretStore, + controller.serviceRegistry().applicationCertificateProvider(), clock); // Update serialization format of all applications Once.after(Duration.ofMinutes(1), () -> { @@ -397,12 +398,7 @@ public class ApplicationController { validateRun(application.get().require(instance), zone, platformVersion, applicationVersion); } - if (controller.zoneRegistry().zones().directlyRouted().ids().contains(zone)) { - // Provisions a new certificate if missing - endpointCertificateMetadata = getEndpointCertificate(application.get().require(instance)); - } else { - endpointCertificateMetadata = Optional.empty(); - } + endpointCertificateMetadata = endpointCertificateManager.getEndpointCertificateMetadata(application.get().require(instance), zone); endpoints = registerEndpointsInDns(applicationPackage.deploymentSpec(), application.get().require(instanceId.instance()), zone); } // Release application lock while doing the deployment, which is a lengthy task. @@ -565,43 +561,6 @@ public class ApplicationController { return Collections.unmodifiableSet(containerEndpoints); } - private Optional<EndpointCertificateMetadata> getEndpointCertificate(Instance instance) { - // Re-use certificate if already provisioned - Optional<EndpointCertificateMetadata> endpointCertificateMetadata = curator.readEndpointCertificateMetadata(instance.id()); - if(endpointCertificateMetadata.isPresent()) - return endpointCertificateMetadata; - - ApplicationCertificate newCertificate = controller.serviceRegistry().applicationCertificateProvider().requestCaSignedCertificate(instance.id(), dnsNamesOf(instance.id())); - EndpointCertificateMetadata provisionedCertificateMetadata = EndpointCertificateMetadataSerializer.fromTlsSecretsKeysString(newCertificate.secretsKeyNamePrefix()); - curator.writeEndpointCertificateMetadata(instance.id(), provisionedCertificateMetadata); - return Optional.of(provisionedCertificateMetadata); - } - - /** Returns all valid DNS names of given application */ - private List<String> dnsNamesOf(ApplicationId applicationId) { - List<String> endpointDnsNames = new ArrayList<>(); - - // We add first an endpoint name based on a hash of the applicationId, - // as the certificate provider requires the first CN to be < 64 characters long. - endpointDnsNames.add(Endpoint.createHashedCn(applicationId, controller.system())); - - var globalDefaultEndpoint = Endpoint.of(applicationId).named(EndpointId.defaultId()); - var rotationEndpoints = Endpoint.of(applicationId).wildcard(); - - var zoneLocalEndpoints = controller.zoneRegistry().zones().directlyRouted().zones().stream().flatMap(zone -> Stream.of( - Endpoint.of(applicationId).target(ClusterSpec.Id.from("default"), zone.getId()), - Endpoint.of(applicationId).wildcard(zone.getId()) - )); - - Stream.concat(Stream.of(globalDefaultEndpoint, rotationEndpoints), zoneLocalEndpoints) - .map(Endpoint.EndpointBuilder::directRouting) - .map(endpoint -> endpoint.on(Endpoint.Port.tls())) - .map(endpointBuilder -> endpointBuilder.in(controller.system())) - .map(Endpoint::dnsName).forEach(endpointDnsNames::add); - - return Collections.unmodifiableList(endpointDnsNames); - } - private ActivateResult unexpectedDeployment(ApplicationId application, ZoneId zone) { Log logEntry = new Log(); logEntry.level = "WARNING"; 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 d3e21f0d399..14b5c5e02c4 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 @@ -9,6 +9,7 @@ import com.yahoo.config.provision.CloudName; import com.yahoo.config.provision.HostName; import com.yahoo.config.provision.SystemName; import com.yahoo.config.provision.zone.ZoneApi; +import com.yahoo.container.jdisc.secretstore.SecretStore; import com.yahoo.jdisc.Metric; import com.yahoo.vespa.curator.Lock; import com.yahoo.vespa.flags.FlagSource; @@ -80,14 +81,14 @@ public class Controller extends AbstractComponent implements ApplicationIdSource */ @Inject public Controller(CuratorDb curator, RotationsConfig rotationsConfig, AccessControl accessControl, FlagSource flagSource, - MavenRepository mavenRepository, ServiceRegistry serviceRegistry, Metric metric) { + MavenRepository mavenRepository, ServiceRegistry serviceRegistry, Metric metric, SecretStore secretStore) { this(curator, rotationsConfig, accessControl, com.yahoo.net.HostName::getLocalhost, flagSource, - mavenRepository, serviceRegistry, metric); + mavenRepository, serviceRegistry, metric, secretStore); } public Controller(CuratorDb curator, RotationsConfig rotationsConfig, AccessControl accessControl, Supplier<String> hostnameSupplier, FlagSource flagSource, MavenRepository mavenRepository, - ServiceRegistry serviceRegistry, Metric metric) { + ServiceRegistry serviceRegistry, Metric metric, SecretStore secretStore) { this.hostnameSupplier = Objects.requireNonNull(hostnameSupplier, "HostnameSupplier cannot be null"); this.curator = Objects.requireNonNull(curator, "Curator cannot be null"); @@ -103,7 +104,7 @@ public class Controller extends AbstractComponent implements ApplicationIdSource jobController = new JobController(this); applicationController = new ApplicationController(this, curator, accessControl, Objects.requireNonNull(rotationsConfig, "RotationsConfig cannot be null"), - clock + clock, secretStore ); tenantController = new TenantController(this, curator, accessControl); auditLogger = new AuditLogger(curator, clock); 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 e0b83670027..480f5c68262 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 @@ -473,6 +473,8 @@ public class InternalStepRunner implements StepRunner { private boolean endpointsAvailable(ApplicationId id, ZoneId zone, DualLogger logger) { + if (useConfigServerForTesterAPI(zone) && id.instance().isTester()) return true; // Endpoints not used in this case, always return true + var endpoints = controller.applications().clusterEndpoints(Set.of(new DeploymentId(id, zone))); if ( ! endpoints.containsKey(zone)) { logger.log("Endpoints not yet ready."); @@ -555,7 +557,7 @@ public class InternalStepRunner implements StepRunner { Optional<URI> testerEndpoint = controller.jobController().testerEndpoint(id); if (useConfigServerForTesterAPI(zoneId)) { - if ( ! controller.serviceRegistry().configServer().isTesterReady(getTesterDeploymentId(id))) { + if ( ! controller.jobController().cloud().testerReady(getTesterDeploymentId(id))) { logger.log(WARNING, "Tester container went bad!"); return Optional.of(error); } @@ -579,21 +581,13 @@ public class InternalStepRunner implements StepRunner { endpoints, controller.applications().contentClustersByZone(deployments)); if (useConfigServerForTesterAPI(zoneId)) { - controller.serviceRegistry().configServer().startTests(getTesterDeploymentId(id), suite, config); + controller.jobController().cloud().startTests(getTesterDeploymentId(id), suite, config); } else { controller.jobController().cloud().startTests(testerEndpoint.get(), suite, config); } return Optional.of(running); } - private boolean testerReady(RunId id, URI testerEndpoint) { - if (useConfigServerForTesterAPI(id.type().zone(controller.system()))) { - return controller.serviceRegistry().configServer().isTesterReady(getTesterDeploymentId(id)); - } else { - return controller.jobController().cloud().testerReady(testerEndpoint); - } - } - private Optional<RunStatus> endTests(RunId id, DualLogger logger) { if (deployment(id.application(), id.type()).isEmpty()) { logger.log(INFO, "Deployment expired before tests could complete."); @@ -615,7 +609,7 @@ public class InternalStepRunner implements StepRunner { TesterCloud.Status testStatus; if (useConfigServerForTesterAPI(id.type().zone(controller.system()))) { - testStatus = controller.serviceRegistry().configServer().getTesterStatus(getTesterDeploymentId(id)); + testStatus = controller.jobController().cloud().getStatus(getTesterDeploymentId(id)); } else { Optional<URI> testerEndpoint = controller.jobController().testerEndpoint(id); if (testerEndpoint.isEmpty()) { diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/JobController.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/JobController.java index 0c2b6ee1744..6d7980396de 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/JobController.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/JobController.java @@ -5,8 +5,10 @@ import com.google.common.collect.ImmutableSortedMap; import com.yahoo.component.Version; import com.yahoo.config.provision.ApplicationId; import com.yahoo.config.provision.zone.ZoneId; -import com.yahoo.jdisc.Metric; import com.yahoo.vespa.curator.Lock; +import com.yahoo.vespa.flags.BooleanFlag; +import com.yahoo.vespa.flags.FetchVector; +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; @@ -181,10 +183,15 @@ public class JobController { return run; Optional<URI> testerEndpoint = testerEndpoint(id); - if ( ! testerEndpoint.isPresent()) + if (testerEndpoint.isEmpty()) return run; - List<LogEntry> entries = cloud.getLog(testerEndpoint.get(), run.lastTestLogEntry()); + List<LogEntry> entries; + ZoneId zone = id.type().zone(controller.system()); + if (useConfigServerForTesterAPI(zone)) + entries = cloud.getLog(new DeploymentId(id.application(), zone), run.lastTestLogEntry()); + else + entries = cloud.getLog(testerEndpoint.get(), run.lastTestLogEntry()); if (entries.isEmpty()) return run; @@ -584,4 +591,9 @@ public class JobController { } } + private boolean useConfigServerForTesterAPI(ZoneId zoneId) { + BooleanFlag useConfigServerForTesterAPI = Flags.USE_CONFIG_SERVER_FOR_TESTER_API_CALLS.bindTo(controller.flagSource()); + return useConfigServerForTesterAPI.with(FetchVector.Dimension.ZONE_ID, zoneId.value()).value(); + } + } diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/JobProfile.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/JobProfile.java index 4c2b53d6ba2..88b3442bd6e 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/JobProfile.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/JobProfile.java @@ -56,13 +56,8 @@ public enum JobProfile { report)), production(EnumSet.of(deployReal, - installReal, - deployTester, - installTester, - startTests, - endTests), - EnumSet.of(deactivateTester, - report)), + installReal), + EnumSet.of(report)), productionTest(EnumSet.of(deployTester, installTester, diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/endpointcertificates/EndpointCertificateManager.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/endpointcertificates/EndpointCertificateManager.java new file mode 100644 index 00000000000..4c7305f08e5 --- /dev/null +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/endpointcertificates/EndpointCertificateManager.java @@ -0,0 +1,145 @@ +package com.yahoo.vespa.hosted.controller.endpointcertificates; + +import com.yahoo.config.provision.ApplicationId; +import com.yahoo.config.provision.ClusterSpec; +import com.yahoo.config.provision.zone.ZoneApi; +import com.yahoo.config.provision.zone.ZoneId; +import com.yahoo.container.jdisc.secretstore.SecretStore; +import com.yahoo.log.LogLevel; +import com.yahoo.security.SubjectAlternativeName; +import com.yahoo.security.X509CertificateUtils; +import com.yahoo.vespa.hosted.controller.Instance; +import com.yahoo.vespa.hosted.controller.api.integration.certificates.ApplicationCertificate; +import com.yahoo.vespa.hosted.controller.api.integration.certificates.ApplicationCertificateProvider; +import com.yahoo.vespa.hosted.controller.api.integration.certificates.EndpointCertificateMetadata; +import com.yahoo.vespa.hosted.controller.api.integration.zone.ZoneRegistry; +import com.yahoo.vespa.hosted.controller.application.Endpoint; +import com.yahoo.vespa.hosted.controller.application.EndpointId; +import com.yahoo.vespa.hosted.controller.persistence.CuratorDb; +import com.yahoo.vespa.hosted.controller.persistence.EndpointCertificateMetadataSerializer; + +import java.security.cert.X509Certificate; +import java.time.Clock; +import java.time.Instant; +import java.util.ArrayList; +import java.util.Collections; +import java.util.List; +import java.util.Optional; +import java.util.Set; +import java.util.logging.Logger; +import java.util.stream.Collectors; +import java.util.stream.Stream; + +public class EndpointCertificateManager { + + private static final Logger log = Logger.getLogger(EndpointCertificateManager.class.getName()); + + private final ZoneRegistry zoneRegistry; + private final CuratorDb curator; + private final SecretStore secretStore; + private final ApplicationCertificateProvider applicationCertificateProvider; + private final Clock clock; + + public EndpointCertificateManager(ZoneRegistry zoneRegistry, + CuratorDb curator, + SecretStore secretStore, + ApplicationCertificateProvider applicationCertificateProvider, + Clock clock) { + this.zoneRegistry = zoneRegistry; + this.curator = curator; + this.secretStore = secretStore; + this.applicationCertificateProvider = applicationCertificateProvider; + this.clock = clock; + } + + public Optional<EndpointCertificateMetadata> getEndpointCertificateMetadata(Instance instance, ZoneId zone) { + + if (!zoneRegistry.zones().directlyRouted().ids().contains(zone)) return Optional.empty(); + + // Re-use certificate if already provisioned + Optional<EndpointCertificateMetadata> endpointCertificateMetadata = + curator.readEndpointCertificateMetadata(instance.id()) + .or(() -> Optional.of(provisionEndpointCertificate(instance))); + + // Only logs warnings for now + endpointCertificateMetadata.ifPresent(certificateMetadata -> verifyEndpointCertificate(certificateMetadata, instance, zone)); + + return endpointCertificateMetadata; + } + + private EndpointCertificateMetadata provisionEndpointCertificate(Instance instance) { + List<ZoneId> directlyRoutedZones = zoneRegistry.zones().directlyRouted().zones().stream().map(ZoneApi::getId).collect(Collectors.toUnmodifiableList()); + ApplicationCertificate newCertificate = applicationCertificateProvider + .requestCaSignedCertificate(instance.id(), dnsNamesOf(instance.id(), directlyRoutedZones)); + EndpointCertificateMetadata provisionedCertificateMetadata = EndpointCertificateMetadataSerializer.fromTlsSecretsKeysString(newCertificate.secretsKeyNamePrefix()); + curator.writeEndpointCertificateMetadata(instance.id(), provisionedCertificateMetadata); + return provisionedCertificateMetadata; + } + + private boolean verifyEndpointCertificate(EndpointCertificateMetadata endpointCertificateMetadata, Instance instance, ZoneId zone) { + try { + var pemEncodedEndpointCertificate = secretStore.getSecret(endpointCertificateMetadata.certName(), endpointCertificateMetadata.version()); + + if (pemEncodedEndpointCertificate == null) return logWarning("Certificate not found in secret store"); + + List<X509Certificate> x509CertificateList = X509CertificateUtils.certificateListFromPem(pemEncodedEndpointCertificate); + + if (x509CertificateList.isEmpty()) return logWarning("Empty certificate list"); + if (x509CertificateList.size() < 2) + return logWarning("Only a single certificate found in chain - intermediate certificates likely missing"); + + Instant now = clock.instant(); + Instant firstExpiry = Instant.MAX; + for (X509Certificate x509Certificate : x509CertificateList) { + Instant notBefore = x509Certificate.getNotBefore().toInstant(); + Instant notAfter = x509Certificate.getNotAfter().toInstant(); + if (now.isBefore(notBefore)) return logWarning("Certificate is not yet valid"); + if (now.isAfter(notAfter)) return logWarning("Certificate has expired"); + if (notAfter.isBefore(firstExpiry)) firstExpiry = notAfter; + } + + X509Certificate endEntityCertificate = x509CertificateList.get(0); + Set<String> subjectAlternativeNames = X509CertificateUtils.getSubjectAlternativeNames(endEntityCertificate).stream() + .filter(san -> san.getType().equals(SubjectAlternativeName.Type.DNS_NAME)) + .map(SubjectAlternativeName::getValue).collect(Collectors.toSet()); + + if (!subjectAlternativeNames.equals(Set.copyOf(dnsNamesOf(instance.id(), List.of(zone))))) + return logWarning("The list of SANs in the certificate does not match what we expect"); + + return true; // All good then, hopefully + } catch (Exception e) { + log.log(LogLevel.WARNING, "Exception thrown when verifying endpoint certificate", e); + return false; + } + } + + private static boolean logWarning(String message) { + log.log(LogLevel.WARNING, message); + return false; + } + + private List<String> dnsNamesOf(ApplicationId applicationId, List<ZoneId> zones) { + List<String> endpointDnsNames = new ArrayList<>(); + + // We add first an endpoint name based on a hash of the applicationId, + // as the certificate provider requires the first CN to be < 64 characters long. + endpointDnsNames.add(Endpoint.createHashedCn(applicationId, zoneRegistry.system())); + + var globalDefaultEndpoint = Endpoint.of(applicationId).named(EndpointId.defaultId()); + var rotationEndpoints = Endpoint.of(applicationId).wildcard(); + + var zoneLocalEndpoints = zones.stream().flatMap(zone -> Stream.of( + Endpoint.of(applicationId).target(ClusterSpec.Id.from("default"), zone), + Endpoint.of(applicationId).wildcard(zone) + )); + + Stream.concat(Stream.of(globalDefaultEndpoint, rotationEndpoints), zoneLocalEndpoints) + .map(Endpoint.EndpointBuilder::directRouting) + .map(endpoint -> endpoint.on(Endpoint.Port.tls())) + .map(endpointBuilder -> endpointBuilder.in(zoneRegistry.system())) + .map(Endpoint::dnsName).forEach(endpointDnsNames::add); + + return Collections.unmodifiableList(endpointDnsNames); + } + +} diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/Maintainer.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/Maintainer.java index 9c3c1dc1f5e..a814f62cb03 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/Maintainer.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/Maintainer.java @@ -6,6 +6,7 @@ import com.yahoo.config.provision.HostName; import com.yahoo.config.provision.SystemName; import com.yahoo.vespa.curator.Lock; import com.yahoo.vespa.hosted.controller.Controller; +import com.yahoo.yolean.Exceptions; import java.time.Duration; import java.time.Instant; @@ -75,7 +76,8 @@ public abstract class Maintainer extends AbstractComponent implements Runnable { // another controller instance is running this job at the moment; ok } catch (Throwable t) { - log.log(Level.WARNING, this + " failed. Will retry in " + maintenanceInterval.toMinutes() + " minutes", t); + log.log(Level.WARNING, "Maintainer " + this.getClass().getSimpleName() + " failed. Will retry in " + + maintenanceInterval + ": " + Exceptions.toMessageString(t)); } } diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/application/JobControllerApiHandlerHelper.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/application/JobControllerApiHandlerHelper.java index 91a0455db11..9ea530c7886 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/application/JobControllerApiHandlerHelper.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/application/JobControllerApiHandlerHelper.java @@ -433,9 +433,11 @@ class JobControllerApiHandlerHelper { .orElseThrow(() -> new IllegalStateException("Unknown run '" + runId + "'")); detailsObject.setBool("active", ! run.hasEnded()); detailsObject.setString("status", nameOf(run.status())); - jobController.updateTestLog(runId); - try { jobController.updateVespaLog(runId); } - catch (RuntimeException ignored) { } // May be perfectly fine, e.g., when logserver isn't up yet. + try { + jobController.updateTestLog(runId); + jobController.updateVespaLog(runId); + } + catch (RuntimeException ignored) { } // Return response when this fails, which it does when, e.g., logserver is booting. RunLog runLog = (after == null ? jobController.details(runId) : jobController.details(runId, Long.parseLong(after))) .orElseThrow(() -> new NotExistsException(String.format( diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/ControllerTester.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/ControllerTester.java index c83463bc1ea..5318d6bdefd 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/ControllerTester.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/ControllerTester.java @@ -33,6 +33,7 @@ import com.yahoo.vespa.hosted.controller.application.TenantAndApplicationId; import com.yahoo.vespa.hosted.controller.athenz.impl.AthenzFacade; import com.yahoo.vespa.hosted.controller.integration.ConfigServerMock; import com.yahoo.vespa.hosted.controller.integration.MetricsMock; +import com.yahoo.vespa.hosted.controller.integration.SecretStoreMock; import com.yahoo.vespa.hosted.controller.integration.ServiceRegistryMock; import com.yahoo.vespa.hosted.controller.integration.ZoneRegistryMock; import com.yahoo.vespa.hosted.controller.persistence.CuratorDb; @@ -393,7 +394,7 @@ public final class ControllerTester { new InMemoryFlagSource(), new MockMavenRepository(), serviceRegistry, - new MetricsMock()); + new MetricsMock(), new SecretStoreMock()); // Calculate initial versions controller.updateVersionStatus(VersionStatus.compute(controller)); return controller; diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentContext.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentContext.java index 2792a59b523..ebf64f90873 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentContext.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentContext.java @@ -331,7 +331,8 @@ public class DeploymentContext { if (job.type().environment().isManuallyDeployed()) return this; } - doTests(job); + if (job.type().isTest()) + doTests(job); doTeardown(job); return this; } @@ -440,7 +441,7 @@ public class DeploymentContext { // First step is always a deployment. runner.advance(currentRun(job)); - if ( ! job.type().environment().isManuallyDeployed()) + if (job.type().isTest()) doInstallTester(job); if (job.type() == JobType.stagingTest) { // Do the initial deployment and installation of the real application. @@ -453,8 +454,7 @@ public class DeploymentContext { assertEquals(Step.Status.succeeded, jobs.run(id).get().stepStatuses().get(Step.installInitialReal)); // All installation is complete and endpoints are ready, so setup may begin. - if (job.type().isDeployment()) - assertEquals(Step.Status.succeeded, jobs.run(id).get().stepStatuses().get(Step.installInitialReal)); + assertEquals(Step.Status.succeeded, jobs.run(id).get().stepStatuses().get(Step.installInitialReal)); assertEquals(Step.Status.succeeded, jobs.run(id).get().stepStatuses().get(Step.installTester)); assertEquals(Step.Status.succeeded, jobs.run(id).get().stepStatuses().get(Step.startStagingSetup)); diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/InternalStepRunnerTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/InternalStepRunnerTest.java index ff66ab38d32..112b8066847 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/InternalStepRunnerTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/InternalStepRunnerTest.java @@ -54,6 +54,7 @@ import static com.yahoo.vespa.hosted.controller.deployment.DeploymentContext.app import static com.yahoo.vespa.hosted.controller.deployment.DeploymentContext.publicCdApplicationPackage; import static com.yahoo.vespa.hosted.controller.deployment.DeploymentTester.instanceId; import static com.yahoo.vespa.hosted.controller.deployment.RunStatus.deploymentFailed; +import static com.yahoo.vespa.hosted.controller.deployment.RunStatus.installationFailed; import static com.yahoo.vespa.hosted.controller.deployment.Step.Status.failed; import static com.yahoo.vespa.hosted.controller.deployment.Step.Status.succeeded; import static com.yahoo.vespa.hosted.controller.deployment.Step.Status.unfinished; @@ -140,9 +141,6 @@ public class InternalStepRunnerTest { ZoneId zone = id.type().zone(system()); HostName host = tester.configServer().hostFor(instanceId, zone); - tester.setEndpoints(app.testerId().id(), JobType.productionUsCentral1.zone(system())); - tester.runner().run(); - tester.configServer().setConfigChangeActions(new ConfigChangeActions(singletonList(new RestartAction("cluster", "container", "search", @@ -165,7 +163,7 @@ public class InternalStepRunnerTest { tester.clock().advance(InternalStepRunner.installationTimeout.plus(Duration.ofSeconds(1))); tester.runner().run(); - assertEquals(RunStatus.error, tester.jobs().run(id).get().status()); + assertEquals(installationFailed, tester.jobs().run(id).get().status()); } @Test diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/endpointcertificates/EndpointCertificateManagerTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/endpointcertificates/EndpointCertificateManagerTest.java new file mode 100644 index 00000000000..231f8be2ed3 --- /dev/null +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/endpointcertificates/EndpointCertificateManagerTest.java @@ -0,0 +1,34 @@ +package com.yahoo.vespa.hosted.controller.endpointcertificates; + +import com.yahoo.config.provision.ApplicationId; +import com.yahoo.config.provision.SystemName; +import com.yahoo.config.provision.zone.ZoneId; +import com.yahoo.vespa.hosted.controller.Instance; +import com.yahoo.vespa.hosted.controller.api.integration.certificates.ApplicationCertificateMock; +import com.yahoo.vespa.hosted.controller.api.integration.certificates.EndpointCertificateMetadata; +import com.yahoo.vespa.hosted.controller.integration.SecretStoreMock; +import com.yahoo.vespa.hosted.controller.integration.ZoneRegistryMock; +import com.yahoo.vespa.hosted.controller.persistence.MockCuratorDb; +import org.junit.Test; + +import java.time.Clock; +import java.util.Optional; + +import static org.junit.Assert.assertTrue; + +public class EndpointCertificateManagerTest { + + @Test + public void getEndpointCertificate() { + SecretStoreMock secretStore = new SecretStoreMock(); + ZoneRegistryMock zoneRegistryMock = new ZoneRegistryMock(SystemName.main); + MockCuratorDb mockCuratorDb = new MockCuratorDb(); + ApplicationCertificateMock applicationCertificateMock = new ApplicationCertificateMock(); + Clock clock = Clock.systemUTC(); + EndpointCertificateManager endpointCertificateManager = new EndpointCertificateManager(zoneRegistryMock, mockCuratorDb, secretStore, applicationCertificateMock, clock); + ZoneId id = zoneRegistryMock.zones().directlyRouted().zones().stream().findFirst().get().getId(); + Instance instance = new Instance(ApplicationId.defaultId()); + Optional<EndpointCertificateMetadata> endpointCertificateMetadata = endpointCertificateManager.getEndpointCertificateMetadata(instance, id); + assertTrue(endpointCertificateMetadata.isPresent()); + } +}
\ No newline at end of file diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/JobControllerApiHandlerHelperTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/JobControllerApiHandlerHelperTest.java index db82adb4940..186534dd288 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/JobControllerApiHandlerHelperTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/JobControllerApiHandlerHelperTest.java @@ -5,12 +5,15 @@ import com.yahoo.component.Version; import com.yahoo.config.provision.zone.ZoneId; import com.yahoo.container.jdisc.HttpResponse; import com.yahoo.vespa.hosted.controller.api.application.v4.model.DeployOptions; +import com.yahoo.vespa.hosted.controller.api.identifiers.DeploymentId; import com.yahoo.vespa.hosted.controller.api.integration.configserver.ConfigServerException; import com.yahoo.vespa.hosted.controller.api.integration.deployment.ApplicationVersion; import com.yahoo.vespa.hosted.controller.api.integration.deployment.JobType; import com.yahoo.vespa.hosted.controller.application.ApplicationPackage; import com.yahoo.vespa.hosted.controller.deployment.ApplicationPackageBuilder; import com.yahoo.vespa.hosted.controller.deployment.DeploymentTester; +import com.yahoo.vespa.hosted.controller.deployment.InternalStepRunner; +import com.yahoo.vespa.hosted.controller.deployment.RunStatus; import org.json.JSONException; import org.json.JSONObject; import org.junit.Test; @@ -37,6 +40,7 @@ import static com.yahoo.vespa.hosted.controller.api.integration.deployment.JobTy import static com.yahoo.vespa.hosted.controller.api.integration.deployment.TesterCloud.Status.FAILURE; import static com.yahoo.vespa.hosted.controller.deployment.DeploymentContext.applicationPackage; import static com.yahoo.vespa.hosted.controller.deployment.RunStatus.deploymentFailed; +import static com.yahoo.vespa.hosted.controller.deployment.RunStatus.error; import static com.yahoo.vespa.hosted.controller.deployment.RunStatus.installationFailed; import static com.yahoo.vespa.hosted.controller.deployment.RunStatus.running; import static com.yahoo.vespa.hosted.controller.deployment.RunStatus.testFailure; @@ -80,15 +84,10 @@ public class JobControllerApiHandlerHelperTest { tester.runner().run(); assertEquals(deploymentFailed, tester.jobs().last(app.instanceId(), productionUsEast3).get().status()); - ZoneId usWest1 = productionUsWest1.zone(tester.controller().system()); - tester.configServer().convergeServices(app.instanceId(), usWest1); - tester.configServer().convergeServices(app.testerId().id(), usWest1); - tester.setEndpoints(app.instanceId(), usWest1); - tester.setEndpoints(app.testerId().id(), usWest1); tester.runner().run(); - tester.cloud().set(FAILURE); + tester.clock().advance(Duration.ofHours(1).plusSeconds(1)); tester.runner().run(); - assertEquals(testFailure, tester.jobs().last(app.instanceId(), productionUsWest1).get().status()); + assertEquals(installationFailed, tester.jobs().last(app.instanceId(), productionUsWest1).get().status()); assertEquals(revision2, app.deployment(productionUsCentral1.zone(tester.controller().system())).applicationVersion()); assertEquals(revision1, app.deployment(productionUsEast3.zone(tester.controller().system())).applicationVersion()); assertEquals(revision2, app.deployment(productionUsWest1.zone(tester.controller().system())).applicationVersion()); @@ -183,7 +182,6 @@ public class JobControllerApiHandlerHelperTest { private void compare(HttpResponse response, String expected) throws JSONException, IOException { ByteArrayOutputStream baos = new ByteArrayOutputStream(); response.render(baos); - System.err.println(baos.toString()); JSONObject actualJSON = new JSONObject(new String(baos.toByteArray())); JSONObject expectedJSON = new JSONObject(expected); assertEquals(expectedJSON.toString(), actualJSON.toString()); diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/deployment-overview-2.json b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/deployment-overview-2.json index fd1e8bc5f20..a8be282deaf 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/deployment-overview-2.json +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/deployment-overview-2.json @@ -23,8 +23,8 @@ { "id": 3, "url": "https://some.url:43/instance/default/job/system-test/run/run 3 of system-test for tenant.application", - "start": 2000, - "end": 2000, + "start": 3603000, + "end": 3603000, "status": "success", "versions": { "targetPlatform": "6.1.0", @@ -239,15 +239,15 @@ } } ], - "readyAt": 752000, - "delayedUntil": 752000, - "coolingDownUntil": 752000, + "readyAt": 4353000, + "delayedUntil": 4353000, + "coolingDownUntil": 4353000, "runs": [ { "id": 5, "url": "https://some.url:43/instance/default/job/staging-test/run/run 5 of staging-test for tenant.application", - "start": 102000, - "end": 102000, + "start": 3703000, + "end": 3703000, "status": "installationFailed", "versions": { "targetPlatform": "6.1.0", @@ -327,8 +327,8 @@ { "id": 4, "url": "https://some.url:43/instance/default/job/staging-test/run/run 4 of staging-test for tenant.application", - "start": 2000, - "end": 2000, + "start": 3603000, + "end": 3603000, "status": "installationFailed", "versions": { "targetPlatform": "6.1.0", @@ -408,8 +408,8 @@ { "id": 3, "url": "https://some.url:43/instance/default/job/staging-test/run/run 3 of staging-test for tenant.application", - "start": 2000, - "end": 2000, + "start": 3603000, + "end": 3603000, "status": "success", "versions": { "targetPlatform": "6.1.0", @@ -663,12 +663,12 @@ "sourceUrl": "repository1/tree/commit1" }, "toRun": [], - "readyAt": 2000, + "readyAt": 3603000, "runs": [ { "id": 3, "url": "https://some.url:43/instance/default/job/production-us-central-1/run/run 3 of production-us-central-1 for tenant.application", - "start": 2000, + "start": 3603000, "status": "running", "versions": { "targetPlatform": "6.1.0", @@ -688,14 +688,6 @@ }, "steps": [ { - "name": "deployTester", - "status": "succeeded" - }, - { - "name": "installTester", - "status": "unfinished" - }, - { "name": "deployReal", "status": "succeeded" }, @@ -704,20 +696,8 @@ "status": "unfinished" }, { - "name": "startTests", - "status": "unfinished" - }, - { - "name": "endTests", - "status": "unfinished" - }, - { - "name": "deactivateTester", - "status": "unfinished" - }, - { "name": "report", - "status": "unfinished" + "status": "succeeded" } ] }, @@ -745,14 +725,6 @@ }, "steps": [ { - "name": "deployTester", - "status": "succeeded" - }, - { - "name": "installTester", - "status": "succeeded" - }, - { "name": "deployReal", "status": "succeeded" }, @@ -761,18 +733,6 @@ "status": "succeeded" }, { - "name": "startTests", - "status": "succeeded" - }, - { - "name": "endTests", - "status": "succeeded" - }, - { - "name": "deactivateTester", - "status": "succeeded" - }, - { "name": "report", "status": "succeeded" } @@ -795,14 +755,6 @@ }, "steps": [ { - "name": "deployTester", - "status": "succeeded" - }, - { - "name": "installTester", - "status": "succeeded" - }, - { "name": "deployReal", "status": "succeeded" }, @@ -811,18 +763,6 @@ "status": "succeeded" }, { - "name": "startTests", - "status": "succeeded" - }, - { - "name": "endTests", - "status": "succeeded" - }, - { - "name": "deactivateTester", - "status": "succeeded" - }, - { "name": "report", "status": "succeeded" } @@ -1005,8 +945,8 @@ "id": 2, "url": "https://some.url:43/instance/default/job/production-us-west-1/run/run 2 of production-us-west-1 for tenant.application", "start": 1000, - "end": 1000, - "status": "testFailure", + "end": 3602000, + "status": "installationFailed", "versions": { "targetPlatform": "6.1.0", "targetApplication": { @@ -1025,34 +965,14 @@ }, "steps": [ { - "name": "deployTester", - "status": "succeeded" - }, - { - "name": "installTester", - "status": "succeeded" - }, - { "name": "deployReal", "status": "succeeded" }, { "name": "installReal", - "status": "succeeded" - }, - { - "name": "startTests", - "status": "succeeded" - }, - { - "name": "endTests", "status": "failed" }, { - "name": "deactivateTester", - "status": "succeeded" - }, - { "name": "report", "status": "succeeded" } @@ -1075,14 +995,6 @@ }, "steps": [ { - "name": "deployTester", - "status": "succeeded" - }, - { - "name": "installTester", - "status": "succeeded" - }, - { "name": "deployReal", "status": "succeeded" }, @@ -1091,18 +1003,6 @@ "status": "succeeded" }, { - "name": "startTests", - "status": "succeeded" - }, - { - "name": "endTests", - "status": "succeeded" - }, - { - "name": "deactivateTester", - "status": "succeeded" - }, - { "name": "report", "status": "succeeded" } @@ -1173,34 +1073,14 @@ }, "steps": [ { - "name": "deployTester", - "status": "failed" - }, - { - "name": "installTester", - "status": "unfinished" - }, - { "name": "deployReal", - "status": "unfinished" + "status": "failed" }, { "name": "installReal", "status": "unfinished" }, { - "name": "startTests", - "status": "unfinished" - }, - { - "name": "endTests", - "status": "unfinished" - }, - { - "name": "deactivateTester", - "status": "succeeded" - }, - { "name": "report", "status": "succeeded" } @@ -1223,14 +1103,6 @@ }, "steps": [ { - "name": "deployTester", - "status": "succeeded" - }, - { - "name": "installTester", - "status": "succeeded" - }, - { "name": "deployReal", "status": "succeeded" }, @@ -1239,18 +1111,6 @@ "status": "succeeded" }, { - "name": "startTests", - "status": "succeeded" - }, - { - "name": "endTests", - "status": "succeeded" - }, - { - "name": "deactivateTester", - "status": "succeeded" - }, - { "name": "report", "status": "succeeded" } diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/deployment-overview.json b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/deployment-overview.json index 33c83a30e38..027cca5dad2 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/deployment-overview.json +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/deployment-overview.json @@ -340,14 +340,6 @@ }, "steps": [ { - "name": "deployTester", - "status": "succeeded" - }, - { - "name": "installTester", - "status": "succeeded" - }, - { "name": "deployReal", "status": "succeeded" }, @@ -356,18 +348,6 @@ "status": "succeeded" }, { - "name": "startTests", - "status": "succeeded" - }, - { - "name": "endTests", - "status": "succeeded" - }, - { - "name": "deactivateTester", - "status": "succeeded" - }, - { "name": "report", "status": "succeeded" } @@ -415,14 +395,6 @@ }, "steps": [ { - "name": "deployTester", - "status": "unfinished" - }, - { - "name": "installTester", - "status": "unfinished" - }, - { "name": "deployReal", "status": "unfinished" }, @@ -431,18 +403,6 @@ "status": "unfinished" }, { - "name": "startTests", - "status": "unfinished" - }, - { - "name": "endTests", - "status": "unfinished" - }, - { - "name": "deactivateTester", - "status": "unfinished" - }, - { "name": "report", "status": "unfinished" } @@ -491,14 +451,6 @@ }, "steps": [ { - "name": "deployTester", - "status": "unfinished" - }, - { - "name": "installTester", - "status": "unfinished" - }, - { "name": "deployReal", "status": "unfinished" }, @@ -507,18 +459,6 @@ "status": "unfinished" }, { - "name": "startTests", - "status": "unfinished" - }, - { - "name": "endTests", - "status": "unfinished" - }, - { - "name": "deactivateTester", - "status": "unfinished" - }, - { "name": "report", "status": "unfinished" } diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/dev-aws-us-east-2a-runs.json b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/dev-aws-us-east-2a-runs.json index fe90ddd772e..f7185d373c4 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/dev-aws-us-east-2a-runs.json +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/dev-aws-us-east-2a-runs.json @@ -2,8 +2,8 @@ "1": { "id": 1, "status": "success", - "start": 102000, - "end": 102000, + "start": 3703000, + "end": 3703000, "wantedPlatform": "7.1", "wantedApplication": { "hash": "unknown" diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/jobs.json b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/jobs.json index dc37c3b4bb4..3ab7078ae6a 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/jobs.json +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/jobs.json @@ -230,19 +230,13 @@ "commit": "commit1" }, "steps": { - "deployTester": "succeeded", - "installTester": "succeeded", "deployReal": "succeeded", "installReal": "succeeded", - "startTests": "succeeded", - "endTests": "succeeded", - "deactivateTester": "succeeded", "report": "succeeded" }, "tasks": { "deploy": "succeeded", - "install": "succeeded", - "test": "succeeded" + "install": "succeeded" }, "log": "http://localhost:8080/application/v4/tenant/tenant1/application/application1/instance/instance1/job/production-us-central-1/run/1" } @@ -268,13 +262,8 @@ "commit": "commit1" }, "steps": { - "deployTester": "unfinished", - "installTester": "unfinished", "deployReal": "unfinished", "installReal": "unfinished", - "startTests": "unfinished", - "endTests": "unfinished", - "deactivateTester": "unfinished", "report": "unfinished" }, "tasks": {}, @@ -302,13 +291,8 @@ "commit": "commit1" }, "steps": { - "deployTester": "unfinished", - "installTester": "unfinished", "deployReal": "unfinished", "installReal": "unfinished", - "startTests": "unfinished", - "endTests": "unfinished", - "deactivateTester": "unfinished", "report": "unfinished" }, "tasks": {}, diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/overview-user-instance.json b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/overview-user-instance.json index cf47e8671b0..95afb12dcbf 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/overview-user-instance.json +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/overview-user-instance.json @@ -1,55 +1,54 @@ { + "lastVersions": { + "platform": { + "platform": "7.1", + "at": 0, + "completed": "0 of 0 complete" + }, + "application": { + "application": { + "hash": "1.0.3-commit1", + "build": 3, + "source": { + "gitRepository": "repository1", + "gitBranch": "master", + "gitCommit": "commit1" + }, + "sourceUrl": "repository1/tree/commit1", + "commit": "commit1" + }, + "at": 1000, + "completed": "0 of 0 complete" + } + }, + "deploying": {}, + "deployments": [], + "jobs": {}, "devJobs": { "dev-aws-us-east-2a": { "runs": [ { + "id": 1, + "status": "success", + "start": 3703000, + "end": 3703000, "wantedPlatform": "7.1", - "log": "https://some.url:43/root/dev-aws-us-east-2a/run/1", "wantedApplication": { "hash": "unknown" }, - "start": 102000, - "end": 102000, - "id": 1, "steps": { "deployReal": "succeeded", "installReal": "succeeded", "copyVespaLogs": "succeeded" }, "tasks": { - "install": "succeeded", - "deploy": "succeeded" + "deploy": "succeeded", + "install": "succeeded" }, - "status": "success" + "log": "https://some.url:43/root/dev-aws-us-east-2a/run/1" } ], "url": "https://some.url:43/root/dev-aws-us-east-2a" } - }, - "deployments": [], - "lastVersions": { - "application": { - "at": 1000, - "application": { - "build": 3, - "source": { - "gitRepository": "repository1", - "gitCommit": "commit1", - "gitBranch": "master" - }, - "sourceUrl": "repository1/tree/commit1", - "commit": "commit1", - "hash": "1.0.3-commit1" - }, - "completed": "0 of 0 complete" - }, - "platform": { - "at": 0, - "completed": "0 of 0 complete", - "platform": "7.1" - } - }, - "deploying": {}, - "jobs": {} + } } - diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/overview.json b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/overview.json index ad00476154c..9510b06fb42 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/overview.json +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/overview.json @@ -37,7 +37,7 @@ "deployments": [ { "us-central-1": { - "at": 2000, + "at": 3603000, "platform": "6.1", "application": { "hash": "1.0.3-commit1", @@ -97,8 +97,8 @@ { "id": 3, "status": "success", - "start": 2000, - "end": 2000, + "start": 3603000, + "end": 3603000, "wantedPlatform": "6.1", "wantedApplication": { "hash": "1.0.3-commit1", @@ -265,8 +265,8 @@ { "id": 5, "status": "installationFailed", - "start": 102000, - "end": 102000, + "start": 3703000, + "end": 3703000, "wantedPlatform": "6.1", "wantedApplication": { "hash": "1.0.3-commit1", @@ -313,8 +313,8 @@ { "id": 4, "status": "installationFailed", - "start": 2000, - "end": 2000, + "start": 3603000, + "end": 3603000, "wantedPlatform": "6.1", "wantedApplication": { "hash": "1.0.3-commit1", @@ -361,8 +361,8 @@ { "id": 3, "status": "success", - "start": 2000, - "end": 2000, + "start": 3603000, + "end": 3603000, "wantedPlatform": "6.1", "wantedApplication": { "hash": "1.0.3-commit1", @@ -510,7 +510,7 @@ { "id": 3, "status": "running", - "start": 2000, + "start": 3603000, "wantedPlatform": "6.1", "wantedApplication": { "hash": "1.0.3-commit1", @@ -536,14 +536,9 @@ "commit": "commit1" }, "steps": { - "deployTester": "succeeded", - "installTester": "unfinished", "deployReal": "succeeded", "installReal": "unfinished", - "startTests": "unfinished", - "endTests": "unfinished", - "deactivateTester": "unfinished", - "report": "unfinished" + "report": "succeeded" }, "tasks": { "deploy": "succeeded", @@ -581,19 +576,13 @@ "commit": "commit1" }, "steps": { - "deployTester": "succeeded", - "installTester": "succeeded", "deployReal": "succeeded", "installReal": "succeeded", - "startTests": "succeeded", - "endTests": "succeeded", - "deactivateTester": "succeeded", "report": "succeeded" }, "tasks": { "deploy": "succeeded", - "install": "succeeded", - "test": "succeeded" + "install": "succeeded" }, "log": "https://some.url:43/root/production-us-central-1/run/2" }, @@ -615,19 +604,13 @@ "commit": "commit1" }, "steps": { - "deployTester": "succeeded", - "installTester": "succeeded", "deployReal": "succeeded", "installReal": "succeeded", - "startTests": "succeeded", - "endTests": "succeeded", - "deactivateTester": "succeeded", "report": "succeeded" }, "tasks": { "deploy": "succeeded", - "install": "succeeded", - "test": "succeeded" + "install": "succeeded" }, "log": "https://some.url:43/root/production-us-central-1/run/1" } @@ -789,9 +772,9 @@ }, { "id": 2, - "status": "testFailure", + "status": "installationFailed", "start": 1000, - "end": 1000, + "end": 3602000, "wantedPlatform": "6.1", "wantedApplication": { "hash": "1.0.2-commit1", @@ -817,19 +800,13 @@ "commit": "commit1" }, "steps": { - "deployTester": "succeeded", - "installTester": "succeeded", "deployReal": "succeeded", - "installReal": "succeeded", - "startTests": "succeeded", - "endTests": "failed", - "deactivateTester": "succeeded", + "installReal": "failed", "report": "succeeded" }, "tasks": { "deploy": "succeeded", - "install": "succeeded", - "test": "failed" + "install": "failed" }, "log": "https://some.url:43/root/production-us-west-1/run/2" }, @@ -851,19 +828,13 @@ "commit": "commit1" }, "steps": { - "deployTester": "succeeded", - "installTester": "succeeded", "deployReal": "succeeded", "installReal": "succeeded", - "startTests": "succeeded", - "endTests": "succeeded", - "deactivateTester": "succeeded", "report": "succeeded" }, "tasks": { "deploy": "succeeded", - "install": "succeeded", - "test": "succeeded" + "install": "succeeded" }, "log": "https://some.url:43/root/production-us-west-1/run/1" } @@ -933,16 +904,13 @@ "commit": "commit1" }, "steps": { - "deployTester": "failed", - "installTester": "unfinished", - "deployReal": "unfinished", + "deployReal": "failed", "installReal": "unfinished", - "startTests": "unfinished", - "endTests": "unfinished", - "deactivateTester": "succeeded", "report": "succeeded" }, - "tasks": {}, + "tasks": { + "deploy": "failed" + }, "log": "https://some.url:43/root/production-us-east-3/run/2" }, { @@ -963,19 +931,13 @@ "commit": "commit1" }, "steps": { - "deployTester": "succeeded", - "installTester": "succeeded", "deployReal": "succeeded", "installReal": "succeeded", - "startTests": "succeeded", - "endTests": "succeeded", - "deactivateTester": "succeeded", "report": "succeeded" }, "tasks": { "deploy": "succeeded", - "install": "succeeded", - "test": "succeeded" + "install": "succeeded" }, "log": "https://some.url:43/root/production-us-east-3/run/1" } diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/staging-runs.json b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/staging-runs.json index 4238ce4b834..a7825681f4b 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/staging-runs.json +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/staging-runs.json @@ -94,8 +94,8 @@ "3": { "id": 3, "status": "success", - "start": 2000, - "end": 2000, + "start": 3603000, + "end": 3603000, "wantedPlatform": "6.1", "wantedApplication": { "hash": "1.0.3-commit1", @@ -146,8 +146,8 @@ "4": { "id": 4, "status": "installationFailed", - "start": 2000, - "end": 2000, + "start": 3603000, + "end": 3603000, "wantedPlatform": "6.1", "wantedApplication": { "hash": "1.0.3-commit1", @@ -194,8 +194,8 @@ "5": { "id": 5, "status": "installationFailed", - "start": 102000, - "end": 102000, + "start": 3703000, + "end": 3703000, "wantedPlatform": "6.1", "wantedApplication": { "hash": "1.0.3-commit1", diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/staging-test-log.json b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/staging-test-log.json index 273887c26c4..8ade8795c86 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/staging-test-log.json +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/staging-test-log.json @@ -4,122 +4,122 @@ "log": { "deployTester": [ { - "at": 102000, + "at": 3703000, "type": "info", "message": "No services requiring restart." }, { - "at": 102000, + "at": 3703000, "type": "info", "message": "Deployment successful." }, { - "at": 102000, + "at": 3703000, "type": "info", "message": "foo" } ], "installTester": [ { - "at": 102000, + "at": 3703000, "type": "info", "message": "host-tenant:application:default-t-staging.us-east-3: unorchestrated" }, { - "at": 102000, + "at": 3703000, "type": "info", "message": "--- platform 6.1" }, { - "at": 102000, + "at": 3703000, "type": "info", "message": "--- container on port 43 has config generation 1, wanted is 2" }, { - "at": 102000, + "at": 3703000, "type": "info", "message": "host-tenant:application:default-t-staging.us-east-3: unorchestrated" }, { - "at": 102000, + "at": 3703000, "type": "info", "message": "--- platform 6.1" }, { - "at": 102000, + "at": 3703000, "type": "info", "message": "--- container on port 43 has config generation 1, wanted is 2" }, { - "at": 102000, + "at": 3703000, "type": "info", "message": "host-tenant:application:default-t-staging.us-east-3: unorchestrated" }, { - "at": 102000, + "at": 3703000, "type": "info", "message": "--- platform 6.1" }, { - "at": 102000, + "at": 3703000, "type": "info", "message": "--- container on port 43 has config generation 1, wanted is 2" } ], "deployInitialReal": [ { - "at": 102000, + "at": 3703000, "type": "info", "message": "Deploying platform version 6.1 and application version 1.0.1-commit1 ..." }, { - "at": 102000, + "at": 3703000, "type": "info", "message": "No services requiring restart." }, { - "at": 102000, + "at": 3703000, "type": "info", "message": "Deployment successful." }, { - "at": 102000, + "at": 3703000, "type": "info", "message": "foo" } ], "installInitialReal": [ { - "at": 102000, + "at": 3703000, "type": "info", "message": "host-tenant:application:default-staging.us-east-3: unorchestrated" }, { - "at": 102000, + "at": 3703000, "type": "info", "message": "--- platform 6.1" }, { - "at": 102000, + "at": 3703000, "type": "info", "message": "--- container on port 43 has config generation 1, wanted is 2" }, { - "at": 102000, + "at": 3703000, "type": "info", "message": "Deployment expired before installation was successful." } ], "deactivateReal": [ { - "at": 102000, + "at": 3703000, "type": "info", "message": "Deactivating deployment of tenant.application in staging.us-east-3 ..." } ], "deactivateTester": [ { - "at": 102000, + "at": 3703000, "type": "info", "message": "Deactivating tester of tenant.application in staging.us-east-3 ..." } @@ -129,19 +129,19 @@ "steps": { "deployTester": { "status": "succeeded", - "startMillis": 102000 + "startMillis": 3703000 }, "installTester": { "status": "unfinished", - "startMillis": 102000 + "startMillis": 3703000 }, "deployInitialReal": { "status": "succeeded", - "startMillis": 102000 + "startMillis": 3703000 }, "installInitialReal": { "status": "failed", - "startMillis": 102000, + "startMillis": 3703000, "convergence": { "nodes": 1, "down": 0, @@ -177,19 +177,19 @@ }, "copyVespaLogs": { "status": "succeeded", - "startMillis": 102000 + "startMillis": 3703000 }, "deactivateReal": { "status": "succeeded", - "startMillis": 102000 + "startMillis": 3703000 }, "deactivateTester": { "status": "succeeded", - "startMillis": 102000 + "startMillis": 3703000 }, "report": { "status": "succeeded", - "startMillis": 102000 + "startMillis": 3703000 } } } diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/us-east-3-log-without-first.json b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/us-east-3-log-without-first.json index 0fa4c541832..6c9315ca64b 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/us-east-3-log-without-first.json +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/us-east-3-log-without-first.json @@ -1,50 +1,27 @@ { + "active": false, + "status": "deploymentFailed", "log": { - "deployTester": [ + "deployReal": [ { "at": 1000, "type": "info", "message": "Failed to deploy application: ERROR!" } - ], - "deactivateTester": [ - { - "at": 1000, - "type": "info", - "message": "Deactivating tester of tenant.application in prod.us-east-3 ..." - } ] }, - "active": false, - "lastId": 2, + "lastId": 1, "steps": { - "startTests": { - "status": "unfinished" - }, - "deployTester": { - "startMillis": 1000, - "status": "failed" - }, - "report": { - "startMillis": 1000, - "status": "succeeded" - }, - "installTester": { - "status": "unfinished" - }, "deployReal": { - "status": "unfinished" + "status": "failed", + "startMillis": 1000 }, "installReal": { "status": "unfinished" }, - "deactivateTester": { - "startMillis": 1000, - "status": "succeeded" - }, - "endTests": { - "status": "unfinished" + "report": { + "status": "succeeded", + "startMillis": 1000 } - }, - "status": "deploymentFailed" + } } diff --git a/eval/src/tests/ann/nns-l2.h b/eval/src/tests/ann/nns-l2.h index dcad5f1bda6..857866ff73b 100644 --- a/eval/src/tests/ann/nns-l2.h +++ b/eval/src/tests/ann/nns-l2.h @@ -2,6 +2,7 @@ #pragma once #include <string.h> +#include <vespa/vespalib/util/arrayref.h> #include <vespa/vespalib/hwaccelrated/iaccelrated.h> template <typename T, size_t VLEN> diff --git a/eval/src/tests/ann/nns.h b/eval/src/tests/ann/nns.h index 79c1aac4379..ffe2882188e 100644 --- a/eval/src/tests/ann/nns.h +++ b/eval/src/tests/ann/nns.h @@ -67,3 +67,7 @@ make_rplsh_nns(uint32_t numDims, const DocVectorAccess<float> &dva); extern std::unique_ptr<NNS<float>> make_hnsw_nns(uint32_t numDims, const DocVectorAccess<float> &dva); + +extern +std::unique_ptr<NNS<float>> +make_hnsw_wrap(uint32_t numDims, const DocVectorAccess<float> &dva); diff --git a/eval/src/tests/ann/sift_benchmark.cpp b/eval/src/tests/ann/sift_benchmark.cpp index 5ca505e5f1e..f3570eb9247 100644 --- a/eval/src/tests/ann/sift_benchmark.cpp +++ b/eval/src/tests/ann/sift_benchmark.cpp @@ -281,10 +281,17 @@ TEST("require that Annoy via NNS api mostly works") { TEST("require that HNSW via NNS api mostly works") { DocVectorAdapter adapter; std::unique_ptr<NNS_API> nns = make_hnsw_nns(NUM_DIMS, adapter); - benchmark_nns("HNSW", *nns, { 100, 200 }); + benchmark_nns("HNSW-like", *nns, { 100, 150, 200 }); } #endif +#if 0 +TEST("require that HNSW wrapped api mostly works") { + DocVectorAdapter adapter; + std::unique_ptr<NNS_API> nns = make_hnsw_wrap(NUM_DIMS, adapter); + benchmark_nns("HNSW-wrap", *nns, { 100, 150, 200 }); +} +#endif /** * Before running the benchmark the ANN_SIFT1M data set must be downloaded and extracted: diff --git a/eval/src/tests/ann/xp-hnsw-wrap.cpp b/eval/src/tests/ann/xp-hnsw-wrap.cpp new file mode 100644 index 00000000000..33895b2bd7c --- /dev/null +++ b/eval/src/tests/ann/xp-hnsw-wrap.cpp @@ -0,0 +1,55 @@ +// Copyright 2020 Oath Inc. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. + +#include "nns.h" +#include <iostream> +#include "/git/hnswlib/hnswlib/hnswlib.h" + +class HnswWrapNns : public NNS<float> +{ +private: + using Implementation = hnswlib::HierarchicalNSW<float>; + hnswlib::L2Space _l2space; + Implementation _hnsw; + +public: + HnswWrapNns(uint32_t numDims, const DocVectorAccess<float> &dva) + : NNS(numDims, dva), + _l2space(numDims), + _hnsw(&_l2space, 1000000, 16, 200) + { + } + + ~HnswWrapNns() {} + + void addDoc(uint32_t docid) override { + Vector vector = _dva.get(docid); + _hnsw.addPoint(vector.cbegin(), docid); + } + + void removeDoc(uint32_t docid) override { + _hnsw.markDelete(docid); + } + + std::vector<NnsHit> topK(uint32_t k, Vector vector, uint32_t search_k) override { + std::vector<NnsHit> reversed; + auto priQ = _hnsw.searchKnn(vector.cbegin(), std::max(k, search_k)); + while (! priQ.empty()) { + auto pair = priQ.top(); + reversed.emplace_back(pair.second, SqDist(pair.first)); + priQ.pop(); + } + std::vector<NnsHit> result; + while (result.size() < k && !reversed.empty()) { + result.push_back(reversed.back()); + reversed.pop_back(); + } + return result; + } +}; + +std::unique_ptr<NNS<float>> +make_hnsw_wrap(uint32_t numDims, const DocVectorAccess<float> &dva) +{ + NNS<float> *p = new HnswWrapNns(numDims, dva); + return std::unique_ptr<NNS<float>>(p); +} diff --git a/eval/src/tests/ann/xp-hnswlike-nns.cpp b/eval/src/tests/ann/xp-hnswlike-nns.cpp index ec831610a71..5ab6bd4bdaf 100644 --- a/eval/src/tests/ann/xp-hnswlike-nns.cpp +++ b/eval/src/tests/ann/xp-hnswlike-nns.cpp @@ -3,10 +3,35 @@ #include <algorithm> #include <assert.h> #include <queue> -#include <random> +#include "std-random.h" #include "nns.h" -using LinkList = std::vector<uint32_t>; +static uint64_t distcalls_simple; +static uint64_t distcalls_search_layer; +static uint64_t distcalls_other; +static uint64_t distcalls_heuristic; +static uint64_t distcalls_shrink; +static uint64_t distcalls_refill; +static uint64_t refill_needed_calls; + +struct LinkList : std::vector<uint32_t> +{ + bool has_link_to(uint32_t id) const { + auto iter = std::find(begin(), end(), id); + return (iter != end()); + } + void remove_link(uint32_t id) { + uint32_t last = back(); + for (iterator iter = begin(); iter != end(); ++iter) { + if (*iter == id) { + *iter = last; + pop_back(); + return; + } + } + fprintf(stderr, "BAD missing link to remove: %u\n", id); + } +}; struct Node { std::vector<LinkList> _links; @@ -61,38 +86,36 @@ struct VisitedSetPool }; struct HnswHit { - float dist; + double dist; uint32_t docid; HnswHit(uint32_t di, SqDist sq) : dist(sq.distance), docid(di) {} }; - -using QueueEntry = HnswHit; struct GreaterDist { - bool operator() (const QueueEntry &lhs, const QueueEntry& rhs) const { + bool operator() (const HnswHit &lhs, const HnswHit& rhs) const { return (rhs.dist < lhs.dist); } }; struct LesserDist { - bool operator() (const QueueEntry &lhs, const QueueEntry& rhs) const { + bool operator() (const HnswHit &lhs, const HnswHit& rhs) const { return (lhs.dist < rhs.dist); } }; -using NearestList = std::vector<QueueEntry>; +using NearestList = std::vector<HnswHit>; -struct NearestPriQ : std::priority_queue<QueueEntry, NearestList, GreaterDist> +struct NearestPriQ : std::priority_queue<HnswHit, NearestList, GreaterDist> { }; -struct FurthestPriQ : std::priority_queue<QueueEntry, NearestList, LesserDist> +struct FurthestPriQ : std::priority_queue<HnswHit, NearestList, LesserDist> { - NearestList steal() { - NearestList result; - c.swap(result); - return result; - } - const NearestList& peek() const { return c; } + NearestList steal() { + NearestList result; + c.swap(result); + return result; + } + const NearestList& peek() const { return c; } }; class HnswLikeNns : public NNS<float> @@ -104,7 +127,7 @@ private: uint32_t _M; uint32_t _efConstruction; double _levelMultiplier; - std::default_random_engine _rndGen; + RndGen _rndGen; VisitedSetPool _visitedSetPool; double distance(Vector v, uint32_t id) const; @@ -115,11 +138,13 @@ private: } int randomLevel() { - std::uniform_real_distribution<double> distribution(0.0, 1.0); - double r = -log(distribution(_rndGen)) * _levelMultiplier; + double unif = _rndGen.nextUniform(); + double r = -log(1.0-unif) * _levelMultiplier; return (int) r; } + void dumpStats() const; + public: HnswLikeNns(uint32_t numDims, const DocVectorAccess<float> &dva) : NNS(numDims, dva), @@ -127,13 +152,14 @@ public: _entryId(0), _entryLevel(-1), _M(16), - _efConstruction(150), - _levelMultiplier(1.0 / log(1.0 * _M)) + _efConstruction(200), + _levelMultiplier(1.0 / log(1.0 * _M)), + _rndGen() { _nodes.reserve(1234567); } - ~HnswLikeNns() {} + ~HnswLikeNns() { dumpStats(); } LinkList& getLinkList(uint32_t docid, uint32_t level) { // assert(docid < _nodes.size()); @@ -141,85 +167,43 @@ public: return _nodes[docid]._links[level]; } + const LinkList& getLinkList(uint32_t docid, uint32_t level) const { + return _nodes[docid]._links[level]; + } + // simple greedy search - QueueEntry search_layer_simple(Vector vector, QueueEntry curPoint, uint32_t searchLevel) { + HnswHit search_layer_simple(Vector vector, HnswHit curPoint, uint32_t searchLevel) { bool keepGoing = true; while (keepGoing) { keepGoing = false; const LinkList& neighbors = getLinkList(curPoint.docid, searchLevel); for (uint32_t n_id : neighbors) { double dist = distance(vector, n_id); + ++distcalls_simple; if (dist < curPoint.dist) { - curPoint = QueueEntry(n_id, SqDist(dist)); - keepGoing = true; + curPoint = HnswHit(n_id, SqDist(dist)); + keepGoing = true; } } } return curPoint; } - void search_layer_foradd(Vector vector, FurthestPriQ &w, - uint32_t ef, uint32_t searchLevel); + void search_layer(Vector vector, FurthestPriQ &w, + uint32_t ef, uint32_t searchLevel); - FurthestPriQ search_layer(Vector vector, NearestList entryPoints, - uint32_t ef, uint32_t searchLevel) { - VisitedSet &visited = _visitedSetPool.get(_nodes.size()); - NearestPriQ candidates; - FurthestPriQ w; - for (auto point : entryPoints) { - candidates.push(point); - w.push(point); - visited.mark(point.docid); - } - double limd = std::numeric_limits<double>::max(); - while (! candidates.empty()) { - QueueEntry cand = candidates.top(); - candidates.pop(); - if (cand.dist > limd) { - break; - } - for (uint32_t e_id : getLinkList(cand.docid, searchLevel)) { - if (visited.isMarked(e_id)) continue; - visited.mark(e_id); - double e_dist = distance(vector, e_id); - if (e_dist < limd) { - candidates.emplace(e_id, SqDist(e_dist)); - w.emplace(e_id, SqDist(e_dist)); - if (w.size() > ef) { - w.pop(); - limd = w.top().dist; - } - } - } - } - return w; - } - - bool haveCloserDistance(QueueEntry e, const LinkList &r) const { + bool haveCloserDistance(HnswHit e, const LinkList &r) const { for (uint32_t prevId : r) { double dist = distance(e.docid, prevId); + ++distcalls_heuristic; if (dist < e.dist) return true; } return false; } - LinkList select_neighbors(NearestPriQ &&w, uint32_t curMax) const; + LinkList select_neighbors(const NearestList &neighbors, uint32_t curMax) const; - LinkList select_neighbors(const NearestList &neighbors, uint32_t curMax) { - if (neighbors.size() <= curMax) { - LinkList result; - result.reserve(curMax+1); - for (const auto & entry : neighbors) { - result.push_back(entry.docid); - } - return result; - } - NearestPriQ w; - for (const QueueEntry & entry : neighbors) { - w.push(entry); - } - return select_neighbors(std::move(w), curMax); - } + LinkList remove_weakest(const NearestList &neighbors, uint32_t curMax, LinkList &removed) const; void addDoc(uint32_t docid) override { Vector vector = _dva.get(docid); @@ -236,7 +220,8 @@ public: } int searchLevel = _entryLevel; double entryDist = distance(vector, _entryId); - QueueEntry entryPoint(_entryId, SqDist(entryDist)); + ++distcalls_other; + HnswHit entryPoint(_entryId, SqDist(entryDist)); while (searchLevel > level) { entryPoint = search_layer_simple(vector, entryPoint, searchLevel); --searchLevel; @@ -245,9 +230,8 @@ public: FurthestPriQ w; w.push(entryPoint); while (searchLevel >= 0) { - search_layer_foradd(vector, w, _efConstruction, searchLevel); - uint32_t maxLinks = (searchLevel > 0) ? _M : (2 * _M); - LinkList neighbors = select_neighbors(w.peek(), maxLinks); + search_layer(vector, w, _efConstruction, searchLevel); + LinkList neighbors = select_neighbors(w.peek(), _M); connect_new_node(docid, neighbors, searchLevel); each_shrink_ifneeded(neighbors, searchLevel); --searchLevel; @@ -256,46 +240,111 @@ public: _entryLevel = level; _entryId = docid; } + if (_nodes.size() % 10000 == 0) { + double div = _nodes.size(); + fprintf(stderr, "added docs: %d\n", (int)div); + fprintf(stderr, "distance calls for layer: %zu is %.3f per doc\n", distcalls_search_layer, distcalls_search_layer/ div); + fprintf(stderr, "distance calls for heuristic: %zu is %.3f per doc\n", distcalls_heuristic, distcalls_heuristic / div); + fprintf(stderr, "distance calls for simple: %zu is %.3f per doc\n", distcalls_simple, distcalls_simple / div); + fprintf(stderr, "distance calls for shrink: %zu is %.3f per doc\n", distcalls_shrink, distcalls_shrink / div); + fprintf(stderr, "distance calls for refill: %zu is %.3f per doc\n", distcalls_refill, distcalls_refill / div); + fprintf(stderr, "distance calls for other: %zu is %.3f per doc\n", distcalls_other, distcalls_other / div); + fprintf(stderr, "refill needed calls: %zu is %.3f per doc\n", refill_needed_calls, refill_needed_calls / div); + } + } + + void remove_link_from(uint32_t from_id, uint32_t remove_id, uint32_t level) { + LinkList &links = getLinkList(from_id, level); + links.remove_link(remove_id); + } + + void refill_ifneeded(uint32_t my_id, const LinkList &replacements, uint32_t level) { + LinkList &my_links = getLinkList(my_id, level); + if (my_links.size() < 8) { + ++refill_needed_calls; + for (uint32_t repl_id : replacements) { + if (repl_id == my_id) continue; + if (my_links.has_link_to(repl_id)) continue; + LinkList &other_links = getLinkList(repl_id, level); + if (other_links.size() >= _M) continue; + other_links.push_back(my_id); + my_links.push_back(repl_id); + } + } } void connect_new_node(uint32_t id, const LinkList &neighbors, uint32_t level); + void shrink_links(uint32_t shrink_id, uint32_t maxLinks, uint32_t level) { + LinkList &links = getLinkList(shrink_id, level); + NearestList distances; + for (uint32_t n_id : links) { + double n_dist = distance(shrink_id, n_id); + ++distcalls_shrink; + distances.emplace_back(n_id, SqDist(n_dist)); + } + LinkList lostLinks; + LinkList oldLinks = links; + links = remove_weakest(distances, maxLinks, lostLinks); + for (uint32_t lost_id : lostLinks) { + remove_link_from(lost_id, shrink_id, level); + refill_ifneeded(lost_id, oldLinks, level); + } + } + void each_shrink_ifneeded(const LinkList &neighbors, uint32_t level); - void removeDoc(uint32_t ) override { + void removeDoc(uint32_t docid) override { + Node &node = _nodes[docid]; + bool need_new_entrypoint = (docid == _entryId); + for (int level = node._links.size(); level-- > 0; ) { + const LinkList &my_links = node._links[level]; + for (uint32_t n_id : my_links) { + if (need_new_entrypoint) { + _entryId = n_id; + _entryLevel = level; + need_new_entrypoint = false; + } + remove_link_from(n_id, docid, level); + refill_ifneeded(n_id, my_links, level); + } + } + node = Node(docid, 0, _M); + if (need_new_entrypoint) { + _entryLevel = -1; + _entryId = 0; + for (uint32_t i = 0; i < _nodes.size(); ++i) { + if (_nodes[i]._links.size() > 0) { + _entryId = i; + _entryLevel = _nodes[i]._links.size() - 1; + break; + } + } + } } std::vector<NnsHit> topK(uint32_t k, Vector vector, uint32_t search_k) override { std::vector<NnsHit> result; if (_entryLevel < 0) return result; double entryDist = distance(vector, _entryId); - QueueEntry entryPoint(_entryId, SqDist(entryDist)); + ++distcalls_other; + HnswHit entryPoint(_entryId, SqDist(entryDist)); int searchLevel = _entryLevel; while (searchLevel > 0) { entryPoint = search_layer_simple(vector, entryPoint, searchLevel); --searchLevel; } - NearestList entryPoints; - entryPoints.push_back(entryPoint); - FurthestPriQ w = search_layer(vector, entryPoints, std::max(k, search_k), 0); - if (w.size() < k) { - fprintf(stderr, "fewer than expected hits: k=%u, ks=%u, got=%zu\n", - k, search_k, w.size()); - } + FurthestPriQ w; + w.push(entryPoint); + search_layer(vector, w, std::max(k, search_k), 0); while (w.size() > k) { w.pop(); } - std::vector<QueueEntry> reversed; - reversed.reserve(w.size()); - while (! w.empty()) { - reversed.push_back(w.top()); - w.pop(); - } - result.reserve(reversed.size()); - while (! reversed.empty()) { - const QueueEntry &hit = reversed.back(); + NearestList tmp = w.steal(); + std::sort(tmp.begin(), tmp.end(), LesserDist()); + result.reserve(tmp.size()); + for (const auto & hit : tmp) { result.emplace_back(hit.docid, SqDist(hit.dist)); - reversed.pop_back(); } return result; } @@ -310,78 +359,193 @@ HnswLikeNns::distance(Vector v, uint32_t b) const void HnswLikeNns::each_shrink_ifneeded(const LinkList &neighbors, uint32_t level) { - uint32_t maxLinks = (level > 0) ? _M : (2 * _M); - for (uint32_t old_id : neighbors) { - LinkList &oldLinks = getLinkList(old_id, level); - if (oldLinks.size() > maxLinks) { - NearestPriQ w; - for (uint32_t n_id : oldLinks) { - double n_dist = distance(old_id, n_id); - w.emplace(n_id, SqDist(n_dist)); - } - oldLinks = select_neighbors(std::move(w), maxLinks); - } + uint32_t maxLinks = (level > 0) ? _M : (2 * _M); + for (uint32_t old_id : neighbors) { + LinkList &oldLinks = getLinkList(old_id, level); + if (oldLinks.size() > maxLinks) { + shrink_links(old_id, maxLinks, level); } + } } void -HnswLikeNns::search_layer_foradd(Vector vector, FurthestPriQ &w, - uint32_t ef, uint32_t searchLevel) +HnswLikeNns::search_layer(Vector vector, FurthestPriQ &w, + uint32_t ef, uint32_t searchLevel) { - NearestPriQ candidates; - VisitedSet &visited = _visitedSetPool.get(_nodes.size()); + NearestPriQ candidates; + VisitedSet &visited = _visitedSetPool.get(_nodes.size()); - for (const QueueEntry& entry : w.peek()) { - candidates.push(entry); - visited.mark(entry.docid); + for (const HnswHit & entry : w.peek()) { + candidates.push(entry); + visited.mark(entry.docid); + } + double limd = std::numeric_limits<double>::max(); + while (! candidates.empty()) { + HnswHit cand = candidates.top(); + if (cand.dist > limd) { + break; } - - double limd = std::numeric_limits<double>::max(); - while (! candidates.empty()) { - QueueEntry cand = candidates.top(); - candidates.pop(); - if (cand.dist > limd) { - break; - } - for (uint32_t e_id : getLinkList(cand.docid, searchLevel)) { - if (visited.isMarked(e_id)) continue; - visited.mark(e_id); - double e_dist = distance(vector, e_id); - if (e_dist < limd) { - candidates.emplace(e_id, SqDist(e_dist)); - w.emplace(e_id, SqDist(e_dist)); - if (w.size() > ef) { - w.pop(); - limd = w.top().dist; - } + candidates.pop(); + for (uint32_t e_id : getLinkList(cand.docid, searchLevel)) { + if (visited.isMarked(e_id)) continue; + visited.mark(e_id); + double e_dist = distance(vector, e_id); + ++distcalls_search_layer; + if (e_dist < limd) { + candidates.emplace(e_id, SqDist(e_dist)); + w.emplace(e_id, SqDist(e_dist)); + if (w.size() > ef) { + w.pop(); + limd = w.top().dist; } } } - return; + } + return; } LinkList -HnswLikeNns::select_neighbors(NearestPriQ &&w, uint32_t curMax) const { - LinkList result; - result.reserve(curMax+1); - while (! w.empty()) { - QueueEntry e = w.top(); - w.pop(); - if (haveCloserDistance(e, result)) continue; +HnswLikeNns::remove_weakest(const NearestList &neighbors, uint32_t curMax, LinkList &lost) const +{ + LinkList result; + result.reserve(curMax+1); + NearestPriQ w; + for (const auto & entry : neighbors) { + w.push(entry); + } + while (! w.empty()) { + HnswHit e = w.top(); + w.pop(); + if (result.size() == curMax || haveCloserDistance(e, result)) { + lost.push_back(e.docid); + } else { result.push_back(e.docid); - if (result.size() >= curMax) break; } - return result; + } + return result; } +#ifdef NO_BACKFILL +LinkList +HnswLikeNns::select_neighbors(const NearestList &neighbors, uint32_t curMax) const +{ + LinkList result; + result.reserve(curMax+1); + bool needFiltering = (neighbors.size() > curMax); + NearestPriQ w; + for (const auto & entry : neighbors) { + w.push(entry); + } + while (! w.empty()) { + HnswHit e = w.top(); + w.pop(); + if (needFiltering && haveCloserDistance(e, result)) { + continue; + } + result.push_back(e.docid); + if (result.size() == curMax) return result; + } + return result; +} +#else +LinkList +HnswLikeNns::select_neighbors(const NearestList &neighbors, uint32_t curMax) const +{ + LinkList result; + result.reserve(curMax+1); + bool needFiltering = (neighbors.size() > curMax); + NearestPriQ w; + for (const auto & entry : neighbors) { + w.push(entry); + } + LinkList backfill; + while (! w.empty()) { + HnswHit e = w.top(); + w.pop(); + if (needFiltering && haveCloserDistance(e, result)) { + backfill.push_back(e.docid); + continue; + } + result.push_back(e.docid); + if (result.size() == curMax) return result; + } + if (result.size() * 4 < curMax) { + for (uint32_t fill_id : backfill) { + result.push_back(fill_id); + if (result.size() * 4 >= curMax) break; + } + } + return result; +} +#endif + void HnswLikeNns::connect_new_node(uint32_t id, const LinkList &neighbors, uint32_t level) { - LinkList &newLinks = getLinkList(id, level); - for (uint32_t neigh_id : neighbors) { - LinkList &oldLinks = getLinkList(neigh_id, level); - newLinks.push_back(neigh_id); - oldLinks.push_back(id); + LinkList &newLinks = getLinkList(id, level); + for (uint32_t neigh_id : neighbors) { + LinkList &oldLinks = getLinkList(neigh_id, level); + newLinks.push_back(neigh_id); + oldLinks.push_back(id); + } +} + +void +HnswLikeNns::dumpStats() const { + std::vector<uint32_t> inLinkCounters; + inLinkCounters.resize(_nodes.size()); + std::vector<uint32_t> outLinkCounters; + outLinkCounters.resize(_nodes.size()); + std::vector<uint32_t> levelCounts; + levelCounts.resize(_entryLevel + 2); + std::vector<uint32_t> outLinkHist; + outLinkHist.resize(2 * _M + 2); + fprintf(stderr, "stats for HnswLikeNns with %zu nodes, entry level = %d, entry id = %u\n", + _nodes.size(), _entryLevel, _entryId); + for (uint32_t id = 0; id < _nodes.size(); ++id) { + const auto &node = _nodes[id]; + uint32_t levels = node._links.size(); + levelCounts[levels]++; + if (levels < 1) { + outLinkCounters[id] = 0; + outLinkHist[0]++; + continue; } + const LinkList &link_list = getLinkList(id, 0); + uint32_t numlinks = link_list.size(); + outLinkCounters[id] = numlinks; + outLinkHist[numlinks]++; + if (numlinks < 2) { + fprintf(stderr, "node with %u links: id %u\n", numlinks, id); + for (uint32_t n_id : link_list) { + const LinkList &neigh_list = getLinkList(n_id, 0); + fprintf(stderr, "neighbor id %u has %zu links\n", n_id, neigh_list.size()); + if (! neigh_list.has_link_to(id)) { + fprintf(stderr, "BAD neighbor %u is missing backlink\n", n_id); + } + } + } + for (uint32_t n_id : link_list) { + inLinkCounters[n_id]++; + } + } + for (uint32_t l = 0; l < levelCounts.size(); ++l) { + fprintf(stderr, "Nodes on %u levels: %u\n", l, levelCounts[l]); + } + for (uint32_t l = 0; l < outLinkHist.size(); ++l) { + fprintf(stderr, "Nodes with %u outward links on L0: %u\n", l, outLinkHist[l]); + } + uint32_t symmetrics = 0; + std::vector<uint32_t> inLinkHist; + for (uint32_t id = 0; id < _nodes.size(); ++id) { + uint32_t cnt = inLinkCounters[id]; + while (cnt >= inLinkHist.size()) inLinkHist.push_back(0); + inLinkHist[cnt]++; + if (cnt == outLinkCounters[id]) ++symmetrics; + } + for (uint32_t l = 0; l < inLinkHist.size(); ++l) { + fprintf(stderr, "Nodes with %u inward links on L0: %u\n", l, inLinkHist[l]); + } + fprintf(stderr, "Symmetric in-out nodes: %u\n", symmetrics); } @@ -390,5 +554,3 @@ make_hnsw_nns(uint32_t numDims, const DocVectorAccess<float> &dva) { return std::make_unique<HnswLikeNns>(numDims, dva); } - - |