Azure / az-hop

The Azure HPC On-Demand Platform provides an HPC Cluster Ready solution
https://azure.github.io/az-hop/
MIT License
65 stars 54 forks source link

deduplicate default values in ansible scripts #1675

Open ltalirz opened 1 year ago

ltalirz commented 1 year ago

In what area(s)?

/area configuration

Describe the feature

We currently repeat the default value for a given config value in a number of places in the ansible scripts.

At the same time, you have the schema which also defines a default value.

In principle, "all one would need to do" (to be figured out how) is to pass the config.yml through the schema validator before it is passed on to ansible. Then one could remove all these | default("...") in the ansible scripts, reducing duplication.

xpillons commented 1 year ago

I don't know how feasible this can be if the default values are optional in the config file. Maybe we would have to define a global ansible var file for all the defaults.

ltalirz commented 1 year ago

This is typically taken care of by the schema validator library

I just checked and it appears there is no way to get the "filled with defaults" version from check-jsonschema but it is straighforward to get it from the underlying jsonschema library

https://python-jsonschema.readthedocs.io/en/stable/faq/#why-doesn-t-my-schema-s-default-property-set-the-default-on-my-instance

I.e. we would need to change our validate_schema.sh to use jsonschema directly (probably anyhow make sense since we need to do this json <> yaml conversion back and forth)

xpillons commented 1 year ago

but I don't see how the default value will be incorporated in the config file if it doesn't exists.

ltalirz commented 1 year ago

config.yml + config.schema.json

via jsonschema

=> config.defaults.json with defaults populated (defaults = from JSON schema, see link above)

via yq

=> config.defaults.yml

Ansible would then read the config.defaults.yml instead of the config.yml

But yeah, there is a small downside in that we currently don't run the schema validation every time we run ansible - only for the install.sh.

So it would no longer be possible to just modify the config.yml and then run ansible afterwards.