diff options
5 files changed, 32 insertions, 25 deletions
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 c79b467568a..2c2dcfe549b 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 @@ -233,8 +233,11 @@ public class ApplicationController { * @throws IllegalArgumentException if the application already exists */ public Application createApplication(ApplicationId id, Optional<NToken> token) { - if ( ! (id.instance().value().equals("default") || id.instance().value().matches("^default-pr\\d+$"))) // TODO: Support instances properly - throw new UnsupportedOperationException("Only the instance names 'default' and names starting with 'default-pr' are supported at the moment"); + // TODO: TLS: PR names change to prXXX. + if ( ! ( id.instance().value().equals("default") + || id.instance().value().matches("^default-pr\\d+$") // TODO: Remove when these no longer deploy. + || id.instance().value().matches("^\\d+$"))) // TODO: Support instances properly + throw new UnsupportedOperationException("Only the instance names 'default' and names starting with 'default-pr' or which are just the PR number are supported at the moment"); try (Lock lock = lock(id)) { com.yahoo.vespa.hosted.controller.api.identifiers.ApplicationId.validate(id.application().value()); diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/ApplicationList.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/ApplicationList.java index 07d51b2b9c7..283d6a75178 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/ApplicationList.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/ApplicationList.java @@ -145,10 +145,10 @@ public class ApplicationList { /** * Returns the subset of applications which are not pull requests: - * Pull requests changes the application instance name to default-pr[pull-request-number] + * Pull requests changes the application instance name to (default-pr)?[pull-request-number] */ public ApplicationList notPullRequest() { - return listOf(list.stream().filter(a -> ! a.id().instance().value().startsWith("default-pr"))); + return listOf(list.stream().filter(a -> ! a.id().instance().value().matches("^(default-pr)?\\d+$"))); } /** Returns the subset of applications which have at least one production deployment */ 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 2b7260d5ffa..ad373cf8e29 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 @@ -10,6 +10,7 @@ import com.yahoo.vespa.hosted.controller.api.identifiers.TenantId; import com.yahoo.vespa.hosted.controller.api.integration.organization.OwnershipIssues; import com.yahoo.vespa.hosted.controller.api.integration.organization.IssueId; import com.yahoo.vespa.hosted.controller.api.integration.organization.User; +import com.yahoo.vespa.hosted.controller.application.ApplicationList; import java.time.Duration; import java.util.NoSuchElementException; @@ -40,21 +41,24 @@ public class ApplicationOwnershipConfirmer extends Maintainer { /** File an ownership issue with the owners of all applications we know about. */ private void confirmApplicationOwnerships() { - for (Application application : controller().applications().asList()) - if (application.id().instance().value().startsWith("default-pr") || application.productionDeployments().isEmpty()) - store(null, application.id()); - else - try { - Tenant tenant = ownerOf(application.id()); - Optional<IssueId> ourIssueId = application.ownershipIssueId(); - ourIssueId = tenant.tenantType() == TenantType.USER - ? ownershipIssues.confirmOwnership(ourIssueId, application.id(), userFor(tenant)) - : ownershipIssues.confirmOwnership(ourIssueId, application.id(), propertyIdFor(tenant)); - ourIssueId.ifPresent(issueId -> store(issueId, application.id())); - } - catch (RuntimeException e) { // Catch errors due to wrong data in the controller, or issues client timeout. - log.log(Level.WARNING, "Exception caught when attempting to file an issue for " + application.id(), e); - } + ApplicationList.from(controller().applications().asList()) + .notPullRequest() + .hasProductionDeployment() + .asList() + .forEach(application -> { + try { + Tenant tenant = ownerOf(application.id()); + Optional<IssueId> ourIssueId = application.ownershipIssueId(); + ourIssueId = tenant.tenantType() == TenantType.USER + ? ownershipIssues.confirmOwnership(ourIssueId, application.id(), userFor(tenant)) + : ownershipIssues.confirmOwnership(ourIssueId, application.id(), propertyIdFor(tenant)); + ourIssueId.ifPresent(issueId -> store(issueId, application.id())); + } + catch (RuntimeException e) { // Catch errors due to wrong data in the controller, or issues client timeout. + log.log(Level.WARNING, "Exception caught when attempting to file an issue for " + application.id(), e); + } + }); + } /** Escalate ownership issues which have not been closed before a defined amount of time has passed. */ diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/ApplicationOwnershipConfirmerTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/ApplicationOwnershipConfirmerTest.java index 5039777ec7d..b0954044c22 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/ApplicationOwnershipConfirmerTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/ApplicationOwnershipConfirmerTest.java @@ -78,8 +78,6 @@ public class ApplicationOwnershipConfirmerTest { confirmer.maintain(); confirmer.maintain(); - assertEquals("Confirmation issue has been forgotten for application without production deployments.", Optional.empty(), userApp.get().ownershipIssueId()); - // Time has passed, and a new confirmation issue is in order for the property which is still in production. Optional<IssueId> issueId2 = Optional.of(IssueId.from("2")); issues.response = issueId2; @@ -87,8 +85,7 @@ public class ApplicationOwnershipConfirmerTest { confirmer.maintain(); assertEquals("A new confirmation issue id is stored when something is returned to the maintainer.", issueId2, propertyApp.get().ownershipIssueId()); - assertEquals("Confirmation issue for application without production deployments has not been filed.", Optional.empty(), userApp.get().ownershipIssueId()); - + assertEquals("Confirmation issue for application without production deployments has not been filed.", issueId, userApp.get().ownershipIssueId()); } private class MockOwnershipIssues implements OwnershipIssues { diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/versions/VersionStatusTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/versions/VersionStatusTest.java index 1c1e2df2c94..7745cae1cb9 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/versions/VersionStatusTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/versions/VersionStatusTest.java @@ -139,10 +139,13 @@ public class VersionStatusTest { // Application without deployment Application ignored0 = tester.createApplication("ignored0", "tenant1", 1000, 1000L); - // Pull request build - Application ignored1 = tester.controllerTester().createApplication(new TenantId("tenant1"), + // Pull request builds + tester.controllerTester().createApplication(new TenantId("tenant1"), "ignored1", "default-pr42", 1000); + tester.controllerTester().createApplication(new TenantId("tenant1"), + "ignored1", + "43", 1000); assertEquals("All applications running on this version: High", Confidence.high, confidence(tester.controller(), version0)); |