|
Thanks for comment Ernesto!, The idea about using "config-change" label to do a retrospective analysis of config changes did not cross my mind. This is an amazing use case. The reference to the "api-change" label is very helpful. Regarding the format of the
comment. A sample comment format looks like below, it's a simple JSON format so it should be very easy to search for the keys that are suspected to have their configuration changed:
{
"new": {
"mgr.yaml.in": [
"mon_warn_on_pool_no_app_grace",
"mgr_max_pg_creating"
],
...
},
"deleted": {
"mon.yaml.in": [
"mon_osd_max_creating_pgs"
],
...
},
"modified": {
"osd_scrub_load_threshold": {
"default": {
"before": 0.5,
"after": 10.0
},
...
},
}
}
Thanks,
Naveen
From: Ernesto Puerta <epuertat@xxxxxxxxxx>
Sent: Tuesday, March 18, 2025 9:44 PM
To: Naveen Naidu <naveen.naidu@xxxxxxx>
Cc: dev@xxxxxxx <dev@xxxxxxx>
Subject: [EXTERNAL] Re: Ceph Configuration Diff Tool - Request for comments
That's a great initiative, Naveen! Regarding #2, if I may: I'd suggest that we add some label (e. g. : "config-change") to those PRs. And we also run this against the last N thousand PRs, so that we also get a retrospective analysis
That's a great initiative, Naveen!
Regarding #2, if I may:
- I'd suggest that we add some label (e.g.: "config-change") to those PRs. And we also run this against the last N thousand PRs, so that we also get a retrospective analysis of config changes.
- BTW, in the Dashboard team we also have a CI check that adds a label ("api-changes") if a change is made to the OpenAPI spec file. I hope that's useful as a reference.
- If the comment added to the PR follows some "structure", that might allow the Pull Request search for fine-grained look-ups. For example, based on a recent change, searching for "osd_scrub_load_threshold = 10" should bring me
to this PR where that setting changed from 0.5 to 10 (in this case, the commit/PR message was nicely written and
a simple PR search already worked, but that's not always the case).
BTW, maybe not for the first iteration, but it'd be interesting to consider that there are Ceph settings lying outside the `src/common/options/` dir, like the `MODULE_OPTIONS` from the mgr-modules.
Thanks for doings this!
For the concerns you called out
1. Description field - can we just ignore changes that are only in the Description field of the yaml, programmatically?
2. Experimental fields - should this just be a field/tag in the yaml file itself? Ie mark the field experimental. This will also be useful for docs generation in general?
-Joseph
Thanks for taking the time to looking at the proposal and leaving a feedback Nathan ^^!
Regarding the GitHub action, the reason we wanted it to be optional is for two reasons:
-
Update happens to a field of already existing configuration that does not warrant an entry in release notes, for example: the "description" field.
-
There may be configuration options that have been introduced only for experimental purpose, that we do not want to expose to the users as of yet. (this maybe a very rare scenario yet imho
a valid one)
I totally understand the concern that making it non-optional would mean increased chances of missing the documentation of a configuration option, but I think we can reduce the probability of
this happening by adding the label (say, "release-note-update-maybe" - I'll come up with better name!) to the PR in addition to the PR comments to make the author and reviewers aware of the extra check of ensuring release notes are updated whenever necessary.
Please let me know if you have any other suggestion ^^
Thanks,
Naveen
This Message Is From an External Sender
This message came from outside your organization.
Hi Naveen,
This is a great idea! +1 on both parts of the proposal. This exact problem has definitely caught people off guard in the past when performing upgrades, so having a quick and easy way to verify changes will help reduce some friction for sure :)
For the Github action, would you consider making the check non-optional to enforce that the release notes have been updated? I'm trying to imagine a scenario where we would want to change the defaults/add new options and not have that documented and I'm drawing
a blank.
Thank you,
Nathan
To:
dev@xxxxxxx
Subject: Ceph Configuration Diff Tool - Request for comments
Hi all,
We are planning to introduce a new tool/script which would help with the problem of visibility of the changes done to Ceph configuration options. Currently there are two main limitations:
Some context on ceph configuration management system:
The default values for all the configuration options for a ceph cluster when starting up are in the yaml files present in the `src/common/options` directory.
For example: ` osd.yaml.in`
file tells us all the configuration options available for `ceph-osd` daemon.
The config options from yaml files are directly injected to the `CephContext` and these files are the sole source of truth to know about the configurations present for all the systems of ceph. It is important to note that, the values in the yaml files only
represent the default values with which the cluster starts with, any changes made to these config options are not reflected in the yaml files.
Proposal to solve this problem is two fold:
1. Tool to compare configuration options between releases or commit hashes
-
A script which takes commit hashes or release names and diff the yaml files present in the `src/common/options` directory and output sections like: `Deleted`, `Added` and `Updated`
-
Developers can run this script whenever they wish to know the configuration diff between two releases
-
For example: `ceph-config-diff --mode diff-branch --ref-repo <repo-url> --ref-branch main --cmp-branch squid`, a command like this will compare the current main branch with the squid branch.
2. A Github action runs on every PR (it only executes when the yaml files are updated). This will be added as an optional check.
-
This action will check if any changes have occurred to yaml config files and write a comment to the PR notifying the PR author to also update the relevant release docs.
-
This action will reuse the script mentioned above to find the diff, if any present
I believe, these two options should help solve the current visibility problems in ceph configuration system.
Please let us know your thoughts about this feature. All inputs are welcome.
Thanks,
Naveen
_______________________________________________
Dev mailing list -- dev@xxxxxxx
To unsubscribe send an email to dev-leave@xxxxxxx
_______________________________________________
Dev mailing list -- dev@xxxxxxx
To unsubscribe send an email to
dev-leave@xxxxxxx
|
