Revert "Merge pull request #26542 from vespa-engine/revert-26509-brat…

container-core/src/main/java/com/yahoo/metrics/ContainerMetrics.java

@@ -122,7 +122,7 @@ public enum ContainerMetrics implements VespaMetrics {
  QUERIES("queries", Unit.OPERATION, "Query volume"),
  QUERY_CONTAINER_LATENCY("query_container_latency", Unit.MILLISECOND, "The query execution time consumed in the container"),
  QUERY_LATENCY("query_latency", Unit.MILLISECOND, "The overall query latency as seen by the container"),
- QUERY_TIMEOUT("query_timeout", Unit.MILLISECOND, "The amount of time allowed for query execytion, from the client"),
+ QUERY_TIMEOUT("query_timeout", Unit.MILLISECOND, "The amount of time allowed for query execution, from the client"),
  FAILED_QUERIES("failed_queries", Unit.OPERATION, "The number of failed queries"),
  DEGRADED_QUERIES("degraded_queries", Unit.OPERATION, "The number of degraded queries, e.g. due to some conent nodes not responding in time"),
  HITS_PER_QUERY("hits_per_query", Unit.HIT_PER_QUERY, "The number of hits returned"),

container-core/src/main/java/com/yahoo/processing/request/Properties.java

@@ -265,7 +265,7 @@ public final void clearAll(CompoundName name) {
 
  /**
  * Sets all properties having this name as a compound prefix to null.
- * I.e clearAll("a") will clear the value of "a" and "a.b" but not "ab".
+ * I.e. clearAll("a") will clear the value of "a" and "a.b" but not "ab".
  *
  * @param name the compound prefix of the properties to clear
  * @throws RuntimeException if no instance in the chain accepted this name-value pair

node-repository/src/main/java/com/yahoo/vespa/hosted/provision/NodeRepository.java

@@ -205,7 +205,7 @@ public NodeRepository(NodeFlavors flavors,
  */
  public boolean exclusiveAllocation(ClusterSpec clusterSpec) {
  return clusterSpec.isExclusive() ||
- ( clusterSpec.type().isContainer() && zone.system().isPublic() &&  !zone.environment().isTest() ) ||
+ ( clusterSpec.type().isContainer() && zone.system().isPublic() && !zone.environment().isTest() ) ||
  ( !zone().cloud().allowHostSharing() && !sharedHosts.value().isEnabled(clusterSpec.type().name()));
  }
 

node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/NodeAllocation.java

@@ -194,27 +194,34 @@ private boolean offeredNodeHasParentHostnameAlreadyAccepted(NodeCandidate candid
  return false;
  }
 
+ /**
+ * Returns whether allocating the candidate on this host would violate exclusivity constraints.
+ * Note that while we currently require that exclusive allocations uses the entire host,
+ * this method also handles the case where smaller exclusive nodes are allocated on it.
+ */
  private boolean violatesExclusivity(NodeCandidate candidate) {
- if (candidate.parentHostname().isEmpty()) return false;
-
- // In nodes which does not allow host sharing, exclusivity is violated if...
- if ( ! nodeRepository.zone().cloud().allowHostSharing()) {
- // TODO: Write this in a way that is simple to read
- // If either the parent is dedicated to a cluster type different from this cluster
- return ! candidate.parent.flatMap(Node::exclusiveToClusterType).map(cluster.type()::equals).orElse(true) ||
- // or this cluster is requiring exclusivity, but the host is exclusive to a different owner
- (requestedNodes.isExclusive() && !candidate.parent.flatMap(Node::exclusiveToApplicationId).map(application::equals).orElse(false));
+ if (candidate.parent.isEmpty()) return false;
+
+ if (nodeRepository.exclusiveAllocation(cluster)) {
+ // Node must allocate the host entirely and not violate application or cluster type constraints
+ var parent = candidate.parent.get();
+ if (!candidate.resources().isUnspecified() &&
+ ! nodeRepository.resourcesCalculator().advertisedResourcesOf(parent.flavor()).compatibleWith(candidate.resources())) return true;
+ if (parent.exclusiveToApplicationId().isPresent() && !parent.exclusiveToApplicationId().get().equals(application)) return true;
+ if (parent.exclusiveToClusterType().isPresent() && !parent.exclusiveToClusterType().get().equals(cluster.type())) return true;
+ return false;
  }
-
- // In zones with shared hosts we require that if either of the nodes on the host requires exclusivity,
- // then all the nodes on the host must have the same owner
- for (Node nodeOnHost : allNodes.childrenOf(candidate.parentHostname().get())) {
- if (nodeOnHost.allocation().isEmpty()) continue;
- if (requestedNodes.isExclusive() || nodeOnHost.allocation().get().membership().cluster().isExclusive()) {
- if ( ! nodeOnHost.allocation().get().owner().equals(application)) return true;
+ else {
+ // If any of the nodes on the host requires exclusivity to another application, allocating on it is a violation
+ for (Node nodeOnHost : allNodes.childrenOf(candidate.parentHostname().get())) {
+ if (nodeOnHost.allocation().isEmpty()) continue;
+ if (nodeOnHost.allocation().get().membership().cluster().isExclusive()) {
+ if (!nodeOnHost.allocation().get().owner().equals(application))
+ return true;
+ }
  }
+ return false;
  }
- return false;
  }
 
  /**

node-repository/src/main/java/com/yahoo/vespa/hosted/provision/testutils/MockHostProvisioner.java

@@ -47,7 +47,7 @@ public class MockHostProvisioner implements HostProvisioner {
 
  private int deprovisionedHosts = 0;
  private EnumSet<Behaviour> behaviours = EnumSet.noneOf(Behaviour.class);
- private Map<ClusterSpec.Type, Flavor> hostFlavors = new HashMap<>();
+ private final Map<ClusterSpec.Type, Flavor> hostFlavors = new HashMap<>();
 
  public MockHostProvisioner(List<Flavor> flavors, MockNameResolver nameResolver, int memoryTaxGb) {
  this.flavors = List.copyOf(flavors);

node-repository/src/main/java/com/yahoo/vespa/hosted/provision/testutils/MockNodeRepository.java

@@ -97,10 +97,10 @@ public MockNodeRepository(MockCurator curator, NodeFlavors flavors, Zone zone) {
  defaultCloudAccount = zone.cloud().account();
 
  curator.setZooKeeperEnsembleConnectionSpec("cfg1:1234,cfg2:1234,cfg3:1234");
- populate();
+ populate(zone);
  }
 
- private void populate() {
+ private void populate(Zone zone) {
  NodeRepositoryProvisioner provisioner = new NodeRepositoryProvisioner(this, Zone.defaultZone(), new MockProvisionServiceProvider());
  List<Node> nodes = new ArrayList<>();
 
@@ -202,16 +202,26 @@ private void populate() {
  .vespaVersion("6.42")
  .loadBalancerSettings(new ZoneEndpoint(false, true, List.of(new AllowedUrn(AccessType.awsPrivateLink, "arne"))))
  .build();
+ ClusterResources min, max;
+ if (zone.system().isPublic()) { // resources must match one of the flavors used in this mock ("large"), since this is a container cluster
+ min = new ClusterResources(2, 1, new NodeResources(4, 32, 1600, 1));
+ max = new ClusterResources(8, 2, new NodeResources(8, 64, 3200, 1));
+ }
+ else { // resources must fit on actually provisioned hosts
+ min = new ClusterResources(2, 1, new NodeResources(2, 8, 50, 1));
+ max = new ClusterResources(8, 2, new NodeResources(4, 16, 1000, 1));
+ }
+
  activate(provisioner.prepare(app1Id,
  cluster1Id,
- Capacity.from(new ClusterResources(2, 1, new NodeResources(2, 8, 50, 1)),
- new ClusterResources(8, 2, new NodeResources(4, 16, 1000, 1)),
+ Capacity.from(min,
+ max,
  IntRange.empty(),
  false,
  true,
  Optional.empty(),
  ClusterInfo.empty()),
- null), app1Id, provisioner);
+  null), app1Id, provisioner);
  Application app1 = applications().get(app1Id).get();
  Cluster cluster1 = app1.cluster(cluster1Id.id()).get();
  cluster1 = cluster1.withSuggested(new Autoscaling(Autoscaling.Status.unavailable,

node-repository/src/main/java/com/yahoo/vespa/hosted/provision/testutils/MockProvisionServiceProvider.java

@@ -50,4 +50,5 @@ public Optional<HostProvisioner> getHostProvisioner() {
  public HostResourcesCalculator getHostResourcesCalculator() {
  return hostResourcesCalculator;
  }
+
 }

node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/HostCapacityMaintainerTest.java

@@ -376,8 +376,8 @@ private void replace_config_server_like(NodeType hostType) {
 
  // Provision config servers
  for (int i = 0; i < provisionedHosts.size(); i++) {
- tester.makeReadyChildren(1, i + 1, new NodeResources(1.5, 8, 50, 0.3), hostType.childNodeType(),
- provisionedHosts.get(i).hostname(), (nodeIndex) -> "cfg" + nodeIndex);
+ tester.makeReadyChildren(1, i + 1, new NodeResources(1.0, 30, 20, 0.3), hostType.childNodeType(),
+  provisionedHosts.get(i).hostname(), (nodeIndex) -> "cfg" + nodeIndex);
  }
  tester.prepareAndActivateInfraApplication(configSrvApp, hostType.childNodeType());
 

node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/DynamicProvisioningTest.java

@@ -9,6 +9,7 @@
 import com.yahoo.config.provision.Environment;
 import com.yahoo.config.provision.Flavor;
 import com.yahoo.config.provision.HostSpec;
+import com.yahoo.config.provision.NodeAllocationException;
 import com.yahoo.config.provision.NodeFlavors;
 import com.yahoo.config.provision.NodeResources;
 import com.yahoo.config.provision.NodeResources.Architecture;
@@ -28,9 +29,11 @@
 import com.yahoo.vespa.hosted.provision.testutils.MockNameResolver;
 import org.junit.Test;
 
+import java.time.Duration;
 import java.time.Instant;
 import java.util.Collection;
 import java.util.List;
+import java.util.Optional;
 import java.util.Set;
 import java.util.function.Function;
 import java.util.stream.Collectors;
@@ -51,6 +54,25 @@ public class DynamicProvisioningTest {
 
  private final MockNameResolver nameResolver = new MockNameResolver().mockAnyLookup();
 
+ @Test
+ public void test_provisioning_containers_wont_use_shared_hosts_on_public() {
+ var resources = new NodeResources(2.7, 9, 17, 0.1);
+ var now = new ClusterResources(2, 1, resources);
+ try {
+ var fixture = DynamicProvisioningTester.fixture()
+ .awsProdSetup(true)
+ .clusterType(ClusterSpec.Type.container)
+ .initialResources(Optional.of(now))
+ .capacity(Capacity.from(now))
+ .hostCount(2)
+ .build();
+ }
+ catch (NodeAllocationException e) {
+ assertTrue("Contains 'No host flavor matches': " + e.getMessage(),
+ e.getMessage().contains("No host flavor matches"));
+ }
+ }
+
  @Test
  public void dynamically_provision_with_empty_node_repo() {
  var tester = tester(true);
@@ -117,7 +139,7 @@ public void in_place_resize_not_allowed_on_exclusive_to_hosts() {
 
  @Test
  public void avoids_allocating_to_empty_hosts() {
- var tester = tester(false);
+ var tester = tester(true);
  tester.makeReadyHosts(6, new NodeResources(12, 12, 200, 12));
  tester.activateTenantHosts();
 

node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/ProvisioningTester.java

@@ -90,16 +90,16 @@ public class ProvisioningTester {
  private int nextIP = 0;
 
  private ProvisioningTester(Curator curator,
- NodeFlavors nodeFlavors,
- HostResourcesCalculator resourcesCalculator,
- Zone zone,
- NameResolver nameResolver,
- DockerImage containerImage,
- Orchestrator orchestrator,
- HostProvisioner hostProvisioner,
- LoadBalancerServiceMock loadBalancerService,
- FlagSource flagSource,
- int spareCount) {
+  NodeFlavors nodeFlavors,
+  HostResourcesCalculator resourcesCalculator,
+  Zone zone,
+  NameResolver nameResolver,
+  DockerImage containerImage,
+  Orchestrator orchestrator,
+  HostProvisioner hostProvisioner,
+  LoadBalancerServiceMock loadBalancerService,
+  FlagSource flagSource,
+  int spareCount) {
  this.curator = curator;
  this.nodeFlavors = nodeFlavors;
  this.clock = new ManualClock();