Merge pull request #11099 from vespa-engine/jvenstad/convert-more-deployment-tests

Various fixes in InternalDeploymentTester, and convert DeploymentIssu…

2 files changed, 65 insertions, 40 deletions

diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/InternalDeploymentTester.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/InternalDeploymentTester.java
index 7b1fff7fb6c..b2e2288dc51 100644
--- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/InternalDeploymentTester.java
+++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/InternalDeploymentTester.java
@@ -262,7 +262,7 @@ public class InternalDeploymentTester {
                       .filter(r -> r.id().type() == job.type())
                       .orElseThrow(() -> new AssertionError(job.type() + " is not among the active: " + jobs.active()));
         assertFalse(run.hasFailed());
-        assertNotEquals(aborted, run.status());
+        assertFalse(run.hasEnded());
         return run;
     }
 
@@ -287,7 +287,8 @@ public class InternalDeploymentTester {
 
         if (job.type() == JobType.stagingTest) { // Do the initial deployment and installation of the real application.
             assertEquals(unfinished, jobs.run(id).get().steps().get(Step.installInitialReal));
-            currentRun(job).versions().sourcePlatform().ifPresent(version -> tester.configServer().nodeRepository().doUpgrade(deployment, Optional.empty(), version));
+            Versions versions = currentRun(job).versions();
+            tester.configServer().nodeRepository().doUpgrade(deployment, Optional.empty(), versions.sourcePlatform().orElse(versions.targetPlatform()));
             tester.configServer().convergeServices(id.application(), zone);
             setEndpoints(id.application(), zone);
             runner.advance(currentRun(job));
@@ -430,7 +431,7 @@ public class InternalDeploymentTester {
 
     /** Pulls the ready job trigger, and then runs the whole of the given job, successfully. */
     public void runJob(JobId job) {
-        tester.readyJobTrigger().run();
+        triggerJobs();
         doDeploy(job);
         doUpgrade(job);
         doConverge(job);
@@ -442,6 +443,22 @@ public class InternalDeploymentTester {
         doTeardown(job);
     }
 
+    public void jobAborted(JobType type) {
+        jobAborted(new JobId(instanceId, type));
+    }
+
+    public void jobAborted(ApplicationId instanceId, JobType type) {
+        jobAborted(new JobId(instanceId, type));
+    }
+
+    public void jobAborted(JobId job) {
+        triggerJobs();
+        RunId id = currentRun(job).id();
+        runner.advance(currentRun(job));
+        assertEquals(aborted, jobs.run(id).get().status());
+        assertTrue(jobs.run(id).get().hasEnded());
+    }
+
     public void failDeployment(JobType type) {
         failDeployment(new JobId(instanceId, type));
     }
@@ -451,8 +468,8 @@ public class InternalDeploymentTester {
     }
 
     public void failDeployment(JobId job) {
+        triggerJobs();
         RunId id = currentRun(job).id();
-        tester.readyJobTrigger().run();
         tester.configServer().throwOnNextPrepare(new IllegalArgumentException("Exception"));
         runner.advance(currentRun(job));
         assertTrue(jobs.run(id).get().hasFailed());
@@ -469,8 +486,8 @@ public class InternalDeploymentTester {
     }
 
     public void timeOutUpgrade(JobId job) {
+        triggerJobs();
         RunId id = currentRun(job).id();
-        tester.readyJobTrigger().run();
         doDeploy(job);
         clock().advance(InternalStepRunner.installationTimeout.plusSeconds(1));
         runner.advance(currentRun(job));
@@ -488,8 +505,8 @@ public class InternalDeploymentTester {
     }
 
     public void timeOutConvergence(JobId job) {
+        triggerJobs();
         RunId id = currentRun(job).id();
-        tester.readyJobTrigger().run();
         doDeploy(job);
         doUpgrade(job);
         clock().advance(InternalStepRunner.installationTimeout.plusSeconds(1));
diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/DeploymentIssueReporterTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/DeploymentIssueReporterTest.java
index 3c1fb2fffd0..9df0e99787d 100644
--- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/DeploymentIssueReporterTest.java
+++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/DeploymentIssueReporterTest.java
@@ -6,13 +6,16 @@ import com.yahoo.config.provision.ApplicationId;
 import com.yahoo.config.provision.Environment;
 import com.yahoo.vespa.hosted.controller.Application;
 import com.yahoo.vespa.hosted.controller.Instance;
+import com.yahoo.vespa.hosted.controller.LockedTenant;
 import com.yahoo.vespa.hosted.controller.api.integration.organization.Contact;
 import com.yahoo.vespa.hosted.controller.api.integration.organization.IssueId;
 import com.yahoo.vespa.hosted.controller.api.integration.stubs.LoggingDeploymentIssues;
 import com.yahoo.vespa.hosted.controller.application.ApplicationPackage;
+import com.yahoo.vespa.hosted.controller.application.Change;
 import com.yahoo.vespa.hosted.controller.application.TenantAndApplicationId;
 import com.yahoo.vespa.hosted.controller.deployment.ApplicationPackageBuilder;
 import com.yahoo.vespa.hosted.controller.deployment.DeploymentTester;
+import com.yahoo.vespa.hosted.controller.deployment.InternalDeploymentTester;
 import com.yahoo.vespa.hosted.controller.persistence.MockCuratorDb;
 import com.yahoo.vespa.hosted.controller.versions.VespaVersion;
 import org.junit.Before;
@@ -23,6 +26,7 @@ import java.util.HashMap;
 import java.util.Map;
 import java.util.Optional;
 
+import static com.yahoo.config.application.api.DeploymentSpec.UpgradePolicy.canary;
 import static com.yahoo.vespa.hosted.controller.api.integration.deployment.JobType.component;
 import static com.yahoo.vespa.hosted.controller.api.integration.deployment.JobType.productionUsWest1;
 import static com.yahoo.vespa.hosted.controller.api.integration.deployment.JobType.stagingTest;
@@ -49,55 +53,55 @@ public class DeploymentIssueReporterTest {
             .upgradePolicy("canary")
             .build();
 
-
-    private DeploymentTester tester;
+    private InternalDeploymentTester tester;
     private DeploymentIssueReporter reporter;
     private MockDeploymentIssues issues;
 
     @Before
     public void setup() {
-        tester = new DeploymentTester();
+        tester = new InternalDeploymentTester();
         issues = new MockDeploymentIssues();
         reporter = new DeploymentIssueReporter(tester.controller(), issues, Duration.ofDays(1), new JobControl(new MockCuratorDb()));
     }
 
     @Test
     public void testDeploymentFailureReporting() {
-        // All applications deploy from unique build projects.
-        Long projectId1 = 10L;
-        Long projectId2 = 20L;
-        Long projectId3 = 30L;
-
-        Long propertyId1 = 1L;
-        Long propertyId2 = 2L;
-        Long propertyId3 = 3L;
-
-        tester.upgradeSystem(Version.fromString("6.2"));
+        tester.controllerTester().upgradeSystem(Version.fromString("6.2"));
 
-        Optional<Contact> contact = Optional.of(tester.controllerTester().serviceRegistry().contactRetrieverMock().contact());
+        /*
         tester.controllerTester().createTenant("tenant1", "domain1", 1L, contact);
         tester.controllerTester().createTenant("tenant2", "domain2", 1L, contact);
         tester.controllerTester().createTenant("tenant3", "domain3", 1L, contact);
+        */
 
         // Create and deploy one application for each of three tenants.
-        Application app1 = tester.createApplication("application1", "tenant1", projectId1, propertyId1);
-        Application app2 = tester.createApplication("application2", "tenant2", projectId2, propertyId2);
-        Application app3 = tester.createApplication("application3", "tenant3", projectId3, propertyId3);
+        Application app1 = tester.createApplication("application1", "tenant1", "default");
+        Application app2 = tester.createApplication("application2", "tenant2", "default");
+        Application app3 = tester.createApplication("application3", "tenant3", "default");
+
+        Contact contact = tester.controllerTester().serviceRegistry().contactRetrieverMock().contact();
+        tester.controller().tenants().lockOrThrow(app1.id().tenant(), LockedTenant.Athenz.class, tenant ->
+                tester.controller().tenants().store(tenant.with(contact)));
+        tester.controller().tenants().lockOrThrow(app2.id().tenant(), LockedTenant.Athenz.class, tenant ->
+                tester.controller().tenants().store(tenant.with(contact)));
+        tester.controller().tenants().lockOrThrow(app3.id().tenant(), LockedTenant.Athenz.class, tenant ->
+                tester.controller().tenants().store(tenant.with(contact)));
 
         // NOTE: All maintenance should be idempotent within a small enough time interval, so maintain is called twice in succession throughout.
 
-        // apps 1 and 3 have one failure each.
-        tester.jobCompletion(component).application(app1).uploadArtifact(applicationPackage).submit();
-        tester.deployAndNotify(app1.id().defaultInstance(), applicationPackage, true, systemTest);
-        tester.deployAndNotify(app1.id().defaultInstance(), applicationPackage, false, stagingTest);
+        // app 1 fails staging tests.
+        tester.newSubmission(app1.id(), applicationPackage);
+        tester.runJob(app1.id().defaultInstance(), systemTest);
+        tester.timeOutConvergence(app1.id().defaultInstance(), stagingTest);
 
         // app2 is successful, but will fail later.
-        tester.deployCompletely(app2, canaryPackage);
+        tester.deployNewSubmission(app2.id(), tester.newSubmission(app2.id(), applicationPackage));
 
-        tester.jobCompletion(component).application(app3).uploadArtifact(applicationPackage).submit();
-        tester.deployAndNotify(app3.id().defaultInstance(), applicationPackage, true, systemTest);
-        tester.deployAndNotify(app3.id().defaultInstance(), applicationPackage, true, stagingTest);
-        tester.deployAndNotify(app3.id().defaultInstance(), applicationPackage, false, productionUsWest1);
+        // app 3 fails a production job.
+        tester.newSubmission(app3.id(), applicationPackage);
+        tester.runJob(app3.id().defaultInstance(), systemTest);
+        tester.runJob(app3.id().defaultInstance(), stagingTest);
+        tester.failDeployment(app3.id().defaultInstance(), productionUsWest1);
 
         reporter.maintain();
         reporter.maintain();
@@ -132,7 +136,7 @@ public class DeploymentIssueReporterTest {
 
 
         // app3 fixes their problems, but the ticket for app3 is left open; see the resolved ticket is not escalated when another escalation period has passed.
-        tester.deployAndNotify(app3.id().defaultInstance(), applicationPackage, true, productionUsWest1);
+        tester.runJob(app3.id().defaultInstance(), productionUsWest1);
         tester.clock().advance(maxInactivity.plus(Duration.ofDays(1)));
 
         reporter.maintain();
@@ -143,8 +147,8 @@ public class DeploymentIssueReporterTest {
 
 
         // app3 now has a new failure past max failure age; see that a new issue is filed.
-        tester.jobCompletion(component).application(app3).nextBuildNumber().uploadArtifact(applicationPackage).submit();
-        tester.deployAndNotify(app3.id().defaultInstance(), applicationPackage, false, systemTest);
+        tester.newSubmission(app3.id(), applicationPackage);
+        tester.failDeployment(app3.id().defaultInstance(), systemTest);
         tester.clock().advance(maxInactivity.plus(maxFailureAge));
 
         reporter.maintain();
@@ -152,14 +156,19 @@ public class DeploymentIssueReporterTest {
         assertTrue("A new issue is filed for app3.", issues.isOpenFor(app3.id()));
 
 
+        // App2 is changed to be a canary
+        tester.deployNewSubmission(app2.id(), tester.newSubmission(app2.id(), canaryPackage));
+        assertEquals(canary, tester.applications().requireApplication(app2.id()).deploymentSpec().requireInstance("default").upgradePolicy());
+        assertEquals(Change.empty(), tester.applications().requireApplication(app2.id()).change());
+
         // Bump system version to upgrade canary app2.
         Version version = Version.fromString("6.3");
-        tester.upgradeSystem(version);
+        tester.controllerTester().upgradeSystem(version);
+        tester.upgrader().maintain();
         assertEquals(version, tester.controller().versionStatus().systemVersion().get().versionNumber());
 
-        tester.readyJobTrigger().maintain();
-        tester.completeUpgradeWithError(app2, version, canaryPackage, systemTest);
-        tester.upgradeSystem(version);
+        tester.timeOutUpgrade(app2.id().defaultInstance(), systemTest);
+        tester.controllerTester().upgradeSystem(version);
         assertEquals(VespaVersion.Confidence.broken, tester.controller().versionStatus().systemVersion().get().confidence());
 
         assertFalse("We have no platform issues initially.", issues.platformIssue());
@@ -171,7 +180,6 @@ public class DeploymentIssueReporterTest {
         reporter.maintain();
         assertTrue("We get a platform issue when confidence is broken", issues.platformIssue());
         assertFalse("No deployment issue is filed for app2, which has a version upgrade failure.", issues.isOpenFor(app2.id()));
-
     }