Re: [PATCH v4 00/11] Define Ansible inventory in the Ansible Configuration file

[Daniel Gomez <da.gomez@kernel.org>]

On 04/07/2025 17.39, Chuck Lever wrote:
> On 7/2/25 3:35 PM, Daniel Gomez wrote:
> Some test results...

Thanks for testing and sharing!

> 
> I see a new warning with the initial "make" :
> 
> Doing minimum kdevops version check using
> scripts/version_check/ansible-playbook ...
> ansible-playbook: OK
> The current minimum kdevops expected dependencies are met.
> 
> [WARNING]: provided hosts list is empty, only localhost is available.
> Note that the implicit localhost does not match 'all'

I noticed that too but I thought it was okay to ignore it since we are
generating the ansible.cfg and no inventory is yet available. This is the
ansible-playbook command:

==> [/media/tarkir/dagomez/src/linux-kdevops/kdevops/ansible.cfg]
+ ansible-playbook -v playbooks/ansible_cfg.yml --extra-vars=@./.extra_vars_auto.yaml
No config file found; using defaults
[WARNING]: No inventory was parsed, only implicit localhost is available
[WARNING]: provided hosts list is empty, only localhost is available. Note that the implicit localhost does not match 'all'

However, reverting the changes removes the WARNING.

==> [/media/tarkir/dagomez/src/linux-kdevops/kdevops/ansible.cfg]
+ ansible-playbook -vv --inventory localhost, --connection=local playbooks/ansible_cfg.yml --extra-vars=@./.extra_vars_auto.yaml
ansible-playbook [core 2.19.0b4]
  config file = None
  configured module search path = ['/home/dagomez/.ansible/plugins/modules', '/usr/share/ansible/plugins/modules']
  ansible python module location = /usr/lib/python3/dist-packages/ansible
  ansible collection location = /home/dagomez/.ansible/collections:/usr/share/ansible/collections
  executable location = /usr/bin/ansible-playbook
  python version = 3.13.3 (main, Apr 10 2025, 21:38:51) [GCC 14.2.0] (/usr/bin/python3)
  jinja version = 3.1.6
  pyyaml version = 6.0.2 (with libyaml v0.2.5)
No config file found; using defaults
Skipping callback 'default', as we already have a stdout callback.
Skipping callback 'minimal', as we already have a stdout callback.
Skipping callback 'oneline', as we already have a stdout callback

One of the purposes of this series was to avoid specifying the inventory and
connection type for localhost runs. But I do now think it makes sense to keep
these args for this specific target/ansible-playbook run.

> 
> 
> PLAY [Ansible Configuration Role]
> **************************************************************************************************************
> 
> TASK [Gathering Facts]
> *************************************************************************************************************************
> ok: [localhost]
> 
> 
> I noticed a minor spelling mistake:
> 
> PLAY [Generate Ansible inentory for guests (hosts)]

Fixed.

> 
> 
> 
> I tried running the gitr workflow. During "make":

FYI, I fixed and pushed [1] some missing build and runtime dependencies for
Debian while trying to replicate this.

[1] Link:
https://github.com/linux-kdevops/kdevops/commit/9af865ba040f94a06c47157bd07e747fb6aca1ef

I've also found that we can remove this line:

diff --git a/workflows/gitr/Makefile b/workflows/gitr/Makefile
index 09ef466..76dcf0e 100644
--- a/workflows/gitr/Makefile
+++ b/workflows/gitr/Makefile
@@ -90,7 +90,6 @@ endif

 gitr:
        $(Q)ansible-playbook $(ANSIBLE_VERBOSE) \
-               --limit 'baseline:dev' \
                --skip-tags run_tests,run_specific_tests,copy_results \
                $(KDEVOPS_PLAYBOOKS_DIR)/gitr.yml

Since gitr playbook runs already only in baseline and dev hosts, we don't need
to be explicit here. Same reasoning for ltp, nfstests and pynfs workflows.

Well, maybe not. I wrote this before finding the issue below.

