Code review comment for ~smoser/maas:cleanup/render-curtin-config-internally

Revision history for this message
Scott Moser (smoser) wrote :

Summary: over all, it will do reasonably well on "upgrade".

What is the upgrade path anyway, for *any* change to that file?
Do you have something in place other than debian conf-file upgrades?

If all you have is debian conf-file, then the admin running 'apt-get
dist-upgrade' is going to get prompted and see that those files are being
replaced and have the ability to diff them.

I'd consider that there are 2 likely paths:
a.) user does nothing, leaving the whole file in place as it was before.
b.) user looks at it, sees their changes and generally correctly merges
them by creating the file with *just* their changes inside.

So 'b' is the easy path, that should work or is generally not fixable
other than by the user.

for 'a', all the "builtin" config values are dictionaries.
When curtin merges dictionaries, the later one wins.
So as long as we keep the same keys in all cases, then nothing
would get run more than once and all should be well.
By 'key', i mean:
  early_commands/driver_*
and
  debconf_selections/maas

If you really wanted to safe-guard you could check the loaded file
and explicitly ignore the previously named keys, but that too could
actually cause breakage, if the user had modified those keys to
their liking.

« Back to merge proposal