diff options
author | Martin Polden <mpolden@mpolden.no> | 2020-05-18 15:35:53 +0200 |
---|---|---|
committer | GitHub <noreply@github.com> | 2020-05-18 15:35:53 +0200 |
commit | b9515323ea98bd91a631371397ef8b8f80c0dc91 (patch) | |
tree | 8b1adfd28829f2a6e4270d2efd37dd89645c685d | |
parent | 451d3944312a7cdf1e03e56ee6a37d5a5f51b166 (diff) | |
parent | a6aba4e9be5e4bc0118f1436ee1d06ca42e21cd5 (diff) |
Merge pull request #13283 from vespa-engine/mpolden/require-combined-id
Require combinedId for cluster type combined
4 files changed, 16 insertions, 22 deletions
diff --git a/config-provisioning/src/main/java/com/yahoo/config/provision/ClusterSpec.java b/config-provisioning/src/main/java/com/yahoo/config/provision/ClusterSpec.java index f7030535573..7c2c491ccf1 100644 --- a/config-provisioning/src/main/java/com/yahoo/config/provision/ClusterSpec.java +++ b/config-provisioning/src/main/java/com/yahoo/config/provision/ClusterSpec.java @@ -31,13 +31,14 @@ public final class ClusterSpec { this.groupId = groupId; this.vespaVersion = Objects.requireNonNull(vespaVersion); this.exclusive = exclusive; - // TODO(mpolden): Require combinedId to always be present for type combined after April 2020 - if (type != Type.combined && combinedId.isPresent()) { - throw new IllegalArgumentException("combinedId must be empty for cluster of type " + type); + if (type == Type.combined) { + if (combinedId.isEmpty()) throw new IllegalArgumentException("combinedId must be set for cluster of type " + type); + } else { + if (combinedId.isPresent()) throw new IllegalArgumentException("combinedId must be empty for cluster of type " + type); } this.combinedId = combinedId; if (dockerImageRepo.isPresent() && dockerImageRepo.get().tag().isPresent()) - throw new IllegalArgumentException("dockerimageRepo is not allowed to have a tag"); + throw new IllegalArgumentException("dockerImageRepo is not allowed to have a tag"); this.dockerImageRepo = dockerImageRepo; } diff --git a/config-provisioning/src/test/java/com/yahoo/config/provision/ClusterSpecTest.java b/config-provisioning/src/test/java/com/yahoo/config/provision/ClusterSpecTest.java index 40ed7500269..be4179e8b03 100644 --- a/config-provisioning/src/test/java/com/yahoo/config/provision/ClusterSpecTest.java +++ b/config-provisioning/src/test/java/com/yahoo/config/provision/ClusterSpecTest.java @@ -53,10 +53,13 @@ public class ClusterSpecTest { } private static ClusterSpec spec(ClusterSpec.Type type, String id) { - return ClusterSpec.specification(type, ClusterSpec.Id.from(id)) - .group(ClusterSpec.Group.from(1)) - .vespaVersion(Version.emptyVersion) - .build(); + ClusterSpec.Builder builder = ClusterSpec.specification(type, ClusterSpec.Id.from(id)) + .group(ClusterSpec.Group.from(1)) + .vespaVersion(Version.emptyVersion); + if (type == ClusterSpec.Type.combined) { + builder = builder.combinedId(Optional.of(ClusterSpec.Id.from("combined"))); + } + return builder.build(); } } 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 ad9d13355dc..5654b4000c7 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 @@ -200,19 +200,6 @@ public class LoadBalancerProvisionerTest { assertEquals(List.of(), tester.nodeRepository().loadBalancers(app1).asList()); } - // TODO(mpolden): Remove when ClusterSpec with combined type rejects empty combinedId - @Test - public void provision_load_balancer_combined_cluster_without_id() { - Supplier<List<LoadBalancer>> lbs = () -> tester.nodeRepository().loadBalancers(app1).asList(); - ClusterSpec.Id cluster = ClusterSpec.Id.from("foo"); - - var nodes = prepare(app1, clusterRequest(ClusterSpec.Type.combined, cluster)); - assertEquals(1, lbs.get().size()); - assertEquals("Prepare provisions load balancer with reserved nodes", 2, lbs.get().get(0).instance().reals().size()); - tester.activate(app1, nodes); - assertSame(LoadBalancer.State.active, lbs.get().get(0).state()); - } - @Test public void provision_load_balancer_combined_cluster() { Supplier<List<LoadBalancer>> lbs = () -> tester.nodeRepository().loadBalancers(app1).asList(); 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 f8fe0d22e84..bafb52170dc 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 @@ -829,7 +829,10 @@ public class ProvisioningTest { Capacity.from(new ClusterResources(2, 1, defaultResources), false, false))); // Application is redeployed with cluster type combined - cluster = ClusterSpec.request(ClusterSpec.Type.combined, ClusterSpec.Id.from("music")).vespaVersion("1.2.3").build(); + cluster = ClusterSpec.request(ClusterSpec.Type.combined, ClusterSpec.Id.from("music")) + .vespaVersion("1.2.3") + .combinedId(Optional.of(ClusterSpec.Id.from("qrs"))) + .build(); var newNodes = tester.activate(application, tester.prepare(application, cluster, Capacity.from(new ClusterResources(2, 1, defaultResources), false, false))); |