Merge pull request #24632 from vespa-engine/mpolden/account-change

Prevent reusing resources across accounts

8 files changed, 166 insertions, 87 deletions

diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/lb/LoadBalancer.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/lb/LoadBalancer.java
index 4befa14942a..ad97ee887c9 100644
--- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/lb/LoadBalancer.java
+++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/lb/LoadBalancer.java
@@ -68,7 +68,7 @@ public class LoadBalancer {
 
     public enum State {
 
-        /** This load balancer has been provisioned and reserved for an application */
+        /** The load balancer has been provisioned and reserved for an application */
         reserved,
 
         /**
@@ -81,6 +81,9 @@ public class LoadBalancer {
         /** The load balancer is in active use by an application */
         active,
 
+        /** The load balancer should be removed immediately by {@link LoadBalancerExpirer} */
+        removable,
+
     }
 
 }
diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/LoadBalancerExpirer.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/LoadBalancerExpirer.java
index 5314d36a3c2..f0abe184614 100644
--- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/LoadBalancerExpirer.java
+++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/LoadBalancerExpirer.java
@@ -48,7 +48,7 @@ public class LoadBalancerExpirer extends NodeRepositoryMaintainer {
     private final LoadBalancerService service;
     private final CuratorDatabaseClient db;
 
-    LoadBalancerExpirer(NodeRepository nodeRepository, Duration interval, LoadBalancerService service, Metric metric) {
+    public LoadBalancerExpirer(NodeRepository nodeRepository, Duration interval, LoadBalancerService service, Metric metric) {
         super(nodeRepository, interval, metric);
         this.service = Objects.requireNonNull(service, "service must be non-null");
         this.db = nodeRepository.database();
@@ -57,26 +57,24 @@ public class LoadBalancerExpirer extends NodeRepositoryMaintainer {
     @Override
     protected double maintain() {
         expireReserved();
-        return ( removeInactive() + pruneReals() ) / 2;
+        return (deprovisionRemovable() + pruneReals()) / 2;
     }
 
     /** Move reserved load balancer that have expired to inactive */
     private void expireReserved() {
         Instant now = nodeRepository().clock().instant();
         Instant expiry = now.minus(reservedExpiry);
-        patchLoadBalancers(lb -> lb.state() == State.reserved && lb.changedAt().isBefore(expiry),
+        patchLoadBalancers(lb -> canDeactivate(lb, expiry),
                            lb -> db.writeLoadBalancer(lb.with(State.inactive, now), lb.state()));
     }
 
-    /** Deprovision inactive load balancers that have expired */
-    private double removeInactive() {
+    /** Deprovision removable load balancers */
+    private double deprovisionRemovable() {
         MutableInteger attempts = new MutableInteger(0);
         var failed = new ArrayList<LoadBalancerId>();
         var lastException = new AtomicReference<Exception>();
         var expiry = nodeRepository().clock().instant().minus(inactiveExpiry);
-        patchLoadBalancers(lb -> lb.state() == State.inactive &&
-                                 lb.changedAt().isBefore(expiry) &&
-                                 allocatedNodes(lb.id()).isEmpty(), lb -> {
+        patchLoadBalancers(lb -> canRemove(lb, expiry), lb -> {
             try {
                 attempts.add(1);
                 log.log(Level.INFO, () -> "Removing expired inactive " + lb.id());
@@ -145,6 +143,16 @@ public class LoadBalancerExpirer extends NodeRepositoryMaintainer {
         }
     }
 
+    private boolean canRemove(LoadBalancer lb, Instant expiry) {
+        return lb.state() == State.removable || (lb.state() == State.inactive &&
+                                                 lb.changedAt().isBefore(expiry) &&
+                                                 allocatedNodes(lb.id()).isEmpty());
+    }
+
+    private boolean canDeactivate(LoadBalancer lb, Instant expiry) {
+        return lb.state() == State.reserved && lb.changedAt().isBefore(expiry);
+    }
+
     private List<Node> allocatedNodes(LoadBalancerId loadBalancer) {
         return nodeRepository().nodes()
                 .list(Node.State.active, Node.State.inactive, Node.State.reserved)
diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/persistence/CuratorDatabaseClient.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/persistence/CuratorDatabaseClient.java
index a5ebc6b3efc..e5f02b38d29 100644
--- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/persistence/CuratorDatabaseClient.java
+++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/persistence/CuratorDatabaseClient.java
@@ -87,7 +87,7 @@ public class CuratorDatabaseClient {
     }
 
     public List<String> cluster() {
-        return db.cluster().stream().map(HostName::value).collect(Collectors.toUnmodifiableList());
+        return db.cluster().stream().map(HostName::value).toList();
     }
 
     private void initZK() {
@@ -162,7 +162,7 @@ public class CuratorDatabaseClient {
     }
 
     /**
-     * Writes the given nodes to the given state (whether or not they are already in this state or another),
+     * Writes the given nodes to the given state (whether they are already in this state or another),
      * and returns a copy of the incoming nodes in their persisted state.
      *
      * @param  toState the state to write the nodes to
@@ -305,19 +305,18 @@ public class CuratorDatabaseClient {
     }
 
     private String toDir(Node.State state) {
-        switch (state) {
-            case active: return "allocated"; // legacy name
-            case dirty: return "dirty";
-            case failed: return "failed";
-            case inactive: return "deallocated"; // legacy name
-            case parked : return "parked";
-            case provisioned: return "provisioned";
-            case ready: return "ready";
-            case reserved: return "reserved";
-            case deprovisioned: return "deprovisioned";
-            case breakfixed: return "breakfixed";
-            default: throw new RuntimeException("Node state " + state + " does not map to a directory name");
-        }
+        return switch (state) {
+            case active -> "allocated"; // legacy name
+            case dirty -> "dirty";
+            case failed -> "failed";
+            case inactive -> "deallocated"; // legacy name
+            case parked -> "parked";
+            case provisioned -> "provisioned";
+            case ready -> "ready";
+            case reserved -> "reserved";
+            case deprovisioned -> "deprovisioned";
+            case breakfixed -> "breakfixed";
+        };
     }
 
     /** Acquires the single cluster-global, reentrant lock for all non-active nodes */
@@ -513,7 +512,7 @@ public class CuratorDatabaseClient {
         return db.getChildren(loadBalancersPath).stream()
                  .map(LoadBalancerId::fromSerializedForm)
                  .filter(predicate)
-                 .collect(Collectors.toUnmodifiableList());
+                 .toList();
     }
 
     /** Returns a given number of unique provision indices */
@@ -524,7 +523,7 @@ public class CuratorDatabaseClient {
         int firstIndex = (int) provisionIndexCounter.add(count) - count;
         return IntStream.range(0, count)
                         .mapToObj(i -> firstIndex + i)
-                        .collect(Collectors.toList());
+                        .toList();
     }
 
     public CacheStats cacheStats() {
diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/persistence/LoadBalancerSerializer.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/persistence/LoadBalancerSerializer.java
index cfc918ce57b..4078a3e80d7 100644
--- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/persistence/LoadBalancerSerializer.java
+++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/persistence/LoadBalancerSerializer.java
@@ -120,22 +120,22 @@ public class LoadBalancerSerializer {
     }
 
     private static String asString(LoadBalancer.State state) {
-        switch (state) {
-            case active: return "active";
-            case inactive: return "inactive";
-            case reserved: return "reserved";
-            default: throw new IllegalArgumentException("No serialization defined for state enum '" + state + "'");
-        }
+        return switch (state) {
+            case active -> "active";
+            case inactive -> "inactive";
+            case reserved -> "reserved";
+            case removable -> "removable";
+        };
     }
 
     private static LoadBalancer.State stateFromString(String state) {
-        switch (state) {
-            case "active": return LoadBalancer.State.active;
-            case "inactive": return LoadBalancer.State.inactive;
-            case "reserved": return LoadBalancer.State.reserved;
-            case "removable": return LoadBalancer.State.inactive; // Read future value
-            default: throw new IllegalArgumentException("No serialization defined for state string '" + state + "'");
-        }
+        return switch (state) {
+            case "active" -> LoadBalancer.State.active;
+            case "inactive" -> LoadBalancer.State.inactive;
+            case "reserved" -> LoadBalancer.State.reserved;
+            case "removable" -> LoadBalancer.State.removable;
+            default -> throw new IllegalArgumentException("No serialization defined for state string '" + state + "'");
+        };
     }
 
 }
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 15c9134d10f..0763d19bae3 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
@@ -48,6 +48,8 @@ import java.util.stream.Collectors;
 // 1) (new) -> reserved -> active
 // 2) active | reserved -> inactive
 // 3) inactive -> active | (removed)
+// 4) active | reserved | inactive -> removable
+// 5) removable -> (removed)
 public class LoadBalancerProvisioner {
 
     private static final Logger log = Logger.getLogger(LoadBalancerProvisioner.class.getName());
@@ -161,7 +163,7 @@ public class LoadBalancerProvisioner {
         return db.readLoadBalancerIds().stream()
                  .filter(id -> id.application().tenant().equals(tenant) &&
                                id.application().application().equals(application))
-                 .collect(Collectors.toUnmodifiableList());
+                 .toList();
     }
 
     /** Require that load balancer IDs do not clash. This prevents name clashing when compacting endpoint DNS names */
@@ -190,9 +192,14 @@ public class LoadBalancerProvisioner {
             newLoadBalancer = loadBalancer.get().with(instance);
             fromState = newLoadBalancer.state();
         }
-        // Always store load balancer so that LoadBalancerExpirer can expire partially provisioned load balancers
+        if (!inAccount(cloudAccount, newLoadBalancer)) {
+            // We have a load balancer, but in the wrong account. Load balancer must be removed before we can
+            // provision a new one in the wanted account
+            newLoadBalancer = newLoadBalancer.with(LoadBalancer.State.removable, now);
+        }
+        // Always store the load balancer. LoadBalancerExpirer will remove unwanted ones
         db.writeLoadBalancer(newLoadBalancer, fromState);
-        requireInstance(id, instance);
+        requireInstance(id, instance, cloudAccount);
     }
 
     private void activate(ApplicationTransaction transaction, ClusterSpec.Id cluster, NodeList nodes) {
@@ -206,7 +213,7 @@ public class LoadBalancerProvisioner {
         LoadBalancer.State state = instance.isPresent() ? LoadBalancer.State.active : loadBalancer.get().state();
         LoadBalancer newLoadBalancer = loadBalancer.get().with(instance).with(state, now);
         db.writeLoadBalancers(List.of(newLoadBalancer), loadBalancer.get().state(), transaction.nested());
-        requireInstance(id, instance);
+        requireInstance(id, instance, loadBalancer.get().instance().get().cloudAccount());
     }
 
     /** Provision or reconfigure a load balancer instance, if necessary */
@@ -276,6 +283,11 @@ public class LoadBalancerProvisioner {
         return ids;
     }
 
+    /** Returns whether load balancer is provisioned in given account */
+    private static boolean inAccount(CloudAccount cloudAccount, LoadBalancer loadBalancer) {
+        return loadBalancer.instance().isEmpty() || loadBalancer.instance().get().cloudAccount().equals(cloudAccount);
+    }
+
     /** Returns whether load balancer has given reals */
     private static boolean hasReals(Optional<LoadBalancer> loadBalancer, Set<Real> reals) {
         if (loadBalancer.isEmpty()) return false;
@@ -293,21 +305,19 @@ public class LoadBalancerProvisioner {
         Set<String> reachable = new LinkedHashSet<>(node.ipConfig().primary());
         // Remove addresses unreachable by the load balancer service
         switch (service.protocol()) {
-            case ipv4:
-                reachable.removeIf(IP::isV6);
-                break;
-            case ipv6:
-                reachable.removeIf(IP::isV4);
-                break;
+            case ipv4 -> reachable.removeIf(IP::isV6);
+            case ipv6 -> reachable.removeIf(IP::isV4);
         }
         return reachable;
     }
 
-    private static void requireInstance(LoadBalancerId id, Optional<LoadBalancerInstance> instance) {
+    private static void requireInstance(LoadBalancerId id, Optional<LoadBalancerInstance> instance, CloudAccount cloudAccount) {
         if (instance.isEmpty()) {
             // Signal that load balancer is not ready yet
-            throw new LoadBalancerServiceException("Could not (re)configure " + id + ". The operation will be retried on next deployment",
-                                                   null);
+            throw new LoadBalancerServiceException("Could not (re)configure " + id + ". The operation will be retried on next deployment");
+        }
+        if (!instance.get().cloudAccount().equals(cloudAccount)) {
+            throw new LoadBalancerServiceException("Could not (re)configure " + id + " due to change in cloud account. The operation will be retried on next deployment");
         }
     }
 
diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/NodeAllocation.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/NodeAllocation.java
index 67dbefcdfcd..fe634a77997 100644
--- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/NodeAllocation.java
+++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/NodeAllocation.java
@@ -104,34 +104,34 @@ class NodeAllocation {
     /**
      * Offer some nodes to this. The nodes may have an allocation to a different application or cluster,
      * an allocation to this cluster, or no current allocation (in which case one is assigned).
-     * 
+     *
      * Note that if unallocated nodes are offered before allocated nodes, this will unnecessarily
      * reject allocated nodes due to index duplicates.
      *
      * @param candidates the nodes which are potentially on offer. These may belong to a different application etc.
-     * @return the subset of offeredNodes which was accepted, with the correct allocation assigned
      */
-    List<Node> offer(List<NodeCandidate> candidates) {
-        List<Node> accepted = new ArrayList<>();
+    void offer(List<NodeCandidate> candidates) {
         for (NodeCandidate candidate : candidates) {
             if (candidate.allocation().isPresent()) {
                 Allocation allocation = candidate.allocation().get();
                 ClusterMembership membership = allocation.membership();
                 if ( ! allocation.owner().equals(application)) continue; // wrong application
                 if ( ! membership.cluster().satisfies(cluster)) continue; // wrong cluster id/type
-                if ((! candidate.isSurplus || saturated()) && ! membership.cluster().group().equals(cluster.group())) continue; // wrong group and we can't or have no reason to change it
+                if ((! candidate.isSurplus || saturated()) && ! membership.cluster().group().equals(cluster.group())) continue; // wrong group, and we can't or have no reason to change it
                 if ( candidate.state() == Node.State.active && allocation.removable()) continue; // don't accept; causes removal
                 if ( candidate.state() == Node.State.active && candidate.wantToFail()) continue; // don't accept; causes failing
                 if ( indexes.contains(membership.index())) continue; // duplicate index (just to be sure)
                 if ( requiredHostFlavor.isPresent() && ! candidate.parent.map(node -> node.flavor().name()).equals(requiredHostFlavor)) continue;
+                if ( candidate.parent.isPresent() && ! candidate.parent.get().cloudAccount().equals(requestedNodes.cloudAccount())) continue; // wrong account
 
                 boolean resizeable = requestedNodes.considerRetiring() && candidate.isResizable;
                 boolean acceptToRetire = acceptToRetire(candidate);
 
                 if ((! saturated() && hasCompatibleFlavor(candidate) && requestedNodes.acceptable(candidate)) || acceptToRetire) {
                     candidate = candidate.withNode();
-                    if (candidate.isValid())
-                        accepted.add(acceptNode(candidate, shouldRetire(candidate, candidates), resizeable));
+                    if (candidate.isValid()) {
+                        acceptNode(candidate, shouldRetire(candidate, candidates), resizeable);
+                    }
                 }
             }
             else if (! saturated() && hasCompatibleFlavor(candidate)) {
@@ -154,12 +154,11 @@ class NodeAllocation {
                                                ClusterMembership.from(cluster, nextIndex.get()),
                                                requestedNodes.resources().orElse(candidate.resources()),
                                                nodeRepository.clock().instant());
-                if (candidate.isValid())
-                    accepted.add(acceptNode(candidate, Retirement.none, false));
+                if (candidate.isValid()) {
+                    acceptNode(candidate, Retirement.none, false);
+                }
             }
         }
-
-        return accepted;
     }
 
     /** Returns the cause of retirement for given candidate */
diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/DynamicProvisioningMaintainerTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/DynamicProvisioningMaintainerTest.java
index de4df1992ee..d28d0321e6c 100644
--- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/DynamicProvisioningMaintainerTest.java
+++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/DynamicProvisioningMaintainerTest.java
@@ -44,6 +44,7 @@ import org.junit.Test;
 
 import java.time.Duration;
 import java.time.Instant;
+import java.util.Collections;
 import java.util.Comparator;
 import java.util.List;
 import java.util.Optional;
@@ -437,16 +438,15 @@ public class DynamicProvisioningMaintainerTest {
         final ApplicationId hostApp;
         final ApplicationId configSrvApp;
         switch (hostType) {
-            case confighost:
+            case confighost -> {
                 hostApp = new ConfigServerHostApplication().getApplicationId();
                 configSrvApp = new ConfigServerApplication().getApplicationId();
-                break;
-            case controllerhost:
+            }
+            case controllerhost -> {
                 hostApp = new ControllerHostApplication().getApplicationId();
                 configSrvApp = new ControllerApplication().getApplicationId();
-                break;
-            default:
-                throw new IllegalArgumentException("Unexpected config server host like node type: " + hostType);
+            }
+            default -> throw new IllegalArgumentException("Unexpected config server host like node type: " + hostType);
         }
 
         Cloud cloud = Cloud.builder().dynamicProvisioning(true).build();
@@ -551,23 +551,32 @@ public class DynamicProvisioningMaintainerTest {
         // Deployment requests capacity in custom account
         ClusterSpec spec = ProvisioningTester.contentClusterSpec();
         ClusterResources resources = new ClusterResources(2, 1, new NodeResources(16, 24, 100, 1));
-        CloudAccount cloudAccount = new CloudAccount("012345678912");
-        Capacity capacity = Capacity.from(resources, resources, false, true, Optional.of(cloudAccount));
-        List<HostSpec> prepared = provisioningTester.prepare(applicationId, spec, capacity);
+        CloudAccount cloudAccount0 = new CloudAccount("000000000000");
+        Capacity capacity0 = Capacity.from(resources, resources, false, true, Optional.of(cloudAccount0));
+        List<HostSpec> prepared = provisioningTester.prepare(applicationId, spec, capacity0);
 
         // Hosts are provisioned in requested account
-        tester.maintainer.maintain();
-        List<ProvisionedHost> newHosts = tester.hostProvisioner.provisionedHosts();
-        assertEquals(2, newHosts.size());
-        assertTrue(newHosts.stream().allMatch(host -> host.cloudAccount().equals(cloudAccount)));
-        for (var host : newHosts) {
-            provisioningTester.nodeRepository().nodes().setReady(host.hostHostname(), Agent.operator, getClass().getSimpleName());
-        }
-        provisioningTester.prepareAndActivateInfraApplication(DynamicProvisioningTester.tenantHostApp, NodeType.host);
-        NodeList activeHosts = provisioningTester.nodeRepository().nodes().list(Node.State.active).nodeType(NodeType.host);
-        assertEquals(2, activeHosts.size());
-        assertTrue(activeHosts.stream().allMatch(host -> host.cloudAccount().equals(cloudAccount)));
+        provisionHostsIn(cloudAccount0, 2, tester);
+        assertEquals(2, provisioningTester.activate(applicationId, prepared).size());
+        NodeList allNodes0 = tester.nodeRepository.nodes().list();
+
+        // Redeployment in different account provisions a new set of hosts
+        CloudAccount cloudAccount1 = new CloudAccount("100000000000");
+        Capacity capacity1 = Capacity.from(resources, resources, false, true, Optional.of(cloudAccount1));
+        prepared = provisioningTester.prepare(applicationId, spec, capacity1);
+        provisionHostsIn(cloudAccount1, 2, tester);
         assertEquals(2, provisioningTester.activate(applicationId, prepared).size());
+
+        // No nodes or hosts are reused
+        NodeList allNodes1 = tester.nodeRepository.nodes().list();
+        NodeList activeNodes0 = allNodes0.state(Node.State.active).owner(applicationId);
+        NodeList activeNodes1 = allNodes1.state(Node.State.active).owner(applicationId);
+        assertTrue("New set of nodes is activated",
+                   Collections.disjoint(activeNodes0.asList(),
+                                        activeNodes1.asList()));
+        assertTrue("New set of parents are used",
+                   Collections.disjoint(allNodes0.parentsOf(activeNodes0).asList(),
+                                        allNodes1.parentsOf(activeNodes1).asList()));
     }
 
     @Test
@@ -627,6 +636,25 @@ public class DynamicProvisioningMaintainerTest {
         assertEquals(0, tester.nodeRepository.nodes().list().rebuilding(true).size());
     }
 
+    private void provisionHostsIn(CloudAccount cloudAccount, int count, DynamicProvisioningTester tester) {
+        tester.maintainer.maintain();
+        List<String> provisionedHostnames = tester.hostProvisioner.provisionedHosts().stream()
+                                                                  .filter(host -> host.cloudAccount().equals(cloudAccount))
+                                                                  .map(ProvisionedHost::hostHostname)
+                                                                  .toList();
+        assertEquals(count, provisionedHostnames.size());
+        for (var hostname : provisionedHostnames) {
+            tester.provisioningTester.nodeRepository().nodes().setReady(hostname, Agent.operator, getClass().getSimpleName());
+        }
+        tester.provisioningTester.prepareAndActivateInfraApplication(DynamicProvisioningTester.tenantHostApp, NodeType.host);
+        NodeList activeHosts = tester.provisioningTester.nodeRepository().nodes()
+                                                        .list(Node.State.active)
+                                                        .nodeType(NodeType.host)
+                                                        .matching(host -> provisionedHostnames.contains(host.hostname()));
+        assertTrue(activeHosts.stream().allMatch(host -> host.cloudAccount().equals(cloudAccount)));
+        assertEquals(count, activeHosts.size());
+    }
+
     private void assertCfghost3IsActive(DynamicProvisioningTester tester) {
         assertEquals(5, tester.nodeRepository.nodes().list(Node.State.active).size());
         assertEquals(3, tester.nodeRepository.nodes().list(Node.State.active).nodeType(NodeType.confighost).size());
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 7706895ebca..d3a8ca2dcc1 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
@@ -20,10 +20,13 @@ import com.yahoo.vespa.hosted.provision.NodeList;
 import com.yahoo.vespa.hosted.provision.lb.LoadBalancer;
 import com.yahoo.vespa.hosted.provision.lb.LoadBalancerList;
 import com.yahoo.vespa.hosted.provision.lb.Real;
+import com.yahoo.vespa.hosted.provision.maintenance.LoadBalancerExpirer;
+import com.yahoo.vespa.hosted.provision.maintenance.TestMetric;
 import com.yahoo.vespa.hosted.provision.node.Agent;
 import com.yahoo.vespa.hosted.provision.node.IP;
 import org.junit.Test;
 
+import java.time.Duration;
 import java.util.Collections;
 import java.util.LinkedHashSet;
 import java.util.List;
@@ -313,12 +316,41 @@ public class LoadBalancerProvisionerTest {
     @Test
     public void load_balancer_with_custom_cloud_account() {
         ClusterResources resources = new ClusterResources(3, 1, nodeResources);
-        CloudAccount cloudAccount = new CloudAccount("012345678912");
-        Capacity capacity = Capacity.from(resources, resources, false, true, Optional.of(cloudAccount));
-        tester.activate(app1, prepare(app1, capacity, clusterRequest(ClusterSpec.Type.container, ClusterSpec.Id.from("c1"))));
+        CloudAccount cloudAccount0 = CloudAccount.empty;
+        {
+            Capacity capacity = Capacity.from(resources, resources, false, true, Optional.of(cloudAccount0));
+            tester.activate(app1, prepare(app1, capacity, clusterRequest(ClusterSpec.Type.container, ClusterSpec.Id.from("c1"))));
+        }
         LoadBalancerList loadBalancers = tester.nodeRepository().loadBalancers().list();
         assertEquals(1, loadBalancers.size());
-        assertEquals(cloudAccount, loadBalancers.first().get().instance().get().cloudAccount());
+        assertEquals(cloudAccount0, loadBalancers.first().get().instance().get().cloudAccount());
+
+        // Changing account fails if there is an existing LB in the previous account.
+        CloudAccount cloudAccount1 = new CloudAccount("111111111111");
+        Capacity capacity = Capacity.from(resources, resources, false, true, Optional.of(cloudAccount1));
+        try {
+            prepare(app1, capacity, clusterRequest(ClusterSpec.Type.container, ClusterSpec.Id.from("c1")));
+            fail("Expected exception");
+        } catch (LoadBalancerServiceException e) {
+            assertTrue(e.getMessage().contains("due to change in cloud account"));
+        }
+
+        // Existing LB is removed
+        loadBalancers = tester.nodeRepository().loadBalancers().list();
+        assertEquals(1, loadBalancers.size());
+        assertSame(LoadBalancer.State.removable, loadBalancers.first().get().state());
+        LoadBalancerExpirer expirer = new LoadBalancerExpirer(tester.nodeRepository(),
+                                                              Duration.ofDays(1),
+                                                              tester.loadBalancerService(),
+                                                              new TestMetric());
+        expirer.run();
+        assertEquals(0, tester.nodeRepository().loadBalancers().list().in(LoadBalancer.State.removable).size());
+
+        // Next deployment provisions a new LB
+        tester.activate(app1, prepare(app1, capacity, clusterRequest(ClusterSpec.Type.container, ClusterSpec.Id.from("c1"))));
+        loadBalancers = tester.nodeRepository().loadBalancers().list();
+        assertEquals(1, loadBalancers.size());
+        assertEquals(cloudAccount1, loadBalancers.first().get().instance().get().cloudAccount());
     }
 
     private void assertReals(ApplicationId application, ClusterSpec.Id cluster, Node.State... states) {