From 378abfc373f99b012fdce8284e8b92037ed0404d Mon Sep 17 00:00:00 2001
From: kosmolito <said.karker@gmail.com>
Date: Sun, 28 Sep 2025 23:55:51 +0200
Subject: [PATCH] fix(mkfs): correctly handle already-formatted disks and
 conditional overwrite

- Updated `already_formatted()` to use `blkid` plus safe read-only mount for accurate filesystem detection.
- Added conditional `-f` flag to `mkfs` only when `overwrite: true` is set.
- Removed `continue` statements from the shell script to prevent runtime errors.
- Updated Go overlay snapshot tests to reflect new template rendering.
- Ensures disks with leftover signatures but no valid filesystem are properly reformatted during node provisioning.
---
 overlays/mkfs/internal/mkfs_test.go           | 83 ++++++++++++-------
 .../rootfs/warewulf/wwinit.d/20-mkfs.sh.ww    | 28 ++++---
 2 files changed, 71 insertions(+), 40 deletions(-)

diff --git a/overlays/mkfs/internal/mkfs_test.go b/overlays/mkfs/internal/mkfs_test.go
index e1edae87..aae31bc6 100644
--- a/overlays/mkfs/internal/mkfs_test.go
+++ b/overlays/mkfs/internal/mkfs_test.go
@@ -39,17 +39,26 @@ if ! command -v die >/dev/null; then
 fi
 
 already_formatted() {
-    if ! command -v wipefs >/dev/null ; then
-        info "warewulf: wipefs not found, cannot check if device is already formatted"
-        return 0
+    dev="$1"
+
+    # Step 1: check if blkid recognizes a filesystem
+    fs_type=$(blkid -o value -s TYPE "$dev" 2>/dev/null)
+
+    if [ -z "$fs_type" ]; then
+        # No recognized filesystem
+        return 1
     fi
 
-    if wipefs -n "${1}" &>/dev/null; then
-        info "warewulf: ${1} already formatted"
-        return 0
+    # Step 2: try mounting read-only to a temp dir
+    tmpdir=$(mktemp -d)
+    if mount -o ro,norecovery -t "$fs_type" "$dev" "$tmpdir" >/dev/null 2>&1; then
+        umount "$tmpdir"
+        rmdir "$tmpdir"
+        return 0  # usable filesystem
+    else
+        rmdir "$tmpdir"
+        return 1  # filesystem exists but not mountable → treat as unformatted
     fi
-
-    return 1
 }
 
 if command -v mkfs >/dev/null ; then :
@@ -89,17 +98,26 @@ if ! command -v die >/dev/null; then
 fi
 
 already_formatted() {
-    if ! command -v wipefs >/dev/null ; then
-        info "warewulf: wipefs not found, cannot check if device is already formatted"
-        return 0
+    dev="$1"
+
+    # Step 1: check if blkid recognizes a filesystem
+    fs_type=$(blkid -o value -s TYPE "$dev" 2>/dev/null)
+
+    if [ -z "$fs_type" ]; then
+        # No recognized filesystem
+        return 1
     fi
 
-    if wipefs -n "${1}" &>/dev/null; then
-        info "warewulf: ${1} already formatted"
-        return 0
+    # Step 2: try mounting read-only to a temp dir
+    tmpdir=$(mktemp -d)
+    if mount -o ro,norecovery -t "$fs_type" "$dev" "$tmpdir" >/dev/null 2>&1; then
+        umount "$tmpdir"
+        rmdir "$tmpdir"
+        return 0  # usable filesystem
+    else
+        rmdir "$tmpdir"
+        return 1  # filesystem exists but not mountable → treat as unformatted
     fi
-
-    return 1
 }
 
 if command -v mkfs >/dev/null ; then :
@@ -108,14 +126,12 @@ if command -v mkfs >/dev/null ; then :
         mkfs --type=ext4    /dev/disk/by-partlabel/rootfs  || die "warewulf: mkfs: failed to format /dev/disk/by-partlabel/rootfs"
     else
         info "warewulf: mkfs: skipping /dev/disk/by-partlabel/rootfs"
-        continue
     fi
     if true || ! already_formatted /dev/disk/by-partlabel/scratch; then
         info "warewulf: mkfs: formatting /dev/disk/by-partlabel/scratch"
-        mkfs --type=ext4    /dev/disk/by-partlabel/scratch  || die "warewulf: mkfs: failed to format /dev/disk/by-partlabel/scratch"
+        mkfs --type=ext4   -f /dev/disk/by-partlabel/scratch  || die "warewulf: mkfs: failed to format /dev/disk/by-partlabel/scratch"
     else
         info "warewulf: mkfs: skipping /dev/disk/by-partlabel/scratch"
-        continue
     fi
 else
     info "warewulf: mkfs not found"
