diff options
13 files changed, 31 insertions, 94 deletions
diff --git a/docker-api/src/main/java/com/yahoo/vespa/hosted/dockerapi/Container.java b/docker-api/src/main/java/com/yahoo/vespa/hosted/dockerapi/Container.java index 9e304e5ef4d..fb979ff639c 100644 --- a/docker-api/src/main/java/com/yahoo/vespa/hosted/dockerapi/Container.java +++ b/docker-api/src/main/java/com/yahoo/vespa/hosted/dockerapi/Container.java @@ -10,7 +10,6 @@ import java.util.Objects; */ // TODO: Move this to node-admin when docker-api module can be removed public class Container { - private final ContainerId id; public final String hostname; public final DockerImage image; public final ContainerResources resources; @@ -19,14 +18,12 @@ public class Container { public final int pid; public Container( - final ContainerId id, final String hostname, final DockerImage image, final ContainerResources resources, final ContainerName containerName, final State state, final int pid) { - this.id = id; this.hostname = hostname; this.image = image; this.resources = resources; @@ -35,18 +32,13 @@ public class Container { this.pid = pid; } - public ContainerId id() { - return id; - } - @Override public boolean equals(final Object obj) { if (!(obj instanceof Container)) { return false; } final Container other = (Container) obj; - return Objects.equals(id, other.id) - && Objects.equals(hostname, other.hostname) + return Objects.equals(hostname, other.hostname) && Objects.equals(image, other.image) && Objects.equals(resources, other.resources) && Objects.equals(name, other.name) @@ -61,7 +53,6 @@ public class Container { @Override public String toString() { return "Container {" - + " id=" + id + " hostname=" + hostname + " image=" + image + " resources=" + resources diff --git a/docker-api/src/main/java/com/yahoo/vespa/hosted/dockerapi/ContainerId.java b/docker-api/src/main/java/com/yahoo/vespa/hosted/dockerapi/ContainerId.java deleted file mode 100644 index b86238324f0..00000000000 --- a/docker-api/src/main/java/com/yahoo/vespa/hosted/dockerapi/ContainerId.java +++ /dev/null @@ -1,36 +0,0 @@ -// Copyright Verizon Media. 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; - -/** - * The ID of a container. - * - * @author hakon - */ -public class ContainerId { - private final String id; - - public ContainerId(String id) { - this.id = Objects.requireNonNull(id, "id cannot be null"); - } - - @Override - public String toString() { - return id; - } - - @Override - public boolean equals(Object o) { - if (this == o) return true; - if (o == null || getClass() != o.getClass()) return false; - ContainerId that = (ContainerId) o; - return id.equals(that.id); - } - - @Override - public int hashCode() { - return Objects.hash(id); - } -} 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 37e265bf411..c61bc3ed919 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 @@ -63,7 +63,8 @@ public class ContainerResources { return cpus; } - /** Returns the CFS CPU quota per {@link #cpuPeriod()}, or -1 if disabled. */ + // Although docker allows to update cpu quota to 0, this is not a legal value, must be set -1 for unlimited + // See: https://github.com/docker/for-linux/issues/558 public int cpuQuota() { return cpus > 0 ? (int) (cpus * CPU_PERIOD_US) : -1; } diff --git a/docker-api/src/main/java/com/yahoo/vespa/hosted/dockerapi/DockerEngine.java b/docker-api/src/main/java/com/yahoo/vespa/hosted/dockerapi/DockerEngine.java index 3b7b2b8d54c..630efb7990f 100644 --- a/docker-api/src/main/java/com/yahoo/vespa/hosted/dockerapi/DockerEngine.java +++ b/docker-api/src/main/java/com/yahoo/vespa/hosted/dockerapi/DockerEngine.java @@ -286,7 +286,6 @@ public class DockerEngine implements ContainerEngine { private Stream<Container> asContainer(String container) { return inspectContainerCmd(container) .map(response -> new Container( - new ContainerId(response.getId()), response.getConfig().getHostName(), DockerImage.fromString(response.getConfig().getImage()), containerResourcesFromHostConfig(response.getHostConfig()), diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/docker/ContainerOperations.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/docker/ContainerOperations.java index ed1c035f543..dddd023aa85 100644 --- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/docker/ContainerOperations.java +++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/docker/ContainerOperations.java @@ -3,7 +3,6 @@ package com.yahoo.vespa.hosted.node.admin.docker; import com.yahoo.config.provision.DockerImage; import com.yahoo.vespa.hosted.dockerapi.Container; -import com.yahoo.vespa.hosted.dockerapi.ContainerId; import com.yahoo.vespa.hosted.dockerapi.ContainerName; import com.yahoo.vespa.hosted.dockerapi.ContainerResources; import com.yahoo.vespa.hosted.dockerapi.ContainerStats; @@ -31,7 +30,7 @@ public interface ContainerOperations { void removeContainer(NodeAgentContext context, Container container); - void updateContainer(NodeAgentContext context, ContainerId containerId, ContainerResources containerResources); + void updateContainer(NodeAgentContext context, ContainerResources containerResources); Optional<Container> getContainer(NodeAgentContext context); diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/docker/ContainerOperationsImpl.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/docker/ContainerOperationsImpl.java index ed130105fff..00fed2ba6b7 100644 --- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/docker/ContainerOperationsImpl.java +++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/docker/ContainerOperationsImpl.java @@ -8,7 +8,6 @@ import com.yahoo.config.provision.NodeType; import com.yahoo.config.provision.SystemName; import com.yahoo.vespa.hosted.dockerapi.Container; import com.yahoo.vespa.hosted.dockerapi.ContainerEngine; -import com.yahoo.vespa.hosted.dockerapi.ContainerId; import com.yahoo.vespa.hosted.dockerapi.ContainerName; import com.yahoo.vespa.hosted.dockerapi.ContainerResources; import com.yahoo.vespa.hosted.dockerapi.ContainerStats; @@ -204,7 +203,7 @@ public class ContainerOperationsImpl implements ContainerOperations { } @Override - public void updateContainer(NodeAgentContext context, ContainerId containerId, ContainerResources containerResources) { + public void updateContainer(NodeAgentContext context, ContainerResources containerResources) { containerEngine.updateContainer(context.containerName(), containerResources); } diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeagent/NodeAgentContext.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeagent/NodeAgentContext.java index c33b084c213..8b095a46dcf 100644 --- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeagent/NodeAgentContext.java +++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeagent/NodeAgentContext.java @@ -49,14 +49,12 @@ public interface NodeAgentContext extends TaskContext { }; /** - * The vcpu value in NodeSpec is the number of vcpus required by the node on a fixed historical - * baseline machine. However the current host has a faster per-vcpu performance by a scale factor - * (see flavors.def cpuSpeedup), and therefore do not need to set aside the full number of vcpus - * to run the node. This method returns that reduced number of vcpus. + * The vcpu value in NodeSpec is multiplied by the speedup factor per cpu core compared to a historical baseline + * for a particular cpu generation of the host (see flavors.def cpuSpeedup). * - * @return the vcpus required by the node on this host. + * @return node vcpu without the cpu speedup factor. */ - double vcpuOnThisHost(); + double unscaledVcpu(); /** The file system used by the NodeAgentContext. All paths must have the same provider. */ FileSystem fileSystem(); diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeagent/NodeAgentContextImpl.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeagent/NodeAgentContextImpl.java index 6ab2469cd64..8ac5a89aaef 100644 --- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeagent/NodeAgentContextImpl.java +++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeagent/NodeAgentContextImpl.java @@ -118,7 +118,7 @@ public class NodeAgentContextImpl implements NodeAgentContext { } @Override - public double vcpuOnThisHost() { + public double unscaledVcpu() { return node.vcpu() / cpuSpeedup; } 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 61de751f60d..dc0b6dc9d85 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 @@ -372,7 +372,7 @@ public class NodeAgentImpl implements NodeAgent { wantedContainerResources.toStringCpu(), existingContainer.resources.toStringCpu()); // Only update CPU resources - containerOperations.updateContainer(context, existingContainer.id(), wantedContainerResources.withMemoryBytes(existingContainer.resources.memoryBytes())); + containerOperations.updateContainer(context, wantedContainerResources.withMemoryBytes(existingContainer.resources.memoryBytes())); return containerOperations.getContainer(context).orElseThrow(() -> new ConvergenceException("Did not find container that was just updated")); } @@ -384,9 +384,9 @@ public class NodeAgentImpl implements NodeAgent { .map(appId -> containerCpuCap.with(FetchVector.Dimension.APPLICATION_ID, appId.serializedForm())) .orElse(containerCpuCap) .with(FetchVector.Dimension.HOSTNAME, context.node().hostname()) - .value() * context.vcpuOnThisHost(); + .value() * context.unscaledVcpu(); - return ContainerResources.from(cpuCap, context.vcpuOnThisHost(), context.node().memoryGb()); + return ContainerResources.from(cpuCap, context.unscaledVcpu(), context.node().memoryGb()); } private boolean noCpuCap(ZoneApi zone) { diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/file/UnixPath.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/file/UnixPath.java index cb7f1637410..6c1c1a48a9c 100644 --- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/file/UnixPath.java +++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/file/UnixPath.java @@ -208,11 +208,6 @@ public class UnixPath { return this; } - public UnixPath createDirectories() { - uncheck(() -> Files.createDirectories(path)); - return this; - } - /** * Returns whether this path is a directory. Symlinks are followed, so this returns true for symlinks pointing to a * directory. diff --git a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/docker/ContainerOperationsImplTest.java b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/docker/ContainerOperationsImplTest.java index e78d5bb754b..240fb492aff 100644 --- a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/docker/ContainerOperationsImplTest.java +++ b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/docker/ContainerOperationsImplTest.java @@ -5,7 +5,6 @@ import com.google.common.net.InetAddresses; import com.yahoo.config.provision.DockerImage; import com.yahoo.vespa.hosted.dockerapi.Container; import com.yahoo.vespa.hosted.dockerapi.ContainerEngine; -import com.yahoo.vespa.hosted.dockerapi.ContainerId; import com.yahoo.vespa.hosted.dockerapi.ContainerName; import com.yahoo.vespa.hosted.dockerapi.ProcessResult; import com.yahoo.vespa.hosted.node.admin.nodeagent.ContainerData; @@ -86,8 +85,8 @@ public class ContainerOperationsImplTest { } private Container makeContainer(String name, Container.State state, int pid) { - final Container container = new Container(new ContainerId(name + "-id"), name + ".fqdn", - DockerImage.fromString("registry.example.com/mock"), null, new ContainerName(name), state, pid); + final Container container = new Container(name + ".fqdn", DockerImage.fromString("registry.example.com/mock"), null, + new ContainerName(name), state, pid); when(containerEngine.getContainer(eq(container.name))).thenReturn(Optional.of(container)); return container; } diff --git a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/integrationTests/ContainerEngineMock.java b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/integrationTests/ContainerEngineMock.java index 7aeaa37b4af..ef6564db2a5 100644 --- a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/integrationTests/ContainerEngineMock.java +++ b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/integrationTests/ContainerEngineMock.java @@ -4,7 +4,6 @@ package com.yahoo.vespa.hosted.node.admin.integrationTests; import com.yahoo.config.provision.DockerImage; import com.yahoo.vespa.hosted.dockerapi.Container; import com.yahoo.vespa.hosted.dockerapi.ContainerEngine; -import com.yahoo.vespa.hosted.dockerapi.ContainerId; import com.yahoo.vespa.hosted.dockerapi.ContainerName; import com.yahoo.vespa.hosted.dockerapi.ContainerResources; import com.yahoo.vespa.hosted.dockerapi.ContainerStats; @@ -26,14 +25,12 @@ import java.util.OptionalLong; * @author freva */ public class ContainerEngineMock implements ContainerEngine { - public static final ContainerId CONTAINER_ID = new ContainerId("af345"); - private final Map<ContainerName, Container> containersByContainerName = new HashMap<>(); private static final Object monitor = new Object(); @Override public CreateContainerCommand createContainerCommand(DockerImage dockerImage, ContainerName containerName) { - return new StartContainerCommandMock(CONTAINER_ID, dockerImage, containerName); + return new StartContainerCommandMock(dockerImage, containerName); } @Override @@ -51,7 +48,7 @@ public class ContainerEngineMock implements ContainerEngine { synchronized (monitor) { Container container = containersByContainerName.get(containerName); containersByContainerName.put(containerName, - new Container(container.id(), container.hostname, container.image, container.resources, container.name, Container.State.EXITED, 0)); + new Container(container.hostname, container.image, container.resources, container.name, Container.State.EXITED, 0)); } } @@ -67,7 +64,7 @@ public class ContainerEngineMock implements ContainerEngine { synchronized (monitor) { Container container = containersByContainerName.get(containerName); containersByContainerName.put(containerName, - new Container(container.id(), container.hostname, container.image, containerResources, container.name, container.state, container.pid)); + new Container(container.hostname, container.image, containerResources, container.name, container.state, container.pid)); } } @@ -107,14 +104,12 @@ public class ContainerEngineMock implements ContainerEngine { public class StartContainerCommandMock implements CreateContainerCommand { - private final ContainerId containerId; private final DockerImage dockerImage; private final ContainerName containerName; private String hostName; private ContainerResources containerResources; - public StartContainerCommandMock(ContainerId containerId, DockerImage dockerImage, ContainerName containerName) { - this.containerId = containerId; + public StartContainerCommandMock(DockerImage dockerImage, ContainerName containerName) { this.dockerImage = dockerImage; this.containerName = containerName; } @@ -205,7 +200,7 @@ public class ContainerEngineMock implements ContainerEngine { public void create() { synchronized (monitor) { containersByContainerName.put( - containerName, new Container(containerId, hostName, dockerImage, containerResources, containerName, Container.State.RUNNING, 2)); + 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/nodeagent/NodeAgentImplTest.java b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/nodeagent/NodeAgentImplTest.java index 97c83956a61..fcd5e8cc187 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 @@ -10,7 +10,6 @@ import com.yahoo.test.ManualClock; import com.yahoo.vespa.flags.InMemoryFlagSource; import com.yahoo.vespa.flags.PermanentFlags; import com.yahoo.vespa.hosted.dockerapi.Container; -import com.yahoo.vespa.hosted.dockerapi.ContainerId; import com.yahoo.vespa.hosted.dockerapi.ContainerName; import com.yahoo.vespa.hosted.dockerapi.ContainerResources; import com.yahoo.vespa.hosted.dockerapi.RegistryCredentials; @@ -55,7 +54,6 @@ import static org.mockito.Mockito.when; public class NodeAgentImplTest { private static final NodeResources resources = new NodeResources(2, 16, 250, 1, NodeResources.DiskSpeed.fast, NodeResources.StorageType.local); private static final Version vespaVersion = Version.fromString("1.2.3"); - private static final ContainerId containerId = new ContainerId("af23"); private static final String hostName = "host1.test.yahoo.com"; private final NodeAgentContextSupplier contextSupplier = mock(NodeAgentContextSupplier.class); @@ -228,7 +226,7 @@ public class NodeAgentImplTest { mockGetContainer(dockerImage, resourcesAfterThird, true); inOrder.verify(orchestrator, never()).suspend(any()); - inOrder.verify(containerOperations).updateContainer(eq(thirdContext), eq(containerId), eq(resourcesAfterThird)); + inOrder.verify(containerOperations).updateContainer(eq(thirdContext), eq(resourcesAfterThird)); inOrder.verify(containerOperations, never()).removeContainer(any(), any()); inOrder.verify(containerOperations, never()).startContainer(any()); inOrder.verify(orchestrator, never()).resume(any()); @@ -236,7 +234,7 @@ public class NodeAgentImplTest { // No changes nodeAgent.converge(thirdContext); inOrder.verify(orchestrator, never()).suspend(any()); - inOrder.verify(containerOperations, never()).updateContainer(eq(thirdContext), eq(containerId), any()); + inOrder.verify(containerOperations, never()).updateContainer(eq(thirdContext), any()); inOrder.verify(containerOperations, never()).removeContainer(any(), any()); inOrder.verify(orchestrator, never()).resume(any()); @@ -244,7 +242,7 @@ public class NodeAgentImplTest { flagSource.withDoubleFlag(PermanentFlags.CONTAINER_CPU_CAP.id(), 2.3); nodeAgent.doConverge(thirdContext); - inOrder.verify(containerOperations).updateContainer(eq(thirdContext), eq(containerId), eq(ContainerResources.from(9.2, 4, 16))); + inOrder.verify(containerOperations).updateContainer(eq(thirdContext), eq(ContainerResources.from(9.2, 4, 16))); inOrder.verify(orchestrator, never()).resume(any()); } @@ -269,13 +267,13 @@ public class NodeAgentImplTest { InOrder inOrder = inOrder(orchestrator, containerOperations, nodeRepository); inOrder.verify(orchestrator).resume(any(String.class)); inOrder.verify(containerOperations).removeContainer(eq(secondContext), any()); - inOrder.verify(containerOperations, never()).updateContainer(any(), any(), any()); + inOrder.verify(containerOperations, never()).updateContainer(any(), any()); inOrder.verify(containerOperations, never()).restartVespa(any()); inOrder.verify(nodeRepository).updateNodeAttributes(eq(hostName), eq(new NodeAttributes().withRestartGeneration(2))); nodeAgent.doConverge(secondContext); inOrder.verify(orchestrator).resume(any(String.class)); - inOrder.verify(containerOperations, never()).updateContainer(any(), any(), any()); + inOrder.verify(containerOperations, never()).updateContainer(any(), any()); inOrder.verify(containerOperations, never()).removeContainer(any(), any()); } @@ -643,14 +641,14 @@ public class NodeAgentImplTest { } inOrder.verify(orchestrator, never()).resume(any()); inOrder.verify(orchestrator, never()).suspend(any()); - inOrder.verify(containerOperations, never()).updateContainer(any(), any(), any()); + inOrder.verify(containerOperations, never()).updateContainer(any(), any()); clock.advance(Duration.ofSeconds(31)); nodeAgent.doConverge(context); inOrder.verify(orchestrator, never()).suspend(any()); - inOrder.verify(containerOperations).updateContainer(eq(context), eq(containerId), eq(ContainerResources.from(0, 2, 16))); + inOrder.verify(containerOperations).updateContainer(eq(context), eq(ContainerResources.from(0, 2, 16))); inOrder.verify(containerOperations, never()).removeContainer(any(), any()); inOrder.verify(containerOperations, never()).startContainer(any()); inOrder.verify(orchestrator, never()).resume(any()); @@ -658,7 +656,7 @@ public class NodeAgentImplTest { // No changes nodeAgent.converge(context); inOrder.verify(orchestrator, never()).suspend(any()); - inOrder.verify(containerOperations, never()).updateContainer(eq(context), eq(containerId), any()); + inOrder.verify(containerOperations, never()).updateContainer(eq(context), any()); inOrder.verify(containerOperations, never()).removeContainer(any(), any()); inOrder.verify(orchestrator, never()).resume(any()); } @@ -680,7 +678,7 @@ public class NodeAgentImplTest { nodeAgent.converge(context); inOrder.verify(orchestrator, never()).suspend(any(String.class)); - inOrder.verify(containerOperations, never()).updateContainer(eq(context), eq(containerId), any()); + inOrder.verify(containerOperations, never()).updateContainer(eq(context), any()); inOrder.verify(containerOperations, never()).removeContainer(any(), any()); inOrder.verify(orchestrator, never()).resume(any(String.class)); } @@ -730,10 +728,10 @@ public class NodeAgentImplTest { doAnswer(invoc -> { NodeAgentContext context = invoc.getArgument(0, NodeAgentContext.class); - ContainerResources resources = invoc.getArgument(2, ContainerResources.class); + ContainerResources resources = invoc.getArgument(1, ContainerResources.class); mockGetContainer(context.node().wantedDockerImage().get(), resources, true); return null; - }).when(containerOperations).updateContainer(any(), any(), any()); + }).when(containerOperations).updateContainer(any(), any()); return new NodeAgentImpl(contextSupplier, nodeRepository, orchestrator, containerOperations, () -> RegistryCredentials.none, storageMaintainer, flagSource, @@ -752,7 +750,6 @@ public class NodeAgentImplTest { throw new IllegalArgumentException(); return dockerImage != null ? Optional.of(new Container( - containerId, hostName, dockerImage, containerResources, |