-
Notifications
You must be signed in to change notification settings - Fork 636
e2e: allow customize Comet's config parameters for all or specific nodes #3832
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
Labels
Comments
For instance, #3368 wants to render one specific Comet parameter, The proposed approach here would render possible to achieve the same feature for any existing CometBFT configuration parameter. |
This was referenced Sep 2, 2024
3 tasks
github-merge-queue bot
pushed a commit
that referenced
this issue
Sep 4, 2024
Addresses #3832. Introduces a flag `config` to the manifest. It is a list of strings that should have the format `key = value`. Keys are any of the configuration parameters included in a CometBFT configuration file. The flag can be global, meaning that the configurations will be applied for every node in the network, or specific for a given node, when inserted in the section referring to that node. Local/specific configurations are applied after the global ones, therefore overriding them. Example: ``` config = [ "p2p.send_rate = 51200", "filter_peers = true", ] ``` The configurations are applying using `viper`, as I could not find another way to load configuration parameters. They are always loaded as a string, and this appears to work correctly. Note: this PR is based atop #3964 for simplicity, but it does not need to. --- #### PR checklist - [ ] Tests written/updated - [ ] Changelog entry added in `.changelog` (we use [unclog](https://github.com/informalsystems/unclog) to manage our changelog) - [ ] Updated relevant documentation (`docs/` or `spec/`) and code comments --------- Co-authored-by: Anton Kaliaev <anton.kalyaev@gmail.com> Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com> Co-authored-by: Andy Nogueira <me@andynogueira.dev>
mergify bot
added a commit
that referenced
this issue
Oct 20, 2024
Addresses #3832. Introduces a flag `config` to the manifest. It is a list of strings that should have the format `key = value`. Keys are any of the configuration parameters included in a CometBFT configuration file. The flag can be global, meaning that the configurations will be applied for every node in the network, or specific for a given node, when inserted in the section referring to that node. Local/specific configurations are applied after the global ones, therefore overriding them. Example: ``` config = [ "p2p.send_rate = 51200", "filter_peers = true", ] ``` The configurations are applying using `viper`, as I could not find another way to load configuration parameters. They are always loaded as a string, and this appears to work correctly. Note: this PR is based atop #3964 for simplicity, but it does not need to. --- #### PR checklist - [ ] Tests written/updated - [ ] Changelog entry added in `.changelog` (we use [unclog](https://github.com/informalsystems/unclog) to manage our changelog) - [ ] Updated relevant documentation (`docs/` or `spec/`) and code comments --------- Co-authored-by: Anton Kaliaev <anton.kalyaev@gmail.com> Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com> Co-authored-by: Andy Nogueira <me@andynogueira.dev> (cherry picked from commit cea82b1)
3 tasks
mergify bot
added a commit
that referenced
this issue
Oct 20, 2024
…ort #3967) (#4309) Addresses #3832. Introduces a flag `config` to the manifest. It is a list of strings that should have the format `key = value`. Keys are any of the configuration parameters included in a CometBFT configuration file. The flag can be global, meaning that the configurations will be applied for every node in the network, or specific for a given node, when inserted in the section referring to that node. Local/specific configurations are applied after the global ones, therefore overriding them. Example: ``` config = [ "p2p.send_rate = 51200", "filter_peers = true", ] ``` The configurations are applying using `viper`, as I could not find another way to load configuration parameters. They are always loaded as a string, and this appears to work correctly. Note: this PR is based atop #3964 for simplicity, but it does not need to. --- #### PR checklist - [ ] Tests written/updated - [ ] Changelog entry added in `.changelog` (we use [unclog](https://github.com/informalsystems/unclog) to manage our changelog) - [ ] Updated relevant documentation (`docs/` or `spec/`) and code comments <hr>This is an automatic backport of pull request #3967 done by [Mergify](https://mergify.com). --------- Co-authored-by: Daniel <daniel.cason@informal.systems>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
The
e2e
infrastructure creates a number of nodes that are executed as a network for testing purposes.The nodes are created using the default configuration values (from
config/config.go
), some of which are changed based on parameters provided in the manifest. But if one wants to run an e2e execution using non-standard parameters, there are two possibilities:config/config.toml
files (manually)config/config.go
, which requires re-compiling Comet and producing a new image (i.e., at least 60s)We should be able to configure in the manifest file for an execution any CometBFT configuration parameter, so that it is applied to the configuration file of all nodes (in the case this setup happens in the general section of the manifest) or specific nodes (in the case this setup happens in the section referring to that specific node).
I can see different ways to implement this feature, but I am open to additional suggestions:
runner
, that reads the manifest, should be able to interpret any config parameter used the CometBFT's configuration file. For instance, ifp2p.send_rate = 10000
is declared in the general section of the manifest (or in the section of a given node), all nodes (resp., that given node) will have this setup on their configuration file, or at least this setup will override the default parameters in the configuration file. This would be an ideal solution, but I am afraid that it is not simple to implement.comet_config_file
pointing to a file that has the same format of traditional config files. The settings in this file would then replace the standards, like in the ordinary execution. This would be the same as creating a defaultconfig.Config
then loading configurations from the provided file, finally dumping the resulting configuration file to the produced home directories used by e2e. This solutions is less practical then the previous, but still much better than what we have now.comet_config
, which can be a field that can contain multiple lines (so, like a file), or a section whose content is interpreted as it was part of Comet configuration. This is pretty similar to 2., the difference is that instead of loading a file, we load a multi-line string parameter or a section, but like option 1. would allow we to keep everything that was customized inside the same manifest file.Any other ideas are welcome, but I think that having this feature will speed up significantly debugging problems using the e2e infra.
The text was updated successfully, but these errors were encountered: