summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorValerij Fredriksen <freva@users.noreply.github.com>2019-01-14 09:08:41 +0100
committerGitHub <noreply@github.com>2019-01-14 09:08:41 +0100
commit8b57fffd5dd15bf5da117191cf40d443a967b5c0 (patch)
treedd281253baec5cecc68616e594e5b3d5a6ef8a24
parent4d7714b6b4a52dcc29dbd0fcd952b37a6f42b561 (diff)
parentea5f61ba4b329ad029e37d04f399f5493fdad2c9 (diff)
Merge pull request #8112 from vespa-engine/freva/cap-docker-container-cpu-feature-flag
Cap docker container CPU with feature flag
-rw-r--r--docker-api/src/main/java/com/yahoo/vespa/hosted/dockerapi/ContainerResources.java55
-rw-r--r--docker-api/src/main/java/com/yahoo/vespa/hosted/dockerapi/CreateContainerCommandImpl.java62
-rw-r--r--docker-api/src/main/java/com/yahoo/vespa/hosted/dockerapi/Docker.java10
-rw-r--r--docker-api/src/main/java/com/yahoo/vespa/hosted/dockerapi/DockerImpl.java42
-rw-r--r--docker-api/src/test/java/com/yahoo/vespa/hosted/dockerapi/CreateContainerCommandImplTest.java9
-rw-r--r--flags/src/main/java/com/yahoo/vespa/flags/DoubleFlag.java18
-rw-r--r--flags/src/main/java/com/yahoo/vespa/flags/Flags.java15
-rw-r--r--flags/src/main/java/com/yahoo/vespa/flags/InMemoryFlagSource.java41
-rw-r--r--flags/src/main/java/com/yahoo/vespa/flags/UnboundDoubleFlag.java23
-rw-r--r--flags/src/test/java/com/yahoo/vespa/flags/FlagsTest.java5
-rw-r--r--node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/noderepository/NodeSpec.java5
-rw-r--r--node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/docker/DockerOperations.java3
-rw-r--r--node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/docker/DockerOperationsImpl.java13
-rw-r--r--node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeagent/NodeAgentImpl.java44
-rw-r--r--node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/integrationTests/DockerFailTest.java7
-rw-r--r--node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/integrationTests/DockerMock.java52
-rw-r--r--node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/integrationTests/DockerTester.java7
-rw-r--r--node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/integrationTests/MultiDockerTest.java4
-rw-r--r--node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/integrationTests/RebootTest.java3
-rw-r--r--node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/integrationTests/RestartTest.java3
-rw-r--r--node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/nodeagent/NodeAgentImplTest.java36
21 files changed, 363 insertions, 94 deletions
diff --git a/docker-api/src/main/java/com/yahoo/vespa/hosted/dockerapi/ContainerResources.java b/docker-api/src/main/java/com/yahoo/vespa/hosted/dockerapi/ContainerResources.java
index 346223d0e7e..36b7c35afc4 100644
--- a/docker-api/src/main/java/com/yahoo/vespa/hosted/dockerapi/ContainerResources.java
+++ b/docker-api/src/main/java/com/yahoo/vespa/hosted/dockerapi/ContainerResources.java
@@ -1,26 +1,60 @@
// Copyright 2018 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
package com.yahoo.vespa.hosted.dockerapi;
+import java.util.Objects;
+
/**
* @author valerijf
*/
public class ContainerResources {
- public static final ContainerResources UNLIMITED = ContainerResources.from(0, 0);
+ public static final ContainerResources UNLIMITED = ContainerResources.from(0, 0, 0);
+ private static final int CPU_PERIOD = 100_000; // 100 µs
+
+ /** Hard limit on container's CPU usage: Implemented using Completely Fair Scheduler (CFS) by allocating a given
+ * time within a given period, Container's processes are not bound to any specific CPU, which may create significant
+ * performance degradation as processes are scheduled on another CPU after exhausting the quota. */
+ private final double cpus;
+ /** Soft limit on container's CPU usage: When plenty of CPU cycles are available, all containers use as much
+ * CPU as they need. It prioritizes container CPU resources for the available CPU cycles.
+ * It does not guarantee or reserve any specific CPU access. */
private final int cpuShares;
+
+ /** The maximum amount, in bytes, of memory the container can use. */
private final long memoryBytes;
- ContainerResources(int cpuShares, long memoryBytes) {
+ ContainerResources(double cpus, int cpuShares, long memoryBytes) {
+ this.cpus = cpus;
this.cpuShares = cpuShares;
this.memoryBytes = memoryBytes;
+
+ if (cpus < 0)
+ throw new IllegalArgumentException("CPUs must be a positive number or 0 for unlimited, was " + cpus);
+ if (cpuShares < 0)
+ throw new IllegalArgumentException("CPU shares must be a positive integer or 0 for unlimited, was " + cpuShares);
+ if (memoryBytes < 0)
+ throw new IllegalArgumentException("memoryBytes must be a positive integer or 0 for unlimited, was " + memoryBytes);
}
- public static ContainerResources from(double cpuCores, double memoryGb) {
+ public static ContainerResources from(double cpus, double cpuCores, double memoryGb) {
return new ContainerResources(
+ cpus,
(int) Math.round(10 * cpuCores),
(long) ((1L << 30) * memoryGb));
}
+ public double cpus() {
+ return cpus;
+ }
+
+ public int cpuQuota() {
+ return (int) cpus * CPU_PERIOD;
+ }
+
+ public int cpuPeriod() {
+ return CPU_PERIOD;
+ }
+
public int cpuShares() {
return cpuShares;
}
@@ -33,22 +67,21 @@ public class ContainerResources {
public boolean equals(Object o) {
if (this == o) return true;
if (o == null || getClass() != o.getClass()) return false;
-
ContainerResources that = (ContainerResources) o;
-
- if (cpuShares != that.cpuShares) return false;
- return memoryBytes == that.memoryBytes;
+ return Math.abs(that.cpus - cpus) < 0.0001 &&
+ cpuShares == that.cpuShares &&
+ memoryBytes == that.memoryBytes;
}
@Override
public int hashCode() {
- int result = cpuShares;
- result = 31 * result + (int) (memoryBytes ^ (memoryBytes >>> 32));
- return result;
+ return Objects.hash(cpus, cpuShares, memoryBytes);
}
@Override
public String toString() {
- return cpuShares + " CPU Shares, " + memoryBytes + "B memory";
+ return (cpus > 0 ? cpus : "unlimited") +" CPUs, " +
+ (cpuShares > 0 ? cpuShares : "unlimited") + " CPU Shares, " +
+ (memoryBytes > 0 ? memoryBytes + "B" : "unlimited") + " memory";
}
}
diff --git a/docker-api/src/main/java/com/yahoo/vespa/hosted/dockerapi/CreateContainerCommandImpl.java b/docker-api/src/main/java/com/yahoo/vespa/hosted/dockerapi/CreateContainerCommandImpl.java
index 04aad562711..febd3ba4d98 100644
--- a/docker-api/src/main/java/com/yahoo/vespa/hosted/dockerapi/CreateContainerCommandImpl.java
+++ b/docker-api/src/main/java/com/yahoo/vespa/hosted/dockerapi/CreateContainerCommandImpl.java
@@ -5,6 +5,7 @@ import com.github.dockerjava.api.DockerClient;
import com.github.dockerjava.api.command.CreateContainerCmd;
import com.github.dockerjava.api.model.Bind;
import com.github.dockerjava.api.model.Capability;
+import com.github.dockerjava.api.model.HostConfig;
import com.github.dockerjava.api.model.Ulimit;
import com.yahoo.vespa.hosted.dockerapi.exception.DockerException;
@@ -25,13 +26,13 @@ import java.util.stream.Collectors;
import java.util.stream.IntStream;
import java.util.stream.Stream;
+import static com.yahoo.vespa.hosted.dockerapi.DockerImpl.LABEL_NAME_MANAGEDBY;
+
class CreateContainerCommandImpl implements Docker.CreateContainerCommand {
private final DockerClient docker;
private final DockerImage dockerImage;
- private final ContainerResources containerResources;
private final ContainerName containerName;
- private final String hostName;
private final Map<String, String> labels = new HashMap<>();
private final List<String> environmentAssignments = new ArrayList<>();
private final List<String> volumeBindSpecs = new ArrayList<>();
@@ -39,22 +40,31 @@ class CreateContainerCommandImpl implements Docker.CreateContainerCommand {
private final Set<Capability> addCapabilities = new HashSet<>();
private final Set<Capability> dropCapabilities = new HashSet<>();
+ private Optional<String> hostName = Optional.empty();
+ private Optional<ContainerResources> containerResources = Optional.empty();
private Optional<String> networkMode = Optional.empty();
private Optional<String> ipv4Address = Optional.empty();
private Optional<String> ipv6Address = Optional.empty();
private Optional<String[]> entrypoint = Optional.empty();
private boolean privileged = false;
- CreateContainerCommandImpl(DockerClient docker,
- DockerImage dockerImage,
- ContainerResources containerResources,
- ContainerName containerName,
- String hostName) {
+ CreateContainerCommandImpl(DockerClient docker, DockerImage dockerImage, ContainerName containerName) {
this.docker = docker;
this.dockerImage = dockerImage;
- this.containerResources = containerResources;
this.containerName = containerName;
- this.hostName = hostName;
+ }
+
+
+ @Override
+ public Docker.CreateContainerCommand withHostName(String hostName) {
+ this.hostName = Optional.of(hostName);
+ return this;
+ }
+
+ @Override
+ public Docker.CreateContainerCommand withResources(ContainerResources containerResources) {
+ this.containerResources = Optional.of(containerResources);
+ return this;
}
@Override
@@ -65,7 +75,7 @@ class CreateContainerCommandImpl implements Docker.CreateContainerCommand {
}
public Docker.CreateContainerCommand withManagedBy(String manager) {
- return withLabel(DockerImpl.LABEL_NAME_MANAGEDBY, manager);
+ return withLabel(LABEL_NAME_MANAGEDBY, manager);
}
@Override
@@ -140,19 +150,25 @@ class CreateContainerCommandImpl implements Docker.CreateContainerCommand {
try {
createCreateContainerCmd().exec();
} catch (RuntimeException e) {
- throw new DockerException("Failed to create container " + containerName.asString(), e);
+ throw new DockerException("Failed to create container " + toString(), e);
}
}
private CreateContainerCmd createCreateContainerCmd() {
List<Bind> volumeBinds = volumeBindSpecs.stream().map(Bind::parse).collect(Collectors.toList());
+ final HostConfig hostConfig = new HostConfig();
+
+ containerResources.ifPresent(cr -> hostConfig
+ .withCpuShares(cr.cpuShares())
+ .withMemory(cr.memoryBytes())
+ .withCpuPeriod(cr.cpuQuota() > 0 ? cr.cpuPeriod() : null)
+ .withCpuQuota(cr.cpuQuota() > 0 ? cr.cpuQuota() : null));
+
final CreateContainerCmd containerCmd = docker
.createContainerCmd(dockerImage.asString())
- .withCpuShares(containerResources.cpuShares())
- .withMemory(containerResources.memoryBytes())
+ .withHostConfig(hostConfig) // MUST BE FIRST (some of the later setters are simply proxied to HostConfig)
.withName(containerName.asString())
- .withHostName(hostName)
.withLabels(labels)
.withEnv(environmentAssignments)
.withBinds(volumeBinds)
@@ -165,6 +181,7 @@ class CreateContainerCommandImpl implements Docker.CreateContainerCommand {
.filter(mode -> ! mode.toLowerCase().equals("host"))
.ifPresent(mode -> containerCmd.withMacAddress(generateMACAddress(hostName, ipv4Address, ipv6Address)));
+ hostName.ifPresent(containerCmd::withHostName);
networkMode.ifPresent(containerCmd::withNetworkMode);
ipv4Address.ifPresent(containerCmd::withIpv4Address);
ipv6Address.ifPresent(containerCmd::withIpv6Address);
@@ -174,17 +191,17 @@ class CreateContainerCommandImpl implements Docker.CreateContainerCommand {
}
/** Maps ("--env", {"A", "B", "C"}) to "--env A --env B --env C" */
- private String toRepeatedOption(String option, List<String> optionValues) {
+ private static String toRepeatedOption(String option, List<String> optionValues) {
return optionValues.stream()
.map(optionValue -> option + " " + optionValue)
.collect(Collectors.joining(" "));
}
- private String toOptionalOption(String option, Optional<String> value) {
+ private static String toOptionalOption(String option, Optional<?> value) {
return value.map(o -> option + " " + o).orElse("");
}
- private String toFlagOption(String option, boolean value) {
+ private static String toFlagOption(String option, boolean value) {
return value ? option : "";
}
@@ -205,9 +222,10 @@ class CreateContainerCommandImpl implements Docker.CreateContainerCommand {
return Stream.of(
"--name " + containerName.asString(),
- "--hostname " + hostName,
- "--cpu-shares " + containerResources.cpuShares(),
- "--memory " + containerResources.memoryBytes(),
+ toOptionalOption("--hostname", hostName),
+ toOptionalOption("--cpu-shares", containerResources.map(ContainerResources::cpuShares)),
+ toOptionalOption("--cpus", containerResources.map(ContainerResources::cpus)),
+ toOptionalOption("--memory", containerResources.map(ContainerResources::memoryBytes)),
toRepeatedOption("--label", labelList),
toRepeatedOption("--ulimit", ulimitList),
toRepeatedOption("--env", environmentAssignments),
@@ -228,8 +246,8 @@ class CreateContainerCommandImpl implements Docker.CreateContainerCommand {
/**
* Generates a pseudo-random MAC address based on the hostname, IPv4- and IPv6-address.
*/
- static String generateMACAddress(String hostname, Optional<String> ipv4Address, Optional<String> ipv6Address) {
- final String seed = hostname + ipv4Address.orElse("") + ipv6Address.orElse("");
+ static String generateMACAddress(Optional<String> hostname, Optional<String> ipv4Address, Optional<String> ipv6Address) {
+ final String seed = hostname.orElse("") + ipv4Address.orElse("") + ipv6Address.orElse("");
Random rand = getPRNG(seed);
byte[] macAddr = new byte[6];
rand.nextBytes(macAddr);
diff --git a/docker-api/src/main/java/com/yahoo/vespa/hosted/dockerapi/Docker.java b/docker-api/src/main/java/com/yahoo/vespa/hosted/dockerapi/Docker.java
index 1713b4570b8..f4cd1d770fb 100644
--- a/docker-api/src/main/java/com/yahoo/vespa/hosted/dockerapi/Docker.java
+++ b/docker-api/src/main/java/com/yahoo/vespa/hosted/dockerapi/Docker.java
@@ -15,6 +15,8 @@ import java.util.OptionalLong;
public interface Docker {
interface CreateContainerCommand {
+ CreateContainerCommand withHostName(String hostname);
+ CreateContainerCommand withResources(ContainerResources containerResources);
CreateContainerCommand withLabel(String name, String value);
CreateContainerCommand withEnvironment(String name, String value);
@@ -53,11 +55,7 @@ public interface Docker {
void create();
}
- CreateContainerCommand createContainerCommand(
- DockerImage dockerImage,
- ContainerResources containerResources,
- ContainerName containerName,
- String hostName);
+ CreateContainerCommand createContainerCommand(DockerImage dockerImage, ContainerName containerName);
Optional<ContainerStats> getContainerStats(ContainerName containerName);
@@ -67,6 +65,8 @@ public interface Docker {
void deleteContainer(ContainerName containerName);
+ void updateContainer(ContainerName containerName, ContainerResources containerResources);
+
List<Container> getAllContainersManagedBy(String manager);
Optional<Container> getContainer(ContainerName containerName);
diff --git a/docker-api/src/main/java/com/yahoo/vespa/hosted/dockerapi/DockerImpl.java b/docker-api/src/main/java/com/yahoo/vespa/hosted/dockerapi/DockerImpl.java
index 2ac68be2b2e..1d0295ebc68 100644
--- a/docker-api/src/main/java/com/yahoo/vespa/hosted/dockerapi/DockerImpl.java
+++ b/docker-api/src/main/java/com/yahoo/vespa/hosted/dockerapi/DockerImpl.java
@@ -6,9 +6,11 @@ import com.github.dockerjava.api.command.ExecCreateCmdResponse;
import com.github.dockerjava.api.command.InspectContainerResponse;
import com.github.dockerjava.api.command.InspectExecResponse;
import com.github.dockerjava.api.command.InspectImageResponse;
+import com.github.dockerjava.api.command.UpdateContainerCmd;
import com.github.dockerjava.api.exception.DockerClientException;
import com.github.dockerjava.api.exception.NotFoundException;
import com.github.dockerjava.api.exception.NotModifiedException;
+import com.github.dockerjava.api.model.HostConfig;
import com.github.dockerjava.api.model.Image;
import com.github.dockerjava.api.model.Statistics;
import com.github.dockerjava.core.DefaultDockerClientConfig;
@@ -114,9 +116,8 @@ public class DockerImpl implements Docker {
}
@Override
- public CreateContainerCommand createContainerCommand(DockerImage image, ContainerResources containerResources,
- ContainerName name, String hostName) {
- return new CreateContainerCommandImpl(dockerClient, image, containerResources, name, hostName);
+ public CreateContainerCommand createContainerCommand(DockerImage image, ContainerName containerName) {
+ return new CreateContainerCommandImpl(dockerClient, image, containerName);
}
@@ -232,6 +233,32 @@ public class DockerImpl implements Docker {
}
@Override
+ public void updateContainer(ContainerName containerName, ContainerResources resources) {
+ try {
+ UpdateContainerCmd updateContainerCmd = dockerClient.updateContainerCmd(containerName.asString())
+ .withCpuShares(resources.cpuShares())
+ .withMemory(resources.memoryBytes())
+
+ // Command line argument `--cpus c` is sent over to docker daemon as "NanoCPUs", which is the
+ // value of `c * 1e9`. This however, is just a shorthand for `--cpu-period p` and `--cpu-quota q`
+ // where p = 100000 and q = c * 100000.
+ // See: https://docs.docker.com/config/containers/resource_constraints/#configure-the-default-cfs-scheduler
+ // --cpus requires API 1.25+ on create and 1.29+ on update
+ // NanoCPUs is supported in docker-java as of 3.1.0 on create and not at all on update
+ // TODO: Simplify this to .withNanoCPUs(resources.cpu()) when docker-java supports it
+ .withCpuPeriod(resources.cpuQuota() > 0 ? resources.cpuPeriod() : null)
+ .withCpuQuota(resources.cpuQuota() > 0 ? resources.cpuQuota() : null);
+
+ updateContainerCmd.exec();
+ } catch (NotFoundException e) {
+ throw new ContainerNotFoundException(containerName);
+ } catch (RuntimeException e) {
+ numberOfDockerDaemonFails.add();
+ throw new DockerException("Failed to update container '" + containerName.asString() + "' to " + resources, e);
+ }
+ }
+
+ @Override
public List<Container> getAllContainersManagedBy(String manager) {
return listAllContainers().stream()
.filter(container -> isManagedBy(container, manager))
@@ -251,8 +278,7 @@ public class DockerImpl implements Docker {
new Container(
response.getConfig().getHostName(),
new DockerImage(response.getConfig().getImage()),
- new ContainerResources(response.getHostConfig().getCpuShares(),
- response.getHostConfig().getMemory()),
+ containerResourcesFromHostConfig(response.getHostConfig()),
new ContainerName(decode(response.getName())),
Container.State.valueOf(response.getState().getStatus().toUpperCase()),
response.getState().getPid()
@@ -261,6 +287,12 @@ public class DockerImpl implements Docker {
.orElse(Stream.empty());
}
+ private static ContainerResources containerResourcesFromHostConfig(HostConfig hostConfig) {
+ final double cpus = hostConfig.getCpuPeriod() > 0 ?
+ (double) hostConfig.getCpuQuota() / hostConfig.getCpuPeriod() : 0;
+ return new ContainerResources(cpus, hostConfig.getCpuShares(), hostConfig.getMemory());
+ }
+
private boolean isManagedBy(com.github.dockerjava.api.model.Container container, String manager) {
final Map<String, String> labels = container.getLabels();
return labels != null && manager.equals(labels.get(LABEL_NAME_MANAGEDBY));
diff --git a/docker-api/src/test/java/com/yahoo/vespa/hosted/dockerapi/CreateContainerCommandImplTest.java b/docker-api/src/test/java/com/yahoo/vespa/hosted/dockerapi/CreateContainerCommandImplTest.java
index dc041b159b5..ccaf8fb652d 100644
--- a/docker-api/src/test/java/com/yahoo/vespa/hosted/dockerapi/CreateContainerCommandImplTest.java
+++ b/docker-api/src/test/java/com/yahoo/vespa/hosted/dockerapi/CreateContainerCommandImplTest.java
@@ -17,12 +17,14 @@ public class CreateContainerCommandImplTest {
@Test
public void testToString() throws UnknownHostException {
DockerImage dockerImage = new DockerImage("docker.registry.domain.tld/my/image:1.2.3");
- ContainerResources containerResources = new ContainerResources(100, 1024);
+ ContainerResources containerResources = new ContainerResources(2.5, 100, 1024);
String hostname = "docker-1.region.domain.tld";
ContainerName containerName = ContainerName.fromHostname(hostname);
Docker.CreateContainerCommand createContainerCommand = new CreateContainerCommandImpl(
- null, dockerImage, containerResources, containerName, hostname)
+ null, dockerImage, containerName)
+ .withHostName(hostname)
+ .withResources(containerResources)
.withLabel("my-label", "test-label")
.withUlimit("nofile", 1, 2)
.withUlimit("nproc", 10, 20)
@@ -41,6 +43,7 @@ public class CreateContainerCommandImplTest {
assertEquals("--name docker-1 " +
"--hostname docker-1.region.domain.tld " +
"--cpu-shares 100 " +
+ "--cpus 2.5 " +
"--memory 1024 " +
"--label my-label=test-label " +
"--ulimit nofile=1:2 " +
@@ -71,7 +74,7 @@ public class CreateContainerCommandImplTest {
Stream.of(addresses).forEach(address -> {
String generatedMac = CreateContainerCommandImpl.generateMACAddress(
- address[0], Optional.ofNullable(address[1]), Optional.ofNullable(address[2]));
+ Optional.of(address[0]), Optional.ofNullable(address[1]), Optional.ofNullable(address[2]));
String expectedMac = address[3];
assertEquals(expectedMac, generatedMac);
});
diff --git a/flags/src/main/java/com/yahoo/vespa/flags/DoubleFlag.java b/flags/src/main/java/com/yahoo/vespa/flags/DoubleFlag.java
new file mode 100644
index 00000000000..8c3974727c6
--- /dev/null
+++ b/flags/src/main/java/com/yahoo/vespa/flags/DoubleFlag.java
@@ -0,0 +1,18 @@
+// Copyright 2019 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
+package com.yahoo.vespa.flags;
+
+import javax.annotation.concurrent.Immutable;
+
+/**
+ * @author freva
+ */
+@Immutable
+public class DoubleFlag extends FlagImpl<Double, DoubleFlag> {
+ public DoubleFlag(FlagId id, Double defaultValue, FetchVector vector, FlagSerializer<Double> serializer, FlagSource source) {
+ super(id, defaultValue, vector, serializer, source, DoubleFlag::new);
+ }
+
+ public double value() {
+ return boxedValue();
+ }
+}
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 545f288af29..cc4d55c0c94 100644
--- a/flags/src/main/java/com/yahoo/vespa/flags/Flags.java
+++ b/flags/src/main/java/com/yahoo/vespa/flags/Flags.java
@@ -8,6 +8,7 @@ import java.util.List;
import java.util.Optional;
import java.util.TreeMap;
+import static com.yahoo.vespa.flags.FetchVector.Dimension.APPLICATION_ID;
import static com.yahoo.vespa.flags.FetchVector.Dimension.HOSTNAME;
/**
@@ -41,7 +42,7 @@ public class Flags {
"use-config-server-cache", true,
"Whether config server will use cache to answer config requests.",
"Takes effect immediately when changed.",
- HOSTNAME, FetchVector.Dimension.APPLICATION_ID);
+ HOSTNAME, APPLICATION_ID);
public static final UnboundBooleanFlag CONFIG_SERVER_BOOTSTRAP_IN_SEPARATE_THREAD = defineFeatureFlag(
"config-server-bootstrap-in-separate-thread", true,
@@ -77,6 +78,12 @@ public class Flags {
HOSTNAME
);
+ public static final UnboundDoubleFlag CONTAINER_CPU_CAP = defineDoubleFlag(
+ "container-cpu-cap", 0,
+ "Hard limit on how many CPUs a container may use",
+ "Takes effect on next node agent tick. Change is orchestrated, but does NOT require container restart",
+ HOSTNAME, APPLICATION_ID);
+
/** 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) {
@@ -102,6 +109,12 @@ public class Flags {
}
/** WARNING: public for testing: All flags should be defined in {@link Flags}. */
+ public static UnboundDoubleFlag defineDoubleFlag(String flagId, double defaultValue, String description,
+ String modificationEffect, FetchVector.Dimension... dimensions) {
+ return define(UnboundDoubleFlag::new, flagId, defaultValue, description, modificationEffect, dimensions);
+ }
+
+ /** WARNING: public for testing: All flags should be defined in {@link Flags}. */
public static <T> UnboundJacksonFlag<T> defineJacksonFlag(String flagId, T defaultValue, Class<T> jacksonClass, String description,
String modificationEffect, FetchVector.Dimension... dimensions) {
return define((id2, defaultValue2, vector2) -> new UnboundJacksonFlag<>(id2, defaultValue2, vector2, jacksonClass),
diff --git a/flags/src/main/java/com/yahoo/vespa/flags/InMemoryFlagSource.java b/flags/src/main/java/com/yahoo/vespa/flags/InMemoryFlagSource.java
new file mode 100644
index 00000000000..5e2afe39dd8
--- /dev/null
+++ b/flags/src/main/java/com/yahoo/vespa/flags/InMemoryFlagSource.java
@@ -0,0 +1,41 @@
+// Copyright 2018 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
+package com.yahoo.vespa.flags;
+
+import com.fasterxml.jackson.databind.JsonNode;
+import com.fasterxml.jackson.databind.node.DoubleNode;
+import com.yahoo.vespa.flags.json.FlagData;
+import com.yahoo.vespa.flags.json.Rule;
+
+import java.util.Map;
+import java.util.Optional;
+import java.util.concurrent.ConcurrentHashMap;
+
+/**
+ * Basic in-memory flag source useful for testing
+ *
+ * @author freva
+ */
+public class InMemoryFlagSource implements FlagSource {
+ private final Map<FlagId, FlagData> flagDataById = new ConcurrentHashMap<>();
+
+ public InMemoryFlagSource withFlag(FlagId flagId) {
+ flagDataById.put(flagId, new FlagData(flagId));
+ return this;
+ }
+
+ public InMemoryFlagSource withDoubleFlag(FlagId flagId, double value) {
+ return withRawFlag(flagId, new DoubleNode(value));
+ }
+
+ private InMemoryFlagSource withRawFlag(FlagId flagId, JsonNode rawFlag) {
+ Rule rule = new Rule(Optional.of(JsonNodeRawFlag.fromJsonNode(rawFlag)));
+ flagDataById.put(flagId, new FlagData(flagId, new FetchVector(), rule));
+ return this;
+ }
+
+ @Override
+ public Optional<RawFlag> fetch(FlagId id, FetchVector vector) {
+ return Optional.ofNullable(flagDataById.get(id))
+ .flatMap(flagData -> flagData.resolve(vector));
+ }
+}
diff --git a/flags/src/main/java/com/yahoo/vespa/flags/UnboundDoubleFlag.java b/flags/src/main/java/com/yahoo/vespa/flags/UnboundDoubleFlag.java
new file mode 100644
index 00000000000..84abaa01608
--- /dev/null
+++ b/flags/src/main/java/com/yahoo/vespa/flags/UnboundDoubleFlag.java
@@ -0,0 +1,23 @@
+// Copyright 2019 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
+package com.yahoo.vespa.flags;
+
+import com.fasterxml.jackson.databind.JsonNode;
+import com.fasterxml.jackson.databind.node.DoubleNode;
+
+import javax.annotation.concurrent.Immutable;
+
+/**
+ * @author freva
+ */
+@Immutable
+public class UnboundDoubleFlag extends UnboundFlagImpl<Double, DoubleFlag, UnboundDoubleFlag> {
+ public UnboundDoubleFlag(FlagId id, double defaultValue) {
+ this(id, defaultValue, new FetchVector());
+ }
+
+ public UnboundDoubleFlag(FlagId id, Double defaultValue, FetchVector defaultFetchVector) {
+ super(id, defaultValue, defaultFetchVector,
+ new SimpleFlagSerializer<>(DoubleNode::new, JsonNode::isFloatingPointNumber, JsonNode::asDouble),
+ UnboundDoubleFlag::new, DoubleFlag::new);
+ }
+}
diff --git a/flags/src/test/java/com/yahoo/vespa/flags/FlagsTest.java b/flags/src/test/java/com/yahoo/vespa/flags/FlagsTest.java
index 591198f5e50..1aa1581f105 100644
--- a/flags/src/test/java/com/yahoo/vespa/flags/FlagsTest.java
+++ b/flags/src/test/java/com/yahoo/vespa/flags/FlagsTest.java
@@ -81,6 +81,11 @@ public class FlagsTest {
}
@Test
+ public void testDouble() {
+ testGeneric(Flags.defineDoubleFlag("double-id", 3.142, "desc", "mod"), 3.142, 2.718);
+ }
+
+ @Test
public void testJacksonClass() {
ExampleJacksonClass defaultInstance = new ExampleJacksonClass();
ExampleJacksonClass instance = new ExampleJacksonClass();
diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/noderepository/NodeSpec.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/noderepository/NodeSpec.java
index 071d4b549de..c00caef5587 100644
--- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/noderepository/NodeSpec.java
+++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/noderepository/NodeSpec.java
@@ -1,6 +1,7 @@
// Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
package com.yahoo.vespa.hosted.node.admin.configserver.noderepository;
+import com.yahoo.config.provision.ApplicationId;
import com.yahoo.config.provision.NodeType;
import com.yahoo.vespa.hosted.dockerapi.DockerImage;
import com.yahoo.vespa.hosted.provision.Node;
@@ -350,6 +351,10 @@ public class NodeSpec {
return instance;
}
+ public ApplicationId asApplicationId() {
+ return ApplicationId.from(tenant, application, instance);
+ }
+
@Override
public boolean equals(Object o) {
if (this == o) return true;
diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/docker/DockerOperations.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/docker/DockerOperations.java
index af8dfb1fd27..27dc4e5237b 100644
--- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/docker/DockerOperations.java
+++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/docker/DockerOperations.java
@@ -3,6 +3,7 @@ package com.yahoo.vespa.hosted.node.admin.docker;
import com.yahoo.vespa.hosted.dockerapi.Container;
import com.yahoo.vespa.hosted.dockerapi.ContainerName;
+import com.yahoo.vespa.hosted.dockerapi.ContainerResources;
import com.yahoo.vespa.hosted.dockerapi.ContainerStats;
import com.yahoo.vespa.hosted.dockerapi.DockerImage;
import com.yahoo.vespa.hosted.dockerapi.ProcessResult;
@@ -20,6 +21,8 @@ public interface DockerOperations {
void removeContainer(NodeAgentContext context, Container container);
+ void updateContainer(NodeAgentContext context, ContainerResources containerResources);
+
Optional<Container> getContainer(NodeAgentContext context);
boolean pullImageAsyncIfNeeded(DockerImage dockerImage);
diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/docker/DockerOperationsImpl.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/docker/DockerOperationsImpl.java
index e1b77b6a41b..bdda155b2f3 100644
--- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/docker/DockerOperationsImpl.java
+++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/docker/DockerOperationsImpl.java
@@ -63,10 +63,10 @@ public class DockerOperationsImpl implements DockerOperations {
". Missing an AAAA DNS entry?"));
Docker.CreateContainerCommand command = docker.createContainerCommand(
- context.node().getWantedDockerImage().get(),
- ContainerResources.from(context.node().getMinCpuCores(), context.node().getMinMainMemoryAvailableGb()),
- context.containerName(),
- context.node().getHostname())
+ context.node().getWantedDockerImage().get(), context.containerName())
+ .withHostName(context.node().getHostname())
+ .withResources(ContainerResources.from(
+ 0, context.node().getMinCpuCores(), context.node().getMinMainMemoryAvailableGb()))
.withManagedBy(MANAGER_NAME)
.withUlimit("nofile", 262_144, 262_144)
// The nproc aka RLIMIT_NPROC resource limit works as follows:
@@ -171,6 +171,11 @@ public class DockerOperationsImpl implements DockerOperations {
}
@Override
+ public void updateContainer(NodeAgentContext context, ContainerResources containerResources) {
+ docker.updateContainer(context.containerName(), containerResources);
+ }
+
+ @Override
public Optional<Container> getContainer(NodeAgentContext context) {
return docker.getContainer(context.containerName());
}
diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeagent/NodeAgentImpl.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeagent/NodeAgentImpl.java
index 0bfff82a055..1035895d411 100644
--- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeagent/NodeAgentImpl.java
+++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeagent/NodeAgentImpl.java
@@ -4,6 +4,10 @@ package com.yahoo.vespa.hosted.node.admin.nodeagent;
import com.fasterxml.jackson.core.JsonProcessingException;
import com.yahoo.concurrent.ThreadFactoryFactory;
import com.yahoo.log.LogLevel;
+import com.yahoo.vespa.flags.DoubleFlag;
+import com.yahoo.vespa.flags.FetchVector;
+import com.yahoo.vespa.flags.FlagSource;
+import com.yahoo.vespa.flags.Flags;
import com.yahoo.vespa.hosted.dockerapi.Container;
import com.yahoo.vespa.hosted.dockerapi.ContainerResources;
import com.yahoo.vespa.hosted.dockerapi.ContainerStats;
@@ -69,6 +73,8 @@ public class NodeAgentImpl implements NodeAgent {
private final Optional<AclMaintainer> aclMaintainer;
private final Optional<HealthChecker> healthChecker;
+ private final DoubleFlag containerCpuCap;
+
private int numberOfUnhandledException = 0;
private DockerImage imageBeingDownloaded = null;
@@ -108,6 +114,7 @@ public class NodeAgentImpl implements NodeAgent {
final Orchestrator orchestrator,
final DockerOperations dockerOperations,
final StorageMaintainer storageMaintainer,
+ final FlagSource flagSource,
final Optional<AthenzCredentialsMaintainer> athenzCredentialsMaintainer,
final Optional<AclMaintainer> aclMaintainer,
final Optional<HealthChecker> healthChecker) {
@@ -120,6 +127,9 @@ public class NodeAgentImpl implements NodeAgent {
this.aclMaintainer = aclMaintainer;
this.healthChecker = healthChecker;
+ this.containerCpuCap = Flags.CONTAINER_CPU_CAP.bindTo(flagSource)
+ .with(FetchVector.Dimension.HOSTNAME, contextSupplier.currentContext().node().getHostname());
+
this.loopThread = new Thread(() -> {
while (!terminated.get()) {
try {
@@ -310,13 +320,6 @@ public class NodeAgentImpl implements NodeAgent {
return Optional.of("Container no longer running");
}
- ContainerResources wantedContainerResources = ContainerResources.from(
- node.getMinCpuCores(), node.getMinMainMemoryAvailableGb());
- if (!wantedContainerResources.equals(existingContainer.resources)) {
- return Optional.of("Container should be running with different resource allocation, wanted: " +
- wantedContainerResources + ", actual: " + existingContainer.resources);
- }
-
if (currentRebootGeneration < node.getWantedRebootGeneration()) {
return Optional.of(String.format("Container reboot wanted. Current: %d, Wanted: %d",
currentRebootGeneration, node.getWantedRebootGeneration()));
@@ -332,9 +335,7 @@ public class NodeAgentImpl implements NodeAgent {
context.log(logger, "Will remove container: " + removeReason.get());
if (existingContainer.state.isRunning()) {
- if (context.node().getState() == Node.State.active) {
- orchestratorSuspendNode(context);
- }
+ orchestratorSuspendNode(context);
try {
if (context.node().getState() != Node.State.dirty) {
@@ -356,6 +357,25 @@ public class NodeAgentImpl implements NodeAgent {
return Optional.of(existingContainer);
}
+ private void updateContainerIfNeeded(NodeAgentContext context, Container existingContainer) {
+ double cpuCap = context.node().getOwner()
+ .map(NodeSpec.Owner::asApplicationId)
+ .map(appId -> containerCpuCap.with(FetchVector.Dimension.APPLICATION_ID, appId.serializedForm()))
+ .orElse(containerCpuCap)
+ .value();
+
+ ContainerResources wantedContainerResources = ContainerResources.from(
+ cpuCap, context.node().getMinCpuCores(), context.node().getMinMainMemoryAvailableGb());
+
+ if (wantedContainerResources.equals(existingContainer.resources)) return;
+ context.log(logger, "Container should be running with different resource allocation, wanted: %s, current: %s",
+ wantedContainerResources, existingContainer.resources);
+
+ orchestratorSuspendNode(context);
+
+ dockerOperations.updateContainer(context, wantedContainerResources);
+ }
+
private void scheduleDownLoadIfNeeded(NodeSpec node) {
if (node.getCurrentDockerImage().equals(node.getWantedDockerImage())) return;
@@ -438,6 +458,8 @@ public class NodeAgentImpl implements NodeAgent {
startContainer(context);
containerState = UNKNOWN;
aclMaintainer.ifPresent(AclMaintainer::converge);
+ } else {
+ updateContainerIfNeeded(context, container.get());
}
startServicesIfNeeded(context);
@@ -663,6 +685,8 @@ public class NodeAgentImpl implements NodeAgent {
// to allow the node admin to make decisions that depend on the docker image. Or, each docker image
// needs to contain routines for drain and suspend. For many images, these can just be dummy routines.
private void orchestratorSuspendNode(NodeAgentContext context) {
+ if (context.node().getState() != Node.State.active) return;
+
context.log(logger, "Ask Orchestrator for permission to suspend node");
orchestrator.suspend(context.hostname().value());
}
diff --git a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/integrationTests/DockerFailTest.java b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/integrationTests/DockerFailTest.java
index b3bf2282988..c7bc8f16477 100644
--- a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/integrationTests/DockerFailTest.java
+++ b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/integrationTests/DockerFailTest.java
@@ -3,7 +3,6 @@ package com.yahoo.vespa.hosted.node.admin.integrationTests;
import com.yahoo.config.provision.NodeType;
import com.yahoo.vespa.hosted.dockerapi.ContainerName;
-import com.yahoo.vespa.hosted.dockerapi.ContainerResources;
import com.yahoo.vespa.hosted.dockerapi.DockerImage;
import com.yahoo.vespa.hosted.node.admin.configserver.noderepository.NodeSpec;
import com.yahoo.vespa.hosted.provision.Node;
@@ -39,16 +38,14 @@ public class DockerFailTest {
.minDiskAvailableGb(1)
.build());
- tester.inOrder(tester.docker).createContainerCommand(
- eq(dockerImage), eq(ContainerResources.from(1, 1)), eq(containerName), eq(hostname));
+ tester.inOrder(tester.docker).createContainerCommand(eq(dockerImage), eq(containerName));
tester.inOrder(tester.docker).executeInContainerAsUser(
eq(containerName), eq("root"), any(), eq(DockerTester.NODE_PROGRAM), eq("resume"));
tester.docker.deleteContainer(new ContainerName("host1"));
tester.inOrder(tester.docker).deleteContainer(eq(containerName));
- tester.inOrder(tester.docker).createContainerCommand(
- eq(dockerImage), eq(ContainerResources.from(1, 1)), eq(containerName), eq(hostname));
+ tester.inOrder(tester.docker).createContainerCommand(eq(dockerImage), eq(containerName));
tester.inOrder(tester.docker).executeInContainerAsUser(
eq(containerName), eq("root"), any(), eq(DockerTester.NODE_PROGRAM), eq("resume"));
diff --git a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/integrationTests/DockerMock.java b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/integrationTests/DockerMock.java
index 75bdaed60cf..87b07ca4ed9 100644
--- a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/integrationTests/DockerMock.java
+++ b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/integrationTests/DockerMock.java
@@ -29,17 +29,8 @@ public class DockerMock implements Docker {
private static final Object monitor = new Object();
@Override
- public CreateContainerCommand createContainerCommand(
- DockerImage dockerImage,
- ContainerResources containerResources,
- ContainerName containerName,
- String hostName) {
- synchronized (monitor) {
- containersByContainerName.put(
- containerName, new Container(hostName, dockerImage, containerResources, containerName, Container.State.RUNNING, 2));
- }
-
- return new StartContainerCommandMock();
+ public CreateContainerCommand createContainerCommand(DockerImage dockerImage, ContainerName containerName) {
+ return new StartContainerCommandMock(dockerImage, containerName);
}
@Override
@@ -76,6 +67,15 @@ public class DockerMock implements Docker {
}
@Override
+ public void updateContainer(ContainerName containerName, ContainerResources containerResources) {
+ synchronized (monitor) {
+ Container container = containersByContainerName.get(containerName);
+ containersByContainerName.put(containerName,
+ new Container(container.hostname, container.image, containerResources, container.name, container.state, container.pid));
+ }
+ }
+
+ @Override
public Optional<Container> getContainer(ContainerName containerName) {
synchronized (monitor) {
return Optional.ofNullable(containersByContainerName.get(containerName));
@@ -100,7 +100,30 @@ public class DockerMock implements Docker {
}
- public static class StartContainerCommandMock implements CreateContainerCommand {
+ public class StartContainerCommandMock implements CreateContainerCommand {
+
+ private final DockerImage dockerImage;
+ private final ContainerName containerName;
+ private String hostName;
+ private ContainerResources containerResources;
+
+ public StartContainerCommandMock(DockerImage dockerImage, ContainerName containerName) {
+ this.dockerImage = dockerImage;
+ this.containerName = containerName;
+ }
+
+ @Override
+ public CreateContainerCommand withHostName(String hostname) {
+ this.hostName = hostname;
+ return this;
+ }
+
+ @Override
+ public CreateContainerCommand withResources(ContainerResources containerResources) {
+ this.containerResources = containerResources;
+ return this;
+ }
+
@Override
public CreateContainerCommand withLabel(String name, String value) {
return this;
@@ -163,7 +186,10 @@ public class DockerMock implements Docker {
@Override
public void create() {
-
+ synchronized (monitor) {
+ containersByContainerName.put(
+ containerName, new Container(hostName, dockerImage, containerResources, containerName, Container.State.RUNNING, 2));
+ }
}
}
}
diff --git a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/integrationTests/DockerTester.java b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/integrationTests/DockerTester.java
index 109bce4c13f..7ac74b190ad 100644
--- a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/integrationTests/DockerTester.java
+++ b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/integrationTests/DockerTester.java
@@ -6,6 +6,8 @@ import com.yahoo.config.provision.HostName;
import com.yahoo.config.provision.NodeType;
import com.yahoo.metrics.simple.MetricReceiver;
import com.yahoo.system.ProcessExecuter;
+import com.yahoo.vespa.flags.Flags;
+import com.yahoo.vespa.flags.InMemoryFlagSource;
import com.yahoo.vespa.hosted.dockerapi.Docker;
import com.yahoo.vespa.hosted.dockerapi.metrics.MetricReceiverWrapper;
import com.yahoo.vespa.hosted.node.admin.configserver.noderepository.NodeSpec;
@@ -58,6 +60,7 @@ public class DockerTester implements AutoCloseable {
final Orchestrator orchestrator = mock(Orchestrator.class);
final StorageMaintainer storageMaintainer = mock(StorageMaintainer.class);
final InOrder inOrder = Mockito.inOrder(docker, nodeRepository, orchestrator, storageMaintainer);
+ final InMemoryFlagSource flagSource = new InMemoryFlagSource().withFlag(Flags.CONTAINER_CPU_CAP.id());
final NodeAdminStateUpdater nodeAdminStateUpdater;
final NodeAdminImpl nodeAdmin;
@@ -92,8 +95,8 @@ public class DockerTester implements AutoCloseable {
MetricReceiverWrapper mr = new MetricReceiverWrapper(MetricReceiver.nullImplementation);
NodeAgentFactory nodeAgentFactory = contextSupplier -> new NodeAgentImpl(
- contextSupplier, nodeRepository,
- orchestrator, dockerOperations, storageMaintainer, Optional.empty(), Optional.empty(), Optional.empty());
+ contextSupplier, nodeRepository, orchestrator, dockerOperations, storageMaintainer, flagSource,
+ Optional.empty(), Optional.empty(), Optional.empty());
NodeAgentContextFactory nodeAgentContextFactory = nodeSpec ->
new NodeAgentContextImpl.Builder(nodeSpec).fileSystem(fileSystem).build();
nodeAdmin = new NodeAdminImpl(nodeAgentFactory, nodeAgentContextFactory, Optional.empty(), mr, Clock.systemUTC());
diff --git a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/integrationTests/MultiDockerTest.java b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/integrationTests/MultiDockerTest.java
index 48fdc564aac..16c9239dcd7 100644
--- a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/integrationTests/MultiDockerTest.java
+++ b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/integrationTests/MultiDockerTest.java
@@ -3,7 +3,6 @@ package com.yahoo.vespa.hosted.node.admin.integrationTests;
import com.yahoo.config.provision.NodeType;
import com.yahoo.vespa.hosted.dockerapi.ContainerName;
-import com.yahoo.vespa.hosted.dockerapi.ContainerResources;
import com.yahoo.vespa.hosted.dockerapi.DockerImage;
import com.yahoo.vespa.hosted.node.admin.configserver.noderepository.NodeAttributes;
import com.yahoo.vespa.hosted.node.admin.configserver.noderepository.NodeSpec;
@@ -60,8 +59,7 @@ public class MultiDockerTest {
tester.addChildNodeRepositoryNode(nodeSpec);
ContainerName containerName = ContainerName.fromHostname(hostName);
- tester.inOrder(tester.docker).createContainerCommand(
- eq(dockerImage), eq(ContainerResources.from(2, 4)), eq(containerName), eq(hostName));
+ tester.inOrder(tester.docker).createContainerCommand(eq(dockerImage), eq(containerName));
tester.inOrder(tester.docker).executeInContainerAsUser(
eq(containerName), eq("root"), any(), eq(DockerTester.NODE_PROGRAM), eq("resume"));
tester.inOrder(tester.nodeRepository).updateNodeAttributes(eq(hostName), eq(new NodeAttributes().withDockerImage(dockerImage)));
diff --git a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/integrationTests/RebootTest.java b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/integrationTests/RebootTest.java
index 7e17e2df530..295368e0426 100644
--- a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/integrationTests/RebootTest.java
+++ b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/integrationTests/RebootTest.java
@@ -33,8 +33,7 @@ public class RebootTest {
try (DockerTester tester = new DockerTester()) {
tester.addChildNodeRepositoryNode(createNodeRepositoryNode());
- tester.inOrder(tester.docker).createContainerCommand(
- eq(dockerImage), eq(ContainerResources.from(0, 0)), eq(new ContainerName("host1")), eq(hostname));
+ tester.inOrder(tester.docker).createContainerCommand(eq(dockerImage), eq(new ContainerName("host1")));
try {
tester.setWantedState(NodeAdminStateUpdater.State.SUSPENDED);
diff --git a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/integrationTests/RestartTest.java b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/integrationTests/RestartTest.java
index 4cf349562a3..f9f455cca79 100644
--- a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/integrationTests/RestartTest.java
+++ b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/integrationTests/RestartTest.java
@@ -36,8 +36,7 @@ public class RestartTest {
.currentRestartGeneration(1)
.build());
- tester.inOrder(tester.docker).createContainerCommand(
- eq(dockerImage), any(), eq(new ContainerName("host1")), eq(hostname));
+ tester.inOrder(tester.docker).createContainerCommand(eq(dockerImage), eq(new ContainerName("host1")));
tester.inOrder(tester.nodeRepository).updateNodeAttributes(
eq(hostname), eq(new NodeAttributes().withDockerImage(dockerImage)));
diff --git a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/nodeagent/NodeAgentImplTest.java b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/nodeagent/NodeAgentImplTest.java
index e392ac34414..1b116b304b2 100644
--- a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/nodeagent/NodeAgentImplTest.java
+++ b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/nodeagent/NodeAgentImplTest.java
@@ -4,6 +4,8 @@ package com.yahoo.vespa.hosted.node.admin.nodeagent;
import com.fasterxml.jackson.databind.ObjectMapper;
import com.yahoo.config.provision.NodeType;
import com.yahoo.metrics.simple.MetricReceiver;
+import com.yahoo.vespa.flags.Flags;
+import com.yahoo.vespa.flags.InMemoryFlagSource;
import com.yahoo.vespa.hosted.dockerapi.Container;
import com.yahoo.vespa.hosted.dockerapi.ContainerName;
import com.yahoo.vespa.hosted.dockerapi.ContainerResources;
@@ -80,6 +82,8 @@ public class NodeAgentImplTest {
private final ContainerStats emptyContainerStats = new ContainerStats(Collections.emptyMap(),
Collections.emptyMap(), Collections.emptyMap(), Collections.emptyMap());
private final AthenzCredentialsMaintainer athenzCredentialsMaintainer = mock(AthenzCredentialsMaintainer.class);
+ private final InMemoryFlagSource flagSource = new InMemoryFlagSource()
+ .withFlag(Flags.CONTAINER_CPU_CAP.id());
@Test
@@ -233,7 +237,7 @@ public class NodeAgentImplTest {
}
@Test
- public void containerIsRestartedIfFlavorChanged() {
+ public void containerIsUpdatedIfFlavorChanged() {
NodeSpec.Builder specBuilder = nodeBuilder
.wantedDockerImage(dockerImage)
.currentDockerImage(dockerImage)
@@ -252,14 +256,30 @@ public class NodeAgentImplTest {
nodeAgent.doConverge(secondContext);
NodeAgentContext thirdContext = createContext(specBuilder.minCpuCores(4).build());
nodeAgent.doConverge(thirdContext);
+ ContainerResources resourcesAfterThird = ContainerResources.from(0, 4, 16);
+ mockGetContainer(dockerImage, resourcesAfterThird, true);
InOrder inOrder = inOrder(orchestrator, dockerOperations);
inOrder.verify(orchestrator).resume(any(String.class));
inOrder.verify(orchestrator).resume(any(String.class));
inOrder.verify(orchestrator).suspend(any(String.class));
- inOrder.verify(dockerOperations).removeContainer(eq(thirdContext), any());
- inOrder.verify(dockerOperations, times(1)).createContainer(eq(thirdContext), any());
- inOrder.verify(dockerOperations).startContainer(eq(thirdContext));
+ inOrder.verify(dockerOperations).updateContainer(eq(thirdContext), eq(resourcesAfterThird));
+ inOrder.verify(dockerOperations, never()).removeContainer(any(), any());
+ inOrder.verify(dockerOperations, never()).startContainer(any());
+ inOrder.verify(orchestrator).resume(any(String.class));
+
+ // No changes
+ nodeAgent.converge(thirdContext);
+ inOrder.verify(orchestrator, never()).suspend(any(String.class));
+ inOrder.verify(dockerOperations, never()).updateContainer(eq(thirdContext), any());
+ inOrder.verify(dockerOperations, never()).removeContainer(any(), any());
+ inOrder.verify(orchestrator).resume(any(String.class));
+
+ // Set the feature flag
+ flagSource.withDoubleFlag(Flags.CONTAINER_CPU_CAP.id(), 2.3);
+
+ nodeAgent.converge(thirdContext);
+ inOrder.verify(dockerOperations).updateContainer(eq(thirdContext), eq(ContainerResources.from(2.3, 4, 16)));
inOrder.verify(orchestrator).resume(any(String.class));
}
@@ -702,11 +722,15 @@ public class NodeAgentImplTest {
doNothing().when(storageMaintainer).writeMetricsConfig(any());
return new NodeAgentImpl(contextSupplier, nodeRepository, orchestrator, dockerOperations,
- storageMaintainer, Optional.of(athenzCredentialsMaintainer), Optional.of(aclMaintainer),
+ storageMaintainer, flagSource, Optional.of(athenzCredentialsMaintainer), Optional.of(aclMaintainer),
Optional.of(healthChecker));
}
private void mockGetContainer(DockerImage dockerImage, boolean isRunning) {
+ mockGetContainer(dockerImage, ContainerResources.from(0, MIN_CPU_CORES, MIN_MAIN_MEMORY_AVAILABLE_GB), isRunning);
+ }
+
+ private void mockGetContainer(DockerImage dockerImage, ContainerResources containerResources, boolean isRunning) {
doAnswer(invoc -> {
NodeAgentContext context = invoc.getArgument(0);
if (!hostName.equals(context.hostname().value()))
@@ -715,7 +739,7 @@ public class NodeAgentImplTest {
Optional.of(new Container(
hostName,
dockerImage,
- ContainerResources.from(MIN_CPU_CORES, MIN_MAIN_MEMORY_AVAILABLE_GB),
+ containerResources,
ContainerName.fromHostname(hostName),
isRunning ? Container.State.RUNNING : Container.State.EXITED,
isRunning ? 1 : 0)) :