[PATCH 0/6] debian / libvirt / devconfig fixes

[PATCH 0/6] debian / libvirt / devconfig fixes

[Luis Chamberlain]

Changes on this v2:

I only modified this series with 1 small change and the commit log
on patch #5 "guestfs: replace ansible group permisison requirement on    
libvirt system uri" with the changes on here:

https://lore.kernel.org/all/Z-h3eTCpRC28eaCm@bombadil.infradead.org/T/#mcdb789863b84ed219ed19eeccac3eb8dcfcca17b

That's it.

Lemme know if this looks good, I just tested this on a bringup on a shared
system.

Luis Chamberlain (6):
  scripts/bringup_guestfs.sh: uninstall unattended-upgrades on debian
    guests
  devconfig: ensure unattended-upgrades is not installed on debian
  libvirt: use consistent pool path variables and use optional yaml
    output
  Kconfig: adopt output yaml for KDEVOPS_FIRST_RUN
  guestfs: replace ansible group permisison requirement on libvirt
    system uri
  gen_nodes: ensure kdevops prefix has no dashes

 Kconfig                                       |  1 +
 kconfigs/Kconfig.guestfs                      | 15 ++++
 kconfigs/Kconfig.libvirt                      | 11 ++-
 .../roles/bringup_guestfs/tasks/main.yml      | 76 +++++++++++++++++++
 .../tasks/install-deps/debian/main.yml        | 22 ++++++
 playbooks/roles/gen_nodes/tasks/main.yml      |  5 ++
 scripts/bringup_guestfs.sh                    |  6 +-
 scripts/bringup_vagrant.sh                    |  2 +-
 scripts/destroy_guestfs.sh                    |  2 +-
 scripts/destroy_vagrant.sh                    |  2 +-
 scripts/gen-nodes.Makefile                    |  6 --
 scripts/guestfs.Makefile                      |  9 +--
 scripts/prune_stale_vagrant.sh                |  2 +-
 scripts/vagrant.Makefile                      |  5 --
 14 files changed, 137 insertions(+), 27 deletions(-)

-- 
2.47.2

[PATCH 1/6] scripts/bringup_guestfs.sh: uninstall unattended-upgrades on debian guests

[Luis Chamberlain]

Be sure to remove unattended-upgrades when taking the downloaded image
from Debian to build our base image which we will use to launch guests
with.

The package unattended-upgrades [0] is one of the biggest pain points
for continous integeration on kdevops, where we race against package
installations being mucked behind our back. So let's just be
extremely vocal about it being a requirement to not be installed
on debian systems and complain and ensure systems don't have it.

Even though we have already in place devconfig rules to remove it,
upon first bringup you can still race against it!

If you do have an old guest with it, you should just remove the old
guest and re-do your new guest as otherwise you will encounter many
odd silly bugs and the issue is just races with debian doing its
upgrades with unattended-upgrades.

To my surprise unattended-upgrades has also been a default on debian
for years now, it would seem we can just move to a debian image of
"netinst" or "minimal" images but that requires more work than what
we can just do by removing the package.

Long term we really should request to see if is debian folks can
ends up packaging a proper guestfs image based on debian testing for us,
and when that does happen we ask it does not have unattended-upgrades.

[0] https://wiki.debian.org/UnattendedUpgrades

Signed-off-by: Luis Chamberlain <mcgrof@kernel.org>
---
 scripts/bringup_guestfs.sh | 1 +
 1 file changed, 1 insertion(+)

diff --git a/scripts/bringup_guestfs.sh b/scripts/bringup_guestfs.sh
index 7166842307b5..1ba2c8c6ff9a 100755
--- a/scripts/bringup_guestfs.sh
+++ b/scripts/bringup_guestfs.sh
@@ -223,6 +223,7 @@ firstboot-command systemctl stop ssh
 firstboot-command DEBIAN_FRONTEND=noninteractive DEBCONF_NONINTERACTIVE_SEEN=true dpkg-reconfigure -p low --force openssh-server
 firstboot-command systemctl start ssh
 firstboot-command apt update && apt upgrade --yes
+uninstall unattended-upgrades
 _EOT
 	# CONFIG_GUESTFS_COPY_SOURCES_FROM_HOST_TO_GUEST will not work
 	# if /etc/nsswitch.conf has a line like this:
-- 
2.47.2

[PATCH 2/6] devconfig: ensure unattended-upgrades is not installed on debian

[Luis Chamberlain]

We already remove the package on debian bringups when downloading a
an official but-not-guestfs-official (and so debian custom) image to
build our base images for our guests. Now that we have that stop-gap
measure, be sure we just fail if the package happens to be installed,
and complain to the user about it.

We can easily fix this with just two command so recommend that.

Signed-off-by: Luis Chamberlain <mcgrof@kernel.org>
---
 .../tasks/install-deps/debian/main.yml        | 22 +++++++++++++++++++
 1 file changed, 22 insertions(+)

diff --git a/playbooks/roles/devconfig/tasks/install-deps/debian/main.yml b/playbooks/roles/devconfig/tasks/install-deps/debian/main.yml
index 5624a9f8406a..954f0aede3b2 100644
--- a/playbooks/roles/devconfig/tasks/install-deps/debian/main.yml
+++ b/playbooks/roles/devconfig/tasks/install-deps/debian/main.yml
@@ -1,4 +1,26 @@
 ---
+- name: Check if unattended-upgrades is installed
+  command: dpkg-query -W -f='${Status}' unattended-upgrades
+  register: unattended_upgrade_status
+  ignore_errors: true
+  changed_when: false
+
+- name: Set fact if unattended-upgrades is installed
+  set_fact:
+    unattended_upgrades_installed: "{{ 'install ok installed' in unattended_upgrade_status.stdout }}"
+
+- name: Verify unattended-upgrades is not installed
+  fail:
+    msg: |
+      The unattended-upgrades package is installed on the base image, this
+      can cause tons of issues with CIs. Fix this by running the following
+      commands:
+
+      make cleancache
+      make bringup
+  when:
+    - unattended_upgrades_installed|bool
+
 - name: Upgrade Packages
   become: yes
   become_method: sudo
-- 
2.47.2

[PATCH 3/6] libvirt: use consistent pool path variables and use optional yaml output

[Luis Chamberlain]

Turns out we define CONFIG_KDEVOPS_STORAGE_POOL_PATH only to make it be the
same as generic     CONFIG_LIBVIRT_STORAGE_POOL_PATH only later to then
use the old GUESTFS_ARGS or VAGRANT_ARGS to define the yaml version with
a postfix "kdevops".

Fix all this mess by using yaml output and cleaing its use up.

This does not fix any bugs, it just make things consistent.

And so where you saw CONFIG_KDEVOPS_STORAGE_POOL_PATH we now just use
the generic version  CONFIG_LIBVIRT_STORAGE_POOL_PATH.

The kconfig symbol CONFIG_KDEVOPS_STORAGE_POOL_PATH gets promoted to be
what we expect it, that is:

  "{{ libvirt_storage_pool_path }}/kdevops"

This also ensure we will barf as we want to promote using ansible for
this anyway and do away with all of the CONFIG_KDEVOPS_STORAGE_POOL_PATH
uses in shell scripts.

This kills all use of CONFIG_KDEVOPS_STORAGE_POOL_PATH from shell
scripts too then.

The motivation for all this is to slowly trim away the large
error-prone script scripts/bringup_guestfs.sh into ansible. This
is just a small step towards making that easier.

Signed-off-by: Luis Chamberlain <mcgrof@kernel.org>
---
 kconfigs/Kconfig.guestfs       |  5 +++++
 kconfigs/Kconfig.libvirt       | 11 ++++++++++-
 scripts/bringup_guestfs.sh     |  2 +-
 scripts/bringup_vagrant.sh     |  2 +-
 scripts/destroy_guestfs.sh     |  2 +-
 scripts/destroy_vagrant.sh     |  2 +-
 scripts/gen-nodes.Makefile     |  6 ------
 scripts/guestfs.Makefile       |  7 +------
 scripts/prune_stale_vagrant.sh |  2 +-
 scripts/vagrant.Makefile       |  5 -----
 10 files changed, 21 insertions(+), 23 deletions(-)

diff --git a/kconfigs/Kconfig.guestfs b/kconfigs/Kconfig.guestfs
index f618fc30fe25..c6d2d1907dd5 100644
--- a/kconfigs/Kconfig.guestfs
+++ b/kconfigs/Kconfig.guestfs
@@ -1,5 +1,10 @@
 if GUESTFS
 
+config STORAGE_POOL_PATH
+	string
+	output yaml
+	default LIBVIRT_STORAGE_POOL_PATH
+
 config GUESTFS_HAS_CUSTOM_RAW_IMAGE
 	bool
 
diff --git a/kconfigs/Kconfig.libvirt b/kconfigs/Kconfig.libvirt
index 1ed967423096..cba8abf1e24b 100644
--- a/kconfigs/Kconfig.libvirt
+++ b/kconfigs/Kconfig.libvirt
@@ -155,13 +155,20 @@ config LIBVIRT_QEMU_GROUP
 	default "qemu" if !DISTRO_DEBIAN && !DISTRO_UBUNTU
 	default "libvirt-qemu" if DISTRO_DEBIAN || DISTRO_UBUNTU
 
-config KDEVOPS_STORAGE_POOL_PATH
+
+config LIBVIRT_STORAGE_POOL_PATH
 	string
+	output yaml
 	default LIBVIRT_STORAGE_POOL_PATH_AUTO if LIBVIRT && !LIBVIRT_STORAGE_POOL_PATH_CUSTOM_MANUAL
 	default LIBVIRT_STORAGE_POOL_PATH_AUTO if LIBVIRT && LIBVIRT_STORAGE_POOL_PATH_CUSTOM_CWD
 	default LIBVIRT_STORAGE_POOL_PATH_CUSTOM if LIBVIRT && LIBVIRT_STORAGE_POOL_PATH_CUSTOM_MANUAL
 	default VIRTUALBOX_STORAGE_POOL_PATH_CUSTOM if VAGRANT_VIRTUALBOX
 
+config KDEVOPS_STORAGE_POOL_PATH
+	string
+	output yaml
+	default "{{ libvirt_storage_pool_path }}/kdevops"
+
 config QEMU_BIN_PATH
 	string
 	default QEMU_BIN_PATH_LIBVIRT if LIBVIRT
@@ -1052,6 +1059,7 @@ endif
 
 config LIBVIRT_STORAGE_POOL_CREATE
 	bool "Should we build a custom storage pool for you?"
+	output yaml
 	default n if !LIBVIRT_STORAGE_POOL_PATH_INFER_ADVANCED
 	default $(shell, ./scripts/get_libvirsh_pool_enabled.sh) if LIBVIRT_STORAGE_POOL_PATH_INFER_ADVANCED
 	help
@@ -1063,6 +1071,7 @@ config LIBVIRT_STORAGE_POOL_CREATE
 
 config LIBVIRT_STORAGE_POOL_NAME
 	string "Libvirt storage pool name"
+	output yaml
 	depends on LIBVIRT_STORAGE_POOL_CREATE
 	default "default" if !LIBVIRT_STORAGE_POOL_PATH_INFER_ADVANCED
 	default $(shell, ./scripts/get_libvirsh_pool_name.sh) if LIBVIRT_STORAGE_POOL_PATH_INFER_ADVANCED
diff --git a/scripts/bringup_guestfs.sh b/scripts/bringup_guestfs.sh
index 1ba2c8c6ff9a..976d1e78ed6a 100755
--- a/scripts/bringup_guestfs.sh
+++ b/scripts/bringup_guestfs.sh
@@ -14,7 +14,7 @@ IMG_FMT="qcow2"
 if [ "${CONFIG_LIBVIRT_EXTRA_DRIVE_FORMAT_RAW}" = "y" ]; then
 	IMG_FMT="raw"
 fi
-STORAGETOPDIR="${CONFIG_KDEVOPS_STORAGE_POOL_PATH}"
+STORAGETOPDIR="${CONFIG_LIBVIRT_STORAGE_POOL_PATH}"
 STORAGEDIR="${STORAGETOPDIR}/kdevops/guestfs"
 QEMU_GROUP=$CONFIG_LIBVIRT_QEMU_GROUP
 GUESTFSDIR="${TOPDIR}/guestfs"
diff --git a/scripts/bringup_vagrant.sh b/scripts/bringup_vagrant.sh
index 4d30c2312000..4e163871d482 100755
--- a/scripts/bringup_vagrant.sh
+++ b/scripts/bringup_vagrant.sh
@@ -55,7 +55,7 @@ vagrant_check_dups()
 			# instances *and* we know one does not exist in another
 			# directory for this user.
 
-			kdevops_pool_path="$CONFIG_KDEVOPS_STORAGE_POOL_PATH"
+			kdevops_pool_path="$CONFIG_LIBVIRT_STORAGE_POOL_PATH"
 			# For libvirt we can do one more global sanity check
 			if [[ "$CONFIG_LIBVIRT" == "y" ]]; then
 				possible_image="${kdevops_pool_path}/vagrant_${instance}.img"
diff --git a/scripts/destroy_guestfs.sh b/scripts/destroy_guestfs.sh
index 58dca78d85cf..ee5dc2b57d6d 100755
--- a/scripts/destroy_guestfs.sh
+++ b/scripts/destroy_guestfs.sh
@@ -7,7 +7,7 @@ source ${TOPDIR}/scripts/lib.sh
 
 export LIBVIRT_DEFAULT_URI=$CONFIG_LIBVIRT_URI
 
