Ignore stale job data in failure re-deployer

5 files changed, 374 insertions, 3 deletions

diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/FailureRedeployer.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/FailureRedeployer.java
index ab5ae6e00c2..38d4a4a8a81 100644
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/FailureRedeployer.java
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/FailureRedeployer.java
@@ -54,7 +54,14 @@ public class FailureRedeployer extends Maintainer {
                 continue;
             }
             Optional<Map.Entry<JobType, JobStatus>> job = oldestRunningJob(application);
-            if (job.isPresent() && job.get().getValue().lastTriggered().get().at().isBefore(maxAge)) {
+            if (!job.isPresent()) {
+                continue;
+            }
+            // Ignore job if it doesn't belong to a zone in this system
+            if (!job.get().getKey().zone(controller().system()).isPresent()) {
+                continue;
+            }
+            if (job.get().getValue().lastTriggered().get().at().isBefore(maxAge)) {
                 triggerFailing(application, "Job " + job.get().getKey().id() +
                         " has been running for more than " + jobTimeout);
             }
diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/ZoneRegistryMock.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/ZoneRegistryMock.java
index 62b935842f7..bf21467bc8d 100644
--- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/ZoneRegistryMock.java
+++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/ZoneRegistryMock.java
@@ -27,9 +27,11 @@ public class ZoneRegistryMock implements ZoneRegistry {
         deploymentTimeToLive.put(zone, duration);
     }
 
+    private SystemName system = SystemName.main;
+
     @Override
     public SystemName system() {
-        return SystemName.main;
+        return system;
     }
 
     @Override
@@ -71,4 +73,8 @@ public class ZoneRegistryMock implements ZoneRegistry {
     public URI getDashboardUri() {
         return URI.create("http://dashboard.test");
     }
+
+    public void setSystem(SystemName system) {
+        this.system = system;
+    }
 }
diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentTester.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentTester.java
index 32d1714ea52..0e816be864d 100644
--- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentTester.java
+++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentTester.java
@@ -160,7 +160,7 @@ public class DeploymentTester {
     }
 
     public void deploy(JobType job, Application application, ApplicationPackage applicationPackage, boolean deployCurrentVersion) {
-        job.zone(SystemName.main).ifPresent(zone -> tester.deploy(application, zone, applicationPackage, deployCurrentVersion));
+        job.zone(controller().system()).ifPresent(zone -> tester.deploy(application, zone, applicationPackage, deployCurrentVersion));
     }
 
     public void deployAndNotify(JobType job, Application application, ApplicationPackage applicationPackage, boolean success) {
diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/FailureRedeployerTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/FailureRedeployerTest.java
index cde511a9076..052ca87f791 100644
--- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/FailureRedeployerTest.java
+++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/FailureRedeployerTest.java
@@ -3,13 +3,20 @@ package com.yahoo.vespa.hosted.controller.maintenance;
 
 import com.yahoo.component.Version;
 import com.yahoo.config.provision.Environment;
+import com.yahoo.config.provision.SystemName;
+import com.yahoo.slime.Slime;
+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;
 import com.yahoo.vespa.hosted.controller.deployment.ApplicationPackageBuilder;
 import com.yahoo.vespa.hosted.controller.deployment.DeploymentTester;
+import com.yahoo.vespa.hosted.controller.persistence.ApplicationSerializer;
 import org.junit.Test;
 
+import java.nio.file.Files;
+import java.nio.file.Paths;
 import java.time.Duration;
 
 import static org.junit.Assert.assertEquals;
@@ -166,4 +173,60 @@ public class FailureRedeployerTest {
         assertTrue("No jobs retried", tester.buildSystem().jobs().isEmpty());
     }
 
+    @Test
+    public void retryIgnoresStaleJobData() throws Exception {
+        DeploymentTester tester = new DeploymentTester();
+        tester.controllerTester().getZoneRegistryMock().setSystem(SystemName.cd);
+
+        // Current system version, matches version in test data
+        Version version = Version.fromString("6.141.117");
+        tester.configServerClientMock().setDefaultConfigServerVersion(version);
+        tester.updateVersionStatus(version);
+        assertEquals(version, tester.controller().versionStatus().systemVersion().get().versionNumber());
+
+        // Load test data data
+        ApplicationSerializer serializer = new ApplicationSerializer();
+        byte[] json = Files.readAllBytes(Paths.get("src/test/java/com/yahoo/vespa/hosted/controller/maintenance/testdata/canary-with-stale-data.json"));
+        Slime slime = SlimeUtils.jsonToSlime(json);
+        Application application = serializer.fromSlime(slime);
+        try (Lock lock = tester.controller().applications().lock(application.id())) {
+            tester.controller().applications().store(application, lock);
+        }
+        ApplicationPackage applicationPackage = new ApplicationPackageBuilder()
+                .upgradePolicy("canary")
+                .region("cd-us-central-1")
+                .build();
+
+        // New version is released
+        version = Version.fromString("6.142.1");
+        tester.configServerClientMock().setDefaultConfigServerVersion(version);
+        tester.updateVersionStatus(version);
+        assertEquals(version, tester.controller().versionStatus().systemVersion().get().versionNumber());
+        tester.upgrader().maintain();
+
+        // Test environments pass
+        tester.deploy(DeploymentJobs.JobType.systemTest, application, applicationPackage);
+        tester.buildSystem().takeJobsToRun();
+        tester.clock().advance(Duration.ofMinutes(10));
+        tester.notifyJobCompletion(DeploymentJobs.JobType.systemTest, application, true);
+
+        tester.deploy(DeploymentJobs.JobType.stagingTest, application, applicationPackage);
+        tester.buildSystem().takeJobsToRun();
+        tester.clock().advance(Duration.ofMinutes(10));
+        tester.notifyJobCompletion(DeploymentJobs.JobType.stagingTest, application, true);
+
+        // Production job starts, but does not complete
+        assertEquals(1, tester.buildSystem().jobs().size());
+        assertEquals("Production job triggered", DeploymentJobs.JobType.productionCdUsCentral1.id(), tester.buildSystem().jobs().get(0).jobName());
+        tester.buildSystem().takeJobsToRun();
+
+        // Failure re-deployer runs
+        tester.failureRedeployer().maintain();
+        assertTrue("No jobs retried", tester.buildSystem().jobs().isEmpty());
+
+        // Deployment completes
+        tester.notifyJobCompletion(DeploymentJobs.JobType.productionCdUsCentral1, application, true);
+        assertFalse("Change deployed", tester.application(application.id()).deploying().isPresent());
+    }
+
 }
diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/testdata/canary-with-stale-data.json b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/testdata/canary-with-stale-data.json
new file mode 100644
index 00000000000..323889c7c45
--- /dev/null
+++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/testdata/canary-with-stale-data.json
@@ -0,0 +1,295 @@
+{
+  "id": "vespa:canary:default",
+  "deploymentSpecField": "<?xml version=\"1.0\" encoding=\"UTF-8\"?>\n<deployment version=\"1.0\">\n    <upgrade policy='canary'/>\n    <test />\n    <staging />\n    <prod>\n        <region active=\"true\">cd-us-central-1</region>\n    </prod>\n</deployment>\n",
+  "validationOverrides": "<validation-overrides>\n    <allow until=\"2017-04-27\">force-automatic-tenant-upgrade-test</allow>\n</validation-overrides>\n",
+  "deployments": [
+    {
+      "zone": {
+        "environment": "prod",
+        "region": "cd-us-central-1"
+      },
+      "version": "6.141.117",
+      "deployTime": 1503901783487,
+      "applicationPackageRevision": {
+        "applicationPackageHash": "72c3961314b96b1155a310f4785ae57ec74b1273",
+        "sourceRevision": {
+          "repositoryField": "git@git.test:vespa/canary-application.git",
+          "branchField": "origin/canary-cd",
+          "commitField": "566b7b30ee7886b845bb70958a0e1bdab2868633"
+        }
+      }
+    }
+  ],
+  "deploymentJobs": {
+    "projectId": 191186,
+    "jobStatus": [
+      {
+        "jobType": "production-eu-west-1",
+        "lastTriggered": {
+          "version": "6.98.12",
+          "at": 1493034019032
+        },
+        "lastCompleted": {
+          "version": "6.98.12",
+          "at": 1493033995026
+        },
+        "lastSuccess": {
+          "version": "6.98.12",
+          "at": 1493033995026
+        }
+      },
+      {
+        "jobType": "production-cd-us-central-1",
+        "jobError": "unknown",
+        "lastTriggered": {
+          "version": "6.141.117",
+          "revision": {
+            "applicationPackageHash": "72c3961314b96b1155a310f4785ae57ec74b1273",
+            "sourceRevision": {
+              "repositoryField": "git@git.test:vespa/canary-application.git",
+              "branchField": "origin/canary-cd",
+              "commitField": "566b7b30ee7886b845bb70958a0e1bdab2868633"
+            }
+          },
+          "at": 1503903384816
+        },
+        "lastCompleted": {
+          "version": "6.141.117",
+          "revision": {
+            "applicationPackageHash": "72c3961314b96b1155a310f4785ae57ec74b1273",
+            "sourceRevision": {
+              "repositoryField": "git@git.test:vespa/canary-application.git",
+              "branchField": "origin/canary-cd",
+              "commitField": "566b7b30ee7886b845bb70958a0e1bdab2868633"
+            }
+          },
+          "at": 1503903659364
+        },
+        "firstFailing": {
+          "version": "6.141.117",
+          "revision": {
+            "applicationPackageHash": "72c3961314b96b1155a310f4785ae57ec74b1273",
+            "sourceRevision": {
+              "repositoryField": "git@git.test:vespa/canary-application.git",
+              "branchField": "origin/canary-cd",
+              "commitField": "566b7b30ee7886b845bb70958a0e1bdab2868633"
+            }
+          },
+          "at": 1503903659364
+        },
+        "lastSuccess": {
+          "version": "6.141.117",
+          "revision": {
+            "applicationPackageHash": "72c3961314b96b1155a310f4785ae57ec74b1273",
+            "sourceRevision": {
+              "repositoryField": "git@git.test:vespa/canary-application.git",
+              "branchField": "origin/canary-cd",
+              "commitField": "566b7b30ee7886b845bb70958a0e1bdab2868633"
+            }
+          },
+          "at": 1503903556127
+        }
+      },
+      {
+        "jobType": "component",
+        "lastTriggered": {
+          "version": "6.141.109",
+          "at": 1503867517105
+        },
+        "lastCompleted": {
+          "version": "6.141.109",
+          "at": 1503867704464
+        },
+        "lastSuccess": {
+          "version": "6.141.109",
+          "at": 1503867704464
+        }
+      },
+      {
+        "jobType": "production-corp-us-east-1",
+        "lastTriggered": {
+          "version": "6.98.12",
+          "at": 1493034428590
+        },
+        "lastCompleted": {
+          "version": "6.98.12",
+          "at": 1493034114538
+        },
+        "lastSuccess": {
+          "version": "6.98.12",
+          "at": 1493034114538
+        }
+      },
+      {
+        "jobType": "production-ap-southeast-1",
+        "lastTriggered": {
+          "version": "6.98.12",
+          "at": 1493034265146
+        },
+        "lastCompleted": {
+          "version": "6.98.12",
+          "at": 1493034097617
+        },
+        "lastSuccess": {
+          "version": "6.98.12",
+          "at": 1493034097617
+        }
+      },
+      {
+        "jobType": "production-us-central-1",
+        "lastTriggered": {
+          "version": "6.98.12",
+          "at": 1493033800484
+        },
+        "lastCompleted": {
+          "version": "6.98.12",
+          "at": 1493034273753
+        },
+        "lastSuccess": {
+          "version": "6.98.12",
+          "at": 1493034273753
+        }
+      },
+      {
+        "jobType": "staging-test",
+        "lastTriggered": {
+          "version": "6.141.117",
+          "revision": {
+            "applicationPackageHash": "72c3961314b96b1155a310f4785ae57ec74b1273",
+            "sourceRevision": {
+              "repositoryField": "git@git.test:vespa/canary-application.git",
+              "branchField": "origin/canary-cd",
+              "commitField": "566b7b30ee7886b845bb70958a0e1bdab2868633"
+            }
+          },
+          "at": 1503900683154
+        },
+        "lastCompleted": {
+          "version": "6.141.117",
+          "revision": {
+            "applicationPackageHash": "72c3961314b96b1155a310f4785ae57ec74b1273",
+            "sourceRevision": {
+              "repositoryField": "git@git.test:vespa/canary-application.git",
+              "branchField": "origin/canary-cd",
+              "commitField": "566b7b30ee7886b845bb70958a0e1bdab2868633"
+            }
+          },
+          "at": 1503901635745
+        },
+        "lastSuccess": {
+          "version": "6.141.117",
+          "revision": {
+            "applicationPackageHash": "72c3961314b96b1155a310f4785ae57ec74b1273",
+            "sourceRevision": {
+              "repositoryField": "git@git.test:vespa/canary-application.git",
+              "branchField": "origin/canary-cd",
+              "commitField": "566b7b30ee7886b845bb70958a0e1bdab2868633"
+            }
+          },
+          "at": 1503901635745
+        }
+      },
+      {
+        "jobType": "system-test",
+        "lastTriggered": {
+          "version": "6.141.117",
+          "revision": {
+            "applicationPackageHash": "72c3961314b96b1155a310f4785ae57ec74b1273",
+            "sourceRevision": {
+              "repositoryField": "git@git.test:vespa/canary-application.git",
+              "branchField": "origin/canary-cd",
+              "commitField": "566b7b30ee7886b845bb70958a0e1bdab2868633"
+            }
+          },
+          "at": 1503899621243
+        },
+        "lastCompleted": {
+          "version": "6.141.117",
+          "revision": {
+            "applicationPackageHash": "72c3961314b96b1155a310f4785ae57ec74b1273",
+            "sourceRevision": {
+              "repositoryField": "git@git.test:vespa/canary-application.git",
+              "branchField": "origin/canary-cd",
+              "commitField": "566b7b30ee7886b845bb70958a0e1bdab2868633"
+            }
+          },
+          "at": 1503900025214
+        },
+        "lastSuccess": {
+          "version": "6.141.117",
+          "revision": {
+            "applicationPackageHash": "72c3961314b96b1155a310f4785ae57ec74b1273",
+            "sourceRevision": {
+              "repositoryField": "git@git.test:vespa/canary-application.git",
+              "branchField": "origin/canary-cd",
+              "commitField": "566b7b30ee7886b845bb70958a0e1bdab2868633"
+            }
+          },
+          "at": 1503900025214
+        }
+      },
+      {
+        "jobType": "production-us-west-1",
+        "lastTriggered": {
+          "version": "6.98.12",
+          "at": 1493034273768
+        },
+        "lastCompleted": {
+          "version": "6.98.12",
+          "at": 1493034019015
+        },
+        "lastSuccess": {
+          "version": "6.98.12",
+          "at": 1493034019015
+        }
+      },
+      {
+        "jobType": "production-ap-northeast-1",
+        "lastTriggered": {
+          "version": "6.98.12",
+          "at": 1493033995045
+        },
+        "lastCompleted": {
+          "version": "6.98.12",
+          "at": 1493034257206
+        },
+        "lastSuccess": {
+          "version": "6.98.12",
+          "at": 1493034257206
+        }
+      },
+      {
+        "jobType": "production-ap-northeast-2",
+        "lastTriggered": {
+          "version": "6.98.12",
+          "at": 1493034257222
+        },
+        "lastCompleted": {
+          "version": "6.98.12",
+          "at": 1493034265048
+        },
+        "lastSuccess": {
+          "version": "6.98.12",
+          "at": 1493034265048
+        }
+      },
+      {
+        "jobType": "production-us-east-3",
+        "lastTriggered": {
+          "version": "6.98.12",
+          "at": 1493034114555
+        },
+        "lastCompleted": {
+          "version": "6.98.12",
+          "at": 1493033800469
+        },
+        "lastSuccess": {
+          "version": "6.98.12",
+          "at": 1493033800469
+        }
+      }
+    ],
+    "selfTriggering": false
+  },
+  "outstandingChangeField": false
+}