Skip to content

This issue was moved to a discussion.

You can continue the conversation there. Go to discussion →

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[proposal] Create an installer with a flag (argument) #122

Closed
josecelano opened this issue Dec 2, 2022 · 8 comments
Closed

[proposal] Create an installer with a flag (argument) #122

josecelano opened this issue Dec 2, 2022 · 8 comments
Labels

Comments

@josecelano
Copy link
Member

When you run the tracker the first time, you see this output:

No config file found.
Creating config file..
thread 'main' panicked at 'Please edit the config.TOML in the root folder and restart the tracker.', src/main.rs:22:13
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

Running the tracker without the setting file has a side effect. It creates a setting file with the default configuration.

$ cat config.toml
log_level = "info"
mode = "public"
db_driver = "Sqlite3"
db_path = "data.db"
announce_interval = 120
min_announce_interval = 120
max_peer_timeout = 900
on_reverse_proxy = false
external_ip = "0.0.0.0"
tracker_usage_statistics = true
persistent_torrent_completed_stat = false
inactive_peer_cleanup_interval = 600
remove_peerless_torrents = true

[[udp_trackers]]
enabled = false
bind_address = "0.0.0.0:6969"

[[http_trackers]]
enabled = false
bind_address = "0.0.0.0:6969"
ssl_enabled = false
ssl_cert_path = ""
ssl_key_path = ""

[http_api]
enabled = true
bind_address = "127.0.0.1:1212"

[http_api.access_tokens]
admin = "MyAccessToken"

I suggest showing a message like this:

No config file found: "config.toml".
You can create the default settings file running: cargo run --bin install

That "install" binary could be extended in the feature to be an installation assistant if you run it in interactive mode.

But the main reason for this change is I'm working on setting up docker configuration to automate deployments for the Turrust tracker and running a process that returns an error that could lead to errors while building docker images or others tasks.

On the other hand, I always prefer to be explicit about side effects. Besides, showing an error the first time you run a tracker could make people think the application is not good.

If you do not like that approach, the second option could be to ask the user if they want to create that file, but I do not like that option because it forces the app to be always interactive. We should have added an option to avoid that question in this case and return an error message with an error return value (not a panic message).

@da2ce7
Copy link
Contributor

da2ce7 commented Dec 3, 2022

I would like to have tracker-configuration file to be specified by some schema, and configured over a management API.

The exception should be the core configuration options of the tracker. Settings that guide the access to the trackers management API (such as the management socket, or the public key of the administrator). These should be passed-in either by launch arguments, or picked up from environmental variables.

Once the tracker has been launched, then all configuration should happen over it's management API.

For example, The torrust index client could then be extended with an interface for management of the Tracker.

@josecelano
Copy link
Member Author

josecelano commented Dec 3, 2022 via email

@da2ce7
Copy link
Contributor

da2ce7 commented Dec 3, 2022

Core settings could be the ones that require a service restart. For example the socket, as you mentioned.

Hmm... I think that server should be configured via it's management API, including for any setting that may need to have the server restarted.

The server can maintain a list of setting that are going to be applied on next startup.

The core settings should be limited to the settings that are needed for the initial access, or override access, to the management api.

We can also use a mounted file for settings especially for secrets. I read on the docker docs that is safer than env vars. I have to read more about that.

We need some sort of database-connection-configuration, this should be limited to the setting required for the server to connect to it's database.

This file should be in some sort of internal format. And created and managed by the management api.

We need to think where to store the default values. If you are planning to move not core setting to the DB we could prefill the database with them, but maybe it's better to hard code default values and use the DB only if you want to override them.

All other settings should be stored inside the database, including what is the default values, and if a setting is was set explicitly or as left as a default. Even if that setting can only be applied on the next load of the server.

When an default value is updated, it should be in a form of database migration, and appropriately deal with the old defaults (i.e. leave them as they are and add a old-default-migration-xyz flag or migrate the value to the updated value).

