-
Notifications
You must be signed in to change notification settings - Fork 3.8k
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
Create A Command for Interacting With Configurations #11051
Comments
The Provenance chain has a config command like this already, and it has been extremely useful for sharing setups and troubleshooting. |
Config files aren't tiny databases that can be mutated over time, they're meant to be sources of truth that are read-only for the consumers that rely on them. This would create a ton of ambiguous circumstances that don't have good answers...
etc. etc. |
The same thing that should happen if a user doesn't have permission to edit the config file now, permission denied.
A related issue here is that there is currently no validation of the configuration reference files in the source with the configuration settings setup through code. The above system actually identified a bug in 0.45 where the IAVL caching config property was not properly setup in the default. In addition this command will output the entire state of the node based on the files in place and the running defaults. Meaning if you are using v1, output your configuration then compare it to the configuration output by v2 which contains new config values that are not present in the config files on disk, the changed configuration will appear in a simple diff.
The same thing that happens today when a config file is edited while the process is running.
This command doesn't attempt to change the configuration management process that an entity is using. The same resolution a user leverages for files on disk vs Ansible, Chef, etc still applies the same as if the admin edited the file directly. If you were to combine the applied config output between version one and version two of a binary it can easily highlight if the configuration has silently changed between versions. One of the common problems when relying on a config file based approach is that the executable itself has its own state of config that evolves over time and can easily diverge from the static configuration file across updates. Without this command there is no way to see this differences between versions unless you review a source code diff or hopefully spot something outlining the update in the changelog. |
I think the disconnect here is about the relationship between a config file and the program which reads it. Config files are typically input to or dependencies of programs, basically equivalent to commandline flags or env vars. The program can read them but doesn't have the right to modify them. If the program which consumes a config file can also modify it, then that relationship changes, and ownership becomes ambiguous. In concrete terms, I can no longer trust that changes I make to that config file, as an administrator or provisioning system or whatever else, won't be overwritten without my knowledge. And comments, whitespace, etc. would get mangled or lost, absent a sufficiently intelligent AST model, parser, etc. This is all hugely problematic! It would break a well-established and well-understood expectation. That's a huge cost. And for sure this capability has value, which you point out. But basically every conceivable capability has value in some circumstance. Having value is a necessary, but insufficient, rationale for adding features. You have to weigh both costs and benefits. |
One example of a program that uses and maintains a config file is The patterns it supports to manage and also leverage its configuration both from a file and the environment are an excellent example of a powerful tool that is easy to use and understand. |
Yeah! It makes a lot of sense for kubectl to own and manipulate its config file. That's because kubectl is a CLI tool, typically run interactively by users on workstations, "alive" in short bursts. That's categorically different than Cosmos SDK applications, which are typically long-running services managed by supervisors on servers. |
It's also worth pointing out again that this command was fully built and is in active use, it's implementation is not hypothetical and it's source code, behavior and interaction, and ultimately real world user feedback can be evaluated or queried directly if desired. The reason we offered this is because the cosmos sdk is severely lacking in the configuration department with multiple overlapping configuration files and non obvious default values that come from code (which changes over time) and the environment it runs in. It presents challenges to experienced engineers and new users alike. Even the core dev team gets this wrong sometimes and makes mistakes this tool trivially identifies. It is obvious that there is some disagreement on the design principal irregardless of the benefits for users. I propose that if one of the foremost systems administration frameworks in the world (kubernetes) has found value in this cli ux design pattern; we could do a lot worse than trying to emulating it. |
Etc... most of the cli on simd has nothing to do with running the node itself. The entire client config file is dedicated to this aspect. |
What config file(s) are we talking about? The OP makes me believe it's the node's app.toml and config.toml. |
All three of |
In the last 1 year that I have played around with cosmos-sdk based protocols as a enterprise grade node operator, I have been wanting this feature for the longest time. We run some internal tests while consuming protocol binaries that require us to generate latest app/config tomls and tweak them for our tests. This is currently done by hand and having a tool to set necessary keys would be extremely useful. |
We have been running this in That doesn't even include the numerous broken config setting problems we encounter while testing sdk updates. After all there are config files that get new features adddd but the existing environment won't have those. With this tool we can rewrite the config with all the new properties while maintaining the existing config overrides. Pretty much every time someone adds/updates a new config setting in the sdk there is some weird artifact which would be caught if a tool like this was integrated. |
This could be tied into the config fix issue we have an have assigned. Is there a link to your tool @iramiller. Cc @julienrbrt in this issue cause he is picking up the config fix issue |
I understand the added value of having a diff view between what is on disk and the default config. However I share @peterbourgon concerns about letting confix mutate config field (which is different than adding the missing fields from this issue: #13661) What the added value compared to a tool like dasel. I can start with the simple config migration (#13661) and add subcommand that shows a simple diff between the default config expected and the one on disk. |
Our work is here: https://github.com/provenance-io/provenance/blob/a019c35966c0c6d2e21c00592927d83ea6f8e3d2/cmd/provenanced/cmd/config.go I encourage the core dev team to explore the use cases and opinions of the various large operation teams in the ecosystem (versus more developer centric use cases). As an example ... here is my current dev environment consolidated configuration (app, config, and client config files)... I can very quickly verify what is set and share this data for troubleshooting ... or set/manage any of these settings using our
and
|
Another thing to point out is that the E.g.
Note, the |
One feature that I'd like to add too, is a |
It's important that default values, as defined in code, aren't persisted to config files. Defaults don't say "this is the ideal value", they say "this is the value we think you should use if you don't give an explicit alternative", and that preference can change over software versions. A |
The whole purpose of having config files is to let users decide. This command helps by providing users with the information they need to make configuration decisions. Already, defaults are defined in code and written to a config file, e.g. with the The primary problem this
I whole-heartedly disagree with this. While it's usually good that this doesn't happen unknowingly, there's nothing wrong with having a command that does exactly that. How do you expect people to properly configure things if they can't know what's configurable?
I fully agree. But it's impossible to provide an explicit alternative if you don't know it's possible to do so. The main purpose of putting things in a config file is because it's impossible for a maintainer to know what the ideal value is for any given user. In order for users to make those decisions, though, they need to know what decisions are possible. There is currently no solution for telling users what's newly configurable (or no longer configurable).
If the field can exist in a config file, maintainers should NEVER assume that changing its default will have an affect on existing users. The purpose of config files is to let users decide, not maintainers. Maintainers can provide guidance, though, which is most often done by putting the field in the config file with a default and explanation.
The entire purpose of the Its folly for maintainers to expect that the hard-coded default is what's used for a configurable param, or that changing the default will affect existing users. In Cosmos-SDK specifically, there's a config file template that uses the current defaults to write a config file to disk for users to manually update. As such, maintainers should expect that this behavior happens. A Lastly, the thing that made me really want the The fix was to have node operators add it to their config file. If the It'd just be really nice to be able to have my config file updated to include new fields, remove old fields, and have updated comments. |
Maintaining software running in production by modifying stuff directly on the production hosts is of course not something that anyone is, or should be, doing. Config files, binaries, systemd unit files, everything that impacts production infrastructure, are provisioned to those production hosts by a separate system — Ansible, Chef, Puppet, Kubernetes, Nomad, whatever — which takes the input from a system of record, often a GitHub repository. Assets are declarative by definition in this context — all changes are e.g. PRs. |
@peterbourgon I appreciate your passion here but your apparent lack of ability even consider that there are a variety of different approaches in use by operations teams is extremely frustrating and quite in appropriate for this context. Within this detailed thread there are in-depth explanations of how the software is being used that outline how the current configuration system in Cosmos SDK is not unsuitable and what might mitigate this (and in some cases has) . There is even feedback from engineers that work with what are quite possibly the largest professional hosting environments in the industry which detail the value being derived from these features. Yes, it is great that Ansible/Chef/K8s/etc and configuration from official record are useful tool for managing a production environment. It is very curious that you do not mention where that configuration comes from, how those teams build up, maintain, test, and review those configs. People working on that side of the operation care about scripting/repeatable and explicit configuration, and testing. These are the people that will benefit highly from establishing configs of record and these tools we describe here support their needs. |
—
I have no doubt that many operations teams routinely edit config/data/etc. files directly on their production servers as you describe. Similarly, I have no doubt that many Cosmos application developers routinely ignore errors returned by function calls. And just as Go code shouldn't try to accommodate broken/invalid consumers that ignore errors, Cosmos tooling shouldn't try to accommodate broken/invalid operators that expect to be able to edit config directly on prod machines. It's just not a valid way of working. This shouldn't be controversial — we figured this out as an industry like 20 years ago, in the LiveJournal era. |
+1 |
I'm very supportive of OP proposal |
added in #14342 |
@tac0turtle lots of people were tracking the progress of this issue ... and it looks like it was fully developed on that other issue without tagging anyone who was following along over here. In the future when issues are implemented separately could discussions be merged or at least an updated link to where the work is being done be posted? It is a disappointment to see that the entire solution was built, shipped, and merged before anyone following along here was updated. |
If you have any feedback on the current implementation direction feel free to chime in in the follow-up PR (#14568). A few ideas from here have been implemented basically set and get) and diff will be added in the follow up. I don't understand what is "disappointing". The PR references both issues from the start. |
I did send the pr to @SpicyLemon in discord and asked if he would want to take a look. just because something was developed does not mean its final, if you have feedback please let us know and we can work on incorporating it. In the future I will make sure to send you a message instead. |
Summary
Create a
config
command for managing configuration values. Interactions would includeget
andset
as well as a way to easily identify values that are different from their defaults. Another desired function is a way topack
andunpack
the configs (to and from a json file containing only configuration entries that are not their default values).Problem Definition
As an engineer and/or node operator, I want a programmatic way to interact with configuration so that it's easier to write scripts and investigate settings.
The TOML files are great for manually opening and editing settings, but are difficult to update programmatically. Additionally, since environment variables can be involved, it'd be nice to have the chain application tell me what it thinks the config values are.
Furthermore, it would be nice to have a single small config file that only contains non-default values and can be copied and shared.
Proposal
Add a
config
command with sub-commands:get
- Gets the value being used for a configuration entry.set
- Sets config values in their appropriate file.changed
- Outputs all config values that are different from the defaults.pack
- Converts the toml config files into a single small config file with only entries there are different from the default.unpack
- Splits the single small config file back into the original toml file formats.home
- Outputs the string being used as the home path.This command will allow management of all three of the configurations: Cosmos (
app.toml
), Tendermint (config.toml
), and Client (client.toml
).This will probably also require some refactoring of the code that loads the configs.
For Admin Use
The text was updated successfully, but these errors were encountered: