diff options
34 files changed, 180 insertions, 267 deletions
diff --git a/athenz-identity-provider-service/src/main/java/com/yahoo/vespa/hosted/athenz/instanceproviderservice/AthenzSslKeyStoreConfigurator.java b/athenz-identity-provider-service/src/main/java/com/yahoo/vespa/hosted/athenz/instanceproviderservice/AthenzSslKeyStoreConfigurator.java index 7910650ed5e..3b7d05bf026 100644 --- a/athenz-identity-provider-service/src/main/java/com/yahoo/vespa/hosted/athenz/instanceproviderservice/AthenzSslKeyStoreConfigurator.java +++ b/athenz-identity-provider-service/src/main/java/com/yahoo/vespa/hosted/athenz/instanceproviderservice/AthenzSslKeyStoreConfigurator.java @@ -3,7 +3,6 @@ package com.yahoo.vespa.hosted.athenz.instanceproviderservice; import com.google.inject.Inject; import com.yahoo.component.AbstractComponent; -import com.yahoo.config.provision.SystemName; import com.yahoo.config.provision.Zone; import com.yahoo.jdisc.http.ssl.SslKeyStoreConfigurator; import com.yahoo.jdisc.http.ssl.SslKeyStoreContext; @@ -41,7 +40,6 @@ public class AthenzSslKeyStoreConfigurator extends AbstractComponent implements private final KeyProvider keyProvider; private final AthenzProviderServiceConfig.Zones zoneConfig; private final AtomicBoolean alreadyConfigured = new AtomicBoolean(); - private final Zone zone; @Inject public AthenzSslKeyStoreConfigurator(KeyProvider keyProvider, @@ -51,15 +49,10 @@ public class AthenzSslKeyStoreConfigurator extends AbstractComponent implements this.certificateClient = new AthenzCertificateClient(config, zoneConfig); this.keyProvider = keyProvider; this.zoneConfig = zoneConfig; - this.zone = zone; } @Override public void configure(SslKeyStoreContext sslKeyStoreContext) { - // TODO Remove this when main is ready - if (zone.system() != SystemName.cd) { - return; - } if (alreadyConfigured.getAndSet(true)) { // For debugging purpose of SslKeyStoreConfigurator interface throw new IllegalStateException("Already configured. configure() can only be called once."); } diff --git a/athenz-identity-provider-service/src/main/java/com/yahoo/vespa/hosted/athenz/instanceproviderservice/AthenzSslTrustStoreConfigurator.java b/athenz-identity-provider-service/src/main/java/com/yahoo/vespa/hosted/athenz/instanceproviderservice/AthenzSslTrustStoreConfigurator.java index 059c91aecd3..8c8b5de2a30 100644 --- a/athenz-identity-provider-service/src/main/java/com/yahoo/vespa/hosted/athenz/instanceproviderservice/AthenzSslTrustStoreConfigurator.java +++ b/athenz-identity-provider-service/src/main/java/com/yahoo/vespa/hosted/athenz/instanceproviderservice/AthenzSslTrustStoreConfigurator.java @@ -20,6 +20,7 @@ import org.bouncycastle.operator.ContentSigner; import org.bouncycastle.operator.OperatorCreationException; import org.bouncycastle.operator.jcajce.JcaContentSignerBuilder; +import java.io.FileInputStream; import java.io.IOException; import java.math.BigInteger; import java.security.KeyPair; @@ -35,7 +36,6 @@ import java.util.logging.Logger; /** * @author bjorncs */ -// TODO Add Athenz CA certificates to trust store public class AthenzSslTrustStoreConfigurator implements SslTrustStoreConfigurator { private static final Logger log = Logger.getLogger(AthenzSslTrustStoreConfigurator.class.getName()); @@ -64,7 +64,9 @@ public class AthenzSslTrustStoreConfigurator implements SslTrustStoreConfigurato X509Certificate selfSignedCertificate = createSelfSignedCertificate(keyPair, configserverConfig); log.log(LogLevel.FINE, "Generated self-signed certificate: " + selfSignedCertificate); KeyStore trustStore = KeyStore.getInstance("JKS"); - trustStore.load(null); + try (FileInputStream in = new FileInputStream(athenzProviderServiceConfig.athenzCaTrustStore())) { + trustStore.load(in, "changeit".toCharArray()); + } trustStore.setCertificateEntry("cfgselfsigned", selfSignedCertificate); return trustStore; } catch (Exception e) { diff --git a/athenz-identity-provider-service/src/main/resources/configdefinitions/athenz-provider-service.def b/athenz-identity-provider-service/src/main/resources/configdefinitions/athenz-provider-service.def index 13cc78b0bd0..21f2aea6ab0 100644 --- a/athenz-identity-provider-service/src/main/resources/configdefinitions/athenz-provider-service.def +++ b/athenz-identity-provider-service/src/main/resources/configdefinitions/athenz-provider-service.def @@ -21,3 +21,6 @@ ztsUrl string # Certificate DNS suffix certDnsSuffix string + +# Path to Athenz CA JKS trust store +athenzCaTrustStore string diff --git a/athenz-identity-provider-service/src/test/java/com/yahoo/vespa/hosted/athenz/instanceproviderservice/TestUtils.java b/athenz-identity-provider-service/src/test/java/com/yahoo/vespa/hosted/athenz/instanceproviderservice/TestUtils.java index c09a9fb1740..da2bf929e82 100644 --- a/athenz-identity-provider-service/src/test/java/com/yahoo/vespa/hosted/athenz/instanceproviderservice/TestUtils.java +++ b/athenz-identity-provider-service/src/test/java/com/yahoo/vespa/hosted/athenz/instanceproviderservice/TestUtils.java @@ -25,7 +25,8 @@ public class TestUtils { .zones(ImmutableMap.of(zone.environment().value() + "." + zone.region().value(), zoneConfig)) .certDnsSuffix(dnsSuffix) .ztsUrl("localhost/zts") - .athenzPrincipalHeaderName("Athenz-Principal-Auth")); + .athenzPrincipalHeaderName("Athenz-Principal-Auth") + .athenzCaTrustStore("/dummy/path/to/athenz-ca.jks")); } } diff --git a/config-model/src/test/java/com/yahoo/vespa/model/clients/test/Gateway20TestCase.java b/config-model/src/test/java/com/yahoo/vespa/model/clients/test/Gateway20TestCase.java deleted file mode 100644 index e7df151a3c4..00000000000 --- a/config-model/src/test/java/com/yahoo/vespa/model/clients/test/Gateway20TestCase.java +++ /dev/null @@ -1,65 +0,0 @@ -// Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -package com.yahoo.vespa.model.clients.test; - -import com.yahoo.container.ComponentsConfig; -import com.yahoo.container.QrConfig; -import com.yahoo.container.QrConfig.Builder; -import com.yahoo.net.HostName; -import com.yahoo.vespa.model.VespaModel; -import com.yahoo.vespa.model.test.utils.VespaModelCreatorWithFilePkg; -import org.junit.Test; - -import java.util.ArrayList; -import java.util.Arrays; -import java.util.List; - -import static org.junit.Assert.*; - -/** - * @author Gunnar Gauslaa Bergem - */ -public class Gateway20TestCase { - - private static String hostname = HostName.getLocalhost(); // Using the same way of getting hostname as filedistribution model - - @Test - public void testSimpleDocprocV3() throws Exception { - VespaModel model = new VespaModelCreatorWithFilePkg("src/test/cfg/clients/simpleconfig.v2.docprocv3").create(); - QrConfig qrConfig = new QrConfig((Builder) model.getConfig(new QrConfig.Builder(), "container/container.0")); - assertEquals(qrConfig.rpc().enabled(), true); - assertEquals("filedistribution/" + hostname, qrConfig.filedistributor().configid()); - assertEquals("container.container.0", qrConfig.discriminator()); - - ComponentsConfig componentsConfig = new ComponentsConfig((ComponentsConfig.Builder) model.getConfig(new ComponentsConfig.Builder(), "container/container.0")); - ArrayList<String> components = new ArrayList<>(); - for (ComponentsConfig.Components component : componentsConfig.components()) { - components.add(component.id()); - } - List<String> expectedComponents = Arrays.asList("com.yahoo.docproc.jdisc.DocumentProcessingHandler", - "com.yahoo.feedhandler.VespaFeedHandler", - "com.yahoo.feedhandler.VespaFeedHandlerCompatibility", - "com.yahoo.feedhandler.VespaFeedHandlerGet", - "com.yahoo.feedhandler.VespaFeedHandlerRemove", - "com.yahoo.feedhandler.VespaFeedHandlerRemoveLocation", - "com.yahoo.feedhandler.VespaFeedHandlerStatus", - "com.yahoo.feedhandler.VespaFeedHandlerVisit", - "com.yahoo.search.handler.SearchHandler", - "com.yahoo.container.jdisc.state.StateHandler"); - assertTrue(components.containsAll(expectedComponents)); - } - - @Test - public void testAdvanced() throws Exception { - VespaModel model = new VespaModelCreatorWithFilePkg("src/test/cfg/clients/advancedconfig.v2").create(); - - QrConfig qrConfig = new QrConfig((Builder) model.getConfig(new QrConfig.Builder(), "container/container.0")); - assertEquals(qrConfig.rpc().enabled(), true); - assertEquals(qrConfig.filedistributor().configid(), "filedistribution/" + hostname); - assertEquals("container.container.0", qrConfig.discriminator()); - - qrConfig = new QrConfig((Builder) model.getConfig(new QrConfig.Builder(), "container/container.0")); - assertEquals(qrConfig.rpc().enabled(), true); - assertEquals(qrConfig.filedistributor().configid(), "filedistribution/" + hostname); - } - -} 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 d09211aea45..5e093bdb32a 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 @@ -16,6 +16,7 @@ import com.yahoo.config.provision.Environment; import com.yahoo.config.provision.RegionName; import com.yahoo.config.provision.Zone; import com.yahoo.container.ComponentsConfig; +import com.yahoo.container.QrConfig; import com.yahoo.container.config.StatisticsRequestHandler; import com.yahoo.container.core.ChainsConfig; import com.yahoo.container.core.VipStatusConfig; @@ -25,6 +26,7 @@ import com.yahoo.container.jdisc.JdiscBindingsConfig; import com.yahoo.container.servlet.ServletConfigConfig; import com.yahoo.container.usability.BindingsOverviewHandler; import com.yahoo.jdisc.http.ServletPathsConfig; +import com.yahoo.net.HostName; import com.yahoo.prelude.cluster.QrMonitorConfig; import com.yahoo.vespa.model.AbstractService; import com.yahoo.vespa.model.VespaModel; @@ -61,7 +63,6 @@ import static org.junit.Assert.fail; /** * @author gjoranv - * @since 5.1.9 */ public class ContainerModelBuilderTest extends ContainerModelBuilderTestBase { @@ -571,6 +572,41 @@ public class ContainerModelBuilderTest extends ContainerModelBuilderTestBase { assertEquals(ContainerModelBuilder.HOSTED_VESPA_STATUS_FILE, vipStatusConfig.statusfile()); } + @Test + public void qrconfig_is_produced() throws IOException, SAXException { + String servicesXml = + "<services>" + + "<admin version='3.0'>" + + " <nodes count='1'/>" + + "</admin>" + + "<jdisc id ='default' version='1.0'>" + + " <nodes>" + + " <node hostalias='node1' />" + + " </nodes>" + + "</jdisc>" + + "</services>"; + + ApplicationPackage applicationPackage = new MockApplicationPackage.Builder() + .withServices(servicesXml) + .build(); + VespaModel model = new VespaModel(new NullConfigModelRegistry(), new DeployState.Builder() + .applicationPackage(applicationPackage) + .properties(new DeployProperties.Builder().build()) + .build()); + + String hostname = HostName.getLocalhost(); // Using the same way of getting hostname as filedistribution model + + QrConfig config = model.getConfig(QrConfig.class, "default/container.0"); + assertEquals("default.container.0", config.discriminator()); + assertEquals(19102, config.rpc().port()); + assertEquals("vespa/service/default/container.0", config.rpc().slobrokId()); + assertEquals(true, config.rpc().enabled()); + assertEquals("", config.rpc().host()); + assertEquals(false, config.restartOnDeploy()); + assertEquals(false, config.coveragereports()); + assertEquals("filedistribution/" + hostname, config.filedistributor().configid()); + } + private Element generateContainerElementWithRenderer(String rendererId) { return DomBuilderTest.parse( "<jdisc id='default' version='1.0'>", diff --git a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/security/KeyService.java b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/security/KeyService.java deleted file mode 100644 index 61cd738314a..00000000000 --- a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/security/KeyService.java +++ /dev/null @@ -1,18 +0,0 @@ -// Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -package com.yahoo.vespa.hosted.controller.api.integration.security; - -/** - * A service for retrieving secrets, such as API keys, private keys and passwords. - * - * @author mpolden - * @author bjorncs - */ -public interface KeyService { - - String getSecret(String key); - - default String getSecret(String key, int version) { - throw new UnsupportedOperationException("KeyService implementation does not support versioned secrets"); - } - -} diff --git a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/security/KeyServiceMock.java b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/security/KeyServiceMock.java deleted file mode 100644 index 46fa2a593c5..00000000000 --- a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/security/KeyServiceMock.java +++ /dev/null @@ -1,13 +0,0 @@ -package com.yahoo.vespa.hosted.controller.api.integration.security; - -/** - * @author mpolden - */ -public class KeyServiceMock implements KeyService { - - @Override - public String getSecret(String key) { - return "fake-secret-for-" + key; - } - -} diff --git a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/security/package-info.java b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/security/package-info.java deleted file mode 100644 index 296eebf8ea5..00000000000 --- a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/security/package-info.java +++ /dev/null @@ -1,5 +0,0 @@ -// Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -@ExportPackage -package com.yahoo.vespa.hosted.controller.api.integration.security; - -import com.yahoo.osgi.annotation.ExportPackage; 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 ce4bb912e97..a489b0f4e91 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 @@ -30,9 +30,9 @@ import com.yahoo.vespa.hosted.controller.api.integration.dns.Record; import com.yahoo.vespa.hosted.controller.api.integration.dns.RecordId; import com.yahoo.vespa.hosted.controller.api.integration.routing.RoutingEndpoint; import com.yahoo.vespa.hosted.controller.api.integration.routing.RoutingGenerator; -import com.yahoo.vespa.hosted.controller.application.ApplicationRotation; import com.yahoo.vespa.hosted.controller.application.ApplicationPackage; import com.yahoo.vespa.hosted.controller.application.ApplicationRevision; +import com.yahoo.vespa.hosted.controller.application.ApplicationRotation; import com.yahoo.vespa.hosted.controller.application.Change; import com.yahoo.vespa.hosted.controller.application.Deployment; import com.yahoo.vespa.hosted.controller.application.DeploymentJobs; @@ -48,7 +48,6 @@ import com.yahoo.vespa.hosted.controller.maintenance.DeploymentExpirer; import com.yahoo.vespa.hosted.controller.persistence.ControllerDb; import com.yahoo.vespa.hosted.controller.persistence.CuratorDb; import com.yahoo.vespa.hosted.controller.rotation.Rotation; -import com.yahoo.vespa.hosted.controller.rotation.RotationId; import com.yahoo.vespa.hosted.controller.rotation.RotationRepository; import com.yahoo.vespa.hosted.rotation.config.RotationsConfig; import com.yahoo.yolean.Exceptions; @@ -116,20 +115,7 @@ public class ApplicationController { this.deploymentTrigger = new DeploymentTrigger(controller, curator, clock); for (Application application : db.listApplications()) { - lockedIfPresent(application.id(), (app) -> { - // TODO: Remove after December 2017. Migrates rotations into application - if (!app.rotation().isPresent()) { - Set<com.yahoo.vespa.hosted.controller.api.identifiers.RotationId> rotations = db.getRotations(application.id()); - if (rotations.size() > 1) { - log.warning("Application " + application.id() + " has more than 1 rotation: " - + rotations.size()); - } - if (!rotations.isEmpty()) { - app = app.with(new RotationId(rotations.iterator().next().id())); - } - } - store(app); - }); + lockIfPresent(application.id(), this::store); } } @@ -249,10 +235,6 @@ public class ApplicationController { if ( ! (id.instance().value().equals("default") || id.instance().value().startsWith("default-pr"))) // TODO: Support instances properly throw new UnsupportedOperationException("Only the instance names 'default' and names starting with 'default-pr' are supported at the moment"); try (Lock lock = lock(id)) { - // TODO: Throwing is duplicated below. - if (get(id).isPresent()) - throw new IllegalArgumentException("An application with id '" + id + "' already exists"); - com.yahoo.vespa.hosted.controller.api.identifiers.ApplicationId.validate(id.application().value()); Optional<Tenant> tenant = controller.tenants().tenant(new TenantId(id.tenant().value())); @@ -525,7 +507,7 @@ public class ApplicationController { if ( ! controller.applications().get(id).isPresent()) throw new NotExistsException("Could not delete application '" + id + "': Application not found"); - lockedOrThrow(id, application -> { + lockOrThrow(id, application -> { if ( ! application.deployments().isEmpty()) throw new IllegalArgumentException("Could not delete '" + application + "': It has active deployments"); @@ -555,24 +537,25 @@ public class ApplicationController { /** * Acquire a locked application to modify and store, if there is an application with the given id. * - * @param applicationId Id of the application to lock and get. - * @param actions Things to do with the locked application. + * @param applicationId ID of the application to lock and get. + * @param action Function which acts on the locked application. */ - public void lockedIfPresent(ApplicationId applicationId, Consumer<LockedApplication> actions) { + public void lockIfPresent(ApplicationId applicationId, Consumer<LockedApplication> action) { try (Lock lock = lock(applicationId)) { - get(applicationId).map(application -> new LockedApplication(application, lock)).ifPresent(actions); + get(applicationId).map(application -> new LockedApplication(application, lock)).ifPresent(action); } } /** * Acquire a locked application to modify and store, or throw an exception if no application has the given id. * - * @param applicationId Id of the application to lock and require. - * @param actions Things to do with the locked application. + * @param applicationId ID of the application to lock and require. + * @param action Function which acts on the locked application. + * @throws IllegalArgumentException when application does not exist. */ - public void lockedOrThrow(ApplicationId applicationId, Consumer<LockedApplication> actions) { + public void lockOrThrow(ApplicationId applicationId, Consumer<LockedApplication> action) { try (Lock lock = lock(applicationId)) { - actions.accept(new LockedApplication(require(applicationId), lock)); + action.accept(new LockedApplication(require(applicationId), lock)); } } @@ -585,18 +568,14 @@ public class ApplicationController { deploymentTrigger.triggerFromCompletion(report); } - // TODO: Collapse this method and the next - public void restart(DeploymentId deploymentId) { - try { - configserverClient.restart(deploymentId, Optional.empty()); - } - catch (NoInstanceException e) { - throw new IllegalArgumentException("Could not restart " + deploymentId + ": No such deployment"); - } - } - public void restartHost(DeploymentId deploymentId, Hostname hostname) { + /** + * Tells config server to schedule a restart of all nodes in this deployment + * + * @param hostname If non-empty, restart will only be scheduled for this host + */ + public void restart(DeploymentId deploymentId, Optional<Hostname> hostname) { try { - configserverClient.restart(deploymentId, Optional.of(hostname)); + configserverClient.restart(deploymentId, hostname); } catch (NoInstanceException e) { throw new IllegalArgumentException("Could not restart " + deploymentId + ": No such deployment"); @@ -619,7 +598,7 @@ public class ApplicationController { && ! DeploymentExpirer.hasExpired(controller.zoneRegistry(), deployment.get(), clock.instant())) return; - lockedOrThrow(application.id(), lockedApplication -> + lockOrThrow(application.id(), lockedApplication -> store(deactivate(lockedApplication, zone))); } diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/athenz/impl/AthenzClientFactoryImpl.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/athenz/impl/AthenzClientFactoryImpl.java index 1c32b35f599..44493d6818a 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/athenz/impl/AthenzClientFactoryImpl.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/athenz/impl/AthenzClientFactoryImpl.java @@ -10,7 +10,7 @@ import com.yahoo.athenz.auth.token.PrincipalToken; import com.yahoo.athenz.auth.util.Crypto; import com.yahoo.athenz.zms.ZMSClient; import com.yahoo.athenz.zts.ZTSClient; -import com.yahoo.vespa.hosted.controller.api.integration.security.KeyService; +import com.yahoo.jdisc.http.SecretStore; import com.yahoo.vespa.hosted.controller.athenz.AthenzClientFactory; import com.yahoo.vespa.hosted.controller.athenz.NToken; import com.yahoo.vespa.hosted.controller.athenz.ZmsClient; @@ -27,13 +27,13 @@ import static com.yahoo.vespa.hosted.controller.athenz.AthenzUtils.USER_PRINCIPA */ public class AthenzClientFactoryImpl implements AthenzClientFactory { - private final KeyService secretService; + private final SecretStore secretStore; private final AthenzConfig config; private final AthenzPrincipalAuthority athenzPrincipalAuthority; @Inject - public AthenzClientFactoryImpl(KeyService secretService, AthenzConfig config) { - this.secretService = secretService; + public AthenzClientFactoryImpl(SecretStore secretStore, AthenzConfig config) { + this.secretStore = secretStore; this.config = config; this.athenzPrincipalAuthority = new AthenzPrincipalAuthority(config.principalHeaderName()); } @@ -82,7 +82,7 @@ public class AthenzClientFactoryImpl implements AthenzClientFactory { private PrivateKey getServicePrivateKey() { AthenzConfig.Service service = config.service(); - String privateKey = secretService.getSecret(service.privateKeySecretName(), service.privateKeyVersion()).trim(); + String privateKey = secretStore.getSecret(service.privateKeySecretName(), service.privateKeyVersion()).trim(); return Crypto.loadPrivateKey(privateKey); } diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/athenz/impl/ZmsClientImpl.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/athenz/impl/ZmsClientImpl.java index ee488914aa9..beca72cc3ca 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/athenz/impl/ZmsClientImpl.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/athenz/impl/ZmsClientImpl.java @@ -66,11 +66,11 @@ public class ZmsClientImpl implements ZmsClient { List<TenantRoleAction> tenantRoleActions = createTenantRoleActions(); log("putProviderResourceGroupRoles(" + "tenantDomain=%s, providerDomain=%s, service=%s, resourceGroup=%s, roleActions=%s)", - tenantDomain, service.getDomain().id(), service.getFullName(), applicationName, tenantRoleActions); + tenantDomain, service.getDomain().id(), service.getName(), applicationName, tenantRoleActions); runOrThrow(() -> { ProviderResourceGroupRoles resourceGroupRoles = new ProviderResourceGroupRoles() .setDomain(service.getDomain().id()) - .setService(service.getFullName()) + .setService(service.getName()) .setTenant(tenantDomain.id()) .setResourceGroup(applicationName.id()) .setRoles(tenantRoleActions); diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentTrigger.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentTrigger.java index 1eee727214b..dada11ca436 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentTrigger.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentTrigger.java @@ -5,7 +5,6 @@ import com.yahoo.component.Version; import com.yahoo.config.provision.ApplicationId; import com.yahoo.config.provision.SystemName; import com.yahoo.config.provision.Zone; -import com.yahoo.vespa.curator.Lock; import com.yahoo.vespa.hosted.controller.Application; import com.yahoo.vespa.hosted.controller.ApplicationController; import com.yahoo.vespa.hosted.controller.Controller; @@ -28,7 +27,6 @@ import java.util.ArrayList; import java.util.List; import java.util.Objects; import java.util.Optional; -import java.util.function.Consumer; import java.util.logging.Logger; /** @@ -79,7 +77,7 @@ public class DeploymentTrigger { * @param report information about the job that just completed */ public void triggerFromCompletion(JobReport report) { - applications().lockedOrThrow(report.applicationId(), application -> { + applications().lockOrThrow(report.applicationId(), application -> { application = application.withJobCompletion(report, clock.instant(), controller); // Handle successful starting and ending @@ -132,7 +130,7 @@ public class DeploymentTrigger { ApplicationList applications = ApplicationList.from(applications().asList()); applications = applications.notPullRequest(); for (Application application : applications.asList()) - applications().lockedIfPresent(application.id(), this::triggerReadyJobs); + applications().lockIfPresent(application.id(), this::triggerReadyJobs); } /** Find the next step to trigger if any, and triggers it */ @@ -219,7 +217,7 @@ public class DeploymentTrigger { * @throws IllegalArgumentException if this application already have an ongoing change */ public void triggerChange(ApplicationId applicationId, Change change) { - applications().lockedOrThrow(applicationId, application -> { + applications().lockOrThrow(applicationId, application -> { if (application.deploying().isPresent() && ! application.deploymentJobs().hasFailures()) throw new IllegalArgumentException("Could not start " + change + " on " + application + ": " + application.deploying().get() + " is already in progress"); @@ -238,7 +236,7 @@ public class DeploymentTrigger { * @param applicationId the application to trigger */ public void cancelChange(ApplicationId applicationId) { - applications().lockedOrThrow(applicationId, application -> { + applications().lockOrThrow(applicationId, application -> { buildSystem.removeJobs(application.id()); applications().store(application.withDeploying(Optional.empty())); }); diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/ApplicationOwnershipConfirmer.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/ApplicationOwnershipConfirmer.java index 09f8df58205..2b7260d5ffa 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/ApplicationOwnershipConfirmer.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/ApplicationOwnershipConfirmer.java @@ -1,7 +1,6 @@ package com.yahoo.vespa.hosted.controller.maintenance; import com.yahoo.config.provision.ApplicationId; -import com.yahoo.vespa.curator.Lock; import com.yahoo.vespa.hosted.controller.Application; import com.yahoo.vespa.hosted.controller.Controller; import com.yahoo.vespa.hosted.controller.api.Tenant; @@ -86,7 +85,7 @@ public class ApplicationOwnershipConfirmer extends Maintainer { } protected void store(IssueId issueId, ApplicationId applicationId) { - controller().applications().lockedIfPresent(applicationId, application -> + controller().applications().lockIfPresent(applicationId, application -> controller().applications().store(application.withOwnershipIssueId(issueId))); } } diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/ClusterInfoMaintainer.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/ClusterInfoMaintainer.java index ae617f87be6..f7284272b38 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/ClusterInfoMaintainer.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/ClusterInfoMaintainer.java @@ -4,10 +4,8 @@ package com.yahoo.vespa.hosted.controller.maintenance; import com.yahoo.config.provision.ClusterSpec; import com.yahoo.config.provision.Flavor; import com.yahoo.config.provision.Zone; -import com.yahoo.vespa.curator.Lock; import com.yahoo.vespa.hosted.controller.Application; import com.yahoo.vespa.hosted.controller.Controller; -import com.yahoo.vespa.hosted.controller.LockedApplication; import com.yahoo.vespa.hosted.controller.api.identifiers.DeploymentId; import com.yahoo.vespa.hosted.controller.api.integration.configserver.NodeList; import com.yahoo.vespa.hosted.controller.application.ApplicationList; @@ -93,7 +91,7 @@ public class ClusterInfoMaintainer extends Maintainer { try { NodeList nodes = controller().applications().configserverClient().getNodeList(deploymentId); Map<ClusterSpec.Id, ClusterInfo> clusterInfo = getClusterInfo(nodes, deployment.zone()); - controller().applications().lockedIfPresent(application.id(), lockedApplication -> + controller().applications().lockIfPresent(application.id(), lockedApplication -> controller.applications().store(lockedApplication.withClusterInfo(deployment.zone(), clusterInfo))); } catch (IOException | IllegalArgumentException e) { diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/ClusterUtilizationMaintainer.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/ClusterUtilizationMaintainer.java index 3744be67135..0d472ab0910 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/ClusterUtilizationMaintainer.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/ClusterUtilizationMaintainer.java @@ -4,7 +4,6 @@ package com.yahoo.vespa.hosted.controller.maintenance; import com.yahoo.config.provision.ApplicationId; import com.yahoo.config.provision.ClusterSpec; import com.yahoo.config.provision.Zone; -import com.yahoo.vespa.curator.Lock; import com.yahoo.vespa.hosted.controller.Application; import com.yahoo.vespa.hosted.controller.Controller; import com.yahoo.vespa.hosted.controller.api.integration.MetricsService; @@ -50,7 +49,7 @@ public class ClusterUtilizationMaintainer extends Maintainer { Map<ClusterSpec.Id, ClusterUtilization> clusterUtilization = getUpdatedClusterUtilizations(application.id(), deployment.zone()); - controller().applications().lockedIfPresent(application.id(), lockedApplication -> + controller().applications().lockIfPresent(application.id(), lockedApplication -> controller().applications().store(lockedApplication.withClusterUtilization(deployment.zone(), clusterUtilization))); } } diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/DeploymentIssueReporter.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/DeploymentIssueReporter.java index ae6ba364d25..324868878af 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/DeploymentIssueReporter.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/DeploymentIssueReporter.java @@ -2,7 +2,6 @@ package com.yahoo.vespa.hosted.controller.maintenance; import com.yahoo.config.provision.ApplicationId; -import com.yahoo.vespa.curator.Lock; import com.yahoo.vespa.hosted.controller.Application; import com.yahoo.vespa.hosted.controller.Controller; import com.yahoo.vespa.hosted.controller.api.Tenant; @@ -131,7 +130,7 @@ public class DeploymentIssueReporter extends Maintainer { } private void store(ApplicationId id, IssueId issueId) { - controller().applications().lockedIfPresent(id, application -> + controller().applications().lockIfPresent(id, application -> controller().applications().store(application.withDeploymentIssueId(issueId))); } diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/DeploymentMetricsMaintainer.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/DeploymentMetricsMaintainer.java index 13eb5075f34..821efba013d 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/DeploymentMetricsMaintainer.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/DeploymentMetricsMaintainer.java @@ -1,7 +1,6 @@ // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.vespa.hosted.controller.maintenance;// Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -import com.yahoo.vespa.curator.Lock; import com.yahoo.vespa.hosted.controller.Application; import com.yahoo.vespa.hosted.controller.Controller; import com.yahoo.vespa.hosted.controller.api.integration.MetricsService; @@ -34,7 +33,7 @@ public class DeploymentMetricsMaintainer extends Maintainer { boolean hasWarned = false; for (Application application : ApplicationList.from(controller().applications().asList()).notPullRequest().asList()) { try { - controller().applications().lockedIfPresent(application.id(), lockedApplication -> + controller().applications().lockIfPresent(application.id(), lockedApplication -> controller().applications().store(lockedApplication.with(controller().metricsService().getApplicationMetrics(application.id())))); for (Deployment deployment : application.deployments().values()) { @@ -46,7 +45,7 @@ public class DeploymentMetricsMaintainer extends Maintainer { deploymentMetrics.queryLatencyMillis(), deploymentMetrics.writeLatencyMillis()); - controller().applications().lockedIfPresent(application.id(), lockedApplication -> + controller().applications().lockIfPresent(application.id(), lockedApplication -> controller().applications().store(lockedApplication.with(deployment.zone(), appMetrics))); } } diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/ControllerDb.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/ControllerDb.java index 34b3ae55c2c..fb6608ea643 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/ControllerDb.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/ControllerDb.java @@ -6,12 +6,10 @@ import com.yahoo.config.provision.ApplicationId; import com.yahoo.vespa.hosted.controller.Application; import com.yahoo.vespa.hosted.controller.api.Tenant; import com.yahoo.vespa.hosted.controller.api.identifiers.Identifier; -import com.yahoo.vespa.hosted.controller.api.identifiers.RotationId; import com.yahoo.vespa.hosted.controller.api.identifiers.TenantId; import java.util.List; import java.util.Optional; -import java.util.Set; /** * Used to store the permanent data of the controller. @@ -25,6 +23,7 @@ public interface ControllerDb { void createTenant(Tenant tenant); + // TODO: Remove exception from all signatures void updateTenant(Tenant tenant) throws PersistenceException; void deleteTenant(TenantId tenantId) throws PersistenceException; @@ -48,18 +47,6 @@ public interface ControllerDb { /** Returns all applications of a tenant */ List<Application> listApplications(TenantId tenantId); - // --------- Rotations - - // TODO: Remove all rotation methods after December 2017 - Set<RotationId> getRotations(); - - Set<RotationId> getRotations(ApplicationId applicationId); - - boolean assignRotation(RotationId rotationId, ApplicationId applicationId); - - Set<RotationId> deleteRotations(ApplicationId applicationId); - // end TODO - /** Returns the given elements joined by dot "." */ default String path(Identifier... elements) { return Joiner.on(".").join(elements); diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/CuratorDb.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/CuratorDb.java index e5616f025ce..95db401d15b 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/CuratorDb.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/CuratorDb.java @@ -222,10 +222,6 @@ public class CuratorDb { // -------------- Paths -------------------------------------------------- - private Path systemVersionPath() { - return root.append("systemVersion"); - } - private Path lockPath(TenantId tenant) { Path lockPath = root.append("locks") .append(tenant.id()); diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/MemoryControllerDb.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/MemoryControllerDb.java index 0dffd7ee520..2c5d77c7773 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/MemoryControllerDb.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/MemoryControllerDb.java @@ -6,7 +6,6 @@ import com.yahoo.vespa.hosted.controller.AlreadyExistsException; import com.yahoo.vespa.hosted.controller.Application; import com.yahoo.vespa.hosted.controller.NotExistsException; import com.yahoo.vespa.hosted.controller.api.Tenant; -import com.yahoo.vespa.hosted.controller.api.identifiers.RotationId; import com.yahoo.vespa.hosted.controller.api.identifiers.TenantId; import java.util.ArrayList; @@ -14,7 +13,6 @@ import java.util.HashMap; import java.util.List; import java.util.Map; import java.util.Optional; -import java.util.Set; import java.util.stream.Collectors; /** @@ -26,7 +24,6 @@ public class MemoryControllerDb implements ControllerDb { private final Map<TenantId, Tenant> tenants = new HashMap<>(); private final Map<String, Application> applications = new HashMap<>(); - private final Map<RotationId, ApplicationId> rotationAssignments = new HashMap<>(); @Override public void createTenant(Tenant tenant) { @@ -52,7 +49,7 @@ public class MemoryControllerDb implements ControllerDb { } @Override - public Optional<Tenant> getTenant(TenantId tenantId) throws PersistenceException { + public Optional<Tenant> getTenant(TenantId tenantId) { return Optional.ofNullable(tenants.get(tenantId)); } @@ -88,36 +85,4 @@ public class MemoryControllerDb implements ControllerDb { .collect(Collectors.toList()); } - @Override - public Set<RotationId> getRotations() { - return rotationAssignments.keySet(); - } - - @Override - public Set<RotationId> getRotations(ApplicationId applicationId) { - return rotationAssignments.entrySet().stream() - .filter(entry -> entry.getValue().equals(applicationId)) - .map(Map.Entry::getKey) - .collect(Collectors.toSet()); - } - - @Override - public boolean assignRotation(RotationId rotationId, ApplicationId applicationId) { - if (rotationAssignments.containsKey(rotationId)) { - return false; - } else { - rotationAssignments.put(rotationId, applicationId); - return true; - } - } - - @Override - public Set<RotationId> deleteRotations(ApplicationId applicationId) { - Set<RotationId> rotations = getRotations(applicationId); - for (RotationId rotation : rotations) { - rotationAssignments.remove(rotation); - } - return rotations; - } - } diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiHandler.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiHandler.java index 432e1a9cc40..f7810744b71 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiHandler.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiHandler.java @@ -710,7 +710,7 @@ public class ApplicationApiHandler extends LoggingRequestHandler { "Active versions: " + controller.versionStatus().versions()); ApplicationId id = ApplicationId.from(tenantName, applicationName, "default"); - controller.applications().lockedOrThrow(id, application -> { + controller.applications().lockOrThrow(id, application -> { if (application.deploying().isPresent()) throw new IllegalArgumentException("Can not start a deployment of " + application + " at this time: " + application.deploying().get() + " is in progress"); @@ -728,7 +728,7 @@ public class ApplicationApiHandler extends LoggingRequestHandler { if ( ! change.isPresent()) return new MessageResponse("No deployment in progress for " + application + " at this time"); - controller.applications().lockedOrThrow(id, lockedApplication -> + controller.applications().lockOrThrow(id, lockedApplication -> controller.applications().deploymentTrigger().cancelChange(id)); return new MessageResponse("Cancelled " + change.get() + " for " + application); @@ -739,10 +739,8 @@ public class ApplicationApiHandler extends LoggingRequestHandler { DeploymentId deploymentId = new DeploymentId(ApplicationId.from(tenantName, applicationName, instanceName), new Zone(Environment.from(environment), RegionName.from(region))); // TODO: Propagate all filters - if (request.getProperty("hostname") != null) - controller.applications().restartHost(deploymentId, new Hostname(request.getProperty("hostname"))); - else - controller.applications().restart(deploymentId); + Optional<Hostname> hostname = Optional.ofNullable(request.getProperty("hostname")).map(Hostname::new); + controller.applications().restart(deploymentId, hostname); // TODO: Change to return JSON return new StringResponse("Requested restart of " + path(TenantResource.API_PATH, tenantName, diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/screwdriver/ScrewdriverApiHandler.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/screwdriver/ScrewdriverApiHandler.java index e350b98adb9..4d4f01bc1a6 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/screwdriver/ScrewdriverApiHandler.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/screwdriver/ScrewdriverApiHandler.java @@ -12,9 +12,7 @@ import com.yahoo.slime.Cursor; import com.yahoo.slime.Inspector; import com.yahoo.slime.Slime; import com.yahoo.vespa.config.SlimeUtils; -import com.yahoo.vespa.curator.Lock; import com.yahoo.vespa.hosted.controller.Controller; -import com.yahoo.vespa.hosted.controller.LockedApplication; import com.yahoo.vespa.hosted.controller.api.integration.BuildService.BuildJob; import com.yahoo.vespa.hosted.controller.application.DeploymentJobs.JobError; import com.yahoo.vespa.hosted.controller.application.DeploymentJobs.JobReport; @@ -109,7 +107,7 @@ public class ScrewdriverApiHandler extends LoggingRequestHandler { .orElse(JobType.component); ApplicationId applicationId = ApplicationId.from(tenantName, applicationName, "default"); - controller.applications().lockedOrThrow(applicationId, application -> { + controller.applications().lockOrThrow(applicationId, application -> { // Since this is a manual operation we likely want it to trigger as soon as possible so we add it at to the // front of the queue application = controller.applications().deploymentTrigger().triggerAllowParallel( diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/restapi/impl/StatusPageResource.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/restapi/impl/StatusPageResource.java index f5852b9dfcf..67c69ddc887 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/restapi/impl/StatusPageResource.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/restapi/impl/StatusPageResource.java @@ -4,7 +4,7 @@ package com.yahoo.vespa.hosted.restapi.impl; import com.fasterxml.jackson.databind.JsonNode; import com.google.inject.Inject; import com.yahoo.container.jaxrs.annotation.Component; -import com.yahoo.vespa.hosted.controller.api.integration.security.KeyService; +import com.yahoo.jdisc.http.SecretStore; import javax.ws.rs.Path; import javax.ws.rs.Produces; @@ -24,20 +24,20 @@ import javax.ws.rs.core.UriBuilder; public class StatusPageResource implements com.yahoo.vespa.hosted.controller.api.statuspage.StatusPageResource { private final Client client; - private final KeyService keyService; + private final SecretStore secretStore; @Inject - public StatusPageResource(@Component KeyService keyService) { - this(keyService, ClientBuilder.newClient()); + public StatusPageResource(@Component SecretStore secretStore) { + this(secretStore, ClientBuilder.newClient()); } - protected StatusPageResource(KeyService keyService, Client client) { - this.keyService = keyService; + protected StatusPageResource(SecretStore secretStore, Client client) { + this.secretStore = secretStore; this.client = client; } protected UriBuilder statusPageURL(String page, String since) { - String[] secrets = keyService.getSecret("vespa_hosted.controller.statuspage_api_key").split(":"); + String[] secrets = secretStore.getSecret("vespa_hosted.controller.statuspage_api_key").split(":"); UriBuilder uriBuilder = UriBuilder.fromUri("https://" + secrets[0] + ".statuspage.io/api/v2/" + page + ".json?api_key=" + secrets[1]); if (since != null) { uriBuilder.queryParam("since", since); diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/ControllerTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/ControllerTest.java index b0fb820ca4c..9d4652941da 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/ControllerTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/ControllerTest.java @@ -521,7 +521,7 @@ public class ControllerTest { TenantId tenant = tester.createTenant("tenant1", "domain1", 11L); Application app = tester.createApplication(tenant, "app1", "default", 1); - tester.controller().applications().lockedOrThrow(app.id(), application -> { + tester.controller().applications().lockOrThrow(app.id(), application -> { application = application.withDeploying(Optional.of(new Change.VersionChange(Version.fromString("6.3")))); applications.store(application); try { 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 f202deafa8a..bdb8160a3f6 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 @@ -192,7 +192,7 @@ public final class ControllerTester { public Application createApplication(TenantId tenant, String applicationName, String instanceName, long projectId) { ApplicationId applicationId = applicationId(tenant.id(), applicationName, instanceName); controller().applications().createApplication(applicationId, Optional.of(TestIdentities.userNToken)); - controller().applications().lockedOrThrow(applicationId, lockedApplication -> + controller().applications().lockOrThrow(applicationId, lockedApplication -> controller().applications().store(lockedApplication.withProjectId(projectId))); return controller().applications().require(applicationId); } diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiTest.java index 7b05f0ce438..d12f78a1db3 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiTest.java @@ -326,7 +326,7 @@ public class ApplicationApiTest extends ControllerContainerTest { } private void addIssues(ContainerControllerTester tester, ApplicationId id) { - tester.controller().applications().lockedOrThrow(id, application -> + tester.controller().applications().lockOrThrow(id, application -> tester.controller().applications().store(application .withDeploymentIssueId(IssueId.from("123")) .withOwnershipIssueId(IssueId.from("321")))); @@ -485,7 +485,7 @@ public class ApplicationApiTest extends ControllerContainerTest { // Create the same application again tester.assertResponse(request("/application/v4/tenant/tenant1/application/application1", POST) .userIdentity(USER_ID), - "{\"error-code\":\"BAD_REQUEST\",\"message\":\"An application with id 'tenant1.application1' already exists\"}", + "{\"error-code\":\"BAD_REQUEST\",\"message\":\"Could not create 'tenant1.application1': Application already exists\"}", 400); ConfigServerClientMock configServer = (ConfigServerClientMock)container.components().getComponent("com.yahoo.vespa.hosted.controller.ConfigServerClientMock"); @@ -836,7 +836,7 @@ public class ApplicationApiTest extends ControllerContainerTest { */ private void setDeploymentMaintainedInfo(ContainerControllerTester controllerTester) { for (Application application : controllerTester.controller().applications().asList()) { - controllerTester.controller().applications().lockedOrThrow(application.id(), lockedApplication -> { + controllerTester.controller().applications().lockOrThrow(application.id(), lockedApplication -> { lockedApplication = lockedApplication.with(new ApplicationMetrics(0.5, 0.7)); for (Deployment deployment : application.deployments().values()) { diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/screwdriver/ScrewdriverApiTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/screwdriver/ScrewdriverApiTest.java index e6b3eacd44e..e2c3725ea5f 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/screwdriver/ScrewdriverApiTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/screwdriver/ScrewdriverApiTest.java @@ -9,7 +9,6 @@ import com.yahoo.config.provision.Environment; import com.yahoo.config.provision.RegionName; import com.yahoo.config.provision.Zone; import com.yahoo.vespa.config.SlimeUtils; -import com.yahoo.vespa.curator.Lock; import com.yahoo.vespa.hosted.controller.Application; import com.yahoo.vespa.hosted.controller.application.ApplicationPackage; import com.yahoo.vespa.hosted.controller.application.DeploymentJobs.JobError; @@ -148,7 +147,7 @@ public class ScrewdriverApiTest extends ControllerContainerTest { tester.containerTester().updateSystemVersion(); Application app = tester.createApplication(); - tester.controller().applications().lockedOrThrow(app.id(), application -> + tester.controller().applications().lockOrThrow(app.id(), application -> tester.controller().applications().store(application.withProjectId(1))); // Unknown application diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/restapi/impl/StatusPageResourceTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/restapi/impl/StatusPageResourceTest.java index 4e2e4bb15b4..2351b26f337 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/restapi/impl/StatusPageResourceTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/restapi/impl/StatusPageResourceTest.java @@ -3,7 +3,7 @@ package com.yahoo.vespa.hosted.restapi.impl; import com.fasterxml.jackson.databind.JsonNode; import com.fasterxml.jackson.databind.ObjectMapper; -import com.yahoo.vespa.hosted.controller.api.integration.security.KeyService; +import com.yahoo.jdisc.http.SecretStore; import org.junit.Before; import org.junit.Test; import org.mockito.Mockito; @@ -30,15 +30,15 @@ public class StatusPageResourceTest { Client mockClient = Mockito.mock(Client.class); WebTarget mockTarget = Mockito.mock(WebTarget.class); Invocation.Builder mockRequest = Mockito.mock(Invocation.Builder.class); - KeyService keyService = Mockito.mock(KeyService.class); + SecretStore secretStore = Mockito.mock(SecretStore.class); Mockito.when(mockClient.target(Mockito.any(UriBuilder.class))).thenReturn(mockTarget); Mockito.when(mockTarget.request()).thenReturn(mockRequest); Mockito.when(mockRequest.get(JsonNode.class)).thenReturn( new ObjectMapper().readTree("{\"page\":{\"name\":\"Vespa\"}}")); - Mockito.when(keyService.getSecret(Mockito.any(String.class))).thenReturn("testpage:testkey"); + Mockito.when(secretStore.getSecret(Mockito.any(String.class))).thenReturn("testpage:testkey"); - statusPage = new StatusPageResource(keyService, mockClient); + statusPage = new StatusPageResource(secretStore, mockClient); } diff --git a/searchcore/src/tests/proton/server/memory_flush_config_updater/memory_flush_config_updater_test.cpp b/searchcore/src/tests/proton/server/memory_flush_config_updater/memory_flush_config_updater_test.cpp index a76596aa927..b3a7f5f525e 100644 --- a/searchcore/src/tests/proton/server/memory_flush_config_updater/memory_flush_config_updater_test.cpp +++ b/searchcore/src/tests/proton/server/memory_flush_config_updater/memory_flush_config_updater_test.cpp @@ -56,9 +56,16 @@ struct Fixture EXPECT_EQUAL(expMaxEachMemory, strategy->getConfig().maxMemoryGain); EXPECT_EQUAL(expMaxGlobalTlsSize, strategy->getConfig().maxGlobalTlsSize); } + void assertStrategyDiskConfig(double expGlobalDiskBloatFactor, double expDiskBloatFactor) { + EXPECT_APPROX(expGlobalDiskBloatFactor, strategy->getConfig().globalDiskBloatFactor, 0.00001); + EXPECT_APPROX(expDiskBloatFactor, strategy->getConfig().diskBloatFactor, 0.00001); + } void notifyDiskMemUsage(const ResourceUsageState &diskState, const ResourceUsageState &memoryState) { updater.notifyDiskMemUsage(DiskMemUsageState(diskState, memoryState)); } + void setNodeRetired(bool nodeRetired) { + updater.setNodeRetired(nodeRetired); + } }; TEST_F("require that strategy is updated when setting new config", Fixture) @@ -133,4 +140,14 @@ TEST_F("require that we must go below low watermark for memory usage before usin TEST_DO(f.assertStrategyConfig(4, 1, 20)); } +TEST_F("require that more disk bloat is allowed while node state is retired", Fixture) +{ + f.notifyDiskMemUsage(ResourceUsageState(0.7, 0.3), belowLimit()); + TEST_DO(f.assertStrategyDiskConfig(0.2, 0.2)); + f.setNodeRetired(true); + TEST_DO(f.assertStrategyDiskConfig((0.8 - 0.3 / 0.7) * 0.8, 1.0)); + f.notifyDiskMemUsage(belowLimit(), belowLimit()); + TEST_DO(f.assertStrategyDiskConfig(0.2, 0.2)); +} + TEST_MAIN() { TEST_RUN_ALL(); } diff --git a/searchcore/src/vespa/searchcore/proton/matching/matcher.cpp b/searchcore/src/vespa/searchcore/proton/matching/matcher.cpp index 32775d7619a..2a3df5aeb52 100644 --- a/searchcore/src/vespa/searchcore/proton/matching/matcher.cpp +++ b/searchcore/src/vespa/searchcore/proton/matching/matcher.cpp @@ -268,9 +268,9 @@ Matcher::match(const SearchRequest &request, ResultProcessor::Result::UP result = master.match(params, limitedThreadBundle, *mtf, rp, _distributionKey, numSearchPartitions); my_stats = MatchMaster::getStats(std::move(master)); - size_t estimate = std::min(static_cast<size_t>(metaStore.getCommittedDocIdLimit()), - mtf->match_limiter().getDocIdSpaceEstimate()); + bool wasLimited = mtf->match_limiter().was_limited(); + size_t spaceEstimate = mtf->match_limiter().getDocIdSpaceEstimate(); uint32_t estHits = mtf->estimate().estHits; if (shouldCacheSearchSession && ((result->_numFs4Hits != 0) || shouldCacheGroupingSession)) { SearchSession::SP session = std::make_shared<SearchSession>(sessionId, request.getTimeOfDoom(), @@ -281,16 +281,32 @@ Matcher::match(const SearchRequest &request, reply = std::move(result->_reply); SearchReply::Coverage & coverage = reply->coverage; if (wasLimited) { + LOG(debug, "was limited, degraded from match phase"); coverage.degradeMatchPhase(); } if (my_stats.softDoomed()) { + LOG(debug, "soft doomed, degraded from timeout"); coverage.degradeTimeout(); } - coverage.setActive(metaStore.getNumActiveLids()); + uint32_t numActiveLids = metaStore.getNumActiveLids(); + // note: this is actually totalSpace+1, since 0 is reserved + uint32_t totalSpace = metaStore.getCommittedDocIdLimit(); + LOG(debug, "docid limit = %d", totalSpace); + LOG(debug, "num active lids = %d", numActiveLids); + LOG(debug, "space Estimate = %zd", spaceEstimate); + if (spaceEstimate >= totalSpace) { + // estimate is too high, clamp it + spaceEstimate = totalSpace; + } else { + // account for docid 0 reserved + spaceEstimate += 1; + } + size_t covered = (spaceEstimate * numActiveLids) / totalSpace; + LOG(debug, "covered = %zd", covered); + coverage.setActive(numActiveLids); //TODO this should be calculated with ClusterState calculator. - coverage.setSoonActive(metaStore.getNumActiveLids()); - coverage.setCovered(std::min(static_cast<size_t>(metaStore.getNumActiveLids()), - (estimate * metaStore.getNumActiveLids())/metaStore.getCommittedDocIdLimit())); + coverage.setSoonActive(numActiveLids); + coverage.setCovered(covered); LOG(debug, "numThreadsPerSearch = %zu. Configured = %d, estimated hits=%d, totalHits=%ld", numThreadsPerSearch, _rankSetup->getNumThreadsPerSearch(), estHits, reply->totalHitCount); } diff --git a/searchcore/src/vespa/searchcore/proton/server/memory_flush_config_updater.cpp b/searchcore/src/vespa/searchcore/proton/server/memory_flush_config_updater.cpp index a5b0df67b77..35d6973e34b 100644 --- a/searchcore/src/vespa/searchcore/proton/server/memory_flush_config_updater.cpp +++ b/searchcore/src/vespa/searchcore/proton/server/memory_flush_config_updater.cpp @@ -20,7 +20,7 @@ shouldUseConservativeMode(const ResourceUsageState &resourceState, } void -MemoryFlushConfigUpdater::considerUseConservativeDiskMode(const LockGuard &, +MemoryFlushConfigUpdater::considerUseConservativeDiskMode(const LockGuard &guard, MemoryFlush::Config &newConfig) { if (shouldUseConservativeMode(_currState.diskState(), _useConservativeDiskMode, @@ -30,6 +30,9 @@ MemoryFlushConfigUpdater::considerUseConservativeDiskMode(const LockGuard &, _useConservativeDiskMode = true; } else { _useConservativeDiskMode = false; + if (_nodeRetired) { + considerUseRelaxedDiskMode(guard, newConfig); + } } } @@ -49,6 +52,18 @@ MemoryFlushConfigUpdater::considerUseConservativeMemoryMode(const LockGuard &, } void +MemoryFlushConfigUpdater::considerUseRelaxedDiskMode(const LockGuard &, MemoryFlush::Config &newConfig) +{ + double utilization = _currState.diskState().utilization(); + double bloatMargin = _currConfig.conservative.lowwatermarkfactor - utilization; + if (bloatMargin > 0.0) { + // Node retired and disk utiliation is below low mater mark factor. + newConfig.diskBloatFactor = 1.0; + newConfig.globalDiskBloatFactor = std::max(bloatMargin * 0.8, _currConfig.diskbloatfactor); + } +} + +void MemoryFlushConfigUpdater::updateFlushStrategy(const LockGuard &guard) { MemoryFlush::Config newConfig = convertConfig(_currConfig, _memory); @@ -66,7 +81,8 @@ MemoryFlushConfigUpdater::MemoryFlushConfigUpdater(const MemoryFlush::SP &flushS _memory(memory), _currState(), _useConservativeDiskMode(false), - _useConservativeMemoryMode(false) + _useConservativeMemoryMode(false), + _nodeRetired(false) { } @@ -86,6 +102,14 @@ MemoryFlushConfigUpdater::notifyDiskMemUsage(DiskMemUsageState newState) updateFlushStrategy(guard); } +void +MemoryFlushConfigUpdater::setNodeRetired(bool nodeRetired) +{ + LockGuard guard(_mutex); + _nodeRetired = nodeRetired; + updateFlushStrategy(guard); +} + namespace { size_t diff --git a/searchcore/src/vespa/searchcore/proton/server/memory_flush_config_updater.h b/searchcore/src/vespa/searchcore/proton/server/memory_flush_config_updater.h index e6d8f7610f8..28ee330689d 100644 --- a/searchcore/src/vespa/searchcore/proton/server/memory_flush_config_updater.h +++ b/searchcore/src/vespa/searchcore/proton/server/memory_flush_config_updater.h @@ -28,12 +28,15 @@ private: DiskMemUsageState _currState; bool _useConservativeDiskMode; bool _useConservativeMemoryMode; + bool _nodeRetired; void considerUseConservativeDiskMode(const LockGuard &guard, MemoryFlush::Config &newConfig); void considerUseConservativeMemoryMode(const LockGuard &guard, MemoryFlush::Config &newConfig); + void considerUseRelaxedDiskMode(const LockGuard &guard, + MemoryFlush::Config &newConfig); void updateFlushStrategy(const LockGuard &guard); public: @@ -43,6 +46,7 @@ public: const ProtonConfig::Flush::Memory &config, const HwInfo::Memory &memory); void setConfig(const ProtonConfig::Flush::Memory &newConfig); + void setNodeRetired(bool nodeRetired); virtual void notifyDiskMemUsage(DiskMemUsageState newState) override; static MemoryFlush::Config convertConfig(const ProtonConfig::Flush::Memory &config, diff --git a/searchcore/src/vespa/searchcore/proton/server/proton.cpp b/searchcore/src/vespa/searchcore/proton/server/proton.cpp index ecdcf1c9c0f..b6b995ee69b 100644 --- a/searchcore/src/vespa/searchcore/proton/server/proton.cpp +++ b/searchcore/src/vespa/searchcore/proton/server/proton.cpp @@ -777,7 +777,11 @@ Proton::setClusterState(const storage::spi::ClusterState &calc) // about whether node is supposed to be up or not. Match engine // needs to know this in order to stop serving queries. bool nodeUp(calc.nodeUp()); + bool nodeRetired(calc.nodeRetired()); _matchEngine->setNodeUp(nodeUp); + if (_memoryFlushConfigUpdater) { + _memoryFlushConfigUpdater->setNodeRetired(nodeRetired); + } } namespace { |