From d6a4984279d8f0fb53cb09c5ac0ca53cd4e2f153 Mon Sep 17 00:00:00 2001 From: Morten Tokle Date: Fri, 18 Jun 2021 15:17:13 +0200 Subject: Add retrigger queue to avoid errors when job already running --- .../controller/deployment/DeploymentTrigger.java | 31 ++++++++++ .../controller/deployment/RetriggerEntry.java | 27 +++++++++ .../deployment/RetriggerEntrySerializer.java | 63 +++++++++++++++++++ .../maintenance/ControllerMaintenance.java | 4 ++ .../maintenance/RetriggerMaintainer.java | 66 ++++++++++++++++++++ .../hosted/controller/persistence/CuratorDb.java | 20 +++++++ .../restapi/application/ApplicationApiHandler.java | 1 + .../restapi/controller/ControllerApiHandler.java | 13 ++-- .../deployment/DeploymentTriggerTest.java | 14 +++++ .../maintenance/RetriggerMaintainerTest.java | 70 ++++++++++++++++++++++ .../restapi/application/ApplicationApiTest.java | 6 ++ .../restapi/controller/responses/maintenance.json | 3 + 12 files changed, 310 insertions(+), 8 deletions(-) create mode 100644 controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/RetriggerEntry.java create mode 100644 controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/RetriggerEntrySerializer.java create mode 100644 controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/RetriggerMaintainer.java create mode 100644 controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/RetriggerMaintainerTest.java (limited to 'controller-server') 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 53e88a9a5ac..4f4f4b59a2c 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,6 +5,7 @@ import com.yahoo.config.application.api.DeploymentInstanceSpec; import com.yahoo.config.application.api.DeploymentSpec; import com.yahoo.config.provision.ApplicationId; import com.yahoo.config.provision.InstanceName; +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; @@ -32,6 +33,7 @@ import java.util.OptionalLong; import java.util.function.Supplier; import java.util.logging.Level; import java.util.logging.Logger; +import java.util.stream.Collectors; import static java.util.Comparator.comparing; import static java.util.stream.Collectors.groupingBy; @@ -199,6 +201,35 @@ public class DeploymentTrigger { return List.copyOf(jobs.keySet()); } + /** retrigger job. If the job is already running, it will be canceled, and retrigger enqueued. */ + public Optional reTriggerOrAddToQueue(DeploymentId deployment) { + JobType jobType = JobType.from(controller.system(), deployment.zoneId()) + .orElseThrow(() -> new IllegalArgumentException(String.format("No job to trigger for (system/zone): %s/%s", controller.system().value(), deployment.zoneId().value()))); + Optional existingRun = controller.jobController().active(deployment.applicationId()).stream() + .filter(run -> run.id().type().equals(jobType)) + .findFirst(); + + if (existingRun.isPresent()) { + Run run = existingRun.get(); + try (Lock lock = controller.curator().lockDeploymentRetriggerQueue()) { + List retriggerEntries = controller.curator().readRetriggerEntries(); + List newList = new ArrayList<>(retriggerEntries); + RetriggerEntry requiredEntry = new RetriggerEntry(new JobId(deployment.applicationId(), jobType), run.id().number() + 1); + if(newList.stream().noneMatch(entry -> entry.jobId().equals(requiredEntry.jobId()) && entry.requiredRun()>=requiredEntry.requiredRun())) { + newList.add(requiredEntry); + } + newList = newList.stream() + .filter(entry -> !(entry.jobId().equals(requiredEntry.jobId()) && entry.requiredRun() < requiredEntry.requiredRun())) + .collect(toList()); + controller.curator().writeRetriggerEntries(newList); + } + controller.jobController().abort(run.id()); + return Optional.empty(); + } else { + return Optional.of(reTrigger(deployment.applicationId(), jobType)); + } + } + /** Prevents jobs of the given type from starting, until the given time. */ public void pauseJob(ApplicationId id, JobType jobType, Instant until) { if (until.isAfter(clock.instant().plus(maxPause))) diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/RetriggerEntry.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/RetriggerEntry.java new file mode 100644 index 00000000000..9c16d80313e --- /dev/null +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/RetriggerEntry.java @@ -0,0 +1,27 @@ +// Copyright Verizon Media. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. + +package com.yahoo.vespa.hosted.controller.deployment; + +import com.yahoo.vespa.hosted.controller.api.integration.deployment.JobId; + +/** + * @author mortent + */ +public class RetriggerEntry { + private final JobId jobId; + private final long requiredRun; + + public RetriggerEntry(JobId jobId, long requiredRun) { + this.jobId = jobId; + this.requiredRun = requiredRun; + } + + public JobId jobId() { + return jobId; + } + + public long requiredRun() { + return requiredRun; + } + +} diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/RetriggerEntrySerializer.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/RetriggerEntrySerializer.java new file mode 100644 index 00000000000..6d9206d42b6 --- /dev/null +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/RetriggerEntrySerializer.java @@ -0,0 +1,63 @@ +// Copyright Verizon Media. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. + +package com.yahoo.vespa.hosted.controller.deployment; + +import com.yahoo.config.provision.ApplicationId; +import com.yahoo.slime.Cursor; +import com.yahoo.slime.Inspector; +import com.yahoo.slime.Slime; +import com.yahoo.slime.SlimeUtils; +import com.yahoo.vespa.hosted.controller.api.integration.deployment.JobId; +import com.yahoo.vespa.hosted.controller.api.integration.deployment.JobType; + +import java.util.List; +import java.util.stream.Collectors; + +/** + * @author mortent + */ +public class RetriggerEntrySerializer { + + private static final String JOB_ID_KEY = "jobId"; + private static final String APPLICATION_ID_KEY = "applicationId"; + private static final String JOB_TYPE_KEY = "jobType"; + private static final String MIN_REQUIRED_RUN_ID_KEY = "minimumRunId"; + + public static List fromSlime(Slime slime) { + return SlimeUtils.entriesStream(slime.get().field("entries")) + .map(RetriggerEntrySerializer::deserializeEntry) + .collect(Collectors.toList()); + } + + public static Slime toSlime(List entryList) { + Slime slime = new Slime(); + Cursor root = slime.setObject(); + Cursor entries = root.setArray("entries"); + entryList.forEach(e -> RetriggerEntrySerializer.serializeEntry(entries, e)); + return slime; + } + + private static void serializeEntry(Cursor array, RetriggerEntry entry) { + Cursor root = array.addObject(); + Cursor jobid = root.setObject(JOB_ID_KEY); + jobid.setString(APPLICATION_ID_KEY, entry.jobId().application().serializedForm()); + jobid.setString(JOB_TYPE_KEY, entry.jobId().type().jobName()); + root.setLong(MIN_REQUIRED_RUN_ID_KEY, entry.requiredRun()); + } + + private static RetriggerEntry deserializeEntry(Inspector inspector) { + Inspector jobid = inspector.field(JOB_ID_KEY); + ApplicationId applicationId = ApplicationId.fromSerializedForm(require(jobid, APPLICATION_ID_KEY).asString()); + JobType jobType = JobType.fromJobName(require(jobid, JOB_TYPE_KEY).asString()); + long minRequiredRunId = require(inspector, MIN_REQUIRED_RUN_ID_KEY).asLong(); + return new RetriggerEntry(new JobId(applicationId, jobType), minRequiredRunId); + } + + private static Inspector require(Inspector inspector, String fieldName) { + Inspector field = inspector.field(fieldName); + if (!field.valid()) { + throw new IllegalStateException("Could not deserialize, field not found in json: " + fieldName); + } + return field; + } +} diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/ControllerMaintenance.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/ControllerMaintenance.java index 97c3c9f4091..91dfed500e3 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/ControllerMaintenance.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/ControllerMaintenance.java @@ -10,6 +10,7 @@ import com.yahoo.jdisc.Metric; import com.yahoo.vespa.hosted.controller.Controller; import java.time.Duration; +import java.time.temporal.ChronoUnit; import java.time.temporal.TemporalUnit; import java.util.Collections; import java.util.List; @@ -71,6 +72,7 @@ public class ControllerMaintenance extends AbstractComponent { maintainers.add(new ChangeRequestMaintainer(controller, intervals.changeRequestMaintainer)); maintainers.add(new VCMRMaintainer(controller, intervals.vcmrMaintainer)); maintainers.add(new CloudTrialExpirer(controller, intervals.defaultInterval)); + maintainers.add(new RetriggerMaintainer(controller, intervals.retriggerMaintainer)); } public Upgrader upgrader() { return upgrader; } @@ -126,6 +128,7 @@ public class ControllerMaintenance extends AbstractComponent { private final Duration tenantRoleMaintainer; private final Duration changeRequestMaintainer; private final Duration vcmrMaintainer; + private final Duration retriggerMaintainer; public Intervals(SystemName system) { this.system = Objects.requireNonNull(system); @@ -158,6 +161,7 @@ public class ControllerMaintenance extends AbstractComponent { this.tenantRoleMaintainer = duration(5, MINUTES); this.changeRequestMaintainer = duration(1, HOURS); this.vcmrMaintainer = duration(1, HOURS); + this.retriggerMaintainer = duration(1, MINUTES); } private Duration duration(long amount, TemporalUnit unit) { diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/RetriggerMaintainer.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/RetriggerMaintainer.java new file mode 100644 index 00000000000..b4ef7fa7ec0 --- /dev/null +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/RetriggerMaintainer.java @@ -0,0 +1,66 @@ +// Copyright Verizon Media. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. + +package com.yahoo.vespa.hosted.controller.maintenance; + +import com.yahoo.vespa.hosted.controller.Controller; +import com.yahoo.vespa.hosted.controller.api.integration.deployment.JobId; +import com.yahoo.vespa.hosted.controller.deployment.RetriggerEntry; +import com.yahoo.vespa.hosted.controller.deployment.Run; + +import java.time.Duration; +import java.util.List; +import java.util.Optional; +import java.util.logging.Level; +import java.util.logging.Logger; +import java.util.stream.Collectors; + +public class RetriggerMaintainer extends ControllerMaintainer { + + private static final Logger logger = Logger.getLogger(RetriggerMaintainer.class.getName()); + + public RetriggerMaintainer(Controller controller, Duration interval) { + super(controller, interval); + } + + @Override + protected double maintain() { + try (var lock = controller().curator().lockDeploymentRetriggerQueue()) { + List retriggerEntries = controller().curator().readRetriggerEntries(); + + // Trigger all jobs that still need triggering and is not running + retriggerEntries.stream() + .filter(this::needsTrigger) + .filter(entry -> readyToTrigger(entry.jobId())) + .forEach(entry -> controller().applications().deploymentTrigger().reTrigger(entry.jobId().application(), entry.jobId().type())); + + // Remove all jobs that has succeeded with the required job run and persist the list + List remaining = retriggerEntries.stream() + .filter(this::needsTrigger) + .collect(Collectors.toList()); + controller().curator().writeRetriggerEntries(remaining); + } catch (Exception e) { + logger.log(Level.WARNING, "Exception while triggering jobs", e); + return 0.0; + } + return 1.0; + } + + /* + Returns true if a job is ready to run, i.e is currently not running + */ + private boolean readyToTrigger(JobId jobId) { + Optional existingRun = controller().jobController().active(jobId.application()).stream() + .filter(run -> run.id().type().equals(jobId.type())) + .findFirst(); + return existingRun.isEmpty(); + } + + /* + Returns true of job needs triggering. I.e the job has not run since the queue item was last run. + */ + private boolean needsTrigger(RetriggerEntry entry) { + return controller().jobController().lastSuccess(entry.jobId()) + .filter(run -> run.id().number() < entry.requiredRun()) + .isPresent(); + } +} 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 a90f10401aa..08f1900c6e8 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 @@ -22,6 +22,8 @@ import com.yahoo.vespa.hosted.controller.api.integration.deployment.JobType; import com.yahoo.vespa.hosted.controller.api.integration.deployment.RunId; import com.yahoo.vespa.hosted.controller.application.TenantAndApplicationId; import com.yahoo.vespa.hosted.controller.auditlog.AuditLog; +import com.yahoo.vespa.hosted.controller.deployment.RetriggerEntry; +import com.yahoo.vespa.hosted.controller.deployment.RetriggerEntrySerializer; import com.yahoo.vespa.hosted.controller.deployment.Run; import com.yahoo.vespa.hosted.controller.deployment.Step; import com.yahoo.vespa.hosted.controller.dns.NameServiceQueue; @@ -221,6 +223,10 @@ public class CuratorDb { return curator.lock(lockRoot.append("supportAccess").append(deploymentId.dottedString()), defaultLockTimeout); } + public Lock lockDeploymentRetriggerQueue() { + return curator.lock(lockRoot.append("deploymentRetriggerQueue"), defaultLockTimeout); + } + // -------------- Helpers ------------------------------------------ /** Try locking with a low timeout, meaning it is OK to fail lock acquisition. @@ -635,6 +641,16 @@ public class CuratorDb { curator.set(supportAccessPath(deploymentId), asJson(SupportAccessSerializer.toSlime(supportAccess))); } + // -------------- Job Retrigger entries ----------------------------------- + + public List readRetriggerEntries() { + return readSlime(deploymentRetriggerPath()).map(RetriggerEntrySerializer::fromSlime).orElse(List.of()); + } + + public void writeRetriggerEntries(List retriggerEntries) { + curator.set(deploymentRetriggerPath(), asJson(RetriggerEntrySerializer.toSlime(retriggerEntries))); + } + // -------------- Paths --------------------------------------------------- private Path lockPath(TenantName tenant) { @@ -772,4 +788,8 @@ public class CuratorDb { return supportAccessRoot.append(deploymentId.dottedString()); } + private static Path deploymentRetriggerPath() { + return root.append("deploymentRetriggerQueue"); + } + } 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 017da94facc..f325e290e87 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 @@ -997,6 +997,7 @@ public class ApplicationApiHandler extends AuditLoggingRequestHandler { DeploymentId deployment = new DeploymentId(ApplicationId.from(tenantName, applicationName, instanceName), requireZone(environment, region)); Principal principal = requireUserPrincipal(request); SupportAccess disallowed = controller.supportAccess().disallow(deployment, principal.getName()); + controller.applications().deploymentTrigger().reTriggerOrAddToQueue(deployment); return new SlimeJsonResponse(SupportAccessSerializer.serializeCurrentState(disallowed, controller.clock().instant())); } diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/controller/ControllerApiHandler.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/controller/ControllerApiHandler.java index cba89fe39cf..59ae2a505bb 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/controller/ControllerApiHandler.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/controller/ControllerApiHandler.java @@ -3,7 +3,6 @@ package com.yahoo.vespa.hosted.controller.restapi.controller; import com.yahoo.component.Version; import com.yahoo.config.provision.ApplicationId; -import com.yahoo.config.provision.Zone; import com.yahoo.config.provision.zone.ZoneId; import com.yahoo.container.jdisc.HttpRequest; import com.yahoo.container.jdisc.HttpResponse; @@ -19,7 +18,7 @@ import com.yahoo.slime.SlimeUtils; import com.yahoo.vespa.athenz.api.AthenzUser; import com.yahoo.vespa.hosted.controller.Controller; import com.yahoo.vespa.hosted.controller.api.identifiers.DeploymentId; -import com.yahoo.vespa.hosted.controller.api.integration.deployment.JobType; +import com.yahoo.vespa.hosted.controller.api.integration.deployment.JobId; import com.yahoo.vespa.hosted.controller.auditlog.AuditLoggingRequestHandler; import com.yahoo.vespa.hosted.controller.maintenance.ControllerMaintenance; import com.yahoo.vespa.hosted.controller.maintenance.Upgrader; @@ -124,12 +123,10 @@ public class ControllerApiHandler extends AuditLoggingRequestHandler { SupportAccess supportAccess = controller.supportAccess().registerGrant(deployment, principal.getName(), certificate); // Trigger deployment to include operator cert - JobType jobType = JobType.from(controller.system(), deployment.zoneId()) - .orElseThrow(() -> new IllegalStateException("No job found to trigger for " + deployment.toUserFriendlyString())); - - String jobName = controller.applications().deploymentTrigger() - .reTrigger(deployment.applicationId(), jobType).type().jobName(); - return new MessageResponse(String.format("Operator %s granted access and job %s triggered", principal.getName(), jobName)); + Optional jobId = controller.applications().deploymentTrigger().reTriggerOrAddToQueue(deployment); + return new MessageResponse( + jobId.map(id -> String.format("Operator %s granted access and job %s triggered", principal.getName(), id.type().jobName())) + .orElseGet(() -> String.format("Operator %s granted access and job trigger queued", principal.getName()))); } private T requireField(Inspector inspector, String field, Function mapper) { diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentTriggerTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentTriggerTest.java index c8b4eaa5236..7077e14a648 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentTriggerTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentTriggerTest.java @@ -11,6 +11,7 @@ import com.yahoo.vespa.hosted.controller.api.integration.deployment.RunId; import com.yahoo.vespa.hosted.controller.application.ApplicationPackage; import com.yahoo.vespa.hosted.controller.application.Change; import com.yahoo.vespa.hosted.controller.versions.VespaVersion; +import org.junit.Assert; import org.junit.Test; import java.time.Duration; @@ -1228,4 +1229,17 @@ public class DeploymentTriggerTest { assertEquals(List.of(), tester.jobs().active()); } + @Test + public void testRetriggerQueue() { + var app = tester.newDeploymentContext().submit().deploy(); + app.submit(); + tester.triggerJobs(); + + tester.deploymentTrigger().reTrigger(app.instanceId(), productionUsEast3); + tester.deploymentTrigger().reTriggerOrAddToQueue(app.deploymentIdIn(ZoneId.from("prod", "us-east-3"))); + tester.deploymentTrigger().reTriggerOrAddToQueue(app.deploymentIdIn(ZoneId.from("prod", "us-east-3"))); + + List retriggerEntries = tester.controller().curator().readRetriggerEntries(); + Assert.assertEquals(1, retriggerEntries.size()); + } } diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/RetriggerMaintainerTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/RetriggerMaintainerTest.java new file mode 100644 index 00000000000..df93efab893 --- /dev/null +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/RetriggerMaintainerTest.java @@ -0,0 +1,70 @@ +// Copyright Verizon Media. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. + +package com.yahoo.vespa.hosted.controller.maintenance; + +import com.yahoo.config.provision.ApplicationId; +import com.yahoo.config.provision.zone.ZoneId; +import com.yahoo.vespa.hosted.controller.api.integration.deployment.JobType; +import com.yahoo.vespa.hosted.controller.application.ApplicationPackage; +import com.yahoo.vespa.hosted.controller.deployment.ApplicationPackageBuilder; +import com.yahoo.vespa.hosted.controller.deployment.DeploymentTester; +import com.yahoo.vespa.hosted.controller.deployment.RetriggerEntry; +import org.junit.Test; + +import java.io.IOException; +import java.time.Duration; +import java.util.List; + +import static org.junit.Assert.assertEquals; + +/** + * @author mortent + */ +public class RetriggerMaintainerTest { + + private final DeploymentTester tester = new DeploymentTester(); + + @Test + public void processes_queue() throws IOException { + RetriggerMaintainer maintainer = new RetriggerMaintainer(tester.controller(), Duration.ofDays(1)); + ApplicationId applicationId = ApplicationId.from("tenant", "app", "default"); + var devApp = tester.newDeploymentContext(applicationId); + ApplicationPackage appPackage = new ApplicationPackageBuilder() + .region("us-west-1") + .build(); + + // Deploy app + devApp.runJob(JobType.devUsEast1, appPackage); + devApp.completeRollout(); + + // Trigger a run (to simulate a running job) + tester.deploymentTrigger().reTrigger(applicationId, JobType.devUsEast1); + + // Add a job to the queue + tester.deploymentTrigger().reTriggerOrAddToQueue(devApp.deploymentIdIn(ZoneId.from("dev", "us-east-1"))); + + // Should be 1 entry in the queue: + List retriggerEntries = tester.controller().curator().readRetriggerEntries(); + assertEquals(1, retriggerEntries.size()); + + // Adding to queue triggers abort + devApp.jobAborted(JobType.devUsEast1); + assertEquals(0, tester.jobs().active(applicationId).size()); + + // The maintainer runs and will actually trigger dev us-east, but keeps the entry in queue to verify it was actually run + maintainer.maintain(); + retriggerEntries = tester.controller().curator().readRetriggerEntries(); + assertEquals(1, retriggerEntries.size()); + assertEquals(1, tester.jobs().active(applicationId).size()); + + // Run outstanding jobs + devApp.runJob(JobType.devUsEast1); + assertEquals(0, tester.jobs().active(applicationId).size()); + + // Run maintainer again, should find that the job has already run successfully and will remove the entry. + maintainer.maintain(); + retriggerEntries = tester.controller().curator().readRetriggerEntries(); + assertEquals(0, retriggerEntries.size()); + assertEquals(0, tester.jobs().active(applicationId).size()); + } +} 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 f4b8a643e28..01d39c0ea1c 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 @@ -1554,6 +1554,9 @@ public class ApplicationApiTest extends ControllerContainerTest { List activeGrants = tester.controller().supportAccess().activeGrantsFor(new DeploymentId(ApplicationId.fromSerializedForm("tenant1:application1:instance1"), zone)); assertEquals(1, activeGrants.size()); + // Adding grant should trigger job + app.assertRunning(JobType.productionUsWest1); + // DELETE removes access String disallowedResponse = grantResponse .replaceAll("ALLOWED\".*?}", "NOT_ALLOWED\"}") @@ -1563,6 +1566,9 @@ public class ApplicationApiTest extends ControllerContainerTest { disallowedResponse, 200 ); + // Revoking access should trigger job + app.assertRunning(JobType.productionUsWest1); + // Should be no available grant activeGrants = tester.controller().supportAccess().activeGrantsFor(new DeploymentId(ApplicationId.fromSerializedForm("tenant1:application1:instance1"), zone)); assertEquals(0, activeGrants.size()); diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/controller/responses/maintenance.json b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/controller/responses/maintenance.json index 914ea2f5518..e906df94023 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/controller/responses/maintenance.json +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/controller/responses/maintenance.json @@ -78,6 +78,9 @@ { "name": "ResourceTagMaintainer" }, + { + "name":"RetriggerMaintainer" + }, { "name": "SystemRoutingPolicyMaintainer" }, -- cgit v1.2.3 From a03ec25df4bf1202ec82680cfdd344a03478dd90 Mon Sep 17 00:00:00 2001 From: Morten Tokle Date: Mon, 21 Jun 2021 09:42:20 +0200 Subject: Skip requirement on 'successful' run --- .../yahoo/vespa/hosted/controller/maintenance/RetriggerMaintainer.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'controller-server') diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/RetriggerMaintainer.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/RetriggerMaintainer.java index b4ef7fa7ec0..2cc3ac1bd6c 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/RetriggerMaintainer.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/RetriggerMaintainer.java @@ -59,7 +59,7 @@ public class RetriggerMaintainer extends ControllerMaintainer { Returns true of job needs triggering. I.e the job has not run since the queue item was last run. */ private boolean needsTrigger(RetriggerEntry entry) { - return controller().jobController().lastSuccess(entry.jobId()) + return controller().jobController().lastCompleted(entry.jobId()) .filter(run -> run.id().number() < entry.requiredRun()) .isPresent(); } -- cgit v1.2.3