Remove DNS records for unassigned rotations

3 files changed, 15 insertions, 13 deletions

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 4e9dd94d8e5..f6dc1326d5e 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
@@ -55,7 +55,7 @@ public class ControllerMaintenance extends AbstractComponent {
         clusterUtilizationMaintainer = new ClusterUtilizationMaintainer(controller, Duration.ofHours(2), jobControl);
         deploymentMetricsMaintainer = new DeploymentMetricsMaintainer(controller, Duration.ofMinutes(10), jobControl);
         applicationOwnershipConfirmer = new ApplicationOwnershipConfirmer(controller, Duration.ofHours(12), jobControl, ownershipIssues);
-        dnsMaintainer = new DnsMaintainer(controller, Duration.ofHours(1), jobControl, nameService);
+        dnsMaintainer = new DnsMaintainer(controller, Duration.ofHours(12), jobControl, nameService);
     }
 
     public Upgrader upgrader() { return upgrader; }
diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/DnsMaintainer.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/DnsMaintainer.java
index 89394bf4dd9..0f957c58a47 100644
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/DnsMaintainer.java
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/DnsMaintainer.java
@@ -49,18 +49,17 @@ public class DnsMaintainer extends Maintainer {
     private void removeDnsAlias(Rotation rotation) {
         // When looking up CNAME by data, the data must be a FQDN
         Optional<Record> record = nameService.findRecord(Record.Type.CNAME, RecordData.fqdn(rotation.name()));
-        record.filter(this::canUpdate)
+        record.filter(DnsMaintainer::canUpdate)
               .ifPresent(r -> {
-                  log.warning(String.format("Want to remove DNS record %s (%s) because it points to the unassigned " +
-                                            "rotation %s (%s)", record.get().id().asString(),
-                                            record.get().name().asString(), rotation.id().asString(), rotation.name()));
-                  // TODO: Actually remove the record
-                  //nameService.removeRecord(r.id());
+                  log.info(String.format("Removing DNS record %s (%s) because it points to the unassigned " +
+                                         "rotation %s (%s)", record.get().id().asString(),
+                                         record.get().name().asString(), rotation.id().asString(), rotation.name()));
+                  nameService.removeRecord(r.id());
               });
     }
 
     /** Returns whether we can update the given record */
-    private boolean canUpdate(Record record) {
+    private static boolean canUpdate(Record record) {
         return record.name().asString().endsWith(ApplicationRotation.DNS_SUFFIX);
     }
 
diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/DnsMaintainerTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/DnsMaintainerTest.java
index 41892ba666b..c8b8dcb6395 100644
--- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/DnsMaintainerTest.java
+++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/DnsMaintainerTest.java
@@ -13,7 +13,6 @@ 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.persistence.MockCuratorDb;
-import org.junit.Ignore;
 import org.junit.Test;
 
 import java.time.Duration;
@@ -31,10 +30,12 @@ import static org.junit.Assert.assertTrue;
 public class DnsMaintainerTest {
 
     @Test
-    @Ignore // TODO: Enable once DnsMaintainer actually removes records
     public void removes_record_for_unassigned_rotation() {
         DeploymentTester tester = new DeploymentTester();
         Application application = tester.createApplication("app1", "tenant1", 1, 1L);
+        DnsMaintainer dnsMaintainer = new DnsMaintainer(tester.controller(), Duration.ofHours(12),
+                                                        new JobControl(new MockCuratorDb()),
+                                                        tester.controllerTester().nameService());
 
         ApplicationPackage applicationPackage = new ApplicationPackageBuilder()
                 .environment(Environment.prod)
@@ -53,6 +54,11 @@ public class DnsMaintainerTest {
         assertEquals("app1.tenant1.global.vespa.yahooapis.com", record.get().name().asString());
         assertEquals("rotation-fqdn-01.", record.get().data().asString());
 
+        // DnsMaintainer does nothing
+        dnsMaintainer.maintain();
+        assertTrue("DNS record is not removed", tester.controllerTester().nameService().findRecord(
+                Record.Type.CNAME, RecordName.from("app1.tenant1.global.vespa.yahooapis.com")).isPresent());
+
         // Remove application
         applicationPackage = new ApplicationPackageBuilder()
                 .environment(Environment.prod)
@@ -65,9 +71,6 @@ public class DnsMaintainerTest {
         tester.applications().deleteApplication(application.id(), Optional.of(new NToken("ntoken")));
 
         // DnsMaintainer removes record
-        DnsMaintainer dnsMaintainer = new DnsMaintainer(tester.controller(), Duration.ofHours(12),
-                                                     new JobControl(new MockCuratorDb()),
-                                                     tester.controllerTester().nameService());
         dnsMaintainer.maintain();
         assertFalse("DNS record removed", tester.controllerTester().nameService().findRecord(
                 Record.Type.CNAME, RecordName.from("app1.tenant1.global.vespa.yahooapis.com")).isPresent());