Re: [PATCH v2 33/37] devconfig: include the "service" group in the playbooks hosts list

On 02/09/2025 22.30, Chuck Lever wrote:
> On 9/2/25 4:27 PM, Daniel Gomez wrote:
>> On 02/09/2025 15.54, Chuck Lever wrote:
>>> From: Chuck Lever <[email protected]>
>>>
>>> Since commit 1cf0800c9ffc ("gen_hosts: templates: include localhost
>>> in the all group"), the devconfig playbook is no longer updating the
>>> nfsd service host.
>>>
>>> 1. Actually all the service hosts need to be updated by devconfig,
>>>    so ensure devconfig runs against the service group.
>>>
>>> 2. Since devconfig can run against most workflows, add blank
>>>    [service] groups to any inventory template that doesn't already
>>>    have it.
>>>
>>> Fixes: 1cf0800c9ffc ("gen_hosts: templates: include localhost in the all group")
>>> Signed-off-by: Chuck Lever <[email protected]>
>>> ---
>>>  playbooks/devconfig.yml                          | 2 +-
>>>  playbooks/roles/gen_hosts/templates/blktests.j2  | 3 +++
>>>  playbooks/roles/gen_hosts/templates/cxl.j2       | 3 +++
>>>  playbooks/roles/gen_hosts/templates/ltp.j2       | 4 ++++
>>>  playbooks/roles/gen_hosts/templates/mmtests.j2   | 3 +++
>>>  playbooks/roles/gen_hosts/templates/selftests.j2 | 3 +++
>>>  playbooks/roles/gen_hosts/templates/sysbench.j2  | 3 +++
>>>  scripts/devconfig.Makefile                       | 9 ++++-----
>>>  scripts/firstconfig.Makefile                     | 2 +-
>>>  scripts/provision.Makefile                       | 2 +-
>>>  10 files changed, 26 insertions(+), 8 deletions(-)
>>>
>>> diff --git a/playbooks/devconfig.yml b/playbooks/devconfig.yml
>>> index cebaa87a7228..f0ca7d767efe 100644
>>> --- a/playbooks/devconfig.yml
>>> +++ b/playbooks/devconfig.yml
>>> @@ -1,6 +1,6 @@
>>>  ---
>>>  - name: Configure developer environment on target systems
>>> -  hosts: baseline:dev
>>> +  hosts: baseline:dev:service
>>>    gather_facts: false
>>>    roles:
>>>      - role: devconfig
>>> diff --git a/playbooks/roles/gen_hosts/templates/blktests.j2 b/playbooks/roles/gen_hosts/templates/blktests.j2
>>> index 5bf50acbddac..6aa434c3383c 100644
>>> --- a/playbooks/roles/gen_hosts/templates/blktests.j2
>>> +++ b/playbooks/roles/gen_hosts/templates/blktests.j2
>>> @@ -22,3 +22,6 @@ ansible_python_interpreter =  "{{ kdevops_python_interpreter }}"
>>>  {% endif %}
>>>  [dev:vars]
>>>  ansible_python_interpreter =  "{{ kdevops_python_interpreter }}"
>>> +[service]
>>> +[service:vars]
>>> +ansible_python_interpreter =  "{{ kdevops_python_interpreter }}"
>>> diff --git a/playbooks/roles/gen_hosts/templates/cxl.j2 b/playbooks/roles/gen_hosts/templates/cxl.j2
>>> index 64bec4561b6c..cab7da246b11 100644
>>> --- a/playbooks/roles/gen_hosts/templates/cxl.j2
>>> +++ b/playbooks/roles/gen_hosts/templates/cxl.j2
>>> @@ -16,3 +16,6 @@ ansible_python_interpreter =  "{{ kdevops_python_interpreter }}"
>>>  {% endif %}
>>>  [dev:vars]
>>>  ansible_python_interpreter =  "{{ kdevops_python_interpreter }}"
>>> +[service]
>>> +[service:vars]
>>> +ansible_python_interpreter =  "{{ kdevops_python_interpreter }}"
>>> diff --git a/playbooks/roles/gen_hosts/templates/ltp.j2 b/playbooks/roles/gen_hosts/templates/ltp.j2
>>> index 828fd6362718..6946b3f3593b 100644
>>> --- a/playbooks/roles/gen_hosts/templates/ltp.j2
>>> +++ b/playbooks/roles/gen_hosts/templates/ltp.j2
>>> @@ -24,3 +24,7 @@ ansible_python_interpreter =  "{{ kdevops_python_interpreter }}"
>>>  {% endif %}
>>>  [dev:vars]
>>>  ansible_python_interpreter =  "{{ kdevops_python_interpreter }}"
>>> +
>>> +[service]
>>> +[service:vars]
>>> +ansible_python_interpreter =  "{{ kdevops_python_interpreter }}"
>>> diff --git a/playbooks/roles/gen_hosts/templates/mmtests.j2 b/playbooks/roles/gen_hosts/templates/mmtests.j2
>>> index 1252fe876e6c..8df4a3c31213 100644
>>> --- a/playbooks/roles/gen_hosts/templates/mmtests.j2
>>> +++ b/playbooks/roles/gen_hosts/templates/mmtests.j2
>>> @@ -31,3 +31,6 @@ ansible_python_interpreter =  "{{ kdevops_python_interpreter }}"
>>>  {% endfor %}
>>>  [mmtests:vars]
>>>  ansible_python_interpreter =  "{{ kdevops_python_interpreter }}"
>>> +[service]
>>> +[service:vars]
>>> +ansible_python_interpreter =  "{{ kdevops_python_interpreter }}"
>>> diff --git a/playbooks/roles/gen_hosts/templates/selftests.j2 b/playbooks/roles/gen_hosts/templates/selftests.j2
>>> index e505bbf194b4..af0d6b341048 100644
>>> --- a/playbooks/roles/gen_hosts/templates/selftests.j2
>>> +++ b/playbooks/roles/gen_hosts/templates/selftests.j2
>>> @@ -22,3 +22,6 @@ ansible_python_interpreter =  "{{ kdevops_python_interpreter }}"
>>>  {% endif %}
>>>  [dev:vars]
>>>  ansible_python_interpreter =  "{{ kdevops_python_interpreter }}"
>>> +[service]
>>> +[service:vars]
>>> +ansible_python_interpreter =  "{{ kdevops_python_interpreter }}"
>>> diff --git a/playbooks/roles/gen_hosts/templates/sysbench.j2 b/playbooks/roles/gen_hosts/templates/sysbench.j2
>>> index 107eebc5fbce..e30773c1b84a 100644
>>> --- a/playbooks/roles/gen_hosts/templates/sysbench.j2
>>> +++ b/playbooks/roles/gen_hosts/templates/sysbench.j2
>>> @@ -22,3 +22,6 @@ ansible_python_interpreter =  "{{ kdevops_python_interpreter }}"
>>>  {% endif %}
>>>  [dev:vars]
>>>  ansible_python_interpreter =  "{{ kdevops_python_interpreter }}"
>>> +[service]
>>> +[service:vars]
>>> +ansible_python_interpreter =  "{{ kdevops_python_interpreter }}"
>>> diff --git a/scripts/devconfig.Makefile b/scripts/devconfig.Makefile
>>> index 146c70f78897..fedf1f2ca21f 100644
>>> --- a/scripts/devconfig.Makefile
>>> +++ b/scripts/devconfig.Makefile
>>> @@ -57,11 +57,10 @@ extend-extra-args-devconfig:
>>>  PHONY += devconfig
>>>  devconfig: $(KDEVOPS_NODES)
>>>  	$(Q)ansible-playbook $(ANSIBLE_VERBOSE) \
>>> -		--limit 'baseline:dev:nfsd' \
>>> +		--limit 'baseline:dev:service' \
>>
>> You don't need --limit at all (I missed this cleanup!). The playbook already has
>> hosts: baseline:dev:service.
>>
>>>  		$(KDEVOPS_PLAYBOOKS_DIR)/devconfig.yml \
>>>  		--extra-vars="$(BOOTLINUX_ARGS)" \
>>> -		--extra-vars '{ kdevops_cli_install: True }' \
>>> -		$(LIMIT_HOSTS)
>>> +		--extra-vars '{ kdevops_cli_install: True }'
>>
>> Any reason to remove LIMIT_HOSTS support from here? Maybe not useful for these
>> targets?
> 
> Agreed, if we are simply removing the "--limit" option then I suppose
> the $LIMIT_HOSTS is no longer redundant and should stay.

Even if we keep the first --limit argument, it can be combined with LIMIT_HOSTS.
Ansible will take the last argument which may be more restrictive and user
controlled.