Re: [PATCH 6/8] ansible_cfg: add inventory support

[Chuck Lever <chuck.lever@oracle.com>]

On 5/1/25 4:22 PM, Daniel Gomez wrote:
> On Thu, May 01, 2025 at 03:41:19PM +0100, Chuck Lever wrote:
>> On 5/1/25 2:35 PM, Daniel Gomez wrote:
>>> From: Daniel Gomez <da.gomez@samsung.com>
>>>
>>> Allow to configure the inventory file (currently via KDEVOPS_HOSTFILE
>>> and KDEVOPS_HOSTS) via ansible.cfg.
>>>
>>> The Ansible inventory file [1][2][3] is used in all kdevops/Makefile
>>> ansible-playbook calls. Enable this configuration to the global Ansible
>>> configuration file (ansible.cfg) and allow to define the inventory via
>>> Kconfig. This will simplify the playbook invocation.
>>>
>>> [1]
>>> https://docs.ansible.com/ansible/latest/reference_appendices/
>>> config.html#default-host-list
>>>
>>> [2]
>>> https://docs.ansible.com/ansible/latest/cli/
>>> ansible-inventory.html#cmdoption-ansible-inventory-i
>>>
>>> [3]
>>> https://docs.ansible.com/ansible/latest/reference_appendices/
>>> config.html#envvar-ANSIBLE_INVENTORY
>>>
>>> This has not effects yet as all ansible-playbook calls specify the
>>> --inventory/-i argument. The next commits will cleanup and allow the
>>> inventory defined in ansible.cfg to actually work.
>>>
>>> Signed-off-by: Daniel Gomez <da.gomez@samsung.com>
>>> ---
>>>  kconfigs/Kconfig.ansible_cfg                         | 5 +++++
>>>  kconfigs/Kconfig.ansible_provisioning                | 6 ------
>>>  playbooks/roles/ansible_cfg/templates/ansible.cfg.j2 | 1 +
>>>  3 files changed, 6 insertions(+), 6 deletions(-)
>>>
>>> diff --git a/kconfigs/Kconfig.ansible_cfg b/kconfigs/Kconfig.ansible_cfg
>>> index b587596bc8d5f1215fa8c006f32d941bea5288cc..05587cf31098cc5ca816d5b5fbbcdc9e8794e641 100644
>>> --- a/kconfigs/Kconfig.ansible_cfg
>>> +++ b/kconfigs/Kconfig.ansible_cfg
>>> @@ -207,6 +207,11 @@ config ANSIBLE_CFG_FORKS
>>>  
>>>  endif # !ANSIBLE_CFG_FORKS_CUSTOM
>>>  
>>> +config ANSIBLE_CFG_INVENTORY
>>> +	string "Ansible Inventory"
>>
>> Perhaps "Pathname to Ansible hosts inventory file" might be more clear.
> 
> The configuartion exposes path + file. The string output will be mapped to:
> 
> 	Section:
> 	[defaults]
> 	Key:
> 	inventory
> 
> What do you think of re-using the Ansible description of DEFAULT_HOST_LIST [1]?
> 
> "Comma-separated list of Ansible inventory sources"
> 
> Or perhaps just "Ansible inventory sources"?
> 
> [1]
> https://docs.ansible.com/ansible/latest/reference_appendices/config.html#default-host-list

For the short help string, "Ansible inventory sources" is concise.

But the help text should explain that this is a list of pathnames, or
something more explicit, IMHO. It might even provide an example.

(In general, I find I will lean on Kconfig help first before diving into
the docs/ directory, so having beefy help text right here would be
awesome.)


>>> +	output yaml
>>> +	default "$(TOPDIR_PATH)/hosts"
>>
>> I've never had need to adjust this pathname. It would be great to
>> include a help section here that explains why changing this pathname
>> might be necessary.
> 
> Users can now use as many inventories (including path) as needed. What I had
> in mind was the 'sandbox' support I sent a few months ago. This enables the
> path towards having one kdevops repo and different sandboxes, where each sandbox
> allows the user to run different kdevops configurations. I'll include a
> description with this suggestion.

That helps! Sandboxing sounds useful, I do something like that with
buildbot that could be easily replaced.

-- 
Chuck Lever