env.d merge/override problems

[Travis Truman]

When testing patch: https://review.openstack.org/#/c/338546/2 I realized that I also had an older copy of env.d in /etc/openstack_deploy/env.d

The behavior I saw is that the additions I had made in inventory/env.d/nova.yml were not applied to the inventory. I tested removing a service from inventory/env.d/nova.yml and it was removed.

Once I deleted the contents of /etc/openstack_deploy/env.d, all worked as expected based on the contents of inventory/env.d

[Nolan Brubaker (nolan-brubaker)]

This will be important for upgrading; the etc files should be removed if they've been edited, but either the deployer needs to decide that they have been, or we need to provide tooling to detect whether it is or not.

Also, this behavior may mean that overrides support isn't functioning correctly - additional keys, especially from the base, should merge, not be deleted.

[Nolan Brubaker (nolan-brubaker)]

This line is the culprit: https://github.com/openstack/openstack-ansible/blob/master/playbooks/inventory/dynamic_inventory.py#L887. I have also confirmed this behavior with a failing test case locally.

My opinion is that this code is correct - the overrides "win". However, we're in a situation here where the overrides are actually old code and unwanted, so we need to do some action on upgrade to clean up the etc override direcftory.

[Nolan Brubaker (nolan-brubaker)]

https://review.openstack.org/#/c/339768/ is a test demonstrating this behavior.

[Steve Lewis (steve-lewis)]

I'd agree with Nolan's assessment and suggest that a pre-flight check for overrides in etc should be done to compare against files in playbooks/inventory on upgrade.

If any files are the same: halt, inform of duplicated files, allow operator to clean them up as desired (either delete or modify files in etc).

A second step in the pre-flight check would be to calculate the overrides and enumerate them so the operator has visibility into what deltas are applied in their environment.

Both of these seem to be candidates for pre-flights which apply in all future upgrades, as they are useful protections against OSA changes to default inventory in the future.

[Cam J. Loader (cjloader)]

Which version is this an issue on?