Break params into three distinct files #339
Conversation
| set_options | ||
| end | ||
|
|
||
| def migrate_answers_to_user_params_file |
There was a problem hiding this comment.
This is my approach to how to migrate an answers file to this new approach. Right now this is "automatic" if an answers file is found and no user params file. This could instead also be triggered by setting a version in the configuration file or some other flag.
| end | ||
|
|
||
| config.store(user_params, config.params_files[ScenarioOption::USER_PARAMS_FILE]) | ||
| config.app[:answer_file].delete |
There was a problem hiding this comment.
Remove the notion of the answers file from the config but do not delete the answers file. We can leave that on the system as a safety mechanism and user reference.
There was a problem hiding this comment.
Does that mean that there remains an answers file on disk, which over time gets out of date, as we don't update it anymore? Or do I miss-read the code and it's still getting updated.
I'm mostly concerned that -- while we preach that the file is not to be consumed -- it is used by other tools to look things up, and could lead to unexpected results.
If it's not getting updated anymore, I think we should consider renaming it to something like asnwers.yml.obsolete or so, so that users still can manually read it, but tools that expect a certain path, fail explicitly.
ehelms
force-pushed
the
breakout-params
branch
from
c89a1e6
to
093cb15
Compare
This changes breaks params into three distinct files: one for default values coming from puppet, one for scenario defaults and one for user input. This change is aimed at allowing more control over the layering by allowing a scenario to define through it's hiera layers where to place user input, scenario defaults and puppet defaults. Additionally, this allows cleaner reasoning of what the default value coming from puppet is versus scenario defaults versus what a user has defined.
ehelms
force-pushed
the
breakout-params
branch
from
093cb15
to
4b8e84e
Compare
|
I like the overall concept and splitting the data into three store makes a ton of sense. 👍 👍 👍 One thing I couldn't wrap my mind completely around: the scenario is still in full control of which kind of storage is used, right? if it doesn't define any param files, the good old answers are used? and if it defines both? As for upgrades, I think the last option, which I read as "we mark some module parameters as 'migrate old default to user params on upgrade' and all others get new defaults", makes the most sense, as in the most cases you do want the new default value. |
The idea, to allow progressive enhancement, is that if param files are defined the answers file is basically ignored. There are still some edge cases and pieces to clean up around how that would actually work. |
Concept
Instead of storing user input, scenario defaults and puppet defaults into a single mashed up file, let's store each of those distinct layer in their own files. That allows cleaner knowledge of what the value at each layer is, cleaner changes of each layer, and the ability for a given scenario to adapt each layer into the hiera heirarchy at it's discretion. Further, this opens the door to having a file that users to edit and manage directly with input as the user input file can be read early in the loading process (not yet implemented) prior to validations.
Risk
This design is intended to be progressive and backwards compatible. Scenarios could opt to take advantage of this new layout style and moved away from the answers file method. Further, a toggle could be added (e.g. version: 2) to the configuration file to indicate a desire to use only the 3-layer approach rather than the answers file. While in the interim, this would support generating the 3-layers while keeping the answer file.
Users could end up with classes and params defined in the user input params file that no longer exist. The code could take the approach of removing any unknown classes and params and logging a warning to the user of what was removed.
Upgrades
There is a question of what happens when a user has come to rely on the functionality currently provided by a default and on upgrade this default value changes. Let's take the example from (https://github.com/theforeman/foreman-installer/blob/develop/config/katello.migrations/210323145816-foreman-tasks-backup.rb) where the default for tasks back changed from true to false. On upgrade, we would want users to retain the previous configuration in case they were relying on the functionality. A few options:
Design
This adds a configuration file option to specify 3 params files: default puppet params, scenario default params, user input params.
Default Puppet Prams
Kafo will generate all of the default params coming from the Puppet layer and write them to a file on each installer run. This creates a file that represents the Puppet params that can be layered into the hiera heirarchy, and will change as defaults change or data coming from the system changes (e.g. if FQDN changes or CPU count changes).
Scenario Default Params
This file would be similar to an un-populated answers file. This file would define the defaults, what is enabled, what are default configurations that the scenario wants to set. By being a "static" file, one not overwritten by Kafo itself, the scenario could use hiera lookups to define some default parameters directly in this file.
User Input Params
This file would contain any input that the user supplies to override parameters. This would be from the CLI inputs and could potentially include direct edits to the file that is loaded early in the process for validations. If a user's input value matches a default value or scenario default the value would be removed from the user input params to help keep user input params as only things being overriden.
Upgrading to 3-layer approach from answers file
To move from answers_file to 3-layer approach is largely about being able to discern the user input present in the answers file. The approach taken here is to effectively calculate puppet defaults, subtract them from the answers file. Then, subtract the scenario default options from the answers file and what is leftover should be user inputs. Given defaults from puppet could have changed, or scenario defaults could have changed, this would treat effectively all differences as user inputs. The plus side is that it would keep the continuity of the configuration and users could analyze the user input file and remove any they wanted handled by defaults.
Other Benefits
Reduced Need for Migrations
This model, given it calculates and produces defaults on each run separately would prevent the need to have any migrations that reset parameters because the default changed. Examples:
Migrations that puppet classes plugins would no longer be needed as they can be added directly to the scenario default params, examples that owuld not be needed: