diff options
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)) : |