diff options
author | Håkon Hallingstad <hakon@oath.com> | 2018-02-02 02:00:10 +0100 |
---|---|---|
committer | Håkon Hallingstad <hakon@oath.com> | 2018-02-02 02:00:10 +0100 |
commit | 555c0c9648105396b0e5d7b6a386a548b77b6bc2 (patch) | |
tree | f85cdceb84baa782454d86640bc7673f30677d5c /node-admin/src/main/java | |
parent | 73555da8073d121ca367b51a1fd38193a47982f3 (diff) |
Cleanup TaskContext
- Removes unnecessary isRunningLocally setting
- Reduce TaskContext to (basically) recordSystemModification, e.g. removing
FileSystem as that can be accessed through other means.
- Remove DockerAdminConfig as it's still unused.
- Remove unnecessary fields in NodeAdminConfig.
- Move json configs to /etc/vespa.
Diffstat (limited to 'node-admin/src/main/java')
11 files changed, 55 insertions, 62 deletions
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); + } } |