Open Hornwitser opened 5 years ago
This just makes too much sense ;)
This makes a lot of sense, although you got a few things wrong about the current system.
Adding a master web interface for modifying the config at runtime is possible, although any modules that load the config for themselves break it (see serverManager, it has a thing for this)
There is a command line parameter for the client to use a different config (--config I think, might be for the master as well)
Everything else is about right. We need a new more robust config system if we are to properly decouple plugins from the core repo.
I would like to raise the possibility of not using JSON for the config at all. The reason for using JSON is simple - its very easy to support with nodejs. The issue is that its meant to be machine readable, not human usable. For one, JSON does not allow comments - the current solution of hacking in comments through config entries with long descriptive names is very ugly.
If we are to find a new format for the config, we have to make sure it has all the features we require
Why not have default configs in "best format for it" and have that read into the master and stored there? From that point on the minion can read it's config from the master and other entities can edit it there. Some kind of descriptive structure could allow the web-frontend to edit the config, without the need to write anywhere near the plugin files that might get overwritten or deleted by anything.
Proposal: No need to support manually editing the config with a text editor as the primary option.
On initial master launch, configManager detects that there is no config and generates a config in any machine readable format in ./database/config. Master spins up, listening to default port (or whatever is supplied by --port). You go onto the site, make an account (we need a better way to make the first admin account) and edit the config there.
- Per plugin configuration. This means that plugin a and b have separate spaces for their configs and do not see the values of each other's configs in their own.
- Per instance configuration so that you can set for example a different password for different servers.
I assume this also does mean per instance per plugin
. So you can have specific settings for a plugin for a specific instance.
When you install a new plugin the first time it will get the default config and the descriptive structure stored in the master. All existing minions and their instances will adopt that config. From this point on you can edit the config cluster wide, minion wide or per single instance. Generally you will want to only have one plugin config for everything, but the ability to set it at each level: master, minion, instance.
Also to decide, if we want to have plugin spanning configurations that get defined and inherited by each plugin.
I've been trying to get master.js to work with my proof of concept implementation, but so far there's been a lot of roadblocks. My idea for the configuration life cycle is not to load any configuration before startServer is invoked in master.js, this is both so that unit tests can be more independent and to simplify the handling of plugin defined configuration options. But decoupling the code from the config is proving to be more work than expected.
I have gotten to the point where I've got most of the concepts pinned down, and my config-rewrite branch is working for master.js.
This is what I've ended up as for the implementation:
The configuration system should have fields that are easy to define, discover, verify, and use. And that configurable fields can be attached to the Master, Slaves, Instances, and Plugins, which can be thought of as domains for the fields.
The configuration for a particular domain is created by extending one of the base config classes. The master config is based on Config, slave and instance configs are based on InstancedConfig, and plugin are based on PluginConfig. The difference between these is that InstancedConfig has a name field telling which instance/slave it belongs to and PluginConfig has a pluginName field telling which plugin it belong as well as an enabled field.
Defining fields to a config class is done with the define
class method, which takes an object describing the field and adds it to the class (see separate description for it.) To give an example of how this works a typical plugin would define it's config as follows:
let config = require("lib/config");
class MyPluginConfig extends config.PluginConfig { }
MyPluginConfig.name = "my_plugin";
MyPluginConfig.version = "0.1.2";
MyPluginConfig.define({
name: "do_foo",
title: "Do Frobnication",
description: "Activate frobnication on instances",
type: "boolean",
initial_value: true
});
MyPluginConfig.finalize();
// The plugin will pass the MyPluginConfig class to the slave/master
// server through some yet to be determined interface
This defines a do_foo
config field with an initial value of true
. When loading the plugin is passed an instance of the class, which it can call .get('do_foo')
and .set('do_foo', newValue)
on to query and change the value.
Configurations are defined separately and before any master, slave or plugin code is runned. The reason for this is that the master server is separated from the slave servers and implementing a configuration system where only one party has access to the right definition for the config makes it needlessly complicated. It's also useful to have the configuration definition for disabled plugins.
The lifecycle of the configuration at startup system is as follows:
config-master.json
and the slaves default to storing it in config-slave.json
.At shutdown the config is serialized and saved back the config file it was loaded from.
It's possible for a master server to be running a different version with a different configuration definition than what's installed on a slave. For this reason a slave, instance, and plugin configs should implement both forward and backwards migrations to its configuration.
If the master server has a newer version of the config class it will run a backwards migration before sending the configuration to a slave. If the master server has an older version of the config classe the configuration will be sent as is and the slave/instance will run a forward migration on the configuration before loading it.
The method for defining migrations has not yet been determined.
A config class represents the configuration of one type of thing. They are base classes that are extended by the actual config class.
They have the following methods.
Defines a field for the config. Can only be called before finalize()
is called. It takes a single argument which is an object containing the following properties:
name
Name of the of config field. Should follow the lower_case_with_underscore
naming convention.
title
(optional)
Name provided to the admin for this configuration field.
description
(optional)
Description provided to the admin for this configuration field.
type
The type of the paramater. One of "boolean"
, "string"
, "number"
, and "object"
. For the string type an empty string is not allowed as the value. This to allow interfaces that let you leave a value unset by leaving the field blank.
optional
(optional)
If the config can be null
. Defaults to false.
enum
(optional)
The allowed values for this configuration field. Effectively makes it an enumerated field. If not set any value is allowed so long as the type is correct.
innital_value
(optional)
The value this option gets when a new config is created, must be either of the type given by type
or a (possibly async) function that generates an initial value. A field that is not optional must have an initial value associated with it.
Locks the config class from adding any more fields to it with define()
. This is to hammer down the point that fields cannot be dynamically added to config classes, and using a an incomplete config class is an error.
Creates a new serialized configuration based on the class. This will have all fields defined with their respective values. The name
parameter is required for configs based on InstancedConfig
and specifies the name of the instance/slave to create a config, for other config classes it must not be passed.
Creates a config object. serializedConfig
should value returned by either create
or serialize
. bases
is an array of live PartialConfig
instances that holds inherited values this config should use for entries that are not in serializedConfig
.
Note that if a field is deleted from a base, and neither the config nor any other base has a value for the field, this config will automatically have the value that was deleted from the base set as the value for the field.
Converts the config into a plain JavaScript object that can be serialized with JSON.serialize()
. This is used for saving the config as well as sending it to remote clients. private
is a boolean that indicates if fields marked private should also be serialized.
Get the config field named name
. Guaranteed to return a value that's of the type specified in the definition for it, or null
if the field is optional.
Set the config field named name
to the given value. Setting a value that's not a type specified in the definition is an error. null
is also allowed if the field was defined as optional.
Remove the value for the field named name
from the config. This is only allowed if at least one base for this config also has a value for the field.
The more I think about it the less sense it makes to have any configuration done locally at slaves. You have to connect to the computer running the instance and run some command or edit a file, The benefit being you can run the instance and set it up when the master server is down, and that's about it. But nothing works when the master is down so what's the point?
If we want communities donating server hardware to run instances on to be a thing for the next Clusterio event then it would be a major pain to manage each instance individually on each server like this (and I'm pretty sure this was a lesson learned from Clusterio 60k.) What I imagine as an ideal situation is that all you configure on a slave is the master server address and token via some simple cli/wizard, start the slave and then you're done. The rest is controlled from the master server web interface.
What I'm thinking now is to get us 80% there. I can't make the local instance configuration method make sense, and I don't see any point in implementing the configuration system that way. I also don't particularly fancy working on the web interface. So I'm thinking of implementing #201 and have the slave listen on the socket.io connection to the master server for commands, implement an HTTP API in the master server for sending these commands, and write a CLI tool to send commands to slaves via the HTTP API running on the master. The HTTP API could then also be used to implement the elusive remote management web interface at some later point.
This will result in the current command line interface to client.js being completely removed. The current HTTP API will likely need a major overhaul. And #220 is probably going to be needed as well.
The remaining tasks before I will call this completed is
The current way configuration is handled is to put it mildly terrible. Everything is dumped into config.json, which is loaded at startup and never touched again. This presents a lot of problems:
Thefore I propose that the whole configuration system is rewritten to have a lib/configManager module that manages all of the configuration needed by instances, slaves, the master server, and plugins. And that is should have at least the following features:
Things that would be nice extensions to add to this system, but is a bit broad in scope for the rewrite:
ajv
I will come up with a more concrete plan for implementation later if this is accepted.