diff options
6 files changed, 100 insertions, 175 deletions
diff --git a/config-model-api/src/main/java/com/yahoo/config/application/api/ValidationId.java b/config-model-api/src/main/java/com/yahoo/config/application/api/ValidationId.java index b4be99ad20b..71d07d494a0 100644 --- a/config-model-api/src/main/java/com/yahoo/config/application/api/ValidationId.java +++ b/config-model-api/src/main/java/com/yahoo/config/application/api/ValidationId.java @@ -13,9 +13,8 @@ public enum ValidationId { indexingChange("indexing-change"), // Changing what tokens are expected and stored in field indexes indexModeChange("indexing-mode-change"), // Changing the index mode (streaming, indexed, store-only) of documents fieldTypeChange("field-type-change"), // Field type changes - clusterSizeReduction("cluster-size-reduction"), // Large reductions in cluster size tensorTypeChange("tensor-type-change"), // Tensor type change - resourcesReduction("resources-reduction"), // Large reductions in node resources (> 50% of the current min resources) + resourcesReduction("resources-reduction"), // Large reductions in node resources (> 50% of the current max total resources) contentTypeRemoval("schema-removal"), // Removal of a schema (causes deletion of all documents) contentClusterRemoval("content-cluster-removal"), // Removal (or id change) of content clusters deploymentRemoval("deployment-removal"), // Removal of production zones from deployment.xml diff --git a/config-model/src/main/java/com/yahoo/vespa/model/application/validation/Validation.java b/config-model/src/main/java/com/yahoo/vespa/model/application/validation/Validation.java index 7b380cd5acf..c7a363010b7 100644 --- a/config-model/src/main/java/com/yahoo/vespa/model/application/validation/Validation.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/application/validation/Validation.java @@ -13,7 +13,7 @@ import com.yahoo.vespa.model.VespaModel; import com.yahoo.vespa.model.application.validation.change.CertificateRemovalChangeValidator; import com.yahoo.vespa.model.application.validation.change.ChangeValidator; import com.yahoo.vespa.model.application.validation.change.CloudAccountChangeValidator; -import com.yahoo.vespa.model.application.validation.change.ClusterSizeReductionValidator; +import com.yahoo.vespa.model.application.validation.change.ResourcesReductionValidator; import com.yahoo.vespa.model.application.validation.change.ConfigValueChangeValidator; import com.yahoo.vespa.model.application.validation.change.ContainerRestartValidator; import com.yahoo.vespa.model.application.validation.change.ContentClusterRemovalValidator; @@ -23,7 +23,6 @@ import com.yahoo.vespa.model.application.validation.change.IndexedSearchClusterC import com.yahoo.vespa.model.application.validation.change.IndexingModeChangeValidator; import com.yahoo.vespa.model.application.validation.change.NodeResourceChangeValidator; import com.yahoo.vespa.model.application.validation.change.RedundancyIncreaseValidator; -import com.yahoo.vespa.model.application.validation.change.ResourcesReductionValidator; import com.yahoo.vespa.model.application.validation.change.StartupCommandChangeValidator; import com.yahoo.vespa.model.application.validation.change.StreamingSearchClusterChangeValidator; import com.yahoo.vespa.model.application.validation.first.RedundancyValidator; @@ -116,7 +115,7 @@ public class Validation { new StartupCommandChangeValidator(), new ContentTypeRemovalValidator(), new ContentClusterRemovalValidator(), - new ClusterSizeReductionValidator(), + new ResourcesReductionValidator(), new ResourcesReductionValidator(), new ContainerRestartValidator(), new NodeResourceChangeValidator(), diff --git a/config-model/src/main/java/com/yahoo/vespa/model/application/validation/change/ClusterSizeReductionValidator.java b/config-model/src/main/java/com/yahoo/vespa/model/application/validation/change/ClusterSizeReductionValidator.java deleted file mode 100644 index 353be99cfa9..00000000000 --- a/config-model/src/main/java/com/yahoo/vespa/model/application/validation/change/ClusterSizeReductionValidator.java +++ /dev/null @@ -1,43 +0,0 @@ -// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -package com.yahoo.vespa.model.application.validation.change; - -import com.yahoo.config.model.api.ConfigChangeAction; -import com.yahoo.config.model.deploy.DeployState; -import com.yahoo.config.provision.Capacity; -import com.yahoo.config.provision.ClusterSpec; -import com.yahoo.vespa.model.VespaModel; -import com.yahoo.config.application.api.ValidationId; - -import java.util.List; - -/** - * Checks that no cluster sizes are reduced too much in one go. - * - * @author bratseth - */ -public class ClusterSizeReductionValidator implements ChangeValidator { - - @Override - public List<ConfigChangeAction> validate(VespaModel current, VespaModel next, DeployState deployState) { - for (var clusterId : current.allClusters()) { - Capacity currentCapacity = current.provisioned().all().get(clusterId); - Capacity nextCapacity = next.provisioned().all().get(clusterId); - if (currentCapacity == null || nextCapacity == null) continue; - validate(currentCapacity, nextCapacity, clusterId, deployState); - } - return List.of(); - } - - private void validate(Capacity current, Capacity next, ClusterSpec.Id clusterId, DeployState deployState) { - int currentSize = current.minResources().nodes(); - int nextSize = next.minResources().nodes(); - // don't allow more than 50% reduction, but always allow to reduce size with 1 - if ( nextSize < currentSize * 0.5 && nextSize != currentSize - 1) - deployState.validationOverrides().invalid(ValidationId.clusterSizeReduction, - "Size reduction in '" + clusterId.value() + "' is too large: " + - "New min size must be at least 50% of the current min size. " + - "Current size: " + currentSize + ", new size: " + nextSize, - deployState.now()); - } - -} diff --git a/config-model/src/main/java/com/yahoo/vespa/model/application/validation/change/ResourcesReductionValidator.java b/config-model/src/main/java/com/yahoo/vespa/model/application/validation/change/ResourcesReductionValidator.java index b27266221d2..e06c14f40c9 100644 --- a/config-model/src/main/java/com/yahoo/vespa/model/application/validation/change/ResourcesReductionValidator.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/application/validation/change/ResourcesReductionValidator.java @@ -6,17 +6,15 @@ import com.yahoo.config.model.api.ConfigChangeAction; import com.yahoo.config.model.deploy.DeployState; import com.yahoo.config.provision.Capacity; import com.yahoo.config.provision.ClusterSpec; +import com.yahoo.config.provision.NodeResources; import com.yahoo.vespa.model.VespaModel; import java.util.List; -import java.util.Locale; -import java.util.Optional; -import java.util.stream.Stream; /** - * Checks that no nodes resources are reduced too much in one go. + * Checks that no cluster sizes are reduced too much in one go. * - * @author freva + * @author bratseth */ public class ResourcesReductionValidator implements ChangeValidator { @@ -28,39 +26,39 @@ public class ResourcesReductionValidator implements ChangeValidator { if (currentCapacity == null || nextCapacity == null) continue; validate(currentCapacity, nextCapacity, clusterId, deployState); } - return List.of(); } - private void validate(Capacity current, Capacity next, ClusterSpec.Id clusterId, DeployState deployState) { - if (current.minResources().nodeResources().isUnspecified()) return; - if (next.minResources().nodeResources().isUnspecified()) return; - - List<String> illegalChanges = Stream.of( - validateResource("vCPU", - current.minResources().nodeResources().vcpu(), - next.minResources().nodeResources().vcpu()), - validateResource("memory GB", - current.minResources().nodeResources().memoryGb(), - next.minResources().nodeResources().memoryGb()), - validateResource("disk GB", - current.minResources().nodeResources().diskGb(), - next.minResources().nodeResources().diskGb())) - .flatMap(Optional::stream) - .toList(); - if (illegalChanges.isEmpty()) return; - - deployState.validationOverrides().invalid(ValidationId.resourcesReduction, - "Resource reduction in '" + clusterId.value() + "' is too large. " + - String.join(" ", illegalChanges) + - " New min resources must be at least 50% of the current min resources", - deployState.now()); - } + private void validate(Capacity current, + Capacity next, + ClusterSpec.Id clusterId, + DeployState deployState) { + if (current.maxResources().nodeResources().isUnspecified() || next.maxResources().nodeResources().isUnspecified()) { + // Unspecified resources; compared node count + int currentNodes = current.maxResources().nodes(); + int nextNodes = next.maxResources().nodes(); + if (nextNodes < 0.5 * currentNodes && nextNodes != currentNodes - 1) { + deployState.validationOverrides().invalid(ValidationId.resourcesReduction, + "Size reduction in '" + clusterId.value() + "' is too large: " + + "To guard against mistakes, the new max nodes must be at least 50% of the current nodes. " + + "Current nodes: " + currentNodes + ", new nodes: " + nextNodes, + deployState.now()); + } + } + else { + NodeResources currentResources = current.maxResources().totalResources(); + NodeResources nextResources = next.maxResources().totalResources(); + if (nextResources.vcpu() < 0.5 * currentResources.vcpu() || + nextResources.memoryGb() < 0.5 * currentResources.memoryGb() || + nextResources.diskGb() < 0.5 * currentResources.diskGb()) + deployState.validationOverrides().invalid(ValidationId.resourcesReduction, + "Resource reduction in '" + clusterId.value() + "' is too large: " + + "To guard against mistakes, the new max resources must be at least 50% of the current " + + "max resources in all dimensions. " + + "Current: " + currentResources + ", new: " + nextResources, + deployState.now()); + } - private static Optional<String> validateResource(String resourceName, double currentValue, double nextValue) { - // don't allow more than 50% reduction, but always allow to reduce by 1 - if (nextValue >= currentValue * 0.5 || nextValue >= currentValue - 1) return Optional.empty(); - return Optional.of(String.format(Locale.ENGLISH ,"Current %s: %.2f, new: %.2f.", resourceName, currentValue, nextValue)); } } diff --git a/config-model/src/test/java/com/yahoo/vespa/model/application/validation/change/ClusterSizeReductionValidatorTest.java b/config-model/src/test/java/com/yahoo/vespa/model/application/validation/change/ClusterSizeReductionValidatorTest.java deleted file mode 100644 index 7e172171052..00000000000 --- a/config-model/src/test/java/com/yahoo/vespa/model/application/validation/change/ClusterSizeReductionValidatorTest.java +++ /dev/null @@ -1,74 +0,0 @@ -// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -package com.yahoo.vespa.model.application.validation.change; - -import com.yahoo.config.application.api.ValidationId; -import com.yahoo.config.application.api.ValidationOverrides; -import com.yahoo.config.provision.Environment; -import com.yahoo.vespa.model.VespaModel; -import com.yahoo.vespa.model.application.validation.ValidationTester; -import com.yahoo.yolean.Exceptions; -import org.junit.jupiter.api.Test; - -import static org.junit.jupiter.api.Assertions.assertEquals; -import static org.junit.jupiter.api.Assertions.fail; - -/** - * @author bratseth - */ -public class ClusterSizeReductionValidatorTest { - - @Test - void testSizeReductionValidation() { - ValidationTester tester = new ValidationTester(33); - - VespaModel previous = tester.deploy(null, getServices(30), Environment.prod, null).getFirst(); - try { - tester.deploy(previous, getServices(14), Environment.prod, null); - fail("Expected exception due to cluster size reduction"); - } - catch (IllegalArgumentException expected) { - assertEquals("cluster-size-reduction: Size reduction in 'default' is too large: " + - "New min size must be at least 50% of the current min size. " + - "Current size: 30, new size: 14. " + - ValidationOverrides.toAllowMessage(ValidationId.clusterSizeReduction), - Exceptions.toMessageString(expected)); - } - } - - @Test - void testSizeReductionValidationMinimalDecreaseIsAllowed() { - ValidationTester tester = new ValidationTester(30); - - VespaModel previous = tester.deploy(null, getServices(3), Environment.prod, null).getFirst(); - tester.deploy(previous, getServices(2), Environment.prod, null); - } - - @Test - void testOverridingSizereductionValidation() { - ValidationTester tester = new ValidationTester(33); - - VespaModel previous = tester.deploy(null, getServices(30), Environment.prod, null).getFirst(); - tester.deploy(previous, getServices(14), Environment.prod, sizeReductionOverride); // Allowed due to override - } - - private static String getServices(int size) { - return "<services version='1.0'>" + - " <content id='default' version='1.0'>" + - " <redundancy>1</redundancy>" + - " <engine>" + - " <proton/>" + - " </engine>" + - " <documents>" + - " <document type='music' mode='index'/>" + - " </documents>" + - " <nodes count='" + size + "'/>" + - " </content>" + - "</services>"; - } - - private static final String sizeReductionOverride = - "<validation-overrides>\n" + - " <allow until='2000-01-03'>cluster-size-reduction</allow>\n" + - "</validation-overrides>\n"; - -} diff --git a/config-model/src/test/java/com/yahoo/vespa/model/application/validation/change/ResourcesReductionValidatorTest.java b/config-model/src/test/java/com/yahoo/vespa/model/application/validation/change/ResourcesReductionValidatorTest.java index 651f87c614b..cf9d2b466b2 100644 --- a/config-model/src/test/java/com/yahoo/vespa/model/application/validation/change/ResourcesReductionValidatorTest.java +++ b/config-model/src/test/java/com/yahoo/vespa/model/application/validation/change/ResourcesReductionValidatorTest.java @@ -16,6 +16,7 @@ import static org.junit.jupiter.api.Assertions.fail; /** * @author freva + * @author bratseth */ public class ResourcesReductionValidatorTest { @@ -24,13 +25,16 @@ public class ResourcesReductionValidatorTest { @Test void fail_when_reduction_by_over_50_percent() { - VespaModel previous = tester.deploy(null, getServices(new NodeResources(8, 64, 800, 1)), Environment.prod, null).getFirst(); + VespaModel previous = tester.deploy(null, getServices(6, new NodeResources(8, 64, 800, 1)), Environment.prod, null).getFirst(); try { - tester.deploy(previous, getServices(new NodeResources(8, 16, 800, 1)), Environment.prod, null); + tester.deploy(previous, getServices(6, new NodeResources(8, 16, 800, 1)), Environment.prod, null); fail("Expected exception due to resources reduction"); } catch (IllegalArgumentException expected) { - assertEquals("resources-reduction: Resource reduction in 'default' is too large. " + - "Current memory GB: 64.00, new: 16.00. New min resources must be at least 50% of the current min resources. " + + assertEquals("resources-reduction: Resource reduction in 'default' is too large: " + + "To guard against mistakes, the new max resources must be at least 50% " + + "of the current max resources in all dimensions. " + + "Current: [vcpu: 48.0, memory: 384.0 Gb, disk 4800.0 Gb, bandwidth: 1.8 Gbps, architecture: x86_64], " + + "new: [vcpu: 48.0, memory: 96.0 Gb, disk 4800.0 Gb, bandwidth: 1.8 Gbps, architecture: x86_64]. " + ValidationOverrides.toAllowMessage(ValidationId.resourcesReduction), Exceptions.toMessageString(expected)); } @@ -38,14 +42,16 @@ public class ResourcesReductionValidatorTest { @Test void fail_when_reducing_multiple_resources_by_over_50_percent() { - VespaModel previous = tester.deploy(null, getServices(new NodeResources(8, 64, 800, 1)), Environment.prod, null).getFirst(); + VespaModel previous = tester.deploy(null, getServices(6,new NodeResources(8, 64, 800, 1)), Environment.prod, null).getFirst(); try { - tester.deploy(previous, getServices(new NodeResources(3, 16, 200, 1)), Environment.prod, null); + tester.deploy(previous, getServices(6, new NodeResources(3, 16, 200, 1)), Environment.prod, null); fail("Expected exception due to resources reduction"); } catch (IllegalArgumentException expected) { - assertEquals("resources-reduction: Resource reduction in 'default' is too large. " + - "Current vCPU: 8.00, new: 3.00. Current memory GB: 64.00, new: 16.00. Current disk GB: 800.00, new: 200.00. " + - "New min resources must be at least 50% of the current min resources. " + + assertEquals("resources-reduction: Resource reduction in 'default' is too large: " + + "To guard against mistakes, the new max resources must be at least 50% " + + "of the current max resources in all dimensions. " + + "Current: [vcpu: 48.0, memory: 384.0 Gb, disk 4800.0 Gb, bandwidth: 1.8 Gbps, architecture: x86_64], " + + "new: [vcpu: 18.0, memory: 96.0 Gb, disk 1200.0 Gb, bandwidth: 1.8 Gbps, architecture: x86_64]. " + ValidationOverrides.toAllowMessage(ValidationId.resourcesReduction), Exceptions.toMessageString(expected)); } @@ -53,29 +59,69 @@ public class ResourcesReductionValidatorTest { @Test void small_resource_decrease_is_allowed() { - VespaModel previous = tester.deploy(null, getServices(new NodeResources(1.5, 64, 800, 1)), Environment.prod, null).getFirst(); - tester.deploy(previous, getServices(new NodeResources(.5, 48, 600, 1)), Environment.prod, null); + VespaModel previous = tester.deploy(null, getServices(6, new NodeResources(1.5, 64, 800, 1)), Environment.prod, null).getFirst(); + tester.deploy(previous, getServices(6, new NodeResources(.5, 48, 600, 1)), Environment.prod, null); + } + + @Test + void reorganizing_resources_is_allowed() { + VespaModel previous = tester.deploy(null, getServices(12, new NodeResources(2, 10, 100, 1)), Environment.prod, null).getFirst(); + tester.deploy(previous, getServices(4, new NodeResources(6, 30, 300, 1)), Environment.prod, null); } @Test void overriding_resource_decrease() { - VespaModel previous = tester.deploy(null, getServices(new NodeResources(8, 64, 800, 1)), Environment.prod, null).getFirst(); - tester.deploy(previous, getServices(new NodeResources(8, 16, 800, 1)), Environment.prod, resourcesReductionOverride); // Allowed due to override + VespaModel previous = tester.deploy(null, getServices(6, new NodeResources(8, 64, 800, 1)), Environment.prod, null).getFirst(); + tester.deploy(previous, getServices(6, new NodeResources(8, 16, 800, 1)), Environment.prod, resourcesReductionOverride); // Allowed due to override } @Test void allowed_to_go_to_not_specifying_resources() { - VespaModel previous = tester.deploy(null, getServices(new NodeResources(1.5, 64, 800, 1)), Environment.prod, null).getFirst(); - tester.deploy(previous, getServices(null), Environment.prod, null); + VespaModel previous = tester.deploy(null, getServices(6, new NodeResources(1.5, 64, 800, 1)), Environment.prod, null).getFirst(); + tester.deploy(previous, getServices(6, null), Environment.prod, null); } @Test void allowed_to_go_from_not_specifying_resources() { - VespaModel previous = tester.deploy(null, getServices(null), Environment.prod, null).getFirst(); - tester.deploy(previous, getServices(new NodeResources(1.5, 64, 800, 1)), Environment.prod, null); + VespaModel previous = tester.deploy(null, getServices(6, null), Environment.prod, null).getFirst(); + tester.deploy(previous, getServices(6, new NodeResources(1.5, 64, 800, 1)), Environment.prod, null); + } + + @Test + void testSizeReductionValidation() { + ValidationTester tester = new ValidationTester(33); + + VespaModel previous = tester.deploy(null, getServices(30, null), Environment.prod, null).getFirst(); + try { + tester.deploy(previous, getServices(14, null), Environment.prod, null); + fail("Expected exception due to resources reduction"); + } + catch (IllegalArgumentException expected) { + assertEquals("resources-reduction: Size reduction in 'default' is too large: " + + "To guard against mistakes, the new max nodes must be at least 50% of the current nodes. " + + "Current nodes: 30, new nodes: 14. " + + ValidationOverrides.toAllowMessage(ValidationId.resourcesReduction), + Exceptions.toMessageString(expected)); + } + } + + @Test + void testSizeReductionValidationMinimalDecreaseIsAllowed() { + ValidationTester tester = new ValidationTester(30); + + VespaModel previous = tester.deploy(null, getServices(3, null), Environment.prod, null).getFirst(); + tester.deploy(previous, getServices(2, null), Environment.prod, null); + } + + @Test + void testOverridingSizeReductionValidation() { + ValidationTester tester = new ValidationTester(33); + + VespaModel previous = tester.deploy(null, getServices(30, null), Environment.prod, null).getFirst(); + tester.deploy(previous, getServices(14, null), Environment.prod, resourcesReductionOverride); // Allowed due to override } - private static String getServices(NodeResources resources) { + private static String getServices(int nodes, NodeResources resources) { String resourcesStr = resources == null ? "" : String.format(" <resources vcpu='%.0f' memory='%.0fG' disk='%.0fG'/>", @@ -89,7 +135,7 @@ public class ResourcesReductionValidatorTest { " <documents>" + " <document type='music' mode='index'/>" + " </documents>" + - " <nodes count='5'>" + + " <nodes count='" + nodes + "'>" + resourcesStr + " </nodes>" + " </content>" + |