> 
> TASK [gen_nodes : Generate XML files for the libvirt guests]
> ***********************************************************************************
> changed: [localhost] => (item={'name': 'cel-nfs-v3'})
> changed: [localhost] => (item={'name': 'cel-nfsd'})
> 
> 
> But then later:
> 
> TASK [update_ssh_config_guestfs : Ensure ~/.ssh/config permissions]
> ****************************************************************************
> changed: [localhost]
> 
> PLAY RECAP
> *************************************************************************************************************************************
> localhost                  : ok=9    changed=5    unreachable=0
> failed=0    skipped=0    rescued=0    ignored=0
> 
> Traceback (most recent call last):
>   File
> "/home/cel/src/kdevops/dev/.//scripts/update_ssh_config_guestfs.py",
> line 98, in <module>
>     main()
>     ~~~~^^
>   File
> "/home/cel/src/kdevops/dev/.//scripts/update_ssh_config_guestfs.py",
> line 87, in main
>     addr = get_addr(name)
>   File
> "/home/cel/src/kdevops/dev/.//scripts/update_ssh_config_guestfs.py",
> line 39, in get_addr
>     raise Exception(f"Unable to get an address for {name} after 60s")
> Exception: Unable to get an address for cel-nfsd after 60s
> make: *** [scripts/guestfs.Makefile:58: .provisioned_once_ssh] Error 1
> cel@renoir:~/src/kdevops/dev$ virsh list
>  Id   Name         State
> ----------------------------
>  2    cel-nfs-v3   running
> 
> cel@renoir:~/src/kdevops/dev$
> 
> 
> Looks like the guestfs playbook brought up only the NFS client, but not
> the NFS server.
> 
> 

I've managed to reproduce the issue. The "problem" is that gitr hosts file (and
also other workflow hosts files) includes targets that are not baseline or dev.

So, removing the --limit for bringup should fix it:

diff --git a/scripts/guestfs.Makefile b/scripts/guestfs.Makefile
index 93d3315..bddf2d9 100644
--- a/scripts/guestfs.Makefile
+++ b/scripts/guestfs.Makefile
@@ -79,7 +79,6 @@ bringup_guestfs: $(GUESTFS_BRINGUP_DEPS)
                --extra-vars=@./extra_vars.yaml \
                --tags network,pool,base_image
        $(Q)ansible-playbook $(ANSIBLE_VERBOSE) \
-               --limit 'baseline:dev' \
                playbooks/guestfs.yml \
                --extra-vars=@./extra_vars.yaml \
                --tags bringup

This is what we do in the destroy path. It works, because guestfs has hosts:
all in the playbook YAML file. However, we cannot apply the same strategy here
because of we have this (in bringup):

- name: List defined libvirt guests
  run_once: true
  community.libvirt.virt:
    command: list_vms
    uri: "{{ libvirt_uri }}"
  register: defined_vms

- name: Provision each target node
  when:
    - 'inventory_hostname not in defined_vms.list_vms'
  block:
    - name: Set the pathname of the ssh directory for each target node
      ansible.builtin.set_fact:
        ssh_key_dir: "{{ guestfs_path }}/{{ inventory_hostname }}/ssh"
{...}

Which loops over all guests including localhost.

So, we should be explicit instead:

diff --git a/scripts/guestfs.Makefile b/scripts/guestfs.Makefile
index 93d3315..a341f25 100644
--- a/scripts/guestfs.Makefile
+++ b/scripts/guestfs.Makefile
@@ -62,7 +62,8 @@ $(KDEVOPS_PROVISIONED_SSH):
                        LIBVIRT_DEFAULT_URI=$(CONFIG_LIBVIRT_URI) \
                        $(TOPDIR)/scripts/update_ssh_config_guestfs.py; \
        fi
-       $(Q)ansible $(ANSIBLE_VERBOSE) 'baseline:dev' -m wait_for_connection
+       $(Q)ansible $(ANSIBLE_VERBOSE) 'baseline:dev:nfsd:iscsi:smbd:kdc:krb5' \
+       -m wait_for_connection
        $(Q)touch $(KDEVOPS_PROVISIONED_SSH)

 install_libguestfs:
@@ -79,8 +80,8 @@ bringup_guestfs: $(GUESTFS_BRINGUP_DEPS)
                --extra-vars=@./extra_vars.yaml \
                --tags network,pool,base_image
        $(Q)ansible-playbook $(ANSIBLE_VERBOSE) \
-               --limit 'baseline:dev' \
                playbooks/guestfs.yml \
+               --limit 'baseline:dev:nfsd:iscsi:smbd:kdc:krb5' \
                --extra-vars=@./extra_vars.yaml \
                --tags bringup
        $(Q)ansible-playbook $(ANSIBLE_VERBOSE) \

This fixes the issue but I'd rather have these hosts as part of a group. Does it
make sense to add them to the baseline group or should we create another group?
What do you think?