-STORAGEDIR="${CONFIG_KDEVOPS_STORAGE_POOL_PATH}/kdevops/guestfs"
+STORAGEDIR="${CONFIG_LIBVIRT_STORAGE_POOL_PATH}/kdevops/guestfs"
 GUESTFSDIR="${TOPDIR}/guestfs"
 
 if [ -f "$GUESTFSDIR/kdevops_nodes.yaml" ]; then
diff --git a/scripts/destroy_vagrant.sh b/scripts/destroy_vagrant.sh
index 4e5bb9d64bef..bd5e43d0044e 100755
--- a/scripts/destroy_vagrant.sh
+++ b/scripts/destroy_vagrant.sh
@@ -18,7 +18,7 @@ rm -rf .vagrant
 # doing so we don't check for global dups or anything like that.
 UNINIT_CURRENT_INSTANCES=$(vagrant status --machine-readable | grep ",state," | grep not_created | awk -F "," '{print $2}')
 for i in $UNINIT_CURRENT_INSTANCES; do
-	UNINIT_INSTANCE_SPARE_DRIVE_DIR="${CONFIG_KDEVOPS_STORAGE_POOL_PATH}/kdevops/$i"
+	UNINIT_INSTANCE_SPARE_DRIVE_DIR="${CONFIG_LIBVIRT_STORAGE_POOL_PATH}/kdevops/$i"
 	if [[ -d $UNINIT_INSTANCE_SPARE_DRIVE_DIR ]]; then
 		echo "Found unitialized (possibly old) instance spare drive directory, removing it ... $i"
 		rm -rf $UNINIT_INSTANCE_SPARE_DRIVE_DIR
diff --git a/scripts/gen-nodes.Makefile b/scripts/gen-nodes.Makefile
index 8bee2db57591..775ec5c49808 100644
--- a/scripts/gen-nodes.Makefile
+++ b/scripts/gen-nodes.Makefile
@@ -42,12 +42,6 @@ GEN_NODES_EXTRA_ARGS += libvirt_session_management_network_device='$(subst ",,$(
 GEN_NODES_EXTRA_ARGS += libvirt_session_public_network_dev='$(subst ",,$(CONFIG_LIBVIRT_SESSION_PUBLIC_NETWORK_DEV))'
 endif
 
-ifeq (y,$(CONFIG_LIBVIRT_STORAGE_POOL_CREATE))
-GEN_NODES_EXTRA_ARGS += libvirt_storage_pool_create='True'
-GEN_NODES_EXTRA_ARGS += libvirt_storage_pool_name='$(subst ",,$(CONFIG_LIBVIRT_STORAGE_POOL_NAME))'
-GEN_NODES_EXTRA_ARGS += libvirt_storage_pool_path='$(subst ",,$(CONFIG_KDEVOPS_STORAGE_POOL_PATH))'
-endif
-
 GEN_NODES_EXTRA_ARGS += libvirt_extra_storage_aio_mode='$(subst ",,$(CONFIG_LIBVIRT_AIO_MODE))'
 GEN_NODES_EXTRA_ARGS += libvirt_extra_storage_aio_cache_mode='$(subst ",,$(CONFIG_LIBVIRT_AIO_CACHE_MODE))'
 
