diff options
author | Valerij Fredriksen <freva@users.noreply.github.com> | 2018-10-19 09:21:52 +0200 |
---|---|---|
committer | GitHub <noreply@github.com> | 2018-10-19 09:21:52 +0200 |
commit | 182384b808434b3f449f459f9bce113f0aa5cc16 (patch) | |
tree | cf37878a77318b7feb96f7cad961f6da3ed4b5bf /node-admin/src | |
parent | 8bb9182d01ad6f9479cf5676d1ee89abeaab9f85 (diff) | |
parent | b31a71306a107ec20895d042b49ec2f68c1c736a (diff) |
Merge pull request #7371 from vespa-engine/revert-7364-revert-7342-freva/move-spec-verifier-to-host-admin-task
Revert 7364 revert 7342 freva/move spec verifier to host admin task
Diffstat (limited to 'node-admin/src')
24 files changed, 310 insertions, 432 deletions
diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/component/ContainerEnvironmentResolver.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/component/ContainerEnvironmentResolver.java index 1b4ddf6aedd..f1218efa67c 100644 --- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/component/ContainerEnvironmentResolver.java +++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/component/ContainerEnvironmentResolver.java @@ -16,9 +16,10 @@ import java.util.Map; * * @author hmusum */ +@FunctionalInterface public interface ContainerEnvironmentResolver { - String createSettings(Environment environment, NodeSpec node); + String createSettings(NodeSpec node); class ContainerEnvironmentSettings { diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/component/DefaultContainerEnvironmentResolver.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/component/DefaultContainerEnvironmentResolver.java deleted file mode 100644 index 38f57c9544a..00000000000 --- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/component/DefaultContainerEnvironmentResolver.java +++ /dev/null @@ -1,18 +0,0 @@ -// 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.node.admin.component; - -import com.yahoo.vespa.hosted.node.admin.configserver.noderepository.NodeSpec; - -/** - * @author hmusum - */ -public class DefaultContainerEnvironmentResolver implements ContainerEnvironmentResolver { - - public String createSettings(Environment environment, NodeSpec node) { - return new ContainerEnvironmentSettings() - .set("nodeType", node.getNodeType()) - .set("cloud", environment.getCloud()) - .build(); - } - -} diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/component/Environment.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/component/Environment.java index 857f796fb5c..aaadd3cb24e 100644 --- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/component/Environment.java +++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/component/Environment.java @@ -307,7 +307,7 @@ public class Environment { Optional.ofNullable(pathResolver).orElseGet(PathResolver::new), logstashNodes, nodeType, - Optional.ofNullable(containerEnvironmentResolver).orElseGet(DefaultContainerEnvironmentResolver::new), + Optional.ofNullable(containerEnvironmentResolver).orElseGet(() -> node -> ""), certificateDnsSuffix, ztsUri, nodeAthenzIdentity, diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/component/ZoneId.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/component/ZoneId.java new file mode 100644 index 00000000000..02f6da2e0e3 --- /dev/null +++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/component/ZoneId.java @@ -0,0 +1,50 @@ +// 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.node.admin.component; + +import com.yahoo.config.provision.Environment; +import com.yahoo.config.provision.RegionName; +import com.yahoo.config.provision.SystemName; + +import java.util.Objects; + +/** + * @author freva + */ +public class ZoneId { + private final SystemName systemName; + private final Environment environment; + private final RegionName regionName; + + public ZoneId(SystemName systemName, Environment environment, RegionName regionName) { + this.systemName = systemName; + this.environment = environment; + this.regionName = regionName; + } + + public SystemName systemName() { + return systemName; + } + + public Environment environment() { + return environment; + } + + public RegionName regionName() { + return regionName; + } + + @Override + public boolean equals(Object o) { + if (this == o) return true; + if (o == null || getClass() != o.getClass()) return false; + ZoneId zoneId = (ZoneId) o; + return systemName == zoneId.systemName && + environment == zoneId.environment && + Objects.equals(regionName, zoneId.regionName); + } + + @Override + public int hashCode() { + return Objects.hash(systemName, environment, regionName); + } +} 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 f1fc6776380..7a83f00e297 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 @@ -12,7 +12,7 @@ import com.yahoo.vespa.hosted.dockerapi.ContainerStats; import com.yahoo.vespa.hosted.dockerapi.Docker; import com.yahoo.vespa.hosted.dockerapi.DockerImage; import com.yahoo.vespa.hosted.dockerapi.ProcessResult; -import com.yahoo.vespa.hosted.node.admin.component.Environment; +import com.yahoo.vespa.hosted.node.admin.component.ContainerEnvironmentResolver; import com.yahoo.vespa.hosted.node.admin.nodeagent.NodeAgentContext; import com.yahoo.vespa.hosted.node.admin.configserver.noderepository.NodeSpec; import com.yahoo.vespa.hosted.node.admin.nodeagent.ContainerData; @@ -46,13 +46,19 @@ public class DockerOperationsImpl implements DockerOperations { private static final String IPV4_NPT_PREFIX = "172.17.0.0"; private final Docker docker; - private final Environment environment; private final ProcessExecuter processExecuter; + private final ContainerEnvironmentResolver containerEnvironmentResolver; + private final List<String> configServerHostnames; + private final IPAddresses ipAddresses; - public DockerOperationsImpl(Docker docker, Environment environment, ProcessExecuter processExecuter) { + public DockerOperationsImpl(Docker docker, ProcessExecuter processExecuter, + ContainerEnvironmentResolver containerEnvironmentResolver, + List<String> configServerHostnames, IPAddresses ipAddresses) { this.docker = docker; - this.environment = environment; this.processExecuter = processExecuter; + this.containerEnvironmentResolver = containerEnvironmentResolver; + this.configServerHostnames = configServerHostnames; + this.ipAddresses = ipAddresses; } @Override @@ -60,11 +66,11 @@ public class DockerOperationsImpl implements DockerOperations { context.log(logger, "Creating container"); // IPv6 - Assume always valid - Inet6Address ipV6Address = environment.getIpAddresses().getIPv6Address(node.getHostname()).orElseThrow( + Inet6Address ipV6Address = ipAddresses.getIPv6Address(node.getHostname()).orElseThrow( () -> new RuntimeException("Unable to find a valid IPv6 address for " + node.getHostname() + ". Missing an AAAA DNS entry?")); - String configServers = String.join(",", environment.getConfigServerHostNames()); + String configServers = String.join(",", configServerHostnames); Docker.CreateContainerCommand command = docker.createContainerCommand( node.getWantedDockerImage().get(), @@ -73,8 +79,7 @@ public class DockerOperationsImpl implements DockerOperations { node.getHostname()) .withManagedBy(MANAGER_NAME) .withEnvironment("VESPA_CONFIGSERVERS", configServers) - .withEnvironment("CONTAINER_ENVIRONMENT_SETTINGS", - environment.getContainerEnvironmentResolver().createSettings(environment, node)) + .withEnvironment("CONTAINER_ENVIRONMENT_SETTINGS", containerEnvironmentResolver.createSettings(node)) .withUlimit("nofile", 262_144, 262_144) .withUlimit("nproc", 32_768, 409_600) .withUlimit("core", -1, -1) @@ -82,7 +87,7 @@ public class DockerOperationsImpl implements DockerOperations { .withAddCapability("SYS_ADMIN"); // Needed for perf - DockerNetworking networking = environment.getDockerNetworking(); + DockerNetworking networking = context.dockerNetworking(); command.withNetworkMode(networking.getDockerNetworkMode()); if (networking == DockerNetworking.NPT) { @@ -91,7 +96,7 @@ public class DockerOperationsImpl implements DockerOperations { command.withIpAddress(ipV6Local); // IPv4 - Only present for some containers - Optional<InetAddress> ipV4Local = environment.getIpAddresses().getIPv4Address(node.getHostname()) + Optional<InetAddress> ipV4Local = ipAddresses.getIPv4Address(node.getHostname()) .map(ipV4Address -> { InetAddress ipV4Prefix = InetAddresses.forString(IPV4_NPT_PREFIX); return IPAddresses.prefixTranslate(ipV4Address, ipV4Prefix, 2); @@ -265,7 +270,6 @@ public class DockerOperationsImpl implements DockerOperations { // Paths unique to each container List<Path> paths = new ArrayList<>(Arrays.asList( Paths.get("/etc/yamas-agent"), - Paths.get("/etc/filebeat"), context.pathInNodeUnderVespaHome("logs/daemontools_y"), context.pathInNodeUnderVespaHome("logs/jdisc_core"), context.pathInNodeUnderVespaHome("logs/langdetect/"), diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/maintenance/StorageMaintainer.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/maintenance/StorageMaintainer.java index 85ec3712126..6a9f0a69249 100644 --- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/maintenance/StorageMaintainer.java +++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/maintenance/StorageMaintainer.java @@ -1,17 +1,12 @@ // 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.maintenance; -import com.yahoo.collections.Pair; import com.yahoo.config.provision.NodeType; import com.yahoo.io.IOUtils; import com.yahoo.log.LogLevel; -import com.yahoo.system.ProcessExecuter; import com.yahoo.vespa.hosted.dockerapi.Container; import com.yahoo.vespa.hosted.node.admin.configserver.noderepository.NodeSpec; -import com.yahoo.vespa.hosted.node.admin.docker.DockerNetworking; import com.yahoo.vespa.hosted.node.admin.docker.DockerOperations; -import com.yahoo.vespa.hosted.node.admin.logging.FilebeatConfigProvider; -import com.yahoo.vespa.hosted.node.admin.component.Environment; import com.yahoo.vespa.hosted.node.admin.nodeagent.NodeAgentContext; import com.yahoo.vespa.hosted.node.admin.task.util.file.FileFinder; import com.yahoo.vespa.hosted.node.admin.task.util.file.UnixPath; @@ -27,7 +22,6 @@ import java.time.Instant; import java.time.ZoneOffset; import java.time.format.DateTimeFormatter; import java.util.ArrayList; -import java.util.Arrays; import java.util.Collections; import java.util.HashMap; import java.util.List; @@ -36,9 +30,7 @@ import java.util.Optional; import java.util.concurrent.TimeUnit; import java.util.logging.Logger; import java.util.regex.Pattern; -import java.util.stream.Stream; -import static com.yahoo.vespa.defaults.Defaults.getDefaults; import static com.yahoo.vespa.hosted.node.admin.task.util.file.FileFinder.nameMatches; import static com.yahoo.vespa.hosted.node.admin.task.util.file.FileFinder.olderThan; import static com.yahoo.vespa.hosted.node.admin.task.util.file.IOExceptionUtil.uncheck; @@ -52,16 +44,11 @@ public class StorageMaintainer { .ofPattern("yyyyMMddHHmmss").withZone(ZoneOffset.UTC); private final DockerOperations dockerOperations; - private final ProcessExecuter processExecuter; - private final Environment environment; private final CoredumpHandler coredumpHandler; private final Path archiveContainerStoragePath; - public StorageMaintainer(DockerOperations dockerOperations, ProcessExecuter processExecuter, - Environment environment, CoredumpHandler coredumpHandler, Path archiveContainerStoragePath) { + public StorageMaintainer(DockerOperations dockerOperations, CoredumpHandler coredumpHandler, Path archiveContainerStoragePath) { this.dockerOperations = dockerOperations; - this.processExecuter = processExecuter; - this.environment = environment; this.coredumpHandler = coredumpHandler; this.archiveContainerStoragePath = archiveContainerStoragePath; } @@ -72,37 +59,37 @@ public class StorageMaintainer { // host-life Path hostLifeCheckPath = context.pathInNodeUnderVespaHome("libexec/yms/yms_check_host_life"); SecretAgentCheckConfig hostLifeSchedule = new SecretAgentCheckConfig("host-life", 60, hostLifeCheckPath); - configs.add(annotatedCheck(node, hostLifeSchedule)); + configs.add(annotatedCheck(context, node, hostLifeSchedule)); // ntp Path ntpCheckPath = context.pathInNodeUnderVespaHome("libexec/yms/yms_check_ntp"); SecretAgentCheckConfig ntpSchedule = new SecretAgentCheckConfig("ntp", 60, ntpCheckPath); - configs.add(annotatedCheck(node, ntpSchedule)); + configs.add(annotatedCheck(context, node, ntpSchedule)); // coredumps (except for the done coredumps which is handled by the host) Path coredumpCheckPath = context.pathInNodeUnderVespaHome("libexec/yms/yms_check_coredumps"); SecretAgentCheckConfig coredumpSchedule = new SecretAgentCheckConfig("system-coredumps-processing", 300, coredumpCheckPath, "--application", "system-coredumps-processing", "--lastmin", "129600", "--crit", "1", "--coredir", context.pathInNodeUnderVespaHome("var/crash/processing").toString()); - configs.add(annotatedCheck(node, coredumpSchedule)); + configs.add(annotatedCheck(context, node, coredumpSchedule)); // athenz certificate check Path athenzCertExpiryCheckPath = context.pathInNodeUnderVespaHome("libexec64/yms/yms_check_athenz_certs"); SecretAgentCheckConfig athenzCertExpirySchedule = new SecretAgentCheckConfig("athenz-certificate-expiry", 60, athenzCertExpiryCheckPath, "--threshold", "20") .withRunAsUser("root"); - configs.add(annotatedCheck(node, athenzCertExpirySchedule)); + configs.add(annotatedCheck(context, node, athenzCertExpirySchedule)); if (context.nodeType() != NodeType.config) { // vespa-health Path vespaHealthCheckPath = context.pathInNodeUnderVespaHome("libexec/yms/yms_check_vespa_health"); SecretAgentCheckConfig vespaHealthSchedule = new SecretAgentCheckConfig("vespa-health", 60, vespaHealthCheckPath, "all"); - configs.add(annotatedCheck(node, vespaHealthSchedule)); + configs.add(annotatedCheck(context, node, vespaHealthSchedule)); // vespa Path vespaCheckPath = context.pathInNodeUnderVespaHome("libexec/yms/yms_check_vespa"); SecretAgentCheckConfig vespaSchedule = new SecretAgentCheckConfig("vespa", 60, vespaCheckPath, "all"); - configs.add(annotatedCheck(node, vespaSchedule)); + configs.add(annotatedCheck(context, node, vespaSchedule)); } if (context.nodeType() == NodeType.config) { @@ -110,14 +97,14 @@ public class StorageMaintainer { Path configServerCheckPath = context.pathInNodeUnderVespaHome("libexec/yms/yms_check_ymonsb2"); SecretAgentCheckConfig configServerSchedule = new SecretAgentCheckConfig("configserver", 60, configServerCheckPath, "-zero", "configserver"); - configs.add(annotatedCheck(node, configServerSchedule)); + configs.add(annotatedCheck(context, node, configServerSchedule)); //zkbackupage Path zkbackupCheckPath = context.pathInNodeUnderVespaHome("libexec/yamas2/yms_check_file_age.py"); SecretAgentCheckConfig zkbackupSchedule = new SecretAgentCheckConfig("zkbackupage", 300, zkbackupCheckPath, "-f", context.pathInNodeUnderVespaHome("var/vespa-hosted/zkbackup.stat").toString(), "-m", "150", "-a", "config-zkbackupage"); - configs.add(annotatedCheck(node, zkbackupSchedule)); + configs.add(annotatedCheck(context, node, zkbackupSchedule)); } if (context.nodeType() == NodeType.proxy) { @@ -126,13 +113,13 @@ public class StorageMaintainer { SecretAgentCheckConfig routingAgeSchedule = new SecretAgentCheckConfig("routing-configage", 60, routingAgeCheckPath, "-f", context.pathInNodeUnderVespaHome("var/vespa-hosted/routing/nginx.conf").toString(), "-m", "90", "-a", "routing-configage"); - configs.add(annotatedCheck(node, routingAgeSchedule)); + configs.add(annotatedCheck(context, node, routingAgeSchedule)); //ssl-check Path sslCheckPath = context.pathInNodeUnderVespaHome("libexec/yms/yms_check_ssl_status"); SecretAgentCheckConfig sslSchedule = new SecretAgentCheckConfig("ssl-status", 300, sslCheckPath, "-e", "localhost", "-p", "4443", "-t", "30"); - configs.add(annotatedCheck(node, sslSchedule)); + configs.add(annotatedCheck(context, node, sslSchedule)); } // Write config and restart yamas-agent @@ -141,14 +128,14 @@ public class StorageMaintainer { dockerOperations.executeCommandInContainerAsRoot(context, "service", "yamas-agent", "restart"); } - private SecretAgentCheckConfig annotatedCheck(NodeSpec node, SecretAgentCheckConfig check) { + private SecretAgentCheckConfig annotatedCheck(NodeAgentContext context, NodeSpec node, SecretAgentCheckConfig check) { check.withTag("namespace", "Vespa") .withTag("role", SecretAgentCheckConfig.nodeTypeToRole(node.getNodeType())) .withTag("flavor", node.getFlavor()) .withTag("canonicalFlavor", node.getCanonicalFlavor()) .withTag("state", node.getState().toString()) - .withTag("zone", environment.getZone()) - .withTag("parentHostname", environment.getParentHostHostname()); + .withTag("zone", String.format("%s.%s", context.zoneId().environment().value(), context.zoneId().regionName().value())); + node.getParentHostname().ifPresent(parent -> check.withTag("parentHostname", parent)); node.getOwner().ifPresent(owner -> check .withTag("tenantName", owner.getTenant()) .withTag("app", owner.getApplication() + "." + owner.getInstance()) @@ -163,20 +150,6 @@ public class StorageMaintainer { return check; } - public void writeFilebeatConfig(NodeAgentContext context, NodeSpec node) { - try { - FilebeatConfigProvider filebeatConfigProvider = new FilebeatConfigProvider(environment); - Optional<String> config = filebeatConfigProvider.getConfig(context, node); - if (!config.isPresent()) return; - - Path filebeatPath = context.pathOnHostFromPathInNode("/etc/filebeat/filebeat.yml"); - Files.write(filebeatPath, config.get().getBytes()); - context.log(logger, "Wrote filebeat config"); - } catch (Throwable t) { - context.log(logger, LogLevel.ERROR, "Failed writing filebeat config", t); - } - } - public Optional<Long> getDiskUsageFor(NodeAgentContext context) { Path containerDir = context.pathOnHostFromPathInNode("/"); try { @@ -241,20 +214,20 @@ public class StorageMaintainer { /** Checks if container has any new coredumps, reports and archives them if so */ public void handleCoreDumpsForContainer(NodeAgentContext context, NodeSpec node, Optional<Container> container) { - final Map<String, Object> nodeAttributes = getCoredumpNodeAttributes(node, container); + final Map<String, Object> nodeAttributes = getCoredumpNodeAttributes(context, node, container); coredumpHandler.converge(context, nodeAttributes); } - private Map<String, Object> getCoredumpNodeAttributes(NodeSpec node, Optional<Container> container) { + private Map<String, Object> getCoredumpNodeAttributes(NodeAgentContext context, NodeSpec node, Optional<Container> container) { Map<String, Object> attributes = new HashMap<>(); attributes.put("hostname", node.getHostname()); - attributes.put("parent_hostname", environment.getParentHostHostname()); - attributes.put("region", environment.getRegion()); - attributes.put("environment", environment.getEnvironment()); + attributes.put("region", context.zoneId().regionName()); + attributes.put("environment", context.zoneId().environment()); attributes.put("flavor", node.getFlavor()); attributes.put("kernel_version", System.getProperty("os.version")); container.map(c -> c.image).ifPresent(image -> attributes.put("docker_image", image.asString())); + node.getParentHostname().ifPresent(parent -> attributes.put("parent_hostname", parent)); node.getVespaVersion().ifPresent(version -> attributes.put("vespa_version", version)); node.getOwner().ifPresent(owner -> { attributes.put("tenant", owner.getTenant()); @@ -278,56 +251,4 @@ public class StorageMaintainer { new UnixPath(containerLogsOnHost).moveIfExists(containerLogsInArchiveDir); new UnixPath(context.pathOnHostFromPathInNode("/")).deleteRecursively(); } - - /** - * Runs node-maintainer's SpecVerifier and returns its output - * @param node Node specification containing the excepted values we want to verify against - * @return new combined hardware divergence - * @throws RuntimeException if exit code != 0 - */ - public String getHardwareDivergence(NodeSpec node) { - List<String> arguments = new ArrayList<>(Arrays.asList("specification", - "--disk", Double.toString(node.getMinDiskAvailableGb()), - "--memory", Double.toString(node.getMinMainMemoryAvailableGb()), - "--cpu_cores", Double.toString(node.getMinCpuCores()), - "--is_ssd", Boolean.toString(node.isFastDisk()), - "--bandwidth", Double.toString(node.getBandwidth()), - "--ips", String.join(",", node.getIpAddresses()))); - - if (environment.getDockerNetworking() == DockerNetworking.HOST_NETWORK) { - arguments.add("--skip-reverse-lookup"); - } - - node.getHardwareDivergence().ifPresent(hardwareDivergence -> { - arguments.add("--divergence"); - arguments.add(hardwareDivergence); - }); - - return executeMaintainer("com.yahoo.vespa.hosted.node.verification.Main", arguments.toArray(new String[0])); - } - - - private String executeMaintainer(String mainClass, String... args) { - String[] command = Stream.concat( - Stream.of("sudo", - "VESPA_HOSTNAME=" + getDefaults().vespaHostname(), - "VESPA_HOME=" + getDefaults().vespaHome(), - getDefaults().underVespaHome("libexec/vespa/node-admin/maintenance.sh"), - mainClass), - Stream.of(args)) - .toArray(String[]::new); - - try { - Pair<Integer, String> result = processExecuter.exec(command); - - if (result.getFirst() != 0) { - throw new RuntimeException( - String.format("Maintainer failed to execute command: %s, Exit code: %d, Stdout/stderr: %s", - Arrays.toString(command), result.getFirst(), result.getSecond())); - } - return result.getSecond().trim(); - } catch (IOException e) { - throw new RuntimeException("Failed to execute maintainer", e); - } - } } diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/maintenance/acl/AclMaintainer.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/maintenance/acl/AclMaintainer.java index 88fb5dcb90f..a733a52ea10 100644 --- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/maintenance/acl/AclMaintainer.java +++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/maintenance/acl/AclMaintainer.java @@ -2,11 +2,10 @@ package com.yahoo.vespa.hosted.node.admin.maintenance.acl; import com.google.common.net.InetAddresses; +import com.yahoo.config.provision.HostName; import com.yahoo.vespa.hosted.dockerapi.Container; -import com.yahoo.vespa.hosted.node.admin.component.Environment; import com.yahoo.vespa.hosted.node.admin.configserver.noderepository.Acl; import com.yahoo.vespa.hosted.node.admin.configserver.noderepository.NodeRepository; -import com.yahoo.vespa.hosted.node.admin.docker.DockerNetworking; import com.yahoo.vespa.hosted.node.admin.docker.DockerOperations; import com.yahoo.vespa.hosted.node.admin.task.util.network.IPAddresses; import com.yahoo.vespa.hosted.node.admin.task.util.network.IPVersion; @@ -31,23 +30,21 @@ import java.util.stream.Collectors; * @author mpolden * @author smorgrav */ -public class AclMaintainer implements Runnable { +public class AclMaintainer { private static final PrefixLogger log = PrefixLogger.getNodeAdminLogger(AclMaintainer.class); private final DockerOperations dockerOperations; private final NodeRepository nodeRepository; private final IPAddresses ipAddresses; - private final String nodeAdminHostname; - private final Environment environment; + private final String hostHostname; public AclMaintainer(DockerOperations dockerOperations, NodeRepository nodeRepository, - String nodeAdminHostname, IPAddresses ipAddresses, Environment environment) { + HostName hostHostname, IPAddresses ipAddresses) { this.dockerOperations = dockerOperations; this.nodeRepository = nodeRepository; this.ipAddresses = ipAddresses; - this.nodeAdminHostname = nodeAdminHostname; - this.environment = environment; + this.hostHostname = hostHostname.value(); } private void applyRedirect(Container container, InetAddress address) { @@ -68,21 +65,18 @@ public class AclMaintainer implements Runnable { } private synchronized void configureAcls() { - if (environment.getDockerNetworking() != DockerNetworking.NPT) return; - log.info("Configuring ACLs"); // Needed to potentially nail down when ACL maintainer stopped working Map<String, Container> runningContainers = dockerOperations .getAllManagedContainers().stream() .filter(container -> container.state.isRunning()) .collect(Collectors.toMap(container -> container.hostname, container -> container)); - nodeRepository.getAcls(nodeAdminHostname).entrySet().stream() + nodeRepository.getAcls(hostHostname).entrySet().stream() .filter(entry -> runningContainers.containsKey(entry.getKey())) .forEach(entry -> apply(runningContainers.get(entry.getKey()), entry.getValue())); } - @Override - public void run() { + public void converge() { try { configureAcls(); } catch (Throwable t) { diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeadmin/NodeAdminImpl.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeadmin/NodeAdminImpl.java index d436e214266..b1c1a99a90a 100644 --- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeadmin/NodeAdminImpl.java +++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeadmin/NodeAdminImpl.java @@ -7,7 +7,7 @@ import com.yahoo.vespa.hosted.dockerapi.metrics.Dimensions; import com.yahoo.vespa.hosted.dockerapi.metrics.GaugeWrapper; import com.yahoo.vespa.hosted.dockerapi.metrics.MetricReceiverWrapper; import com.yahoo.vespa.hosted.node.admin.configserver.noderepository.NodeSpec; -import com.yahoo.vespa.hosted.node.admin.docker.DockerOperations; +import com.yahoo.vespa.hosted.node.admin.maintenance.acl.AclMaintainer; import com.yahoo.vespa.hosted.node.admin.nodeagent.NodeAgent; import com.yahoo.vespa.hosted.node.admin.util.PrefixLogger; @@ -18,6 +18,7 @@ import java.util.HashSet; import java.util.LinkedHashMap; import java.util.List; import java.util.Map; +import java.util.Optional; import java.util.Set; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.Executors; @@ -39,7 +40,7 @@ public class NodeAdminImpl implements NodeAdmin { Executors.newScheduledThreadPool(1, ThreadFactoryFactory.getDaemonThreadFactory("metricsscheduler")); private final Function<String, NodeAgent> nodeAgentFactory; - private final Runnable aclMaintainer; + private final Optional<AclMaintainer> aclMaintainer; private final Clock clock; private boolean previousWantFrozen; @@ -51,16 +52,8 @@ public class NodeAdminImpl implements NodeAdmin { private final GaugeWrapper numberOfContainersInLoadImageState; private final CounterWrapper numberOfUnhandledExceptionsInNodeAgent; - public NodeAdminImpl(DockerOperations dockerOperations, - Function<String, NodeAgent> nodeAgentFactory, - Runnable aclMaintainer, - MetricReceiverWrapper metricReceiver, - Clock clock) { - this(nodeAgentFactory, aclMaintainer, metricReceiver, clock); - } - public NodeAdminImpl(Function<String, NodeAgent> nodeAgentFactory, - Runnable aclMaintainer, + Optional<AclMaintainer> aclMaintainer, MetricReceiverWrapper metricReceiver, Clock clock) { this.nodeAgentFactory = nodeAgentFactory; @@ -175,10 +168,12 @@ public class NodeAdminImpl implements NodeAdmin { } }, 10, 55, TimeUnit.SECONDS); - int delay = 120; // WARNING: Reducing this will increase the load on config servers. - aclScheduler.scheduleWithFixedDelay(() -> { - if (!isFrozen()) aclMaintainer.run(); - }, 30, delay, TimeUnit.SECONDS); + aclMaintainer.ifPresent(maintainer -> { + int delay = 120; // WARNING: Reducing this will increase the load on config servers. + aclScheduler.scheduleWithFixedDelay(() -> { + if (!isFrozen()) maintainer.converge(); + }, 30, delay, TimeUnit.SECONDS); + }); } @Override diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeadmin/NodeAdminStateUpdaterImpl.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeadmin/NodeAdminStateUpdaterImpl.java index 5b0a3c7771f..296745c8e37 100644 --- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeadmin/NodeAdminStateUpdaterImpl.java +++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeadmin/NodeAdminStateUpdaterImpl.java @@ -1,16 +1,11 @@ // 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.nodeadmin; -import com.yahoo.concurrent.ThreadFactoryFactory; -import com.yahoo.concurrent.classlock.ClassLock; -import com.yahoo.concurrent.classlock.ClassLocking; -import com.yahoo.concurrent.classlock.LockInterruptException; +import com.yahoo.config.provision.HostName; import com.yahoo.log.LogLevel; import com.yahoo.vespa.hosted.node.admin.configserver.noderepository.NodeSpec; import com.yahoo.vespa.hosted.node.admin.configserver.noderepository.NodeRepository; import com.yahoo.vespa.hosted.node.admin.configserver.orchestrator.Orchestrator; -import com.yahoo.vespa.hosted.node.admin.maintenance.StorageMaintainer; -import com.yahoo.vespa.hosted.node.admin.configserver.noderepository.NodeAttributes; import com.yahoo.vespa.hosted.node.admin.configserver.orchestrator.OrchestratorException; import com.yahoo.vespa.hosted.node.admin.provider.NodeAdminStateUpdater; import com.yahoo.vespa.hosted.node.admin.configserver.HttpException; @@ -24,11 +19,7 @@ import java.util.LinkedHashMap; import java.util.List; import java.util.Map; import java.util.Optional; -import java.util.concurrent.Executors; -import java.util.concurrent.ScheduledExecutorService; -import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicBoolean; -import java.util.logging.Level; import java.util.logging.Logger; import java.util.stream.Collectors; @@ -54,55 +45,34 @@ public class NodeAdminStateUpdaterImpl implements NodeAdminStateUpdater { private RuntimeException lastConvergenceException; private final Logger log = Logger.getLogger(NodeAdminStateUpdater.class.getName()); - private final ScheduledExecutorService specVerifierScheduler = - Executors.newScheduledThreadPool(1, ThreadFactoryFactory.getDaemonThreadFactory("specverifier")); private final Thread loopThread; private final NodeRepository nodeRepository; private final Orchestrator orchestrator; private final NodeAdmin nodeAdmin; private final Clock clock; - private final String dockerHostHostName; + private final String hostHostname; private final Duration nodeAdminConvergeStateInterval; - private final Optional<ClassLocking> classLocking; - private Optional<ClassLock> classLock = Optional.empty(); private Instant lastTick; public NodeAdminStateUpdaterImpl( NodeRepository nodeRepository, Orchestrator orchestrator, - StorageMaintainer storageMaintainer, NodeAdmin nodeAdmin, - String dockerHostHostName, + HostName hostHostname, Clock clock, - Duration nodeAdminConvergeStateInterval, - Optional<ClassLocking> classLocking) { - log.info(objectToString() + ": Creating object"); + Duration nodeAdminConvergeStateInterval) { this.nodeRepository = nodeRepository; this.orchestrator = orchestrator; this.nodeAdmin = nodeAdmin; - this.dockerHostHostName = dockerHostHostName; + this.hostHostname = hostHostname.value(); this.clock = clock; this.nodeAdminConvergeStateInterval = nodeAdminConvergeStateInterval; - this.classLocking = classLocking; this.lastTick = clock.instant(); this.loopThread = new Thread(() -> { - if (classLocking.isPresent()) { - log.info(objectToString() + ": Acquiring lock"); - try { - classLock = Optional.of(classLocking.get().lockWhile(NodeAdminStateUpdater.class, () -> !terminated.get())); - } catch (LockInterruptException e) { - classLock = Optional.empty(); - return; - } - } - - log.info(objectToString() + ": Starting threads and schedulers"); nodeAdmin.start(); - specVerifierScheduler.scheduleWithFixedDelay(() -> - updateHardwareDivergence(storageMaintainer), 5, 60, TimeUnit.MINUTES); while (! terminated.get()) { tick(); @@ -111,15 +81,11 @@ public class NodeAdminStateUpdaterImpl implements NodeAdminStateUpdater { this.loopThread.setName("tick-NodeAdminStateUpdater"); } - private String objectToString() { - return this.getClass().getSimpleName() + "@" + Integer.toString(System.identityHashCode(this)); - } - @Override public Map<String, Object> getDebugPage() { Map<String, Object> debug = new LinkedHashMap<>(); synchronized (monitor) { - debug.put("dockerHostHostName", dockerHostHostName); + debug.put("hostHostname", hostHostname); debug.put("wantedState", wantedState); debug.put("currentState", currentState); debug.put("NodeAdmin", nodeAdmin.debugInfo()); @@ -127,25 +93,6 @@ public class NodeAdminStateUpdaterImpl implements NodeAdminStateUpdater { return debug; } - private void updateHardwareDivergence(StorageMaintainer maintainer) { - if (currentState != RESUMED) return; - - try { - NodeSpec node = nodeRepository.getNode(dockerHostHostName); - if (node.getState() == Node.State.parked) return; - String hardwareDivergence = maintainer.getHardwareDivergence(node); - - // Only update hardware divergence if there is a change. - if (!node.getHardwareDivergence().orElse("null").equals(hardwareDivergence)) { - NodeAttributes nodeAttributes = new NodeAttributes().withHardwareDivergence(hardwareDivergence); - log.info("Updating hardware divergence to " + hardwareDivergence); - nodeRepository.updateNodeAttributes(dockerHostHostName, nodeAttributes); - } - } catch (RuntimeException e) { - log.log(Level.WARNING, "Failed to report hardware divergence", e); - } - } - @Override public void setResumeStateAndCheckIfResumed(State wantedState) { synchronized (monitor) { @@ -239,13 +186,13 @@ public class NodeAdminStateUpdaterImpl implements NodeAdminStateUpdater { throw new ConvergenceException("NodeAdmin is not yet " + (wantFrozen ? "frozen" : "unfrozen")); } - boolean hostIsActiveInNR = nodeRepository.getNode(dockerHostHostName).getState() == Node.State.active; + boolean hostIsActiveInNR = nodeRepository.getNode(hostHostname).getState() == Node.State.active; switch (wantedState) { case RESUMED: - if (hostIsActiveInNR) orchestrator.resume(dockerHostHostName); + if (hostIsActiveInNR) orchestrator.resume(hostHostname); break; case SUSPENDED_NODE_ADMIN: - if (hostIsActiveInNR) orchestrator.suspend(dockerHostHostName); + if (hostIsActiveInNR) orchestrator.suspend(hostHostname); break; case SUSPENDED: // Fetch active nodes from node repo before suspending nodes. @@ -257,9 +204,9 @@ public class NodeAdminStateUpdaterImpl implements NodeAdminStateUpdater { List<String> nodesInActiveState = getNodesInActiveState(); List<String> nodesToSuspend = new ArrayList<>(nodesInActiveState); - if (hostIsActiveInNR) nodesToSuspend.add(dockerHostHostName); + if (hostIsActiveInNR) nodesToSuspend.add(hostHostname); if (!nodesToSuspend.isEmpty()) { - orchestrator.suspend(dockerHostHostName, nodesToSuspend); + orchestrator.suspend(hostHostname, nodesToSuspend); log.info("Orchestrator allows suspension of " + nodesToSuspend); } @@ -281,7 +228,7 @@ public class NodeAdminStateUpdaterImpl implements NodeAdminStateUpdater { private void fetchContainersToRunFromNodeRepository() { try { - final List<NodeSpec> containersToRun = nodeRepository.getNodes(dockerHostHostName); + final List<NodeSpec> containersToRun = nodeRepository.getNodes(hostHostname); nodeAdmin.refreshContainersToRun(containersToRun); } catch (Exception e) { log.log(LogLevel.WARNING, "Failed to update which containers should be running", e); @@ -289,7 +236,7 @@ public class NodeAdminStateUpdaterImpl implements NodeAdminStateUpdater { } private List<String> getNodesInActiveState() { - return nodeRepository.getNodes(dockerHostHostName) + return nodeRepository.getNodes(hostHostname) .stream() .filter(node -> node.getState() == Node.State.active) .map(NodeSpec::getHostname) @@ -301,33 +248,22 @@ public class NodeAdminStateUpdaterImpl implements NodeAdminStateUpdater { } public void stop() { - log.info(objectToString() + ": Stop called"); if (!terminated.compareAndSet(false, true)) { throw new RuntimeException("Can not re-stop a node agent."); } - classLocking.ifPresent(ClassLocking::interrupt); - // First we need to stop NodeAdminStateUpdaterImpl thread to make sure no new NodeAgents are spawned signalWorkToBeDone(); - specVerifierScheduler.shutdown(); do { try { loopThread.join(); - specVerifierScheduler.awaitTermination(Long.MAX_VALUE, TimeUnit.NANOSECONDS); } catch (InterruptedException e1) { log.info("Interrupted while waiting for NodeAdminStateUpdater thread and specVerfierScheduler to shutdown"); } - } while (loopThread.isAlive() || !specVerifierScheduler.isTerminated()); + } while (loopThread.isAlive()); // Finally, stop NodeAdmin and all the NodeAgents nodeAdmin.stop(); - - classLock.ifPresent(lock -> { - log.info(objectToString() + ": Releasing lock"); - lock.close(); - }); - log.info(objectToString() + ": Stop complete"); } } 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 f65f371ff67..2025d422331 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 @@ -5,6 +5,8 @@ import com.yahoo.config.provision.NodeType; import com.yahoo.vespa.athenz.api.AthenzService; import com.yahoo.vespa.hosted.dockerapi.ContainerName; import com.yahoo.vespa.hosted.node.admin.component.TaskContext; +import com.yahoo.vespa.hosted.node.admin.component.ZoneId; +import com.yahoo.vespa.hosted.node.admin.docker.DockerNetworking; import java.nio.file.Path; import java.nio.file.Paths; @@ -19,6 +21,10 @@ public interface NodeAgentContext extends TaskContext { AthenzService identity(); + DockerNetworking dockerNetworking(); + + ZoneId zoneId(); + /** * This method is the inverse of {@link #pathInNodeFromPathOnHost(Path)}} 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 d3c8b145488..d1b5ec77c6a 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 @@ -1,13 +1,20 @@ package com.yahoo.vespa.hosted.node.admin.nodeagent; +import com.yahoo.config.provision.Environment; import com.yahoo.config.provision.HostName; import com.yahoo.config.provision.NodeType; +import com.yahoo.config.provision.RegionName; +import com.yahoo.config.provision.SystemName; import com.yahoo.vespa.athenz.api.AthenzService; import com.yahoo.vespa.hosted.dockerapi.ContainerName; +import com.yahoo.vespa.hosted.node.admin.component.ZoneId; +import com.yahoo.vespa.hosted.node.admin.docker.DockerNetworking; +import java.nio.file.FileSystem; import java.nio.file.Path; import java.nio.file.Paths; import java.util.Objects; +import java.util.Optional; import java.util.logging.Level; import java.util.logging.Logger; @@ -22,15 +29,20 @@ public class NodeAgentContextImpl implements NodeAgentContext { private final HostName hostName; private final NodeType nodeType; private final AthenzService identity; + private final DockerNetworking dockerNetworking; + private final ZoneId zoneId; private final Path pathToNodeRootOnHost; private final Path pathToVespaHome; public NodeAgentContextImpl(String hostname, NodeType nodeType, AthenzService identity, + DockerNetworking dockerNetworking, ZoneId zoneId, Path pathToContainerStorage, Path pathToVespaHome) { this.hostName = HostName.from(Objects.requireNonNull(hostname)); this.containerName = ContainerName.fromHostname(hostname); this.nodeType = Objects.requireNonNull(nodeType); this.identity = Objects.requireNonNull(identity); + this.dockerNetworking = Objects.requireNonNull(dockerNetworking); + this.zoneId = Objects.requireNonNull(zoneId); this.pathToNodeRootOnHost = Objects.requireNonNull(pathToContainerStorage).resolve(containerName.asString()); this.pathToVespaHome = Objects.requireNonNull(pathToVespaHome); this.logPrefix = containerName.asString() + ": "; @@ -57,6 +69,16 @@ public class NodeAgentContextImpl implements NodeAgentContext { } @Override + public DockerNetworking dockerNetworking() { + return dockerNetworking; + } + + @Override + public ZoneId zoneId() { + return zoneId; + } + + @Override public Path pathOnHostFromPathInNode(Path pathInNode) { if (! pathInNode.isAbsolute()) throw new IllegalArgumentException("Expected an absolute path in the container, got: " + pathInNode); @@ -97,4 +119,66 @@ public class NodeAgentContextImpl implements NodeAgentContext { public void log(Logger logger, Level level, String message, Throwable throwable) { logger.log(level, logPrefix + message, throwable); } + + + /** For testing only! */ + public static class Builder { + private final String hostname; + private NodeType nodeType; + private AthenzService identity; + private DockerNetworking dockerNetworking; + private ZoneId zoneId; + private Path pathToContainerStorage; + private Path pathToVespaHome; + + public Builder(String hostname) { + this.hostname = hostname; + } + + public Builder nodeType(NodeType nodeType) { + this.nodeType = nodeType; + return this; + } + + public Builder identity(AthenzService identity) { + this.identity = identity; + return this; + } + + public Builder dockerNetworking(DockerNetworking dockerNetworking) { + this.dockerNetworking = dockerNetworking; + return this; + } + + public Builder zoneId(ZoneId zoneId) { + this.zoneId = zoneId; + return this; + } + + public Builder pathToContainerStorage(Path pathToContainerStorage) { + this.pathToContainerStorage = pathToContainerStorage; + return this; + } + + public Builder pathToVespaHome(Path pathToVespaHome) { + this.pathToVespaHome = pathToVespaHome; + return this; + } + + public Builder fileSystem(FileSystem fileSystem) { + return pathToContainerStorage(fileSystem.getPath("/home/docker")); + } + + public NodeAgentContextImpl build() { + return new NodeAgentContextImpl( + hostname, + Optional.ofNullable(nodeType).orElse(NodeType.tenant), + Optional.ofNullable(identity).orElseGet(() -> new AthenzService("domain", "service")), + Optional.ofNullable(dockerNetworking).orElse(DockerNetworking.HOST_NETWORK), + Optional.ofNullable(zoneId).orElseGet(() -> new ZoneId(SystemName.dev, Environment.dev, RegionName.defaultName())), + Optional.ofNullable(pathToContainerStorage).orElseGet(() -> Paths.get("/home/docker")), + Optional.ofNullable(pathToVespaHome).orElseGet(() -> Paths.get("/opt/vespa")) + ); + } + } } 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 ef2c6ad2800..395b4d458a2 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 @@ -22,7 +22,7 @@ import com.yahoo.vespa.hosted.node.admin.maintenance.StorageMaintainer; import com.yahoo.vespa.hosted.node.admin.configserver.noderepository.NodeRepository; import com.yahoo.vespa.hosted.node.admin.configserver.orchestrator.Orchestrator; import com.yahoo.vespa.hosted.node.admin.configserver.orchestrator.OrchestratorException; -import com.yahoo.vespa.hosted.node.admin.component.Environment; +import com.yahoo.vespa.hosted.node.admin.maintenance.acl.AclMaintainer; import com.yahoo.vespa.hosted.node.admin.maintenance.identity.AthenzCredentialsMaintainer; import com.yahoo.vespa.hosted.provision.Node; @@ -72,11 +72,10 @@ public class NodeAgentImpl implements NodeAgent { private final Orchestrator orchestrator; private final DockerOperations dockerOperations; private final StorageMaintainer storageMaintainer; - private final Runnable aclMaintainer; - private final Environment environment; private final Clock clock; private final Duration timeBetweenEachConverge; private final Optional<AthenzCredentialsMaintainer> athenzCredentialsMaintainer; + private final Optional<AclMaintainer> aclMaintainer; private int numberOfUnhandledException = 0; private Instant lastConverge; @@ -116,22 +115,20 @@ public class NodeAgentImpl implements NodeAgent { final Orchestrator orchestrator, final DockerOperations dockerOperations, final StorageMaintainer storageMaintainer, - final Runnable aclMaintainer, - final Environment environment, final Clock clock, final Duration timeBetweenEachConverge, - final Optional<AthenzCredentialsMaintainer> athenzCredentialsMaintainer) { + final Optional<AthenzCredentialsMaintainer> athenzCredentialsMaintainer, + final Optional<AclMaintainer> aclMaintainer) { this.context = context; this.nodeRepository = nodeRepository; this.orchestrator = orchestrator; this.dockerOperations = dockerOperations; this.storageMaintainer = storageMaintainer; - this.aclMaintainer = aclMaintainer; - this.environment = environment; this.clock = clock; this.timeBetweenEachConverge = timeBetweenEachConverge; this.lastConverge = clock.instant(); this.athenzCredentialsMaintainer = athenzCredentialsMaintainer; + this.aclMaintainer = aclMaintainer; this.loopThread = new Thread(() -> { try { @@ -228,7 +225,6 @@ public class NodeAgentImpl implements NodeAgent { if (!hasResumedNode) { if (!currentFilebeatRestarter.isPresent()) { storageMaintainer.writeMetricsConfig(context, node); - storageMaintainer.writeFilebeatConfig(context, node); currentFilebeatRestarter = Optional.of(filebeatRestarter.scheduleWithFixedDelay( () -> serviceRestarter.accept("filebeat"), 1, 1, TimeUnit.DAYS)); } @@ -509,7 +505,7 @@ public class NodeAgentImpl implements NodeAgent { containerState = STARTING; startContainer(node); containerState = UNKNOWN; - aclMaintainer.run(); + aclMaintainer.ifPresent(AclMaintainer::converge); } verifyHealth(node); @@ -606,8 +602,8 @@ public class NodeAgentImpl implements NodeAgent { Dimensions.Builder dimensionsBuilder = new Dimensions.Builder() .add("host", context.hostname().value()) .add("role", "tenants") - .add("state", node.getState().toString()) - .add("parentHostname", environment.getParentHostHostname()); + .add("state", node.getState().toString()); + node.getParentHostname().ifPresent(parent -> dimensionsBuilder.add("parentHostname", parent)); node.getAllowedToBeDown().ifPresent(allowed -> dimensionsBuilder.add("orchestratorState", allowed ? "ALLOWED_TO_BE_DOWN" : "NO_REMARKS")); Dimensions dimensions = dimensionsBuilder.build(); diff --git a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/docker/DockerOperationsImplTest.java b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/docker/DockerOperationsImplTest.java index 760326f061e..2a4bf6bf488 100644 --- a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/docker/DockerOperationsImplTest.java +++ b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/docker/DockerOperationsImplTest.java @@ -9,17 +9,19 @@ import com.yahoo.vespa.hosted.dockerapi.ContainerName; import com.yahoo.vespa.hosted.dockerapi.Docker; import com.yahoo.vespa.hosted.dockerapi.DockerImage; import com.yahoo.vespa.hosted.dockerapi.ProcessResult; -import com.yahoo.vespa.hosted.node.admin.component.Environment; -import com.yahoo.vespa.hosted.node.admin.config.ConfigServerConfig; +import com.yahoo.vespa.hosted.node.admin.component.ContainerEnvironmentResolver; import com.yahoo.vespa.hosted.node.admin.nodeagent.ContainerData; import com.yahoo.vespa.hosted.node.admin.nodeagent.NodeAgentContext; -import com.yahoo.vespa.hosted.node.admin.nodeagent.NodeAgentContextImplTest; +import com.yahoo.vespa.hosted.node.admin.nodeagent.NodeAgentContextImpl; +import com.yahoo.vespa.hosted.node.admin.task.util.network.IPAddresses; +import com.yahoo.vespa.hosted.node.admin.task.util.network.IPAddressesMock; import org.junit.Test; import org.mockito.InOrder; import java.io.IOException; import java.net.InetAddress; import java.nio.file.Paths; +import java.util.Collections; import java.util.Optional; import java.util.OptionalLong; @@ -36,22 +38,16 @@ import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; public class DockerOperationsImplTest { - private final Environment environment = new Environment.Builder() - .configServerConfig(new ConfigServerConfig(new ConfigServerConfig.Builder())) - .region("us-east-1") - .environment("prod") - .system("main") - .cloud("mycloud") - .dockerNetworking(DockerNetworking.HOST_NETWORK) - .build(); private final Docker docker = mock(Docker.class); private final ProcessExecuter processExecuter = mock(ProcessExecuter.class); - private final DockerOperationsImpl dockerOperations - = new DockerOperationsImpl(docker, environment, processExecuter); + private final ContainerEnvironmentResolver containerEnvironmentResolver = node -> ""; + private final IPAddresses ipAddresses = new IPAddressesMock(); + private final DockerOperationsImpl dockerOperations = new DockerOperationsImpl( + docker, processExecuter, containerEnvironmentResolver, Collections.emptyList(), ipAddresses); @Test public void processResultFromNodeProgramWhenSuccess() { - final NodeAgentContext context = NodeAgentContextImplTest.nodeAgentFromHostname("container-123.domain.tld"); + final NodeAgentContext context = new NodeAgentContextImpl.Builder("container-123.domain.tld").build(); final ProcessResult actualResult = new ProcessResult(0, "output", "errors"); when(docker.executeInContainerAsUser(any(), any(), any(), anyVararg())) @@ -72,7 +68,7 @@ public class DockerOperationsImplTest { @Test(expected = RuntimeException.class) public void processResultFromNodeProgramWhenNonZeroExitCode() { - final NodeAgentContext context = NodeAgentContextImplTest.nodeAgentFromHostname("container-123.domain.tld"); + final NodeAgentContext context = new NodeAgentContextImpl.Builder("container-123.domain.tld").build(); final ProcessResult actualResult = new ProcessResult(3, "output", "errors"); when(docker.executeInContainerAsUser(any(), any(), any(), anyVararg())) @@ -105,10 +101,6 @@ public class DockerOperationsImplTest { InetAddress ipV6Local = InetAddresses.forString("::1"); InetAddress ipV4Local = InetAddresses.forString("127.0.0.1"); - DockerOperationsImpl dockerOperations = new DockerOperationsImpl( - docker, - environment, - processExecuter); dockerOperations.addEtcHosts(containerData, hostname, Optional.empty(), ipV6Local); verify(containerData, times(1)).addFile( 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 bca9dbc44fa..aa40d5d805b 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 @@ -2,25 +2,20 @@ package com.yahoo.vespa.hosted.node.admin.integrationTests; import com.yahoo.collections.Pair; -import com.yahoo.concurrent.classlock.ClassLocking; +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.hosted.dockerapi.Docker; import com.yahoo.vespa.hosted.dockerapi.metrics.MetricReceiverWrapper; import com.yahoo.vespa.hosted.node.admin.configserver.noderepository.NodeSpec; -import com.yahoo.vespa.hosted.node.admin.config.ConfigServerConfig; -import com.yahoo.vespa.hosted.node.admin.docker.DockerNetworking; import com.yahoo.vespa.hosted.node.admin.docker.DockerOperations; import com.yahoo.vespa.hosted.node.admin.docker.DockerOperationsImpl; -import com.yahoo.vespa.hosted.node.admin.maintenance.acl.AclMaintainer; import com.yahoo.vespa.hosted.node.admin.nodeadmin.NodeAdminImpl; import com.yahoo.vespa.hosted.node.admin.nodeadmin.NodeAdminStateUpdaterImpl; import com.yahoo.vespa.hosted.node.admin.nodeagent.NodeAgent; -import com.yahoo.vespa.hosted.node.admin.nodeagent.NodeAgentContextImplTest; +import com.yahoo.vespa.hosted.node.admin.nodeagent.NodeAgentContextImpl; import com.yahoo.vespa.hosted.node.admin.nodeagent.NodeAgentImpl; -import com.yahoo.vespa.hosted.node.admin.component.Environment; -import com.yahoo.vespa.hosted.node.admin.component.PathResolver; import com.yahoo.vespa.hosted.node.admin.task.util.network.IPAddressesMock; import com.yahoo.vespa.hosted.provision.Node; import com.yahoo.vespa.test.file.TestFileSystem; @@ -30,6 +25,7 @@ import java.nio.file.Path; import java.nio.file.Paths; import java.time.Clock; import java.time.Duration; +import java.util.Collections; import java.util.Optional; import java.util.function.Function; @@ -47,7 +43,7 @@ public class DockerTester implements AutoCloseable { private static final Duration NODE_ADMIN_CONVERGE_STATE_INTERVAL = Duration.ofMillis(10); private static final Path PATH_TO_VESPA_HOME = Paths.get("/opt/vespa"); static final String NODE_PROGRAM = PATH_TO_VESPA_HOME.resolve("bin/vespa-nodectl").toString(); - static final String DOCKER_HOST_HOSTNAME = "host.test.yahoo.com"; + static final HostName HOST_HOSTNAME = HostName.from("host.test.yahoo.com"); final CallOrderVerifier callOrderVerifier = new CallOrderVerifier(); final Docker dockerMock = new DockerMock(callOrderVerifier); @@ -60,26 +56,15 @@ public class DockerTester implements AutoCloseable { DockerTester() { IPAddressesMock ipAddresses = new IPAddressesMock(); - ipAddresses.addAddress(DOCKER_HOST_HOSTNAME, "1.1.1.1"); - ipAddresses.addAddress(DOCKER_HOST_HOSTNAME, "f000::"); + ipAddresses.addAddress(HOST_HOSTNAME.value(), "1.1.1.1"); + ipAddresses.addAddress(HOST_HOSTNAME.value(), "f000::"); for (int i = 1; i < 4; i++) ipAddresses.addAddress("host" + i + ".test.yahoo.com", "f000::" + i); ProcessExecuter processExecuter = mock(ProcessExecuter.class); uncheck(() -> when(processExecuter.exec(any(String[].class))).thenReturn(new Pair<>(0, ""))); - Environment environment = new Environment.Builder() - .configServerConfig(new ConfigServerConfig(new ConfigServerConfig.Builder())) - .ipAddresses(ipAddresses) - .region("us-east-1") - .environment("prod") - .system("main") - .cloud("mycloud") - .pathResolver(new PathResolver(PATH_TO_VESPA_HOME, Paths.get("/tmp"), Paths.get("/tmp"))) - .dockerNetworking(DockerNetworking.HOST_NETWORK) - .build(); - NodeSpec hostSpec = new NodeSpec.Builder() - .hostname(DOCKER_HOST_HOSTNAME) + .hostname(HOST_HOSTNAME.value()) .state(Node.State.active) .nodeType(NodeType.host) .flavor("default") @@ -89,26 +74,23 @@ public class DockerTester implements AutoCloseable { nodeRepositoryMock.updateNodeRepositoryNode(hostSpec); Clock clock = Clock.systemUTC(); - DockerOperations dockerOperations = new DockerOperationsImpl(dockerMock, environment, processExecuter); - StorageMaintainerMock storageMaintainer = new StorageMaintainerMock(dockerOperations, null, environment, callOrderVerifier); - AclMaintainer aclMaintainer = mock(AclMaintainer.class); - + DockerOperations dockerOperations = new DockerOperationsImpl(dockerMock, processExecuter, node -> "", Collections.emptyList(), ipAddresses); + StorageMaintainerMock storageMaintainer = new StorageMaintainerMock(dockerOperations, callOrderVerifier); MetricReceiverWrapper mr = new MetricReceiverWrapper(MetricReceiver.nullImplementation); Function<String, NodeAgent> nodeAgentFactory = (hostName) -> new NodeAgentImpl( - NodeAgentContextImplTest.nodeAgentFromHostname(fileSystem, hostName), nodeRepositoryMock, - orchestratorMock, dockerOperations, storageMaintainer, aclMaintainer, environment, clock, NODE_AGENT_SCAN_INTERVAL, Optional.empty()); - nodeAdmin = new NodeAdminImpl(nodeAgentFactory, aclMaintainer, mr, Clock.systemUTC()); - nodeAdminStateUpdater = new NodeAdminStateUpdaterImpl(nodeRepositoryMock, orchestratorMock, storageMaintainer, - nodeAdmin, DOCKER_HOST_HOSTNAME, clock, NODE_ADMIN_CONVERGE_STATE_INTERVAL, - Optional.of(new ClassLocking())); + new NodeAgentContextImpl.Builder(hostName).fileSystem(fileSystem).build(), nodeRepositoryMock, + orchestratorMock, dockerOperations, storageMaintainer, clock, NODE_AGENT_SCAN_INTERVAL, Optional.empty(), Optional.empty()); + nodeAdmin = new NodeAdminImpl(nodeAgentFactory, Optional.empty(), mr, Clock.systemUTC()); + nodeAdminStateUpdater = new NodeAdminStateUpdaterImpl(nodeRepositoryMock, orchestratorMock, + nodeAdmin, HOST_HOSTNAME, clock, NODE_ADMIN_CONVERGE_STATE_INTERVAL); nodeAdminStateUpdater.start(); } /** Adds a node to node-repository mock that is running on this host */ void addChildNodeRepositoryNode(NodeSpec nodeSpec) { nodeRepositoryMock.updateNodeRepositoryNode(new NodeSpec.Builder(nodeSpec) - .parentHostname(DOCKER_HOST_HOSTNAME) + .parentHostname(HOST_HOSTNAME.value()) .build()); } diff --git a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/integrationTests/StorageMaintainerMock.java b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/integrationTests/StorageMaintainerMock.java index e91787ca540..298c185266b 100644 --- a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/integrationTests/StorageMaintainerMock.java +++ b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/integrationTests/StorageMaintainerMock.java @@ -1,13 +1,11 @@ // 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.integrationTests; -import com.yahoo.system.ProcessExecuter; import com.yahoo.vespa.hosted.dockerapi.Container; import com.yahoo.vespa.hosted.node.admin.nodeagent.NodeAgentContext; import com.yahoo.vespa.hosted.node.admin.configserver.noderepository.NodeSpec; import com.yahoo.vespa.hosted.node.admin.docker.DockerOperations; import com.yahoo.vespa.hosted.node.admin.maintenance.StorageMaintainer; -import com.yahoo.vespa.hosted.node.admin.component.Environment; import java.util.Optional; @@ -17,8 +15,8 @@ import java.util.Optional; public class StorageMaintainerMock extends StorageMaintainer { private final CallOrderVerifier callOrderVerifier; - public StorageMaintainerMock(DockerOperations dockerOperations, ProcessExecuter processExecuter, Environment environment, CallOrderVerifier callOrderVerifier) { - super(dockerOperations, processExecuter, environment, null, null); + public StorageMaintainerMock(DockerOperations dockerOperations, CallOrderVerifier callOrderVerifier) { + super(dockerOperations, null, null); this.callOrderVerifier = callOrderVerifier; } diff --git a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/logging/FilebeatConfigProviderTest.java b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/logging/FilebeatConfigProviderTest.java index f7e10c3aa13..d4fadabe695 100644 --- a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/logging/FilebeatConfigProviderTest.java +++ b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/logging/FilebeatConfigProviderTest.java @@ -8,7 +8,7 @@ import com.yahoo.vespa.hosted.node.admin.component.Environment; import com.yahoo.vespa.hosted.node.admin.config.ConfigServerConfig; import com.yahoo.vespa.hosted.node.admin.docker.DockerNetworking; import com.yahoo.vespa.hosted.node.admin.nodeagent.NodeAgentContext; -import com.yahoo.vespa.hosted.node.admin.nodeagent.NodeAgentContextImplTest; +import com.yahoo.vespa.hosted.node.admin.nodeagent.NodeAgentContextImpl; import com.yahoo.vespa.hosted.provision.Node; import org.junit.Test; @@ -32,7 +32,7 @@ public class FilebeatConfigProviderTest { private static final String region = "us-north-1"; private static final String system = "main"; private static final List<String> logstashNodes = ImmutableList.of("logstash1", "logstash2"); - private final NodeAgentContext context = NodeAgentContextImplTest.nodeAgentFromHostname("node-123.hostname.tld"); + private final NodeAgentContext context = new NodeAgentContextImpl.Builder("node-123.hostname.tld").build(); @Test public void it_replaces_all_fields_correctly() { diff --git a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/maintenance/StorageMaintainerTest.java b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/maintenance/StorageMaintainerTest.java index 7c10e703bb5..42b161b8e2d 100644 --- a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/maintenance/StorageMaintainerTest.java +++ b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/maintenance/StorageMaintainerTest.java @@ -2,14 +2,9 @@ package com.yahoo.vespa.hosted.node.admin.maintenance; import com.google.common.collect.ImmutableSet; -import com.yahoo.system.ProcessExecuter; -import com.yahoo.vespa.hosted.node.admin.config.ConfigServerConfig; -import com.yahoo.vespa.hosted.node.admin.docker.DockerNetworking; import com.yahoo.vespa.hosted.node.admin.docker.DockerOperations; -import com.yahoo.vespa.hosted.node.admin.component.Environment; -import com.yahoo.vespa.hosted.node.admin.component.PathResolver; import com.yahoo.vespa.hosted.node.admin.nodeagent.NodeAgentContext; -import com.yahoo.vespa.hosted.node.admin.nodeagent.NodeAgentContextImplTest; +import com.yahoo.vespa.hosted.node.admin.nodeagent.NodeAgentContextImpl; import com.yahoo.vespa.hosted.node.admin.task.util.file.FileFinder; import com.yahoo.vespa.test.file.TestFileSystem; import org.junit.Rule; @@ -35,24 +30,14 @@ import static org.mockito.Mockito.mock; * @author dybis */ public class StorageMaintainerTest { - private final Environment environment = new Environment.Builder() - .configServerConfig(new ConfigServerConfig(new ConfigServerConfig.Builder())) - .region("us-east-1") - .environment("prod") - .system("main") - .cloud("mycloud") - .pathResolver(new PathResolver()) - .dockerNetworking(DockerNetworking.HOST_NETWORK) - .build(); private final DockerOperations docker = mock(DockerOperations.class); - private final ProcessExecuter processExecuter = mock(ProcessExecuter.class); @Rule public TemporaryFolder folder = new TemporaryFolder(); @Test public void testDiskUsed() throws IOException, InterruptedException { - StorageMaintainer storageMaintainer = new StorageMaintainer(docker, processExecuter, environment, null, null); + StorageMaintainer storageMaintainer = new StorageMaintainer(docker, null, null); int writeSize = 10000; Files.write(folder.newFile().toPath(), new byte[writeSize]); @@ -63,7 +48,7 @@ public class StorageMaintainerTest { @Test public void testNonExistingDiskUsed() throws IOException, InterruptedException { - StorageMaintainer storageMaintainer = new StorageMaintainer(docker, processExecuter, environment, null, null); + StorageMaintainer storageMaintainer = new StorageMaintainer(docker, null, null); long usedBytes = storageMaintainer.getDiskUsedInBytes(folder.getRoot().toPath().resolve("doesn't exist")); assertEquals(0L, usedBytes); } @@ -88,7 +73,7 @@ public class StorageMaintainerTest { // Archive container-1 - StorageMaintainer storageMaintainer = new StorageMaintainer(docker, processExecuter, environment, null, pathToArchiveDir); + StorageMaintainer storageMaintainer = new StorageMaintainer(docker, null, pathToArchiveDir); storageMaintainer.archiveNodeStorage(context1); // container-1 should be gone from container-storage @@ -112,7 +97,8 @@ public class StorageMaintainerTest { } private NodeAgentContext createNodeAgentContextAndContainerStorage(FileSystem fileSystem, String containerName) throws IOException { - NodeAgentContext context = NodeAgentContextImplTest.nodeAgentFromHostname(fileSystem, containerName + ".domain.tld"); + NodeAgentContext context = new NodeAgentContextImpl.Builder(containerName + ".domain.tld") + .fileSystem(fileSystem).build(); Path containerVespaHomeOnHost = context.pathOnHostFromPathInNode(context.pathInNodeUnderVespaHome("")); Files.createDirectories(context.pathOnHostFromPathInNode("/etc/something")); diff --git a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/maintenance/acl/AclMaintainerTest.java b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/maintenance/acl/AclMaintainerTest.java index 5b882461937..a270585bcaa 100644 --- a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/maintenance/acl/AclMaintainerTest.java +++ b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/maintenance/acl/AclMaintainerTest.java @@ -1,14 +1,13 @@ // 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.maintenance.acl; +import com.yahoo.config.provision.HostName; import com.yahoo.vespa.hosted.dockerapi.Container; import com.yahoo.vespa.hosted.dockerapi.ContainerName; import com.yahoo.vespa.hosted.dockerapi.DockerImage; import com.yahoo.vespa.hosted.dockerapi.ProcessResult; -import com.yahoo.vespa.hosted.node.admin.component.Environment; import com.yahoo.vespa.hosted.node.admin.configserver.noderepository.Acl; import com.yahoo.vespa.hosted.node.admin.configserver.noderepository.NodeRepository; -import com.yahoo.vespa.hosted.node.admin.docker.DockerNetworking; import com.yahoo.vespa.hosted.node.admin.docker.DockerOperations; import com.yahoo.vespa.hosted.node.admin.task.util.network.IPAddressesMock; import com.yahoo.vespa.hosted.node.admin.task.util.network.IPVersion; @@ -32,21 +31,18 @@ import static org.mockito.Mockito.when; public class AclMaintainerTest { - private static final String NODE_ADMIN_HOSTNAME = "node-admin.region-1.yahoo.com"; - + private final HostName hostHostname = HostName.from("node-admin.region-1.yahoo.com"); private final IPAddressesMock ipAddresses = new IPAddressesMock(); private final DockerOperations dockerOperations = mock(DockerOperations.class); private final NodeRepository nodeRepository = mock(NodeRepository.class); private final Map<String, Container> containers = new HashMap<>(); private final List<Container> containerList = new ArrayList<>(); - private final Environment env = mock(Environment.class); private final AclMaintainer aclMaintainer = - new AclMaintainer(dockerOperations, nodeRepository, NODE_ADMIN_HOSTNAME, ipAddresses, env); + new AclMaintainer(dockerOperations, nodeRepository, hostHostname, ipAddresses); @Before public void before() { when(dockerOperations.getAllManagedContainers()).thenReturn(containerList); - when(env.getDockerNetworking()).thenReturn(DockerNetworking.NPT); } @Test @@ -54,14 +50,14 @@ public class AclMaintainerTest { Container container = addContainer("container1", "container1.host.com", Container.State.RUNNING); Map<String, Acl> acls = makeAcl(container.hostname, "4321", "2001::1"); - when(nodeRepository.getAcls(NODE_ADMIN_HOSTNAME)).thenReturn(acls); + when(nodeRepository.getAcls(hostHostname.value())).thenReturn(acls); whenListRules(container.name, "filter", IPVersion.IPv6, ""); whenListRules(container.name, "filter", IPVersion.IPv4, ""); whenListRules(container.name, "nat", IPVersion.IPv4, ""); whenListRules(container.name, "nat", IPVersion.IPv6, ""); - aclMaintainer.run(); + aclMaintainer.converge(); verify(dockerOperations, times(1)).executeCommandInNetworkNamespace(eq(container.name), eq("iptables-restore"), anyVararg()); //we don;t have a ip4 address for the container so no redirect either verify(dockerOperations, times(2)).executeCommandInNetworkNamespace(eq(container.name), eq("ip6tables-restore"), anyVararg()); @@ -72,14 +68,14 @@ public class AclMaintainerTest { Container container = addContainer("container1", "container1.host.com", Container.State.RUNNING); Map<String, Acl> acls = makeAcl(container.hostname, "4321", "2001::1"); - when(nodeRepository.getAcls(NODE_ADMIN_HOSTNAME)).thenReturn(acls); + when(nodeRepository.getAcls(hostHostname.value())).thenReturn(acls); whenListRules(container.name, "filter", IPVersion.IPv6, ""); whenListRules(container.name, "filter", IPVersion.IPv4, ""); whenListRules(container.name, "nat", IPVersion.IPv4, ""); whenListRules(container.name, "nat", IPVersion.IPv6, ""); - aclMaintainer.run(); + aclMaintainer.converge(); verify(dockerOperations, times(1)).executeCommandInNetworkNamespace(eq(container.name), eq("iptables-restore"), anyVararg()); //we don;t have a ip4 address for the container so no redirect either verify(dockerOperations, times(2)).executeCommandInNetworkNamespace(eq(container.name), eq("ip6tables-restore"), anyVararg()); @@ -90,9 +86,9 @@ public class AclMaintainerTest { Container container = addContainer("container1", "container1.host.com", Container.State.EXITED); Map<String, Acl> acls = makeAcl(container.hostname, "4321", "2001::1"); - when(nodeRepository.getAcls(NODE_ADMIN_HOSTNAME)).thenReturn(acls); + when(nodeRepository.getAcls(hostHostname.value())).thenReturn(acls); - aclMaintainer.run(); + aclMaintainer.converge(); verify(dockerOperations, never()).executeCommandInNetworkNamespace(eq(container.name), anyVararg()); } @@ -102,7 +98,7 @@ public class AclMaintainerTest { Container container = addContainer("container1", "container1.host.com", Container.State.RUNNING); Map<String, Acl> acls = makeAcl(container.hostname, "4321,2345,22", "2001::1", "fd01:1234::4321"); - when(nodeRepository.getAcls(NODE_ADMIN_HOSTNAME)).thenReturn(acls); + when(nodeRepository.getAcls(hostHostname.value())).thenReturn(acls); String IPV6 = "-P INPUT ACCEPT\n" + "-P FORWARD ACCEPT\n" + @@ -125,7 +121,7 @@ public class AclMaintainerTest { whenListRules(container.name, "filter", IPVersion.IPv4, ""); //IPv4 will then differ from wanted whenListRules(container.name, "nat", IPVersion.IPv6, NATv6); - aclMaintainer.run(); + aclMaintainer.converge(); verify(dockerOperations, times(1)).executeCommandInNetworkNamespace(eq(container.name), eq("iptables-restore"), anyVararg()); verify(dockerOperations, never()).executeCommandInNetworkNamespace(eq(container.name), eq("ip6tables-restore"), anyVararg()); @@ -136,7 +132,7 @@ public class AclMaintainerTest { Container container = addContainer("container1", "container1.host.com", Container.State.RUNNING); Map<String, Acl> acls = makeAcl(container.hostname, "22,4443,2222", "2001::1", "192.64.13.2"); - when(nodeRepository.getAcls(NODE_ADMIN_HOSTNAME)).thenReturn(acls); + when(nodeRepository.getAcls(hostHostname.value())).thenReturn(acls); String IPV4_FILTER = "-P INPUT ACCEPT\n" + "-P FORWARD ACCEPT\n" + @@ -168,7 +164,7 @@ public class AclMaintainerTest { whenListRules(container.name, "nat", IPVersion.IPv6, IPV6_NAT); whenListRules(container.name, "filter", IPVersion.IPv4, IPV4_FILTER); - aclMaintainer.run(); + aclMaintainer.converge(); verify(dockerOperations, never()).executeCommandInNetworkNamespace(any(), eq("ip6tables-restore"), anyVararg()); verify(dockerOperations, never()).executeCommandInNetworkNamespace(any(), eq("iptables-restore"), anyVararg()); @@ -179,7 +175,7 @@ public class AclMaintainerTest { public void rollback_is_attempted_when_applying_acl_fail() { Container container = addContainer("container1", "container1.host.com", Container.State.RUNNING); Map<String, Acl> acls = makeAcl(container.hostname, "4321", "2001::1"); - when(nodeRepository.getAcls(NODE_ADMIN_HOSTNAME)).thenReturn(acls); + when(nodeRepository.getAcls(hostHostname.value())).thenReturn(acls); String IPV6_NAT = "-P PREROUTING ACCEPT\n" + "-P INPUT ACCEPT\n" + @@ -199,7 +195,7 @@ public class AclMaintainerTest { eq(container.name), eq("iptables-restore"), anyVararg())).thenThrow(new RuntimeException("iptables restore failed")); - aclMaintainer.run(); + aclMaintainer.converge(); verify(dockerOperations, times(1)).executeCommandInNetworkNamespace(eq(container.name), eq("ip6tables"), eq("-F"), eq("-t"), eq("filter")); diff --git a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/maintenance/coredump/CoreCollectorTest.java b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/maintenance/coredump/CoreCollectorTest.java index 0fed9433432..d809d9cbf96 100644 --- a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/maintenance/coredump/CoreCollectorTest.java +++ b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/maintenance/coredump/CoreCollectorTest.java @@ -4,7 +4,7 @@ package com.yahoo.vespa.hosted.node.admin.maintenance.coredump; import com.yahoo.vespa.hosted.dockerapi.ProcessResult; import com.yahoo.vespa.hosted.node.admin.docker.DockerOperations; import com.yahoo.vespa.hosted.node.admin.nodeagent.NodeAgentContext; -import com.yahoo.vespa.hosted.node.admin.nodeagent.NodeAgentContextImplTest; +import com.yahoo.vespa.hosted.node.admin.nodeagent.NodeAgentContextImpl; import org.junit.Test; import java.nio.file.Path; @@ -27,7 +27,7 @@ public class CoreCollectorTest { private final String GDB_PATH = "/my/path/to/gdb"; private final DockerOperations docker = mock(DockerOperations.class); private final CoreCollector coreCollector = new CoreCollector(docker, Paths.get(GDB_PATH)); - private final NodeAgentContext context = NodeAgentContextImplTest.nodeAgentFromHostname("container-123.domain.tld"); + private final NodeAgentContext context = new NodeAgentContextImpl.Builder("container-123.domain.tld").build(); private final Path TEST_CORE_PATH = Paths.get("/tmp/core.1234"); private final Path TEST_BIN_PATH = Paths.get("/usr/bin/program"); diff --git a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/maintenance/coredump/CoredumpHandlerTest.java b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/maintenance/coredump/CoredumpHandlerTest.java index d567cac9a70..ca8435c9edd 100644 --- a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/maintenance/coredump/CoredumpHandlerTest.java +++ b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/maintenance/coredump/CoredumpHandlerTest.java @@ -3,7 +3,7 @@ package com.yahoo.vespa.hosted.node.admin.maintenance.coredump; import com.google.common.collect.ImmutableMap; import com.yahoo.vespa.hosted.node.admin.nodeagent.NodeAgentContext; -import com.yahoo.vespa.hosted.node.admin.nodeagent.NodeAgentContextImplTest; +import com.yahoo.vespa.hosted.node.admin.nodeagent.NodeAgentContextImpl; import com.yahoo.vespa.hosted.node.admin.task.util.file.UnixPath; import com.yahoo.vespa.hosted.node.admin.task.util.process.TestChildProcess2; import com.yahoo.vespa.hosted.node.admin.task.util.process.TestTerminal; @@ -47,7 +47,8 @@ import static org.mockito.Mockito.when; public class CoredumpHandlerTest { private final FileSystem fileSystem = TestFileSystem.create(); private final Path donePath = fileSystem.getPath("/home/docker/dumps"); - private final NodeAgentContext context = NodeAgentContextImplTest.nodeAgentFromHostname(fileSystem, "container-123.domain.tld"); + private final NodeAgentContext context = new NodeAgentContextImpl.Builder("container-123.domain.tld") + .fileSystem(fileSystem).build(); private final Path crashPathInContainer = Paths.get("/var/crash"); private final Path doneCoredumpsPath = fileSystem.getPath("/home/docker/dumps"); diff --git a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/nodeadmin/NodeAdminImplTest.java b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/nodeadmin/NodeAdminImplTest.java index 2431433b17f..bb229a10f59 100644 --- a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/nodeadmin/NodeAdminImplTest.java +++ b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/nodeadmin/NodeAdminImplTest.java @@ -14,6 +14,7 @@ import java.util.ArrayList; import java.util.Collections; import java.util.HashSet; import java.util.List; +import java.util.Optional; import java.util.Set; import java.util.function.Function; @@ -37,10 +38,9 @@ public class NodeAdminImplTest { // Trick to allow mocking of typed interface without casts/warnings. private interface NodeAgentFactory extends Function<String, NodeAgent> {} private final Function<String, NodeAgent> nodeAgentFactory = mock(NodeAgentFactory.class); - private final Runnable aclMaintainer = mock(Runnable.class); private final ManualClock clock = new ManualClock(); - private final NodeAdminImpl nodeAdmin = new NodeAdminImpl(nodeAgentFactory, aclMaintainer, + private final NodeAdminImpl nodeAdmin = new NodeAdminImpl(nodeAgentFactory, Optional.empty(), new MetricReceiverWrapper(MetricReceiver.nullImplementation), clock); @Test diff --git a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/nodeadmin/NodeAdminStateUpdaterImplTest.java b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/nodeadmin/NodeAdminStateUpdaterImplTest.java index fd54773910d..82f2408b252 100644 --- a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/nodeadmin/NodeAdminStateUpdaterImplTest.java +++ b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/nodeadmin/NodeAdminStateUpdaterImplTest.java @@ -1,10 +1,10 @@ // 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.nodeadmin; +import com.yahoo.config.provision.HostName; import com.yahoo.config.provision.NodeType; import com.yahoo.test.ManualClock; import com.yahoo.vespa.hosted.node.admin.configserver.noderepository.NodeSpec; -import com.yahoo.vespa.hosted.node.admin.maintenance.StorageMaintainer; import com.yahoo.vespa.hosted.node.admin.configserver.noderepository.NodeRepository; import com.yahoo.vespa.hosted.node.admin.configserver.orchestrator.Orchestrator; import com.yahoo.vespa.hosted.node.admin.configserver.orchestrator.OrchestratorException; @@ -15,7 +15,6 @@ import org.junit.Test; import java.time.Duration; import java.util.ArrayList; import java.util.List; -import java.util.Optional; import java.util.stream.Collectors; import java.util.stream.IntStream; @@ -44,33 +43,31 @@ import static org.mockito.Mockito.when; public class NodeAdminStateUpdaterImplTest { private final NodeRepository nodeRepository = mock(NodeRepository.class); private final Orchestrator orchestrator = mock(Orchestrator.class); - private final StorageMaintainer storageMaintainer = mock(StorageMaintainer.class); private final NodeAdmin nodeAdmin = mock(NodeAdmin.class); - private final String parentHostname = "basehost1.test.yahoo.com"; + private final HostName hostHostname = HostName.from("basehost1.test.yahoo.com"); private final ManualClock clock = new ManualClock(); private final Duration convergeStateInterval = Duration.ofSeconds(30); private final NodeAdminStateUpdaterImpl refresher = spy(new NodeAdminStateUpdaterImpl( - nodeRepository, orchestrator, storageMaintainer, nodeAdmin, parentHostname, clock, - convergeStateInterval, Optional.empty())); + nodeRepository, orchestrator, nodeAdmin, hostHostname, clock, convergeStateInterval)); @Test public void testStateConvergence() { mockNodeRepo(Node.State.active, 4); - List<String> activeHostnames = nodeRepository.getNodes(parentHostname).stream() + List<String> activeHostnames = nodeRepository.getNodes(hostHostname.value()).stream() .map(NodeSpec::getHostname) .collect(Collectors.toList()); List<String> suspendHostnames = new ArrayList<>(activeHostnames); - suspendHostnames.add(parentHostname); + suspendHostnames.add(hostHostname.value()); // Initially everything is frozen to force convergence assertResumeStateError(NodeAdminStateUpdater.State.RESUMED, TRANSITION_EXCEPTION_MESSAGE); when(nodeAdmin.setFrozen(eq(false))).thenReturn(true); - doNothing().when(orchestrator).resume(parentHostname); + doNothing().when(orchestrator).resume(hostHostname.value()); tickAfter(0); // The first tick should unfreeze - verify(orchestrator, times(1)).resume(parentHostname); // Resume host - verify(orchestrator, times(1)).resume(parentHostname); + verify(orchestrator, times(1)).resume(hostHostname.value()); // Resume host + verify(orchestrator, times(1)).resume(hostHostname.value()); // Everything is running and we want to continue running, therefore we have converged refresher.setResumeStateAndCheckIfResumed(NodeAdminStateUpdater.State.RESUMED); @@ -78,7 +75,7 @@ public class NodeAdminStateUpdaterImplTest { tickAfter(35); refresher.setResumeStateAndCheckIfResumed(NodeAdminStateUpdater.State.RESUMED); verify(refresher, never()).signalWorkToBeDone(); // No attempt in changing state - verify(orchestrator, times(1)).resume(parentHostname); // Already resumed + verify(orchestrator, times(1)).resume(hostHostname.value()); // Already resumed // Lets try to suspend node admin only, immediately we get false back, and need to wait until next // tick before any change can happen @@ -100,7 +97,7 @@ public class NodeAdminStateUpdaterImplTest { when(nodeAdmin.setFrozen(eq(true))).thenReturn(true); when(nodeAdmin.subsystemFreezeDuration()).thenReturn(Duration.ofSeconds(1)); doThrow(new RuntimeException(exceptionMessage)) - .when(orchestrator).suspend(eq(parentHostname)); + .when(orchestrator).suspend(eq(hostHostname.value())); tickAfter(35); assertResumeStateError(NodeAdminStateUpdater.State.SUSPENDED_NODE_ADMIN, exceptionMessage); verify(refresher, times(1)).signalWorkToBeDone(); @@ -111,7 +108,7 @@ public class NodeAdminStateUpdaterImplTest { when(nodeAdmin.setFrozen(eq(true))).thenReturn(true); when(nodeAdmin.subsystemFreezeDuration()).thenReturn(NodeAdminStateUpdaterImpl.FREEZE_CONVERGENCE_TIMEOUT.plusMinutes(1)); doThrow(new RuntimeException(exceptionMessage)).doNothing() - .when(orchestrator).suspend(eq(parentHostname)); + .when(orchestrator).suspend(eq(hostHostname.value())); tickAfter(35); assertResumeStateError(NodeAdminStateUpdater.State.SUSPENDED_NODE_ADMIN, exceptionMessage); verify(refresher, times(1)).signalWorkToBeDone(); @@ -122,12 +119,12 @@ public class NodeAdminStateUpdaterImplTest { verify(nodeAdmin, times(2)).setFrozen(eq(false)); // At this point orchestrator will say its OK to suspend, but something goes wrong when we try to stop services - verify(orchestrator, times(0)).suspend(eq(parentHostname), eq(suspendHostnames)); + verify(orchestrator, times(0)).suspend(eq(hostHostname.value()), eq(suspendHostnames)); doThrow(new RuntimeException("Failed to stop services")).doNothing().when(nodeAdmin).stopNodeAgentServices(eq(activeHostnames)); when(nodeAdmin.subsystemFreezeDuration()).thenReturn(Duration.ofSeconds(1)); assertResumeStateError(NodeAdminStateUpdater.State.SUSPENDED, TRANSITION_EXCEPTION_MESSAGE); tickAfter(0); // Change in wanted state, no need to wait - verify(orchestrator, times(1)).suspend(eq(parentHostname), eq(suspendHostnames)); + verify(orchestrator, times(1)).suspend(eq(hostHostname.value()), eq(suspendHostnames)); verify(refresher, times(2)).signalWorkToBeDone(); // No change in desired state // Make sure we dont roll back if we fail to stop services - we will try to stop again next tick verify(nodeAdmin, times(2)).setFrozen(eq(false)); @@ -150,8 +147,8 @@ public class NodeAdminStateUpdaterImplTest { tickAfter(35); assertResumeStateError(NodeAdminStateUpdater.State.RESUMED, "NodeAdmin is not yet unfrozen"); - doThrow(new OrchestratorException("Cannot allow to suspend " + parentHostname)).doNothing() - .when(orchestrator).resume(parentHostname); + doThrow(new OrchestratorException("Cannot allow to suspend " + hostHostname.value())).doNothing() + .when(orchestrator).resume(hostHostname.value()); tickAfter(35); assertResumeStateError(NodeAdminStateUpdater.State.RESUMED, "Cannot allow to suspend basehost1.test.yahoo.com"); tickAfter(35); @@ -165,7 +162,7 @@ public class NodeAdminStateUpdaterImplTest { // Initially everything is frozen to force convergence when(nodeAdmin.setFrozen(eq(false))).thenReturn(true); - doNothing().when(orchestrator).resume(parentHostname); + doNothing().when(orchestrator).resume(hostHostname.value()); tickAfter(0); // The first tick should unfreeze refresher.setResumeStateAndCheckIfResumed(NodeAdminStateUpdater.State.RESUMED); verify(nodeAdmin, times(1)).setFrozen(eq(false)); @@ -173,7 +170,7 @@ public class NodeAdminStateUpdaterImplTest { // Let's start suspending, we are able to freeze the nodes, but orchestrator denies suspension when(nodeAdmin.subsystemFreezeDuration()).thenReturn(Duration.ofSeconds(1)); when(nodeAdmin.setFrozen(eq(true))).thenReturn(true); - doThrow(new RuntimeException(exceptionMsg)).when(orchestrator).suspend(eq(parentHostname)); + doThrow(new RuntimeException(exceptionMsg)).when(orchestrator).suspend(eq(hostHostname.value())); assertResumeStateError(NodeAdminStateUpdater.State.SUSPENDED_NODE_ADMIN, TRANSITION_EXCEPTION_MESSAGE); tickAfter(30); @@ -207,21 +204,21 @@ public class NodeAdminStateUpdaterImplTest { // orchestrator to resume/suspend host. Therefore, if host is not active, we only need to freeze. tickAfter(0); refresher.setResumeStateAndCheckIfResumed(NodeAdminStateUpdater.State.RESUMED); - verify(orchestrator, never()).resume(eq(parentHostname)); + verify(orchestrator, never()).resume(eq(hostHostname.value())); assertResumeStateError(NodeAdminStateUpdater.State.SUSPENDED_NODE_ADMIN, TRANSITION_EXCEPTION_MESSAGE); tickAfter(0); refresher.setResumeStateAndCheckIfResumed(NodeAdminStateUpdater.State.SUSPENDED_NODE_ADMIN); - verify(orchestrator, never()).suspend(eq(parentHostname)); + verify(orchestrator, never()).suspend(eq(hostHostname.value())); // When doing batch suspend, only suspend the containers if the host is not active - List<String> activeHostnames = nodeRepository.getNodes(parentHostname).stream() + List<String> activeHostnames = nodeRepository.getNodes(hostHostname.value()).stream() .map(NodeSpec::getHostname) .collect(Collectors.toList()); assertResumeStateError(NodeAdminStateUpdater.State.SUSPENDED, TRANSITION_EXCEPTION_MESSAGE); tickAfter(0); refresher.setResumeStateAndCheckIfResumed(NodeAdminStateUpdater.State.SUSPENDED); - verify(orchestrator, times(1)).suspend(eq(parentHostname), eq(activeHostnames)); + verify(orchestrator, times(1)).suspend(eq(hostHostname.value()), eq(activeHostnames)); } private void assertResumeStateError(NodeAdminStateUpdater.State targetState, String reason) { @@ -246,10 +243,10 @@ public class NodeAdminStateUpdaterImplTest { .build()) .collect(Collectors.toList()); - when(nodeRepository.getNodes(eq(parentHostname))).thenReturn(containersToRun); + when(nodeRepository.getNodes(eq(hostHostname.value()))).thenReturn(containersToRun); - when(nodeRepository.getNode(eq(parentHostname))).thenReturn(new NodeSpec.Builder() - .hostname(parentHostname) + when(nodeRepository.getNode(eq(hostHostname.value()))).thenReturn(new NodeSpec.Builder() + .hostname(hostHostname.value()) .state(hostState) .nodeType(NodeType.tenant) .flavor("default") diff --git a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/nodeagent/NodeAgentContextImplTest.java b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/nodeagent/NodeAgentContextImplTest.java index 31ac8d1c114..84f13ed299a 100644 --- a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/nodeagent/NodeAgentContextImplTest.java +++ b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/nodeagent/NodeAgentContextImplTest.java @@ -1,13 +1,9 @@ package com.yahoo.vespa.hosted.node.admin.nodeagent; -import com.yahoo.config.provision.NodeType; -import com.yahoo.vespa.athenz.api.AthenzService; import com.yahoo.vespa.test.file.TestFileSystem; import org.junit.Test; import java.nio.file.FileSystem; -import java.nio.file.Path; -import java.nio.file.Paths; import static org.junit.Assert.assertEquals; @@ -16,7 +12,8 @@ import static org.junit.Assert.assertEquals; */ public class NodeAgentContextImplTest { private final FileSystem fileSystem = TestFileSystem.create(); - private final NodeAgentContext context = nodeAgentFromHostname(fileSystem, "container-1.domain.tld"); + private final NodeAgentContext context = new NodeAgentContextImpl.Builder("container-1.domain.tld") + .fileSystem(fileSystem).build(); @Test public void path_on_host_from_path_in_node_test() { @@ -71,18 +68,4 @@ public class NodeAgentContextImplTest { public void path_under_vespa_home_must_be_relative() { context.pathInNodeUnderVespaHome("/home"); } - - - public static NodeAgentContext nodeAgentFromHostname(String hostname) { - FileSystem fileSystem = TestFileSystem.create(); - return nodeAgentFromHostname(fileSystem, hostname); - } - - public static NodeAgentContext nodeAgentFromHostname(FileSystem fileSystem, String hostname) { - final Path vespaHomeInContainer = Paths.get("/opt/vespa"); - final Path containerStoragePath = fileSystem.getPath("/home/docker"); - - return new NodeAgentContextImpl(hostname, NodeType.tenant, new AthenzService("domain", "service"), - containerStoragePath, vespaHomeInContainer); - } } 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 5cc881fe62d..00326f90caa 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 @@ -12,22 +12,17 @@ import com.yahoo.vespa.hosted.dockerapi.exception.DockerException; import com.yahoo.vespa.hosted.dockerapi.DockerImage; import com.yahoo.vespa.hosted.dockerapi.metrics.MetricReceiverWrapper; import com.yahoo.vespa.hosted.node.admin.configserver.noderepository.NodeSpec; -import com.yahoo.vespa.hosted.node.admin.config.ConfigServerConfig; import com.yahoo.vespa.hosted.node.admin.configserver.noderepository.NodeAttributes; -import com.yahoo.vespa.hosted.node.admin.docker.DockerNetworking; import com.yahoo.vespa.hosted.node.admin.docker.DockerOperations; import com.yahoo.vespa.hosted.node.admin.maintenance.StorageMaintainer; import com.yahoo.vespa.hosted.node.admin.maintenance.acl.AclMaintainer; import com.yahoo.vespa.hosted.node.admin.configserver.noderepository.NodeRepository; import com.yahoo.vespa.hosted.node.admin.configserver.orchestrator.Orchestrator; -import com.yahoo.vespa.hosted.node.admin.component.Environment; import com.yahoo.vespa.hosted.node.admin.maintenance.identity.AthenzCredentialsMaintainer; -import com.yahoo.vespa.hosted.node.admin.component.PathResolver; import com.yahoo.vespa.hosted.provision.Node; import org.junit.Test; import org.mockito.InOrder; -import java.io.IOException; import java.net.URL; import java.nio.file.Files; import java.nio.file.Path; @@ -68,7 +63,7 @@ public class NodeAgentImplTest { private static final String vespaVersion = "1.2.3"; private final String hostName = "host1.test.yahoo.com"; - private final NodeAgentContext context = NodeAgentContextImplTest.nodeAgentFromHostname(hostName); + private final NodeAgentContext context = new NodeAgentContextImpl.Builder(hostName).build(); private final DockerImage dockerImage = new DockerImage("dockerImage"); private final DockerOperations dockerOperations = mock(DockerOperations.class); private final NodeRepository nodeRepository = mock(NodeRepository.class); @@ -80,18 +75,7 @@ public class NodeAgentImplTest { Collections.emptyMap(), Collections.emptyMap(), Collections.emptyMap()); private final AthenzCredentialsMaintainer athenzCredentialsMaintainer = mock(AthenzCredentialsMaintainer.class); - private final PathResolver pathResolver = mock(PathResolver.class); private final ManualClock clock = new ManualClock(); - private final Environment environment = new Environment.Builder() - .configServerConfig(new ConfigServerConfig(new ConfigServerConfig.Builder())) - .environment("dev") - .region("us-east-1") - .system("main") - .cloud("mycloud") - .parentHostHostname("parent.host.name.yahoo.com") - .pathResolver(pathResolver) - .dockerNetworking(DockerNetworking.HOST_NETWORK) - .build(); private final NodeSpec.Builder nodeBuilder = new NodeSpec.Builder() .hostname(context.hostname().value()) @@ -196,7 +180,7 @@ public class NodeAgentImplTest { } @Test - public void absentContainerCausesStart() throws Exception { + public void absentContainerCausesStart() { final Optional<Long> restartGeneration = Optional.of(1L); final long rebootGeneration = 0; final NodeSpec node = nodeBuilder @@ -211,8 +195,6 @@ public class NodeAgentImplTest { NodeAgentImpl nodeAgent = makeNodeAgent(null, false); when(nodeRepository.getOptionalNode(hostName)).thenReturn(Optional.of(node)); - when(pathResolver.getApplicationStoragePathForNodeAdmin()).thenReturn(Files.createTempDirectory("foo")); - when(pathResolver.getApplicationStoragePathForHost()).thenReturn(Files.createTempDirectory("bar")); when(dockerOperations.pullImageAsyncIfNeeded(eq(dockerImage))).thenReturn(false); when(storageMaintainer.getDiskUsageFor(eq(context))).thenReturn(Optional.of(201326592000L)); @@ -226,7 +208,7 @@ public class NodeAgentImplTest { inOrder.verify(dockerOperations, times(1)).pullImageAsyncIfNeeded(eq(dockerImage)); inOrder.verify(dockerOperations, times(1)).createContainer(eq(context), eq(node), any()); inOrder.verify(dockerOperations, times(1)).startContainer(eq(context)); - inOrder.verify(aclMaintainer, times(1)).run(); + inOrder.verify(aclMaintainer, times(1)).converge(); inOrder.verify(dockerOperations, times(1)).resumeNode(eq(context)); inOrder.verify(nodeRepository).updateNodeAttributes( hostName, new NodeAttributes() @@ -268,7 +250,7 @@ public class NodeAgentImplTest { } @Test - public void containerIsRestartedIfFlavorChanged() throws IOException { + public void containerIsRestartedIfFlavorChanged() { final long wantedRestartGeneration = 1; final long currentRestartGeneration = 1; NodeSpec.Builder specBuilder = nodeBuilder @@ -291,7 +273,6 @@ public class NodeAgentImplTest { .thenReturn(Optional.of(thirdSpec)); when(dockerOperations.pullImageAsyncIfNeeded(any())).thenReturn(true); when(storageMaintainer.getDiskUsageFor(eq(context))).thenReturn(Optional.of(201326592000L)); - when(pathResolver.getApplicationStoragePathForHost()).thenReturn(Files.createTempDirectory("bar")); nodeAgent.converge(); nodeAgent.converge(); @@ -501,7 +482,7 @@ public class NodeAgentImplTest { } @Test - public void testRestartDeadContainerAfterNodeAdminRestart() throws IOException { + public void testRestartDeadContainerAfterNodeAdminRestart() { final NodeSpec node = nodeBuilder .currentDockerImage(dockerImage) .wantedDockerImage(dockerImage) @@ -512,8 +493,6 @@ public class NodeAgentImplTest { NodeAgentImpl nodeAgent = makeNodeAgent(dockerImage, false); when(nodeRepository.getOptionalNode(eq(hostName))).thenReturn(Optional.of(node)); - when(pathResolver.getApplicationStoragePathForNodeAdmin()).thenReturn(Files.createTempDirectory("foo")); - when(pathResolver.getApplicationStoragePathForHost()).thenReturn(Files.createTempDirectory("bar")); when(storageMaintainer.getDiskUsageFor(eq(context))).thenReturn(Optional.of(201326592000L)); nodeAgent.tick(); @@ -590,7 +569,7 @@ public class NodeAgentImplTest { } @Test - public void start_container_subtask_failure_leads_to_container_restart() throws IOException { + public void start_container_subtask_failure_leads_to_container_restart() { final long restartGeneration = 1; final long rebootGeneration = 0; final NodeSpec node = nodeBuilder @@ -605,8 +584,6 @@ public class NodeAgentImplTest { NodeAgentImpl nodeAgent = spy(makeNodeAgent(null, false)); when(nodeRepository.getOptionalNode(hostName)).thenReturn(Optional.of(node)); - when(pathResolver.getApplicationStoragePathForNodeAdmin()).thenReturn(Files.createTempDirectory("foo")); - when(pathResolver.getApplicationStoragePathForHost()).thenReturn(Files.createTempDirectory("bar")); when(dockerOperations.pullImageAsyncIfNeeded(eq(dockerImage))).thenReturn(false); when(storageMaintainer.getDiskUsageFor(eq(context))).thenReturn(Optional.of(201326592000L)); doThrow(new DockerException("Failed to set up network")).doNothing().when(dockerOperations).startContainer(eq(context)); @@ -659,6 +636,7 @@ public class NodeAgentImplTest { .membership(membership) .minMainMemoryAvailableGb(2) .allowedToBeDown(true) + .parentHostname("parent.host.name.yahoo.com") .build(); NodeAgentImpl nodeAgent = makeNodeAgent(dockerImage, true); @@ -668,7 +646,6 @@ public class NodeAgentImplTest { when(dockerOperations.getContainerStats(eq(context))) .thenReturn(Optional.of(stats1)) .thenReturn(Optional.of(stats2)); - when(pathResolver.getApplicationStoragePathForHost()).thenReturn(Files.createTempDirectory("bar")); nodeAgent.converge(); // Run the converge loop once to initialize lastNode nodeAgent.updateContainerNodeMetrics(); // Update metrics once to init and lastCpuMetric @@ -717,7 +694,7 @@ public class NodeAgentImplTest { } @Test - public void testRunningConfigServer() throws IOException { + public void testRunningConfigServer() { final long rebootGeneration = 0; final NodeSpec node = nodeBuilder .nodeType(NodeType.config) @@ -729,8 +706,6 @@ public class NodeAgentImplTest { NodeAgentImpl nodeAgent = makeNodeAgent(null, false); when(nodeRepository.getOptionalNode(hostName)).thenReturn(Optional.of(node)); - Path tempDirectory = Files.createTempDirectory("foo"); - when(pathResolver.getApplicationStoragePathForHost()).thenReturn(tempDirectory); when(dockerOperations.pullImageAsyncIfNeeded(eq(dockerImage))).thenReturn(false); when(storageMaintainer.getDiskUsageFor(eq(context))).thenReturn(Optional.of(201326592000L)); @@ -743,7 +718,7 @@ public class NodeAgentImplTest { inOrder.verify(dockerOperations, times(1)).pullImageAsyncIfNeeded(eq(dockerImage)); inOrder.verify(dockerOperations, times(1)).createContainer(eq(context), eq(node), any()); inOrder.verify(dockerOperations, times(1)).startContainer(eq(context)); - inOrder.verify(aclMaintainer, times(1)).run(); + inOrder.verify(aclMaintainer, times(1)).converge(); inOrder.verify(dockerOperations, times(1)).resumeNode(eq(context)); inOrder.verify(nodeRepository).updateNodeAttributes( hostName, new NodeAttributes() @@ -756,11 +731,10 @@ public class NodeAgentImplTest { mockGetContainer(dockerImage, isRunning); when(dockerOperations.getContainerStats(any())).thenReturn(Optional.of(emptyContainerStats)); - doNothing().when(storageMaintainer).writeFilebeatConfig(any(), any()); doNothing().when(storageMaintainer).writeMetricsConfig(any(), any()); return new NodeAgentImpl(context, nodeRepository, orchestrator, dockerOperations, - storageMaintainer, aclMaintainer, environment, clock, NODE_AGENT_SCAN_INTERVAL, Optional.of(athenzCredentialsMaintainer)); + storageMaintainer, clock, NODE_AGENT_SCAN_INTERVAL, Optional.of(athenzCredentialsMaintainer), Optional.of(aclMaintainer)); } private void mockGetContainer(DockerImage dockerImage, boolean isRunning) { |