diff options
Diffstat (limited to 'node-admin')
16 files changed, 63 insertions, 93 deletions
diff --git a/node-admin/src/main/application/services.xml b/node-admin/src/main/application/services.xml index da782363508..99733552f76 100644 --- a/node-admin/src/main/application/services.xml +++ b/node-admin/src/main/application/services.xml @@ -11,10 +11,6 @@ <component id="docker-api" class="com.yahoo.vespa.hosted.dockerapi.DockerImpl" bundle="docker-api"/> <component id="metrics-wrapper" class="com.yahoo.vespa.hosted.dockerapi.metrics.MetricReceiverWrapper" bundle="docker-api"/> - <config name="vespa.hosted.dockerapi.docker"> - <isRunningLocally>false</isRunningLocally> - </config> - <nodes type="host"/> <preprocess:include file="variant.xml" required="false"/> diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/component/TaskContext.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/component/TaskContext.java index cee2dc9b66b..2d94db3d96e 100644 --- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/component/TaskContext.java +++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/component/TaskContext.java @@ -1,26 +1,29 @@ // 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 java.nio.file.FileSystem; -import java.util.EnumSet; import java.util.logging.Logger; public interface TaskContext { - enum Cloud { YAHOO, AWS } - Cloud cloud(); + /** + * Record a system modification. IdempotentTask is supposed to converge the system (files, + * directory permission, iptable rules, etc) to some wanted state. It is especially important + * to produce a truthful log of system changes to understand what may or may not be going on. + * + * All tasks should: + * 1. Record any and all modifications to the system + * 2. Avoid recording system interactions that does not actually change the system. + * 3. Record system modifications as early as possible and preferably before they are + * performed (sometimes this is not possible). + * + * @param logger Used to log the modification to help locate the source of the modification. + * @param description Description of the modification, e.g. "Changing owner of /foo from alice + * to bob". + */ + void recordSystemModification(Logger logger, String description); - enum Role { TENANT_DOCKER_HOST, CONFIG_SERVER_DOCKER_HOST } - EnumSet<Role> roles(); - default boolean hasRole(Role role) { - return roles().contains(role); - } - - FileSystem fileSystem(); - - void logSystemModification(Logger logger, String actionDescription); - default void logSystemModification(Logger logger, String format, String... args) { - logSystemModification(logger, String.format(format, (Object[]) args)); - } - - default boolean executeSubtask(IdempotentTask task) { return false; } + /** + * Execute a task as a child of this task, and with its own sub-TaskContext. Please avoid + * excessive task hierarchies. + */ + boolean executeSubtask(IdempotentTask task); } diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeadmin/DockerAdminConfig.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeadmin/DockerAdminConfig.java deleted file mode 100644 index d1aaefab3dd..00000000000 --- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeadmin/DockerAdminConfig.java +++ /dev/null @@ -1,9 +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.nodeadmin; - -import com.fasterxml.jackson.annotation.JsonIgnoreProperties; - -@JsonIgnoreProperties(ignoreUnknown = true) -public class DockerAdminConfig { -} diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeadmin/NodeAdminConfig.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeadmin/NodeAdminConfig.java index df26bc2dee5..c3a1f47dfc5 100644 --- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeadmin/NodeAdminConfig.java +++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeadmin/NodeAdminConfig.java @@ -18,18 +18,6 @@ public class NodeAdminConfig { @JsonProperty("main-component") public String mainComponent = null; - public enum Mode { - aws_tenant, - config_server_host, - tenant, - } - - @JsonProperty("mode") - public Mode mode = Mode.tenant; - - @JsonProperty("docker") - public DockerAdminConfig docker = new DockerAdminConfig(); - public static NodeAdminConfig fromFile(File file) { if (!file.exists()) { return new NodeAdminConfig(); diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeadmin/NodeAdminMain.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeadmin/NodeAdminMain.java index f412ec5ef2f..b0631fc712b 100644 --- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeadmin/NodeAdminMain.java +++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeadmin/NodeAdminMain.java @@ -5,11 +5,10 @@ import com.yahoo.component.ComponentId; import com.yahoo.component.provider.ComponentRegistry; import com.yahoo.concurrent.classlock.ClassLocking; import com.yahoo.log.LogLevel; -import com.yahoo.vespa.defaults.Defaults; import com.yahoo.vespa.hosted.dockerapi.Docker; import com.yahoo.vespa.hosted.dockerapi.metrics.MetricReceiverWrapper; -import com.yahoo.vespa.hosted.node.admin.config.ConfigServerConfig; import com.yahoo.vespa.hosted.node.admin.component.AdminComponent; +import com.yahoo.vespa.hosted.node.admin.config.ConfigServerConfig; import com.yahoo.vespa.hosted.node.admin.provider.NodeAdminStateUpdater; import java.io.File; @@ -49,8 +48,7 @@ public class NodeAdminMain implements AutoCloseable { } public static NodeAdminConfig getConfig() { - String path = Defaults.getDefaults().underVespaHome("conf/node-admin.json"); - return NodeAdminConfig.fromFile(new File(path)); + return NodeAdminConfig.fromFile(new File("/etc/vespa/node-admin.json")); } public void start() { diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/file/AttributeSync.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/file/AttributeSync.java index a20d30b2bf9..cba3912ce49 100644 --- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/file/AttributeSync.java +++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/file/AttributeSync.java @@ -111,13 +111,13 @@ public class AttributeSync { return false; } - context.logSystemModification( + context.recordSystemModification( logger, - "Changing %s of %s from %s to %s", - attributeName, - path.toString(), - currentValue, - wantedValue.get()); + String.format("Changing %s of %s from %s to %s", + attributeName, + path.toString(), + currentValue, + wantedValue.get())); valueSetter.accept(wantedValue.get()); diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/file/FileSync.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/file/FileSync.java index 18d2a2e3aa9..7f11707c1cc 100644 --- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/file/FileSync.java +++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/file/FileSync.java @@ -53,7 +53,7 @@ public class FileSync { } if (!currentAttributes.exists()) { - taskContext.logSystemModification(logger, "Creating file " + path); + taskContext.recordSystemModification(logger, "Creating file " + path); path.createParents(); path.writeUtf8File(content.get()); contentCache.updateWith(content.get(), currentAttributes.forceGet().lastModifiedTime()); @@ -63,7 +63,7 @@ public class FileSync { if (Objects.equals(content.get(), contentCache.get(currentAttributes.get().lastModifiedTime()))) { return false; } else { - taskContext.logSystemModification(logger, "Patching file " + path); + taskContext.recordSystemModification(logger, "Patching file " + path); path.writeUtf8File(content.get()); contentCache.updateWith(content.get(), currentAttributes.forceGet().lastModifiedTime()); return true; diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/file/MakeDirectory.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/file/MakeDirectory.java index e815ab8bd86..2a88387f8fc 100644 --- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/file/MakeDirectory.java +++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/file/MakeDirectory.java @@ -54,7 +54,7 @@ public class MakeDirectory { path.createParents(); } - context.logSystemModification(logger, "Creating directory " + path); + context.recordSystemModification(logger, "Creating directory " + path); systemModified = true; Optional<String> permissions = attributeSync.getPermissions(); diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/process/ChildProcess.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/process/ChildProcess.java index 71a4c7c109b..dc2ab07d97c 100644 --- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/process/ChildProcess.java +++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/process/ChildProcess.java @@ -18,7 +18,7 @@ public interface ChildProcess extends AutoCloseable { * Only call this if process was spawned under the assumption the program had no side * effects (see Command::spawnProgramWithoutSideEffects). If it is determined later * that the program did in fact have side effects (modified system), this method can - * be used to log that fact. Alternatively, call TaskContext::logSystemModification + * be used to log that fact. Alternatively, call TaskContext::recordSystemModification * directly. */ void logAsModifyingSystemAfterAll(Logger logger); diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/process/ChildProcessImpl.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/process/ChildProcessImpl.java index 98fa18db52d..cabeafd2727 100644 --- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/process/ChildProcessImpl.java +++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/process/ChildProcessImpl.java @@ -73,7 +73,7 @@ public class ChildProcessImpl implements ChildProcess { @Override public void logAsModifyingSystemAfterAll(Logger logger) { - taskContext.logSystemModification(logger, "Executed command: " + commandLine); + taskContext.recordSystemModification(logger, "Executed command: " + commandLine); } @Override diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/process/Command.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/process/Command.java index 275440f7840..97066748b2b 100644 --- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/process/Command.java +++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/process/Command.java @@ -42,7 +42,7 @@ public class Command { String commandLine = commandLine(); if (commandLogger != null) { - context.logSystemModification(commandLogger, "Executing command: " + commandLine); + context.recordSystemModification(commandLogger, "Executing command: " + commandLine); } // Why isn't this using TaskContext.fileSystem? Because createTempFile assumes @@ -94,7 +94,7 @@ public class Command { * * This method can also be used to spawn a process that MAY have side effects * to be determined at some later time. The caller is then responsible for calling - * TaskContext::logSystemModification afterwards. The caller is encouraged to + * TaskContext::recordSystemModification afterwards. The caller is encouraged to * call ChildProcess::logAsModifyingSystemAfterAll to do this. */ public ChildProcess spawnProgramWithoutSideEffects() { diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/yum/AddYumRepo.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/yum/AddYumRepo.java index 2b1cbbed974..9485e5d6728 100644 --- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/yum/AddYumRepo.java +++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/yum/AddYumRepo.java @@ -4,6 +4,8 @@ package com.yahoo.vespa.hosted.node.admin.task.util.yum; import com.yahoo.vespa.hosted.node.admin.component.TaskContext; import com.yahoo.vespa.hosted.node.admin.task.util.file.FileWriter; +import java.nio.file.FileSystem; +import java.nio.file.FileSystems; import java.nio.file.Path; import java.util.regex.Pattern; @@ -17,20 +19,17 @@ public class AddYumRepo { private final String name; // e.g. "Platform RPM Latest Repo" private final String baseurl; private final boolean enabled; + private final FileSystem fileSystem; public AddYumRepo(String repositoryId, String name, String baseurl, boolean enabled) { - this.repositoryId = repositoryId; - this.name = name; - this.baseurl = baseurl; - this.enabled = enabled; - validateRepositoryId(repositoryId); + this(repositoryId, name, baseurl, enabled, FileSystems.getDefault()); } public boolean converge(TaskContext context) { - Path path = context.fileSystem().getPath("/etc/yum.repos.d",repositoryId + ".repo"); + Path path = fileSystem.getPath("/etc/yum.repos.d",repositoryId + ".repo"); FileWriter fileWriter = new FileWriter(path, this::getRepoFileContent) .withOwner("root") @@ -59,4 +58,18 @@ public class AddYumRepo { throw new IllegalArgumentException("Invalid repository ID '" + repositoryId + "'"); } } + + // For testing + AddYumRepo(String repositoryId, + String name, + String baseurl, + boolean enabled, + FileSystem fileSystem) { + this.repositoryId = repositoryId; + this.name = name; + this.baseurl = baseurl; + this.enabled = enabled; + this.fileSystem = fileSystem; + validateRepositoryId(repositoryId); + } } diff --git a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/task/util/file/FileWriterTest.java b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/task/util/file/FileWriterTest.java index bb8ca2586c8..1e4c7482aaf 100644 --- a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/task/util/file/FileWriterTest.java +++ b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/task/util/file/FileWriterTest.java @@ -37,7 +37,7 @@ public class FileWriterTest { .onlyIfFileDoesNotAlreadyExist(); TaskContext context = mock(TaskContext.class); assertTrue(writer.converge(context)); - verify(context, times(1)).logSystemModification(any(), eq("Creating file " + path)); + verify(context, times(1)).recordSystemModification(any(), eq("Creating file " + path)); UnixPath unixPath = new UnixPath(path); assertEquals(content, unixPath.readUtf8File()); diff --git a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/task/util/file/TestTaskContext.java b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/task/util/file/TestTaskContext.java index 757f3004683..1fe63e84605 100644 --- a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/task/util/file/TestTaskContext.java +++ b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/task/util/file/TestTaskContext.java @@ -5,9 +5,7 @@ package com.yahoo.vespa.hosted.node.admin.task.util.file; import com.yahoo.vespa.hosted.node.admin.component.IdempotentTask; import com.yahoo.vespa.hosted.node.admin.component.TaskContext; -import java.nio.file.FileSystem; import java.util.ArrayList; -import java.util.EnumSet; import java.util.List; import java.util.logging.Logger; @@ -15,23 +13,8 @@ public class TestTaskContext implements TaskContext { private final List<String> logs = new ArrayList<>(); @Override - public Cloud cloud() { - throw new UnsupportedOperationException(); - } - - @Override - public EnumSet<Role> roles() { - throw new UnsupportedOperationException(); - } - - @Override - public FileSystem fileSystem() { - throw new UnsupportedOperationException(); - } - - @Override - public void logSystemModification(Logger logger, String actionDescription) { - logs.add(actionDescription); + public void recordSystemModification(Logger logger, String description) { + logs.add(description); } public List<String> getSystemModificationLog() { diff --git a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/task/util/process/CommandTest.java b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/task/util/process/CommandTest.java index acc27353077..6dad95aeb6a 100644 --- a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/task/util/process/CommandTest.java +++ b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/task/util/process/CommandTest.java @@ -27,7 +27,7 @@ public class CommandTest { Path outputFile; // Assumes /bin/bash exists on all hosts running this test. try (ChildProcess childProcess = command.spawn(logger)) { - verify(taskContext).logSystemModification(eq(logger), any()); + verify(taskContext).recordSystemModification(eq(logger), any()); outputFile = childProcess.getProcessOutputPath(); int exitValue = childProcess.waitForTermination().exitValue(); diff --git a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/task/util/yum/AddYumRepoTest.java b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/task/util/yum/AddYumRepoTest.java index ad1fefe782f..399759c7c0a 100644 --- a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/task/util/yum/AddYumRepoTest.java +++ b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/task/util/yum/AddYumRepoTest.java @@ -3,8 +3,8 @@ package com.yahoo.vespa.hosted.node.admin.task.util.yum; import com.yahoo.vespa.hosted.node.admin.component.TaskContext; -import com.yahoo.vespa.test.file.TestFileSystem; import com.yahoo.vespa.hosted.node.admin.task.util.file.UnixPath; +import com.yahoo.vespa.test.file.TestFileSystem; import org.junit.Test; import java.nio.file.FileSystem; @@ -14,7 +14,6 @@ import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertTrue; import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.when; public class AddYumRepoTest { @Test @@ -24,17 +23,16 @@ public class AddYumRepoTest { String baseurl = "http://foo.com/bar"; boolean enabled = true; + FileSystem fileSystem = TestFileSystem.create(); AddYumRepo addYumRepo = new AddYumRepo( repositoryId, name, baseurl, - enabled); + enabled, + fileSystem); TaskContext context = mock(TaskContext.class); - FileSystem fileSystem = TestFileSystem.create(); - when(context.fileSystem()).thenReturn(fileSystem); - assertTrue(addYumRepo.converge(context)); UnixPath unixPath = new UnixPath(fileSystem.getPath("/etc/yum.repos.d/" + repositoryId + ".repo")); |