8000 e2e: allow customize Comet's config parameters for all or specific nodes · Issue #3832 · cometbft/cometbft · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

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

Closed
cason opened this issue Aug 22, 2024 · 2 comments · Fixed by #3967
Closed

e2e: allow customize Comet's config parameters for all or specific nodes #3832

cason opened this issue Aug 22, 2024 · 2 comments · Fixed by #3967
Labels
e2e Related to our end-to-end tests enhancement New feature or request

Comments

@cason
Copy link
Contributor
cason commented Aug 22, 2024

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:

  1. Generate the network/testnet from the manifest, then edit the multiple config/config.toml files (manually)
  2. Change the defaults values of the same parameters in 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:

  1. The runner, that reads the manifest, should be able to interpret any config parameter used the CometBFT's configuration file. For instance, if p2p.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.
  2. The manifest could contain a flag with a filename, say 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 default config.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.
  3. This is an option that somehow combines the previous two. We could create a configuration flag, say 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.

@cason cason added enhancement New feature or request e2e Related to our end-to-end tests labels Aug 22, 2024
@cason
Copy link
Contributor Author
cason commented Aug 22, 2024

For instance, #3368 wants to render one specific Comet parameter, log_level customizable. The solution at the end is ad hoc, as we add a new field in the manifest and use the content of this field to override the default value of log_level defined in the config package.

The proposed approach here would render possible to achieve the same feature for any existing CometBFT configuration parameter.

@andynog
Copy link
Contributor
andynog commented Aug 22, 2024

the log_level in the manifest was implemented at #3819 if that's what you are referring to, I'm also going to implement a log_format #3836

@cason cason changed the title e2e: allow customize config parameters for all or specific nodes e2e: allow customize Comet's config parameters for all or specific nodes Sep 2, 2024
@cason cason linked a pull request Sep 2, 2024 that will close this issue
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)
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
Labels
e2e Related to our end-to-end tests enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants
0