diff options
author | Jon Bratseth <bratseth@gmail.com> | 2021-05-27 12:33:43 +0200 |
---|---|---|
committer | Jon Bratseth <bratseth@gmail.com> | 2021-05-27 12:33:43 +0200 |
commit | f229f961e22f1f2c623872164f6d8d9430e70481 (patch) | |
tree | 06a50b357443ff93bd96e33141241a9647c2fc35 /configserver | |
parent | 8d79f2813b5b9ab687219ce3b6cc73b0081b8300 (diff) |
Log model build errors to the deploy log
Diffstat (limited to 'configserver')
8 files changed, 48 insertions, 35 deletions
diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/ApplicationRepository.java b/configserver/src/main/java/com/yahoo/vespa/config/server/ApplicationRepository.java index d1cf011d33a..52bb8442982 100644 --- a/configserver/src/main/java/com/yahoo/vespa/config/server/ApplicationRepository.java +++ b/configserver/src/main/java/com/yahoo/vespa/config/server/ApplicationRepository.java @@ -838,15 +838,14 @@ public class ApplicationRepository implements com.yahoo.config.provision.Deploye public void validateThatSessionIsNotActive(Tenant tenant, long sessionId) { Session session = getRemoteSession(tenant, sessionId); - if (Session.Status.ACTIVATE.equals(session.getStatus())) { - throw new IllegalStateException("Session is active: " + sessionId); - } + if (Session.Status.ACTIVATE == session.getStatus()) + throw new IllegalArgumentException("Session is active: " + sessionId); } public void validateThatSessionIsPrepared(Tenant tenant, long sessionId) { Session session = getRemoteSession(tenant, sessionId); - if ( ! Session.Status.PREPARE.equals(session.getStatus())) - throw new IllegalStateException("Session not prepared: " + sessionId); + if ( Session.Status.PREPARE != session.getStatus()) + throw new IllegalArgumentException("Session not prepared: " + sessionId); } public long createSessionFromExisting(ApplicationId applicationId, boolean internalRedeploy, TimeoutBudget timeoutBudget) { diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/http/BadRequestException.java b/configserver/src/main/java/com/yahoo/vespa/config/server/http/BadRequestException.java index 50b494d2d56..ca2ab456564 100644 --- a/configserver/src/main/java/com/yahoo/vespa/config/server/http/BadRequestException.java +++ b/configserver/src/main/java/com/yahoo/vespa/config/server/http/BadRequestException.java @@ -5,11 +5,11 @@ package com.yahoo.vespa.config.server.http; * Exception that will create a http response with BAD_REQUEST response code (400) * * @author hmusum - * @since 5.1.17 */ -public class BadRequestException extends RuntimeException { +public class BadRequestException extends IllegalArgumentException { public BadRequestException(String message) { super(message); } + } diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/http/HttpHandler.java b/configserver/src/main/java/com/yahoo/vespa/config/server/http/HttpHandler.java index 78a7b8aaa8f..510fb3f4cc2 100644 --- a/configserver/src/main/java/com/yahoo/vespa/config/server/http/HttpHandler.java +++ b/configserver/src/main/java/com/yahoo/vespa/config/server/http/HttpHandler.java @@ -50,12 +50,12 @@ public class HttpHandler extends LoggingRequestHandler { return HttpErrorResponse.notFoundError(getMessage(e, request)); } catch (ActivationConflictException e) { return HttpErrorResponse.conflictWhenActivating(getMessage(e, request)); - } catch (BadRequestException | IllegalArgumentException | IllegalStateException e) { + } catch (InvalidApplicationException e) { + return HttpErrorResponse.invalidApplicationPackage(getMessage(e, request)); + } catch (IllegalArgumentException e) { return HttpErrorResponse.badRequest(getMessage(e, request)); } catch (OutOfCapacityException e) { return HttpErrorResponse.outOfCapacity(getMessage(e, request)); - } catch (InvalidApplicationException e) { - return HttpErrorResponse.invalidApplicationPackage(getMessage(e, request)); } catch (InternalServerException e) { return HttpErrorResponse.internalServerError(getMessage(e, request)); } catch (UnknownVespaVersionException e) { diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/http/InvalidApplicationException.java b/configserver/src/main/java/com/yahoo/vespa/config/server/http/InvalidApplicationException.java index ae7557588c8..9ff81bde130 100644 --- a/configserver/src/main/java/com/yahoo/vespa/config/server/http/InvalidApplicationException.java +++ b/configserver/src/main/java/com/yahoo/vespa/config/server/http/InvalidApplicationException.java @@ -4,7 +4,7 @@ package com.yahoo.vespa.config.server.http; /** * @author hmusum */ -public class InvalidApplicationException extends RuntimeException { +public class InvalidApplicationException extends IllegalArgumentException { public InvalidApplicationException(String message) { super(message); diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/modelfactory/ActivatedModelsBuilder.java b/configserver/src/main/java/com/yahoo/vespa/config/server/modelfactory/ActivatedModelsBuilder.java index f878831b8c5..4e7afa7b3db 100644 --- a/configserver/src/main/java/com/yahoo/vespa/config/server/modelfactory/ActivatedModelsBuilder.java +++ b/configserver/src/main/java/com/yahoo/vespa/config/server/modelfactory/ActivatedModelsBuilder.java @@ -5,6 +5,7 @@ import com.google.common.collect.ImmutableSet; import com.yahoo.cloud.config.ConfigserverConfig; import com.yahoo.component.Version; import com.yahoo.config.application.api.ApplicationPackage; +import com.yahoo.config.application.api.DeployLogger; import com.yahoo.config.model.api.ConfigDefinitionRepo; import com.yahoo.config.model.api.Model; import com.yahoo.config.model.api.ModelContext; @@ -80,7 +81,8 @@ public class ActivatedModelsBuilder extends ModelsBuilder<Application> { super(modelFactoryRegistry, configserverConfig, zone, - hostProvisionerProvider); + hostProvisionerProvider, + new SilentDeployLogger()); this.tenant = tenant; this.applicationGeneration = applicationGeneration; this.zkClient = zkClient; diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/modelfactory/ModelsBuilder.java b/configserver/src/main/java/com/yahoo/vespa/config/server/modelfactory/ModelsBuilder.java index 227e9c6a977..707a828bdd6 100644 --- a/configserver/src/main/java/com/yahoo/vespa/config/server/modelfactory/ModelsBuilder.java +++ b/configserver/src/main/java/com/yahoo/vespa/config/server/modelfactory/ModelsBuilder.java @@ -5,6 +5,7 @@ import com.google.common.util.concurrent.UncheckedTimeoutException; import com.yahoo.cloud.config.ConfigserverConfig; import com.yahoo.component.Version; import com.yahoo.config.application.api.ApplicationPackage; +import com.yahoo.config.application.api.DeployLogger; import com.yahoo.config.model.api.HostProvisioner; import com.yahoo.config.model.api.ModelFactory; import com.yahoo.config.model.api.Provisioned; @@ -18,10 +19,12 @@ import com.yahoo.config.provision.TransientException; import com.yahoo.config.provision.Zone; import com.yahoo.lang.SettableOptional; import com.yahoo.vespa.config.server.http.InternalServerException; +import com.yahoo.vespa.config.server.http.InvalidApplicationException; import com.yahoo.vespa.config.server.http.UnknownVespaVersionException; import com.yahoo.vespa.config.server.provision.HostProvisionerProvider; import com.yahoo.vespa.config.server.provision.ProvisionerAdapter; import com.yahoo.vespa.config.server.provision.StaticProvisioner; +import com.yahoo.yolean.Exceptions; import java.time.Duration; import java.time.Instant; @@ -56,18 +59,26 @@ public abstract class ModelsBuilder<MODELRESULT extends ModelResult> { private final HostProvisionerProvider hostProvisionerProvider; - ModelsBuilder(ModelFactoryRegistry modelFactoryRegistry, ConfigserverConfig configserverConfig, - Zone zone, HostProvisionerProvider hostProvisionerProvider) { + private final DeployLogger deployLogger; + + ModelsBuilder(ModelFactoryRegistry modelFactoryRegistry, + ConfigserverConfig configserverConfig, + Zone zone, + HostProvisionerProvider hostProvisionerProvider, + DeployLogger deployLogger) { this.modelFactoryRegistry = modelFactoryRegistry; this.configserverConfig = configserverConfig; this.hosted = configserverConfig.hostedVespa(); this.zone = zone; this.hostProvisionerProvider = hostProvisionerProvider; + this.deployLogger = deployLogger; } /** Returns the zone this is running in */ protected Zone zone() { return zone; } + protected DeployLogger deployLogger() { return deployLogger; } + /** * Builds all applicable model versions * @@ -122,23 +133,26 @@ public abstract class ModelsBuilder<MODELRESULT extends ModelResult> { catch (RuntimeException e) { if (shouldSkipCreatingMajorVersionOnError(majorVersions, majorVersion)) { log.log(Level.INFO, applicationId + ": Skipping major version " + majorVersion, e); - } else { - if (e instanceof NullPointerException || e instanceof NoSuchElementException | e instanceof UncheckedTimeoutException) { - log.log(Level.WARNING, "Unexpected error when building model ", e); - throw new InternalServerException(applicationId + ": Error loading model", e); - } else { - log.log(Level.WARNING, "Input error when building model ", e); - throw new IllegalArgumentException(applicationId + ": Error loading model", e); + } + else { + if (e instanceof IllegalArgumentException) { + var wrapped = new InvalidApplicationException("Error loading " + applicationId, e); + deployLogger.logApplicationPackage(Level.SEVERE, Exceptions.toMessageString(wrapped)); + throw wrapped; + } + else { + log.log(Level.WARNING, "Unexpected error building " + applicationId, e); + throw new InternalServerException("Unexpected error building " + applicationId, e); } } } } log.log(Level.FINE, () -> "Done building models for " + applicationId + ". Built models for versions " + - allApplicationModels.stream() - .map(result -> result.getModel().version()) - .map(Version::toFullString) - .collect(Collectors.toSet()) + - " in " + Duration.between(start, Instant.now())); + allApplicationModels.stream() + .map(result -> result.getModel().version()) + .map(Version::toFullString) + .collect(Collectors.toSet()) + + " in " + Duration.between(start, Instant.now())); return allApplicationModels; } diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/modelfactory/PreparedModelsBuilder.java b/configserver/src/main/java/com/yahoo/vespa/config/server/modelfactory/PreparedModelsBuilder.java index 7a0a261c7ad..86eee9e1623 100644 --- a/configserver/src/main/java/com/yahoo/vespa/config/server/modelfactory/PreparedModelsBuilder.java +++ b/configserver/src/main/java/com/yahoo/vespa/config/server/modelfactory/PreparedModelsBuilder.java @@ -52,7 +52,6 @@ public class PreparedModelsBuilder extends ModelsBuilder<PreparedModelsBuilder.P private final PermanentApplicationPackage permanentApplicationPackage; private final ConfigDefinitionRepo configDefinitionRepo; private final HostValidator<ApplicationId> hostValidator; - private final DeployLogger logger; private final PrepareParams params; private final FileDistributionProvider fileDistributionProvider; private final Optional<ApplicationSet> currentActiveApplicationSet; @@ -66,18 +65,17 @@ public class PreparedModelsBuilder extends ModelsBuilder<PreparedModelsBuilder.P HostProvisionerProvider hostProvisionerProvider, Curator curator, HostValidator<ApplicationId> hostValidator, - DeployLogger logger, + DeployLogger deployLogger, PrepareParams params, Optional<ApplicationSet> currentActiveApplicationSet, ModelContext.Properties properties, ConfigserverConfig configserverConfig) { - super(modelFactoryRegistry, configserverConfig, properties.zone(), hostProvisionerProvider); + super(modelFactoryRegistry, configserverConfig, properties.zone(), hostProvisionerProvider, deployLogger); this.permanentApplicationPackage = permanentApplicationPackage; this.configDefinitionRepo = configDefinitionRepo; this.fileDistributionProvider = fileDistributionProvider; this.hostValidator = hostValidator; this.curator = curator; - this.logger = logger; this.params = params; this.currentActiveApplicationSet = currentActiveApplicationSet; this.properties = properties; @@ -99,7 +97,7 @@ public class PreparedModelsBuilder extends ModelsBuilder<PreparedModelsBuilder.P applicationPackage, modelOf(modelVersion), permanentApplicationPackage.applicationPackage(), - logger, + deployLogger(), configDefinitionRepo, fileDistributionProvider.getFileRegistry(), new ApplicationCuratorDatabase(applicationId.tenant(), curator).readReindexingStatus(applicationId), diff --git a/configserver/src/test/java/com/yahoo/vespa/config/server/deploy/HostedDeployTest.java b/configserver/src/test/java/com/yahoo/vespa/config/server/deploy/HostedDeployTest.java index f1404b30f30..7f0a49e6d32 100644 --- a/configserver/src/test/java/com/yahoo/vespa/config/server/deploy/HostedDeployTest.java +++ b/configserver/src/test/java/com/yahoo/vespa/config/server/deploy/HostedDeployTest.java @@ -242,7 +242,7 @@ public class HostedDeployTest { * Tests that we create the minimal set of models and that version 7.x is created * if creating version 8.x fails (to support upgrades to new major version for applications * that are still using features that do not work on version 8.x) - **/ + */ @Test public void testCreateLatestMajorOnPreviousMajorIfItFailsOnMajorVersion8() throws IOException { deployWithModelForLatestMajorVersionFailing(8); @@ -252,7 +252,7 @@ public class HostedDeployTest { * Tests that we fail deployment for version 7.x if creating version 7.x fails (i.e. that we do not skip * building 7.x and only build version 6.x). Skipping creation of models for a major version is only supported * for major version >= 8 (see test above) or when major-version=6 is set in application package. - **/ + */ @Test(expected = InvalidApplicationException.class) public void testFailingToCreateModelVersion7FailsDeployment() throws IOException { deployWithModelForLatestMajorVersionFailing(7); @@ -286,7 +286,7 @@ public class HostedDeployTest { /** * Tests that we fail deployment if a needed model version fails to be created - **/ + */ @Test(expected = InvalidApplicationException.class) public void testDeploymentFailsIfNeededModelVersionFails() throws IOException { List<Host> hosts = createHosts(7, "7.0.0"); @@ -302,7 +302,7 @@ public class HostedDeployTest { * Test that deploying an application works when there are no allocated hosts in the system * (the bootstrap a new zone case, so deploying the routing app since that is the first deployment * that will be done) - **/ + */ @Test public void testCreateOnlyNeededModelVersionsWhenNoHostsAllocated() throws IOException { CountingModelFactory factory700 = createHostedModelFactory(Version.fromString("7.0.0")); |