diff --git a/scripts/guestfs.Makefile b/scripts/guestfs.Makefile
index 8b1b4e9fc7d5..d08e697f3cfb 100644
--- a/scripts/guestfs.Makefile
+++ b/scripts/guestfs.Makefile
@@ -18,11 +18,6 @@ QEMU_GROUP:=$(subst ",,$(CONFIG_LIBVIRT_QEMU_GROUP))
 GUESTFS_ARGS += kdevops_storage_pool_group='$(QEMU_GROUP)'
 GUESTFS_ARGS += storage_pool_group='$(QEMU_GROUP)'
 
-STORAGE_POOL_PATH:=$(subst ",,$(CONFIG_KDEVOPS_STORAGE_POOL_PATH))
-KDEVOPS_STORAGE_POOL_PATH:=$(STORAGE_POOL_PATH)/kdevops
-GUESTFS_ARGS += storage_pool_path=$(STORAGE_POOL_PATH)
-GUESTFS_ARGS += kdevops_storage_pool_path=$(KDEVOPS_STORAGE_POOL_PATH)
-
 9P_HOST_CLONE :=
 ifeq (y,$(CONFIG_BOOTLINUX_9P))
 9P_HOST_CLONE := 9p_linux_clone
@@ -109,4 +104,4 @@ destroy_guestfs:
 PHONY += destroy_guestfs
 
 cleancache:
-	$(Q)rm -f $(subst ",,$(CONFIG_KDEVOPS_STORAGE_POOL_PATH))/kdevops/guestfs/base_images/*
+	$(Q)rm -f $(subst ",,$(CONFIG_LIBVIRT_STORAGE_POOL_PATH))/kdevops/guestfs/base_images/*
diff --git a/scripts/prune_stale_vagrant.sh b/scripts/prune_stale_vagrant.sh
index 95b88911c0a6..61ac0e86b6c2 100755
--- a/scripts/prune_stale_vagrant.sh
+++ b/scripts/prune_stale_vagrant.sh
@@ -37,7 +37,7 @@ if [[ "$CONFIG_LIBVIRT" != "y" ]]; then
 fi
 
 if [[ $# -eq 0 ]]; then
-	KDEVOPS_POOL_PATH="$CONFIG_KDEVOPS_STORAGE_POOL_PATH"
+	KDEVOPS_POOL_PATH="$CONFIG_LIBVIRT_STORAGE_POOL_PATH"
 elif [[ $# -eq 1 ]]; then
 	if [[ "$1" == "--help" ]]; then
 		echo "Usage: $0"
diff --git a/scripts/vagrant.Makefile b/scripts/vagrant.Makefile
index 368c2f84aa89..664c8f9cfc5c 100644
--- a/scripts/vagrant.Makefile
+++ b/scripts/vagrant.Makefile
@@ -44,11 +44,6 @@ ifeq (y,$(CONFIG_VAGRANT_VIRTUALBOX))
 VAGRANT_ARGS += virtualbox_provider=True
 endif
 
-STORAGE_POOL_PATH:=$(subst ",,$(CONFIG_KDEVOPS_STORAGE_POOL_PATH))
-KDEVOPS_STORAGE_POOL_PATH:=$(STORAGE_POOL_PATH)/kdevops
-VAGRANT_ARGS += storage_pool_path=$(STORAGE_POOL_PATH)
-VAGRANT_ARGS += kdevops_storage_pool_path=$(KDEVOPS_STORAGE_POOL_PATH)
-
 VAGRANT_9P_HOST_CLONE :=
 ifeq (y,$(CONFIG_BOOTLINUX_9P))
 VAGRANT_9P_HOST_CLONE := vagrant_9p_linux_clone
-- 
2.47.2

[PATCH 4/6] Kconfig: adopt output yaml for KDEVOPS_FIRST_RUN

[Luis Chamberlain]

This will be used later.

Signed-off-by: Luis Chamberlain <mcgrof@kernel.org>
---
 Kconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Kconfig b/Kconfig
index 357341be108d..988782a9dc83 100644
--- a/Kconfig
+++ b/Kconfig
@@ -30,6 +30,7 @@ config NEEDS_LOCAL_DEVELOPMENT_PATH
 
 config KDEVOPS_FIRST_RUN
 	bool "Is this your first time running kdevops on this system?"
+	output yaml
 	default n
 	help
 	  When you run kdevops for the first time we can enable options
-- 
2.47.2

[PATCH 5/6] guestfs: replace ansible group permisison requirement on libvirt system uri

[Luis Chamberlain]

The bringup process for libvirt system URI support (not session),
so all debian based distros, requieres us to be paranoid about the
permissions of our storage directory where we place our libvirt
storage pool, and guestfs images.

We used to be stupid and were hammering with a sledge hammer a crazy
sudo chown -R on a target storage path. That was removed by commit
c31459dc384c ("scripts/bringup_guestfs.sh: fix silly directory permission
fix"). I rushed that change in because it was affecting live systems
and we needed to get testing moving.

This replaces the old requirement with some more less aggressive
directory creation and directory permission requirements and adds some
sanity checks which don't do the crazy wild permission changes that
are possible with a recursive call.

Signed-off-by: Luis Chamberlain <mcgrof@kernel.org>
---
 kconfigs/Kconfig.guestfs                      | 10 +++
 .../roles/bringup_guestfs/tasks/main.yml      | 76 +++++++++++++++++++
 scripts/bringup_guestfs.sh                    |  3 -
 scripts/guestfs.Makefile                      |  2 +-
 4 files changed, 87 insertions(+), 4 deletions(-)

diff --git a/kconfigs/Kconfig.guestfs b/kconfigs/Kconfig.guestfs
index c6d2d1907dd5..d309436fa7c9 100644
--- a/kconfigs/Kconfig.guestfs
+++ b/kconfigs/Kconfig.guestfs
@@ -5,6 +5,16 @@ config STORAGE_POOL_PATH
 	output yaml
 	default LIBVIRT_STORAGE_POOL_PATH
 
+config GUESTFS_STORAGE_DIR
+	string
+	output yaml
+	default "{{ kdevops_storage_pool_path }}/kdevops/guestfs"
+
+config GUESTFS_BASE_IMAGE_DIR
+	string
+	output yaml
+	default "{{ guestfs_storage_dir }}/base_images"
+
 config GUESTFS_HAS_CUSTOM_RAW_IMAGE
 	bool
 
diff --git a/playbooks/roles/bringup_guestfs/tasks/main.yml b/playbooks/roles/bringup_guestfs/tasks/main.yml
index dcbbaef02522..0b193dad807f 100644
--- a/playbooks/roles/bringup_guestfs/tasks/main.yml
+++ b/playbooks/roles/bringup_guestfs/tasks/main.yml
@@ -42,6 +42,82 @@
   when: guestfs_subdirectories.matched == 0
   tags: [ 'config-check' ]
 
+- name: Create storage pool path directory if (libvirt session uri)
+  file:
+    path: "{{ libvirt_storage_pool_path }}"
+    state: directory
+  when: 'not libvirt_uri_system|bool'
+  tags: ['storage-pool-path']
+
+- name: Create storage pool path directory and set group if using (libvirt system uri)
+  file:
+    path: "{{ libvirt_storage_pool_path }}"
+    state: directory
+    owner: root
+    group: "{{ libvirt_qemu_group }}"
+    mode: "0775"
+  when: 'libvirt_uri_system|bool'
+  tags: ['storage-pool-path']
+
+- name: Create kdevops guestfs storage directory if missing (libvirt session uri)
+  file:
+    path: "{{ guestfs_base_image_dir }}"
+    state: directory
+    mode: '0755'
+  tags: ['storage-pool-path']
+  when:
+    - 'not libvirt_uri_system|bool'
+
+- name: Create kdevops guestfs storage directory if missing (libvirt system uri)
+  become: yes
+  become_flags: 'su - -c'
+  become_method: sudo
+  file:
+    path: "{{ guestfs_base_image_dir }}"
+    state: directory
+    mode: '0775'
+    group: "{{ libvirt_qemu_group }}"
+  tags: ['storage-pool-path']
+  when:
+    - 'libvirt_uri_system|bool'
+
+- name: Check if directory is owned by the correct group (libvirt system uri)
+  become: yes
+  become_flags: 'su - -c'
+  become_method: sudo
+  command: stat -c '%G' "{{ libvirt_storage_pool_path }}"
+  register: dir_group
+  changed_when: false
+  tags: ['storage-pool-path']
+  when:
+    - 'libvirt_uri_system|bool'
+
+- name: Check if directory has group write permissions (libvirt system uri)
+  become: yes
+  become_flags: 'su - -c'
+  become_method: sudo
+  command: stat -c '%A' "{{ libvirt_storage_pool_path }}"
+  register: dir_perms
+  changed_when: false
+  tags: ['storage-pool-path']
+  when:
+    - 'libvirt_uri_system|bool'
+
+- name: Verify storage pool path directory is group-writable (libvirt system uri)
+  become: yes
+  become_flags: 'su - -c'
+  become_method: sudo
+  fail:
+    msg: |
+      The permissions for {{ libvirt_storage_pool_path }} should be group
+      writeable by the group used by libvirt: {{ libvirt_qemu_group }}
+      Current group: {{ dir_group.stdout }}
+      Current permissions: {{ dir_perms.stdout }}
+  tags: ['storage-pool-path']
+  when:
+    - 'libvirt_uri_system|bool'
+    - (dir_group.stdout != libvirt_qemu_group) or (dir_perms.stdout[5] != 'w')
+
 - name: Check for dnsmasq configuration files
   stat:
     path: "{{ item }}"
diff --git a/scripts/bringup_guestfs.sh b/scripts/bringup_guestfs.sh
index 976d1e78ed6a..bc0176f8f5b4 100755
--- a/scripts/bringup_guestfs.sh
+++ b/scripts/bringup_guestfs.sh
@@ -271,9 +271,6 @@ if [[ "$CONFIG_LIBVIRT_URI_SYSTEM" == "y" ]]; then
 	USE_SUDO="sudo "
 fi
 
-$USE_SUDO mkdir -p $STORAGEDIR
-$USE_SUDO mkdir -p $BASE_IMAGE_DIR
-
 cmdfile=$(mktemp)
 
 if [ ! -f $BASE_IMAGE ]; then
diff --git a/scripts/guestfs.Makefile b/scripts/guestfs.Makefile
index d08e697f3cfb..e1cf25d62d04 100644
--- a/scripts/guestfs.Makefile
+++ b/scripts/guestfs.Makefile
@@ -83,7 +83,7 @@ bringup_guestfs: $(GUESTFS_BRINGUP_DEPS)
 		playbooks/bringup_guestfs.yml \
 		-e 'ansible_python_interpreter=/usr/bin/python3' \
 		--extra-vars=@./extra_vars.yaml \
-		--tags config-check,network
+		--tags config-check,network,storage-pool-path
 	$(Q)$(TOPDIR)/scripts/bringup_guestfs.sh
 	$(Q)ansible-playbook $(ANSIBLE_VERBOSE) --connection=local \
 		--inventory localhost, \
-- 
2.47.2

[PATCH 6/6] gen_nodes: ensure kdevops prefix has no dashes

[Luis Chamberlain]

Folks trying to use kdevops and testing with fstests will quickly
find out a surprise that their config is not being parsed correctly
until later.

Fix this by preventing bringup if the prefix has a dash.

We use the dash to help parallelize testing filesystem profiles and
so the host prefix goes before the filesystem name and test profile.

Signed-off-by: Luis Chamberlain <mcgrof@kernel.org>
---
 playbooks/roles/gen_nodes/tasks/main.yml | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/playbooks/roles/gen_nodes/tasks/main.yml b/playbooks/roles/gen_nodes/tasks/main.yml
index d541dcbf1f54..8c6a1f705ee2 100644
--- a/playbooks/roles/gen_nodes/tasks/main.yml
+++ b/playbooks/roles/gen_nodes/tasks/main.yml
@@ -18,6 +18,11 @@
   command: "id -g -n"
   register: my_group
 
+- name: Fail if kdevops_host_prefix contains a dash
+  fail:
+    msg: "Invalid kdevops_host_prefix '{{ kdevops_host_prefix }}'. The prefix cannot contain a dash ('-')."
+  when: kdevops_host_prefix is search("-")
+
 - name: Create guestfs directory
   ansible.builtin.file:
     path: "{{ guestfs_path }}"
-- 
2.47.2

Re: [PATCH 6/6] gen_nodes: ensure kdevops prefix has no dashes

[Scott Mayhew]

On Sat, 29 Mar 2025, Luis Chamberlain wrote:

> Folks trying to use kdevops and testing with fstests will quickly
> find out a surprise that their config is not being parsed correctly
> until later.
> 
> Fix this by preventing bringup if the prefix has a dash.
> 
> We use the dash to help parallelize testing filesystem profiles and
> so the host prefix goes before the filesystem name and test profile.

Where does that happen?  All of my kdevops setups use multiple dashes in
the host prefix (e.g. "kdevops-nfs-fstests-rhel-10-nightly"), so this
breaks my setup, and I'd imagine it breaks Chuck's too.

-Scott
> 
> Signed-off-by: Luis Chamberlain <mcgrof@kernel.org>
> ---
>  playbooks/roles/gen_nodes/tasks/main.yml | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/playbooks/roles/gen_nodes/tasks/main.yml b/playbooks/roles/gen_nodes/tasks/main.yml
> index d541dcbf1f54..8c6a1f705ee2 100644
> --- a/playbooks/roles/gen_nodes/tasks/main.yml
> +++ b/playbooks/roles/gen_nodes/tasks/main.yml
> @@ -18,6 +18,11 @@
>    command: "id -g -n"
>    register: my_group
>  
> +- name: Fail if kdevops_host_prefix contains a dash
> +  fail:
> +    msg: "Invalid kdevops_host_prefix '{{ kdevops_host_prefix }}'. The prefix cannot contain a dash ('-')."
> +  when: kdevops_host_prefix is search("-")
> +
>  - name: Create guestfs directory
>    ansible.builtin.file:
>      path: "{{ guestfs_path }}"
> -- 
> 2.47.2
> 
> 

Re: [PATCH 6/6] gen_nodes: ensure kdevops prefix has no dashes

[Luis Chamberlain]

On Mon, Mar 31, 2025 at 01:35:41PM -0400, Scott Mayhew wrote:
> On Sat, 29 Mar 2025, Luis Chamberlain wrote:
> 
> > Folks trying to use kdevops and testing with fstests will quickly
> > find out a surprise that their config is not being parsed correctly
> > until later.
> > 
> > Fix this by preventing bringup if the prefix has a dash.
> > 
> > We use the dash to help parallelize testing filesystem profiles and
> > so the host prefix goes before the filesystem name and test profile.
> 
> Where does that happen?  All of my kdevops setups use multiple dashes in
> the host prefix (e.g. "kdevops-nfs-fstests-rhel-10-nightly"), so this
> breaks my setup, and I'd imagine it breaks Chuck's too.

Oh crap, sorry, does the NFS use of fstset not rely on the hostname to
infer the target test section? If not then how about this:

From a60443671181d889866265899ba8d1bc1d453d67 Mon Sep 17 00:00:00 2001
From: Luis Chamberlain <mcgrof@kernel.org>
Date: Mon, 31 Mar 2025 11:28:14 -0700
Subject: [PATCH] gen_nodes: make prefix check only for fstests

The oscheck script relies on the prefix heuristic to determine
the target filesystem test section, but not all users of fstests
use this. NFS uses its own heuristics. Fix this by localizing the
prefix check for fstests dedicated tests only.

Fixes: 2b61c9992d9ee9 ("gen_nodes: ensure kdevops prefix has no dashes")
Reported-by: Scott Mayhew <smayhew@redhat.com>
Signed-off-by: Luis Chamberlain <mcgrof@kernel.org>
---
 kconfigs/workflows/Kconfig                  | 1 +
 playbooks/roles/gen_nodes/defaults/main.yml | 1 +
 playbooks/roles/gen_nodes/tasks/main.yml    | 4 +++-
 3 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/kconfigs/workflows/Kconfig b/kconfigs/workflows/Kconfig
index 4f969c60cab8..3075cbc13a82 100644
--- a/kconfigs/workflows/Kconfig
+++ b/kconfigs/workflows/Kconfig
@@ -106,6 +106,7 @@ choice
 config KDEVOPS_WORKFLOW_DEDICATE_FSTESTS
 	bool "fstests"
 	select KDEVOPS_WORKFLOW_ENABLE_FSTESTS
+	output yaml
 	help
 	  This will dedicate your configuration only to fstests.
 
diff --git a/playbooks/roles/gen_nodes/defaults/main.yml b/playbooks/roles/gen_nodes/defaults/main.yml
index 8ff9b87993a7..dff556c3b716 100644
--- a/playbooks/roles/gen_nodes/defaults/main.yml
+++ b/playbooks/roles/gen_nodes/defaults/main.yml
@@ -83,6 +83,7 @@ libvirt_largeio_base_size: 10240
 libvirt_largeio_pow_limit: 12
 
 kdevops_workflows_dedicated_workflow: False
+kdevops_workflow_dedicate_fstests: False
 kdevops_workflow_enable_fstests: False
 kdevops_workflow_enable_blktests: False
 
diff --git a/playbooks/roles/gen_nodes/tasks/main.yml b/playbooks/roles/gen_nodes/tasks/main.yml
index 8c6a1f705ee2..f14f5879dc58 100644
--- a/playbooks/roles/gen_nodes/tasks/main.yml
+++ b/playbooks/roles/gen_nodes/tasks/main.yml
@@ -21,7 +21,9 @@
 - name: Fail if kdevops_host_prefix contains a dash
   fail:
     msg: "Invalid kdevops_host_prefix '{{ kdevops_host_prefix }}'. The prefix cannot contain a dash ('-')."
-  when: kdevops_host_prefix is search("-")
+  when:
+    - kdevops_workflow_dedicate_fstests|bool
+    - kdevops_host_prefix is search("-")
 
 - name: Create guestfs directory
   ansible.builtin.file:
-- 
2.47.2

Re: [PATCH 6/6] gen_nodes: ensure kdevops prefix has no dashes

[Scott Mayhew]

On Mon, 31 Mar 2025, Luis Chamberlain wrote:

> On Mon, Mar 31, 2025 at 01:35:41PM -0400, Scott Mayhew wrote:
> > On Sat, 29 Mar 2025, Luis Chamberlain wrote:
> > 
> > > Folks trying to use kdevops and testing with fstests will quickly
> > > find out a surprise that their config is not being parsed correctly
> > > until later.
> > > 
> > > Fix this by preventing bringup if the prefix has a dash.
> > > 
> > > We use the dash to help parallelize testing filesystem profiles and
> > > so the host prefix goes before the filesystem name and test profile.
> > 
> > Where does that happen?  All of my kdevops setups use multiple dashes in
> > the host prefix (e.g. "kdevops-nfs-fstests-rhel-10-nightly"), so this
> > breaks my setup, and I'd imagine it breaks Chuck's too.
> 
> Oh crap, sorry, does the NFS use of fstset not rely on the hostname to
> infer the target test section? If not then how about this:

No, it still maps the hostname to the test section, and unless I'm
missing something there's no special handling.  I use the same naming
scheme for SMB as well, although I don't have nearly as many SMB workflows
running as NFS.

Unfortunately I'm using dedicated workflows for all my stuff so this
patch won't work.  Maybe just add a KDEVOPS_HOSTS_PREFIX_ALLOW_DASHES
config option to allow this check to be bypassed?

> 
> From a60443671181d889866265899ba8d1bc1d453d67 Mon Sep 17 00:00:00 2001
> From: Luis Chamberlain <mcgrof@kernel.org>
> Date: Mon, 31 Mar 2025 11:28:14 -0700
> Subject: [PATCH] gen_nodes: make prefix check only for fstests
> 
> The oscheck script relies on the prefix heuristic to determine
> the target filesystem test section, but not all users of fstests
> use this. NFS uses its own heuristics. Fix this by localizing the
> prefix check for fstests dedicated tests only.
> 
> Fixes: 2b61c9992d9ee9 ("gen_nodes: ensure kdevops prefix has no dashes")
> Reported-by: Scott Mayhew <smayhew@redhat.com>
> Signed-off-by: Luis Chamberlain <mcgrof@kernel.org>
> ---
>  kconfigs/workflows/Kconfig                  | 1 +
>  playbooks/roles/gen_nodes/defaults/main.yml | 1 +
>  playbooks/roles/gen_nodes/tasks/main.yml    | 4 +++-
>  3 files changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/kconfigs/workflows/Kconfig b/kconfigs/workflows/Kconfig
> index 4f969c60cab8..3075cbc13a82 100644
> --- a/kconfigs/workflows/Kconfig
> +++ b/kconfigs/workflows/Kconfig
> @@ -106,6 +106,7 @@ choice
>  config KDEVOPS_WORKFLOW_DEDICATE_FSTESTS
>  	bool "fstests"
>  	select KDEVOPS_WORKFLOW_ENABLE_FSTESTS
> +	output yaml
>  	help
>  	  This will dedicate your configuration only to fstests.
>  
> diff --git a/playbooks/roles/gen_nodes/defaults/main.yml b/playbooks/roles/gen_nodes/defaults/main.yml
> index 8ff9b87993a7..dff556c3b716 100644
> --- a/playbooks/roles/gen_nodes/defaults/main.yml
> +++ b/playbooks/roles/gen_nodes/defaults/main.yml
> @@ -83,6 +83,7 @@ libvirt_largeio_base_size: 10240
>  libvirt_largeio_pow_limit: 12
>  
>  kdevops_workflows_dedicated_workflow: False
> +kdevops_workflow_dedicate_fstests: False
>  kdevops_workflow_enable_fstests: False
>  kdevops_workflow_enable_blktests: False
>  
> diff --git a/playbooks/roles/gen_nodes/tasks/main.yml b/playbooks/roles/gen_nodes/tasks/main.yml
> index 8c6a1f705ee2..f14f5879dc58 100644
> --- a/playbooks/roles/gen_nodes/tasks/main.yml
> +++ b/playbooks/roles/gen_nodes/tasks/main.yml
> @@ -21,7 +21,9 @@
>  - name: Fail if kdevops_host_prefix contains a dash
>    fail:
>      msg: "Invalid kdevops_host_prefix '{{ kdevops_host_prefix }}'. The prefix cannot contain a dash ('-')."
> -  when: kdevops_host_prefix is search("-")
> +  when:
> +    - kdevops_workflow_dedicate_fstests|bool
> +    - kdevops_host_prefix is search("-")
>  
>  - name: Create guestfs directory
>    ansible.builtin.file:
> -- 
> 2.47.2
> 

Re: [PATCH 6/6] gen_nodes: ensure kdevops prefix has no dashes

[Luis Chamberlain]

On Mon, Mar 31, 2025 at 03:14:28PM -0400, Scott Mayhew wrote:
> On Mon, 31 Mar 2025, Luis Chamberlain wrote:
> 
> > On Mon, Mar 31, 2025 at 01:35:41PM -0400, Scott Mayhew wrote:
> > > On Sat, 29 Mar 2025, Luis Chamberlain wrote:
> > > 
> > > > Folks trying to use kdevops and testing with fstests will quickly
> > > > find out a surprise that their config is not being parsed correctly
> > > > until later.
> > > > 
> > > > Fix this by preventing bringup if the prefix has a dash.
> > > > 
> > > > We use the dash to help parallelize testing filesystem profiles and
> > > > so the host prefix goes before the filesystem name and test profile.
> > > 
> > > Where does that happen?  All of my kdevops setups use multiple dashes in
> > > the host prefix (e.g. "kdevops-nfs-fstests-rhel-10-nightly"), so this
> > > breaks my setup, and I'd imagine it breaks Chuck's too.
> > 
> > Oh crap, sorry, does the NFS use of fstset not rely on the hostname to
> > infer the target test section? If not then how about this:
> 
> No, it still maps the hostname to the test section, and unless I'm
> missing something there's no special handling.  I use the same naming
> scheme for SMB as well, although I don't have nearly as many SMB workflows
> running as NFS.
> 
> Unfortunately I'm using dedicated workflows for all my stuff so this
> patch won't work.  Maybe just add a KDEVOPS_HOSTS_PREFIX_ALLOW_DASHES
> config option to allow this check to be bypassed?

Yeah sure we can add KDEVOPS_HOSTS_PREFIX_ALLOW_DASHES, at least now
we have a TODO item to review if in fact prefixes with dashes may
break some existing heuristics for nfs/smb workoads.

How is the configuration for fstests inferred for these workloads then?
The exisitng 'make fstests-baseline' will use the hostname and extract
the first part up to to the first dash, and then replace remaining
"-" with "_", and we use that to infer what test section we will use
from the /var/lib/xfstests/config/*.config file used.

So for example, my extrafluff-ext4-4k host will use ext4_4k test section
for its test when run. How is the test section inferred for NFS?

  Luis

Re: [PATCH 6/6] gen_nodes: ensure kdevops prefix has no dashes

[Scott Mayhew]

On Mon, 31 Mar 2025, Luis Chamberlain wrote:

> On Mon, Mar 31, 2025 at 03:14:28PM -0400, Scott Mayhew wrote:
> > On Mon, 31 Mar 2025, Luis Chamberlain wrote:
> > 
> > > On Mon, Mar 31, 2025 at 01:35:41PM -0400, Scott Mayhew wrote:
> > > > On Sat, 29 Mar 2025, Luis Chamberlain wrote:
> > > > 
> > > > > Folks trying to use kdevops and testing with fstests will quickly
> > > > > find out a surprise that their config is not being parsed correctly
> > > > > until later.
> > > > > 
> > > > > Fix this by preventing bringup if the prefix has a dash.
> > > > > 
> > > > > We use the dash to help parallelize testing filesystem profiles and
> > > > > so the host prefix goes before the filesystem name and test profile.
> > > > 
> > > > Where does that happen?  All of my kdevops setups use multiple dashes in
> > > > the host prefix (e.g. "kdevops-nfs-fstests-rhel-10-nightly"), so this
> > > > breaks my setup, and I'd imagine it breaks Chuck's too.
> > > 
> > > Oh crap, sorry, does the NFS use of fstset not rely on the hostname to
> > > infer the target test section? If not then how about this:
> > 
> > No, it still maps the hostname to the test section, and unless I'm
> > missing something there's no special handling.  I use the same naming
> > scheme for SMB as well, although I don't have nearly as many SMB workflows
> > running as NFS.
> > 
> > Unfortunately I'm using dedicated workflows for all my stuff so this
> > patch won't work.  Maybe just add a KDEVOPS_HOSTS_PREFIX_ALLOW_DASHES
> > config option to allow this check to be bypassed?
> 
> Yeah sure we can add KDEVOPS_HOSTS_PREFIX_ALLOW_DASHES, at least now
> we have a TODO item to review if in fact prefixes with dashes may
> break some existing heuristics for nfs/smb workoads.
> 
> How is the configuration for fstests inferred for these workloads then?
> The exisitng 'make fstests-baseline' will use the hostname and extract
> the first part up to to the first dash, and then replace remaining

Maybe we're not looking at the same stuff.

Where are you seeing "the first part up to the first dash"?  I'm looking
at this bit which starts on line 1241 in playbooks/roles/fstests/tasks/main.yml:

---8<---
- name: Run fstests using ./oscheck.sh --print-start --journal {{ fstests_journal }} --print-done --test-section {{ fstests_section }} {{ oscheck_extra_args }} {{ all_limit_tests }}
  vars:
    fstests_section: "{{ ansible_host | regex_replace(kdevops_host_prefix + '-') | regex_replace('-dev') | regex_replace('-', '_') }}"
---8<---

I see that as 
1. strip off the entire host prefix *and* the first dash
2. strip off the '-dev' suffix
3. replace the remaining dashes with underscores

If I boil that down to a simple test, that appears to be exactly what it
does:

$ cat test.yml 
---
- name: Just a test
  hosts: all
  
  tasks:
    - name: Import extra_vars.yaml
      include_vars: extra_vars.yaml

    - name: Just a task
      vars:
        fstests_section: "{{ ansible_host | regex_replace(kdevops_host_prefix + '-') | regex_replace('-dev') | regex_replace('-', '_') }}"
      ansible.builtin.debug:
        var: fstests_section

$ ansible-playbook -i hosts test.yml
---8<---
TASK [Just a task] ***************************************************************************************************************************************************************************************************************************
ok: [kdevops-nfs-fstests-rhel-10-nightly-nfs-tls] => {
    "fstests_section": "nfs_tls"
}
ok: [kdevops-nfs-fstests-rhel-10-nightly-nfs-v42] => {
    "fstests_section": "nfs_v42"
}
ok: [kdevops-nfs-fstests-rhel-10-nightly-nfs-v40] => {
    "fstests_section": "nfs_v40"
}
ok: [kdevops-nfs-fstests-rhel-10-nightly-nfs-v3] => {
    "fstests_section": "nfs_v3"
}
ok: [kdevops-nfs-fstests-rhel-10-nightly-nfsd] => {
    "fstests_section": "nfsd"
}
---8<---

Those match the sections in playbooks/roles/fstests/templates/nfs/nfs.config

-Scott

> "-" with "_", and we use that to infer what test section we will use
> from the /var/lib/xfstests/config/*.config file used.
> 
> So for example, my extrafluff-ext4-4k host will use ext4_4k test section
> for its test when run. How is the test section inferred for NFS?
> 
>   Luis
> 

Re: [PATCH 6/6] gen_nodes: ensure kdevops prefix has no dashes

[Luis Chamberlain]

On Mon, Mar 31, 2025 at 04:49:32PM -0400, Scott Mayhew wrote:
> On Mon, 31 Mar 2025, Luis Chamberlain wrote:
> 
> > On Mon, Mar 31, 2025 at 03:14:28PM -0400, Scott Mayhew wrote:
> > > On Mon, 31 Mar 2025, Luis Chamberlain wrote:
> > > 
> > > > On Mon, Mar 31, 2025 at 01:35:41PM -0400, Scott Mayhew wrote:
> > > > > On Sat, 29 Mar 2025, Luis Chamberlain wrote:
> > > > > 
> > > > > > Folks trying to use kdevops and testing with fstests will quickly
> > > > > > find out a surprise that their config is not being parsed correctly
> > > > > > until later.
> > > > > > 
> > > > > > Fix this by preventing bringup if the prefix has a dash.
> > > > > > 
> > > > > > We use the dash to help parallelize testing filesystem profiles and
> > > > > > so the host prefix goes before the filesystem name and test profile.
> > > > > 
> > > > > Where does that happen?  All of my kdevops setups use multiple dashes in
> > > > > the host prefix (e.g. "kdevops-nfs-fstests-rhel-10-nightly"), so this
> > > > > breaks my setup, and I'd imagine it breaks Chuck's too.
> > > > 
> > > > Oh crap, sorry, does the NFS use of fstset not rely on the hostname to
> > > > infer the target test section? If not then how about this:
> > > 
> > > No, it still maps the hostname to the test section, and unless I'm
> > > missing something there's no special handling.  I use the same naming
> > > scheme for SMB as well, although I don't have nearly as many SMB workflows
> > > running as NFS.
> > > 
> > > Unfortunately I'm using dedicated workflows for all my stuff so this
> > > patch won't work.  Maybe just add a KDEVOPS_HOSTS_PREFIX_ALLOW_DASHES
> > > config option to allow this check to be bypassed?
> > 
> > Yeah sure we can add KDEVOPS_HOSTS_PREFIX_ALLOW_DASHES, at least now
> > we have a TODO item to review if in fact prefixes with dashes may
> > break some existing heuristics for nfs/smb workoads.
> > 
> > How is the configuration for fstests inferred for these workloads then?
> > The exisitng 'make fstests-baseline' will use the hostname and extract
> > the first part up to to the first dash, and then replace remaining
> 
> Maybe we're not looking at the same stuff.
> 
> Where are you seeing "the first part up to the first dash"?  I'm looking
> at this bit which starts on line 1241 in playbooks/roles/fstests/tasks/main.yml:
> 
> ---8<---
> - name: Run fstests using ./oscheck.sh --print-start --journal {{ fstests_journal }} --print-done --test-section {{ fstests_section }} {{ oscheck_extra_args }} {{ all_limit_tests }}
>   vars:
>     fstests_section: "{{ ansible_host | regex_replace(kdevops_host_prefix + '-') | regex_replace('-dev') | regex_replace('-', '_') }}"
> ---8<---
> 
> I see that as 
> 1. strip off the entire host prefix *and* the first dash
> 2. strip off the '-dev' suffix
> 3. replace the remaining dashes with underscores
> 
> If I boil that down to a simple test, that appears to be exactly what it
> does:
> 
> $ cat test.yml 
> ---
> - name: Just a test
>   hosts: all
>   
>   tasks:
>     - name: Import extra_vars.yaml
>       include_vars: extra_vars.yaml
> 
>     - name: Just a task
>       vars:
>         fstests_section: "{{ ansible_host | regex_replace(kdevops_host_prefix + '-') | regex_replace('-dev') | regex_replace('-', '_') }}"
>       ansible.builtin.debug:
>         var: fstests_section
> 
> $ ansible-playbook -i hosts test.yml
> ---8<---
> TASK [Just a task] ***************************************************************************************************************************************************************************************************************************
> ok: [kdevops-nfs-fstests-rhel-10-nightly-nfs-tls] => {
>     "fstests_section": "nfs_tls"
> }
> ok: [kdevops-nfs-fstests-rhel-10-nightly-nfs-v42] => {
>     "fstests_section": "nfs_v42"
> }
> ok: [kdevops-nfs-fstests-rhel-10-nightly-nfs-v40] => {
>     "fstests_section": "nfs_v40"
> }
> ok: [kdevops-nfs-fstests-rhel-10-nightly-nfs-v3] => {
>     "fstests_section": "nfs_v3"
> }
> ok: [kdevops-nfs-fstests-rhel-10-nightly-nfsd] => {
>     "fstests_section": "nfsd"
> }
> ---8<---
> 
> Those match the sections in playbooks/roles/fstests/templates/nfs/nfs.config

Ah yeah,  *and* the first dash, I reverted that commit, sorry about that!
Pushed.

  Luis

[PATCH 0/6] debian / libvirt / devconfig fixes

[Luis Chamberlain]

Here are a few enhancements to debian guests so to ensure we remove
the unattended-upgrades package from the start so to prevent tons of
silly issues it can create.

The next set of changes are related to the confusing way in which we
define the kdevops storage pool path in one way in kconfig but use it
differently in yaml / ansible. This then phases out a few commands from
shell into ansible and adds sanity checks back, this shows how using
ansible can be more precise.

Lastly, there's one generic fix for devconfig so we ensure we don't use
dashes on the kdevops host prefix.

Luis Chamberlain (6):
  scripts/bringup_guestfs.sh: uninstall unattended-upgrades on debian
    guests
  devconfig: ensure unattended-upgrades is not installed on debian
  libvirt: use consistent pool path variables and use optional yaml
    output
  Kconfig: adopt output yaml for KDEVOPS_FIRST_RUN
  guestfs: add ansible group permisison check on libvirt system uri
  gen_nodes: ensure kdevops prefix has no dashes

 Kconfig                                       |  1 +
 kconfigs/Kconfig.guestfs                      | 15 +++++
 kconfigs/Kconfig.libvirt                      | 11 +++-
 .../roles/bringup_guestfs/tasks/main.yml      | 59 +++++++++++++++++++
 .../tasks/install-deps/debian/main.yml        | 22 +++++++
 playbooks/roles/gen_nodes/tasks/main.yml      |  5 ++
 scripts/bringup_guestfs.sh                    |  6 +-
 scripts/bringup_vagrant.sh                    |  2 +-
 scripts/destroy_guestfs.sh                    |  2 +-
 scripts/destroy_vagrant.sh                    |  2 +-
 scripts/gen-nodes.Makefile                    |  6 --
 scripts/guestfs.Makefile                      |  9 +--
 scripts/prune_stale_vagrant.sh                |  2 +-
 scripts/vagrant.Makefile                      |  5 --
 14 files changed, 120 insertions(+), 27 deletions(-)

-- 
2.47.2

[PATCH 6/6] gen_nodes: ensure kdevops prefix has no dashes

[Luis Chamberlain]

Folks trying to use kdevops and testing with fstests will quickly
find out a surprise that their config is not being parsed correctly
until later.

Fix this by preventing bringup if the prefix has a dash.

We use the dash to help parallelize testing filesystem profiles and
so the host prefix goes before the filesystem name and test profile.

Signed-off-by: Luis Chamberlain <mcgrof@kernel.org>
---
 playbooks/roles/gen_nodes/tasks/main.yml | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/playbooks/roles/gen_nodes/tasks/main.yml b/playbooks/roles/gen_nodes/tasks/main.yml
index d541dcbf1f54..8c6a1f705ee2 100644
--- a/playbooks/roles/gen_nodes/tasks/main.yml
+++ b/playbooks/roles/gen_nodes/tasks/main.yml
@@ -18,6 +18,11 @@
   command: "id -g -n"
   register: my_group
 
+- name: Fail if kdevops_host_prefix contains a dash
+  fail:
+    msg: "Invalid kdevops_host_prefix '{{ kdevops_host_prefix }}'. The prefix cannot contain a dash ('-')."
+  when: kdevops_host_prefix is search("-")
+
 - name: Create guestfs directory
   ansible.builtin.file:
     path: "{{ guestfs_path }}"
-- 
2.47.2