Re: [RFC PATCH] Makefile: terraform wasn't building the kdevops_nodes file

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

On 28/09/2025 17.53, Chuck Lever wrote:
> From: Chuck Lever <chuck.lever@oracle.com>
> 
> When "make bringup" runs "terraform plan", it complains that the
> kdevops_nodes variable isn't set.
> 
> extra_vars.yaml does contain the variable, but I'm guessing that
> somehow gen_nodes runs after gen_tfvars now? Not really sure.

I see! Thanks for reporting. Indeed my fix introduced this bug. I think
it'd be best to move KDEVOPS_NODES just after we assign the filename to it
(provision.Makefile -> guestfs.Makefile):

diff --git a/Makefile b/Makefile
index 3653591d..26615b22 100644
--- a/Makefile
+++ b/Makefile
@@ -144,16 +144,16 @@ endif
 DEFAULT_DEPS += $(ANSIBLE_CFG_FILE)
 DEFAULT_DEPS += $(ANSIBLE_INVENTORY_FILE)

-ifneq (,$(KDEVOPS_NODES))
-DEFAULT_DEPS += $(KDEVOPS_NODES)
-endif
-
 include scripts/provision.Makefile
 include scripts/firstconfig.Makefile
 include scripts/systemd-timesync.Makefile
 include scripts/journal-server.Makefile
 include scripts/update_etc_hosts.Makefile

+ifneq (,$(KDEVOPS_NODES))
+DEFAULT_DEPS += $(KDEVOPS_NODES)
+endif
+
 KDEVOPS_BRING_UP_DEPS += $(KDEVOPS_BRING_UP_DEPS_EARLY)
 KDEVOPS_BRING_UP_DEPS += $(KDEVOPS_PROVISIONED_DEVCONFIG)

The patch below changes the proposed order (ansible.cfg -> hosts -> nodes ->
rest)  in patch 5457b742d611 to:

==> [/home/dagomez.linux/src/kdevops/ansible.cfg]
==> [guestfs/kdevops_nodes.yaml]
==> [/home/dagomez.linux/src/kdevops/hosts]

I think both approaches work because nodes do not seem to be requiring hosts.
The questions is whether we need both changes? I think we need to ensure 
nodes are consistent with inventory changes. And they other way around in 
case the user deletes any of the files. So, what do you think about adding
this on top of the above?

@@ -264,13 +264,13 @@ ifneq (,$(KDEVOPS_BRING_UP_DEPS))
 include scripts/bringup.Makefile
 endif

-$(ANSIBLE_INVENTORY_FILE): .config $(ANSIBLE_CFG_FILE) $(KDEVOPS_HOSTS_TEMPLATE) $(KDEVOPS_EXTRA_VARS)
+$(ANSIBLE_INVENTORY_FILE): .config $(ANSIBLE_CFG_FILE) $(KDEVOPS_HOSTS_TEMPLATE) $(KDEVOPS_NODES) $(KDEVOPS_EXTRA_VARS)
        $(Q)ansible-playbook $(ANSIBLE_VERBOSE) --connection=local \
                --inventory localhost, \
                $(KDEVOPS_PLAYBOOKS_DIR)/gen_hosts.yml \
                --extra-vars=@./extra_vars.yaml

-$(KDEVOPS_NODES): .config $(ANSIBLE_CFG_FILE) $(KDEVOPS_NODES_TEMPLATE) $(KDEVOPS_EXTRA_VARS)
+$(KDEVOPS_NODES): .config $(ANSIBLE_CFG_FILE) $(ANSIBLE_INVENTORY_FILE) $(KDEVOPS_NODES_TEMPLATE) $(KDEVOPS_EXTRA_VARS)

Target result:
make -j12 V=1
==> [sudo]
==> [sudo]
==> [make]
==> [make]
==> [nc]
==> [nc]
==> [ansible-playbook]
==> [ansible-playbook]
==> [scripts/version_check/ansible-playbook]
==> [scripts/version_check/ansible-playbook]
==> [ansible-requirements]
==> [/home/dagomez.linux/src/kdevops/ansible.cfg]
==> [timesyncd-server]
==> [journal-server]
==> [.kdevops.depcheck]
==> [.kdevops.depcheck]
==> [guestfs/kdevops_nodes.yaml]
==> [/home/dagomez.linux/src/kdevops/hosts]

> 
> Fixes: 5457b742d611 ("Makefile: fix target dependency order")
> Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
> ---
>  Makefile | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/Makefile b/Makefile
> index fb7e9b816dbc..c1dd1232c7ff 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -258,7 +258,7 @@ ifneq (,$(KDEVOPS_BRING_UP_DEPS))
>  include scripts/bringup.Makefile
>  endif
>  
> -$(ANSIBLE_INVENTORY_FILE): .config $(ANSIBLE_CFG_FILE) $(KDEVOPS_HOSTS_TEMPLATE) $(KDEVOPS_EXTRA_VARS)
> +$(ANSIBLE_INVENTORY_FILE): .config $(ANSIBLE_CFG_FILE) $(KDEVOPS_HOSTS_TEMPLATE) $(KDEVOPS_NODES) $(KDEVOPS_EXTRA_VARS)
>  	$(Q)ANSIBLE_LOCALHOST_WARNING=False ANSIBLE_INVENTORY_UNPARSED_WARNING=False \
>  		ansible-playbook $(ANSIBLE_VERBOSE) \
>  		$(KDEVOPS_PLAYBOOKS_DIR)/gen_hosts.yml \