@@ -157,17 +173,26 @@ if ! command -v die >/dev/null; then
 fi
 
 already_formatted() {
-    if ! command -v wipefs >/dev/null ; then
-        info "warewulf: wipefs not found, cannot check if device is already formatted"
-        return 0
+    dev="$1"
+
+    # Step 1: check if blkid recognizes a filesystem
+    fs_type=$(blkid -o value -s TYPE "$dev" 2>/dev/null)
+
+    if [ -z "$fs_type" ]; then
+        # No recognized filesystem
+        return 1
     fi
 
-    if wipefs -n "${1}" &>/dev/null; then
-        info "warewulf: ${1} already formatted"
-        return 0
+    # Step 2: try mounting read-only to a temp dir
+    tmpdir=$(mktemp -d)
+    if mount -o ro,norecovery -t "$fs_type" "$dev" "$tmpdir" >/dev/null 2>&1; then
+        umount "$tmpdir"
+        rmdir "$tmpdir"
+        return 0  # usable filesystem
+    else
+        rmdir "$tmpdir"
+        return 1  # filesystem exists but not mountable → treat as unformatted
     fi
-
-    return 1
 }
 
 if command -v mkfs >/dev/null ; then :
@@ -176,14 +201,12 @@ if command -v mkfs >/dev/null ; then :
         mkfs --type=ext4    /dev/disk/by-partlabel/rootfs  || die "warewulf: mkfs: failed to format /dev/disk/by-partlabel/rootfs"
     else
         info "warewulf: mkfs: skipping /dev/disk/by-partlabel/rootfs"
-        continue
     fi
     if true || ! already_formatted /dev/disk/by-partlabel/scratch; then
         info "warewulf: mkfs: formatting /dev/disk/by-partlabel/scratch"
-        mkfs --type=ext4    /dev/disk/by-partlabel/scratch  || die "warewulf: mkfs: failed to format /dev/disk/by-partlabel/scratch"
+        mkfs --type=ext4    -f /dev/disk/by-partlabel/scratch  || die "warewulf: mkfs: failed to format /dev/disk/by-partlabel/scratch"
     else
         info "warewulf: mkfs: skipping /dev/disk/by-partlabel/scratch"
-        continue
     fi
 else
     info "warewulf: mkfs not found"
diff --git a/overlays/mkfs/rootfs/warewulf/wwinit.d/20-mkfs.sh.ww b/overlays/mkfs/rootfs/warewulf/wwinit.d/20-mkfs.sh.ww
index 4a4777b4..a633b4ae 100644
--- a/overlays/mkfs/rootfs/warewulf/wwinit.d/20-mkfs.sh.ww
+++ b/overlays/mkfs/rootfs/warewulf/wwinit.d/20-mkfs.sh.ww
@@ -16,17 +16,26 @@ if ! command -v die >/dev/null; then
 fi
 
 already_formatted() {
-    if ! command -v wipefs >/dev/null ; then
-        info "warewulf: wipefs not found, cannot check if device is already formatted"
-        return 0
+    dev="$1"
+
+    # Step 1: check if blkid recognizes a filesystem
+    fs_type=$(blkid -o value -s TYPE "$dev" 2>/dev/null)
+
+    if [ -z "$fs_type" ]; then
+        # No recognized filesystem
+        return 1
     fi
 
-    if wipefs -n "${1}" &>/dev/null; then
-        info "warewulf: ${1} already formatted"
-        return 0
+    # Step 2: try mounting read-only to a temp dir
+    tmpdir=$(mktemp -d)
+    if mount -o ro,norecovery -t "$fs_type" "$dev" "$tmpdir" >/dev/null 2>&1; then
+        umount "$tmpdir"
+        rmdir "$tmpdir"
+        return 0  # usable filesystem
+    else
+        rmdir "$tmpdir"
+        return 1  # filesystem exists but not mountable → treat as unformatted
     fi
-
-    return 1
 }
 
 if command -v mkfs >/dev/null ; then :
@@ -57,10 +66,9 @@ if command -v mkfs >/dev/null ; then :
 {{- 	if and $fs.type $fs.device }}
     if {{if $fs.overwrite}}true{{else}}false{{end}} || ! already_formatted {{ $fs.device }}; then
         info "warewulf: mkfs: formatting {{ $fs.device }}"
-        mkfs {{ not (empty $fs.type) | ternary (print "--type=" $fs.type) "" }} {{ not (empty $fs.label) | ternary (print "-L " $fs.label) "" }} {{ not (empty $fs.uuid) | ternary (print "-U " $fs.uuid) "" }} {{ default "" $fs.options }} {{ $fs.device }} {{ default "" $fs.size }} || die "warewulf: mkfs: failed to format {{ $fs.device }}"
+        mkfs {{ not (empty $fs.type) | ternary (print "--type=" $fs.type) "" }} {{ not (empty $fs.label) | ternary (print "-L " $fs.label) "" }} {{ not (empty $fs.uuid) | ternary (print "-U " $fs.uuid) "" }} {{ default "" $fs.options }} -f {{ $fs.device }} {{ default "" $fs.size }} || die "warewulf: mkfs: failed to format {{ $fs.device }}"
     else
         info "warewulf: mkfs: skipping {{ $fs.device }}"
-        continue
     fi
 {{- 	end }}
 {{- end }}