fix(satellite): record only hex generations

pkg/drbd/dumpmd.go

@@ -185,11 +185,11 @@ func appendUUIDStatements(out []string, text string) []string {
 	return out
 }
 
-// uuidStatement answers the UUID a statement is, when it is a bare non-zero
-// hex UUID.
+// uuidStatement answers the UUID a statement is, when it is a bare generation
+// UUID (IsGenerationUUID).
 func uuidStatement(statement string) (string, bool) {
 	digits, ok := strings.CutPrefix(strings.TrimSpace(statement), "0x")
-	if !ok || !isHex(digits) || strings.Trim(digits, "0") == "" {
+	if !ok || !IsGenerationUUID(digits) {
 		return "", false
 	}
 
@@ -208,6 +208,18 @@ func firstStatement(text string) (string, bool) {
 	return "", false
 }
 
+// generationUUIDDigits is how many hex digits drbdsetup and drbdmeta print a
+// generation UUID with: a 64-bit value as %016llX, zero-padded.
+const generationUUIDDigits = 16
+
+// IsGenerationUUID says whether s is a generation UUID as drbdsetup and
+// drbdmeta print it without the 0x prefix: 16 hex digits, not all zero.
+// Zero is DRBD's "no generation", which identifies nothing, and a value of
+// another length is not one the tools print, so it is damage, not a GI.
+func IsGenerationUUID(s string) bool {
+	return len(s) == generationUUIDDigits && isHex(s) && strings.Trim(s, "0") != ""
+}
+
 func isHex(digits string) bool {
 	if digits == "" {
 		return false

pkg/drbd/dumpmd_test.go

@@ -144,6 +144,9 @@ func TestParseMetadataUUIDsRefusesWhatItCannotRead(t *testing.T) {
 		"empty":              "",
 		"current without 0x": "version \"v09\";\ncurrent-uuid 6E3A5D0B2C1F4A96;\n",
 		"zero current":       "version \"v09\";\ncurrent-uuid 0x0000000000000000;\n",
+		"short current":      "version \"v09\";\ncurrent-uuid 0xABCD;\n",
+		"long current":       "version \"v09\";\ncurrent-uuid 0x16E3A5D0B2C1F4A96;\n",
+		"non-hex current":    "version \"v09\";\ncurrent-uuid 0x6E3A5D0B2C1F4A9G;\n",
 		"unreadable current next to a bitmap UUID": "version \"v09\";\ncurrent-uuid 6E3A5D0B2C1F4A96;\n" +
 			"peer[1] {\n    bitmap-uuid 0x1234567890ABCDEF;\n}\n",
 		"v08 zero current next to a bitmap UUID": "version \"v08\";\nuuid {\n" +

pkg/satellite/createmd_test.go

@@ -994,6 +994,43 @@ func mustRecordedGIs(t *testing.T, rec *Reconciler, rdName string, vol int32) []
 	return got
 }
 
+// Only a GI is recorded: a zero one is DRBD's "none yet", and a value DRBD
+// would not print, not hex or not 16 digits, would make the whole record read
+// as damaged later.
+func TestOnlyGenerationUUIDsAreRecorded(t *testing.T) {
+	rec, _, _, _, _ := uncleanFixture(t) //nolint:dogsled // only the reconciler is needed
+
+	for _, generation := range []string{
+		"0000000000000000", "not-a-gi", "0x" + uncleanReportedGI, "6E3A 5D0B",
+		"ABCD", "1" + uncleanReportedGI, uncleanReportedGI[1:], "6E3A5D0B2C1F4A9G",
+	} {
+		rec.RecordObservedGI("pvc-unclean", 0, generation)
+	}
+
+	if got := mustRecordedGIs(t, rec, "pvc-unclean", 0); len(got) != 0 {
+		t.Fatalf("recorded %v, want nothing", got)
+	}
+
+	rec.RecordObservedGI("pvc-unclean", 0, strings.ToLower(uncleanReportedGI))
+
+	if got := mustRecordedGIs(t, rec, "pvc-unclean", 0); !slices.Equal(got, []string{uncleanReportedGI}) {
+		t.Fatalf("recorded %v, want [%s]", got, uncleanReportedGI)
+	}
+}
+
+// DRBD zero-pads a GI to 16 digits, so a leading zero is part of one.
+func TestAZeroPaddedGenerationIsRecorded(t *testing.T) {
+	rec, _, _, _, _ := uncleanFixture(t) //nolint:dogsled // only the reconciler is needed
+
+	const padded = "006E3A5D0B2C1F4A"
+
+	rec.RecordObservedGI("pvc-unclean", 0, padded)
+
+	if got := mustRecordedGIs(t, rec, "pvc-unclean", 0); !slices.Equal(got, []string{padded}) {
+		t.Fatalf("recorded %v, want [%s]", got, padded)
+	}
+}
+
 // A record that cannot be read is not "no record": the repair fails on it
 // with no create-md, on both paths, and an observation does not write over
 // it with its one GI. A damaged line counts as unreadable: skipping it
@@ -1004,6 +1041,11 @@ func TestUnreadableGIRecordFailsClosed(t *testing.T) {
 			"directory":           "",
 			"line with no GI":     "0\n",
 			"volume not a number": "zero " + uncleanReportedGI + "\n",
+			"GI not hex":          "0 not-a-gi\n",
+			"zero GI":             "0 0000000000000000\n",
+			"GI too short":        "0 ABCD\n",
+			"GI too long":         "0 1" + uncleanReportedGI + "\n",
+			"GI of 16 not hex":    "0 6E3A5D0B2C1F4A9G\n",
 		} {
 			t.Run(path+"/"+damage, func(t *testing.T) {
 				rec, dr, devices, mdMarkerPath, fx := uncleanFixture(t)

pkg/satellite/gi_history.go

@@ -30,6 +30,7 @@ import (
 	"strconv"
 	"strings"
 
+	"github.com/cozystack/blockstor/pkg/drbd"
 	intent "github.com/cozystack/blockstor/pkg/satellite/intent"
 )
 
@@ -54,6 +55,19 @@ func (r *Reconciler) RecordObservedGI(rdName string, vol int32, generation strin
 		return
 	}
 
+	// A zero GI is DRBD's "none yet" and identifies nothing. Anything else
+	// that is not a GI as DRBD prints it is not one at all, and a record of
+	// it would later read as damaged and stop the repair for the whole
+	// resource.
+	if !drbd.IsGenerationUUID(generation) {
+		if strings.Trim(generation, "0") != "" {
+			slog.Default().Warn("not recording a value that is not a generation UUID", "resource", rdName,
+				"volume", vol, "generation", generation)
+		}
+
+		return
+	}
+
 	generation = strings.ToUpper(generation)
 	key := rdName + "/" + strconv.Itoa(int(vol))
 
@@ -189,7 +203,7 @@ func (r *Reconciler) readGIHistory(rdName string) (map[int32][]string, error) {
 		volField, generation, ok := strings.Cut(line, " ")
 		vol, convErr := strconv.ParseInt(volField, 10, 32)
 
-		if !ok || generation == "" || convErr != nil {
+		if !ok || !drbd.IsGenerationUUID(generation) || convErr != nil {
 			return nil, fmt.Errorf("the generation record of %s: %w: %q", rdName, errDamagedGIRecord, line)
 		}