Skip to content
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

add configuration file #111

Closed
ffromani opened this issue Jun 14, 2022 · 2 comments · Fixed by #250
Closed

add configuration file #111

ffromani opened this issue Jun 14, 2022 · 2 comments · Fixed by #250
Milestone

Comments

@ffromani
Copy link
Contributor

we have too many command line switches already.
We should add a configuration file to consolidate them and to unclutter the command line.
If practical, we should keep all the flags we have to easily override the config value settings;
alternatively, we can get rid of them and add drop-in support for configuration

@ffromani ffromani added good first issue Good for newcomers help wanted Extra attention is needed labels Jun 14, 2022
@ffromani ffromani removed the good first issue Good for newcomers label Jan 18, 2023
@ffromani ffromani added this to the v0.17.0 milestone Jan 19, 2024
@ffromani
Copy link
Contributor Author

ffromani commented Jan 19, 2024

As v0.16.0, the situation is now rully untenable and is high time we solve this issue.
Our config system grew unhorganically and chaotically, leading in wild proliferation of flags just because it was the path of least resistance.
There are a handfuls of thing which is in hindsight done quite right:

  • the separation of kubelet config data (RTE being told vs figuring out reading the kubelet config file with all the headaches related to privileges attached).
  • the hierarchical structure of the ProgArgs struct, even though this is not very clean

The way RTE is used is by far and large as daemon, rarely interactively. Thus, a config file (hier) is the best way to have config from both consumer and producer side. Flags or environs shoudl rarely be used, if at all.
Being able to extend the config file with configlets is also a requirement, to foster consumbability of the code.

All in all, moving forward we should ADD a config source in parallel, brand new, to fill ProgArgs (or equivalent) from a configfile + configlets (conf.d). Exactly another new one commandline flag (or param) should be added to support this source, then ALL THE EXISTING flags and environs should be deprecated and removed in few months/releases (whatever comes last). Details to be filled in the implementation PR.

The new config source MUST allow to set all the values currently enabled by flags, and new options should only be added from the config file.

Note the separate kubelet source is a feature, so it should be kept alongside the new config source. Merging it seems a step back.

@ffromani
Copy link
Contributor Author

ffromani commented Jan 19, 2024

It could look like this:

/etc/
└── rte
    ├── daemon
    │   ├── config.yaml    # main config source gives the baseline config
    │   └── config.yaml.d    # configlets, optional; read in lexicographicaly order; each patches the current config
    │       ├── 00-foo.yaml
    │       ├── 20-bar.yaml
    │       └── 70-baz.yaml
    └── extra
        └── config.yaml     # 1:1 move of current /etc/resource-topology-exporter/config.yaml - data RTE is told about

I don't think we should necessarily stick with yaml. The extra config should for backward compatibility, but the main config can be also json or (perhaps better) toml

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
1 participant