From ee847c3bc8e04ffdc67f9ceb748429162d424d4d Mon Sep 17 00:00:00 2001 From: Ola Aunrønning Date: Mon, 19 Apr 2021 13:53:09 +0200 Subject: Delete old and closed change requests. Get status of all persisted change requests --- .../api/integration/vcmr/ChangeRequestClient.java | 3 +- .../integration/vcmr/MockChangeRequestClient.java | 2 +- .../maintenance/ChangeRequestMaintainer.java | 29 +++++++++++++++++- .../hosted/controller/persistence/CuratorDb.java | 4 +++ .../maintenance/ChangeRequestMaintainerTest.java | 35 ++++++++++++++++++---- 5 files changed, 65 insertions(+), 8 deletions(-) diff --git a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/vcmr/ChangeRequestClient.java b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/vcmr/ChangeRequestClient.java index e8ff768927f..f8f54567bea 100644 --- a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/vcmr/ChangeRequestClient.java +++ b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/vcmr/ChangeRequestClient.java @@ -8,7 +8,8 @@ import java.util.List; */ public interface ChangeRequestClient { - List getUpcomingChangeRequests(); + /** Get upcoming change requests and updated status of previously stored requests */ + List getChangeRequests(List changeRequests); void approveChangeRequests(List changeRequests); diff --git a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/vcmr/MockChangeRequestClient.java b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/vcmr/MockChangeRequestClient.java index e85c0afcb0e..10175f36991 100644 --- a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/vcmr/MockChangeRequestClient.java +++ b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/vcmr/MockChangeRequestClient.java @@ -13,7 +13,7 @@ public class MockChangeRequestClient implements ChangeRequestClient { private List approvedChangeRequests = new ArrayList<>(); @Override - public List getUpcomingChangeRequests() { + public List getChangeRequests(List changeRequests) { return upcomingChangeRequests; } diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/ChangeRequestMaintainer.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/ChangeRequestMaintainer.java index 8f30a932116..ba72f812e34 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/ChangeRequestMaintainer.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/ChangeRequestMaintainer.java @@ -8,10 +8,13 @@ import com.yahoo.vespa.hosted.controller.Controller; import com.yahoo.vespa.hosted.controller.api.integration.configserver.NodeRepository; import com.yahoo.vespa.hosted.controller.api.integration.vcmr.ChangeRequest; import com.yahoo.vespa.hosted.controller.api.integration.vcmr.ChangeRequestClient; +import com.yahoo.vespa.hosted.controller.api.integration.vcmr.ChangeRequestSource; import com.yahoo.vespa.hosted.controller.api.integration.vcmr.VespaChangeRequest; import com.yahoo.vespa.hosted.controller.persistence.CuratorDb; import java.time.Duration; +import java.time.ZonedDateTime; +import java.util.ArrayList; import java.util.Collections; import java.util.List; import java.util.Map; @@ -43,7 +46,8 @@ public class ChangeRequestMaintainer extends ControllerMaintainer { @Override protected boolean maintain() { - var changeRequests = changeRequestClient.getUpcomingChangeRequests(); + var currentChangeRequests = pruneOldChangeRequests(); + var changeRequests = changeRequestClient.getChangeRequests(currentChangeRequests); storeChangeRequests(changeRequests); if (system.equals(SystemName.main)) { @@ -82,6 +86,22 @@ public class ChangeRequestMaintainer extends ControllerMaintainer { } } + // Deletes closed change requests older than 7 days, returns the current list of requests + private List pruneOldChangeRequests() { + List currentChangeRequests = new ArrayList<>(); + + try (var lock = curator.lockChangeRequests()) { + for (var changeRequest : curator.readChangeRequests()) { + if (shouldDeleteChangeRequest(changeRequest.getChangeRequestSource())) { + curator.deleteChangeRequest(changeRequest); + } else { + currentChangeRequests.add(changeRequest); + } + } + } + return currentChangeRequests; + } + private Map> hostsByZone() { return controller().zoneRegistry() .zones() @@ -104,4 +124,11 @@ public class ChangeRequestMaintainer extends ControllerMaintainer { .map(Map.Entry::getKey) .findFirst(); } + + private boolean shouldDeleteChangeRequest(ChangeRequestSource source) { + return source.isClosed() && + source.getPlannedStartTime() + .plus(Duration.ofDays(7)) + .isBefore(ZonedDateTime.now()); + } } 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 aea0eebd374..bf1ee4b23ce 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 @@ -585,6 +585,10 @@ public class CuratorDb { curator.set(changeRequestPath(changeRequest.getId()), asJson(ChangeRequestSerializer.toSlime(changeRequest))); } + public void deleteChangeRequest(VespaChangeRequest changeRequest) { + curator.delete(changeRequestPath(changeRequest.getId())); + } + // -------------- Paths --------------------------------------------------- private Path lockPath(TenantName tenant) { diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/ChangeRequestMaintainerTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/ChangeRequestMaintainerTest.java index 183130a8ec8..117002e6074 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/ChangeRequestMaintainerTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/ChangeRequestMaintainerTest.java @@ -26,8 +26,9 @@ public class ChangeRequestMaintainerTest { @Test public void only_approve_requests_pending_approval() { - var changeRequest1 = newChangeRequest("id1", ChangeRequest.Approval.APPROVED); - var changeRequest2 = newChangeRequest("id2", ChangeRequest.Approval.REQUESTED); + var time = ZonedDateTime.now(); + var changeRequest1 = newChangeRequest("id1", ChangeRequest.Approval.APPROVED, time); + var changeRequest2 = newChangeRequest("id2", ChangeRequest.Approval.REQUESTED, time); var upcomingChangeRequests = List.of( changeRequest1, changeRequest2 @@ -47,7 +48,24 @@ public class ChangeRequestMaintainerTest { assertEquals(expectedChangeRequest, writtenChangeRequests.get(0)); } - private ChangeRequest newChangeRequest(String id, ChangeRequest.Approval approval) { + @Test + public void deletes_old_change_requests() { + var now = ZonedDateTime.now(); + var before = now.minus(Duration.ofDays(8)); + var newChangeRequest = persistedChangeRequest("new", now); + var oldChangeRequest = persistedChangeRequest("old", before); + + tester.curator().writeChangeRequest(newChangeRequest); + tester.curator().writeChangeRequest(oldChangeRequest); + + changeRequestMaintainer.maintain(); + + var persistedChangeRequests = tester.curator().readChangeRequests(); + assertEquals(1, persistedChangeRequests.size()); + assertEquals(newChangeRequest, persistedChangeRequests.get(0)); + } + + private ChangeRequest newChangeRequest(String id, ChangeRequest.Approval approval, ZonedDateTime time) { return new ChangeRequest.Builder() .id(id) .approval(approval) @@ -55,8 +73,8 @@ public class ChangeRequestMaintainerTest { .impactedSwitches(List.of()) .impactedHosts(List.of("node-1-tenant-host-prod.us-east-3")) .changeRequestSource(new ChangeRequestSource.Builder() - .plannedStartTime(ZonedDateTime.now()) - .plannedEndTime(ZonedDateTime.now()) + .plannedStartTime(time) + .plannedEndTime(time) .id("some-id") .url("some-url") .system("some-system") @@ -64,4 +82,11 @@ public class ChangeRequestMaintainerTest { .build()) .build(); } + + private VespaChangeRequest persistedChangeRequest(String id, ZonedDateTime time) { + return new VespaChangeRequest( + newChangeRequest(id, ChangeRequest.Approval.APPROVED, time), + ZoneId.from("prod.us-east-3") + ); + } } -- cgit v1.2.3