diff options
Diffstat (limited to 'controller-server')
4 files changed, 28 insertions, 14 deletions
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 a25aa9797ba..dc9c4650191 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 @@ -7,6 +7,7 @@ import com.yahoo.component.annotation.Inject; import com.yahoo.concurrent.UncheckedTimeoutException; import com.yahoo.config.provision.ApplicationId; import com.yahoo.config.provision.ClusterSpec; +import com.yahoo.config.provision.ClusterSpec.Id; import com.yahoo.config.provision.HostName; import com.yahoo.config.provision.InstanceName; import com.yahoo.config.provision.TenantName; @@ -602,7 +603,7 @@ public class CuratorDb { public List<DnsChallenge> readDnsChallenges(DeploymentId id) { return curator.getChildren(dnsChallengePath(id)).stream() - .map(cluster -> readDnsChallenge(new ClusterId(id, ClusterSpec.Id.from(cluster)))) + .map(cluster -> readDnsChallenge(new ClusterId(id, Id.from(cluster)))) .toList(); } diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/routing/RoutingPolicies.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/routing/RoutingPolicies.java index de25161c461..5919889cdbd 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/routing/RoutingPolicies.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/routing/RoutingPolicies.java @@ -416,7 +416,7 @@ public class RoutingPolicies { private void setPrivateDns(Endpoint endpoint, LoadBalancer loadBalancer, DeploymentId deploymentId) { if (loadBalancer.service().isEmpty()) return; - // TODO(mpolden): Why is this done? Consider creating private DNS for all auth methods + // TODO(mpolden): Model one service for each endpoint (type), to allow private endpoints with tokens. boolean skipBasedOnAuthMethod = switch (endpoint.authMethod()) { case token -> true; case mtls -> false; @@ -436,10 +436,21 @@ public class RoutingPolicies { }); } + /** Deletes all DNS challenges, and corresponding TXT records, for the given deployment. */ + public void removeDnsChallenges(DeploymentId deploymentId) { + try (Mutex lock = db.lockNameServiceQueue()) { + for (DnsChallenge challenge : db.readDnsChallenges(deploymentId)) { + controller.nameServiceForwarder().removeRecords(Record.Type.TXT, challenge.name(), Priority.normal, ownerOf(deploymentId)); + db.deleteDnsChallenge(challenge.clusterId()); + } + } + } + /** Returns true iff. the given deployment has no incomplete DNS challenges, or throws (and cleans up) on errors. */ public boolean processDnsChallenges(DeploymentId deploymentId) { try (Mutex lock = db.lockNameServiceQueue()) { List<DnsChallenge> challenges = new ArrayList<>(db.readDnsChallenges(deploymentId)); + challenges.removeIf(challenge -> challenge.state() == ChallengeState.done); Set<RecordName> pendingRequests = controller.curator().readNameServiceQueue().requests().stream() .map(NameServiceRequest::name) .collect(Collectors.toSet()); @@ -450,14 +461,8 @@ public class RoutingPolicies { challenge = challenge.withState(ChallengeState.ready); } ChallengeState state = controller.serviceRegistry().vpcEndpointService().process(challenge); - if (state == ChallengeState.done) { - removeDnsChallenge(challenge); - return true; - } - else { - db.writeDnsChallenge(challenge.withState(state)); - return false; - } + db.writeDnsChallenge(challenge.withState(state)); + return state == ChallengeState.done; }); return challenges.isEmpty(); } diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/routing/context/DeploymentRoutingContext.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/routing/context/DeploymentRoutingContext.java index df0226176a2..99f60735f6e 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/routing/context/DeploymentRoutingContext.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/routing/context/DeploymentRoutingContext.java @@ -57,6 +57,7 @@ public abstract class DeploymentRoutingContext implements RoutingContext { /** Deactivate routing configuration for the deployment in this context, using given deployment spec */ public final void deactivate(DeploymentSpec deploymentSpec) { routing.policies().refresh(deployment, deploymentSpec, EndpointList.EMPTY); + routing.policies().removeDnsChallenges(deployment); } /** Routing method of this context */ diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/routing/RoutingPoliciesTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/routing/RoutingPoliciesTest.java index 630de5137bb..1b2fa956763 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/routing/RoutingPoliciesTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/routing/RoutingPoliciesTest.java @@ -598,21 +598,28 @@ public class RoutingPoliciesTest { app.deploy(); - // TXT records are cleaned up as we go—the last challenge is the last to go here, and we must flush it ourselves. + // TXT records are cleaned up when deployments are deactivated. + // The last challenge is the last to go here, and we must flush it ourselves. assertEquals(List.of("a.t.aws-us-east-33a.vespa.oath.cloud", "challenge--a.t.aws-us-east-33a.vespa.oath.cloud"), tester.recordNames()); app.flushDnsUpdates(); assertEquals(Set.of(new Record(Type.CNAME, RecordName.from("a.t.aws-us-east-33a.vespa.oath.cloud"), - RecordData.from("lb-0--t.a.default--prod.aws-us-east-33a."))), + RecordData.from("lb-0--t.a.default--prod.aws-us-east-33a.")), + new Record(Type.TXT, + RecordName.from("challenge--a.t.aws-us-east-33a.vespa.oath.cloud"), + RecordData.from("system"))), tester.controllerTester().nameService().records()); + tester.controllerTester().controller().applications().deactivate(app.instanceId(), zone3); + app.flushDnsUpdates(); + assertEquals(Set.of(), + tester.controllerTester().nameService().records()); + // Deployment fails because challenge is not answered (immediately). tester.tester.controllerTester().serviceRegistry().vpcEndpointService().outcomes .put(RecordName.from("challenge--a.t.aws-us-east-33a.vespa.oath.cloud"), ChallengeState.running); - - // Deployment fails because challenge is not answered (immediately). assertEquals("Status of run 2 of production-aws-us-east-33a for t.a ==> expected: <succeeded> but was: <unfinished>", assertThrows(AssertionError.class, () -> app.submit(appPackage).deploy()) |