diff options
27 files changed, 311 insertions, 133 deletions
diff --git a/config-provisioning/src/main/java/com/yahoo/config/provision/ProvisionLock.java b/config-provisioning/src/main/java/com/yahoo/config/provision/ProvisionLock.java new file mode 100644 index 00000000000..633c8520e72 --- /dev/null +++ b/config-provisioning/src/main/java/com/yahoo/config/provision/ProvisionLock.java @@ -0,0 +1,32 @@ +// Copyright Verizon Media. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package com.yahoo.config.provision; + +import com.yahoo.transaction.Mutex; + +import java.util.Objects; + +/** + * A type-safe wrapper for an application's provision lock. + * + * @author mpolden + */ +public class ProvisionLock implements AutoCloseable { + + private final ApplicationId application; + private final Mutex lock; + + public ProvisionLock(ApplicationId application, Mutex lock) { + this.application = Objects.requireNonNull(application); + this.lock = Objects.requireNonNull(lock); + } + + public ApplicationId application() { + return application; + } + + @Override + public void close() { + lock.close(); + } + +} diff --git a/config-provisioning/src/main/java/com/yahoo/config/provision/Provisioner.java b/config-provisioning/src/main/java/com/yahoo/config/provision/Provisioner.java index 2a1528f5368..1eb2c1e61b2 100644 --- a/config-provisioning/src/main/java/com/yahoo/config/provision/Provisioner.java +++ b/config-provisioning/src/main/java/com/yahoo/config/provision/Provisioner.java @@ -31,17 +31,36 @@ public interface Provisioner { * @param application The {@link ApplicationId} that was activated. * @param hosts a set of {@link HostSpec}. */ + // TODO(mpolden): Remove void activate(NestedTransaction transaction, ApplicationId application, Collection<HostSpec> hosts); /** + * Activates the allocation of nodes to this application captured in the hosts argument. + * + * @param transaction Transaction with operations to commit together with any operations done within the provisioner. + * @param hosts a set of {@link HostSpec}. + * @param lock A provision lock for the relevant application. This must be held when calling this. + */ + void activate(NestedTransaction transaction, Collection<HostSpec> hosts, ProvisionLock lock); + + /** * Transactionally remove this application. * * @param transaction Transaction with operations to commit together with any operations done within the provisioner. * @param application the application to remove */ + // TODO(mpolden): Remove void remove(NestedTransaction transaction, ApplicationId application); /** + * Transactionally remove application guarded by given lock. + * + * @param transaction Transaction with operations to commit together with any operations done within the provisioner. + * @param lock A provision lock for the relevant application. This must be held when calling this. + */ + void remove(NestedTransaction transaction, ProvisionLock lock); + + /** * Requests a restart of the services of the given application * * @param application the application to restart @@ -49,4 +68,7 @@ public interface Provisioner { */ void restart(ApplicationId application, HostFilter filter); + /** Returns a provision lock for the given application */ + ProvisionLock lock(ApplicationId application); + } diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/ApplicationRepository.java b/configserver/src/main/java/com/yahoo/vespa/config/server/ApplicationRepository.java index 4fba0204f55..083112d8456 100644 --- a/configserver/src/main/java/com/yahoo/vespa/config/server/ApplicationRepository.java +++ b/configserver/src/main/java/com/yahoo/vespa/config/server/ApplicationRepository.java @@ -132,6 +132,7 @@ public class ApplicationRepository implements com.yahoo.config.provision.Deploye private final TesterClient testerClient; private final Metric metric; private final BooleanFlag deployWithInternalRestart; + private final BooleanFlag acquireProvisionLock; @Inject public ApplicationRepository(TenantRepository tenantRepository, @@ -182,6 +183,7 @@ public class ApplicationRepository implements com.yahoo.config.provision.Deploye this.testerClient = Objects.requireNonNull(testerClient); this.metric = Objects.requireNonNull(metric); this.deployWithInternalRestart = Flags.DEPLOY_WITH_INTERNAL_RESTART.bindTo(flagSource); + this.acquireProvisionLock = Flags.ALWAYS_ACQUIRE_PROVISION_LOCK.bindTo(flagSource); } public static class Builder { @@ -508,7 +510,15 @@ public class ApplicationRepository implements com.yahoo.config.provision.Deploye // and allocated hosts in model and handlers in RPC server transaction.add(tenantApplications.createDeleteTransaction(applicationId)); - hostProvisioner.ifPresent(provisioner -> provisioner.remove(transaction, applicationId)); + hostProvisioner.ifPresent(provisioner -> { + if (acquireProvisionLock.value()) { + try (var provisionLock = provisioner.lock(applicationId)) { + provisioner.remove(transaction, provisionLock); + } + } else { + provisioner.remove(transaction, applicationId); + } + }); transaction.onCommitted(() -> log.log(Level.INFO, "Deleted " + applicationId)); transaction.commit(); return true; @@ -722,13 +732,19 @@ public class ApplicationRepository implements com.yahoo.config.provision.Deploye // ---------------- Session operations ---------------------------------------------------------------- - - public CompletionWaiter activate(LocalSession session, Session previousActiveSession, ApplicationId applicationId, boolean force) { CompletionWaiter waiter = session.getSessionZooKeeperClient().createActiveWaiter(); NestedTransaction transaction = new NestedTransaction(); transaction.add(deactivateCurrentActivateNew(previousActiveSession, session, force)); - hostProvisioner.ifPresent(provisioner -> provisioner.activate(transaction, applicationId, session.getAllocatedHosts().getHosts())); + hostProvisioner.ifPresent(provisioner -> { + if (acquireProvisionLock.value()) { + try (var lock = provisioner.lock(applicationId)) { + provisioner.activate(transaction, session.getAllocatedHosts().getHosts(), lock); + } + } else { + provisioner.activate(transaction, applicationId, session.getAllocatedHosts().getHosts()); + } + }); transaction.commit(); return waiter; } diff --git a/configserver/src/test/java/com/yahoo/vespa/config/server/deploy/DeployTester.java b/configserver/src/test/java/com/yahoo/vespa/config/server/deploy/DeployTester.java index 7553583e70c..87122fdba45 100644 --- a/configserver/src/test/java/com/yahoo/vespa/config/server/deploy/DeployTester.java +++ b/configserver/src/test/java/com/yahoo/vespa/config/server/deploy/DeployTester.java @@ -22,6 +22,7 @@ import com.yahoo.config.provision.Capacity; import com.yahoo.config.provision.ClusterSpec; import com.yahoo.config.provision.HostFilter; import com.yahoo.config.provision.HostSpec; +import com.yahoo.config.provision.ProvisionLock; import com.yahoo.config.provision.ProvisionLogger; import com.yahoo.config.provision.Provisioner; import com.yahoo.config.provision.TenantName; @@ -227,15 +228,28 @@ public class DeployTester { } @Override + public void activate(NestedTransaction transaction, Collection<HostSpec> hosts, ProvisionLock lock) { + } + + @Override public void remove(NestedTransaction transaction, ApplicationId application) { // noop } @Override + public void remove(NestedTransaction transaction, ProvisionLock lock) { + } + + @Override public void restart(ApplicationId application, HostFilter filter) { // noop } + @Override + public ProvisionLock lock(ApplicationId application) { + return null; + } + } private static class FailingModelFactory implements ModelFactory { diff --git a/configserver/src/test/java/com/yahoo/vespa/config/server/http/SessionHandlerTest.java b/configserver/src/test/java/com/yahoo/vespa/config/server/http/SessionHandlerTest.java index 9d696ae34e7..32a9e867684 100644 --- a/configserver/src/test/java/com/yahoo/vespa/config/server/http/SessionHandlerTest.java +++ b/configserver/src/test/java/com/yahoo/vespa/config/server/http/SessionHandlerTest.java @@ -6,6 +6,7 @@ import com.yahoo.config.provision.Capacity; import com.yahoo.config.provision.ClusterSpec; import com.yahoo.config.provision.HostFilter; import com.yahoo.config.provision.HostSpec; +import com.yahoo.config.provision.ProvisionLock; import com.yahoo.config.provision.ProvisionLogger; import com.yahoo.config.provision.Provisioner; import com.yahoo.container.jdisc.HttpRequest; @@ -102,17 +103,32 @@ public class SessionHandlerTest { } @Override + public void activate(NestedTransaction transaction, Collection<HostSpec> hosts, ProvisionLock lock) { + + } + + @Override public void remove(NestedTransaction transaction, ApplicationId application) { removed = true; lastApplicationId = application; } @Override + public void remove(NestedTransaction transaction, ProvisionLock lock) { + + } + + @Override public void restart(ApplicationId application, HostFilter filter) { restarted = true; lastApplicationId = application; } + @Override + public ProvisionLock lock(ApplicationId application) { + return null; + } + } } diff --git a/configserver/src/test/java/com/yahoo/vespa/config/server/maintenance/MaintainerTester.java b/configserver/src/test/java/com/yahoo/vespa/config/server/maintenance/MaintainerTester.java index 712242a69e6..d092a3139a0 100644 --- a/configserver/src/test/java/com/yahoo/vespa/config/server/maintenance/MaintainerTester.java +++ b/configserver/src/test/java/com/yahoo/vespa/config/server/maintenance/MaintainerTester.java @@ -9,6 +9,7 @@ import com.yahoo.config.provision.Capacity; import com.yahoo.config.provision.ClusterSpec; import com.yahoo.config.provision.HostFilter; import com.yahoo.config.provision.HostSpec; +import com.yahoo.config.provision.ProvisionLock; import com.yahoo.config.provision.ProvisionLogger; import com.yahoo.config.provision.Provisioner; import com.yahoo.transaction.NestedTransaction; @@ -95,15 +96,30 @@ class MaintainerTester { } @Override + public void activate(NestedTransaction transaction, Collection<HostSpec> hosts, ProvisionLock lock) { + + } + + @Override public void remove(NestedTransaction transaction, ApplicationId application) { // noop } @Override + public void remove(NestedTransaction transaction, ProvisionLock lock) { + + } + + @Override public void restart(ApplicationId application, HostFilter filter) { // noop } + @Override + public ProvisionLock lock(ApplicationId application) { + return null; + } + } -}
\ No newline at end of file +} diff --git a/configserver/src/test/java/com/yahoo/vespa/config/server/session/SessionPreparerTest.java b/configserver/src/test/java/com/yahoo/vespa/config/server/session/SessionPreparerTest.java index 3858388ed39..43504988d67 100644 --- a/configserver/src/test/java/com/yahoo/vespa/config/server/session/SessionPreparerTest.java +++ b/configserver/src/test/java/com/yahoo/vespa/config/server/session/SessionPreparerTest.java @@ -17,6 +17,7 @@ import com.yahoo.config.provision.ClusterSpec; import com.yahoo.config.provision.HostFilter; import com.yahoo.config.provision.HostSpec; import com.yahoo.config.provision.InstanceName; +import com.yahoo.config.provision.ProvisionLock; import com.yahoo.config.provision.ProvisionLogger; import com.yahoo.config.provision.Provisioner; import com.yahoo.config.provision.TenantName; @@ -422,11 +423,26 @@ public class SessionPreparerTest { public void activate(NestedTransaction transaction, ApplicationId application, Collection<HostSpec> hosts) { } @Override + public void activate(NestedTransaction transaction, Collection<HostSpec> hosts, ProvisionLock lock) { + + } + + @Override public void remove(NestedTransaction transaction, ApplicationId application) { } @Override + public void remove(NestedTransaction transaction, ProvisionLock lock) { + + } + + @Override public void restart(ApplicationId application, HostFilter filter) { } + @Override + public ProvisionLock lock(ApplicationId application) { + return null; + } + } } diff --git a/flags/src/main/java/com/yahoo/vespa/flags/Flags.java b/flags/src/main/java/com/yahoo/vespa/flags/Flags.java index 6797c06891a..bb2174cb1e6 100644 --- a/flags/src/main/java/com/yahoo/vespa/flags/Flags.java +++ b/flags/src/main/java/com/yahoo/vespa/flags/Flags.java @@ -375,6 +375,12 @@ public class Flags { "Whether application containers should use the new restapi handler implementation", "Takes effect on next internal redeployment"); + public static final UnboundBooleanFlag ALWAYS_ACQUIRE_PROVISION_LOCK = defineFeatureFlag( + "always-acquire-provision-lock", + false, + "Whether provision lock should always be taken when writing nodes", + "Takes effect on config server restart"); + /** WARNING: public for testing: All flags should be defined in {@link Flags}. */ public static UnboundBooleanFlag defineFeatureFlag(String flagId, boolean defaultValue, String description, String modificationEffect, FetchVector.Dimension... dimensions) { diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/NodeRepository.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/NodeRepository.java index 455d9e59734..e92f039ad01 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/NodeRepository.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/NodeRepository.java @@ -11,6 +11,7 @@ import com.yahoo.config.provision.DockerImage; import com.yahoo.config.provision.Flavor; import com.yahoo.config.provision.NodeFlavors; import com.yahoo.config.provision.NodeType; +import com.yahoo.config.provision.ProvisionLock; import com.yahoo.config.provision.TenantName; import com.yahoo.config.provision.Zone; import com.yahoo.config.provisioning.NodeRepositoryConfig; @@ -502,19 +503,17 @@ public class NodeRepository extends AbstractComponent { } } - public void deactivate(ApplicationId application, NestedTransaction transaction) { - try (Mutex lock = lock(application)) { - deactivate(db.readNodes(application, State.reserved, State.active), transaction); - applications.remove(application, transaction, lock); - } + /** Deactivate nodes owned by application guarded by given lock */ + public void deactivate(NestedTransaction transaction, ProvisionLock lock) { + deactivate(db.readNodes(lock.application(), State.reserved, State.active), transaction, lock); + applications.remove(lock.application(), transaction, lock); } /** - * Deactivates these nodes in a transaction and returns - * the nodes in the new state which will hold if the transaction commits. - * This method does <b>not</b> lock + * Deactivates these nodes in a transaction and returns the nodes in the new state which will hold if the + * transaction commits. */ - public List<Node> deactivate(List<Node> nodes, NestedTransaction transaction) { + public List<Node> deactivate(List<Node> nodes, NestedTransaction transaction, @SuppressWarnings("unused") ProvisionLock lock) { return db.writeTo(State.inactive, nodes, Agent.application, Optional.empty(), transaction); } diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/applications/Applications.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/applications/Applications.java index 17f8d73c88f..9f45839f1c3 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/applications/Applications.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/applications/Applications.java @@ -2,6 +2,7 @@ package com.yahoo.vespa.hosted.provision.applications; import com.yahoo.config.provision.ApplicationId; +import com.yahoo.config.provision.ProvisionLock; import com.yahoo.transaction.Mutex; import com.yahoo.transaction.NestedTransaction; import com.yahoo.vespa.hosted.provision.persistence.CuratorDatabaseClient; @@ -38,17 +39,19 @@ public class Applications { return db.readApplication(id); } + // TODO: Require ProvisionLock instead of Mutex public void put(Application application, Mutex applicationLock) { NestedTransaction transaction = new NestedTransaction(); put(application, transaction, applicationLock); transaction.commit(); } + // TODO: Require ProvisionLock instead of Mutex public void put(Application application, NestedTransaction transaction, Mutex applicationLock) { db.writeApplication(application, transaction); } - public void remove(ApplicationId application, NestedTransaction transaction, Mutex applicationLock) { + public void remove(ApplicationId application, NestedTransaction transaction, @SuppressWarnings("unused") ProvisionLock lock) { db.deleteApplication(application, transaction); } diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/Activator.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/Activator.java index e36d5fa4075..b85862446a8 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/Activator.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/Activator.java @@ -7,6 +7,7 @@ import com.yahoo.config.provision.ClusterSpec; import com.yahoo.config.provision.Flavor; import com.yahoo.config.provision.HostSpec; import com.yahoo.config.provision.ParentHostUnavailableException; +import com.yahoo.config.provision.ProvisionLock; import com.yahoo.transaction.Mutex; import com.yahoo.transaction.NestedTransaction; import com.yahoo.vespa.hosted.provision.Node; @@ -37,12 +38,10 @@ class Activator { this.loadBalancerProvisioner = loadBalancerProvisioner; } - /** Activate required resources for given application */ - public void activate(ApplicationId application, Collection<HostSpec> hosts, NestedTransaction transaction) { - try (Mutex lock = nodeRepository.lock(application)) { - activateNodes(application, hosts, transaction, lock); - activateLoadBalancers(application, hosts, transaction, lock); - } + /** Activate required resources for application guarded by given lock */ + public void activate(Collection<HostSpec> hosts, NestedTransaction transaction, ProvisionLock lock) { + activateNodes(hosts, transaction, lock); + activateLoadBalancers(hosts, transaction, lock); } /** @@ -55,12 +54,11 @@ class Activator { * Nodes in active which are not present in <code>hosts</code> are moved to inactive. * * @param transaction Transaction with operations to commit together with any operations done within the repository. - * @param application the application to allocate nodes for * @param hosts the hosts to make the set of active nodes of this - * @param applicationLock application lock that must be held when calling this + * @param lock provision lock that must be held when calling this */ - private void activateNodes(ApplicationId application, Collection<HostSpec> hosts, NestedTransaction transaction, - @SuppressWarnings("unused") Mutex applicationLock) { + private void activateNodes(Collection<HostSpec> hosts, NestedTransaction transaction, ProvisionLock lock) { + ApplicationId application = lock.application(); Set<String> hostnames = hosts.stream().map(HostSpec::hostname).collect(Collectors.toSet()); NodeList allNodes = nodeRepository.list(); NodeList applicationNodes = allNodes.owner(application); @@ -84,7 +82,7 @@ class Activator { List<Node> activeToRemove = removeHostsFromList(hostnames, active); activeToRemove = activeToRemove.stream().map(Node::unretire).collect(Collectors.toList()); // only active nodes can be retired - nodeRepository.deactivate(activeToRemove, transaction); + nodeRepository.deactivate(activeToRemove, transaction, lock); nodeRepository.activate(updateFrom(hosts, continuedActive), transaction); // update active with any changes nodeRepository.activate(updatePortsFrom(hosts, reservedToActivate), transaction); unreserveParentsOf(reservedToActivate); @@ -106,9 +104,8 @@ class Activator { } /** Activate load balancers */ - private void activateLoadBalancers(ApplicationId application, Collection<HostSpec> hosts, NestedTransaction transaction, - @SuppressWarnings("unused") Mutex applicationLock) { - loadBalancerProvisioner.ifPresent(provisioner -> provisioner.activate(application, allClustersOf(hosts), applicationLock, transaction)); + private void activateLoadBalancers(Collection<HostSpec> hosts, NestedTransaction transaction, ProvisionLock lock) { + loadBalancerProvisioner.ifPresent(provisioner -> provisioner.activate(transaction, allClustersOf(hosts), lock)); } private static Set<ClusterSpec> allClustersOf(Collection<HostSpec> hosts) { diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/InfraDeployerImpl.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/InfraDeployerImpl.java index e6ace49cacd..b3506a0c102 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/InfraDeployerImpl.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/InfraDeployerImpl.java @@ -11,7 +11,6 @@ import com.yahoo.config.provision.HostSpec; import com.yahoo.config.provision.InfraDeployer; import com.yahoo.config.provision.NodeType; import com.yahoo.config.provision.Provisioner; -import java.util.logging.Level; import com.yahoo.transaction.Mutex; import com.yahoo.transaction.NestedTransaction; import com.yahoo.vespa.hosted.provision.NodeRepository; @@ -21,6 +20,7 @@ import com.yahoo.vespa.service.monitor.InfraApplicationApi; import java.util.List; import java.util.Optional; +import java.util.logging.Level; import java.util.logging.Logger; import java.util.stream.Collectors; @@ -97,7 +97,7 @@ public class InfraDeployerImpl implements InfraDeployer { @Override public long activate() { - try (Mutex lock = nodeRepository.lock(application.getApplicationId())) { + try (var lock = provisioner.lock(application.getApplicationId())) { prepare(); if (hostSpecs.isEmpty()) { @@ -105,7 +105,7 @@ public class InfraDeployerImpl implements InfraDeployer { removeApplication(application.getApplicationId()); } else { NestedTransaction nestedTransaction = new NestedTransaction(); - provisioner.activate(nestedTransaction, application.getApplicationId(), hostSpecs); + provisioner.activate(nestedTransaction, hostSpecs, lock); nestedTransaction.commit(); duperModel.infraApplicationActivated( @@ -128,10 +128,12 @@ public class InfraDeployerImpl implements InfraDeployer { private void removeApplication(ApplicationId applicationId) { // Use the DuperModel as source-of-truth on whether it has also been activated (to avoid periodic removals) if (duperModel.infraApplicationIsActive(applicationId)) { - NestedTransaction nestedTransaction = new NestedTransaction(); - provisioner.remove(nestedTransaction, applicationId); - nestedTransaction.commit(); - duperModel.infraApplicationRemoved(applicationId); + try (var lock = provisioner.lock(applicationId)) { + NestedTransaction nestedTransaction = new NestedTransaction(); + provisioner.remove(nestedTransaction, lock); + nestedTransaction.commit(); + duperModel.infraApplicationRemoved(applicationId); + } } } diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/LoadBalancerProvisioner.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/LoadBalancerProvisioner.java index cc0b5e411ca..634726f9d71 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/LoadBalancerProvisioner.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/LoadBalancerProvisioner.java @@ -5,8 +5,8 @@ import com.yahoo.config.provision.ApplicationId; import com.yahoo.config.provision.ClusterSpec; import com.yahoo.config.provision.HostName; import com.yahoo.config.provision.NodeType; +import com.yahoo.config.provision.ProvisionLock; import com.yahoo.config.provision.exception.LoadBalancerServiceException; -import com.yahoo.transaction.Mutex; import com.yahoo.transaction.NestedTransaction; import com.yahoo.vespa.flags.BooleanFlag; import com.yahoo.vespa.flags.FlagSource; @@ -82,7 +82,7 @@ public class LoadBalancerProvisioner { try (var lock = db.lock(application)) { ClusterSpec.Id clusterId = effectiveId(cluster); List<Node> nodes = nodesOf(clusterId, application); - provision(application, clusterId, nodes, false, lock); + provision(application, clusterId, nodes, false, new ProvisionLock(application, lock)); } } @@ -96,29 +96,24 @@ public class LoadBalancerProvisioner { * * Calling this when no load balancer has been prepared for given cluster is a no-op. */ - public void activate(ApplicationId application, Set<ClusterSpec> clusters, - @SuppressWarnings("unused") Mutex applicationLock, NestedTransaction transaction) { - try (var lock = db.lock(application)) { - for (var cluster : loadBalancedClustersOf(application).entrySet()) { - // Provision again to ensure that load balancer instance is re-configured with correct nodes - provision(application, cluster.getKey(), cluster.getValue(), true, lock); - } - // Deactivate any surplus load balancers, i.e. load balancers for clusters that have been removed - var surplusLoadBalancers = surplusLoadBalancersOf(application, clusters.stream() - .map(LoadBalancerProvisioner::effectiveId) - .collect(Collectors.toSet())); - deactivate(surplusLoadBalancers, transaction); + public void activate(NestedTransaction transaction, Set<ClusterSpec> clusters, ProvisionLock lock) { + for (var cluster : loadBalancedClustersOf(lock.application()).entrySet()) { + // Provision again to ensure that load balancer instance is re-configured with correct nodes + provision(lock.application(), cluster.getKey(), cluster.getValue(), true, lock); } + // Deactivate any surplus load balancers, i.e. load balancers for clusters that have been removed + var surplusLoadBalancers = surplusLoadBalancersOf(lock.application(), clusters.stream() + .map(LoadBalancerProvisioner::effectiveId) + .collect(Collectors.toSet())); + deactivate(surplusLoadBalancers, transaction); } /** * Deactivate all load balancers assigned to given application. This is a no-op if an application does not have any * load balancer(s). */ - public void deactivate(ApplicationId application, NestedTransaction transaction) { - try (var lock = nodeRepository.lock(application)) { - deactivate(nodeRepository.loadBalancers(application).asList(), transaction); - } + public void deactivate(NestedTransaction transaction, ProvisionLock lock) { + deactivate(nodeRepository.loadBalancers(lock.application()).asList(), transaction); } /** Returns load balancers of given application that are no longer referenced by given clusters */ @@ -156,7 +151,7 @@ public class LoadBalancerProvisioner { /** Idempotently provision a load balancer for given application and cluster */ private void provision(ApplicationId application, ClusterSpec.Id clusterId, List<Node> nodes, boolean activate, - @SuppressWarnings("unused") Mutex loadBalancersLock) { + @SuppressWarnings("unused") ProvisionLock lock) { var id = new LoadBalancerId(application, clusterId); var now = nodeRepository.clock().instant(); var loadBalancer = db.readLoadBalancer(id); diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/NodeRepositoryProvisioner.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/NodeRepositoryProvisioner.java index aea85729d6d..b79b87ae86c 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/NodeRepositoryProvisioner.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/NodeRepositoryProvisioner.java @@ -10,6 +10,7 @@ import com.yahoo.config.provision.HostFilter; import com.yahoo.config.provision.HostSpec; import com.yahoo.config.provision.NodeResources; import com.yahoo.config.provision.NodeType; +import com.yahoo.config.provision.ProvisionLock; import com.yahoo.config.provision.ProvisionLogger; import com.yahoo.config.provision.Provisioner; import com.yahoo.config.provision.Zone; @@ -118,9 +119,17 @@ public class NodeRepositoryProvisioner implements Provisioner { } @Override + // TODO(mpolden): Remove public void activate(NestedTransaction transaction, ApplicationId application, Collection<HostSpec> hosts) { + try (var lock = lock(application)) { + activate(transaction, hosts, lock); + } + } + + @Override + public void activate(NestedTransaction transaction, Collection<HostSpec> hosts, ProvisionLock lock) { validate(hosts); - activator.activate(application, hosts, transaction); + activator.activate(hosts, transaction, lock); } @Override @@ -129,9 +138,22 @@ public class NodeRepositoryProvisioner implements Provisioner { } @Override + // TODO(mpolden): Remove public void remove(NestedTransaction transaction, ApplicationId application) { - nodeRepository.deactivate(application, transaction); - loadBalancerProvisioner.ifPresent(lbProvisioner -> lbProvisioner.deactivate(application, transaction)); + try (var lock = lock(application)) { + remove(transaction, lock); + } + } + + @Override + public void remove(NestedTransaction transaction, ProvisionLock lock) { + nodeRepository.deactivate(transaction, lock); + loadBalancerProvisioner.ifPresent(lbProvisioner -> lbProvisioner.deactivate(transaction, lock)); + } + + @Override + public ProvisionLock lock(ApplicationId application) { + return new ProvisionLock(application, nodeRepository.lock(application)); } /** diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/testutils/MockDeployer.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/testutils/MockDeployer.java index 7f613c85cf5..4baaeb12167 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/testutils/MockDeployer.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/testutils/MockDeployer.java @@ -155,10 +155,12 @@ public class MockDeployer implements Deployer { prepare(); if (failActivate) throw new IllegalStateException("failActivate is true"); - try (NestedTransaction t = new NestedTransaction()) { - provisioner.activate(t, application.id(), preparedHosts); - t.commit(); - lastDeployTimes.put(application.id, clock.instant()); + try (var lock = provisioner.lock(application.id)) { + try (NestedTransaction t = new NestedTransaction()) { + provisioner.activate(t, preparedHosts, lock); + t.commit(); + lastDeployTimes.put(application.id, clock.instant()); + } } return redeployments++; } diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/testutils/MockNodeRepository.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/testutils/MockNodeRepository.java index e3a3c0f4fce..042b4aa049c 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/testutils/MockNodeRepository.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/testutils/MockNodeRepository.java @@ -197,9 +197,11 @@ public class MockNodeRepository extends NodeRepository { } private void activate(List<HostSpec> hosts, ApplicationId application, NodeRepositoryProvisioner provisioner) { - NestedTransaction transaction = new NestedTransaction(); - provisioner.activate(transaction, application, hosts); - transaction.commit(); + try (var lock = provisioner.lock(application)) { + NestedTransaction transaction = new NestedTransaction(); + provisioner.activate(transaction, hosts, lock); + transaction.commit(); + } } private void addRecord(String name, String ipAddress) { diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/testutils/MockProvisioner.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/testutils/MockProvisioner.java index 12731e30f46..3dc96e0011b 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/testutils/MockProvisioner.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/testutils/MockProvisioner.java @@ -6,12 +6,12 @@ import com.yahoo.config.provision.Capacity; import com.yahoo.config.provision.ClusterSpec; import com.yahoo.config.provision.HostFilter; import com.yahoo.config.provision.HostSpec; +import com.yahoo.config.provision.ProvisionLock; import com.yahoo.config.provision.ProvisionLogger; import com.yahoo.config.provision.Provisioner; import com.yahoo.transaction.NestedTransaction; import java.util.Collection; -import java.util.Collections; import java.util.List; /** @@ -21,7 +21,7 @@ public class MockProvisioner implements Provisioner { @Override public List<HostSpec> prepare(ApplicationId applicationId, ClusterSpec cluster, Capacity capacity, ProvisionLogger logger) { - return Collections.emptyList(); + return List.of(); } @Override @@ -30,13 +30,28 @@ public class MockProvisioner implements Provisioner { } @Override + public void activate(NestedTransaction transaction, Collection<HostSpec> hosts, ProvisionLock lock) { + + } + + @Override public void remove(NestedTransaction transaction, ApplicationId application) { } @Override + public void remove(NestedTransaction transaction, ProvisionLock lock) { + + } + + @Override public void restart(ApplicationId application, HostFilter filter) { } + @Override + public ProvisionLock lock(ApplicationId application) { + return null; + } + } diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/applications/ApplicationsTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/applications/ApplicationsTest.java index bdb14a868bd..ea304233f8a 100644 --- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/applications/ApplicationsTest.java +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/applications/ApplicationsTest.java @@ -2,6 +2,7 @@ package com.yahoo.vespa.hosted.provision.applications; import com.yahoo.config.provision.ApplicationId; +import com.yahoo.config.provision.ProvisionLock; import com.yahoo.transaction.NestedTransaction; import com.yahoo.vespa.hosted.provision.NodeRepositoryTester; import org.junit.Test; @@ -30,7 +31,7 @@ public class ApplicationsTest { assertEquals(app1, applications.get(app1).get().id()); assertEquals(List.of(app1), applications.ids()); NestedTransaction t = new NestedTransaction(); - applications.remove(app1, t, tester.nodeRepository().lock(app1)); + applications.remove(app1, t, provisionLock(app1, tester)); t.commit(); assertTrue(applications.get(app1).isEmpty()); assertEquals(List.of(), applications.ids()); @@ -43,13 +44,17 @@ public class ApplicationsTest { t.commit(); assertEquals(List.of(app1, app2, app3), applications.ids()); t = new NestedTransaction(); - applications.remove(app1, t, tester.nodeRepository().lock(app1)); - applications.remove(app2, t, tester.nodeRepository().lock(app2)); - applications.remove(app3, t, tester.nodeRepository().lock(app3)); + applications.remove(app1, t, provisionLock(app1, tester)); + applications.remove(app2, t, provisionLock(app2, tester)); + applications.remove(app3, t, provisionLock(app3, tester)); assertEquals(List.of(app1, app2, app3), applications.ids()); t.commit(); assertTrue(applications.get(app1).isEmpty()); assertEquals(List.of(), applications.ids()); } + private ProvisionLock provisionLock(ApplicationId application, NodeRepositoryTester tester) { + return new ProvisionLock(application, tester.nodeRepository().lock(application)); + } + } diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/FailedExpirerTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/FailedExpirerTest.java index a817cc4f413..258e2c7cd42 100644 --- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/FailedExpirerTest.java +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/FailedExpirerTest.java @@ -338,9 +338,11 @@ public class FailedExpirerTest { public FailureScenario allocate(ApplicationId applicationId, ClusterSpec clusterSpec, Capacity capacity) { List<HostSpec> preparedNodes = provisioner.prepare(applicationId, clusterSpec, capacity, (level, message) -> System.out.println(level + ": " + message) ); - NestedTransaction transaction = new NestedTransaction().add(new CuratorTransaction(curator)); - provisioner.activate(transaction, applicationId, Set.copyOf(preparedNodes)); - transaction.commit(); + try (var lock = provisioner.lock(applicationId)) { + NestedTransaction transaction = new NestedTransaction().add(new CuratorTransaction(curator)); + provisioner.activate(transaction, Set.copyOf(preparedNodes), lock); + transaction.commit(); + } return this; } diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/LoadBalancerExpirerTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/LoadBalancerExpirerTest.java index 4185ff2c25c..de0e5d4223a 100644 --- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/LoadBalancerExpirerTest.java +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/LoadBalancerExpirerTest.java @@ -6,7 +6,6 @@ import com.yahoo.config.provision.ApplicationId; import com.yahoo.config.provision.ClusterSpec; import com.yahoo.config.provision.HostSpec; import com.yahoo.config.provision.NodeResources; -import com.yahoo.transaction.NestedTransaction; import com.yahoo.vespa.hosted.provision.lb.LoadBalancer; import com.yahoo.vespa.hosted.provision.lb.LoadBalancerId; import com.yahoo.vespa.hosted.provision.node.Agent; @@ -55,7 +54,7 @@ public class LoadBalancerExpirerTest { assertEquals(3, loadBalancers.get().size()); // Remove one application deactivates load balancers for that application - removeApplication(app1); + tester.remove(app1); assertSame(LoadBalancer.State.inactive, loadBalancers.get().get(lb1).state()); assertNotSame(LoadBalancer.State.inactive, loadBalancers.get().get(lb2).state()); @@ -146,12 +145,6 @@ public class LoadBalancerExpirerTest { Agent.system, this.getClass().getSimpleName()); } - private void removeApplication(ApplicationId application) { - NestedTransaction transaction = new NestedTransaction(); - tester.provisioner().remove(transaction, application); - transaction.commit(); - } - private void deployApplication(ApplicationId application, ClusterSpec.Id... clusters) { deployApplication(application, true, clusters); } diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/NodeFailTester.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/NodeFailTester.java index 84b04e040e7..250555ae4fb 100644 --- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/NodeFailTester.java +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/NodeFailTester.java @@ -265,9 +265,11 @@ public class NodeFailTester { private void activate(ApplicationId applicationId, ClusterSpec cluster, Capacity capacity) { List<HostSpec> hosts = provisioner.prepare(applicationId, cluster, capacity, null); - NestedTransaction transaction = new NestedTransaction().add(new CuratorTransaction(curator)); - provisioner.activate(transaction, applicationId, hosts); - transaction.commit(); + try (var lock = provisioner.lock(applicationId)) { + NestedTransaction transaction = new NestedTransaction().add(new CuratorTransaction(curator)); + provisioner.activate(transaction, hosts, lock); + transaction.commit(); + } } /** Returns the node with the highest membership index from the given set of allocated nodes */ diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/RebalancerTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/RebalancerTest.java index 65b79c2df04..390c1213718 100644 --- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/RebalancerTest.java +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/RebalancerTest.java @@ -9,6 +9,7 @@ import com.yahoo.config.provision.Environment; import com.yahoo.config.provision.Flavor; import com.yahoo.config.provision.NodeResources; import com.yahoo.config.provision.NodeType; +import com.yahoo.config.provision.ProvisionLock; import com.yahoo.config.provision.RegionName; import com.yahoo.config.provision.Zone; import com.yahoo.config.provisioning.FlavorsConfig; @@ -81,7 +82,7 @@ public class RebalancerTest { // --- Making the system stable enables rebalancing NestedTransaction tx = new NestedTransaction(); - tester.nodeRepository().deactivate(List.of(cpuSkewedNode), tx); + tester.nodeRepository().deactivate(List.of(cpuSkewedNode), tx, new ProvisionLock(cpuApp, () -> {})); tx.commit(); // ... if activation fails when trying, we clean up the state diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/RetiredExpirerTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/RetiredExpirerTest.java index 6062b501513..8de769ae113 100644 --- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/RetiredExpirerTest.java +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/RetiredExpirerTest.java @@ -183,9 +183,11 @@ public class RetiredExpirerTest { private void activate(ApplicationId applicationId, ClusterSpec cluster, int nodes, int groups, NodeRepositoryProvisioner provisioner) { List<HostSpec> hosts = provisioner.prepare(applicationId, cluster, Capacity.from(new ClusterResources(nodes, groups, nodeResources)), null); - NestedTransaction transaction = new NestedTransaction().add(new CuratorTransaction(curator)); - provisioner.activate(transaction, applicationId, hosts); - transaction.commit(); + try (var lock = provisioner.lock(applicationId)) { + NestedTransaction transaction = new NestedTransaction().add(new CuratorTransaction(curator)); + provisioner.activate(transaction, hosts, lock); + transaction.commit(); + } } private void createReadyNodes(int count, NodeResources nodeResources, NodeRepository nodeRepository) { diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/InfraDeployerImplTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/InfraDeployerImplTest.java index edbbb3ed2e3..4fae7cf0ab9 100644 --- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/InfraDeployerImplTest.java +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/InfraDeployerImplTest.java @@ -7,6 +7,7 @@ import com.yahoo.config.provision.ClusterSpec; import com.yahoo.config.provision.HostName; import com.yahoo.config.provision.HostSpec; import com.yahoo.config.provision.NodeType; +import com.yahoo.config.provision.ProvisionLock; import com.yahoo.config.provision.Provisioner; import com.yahoo.config.provision.Zone; import com.yahoo.vespa.flags.InMemoryFlagSource; @@ -24,7 +25,9 @@ import com.yahoo.vespa.service.monitor.InfraApplicationApi; import org.junit.Test; import org.junit.runner.RunWith; import org.junit.runners.Parameterized; +import org.mockito.ArgumentMatcher; +import java.util.Collection; import java.util.List; import java.util.Optional; import java.util.Set; @@ -98,10 +101,14 @@ public class InfraDeployerImplTest { verify(duperModelInfraApi, never()).infraApplicationActivated(any(), any()); if (applicationIsActive) { verify(duperModelInfraApi).infraApplicationRemoved(application.getApplicationId()); - verify(provisioner).remove(any(), eq(application.getApplicationId())); + ArgumentMatcher<ProvisionLock> lockMatcher = lock -> { + assertEquals(application.getApplicationId(), lock.application()); + return true; + }; + verify(provisioner).remove(any(), argThat(lockMatcher)); verify(duperModelInfraApi).infraApplicationRemoved(eq(application.getApplicationId())); } else { - verify(provisioner, never()).remove(any(), any()); + verify(provisioner, never()).remove(any(), any(ProvisionLock.class)); verify(duperModelInfraApi, never()).infraApplicationRemoved(any()); } } @@ -129,10 +136,15 @@ public class InfraDeployerImplTest { private void verifyActivated(String... hostnames) { verify(duperModelInfraApi).infraApplicationActivated( eq(application.getApplicationId()), eq(Stream.of(hostnames).map(HostName::from).collect(Collectors.toList()))); - verify(provisioner).activate(any(), eq(application.getApplicationId()), argThat(hostSpecs -> { + ArgumentMatcher<ProvisionLock> lockMatcher = lock -> { + assertEquals(application.getApplicationId(), lock.application()); + return true; + }; + ArgumentMatcher<Collection<HostSpec>> hostsMatcher = hostSpecs -> { assertEquals(Set.of(hostnames), hostSpecs.stream().map(HostSpec::hostname).collect(Collectors.toSet())); return true; - })); + }; + verify(provisioner).activate(any(), argThat(hostsMatcher), argThat(lockMatcher)); } private Node addNode(int id, Node.State state, Optional<Version> wantedVespaVersion) { diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/LoadBalancerProvisionerTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/LoadBalancerProvisionerTest.java index 3d784c403f3..735ef2f0fcb 100644 --- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/LoadBalancerProvisionerTest.java +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/LoadBalancerProvisionerTest.java @@ -6,12 +6,10 @@ import com.yahoo.config.provision.ApplicationId; import com.yahoo.config.provision.Capacity; import com.yahoo.config.provision.ClusterResources; import com.yahoo.config.provision.ClusterSpec; -import com.yahoo.config.provision.Flavor; import com.yahoo.config.provision.HostName; import com.yahoo.config.provision.HostSpec; import com.yahoo.config.provision.NodeResources; import com.yahoo.config.provision.NodeType; -import com.yahoo.transaction.NestedTransaction; import com.yahoo.vespa.flags.Flags; import com.yahoo.vespa.flags.InMemoryFlagSource; import com.yahoo.vespa.hosted.provision.Node; @@ -22,7 +20,6 @@ import com.yahoo.vespa.hosted.provision.node.Agent; import com.yahoo.vespa.hosted.provision.node.IP; import org.junit.Test; -import java.util.ArrayList; import java.util.Collection; import java.util.Collections; import java.util.LinkedHashSet; @@ -136,9 +133,7 @@ public class LoadBalancerProvisionerTest { .get()); // Application is removed, nodes and load balancer are deactivated - NestedTransaction removeTransaction = new NestedTransaction(); - tester.provisioner().remove(removeTransaction, app1); - removeTransaction.commit(); + tester.remove(app1); dirtyNodesOf(app1); assertTrue("No nodes are allocated to " + app1, tester.nodeRepository().getNodes(app1, Node.State.reserved, Node.State.active).isEmpty()); assertEquals(2, lbApp1.get().size()); @@ -170,9 +165,7 @@ public class LoadBalancerProvisionerTest { assertFalse("Load balancer is reconfigured with reals", tester.loadBalancerService().instances().get(lb.get().id()).reals().isEmpty()); // Application is removed, nodes are deleted and load balancer is deactivated - NestedTransaction removeTransaction = new NestedTransaction(); - tester.provisioner().remove(removeTransaction, app1); - removeTransaction.commit(); + tester.remove(app1); tester.nodeRepository().database().removeNodes(tester.nodeRepository().getNodes(NodeType.tenant)); assertTrue("Nodes are deleted", tester.nodeRepository().getNodes(NodeType.tenant).isEmpty()); assertSame("Load balancer is deactivated", LoadBalancer.State.inactive, lb.get().state()); @@ -260,21 +253,6 @@ public class LoadBalancerProvisionerTest { return allNodes; } - private void makeDynamicDockerNodes(int n, NodeType nodeType) { - tester.makeReadyHosts(n, new NodeResources(1, 4, 10, 0.3)); - List<Node> nodes = new ArrayList<>(n); - for (int i = 1; i <= n; i++) { - var node = Node.createDockerNode(Set.of(), "vnode" + i, - tester.nodeRepository().getNodes(NodeType.host).get(n - 1).hostname(), - new NodeResources(1, 4, 10, 0.3), - nodeType); - nodes.add(node); - } - nodes = tester.nodeRepository().database().addNodesInState(nodes, Node.State.reserved, Agent.system); - nodes = tester.nodeRepository().setDirty(nodes, Agent.system, getClass().getSimpleName()); - tester.nodeRepository().setReady(nodes, Agent.system, getClass().getSimpleName()); - } - private void assignIps(List<Node> nodes) { try (var lock = tester.nodeRepository().lockUnallocated()) { for (int i = 0; i < nodes.size(); i++) { diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/ProvisioningTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/ProvisioningTest.java index 1c03bbc6a56..567ec78576c 100644 --- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/ProvisioningTest.java +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/ProvisioningTest.java @@ -98,9 +98,7 @@ public class ProvisioningTest { assertEquals(5, tester.getNodes(application1, Node.State.inactive).size()); // delete app - NestedTransaction removeTransaction = new NestedTransaction(); - tester.provisioner().remove(removeTransaction, application1); - removeTransaction.commit(); + tester.remove(application1); assertEquals(tester.toHostNames(state1.allHosts), tester.toHostNames(tester.nodeRepository().getNodes(application1, Node.State.inactive))); assertEquals(0, tester.getNodes(application1, Node.State.active).size()); @@ -250,9 +248,7 @@ public class ProvisioningTest { SystemState state7 = prepare(application1, 8, 2, 2, 2, defaultResources, tester); // delete app - NestedTransaction removeTransaction = new NestedTransaction(); - tester.provisioner().remove(removeTransaction, application1); - removeTransaction.commit(); + tester.remove(application1); assertEquals(0, tester.getNodes(application1, Node.State.active).size()); assertEquals(0, tester.getNodes(application1, Node.State.reserved).size()); } @@ -600,9 +596,7 @@ public class ProvisioningTest { SystemState state = prepare(application, 2, 2, 3, 3, defaultResources, tester); // Simulate expiry - NestedTransaction deactivateTransaction = new NestedTransaction(); - tester.nodeRepository().deactivate(application, deactivateTransaction); - deactivateTransaction.commit(); + tester.deactivate(application); try { tester.activate(application, state.allHosts); diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/ProvisioningTester.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/ProvisioningTester.java index a176b28182e..6b7b979e083 100644 --- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/ProvisioningTester.java +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/ProvisioningTester.java @@ -17,6 +17,7 @@ import com.yahoo.config.provision.NodeResources; import com.yahoo.config.provision.NodeResources.DiskSpeed; import com.yahoo.config.provision.NodeResources.StorageType; import com.yahoo.config.provision.NodeType; +import com.yahoo.config.provision.ProvisionLock; import com.yahoo.config.provision.ProvisionLogger; import com.yahoo.config.provision.TenantName; import com.yahoo.config.provision.Zone; @@ -203,14 +204,25 @@ public class ProvisioningTester { } public Collection<HostSpec> activate(ApplicationId application, Collection<HostSpec> hosts) { - NestedTransaction transaction = new NestedTransaction(); - transaction.add(new CuratorTransaction(curator)); - provisioner.activate(transaction, application, hosts); - transaction.commit(); + try (var lock = provisioner.lock(application)) { + NestedTransaction transaction = new NestedTransaction(); + transaction.add(new CuratorTransaction(curator)); + provisioner.activate(transaction, hosts, lock); + transaction.commit(); + } assertEquals(toHostNames(hosts), toHostNames(nodeRepository.getNodes(application, Node.State.active))); return hosts; } + /** Remove all resources allocated to application */ + public void remove(ApplicationId application) { + try (var lock = provisioner.lock(application)) { + NestedTransaction transaction = new NestedTransaction(); + provisioner.remove(transaction, lock); + transaction.commit(); + } + } + public void prepareAndActivateInfraApplication(ApplicationId application, NodeType nodeType, Version version) { ClusterSpec cluster = ClusterSpec.request(ClusterSpec.Type.container, ClusterSpec.Id.from(nodeType.toString())).vespaVersion(version).build(); Capacity capacity = Capacity.fromRequiredNodeType(nodeType); @@ -223,9 +235,11 @@ public class ProvisioningTester { } public void deactivate(ApplicationId applicationId) { - NestedTransaction deactivateTransaction = new NestedTransaction(); - nodeRepository.deactivate(applicationId, deactivateTransaction); - deactivateTransaction.commit(); + try (var lock = nodeRepository.lock(applicationId)) { + NestedTransaction deactivateTransaction = new NestedTransaction(); + nodeRepository.deactivate(deactivateTransaction, new ProvisionLock(applicationId, lock)); + deactivateTransaction.commit(); + } } public Collection<String> toHostNames(Collection<HostSpec> hosts) { |