The management schema that the server gives to the clients should always accurately describe the current default values, and should be versioned by the db-migrations.

An additional schema should be provided to the client that describes the history the settings-db-migrations. (i.e. what value was updated to and when).

Over the management api, Clients should be able to review the changes to the settings by the migrations of the server to a new version, and keep/apply/or/revert new updated defaults.

@josecelano
Copy link
Member Author

hi @da2ce7, this is an interesting discussion. I'm going to ask you some questions to ensure I understand your proposal.

Questions

  • What do you consider an internal format? A JSON schema that is only used by this app?
  • What is the management API? Part of a new RES API served by the tracker?
  • Is not a DB migration and a Rust hard-coded value the same for default values? The default values change because you change the source code, and you use versioning to keep track of changes. The only difference is you can get the default value from the DB, like overwriting values.
  • What is the management schema? The JSON schema for the settings?
  • What are the "clients" in this context? Tracker clients? but REST API clients? Are you suggesting using a new JS app to handle the setting, or would it be the index frontend?

Maybe It would be better if you explain it to us in the next meeting until we have the same understanding and we can write down the final proposal. It could maybe even be a BEP.

@da2ce7
Copy link
Contributor

da2ce7 commented Dec 6, 2022

When the application loads it has the following goals, in this order:

  1. Serve the Management API to the Administrator.
  2. Connect to the Database.
  3. Provide Services.

Each step takes a different form of configuration.

  1. Management API.
    This step is the most critical. If the app is not able to be managed, it is useless for anything else.

The configuration for the Management API is provide by the Host. (i.e. Environmental Variables, Command Arguments, or Injected Configuration Files, it dose not matter). The app somehow needs to be supplied with:

  • The Hash of the Public Key of the Recovery Administrator.
  • The Socket for where to Provide the Management API.
  • (Optional) Database Connection Details.
  1. Database
    The database is the persistent memory for the application. We should assume that the app runs on a transient read-only file-system.

If the application has some persistent private storage. This can be used to keep a cached copy of the database configuration. (If not supplied by the host, but the management api).

  1. Everything Else
    Once the application has connected to the database it can restore it's state, and start preforming it's core functionally.

@Power2All
Copy link
Contributor

Power2All commented Dec 10, 2022

Good idea, but I will add it to my own project as a flag, so that it creates it when asked.
Making a whole separate binary for this, is ridiculous.

@josecelano josecelano mentioned this issue Dec 15, 2022
17 tasks
@josecelano
Copy link
Member Author

Good idea, but I will add it to my own project as a flag, so that it creates it when asked. Making a whole separate binary for this, is ridiculous.

hi @Power2All, yes that was my first idea, running something like:

cargo run --install

In fact, I thought it could have a silent mode and an interactive mode with a nice console assistant than can generate the final config.toml.

I decided to use a different binary just to skip parsing the arguments for the time being in the main. But then when I implemented it I realized that it made things harder in the building phase. You need to build both binaries in the Dockerfile and overite the entry-point. SO definitively I would use a flag. I was trying to sue the application to give me the default configuration file instead of building it by my self because the format could change and I will need to re-write the code to generate the configuration.

@josecelano josecelano changed the title [proposal] Move initial setting setup to its own binary [proposal] Create an installer with a flag (argument) Dec 16, 2022
@josecelano
Copy link
Member Author

We can define a more concrete behaviour:

It generates the default config.toml file.

cargo run installer --default --file config.toml

It prints the default config.toml to the standard output.

cargo run installer --default

It runs an interactive installer that can generate the final configuration (long-term feature):

cargo run installer -i

@torrust torrust locked and limited conversation to collaborators Dec 21, 2022
@josecelano josecelano converted this issue into discussion #134 Dec 21, 2022

This issue was moved to a discussion.

You can continue the conversation there. Go to discussion →

Labels
Projects
Archived in project
Development

No branches or pull requests

3 participants