kernelci.cli: implement kci config set command

[JenySadadia]

Implement a command to set a value in the YAML config file at the provided path where path is a dotted string of a YAML hierarchy e.g. api.docker-host.url=new-url. Implement a helper function in kernelci.config to interate over YAML files and update a value.

[gctucker]

OK I'm going to draw the line before this PR, someone else will have to review it and any future ones I think. I'll make sure the changes that I've been holding back for a month now get to work on top of the current main branch and submit PRs (probably today).

[nuclearcat]

I tried to use it in following way:

kernelci-core$ ./kci config list-files
config/core/build-configs-cip.yaml
config/core/db-configs.yaml
config/core/test-configs.yaml
config/core/build-configs-android.yaml
config/core/rootfs-configs-chromeos.yaml
config/core/rootfs-configs.yaml
config/core/test-configs-riscv.yaml
config/core/rootfs-images.yaml
config/core/test-configs-chromeos.yaml
config/core/lab-configs-dummy.yaml
config/core/test-configs-cip.yaml
config/core/api-configs.yaml
config/core/runtime-configs.yaml
config/core/build-configs.yaml
config/core/storage.yaml
config/core/build-configs-stable.yaml

kernelci-core$ cat config/core/api-configs.yaml
api:
  docker-host:
    url: asasas
  early-access:
    url: https://kernelci-api.eastus.cloudapp.azure.com
  staging.kernelci.org:
    url: https://staging.kernelci.org:9000

kernelci-core$ /kci config dump api.docker-host
timeout: 60
url: asasas
version: latest



kernelci-core$ ./kci config dump api.docker-host.timeout
60
kernelci-core$ ./kci config set api.docker-host.timeout=6
kernelci-core$ ./kci config dump api.docker-host.timeout
60

It doesnt look it is changing anything, this value doesn't exist in config, but appears in dump.
Probably it is some default? Then likely it should be overwritten in config.

But worked with:

kernelci-core$ ./kci-staging.sh config dump api.docker-host.url
http://172.17.0.1:8001
kernelci-core$ ./kci-staging.sh config set api.docker-host.url=asasas
kernelci-core$ ./kci-staging.sh config dump api.docker-host.url
asasas

Also as i understand on change it will remove all comments from config files?
(Seems it is unlikely can be avoided.)
What is use cases of this command?

[gctucker]

I'm not sure it's going to work, as Denys pointed out. It's nice to have the feature available, a bit like git config can set values in the config file and many other tools can do that too. Editing YAML by hand can be cumbersome, especially when the data is large. However, I think it should be instead changing the value of the config object loaded from YAML and then doing a dump to automatically generate the YAML files, rather than editing dictionaries with the raw data. Also this would mostly work with files already generated to avoid undoing any manual formatting. I believe comments can be preserved as the YAML loader should be able to parse and store them somehow but it's not currently taken into account. I would say, this will be easier to implement once the legacy YAML stuff is entirely out of the way so in about one year.

As a side note: a much more useful set kind of command to implement right now would probably be the one to edit node JSON data I think. That's for manual interaction with the API, e.g. first you get a node then you add some results to it with kci node set result=pass or something and submit it again.

[nuclearcat]

Since efforts are done i think we can have use cases for this command, i can see it might be used in some cases of automated deployments (especially staging/tests/etc). But i am wondering if there is any other use cases.
Just we need to make sure it works in all combinations.

[gctucker]

Also it would need to track which file was used to load which config, which gets very complicated with overlays. So, I would personally choose to drop it for now and have a separate tool to edit the YAML automatically if we need this kind of feature in the meantime, just like we have one to update the rootfs URLs.

[JenySadadia]

Thanks for the reviews @nuclearcat @gctucker
Yes, it drops all the comments and even blank lines between different tags as I have used yaml.dump to write to the YAML file. I had searched for alternatives to the dump method but couldn't find anything.
Maybe using a loader would solve the issue as Guillaume suggested. But I'm not sure atm.

@nuclearcat Thanks for testing. I need to investigate why it didn't work when you tried to update timeout.

[gctucker]

Another thought I had later on, the command could have an argument with an explicit file name. So then it would load configs only from that file, set a value there and generate the new YAML which could be used to overwrite the same file or dumped on stdout. That way, users would manage some generated files separately from the manually edited ones. The --yaml-config argument could be used for this but there could be a check to enforce loading only a single file with the kci config set command.

[JenySadadia]

I checked with different loader options. I also tested with loader.FullLoader. However, it does not preserve comments or blank spaces. So, I investigated further and found out that pyyaml doesn't support it yet.
There is also an open GH issue asking for support yaml/pyyaml#90.
So, if we think about alternate options, as Guillaume suggested, we can have a different copy of the file with edits if we really need this kind of automation. Bdw, creating a different YAML file instead of updating doesn't make lot of a sense to me as I think it would create a mess of multiple files.
I'd rather prefer to update configurations manually and drop this PR.
What do you suggest? @nuclearcat @gctucker

[gctucker]

Right, my suggestion is based on the idea that some files are managed by hand and some are auto-generated. When setting a value using kci config set, the user needs to specify the file to load / edit / dump for this command and the user can do this with automatically-generated files only such as the one with the rootfs URLs. So it would be a convenience feature for the user and up to the user to use it correctly on the right files.

[JenySadadia]

Closing the PR as per the above discussion.