Add SBR must-gather test and fix the flaky SBRC agent-DaemonSet readiness wait

tests/sbr-operator/README.md

@@ -135,3 +135,17 @@ independently using a short-lived privileged hostPID pod per node.
 - **Environment**: Connected or disconnected
 - **Standalone**: `ginkgo --label-filter="sbr" --focus="Verify watchdog device" ./tests/sbr-operator/...`
 - **Pass criteria**: All hardware watchdog devices are character devices; nodes without hardware watchdog have softdog.ko present in the kernel module tree
+
+### 9. Verify SBR Must-Gather Collects Diagnostic Data ([OCP-88733](https://polarion.engineering.redhat.com/polarion/#/project/OSE/workitem?id=OCP-88733))
+
+Validates that `oc adm must-gather` with the RHWA image collects
+SBR-related diagnostic data: node manifests, CRD definitions, and
+MachineHealthCheck resources.
+
+- **Operators**: SBR v0.3.0, NHC (for must-gather image resolution)
+- **Cluster**: Any topology
+- **Storage**: None
+- **Environment**: Connected by default; disconnected clusters can set
+  `MUST_GATHER_IMAGE` to an accessible mirrored image
+- **Standalone**: `ginkgo --label-filter="sbr" --focus="must-gather" ./tests/sbr-operator/...`
+- **Pass criteria**: SBR deployment is Ready; must-gather completes successfully; output contains node YAMLs for all cluster nodes, all 3 SBR CRD definition files, and MachineHealthCheck data

tests/sbr-operator/internal/sbrparams/const.go

@@ -97,8 +97,20 @@ const (
 	SBRAgentDaemonSetPrefix = "sbr-agent-"
 
 	// SBRCReadyTimeout is the time allowed for the SBRC's agent DaemonSet to have all scheduled
-	// pods reach Ready before a functional test begins.
-	SBRCReadyTimeout = 7 * time.Minute
+	// pods reach Ready before a functional test begins. The first SBRC created in a suite pays a
+	// one-time cold-start cost (first CephFS PVC bind plus SBD device-init Job on a freshly
+	// provisioned ODF cluster); later SBRCs reuse the warmed CSI path and become ready in ~90s.
+	// The ceiling is sized for that worst-case cold start - a ready DaemonSet returns immediately,
+	// so healthy runs never wait this long.
+	SBRCReadyTimeout = 10 * time.Minute
+
+	// SBRCReadyDiagMaxEvents caps how many recent Warning events are included in the on-timeout
+	// readiness diagnostics dumped by waitForSBRCReady, to keep the failure message readable.
+	SBRCReadyDiagMaxEvents = 15
+
+	// SBRCReadyDiagTimeout bounds the API calls made while collecting on-timeout readiness
+	// diagnostics, so a slow or wedged apiserver cannot hang the already-failed test.
+	SBRCReadyDiagTimeout = 30 * time.Second
 
 	// WatchdogProbeLogTimeout is the deadline for reading a completed watchdog probe pod's logs
 	// via the in-cluster API.
@@ -251,6 +263,21 @@ const (
 	// AgentMetricsPort is the port on which SBR agent pods expose custom Prometheus metrics.
 	// Port 8080 is controller-runtime's built-in metrics; port 8082 is the SBR agent's own metrics.
 	AgentMetricsPort = "8082"
+
+	// MustGatherOCTimeout is the --timeout flag passed to oc adm must-gather so it cleans up gracefully.
+	MustGatherOCTimeout = 14 * time.Minute
+
+	// MustGatherContextTimeout is the outer Go context timeout, strictly greater than MustGatherOCTimeout.
+	MustGatherContextTimeout = 16 * time.Minute
+
+	// MustGatherImageRepo is the registry path for the RHWA must-gather image (without tag).
+	MustGatherImageRepo = "registry.redhat.io/workload-availability/node-healthcheck-must-gather-rhel9"
+
+	// MustGatherDefaultTag is the fallback image tag when dynamic resolution fails.
+	MustGatherDefaultTag = "v0.9.0"
+
+	// MustGatherCleanupTimeout is the deadline for best-effort cleanup of leftover must-gather namespaces.
+	MustGatherCleanupTimeout = 2 * time.Minute
 )
 
 // AgentExpectedMetricNames lists the Prometheus metric names that must be present in the agent output.

tests/sbr-operator/internal/sbrparams/sbrvars.go

@@ -35,6 +35,13 @@ var (
 		"StorageBasedRemediationTemplate",
 	}
 
+	// SBRCRDNames lists the full CRD names (plural.group) for all SBR custom resource definitions.
+	SBRCRDNames = []string{
+		"storagebasedremediationconfigs." + CRDGroup,
+		"storagebasedremediations." + CRDGroup,
+		"storagebasedremediationtemplates." + CRDGroup,
+	}
+
 	// RequiredAnnotations defines the required annotations and expected values for SBR CSV.
 	RequiredAnnotations = map[string]string{
 		"features.operators.openshift.io/tls-profiles":     "false",

tests/sbr-operator/tests/must_gather.go

@@ -0,0 +1,272 @@
+package tests
+
+import (
+	"context"
+	"fmt"
+	"io/fs"
+	"os"
+	"os/exec"
+	"path/filepath"
+	"strings"
+	"time"
+
+	. "github.com/onsi/ginkgo/v2"
+	. "github.com/onsi/gomega"
+
+	oplmV1alpha1 "github.com/rh-ecosystem-edge/eco-goinfra/pkg/schemes/olm/operators/v1alpha1"
+
+	"github.com/rh-ecosystem-edge/eco-goinfra/pkg/deployment"
+	"github.com/rh-ecosystem-edge/eco-goinfra/pkg/olm"
+	"github.com/rh-ecosystem-edge/eco-goinfra/pkg/reportxml"
+
+	"github.com/medik8s/system-tests/tests/internal/labels"
+	. "github.com/medik8s/system-tests/tests/internal/medik8sinittools"
+	"github.com/medik8s/system-tests/tests/internal/medik8sparams"
+	"github.com/medik8s/system-tests/tests/sbr-operator/internal/sbrparams"
+
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+)
+
+var _ = Describe(
+	"SBR Must-Gather Diagnostics",
+	Serial,
+	Ordered,
+	Label(labels.OperatorSBR), func() {
+		It("Verify SBR must-gather collects diagnostic data",
+			reportxml.ID("88733"),
+			Label(
+				labels.DisruptionNonDestructive,
+				labels.TierAcceptance,
+				labels.PlatformAny,
+				labels.ComponentController,
+				labels.FrequencyWeekly,
+			), func() {
+				By("Verifying SBR deployment is Ready")
+
+				sbrDeployment, err := deployment.Pull(
+					APIClient, sbrparams.OperatorDeploymentName, medik8sparams.OperatorNs)
+				Expect(err).ToNot(HaveOccurred(), "Failed to get SBR deployment")
+				Expect(sbrDeployment.IsReady(medik8sparams.DefaultTimeout)).To(BeTrue(),
+					"SBR deployment is not Ready")
+
+				By("Resolving the RHWA must-gather image")
+
+				mustGatherImage := resolveMustGatherImage()
+				Expect(mustGatherImage).To(ContainSubstring(":"),
+					"must-gather image %q should contain a tag separator", mustGatherImage)
+				GinkgoWriter.Printf("Using must-gather image: %s\n", mustGatherImage)
+
+				By("Creating artifact directory for must-gather output")
+
+				destDir := createMustGatherDestDir()
+
+				By("Capturing cluster state before must-gather for validation")
+
+				listCtx, listCancel := context.WithTimeout(context.Background(), medik8sparams.DefaultTimeout)
+				defer listCancel()
+
+				nodeList, err := APIClient.CoreV1Interface.Nodes().List(listCtx, metav1.ListOptions{})
+				Expect(err).ToNot(HaveOccurred(), "Failed to list cluster nodes")
+				Expect(nodeList.Items).ToNot(BeEmpty(), "Cluster has no nodes")
+
+				var nodeNames []string
+				for i := range nodeList.Items {
+					nodeNames = append(nodeNames, nodeList.Items[i].Name)
+				}
+
+				By("Running oc adm must-gather")
+
+				testStartTime := time.Now()
+
+				ctx, cancel := context.WithTimeout(context.Background(), sbrparams.MustGatherContextTimeout)
+				defer cancel()
+
+				DeferCleanup(func() {
+					cleanupMustGatherNamespaces(context.Background(), testStartTime)
+				})
+
+				runMustGather(ctx, mustGatherImage, destDir)
+
+				By("Collecting gathered file paths")
+
+				collectedFiles, walkErr := collectRelativePaths(destDir)
+				Expect(walkErr).ToNot(HaveOccurred(), "Failed to walk must-gather output directory")
+				Expect(collectedFiles).ToNot(BeEmpty(), "No files collected by must-gather")
+
+				if writeErr := os.WriteFile(filepath.Join(destDir, "collected-paths.txt"),
+					[]byte(strings.Join(collectedFiles, "\n")+"\n"), 0o644); writeErr != nil {
+					GinkgoWriter.Printf("Warning: failed to write collected-paths.txt: %v\n", writeErr)
+				}
+
+				By("Validating node YAMLs for all cluster nodes")
+
+				for _, nodeName := range nodeNames {
+					Expect(hasMatchingFile(collectedFiles, "/nodes/"+nodeName+".yaml")).To(BeTrue(),
+						"must-gather should contain YAML for node %s", nodeName)
+				}
+
+				By("Validating SBR CRD definitions are present")
+
+				for _, crdName := range sbrparams.SBRCRDNames {
+					Expect(hasMatchingFile(collectedFiles, crdName+".yaml")).To(BeTrue(),
+						"must-gather should contain CRD definition for %s", crdName)
+				}
+
+				By("Validating MachineHealthCheck data is collected")
+
+				Expect(hasMatchingFile(collectedFiles, "machinehealthchecks")).To(BeTrue(),
+					"must-gather should contain MachineHealthCheck data")
+			})
+	})
+
+func resolveMustGatherImage() string {
+	if envImg := os.Getenv("MUST_GATHER_IMAGE"); envImg != "" {
+		GinkgoWriter.Printf("must-gather image resolved from MUST_GATHER_IMAGE env var\n")
+
+		return envImg
+	}
+
+	nhcCSVs, err := olm.ListClusterServiceVersionWithNamePattern(
+		APIClient, "node-healthcheck", medik8sparams.OperatorNs)
+	if err != nil {
+		GinkgoWriter.Printf("Warning: failed to list NHC CSVs for must-gather image resolution: %v\n", err)
+	} else {
+		for _, csv := range nhcCSVs {
+			phase, phaseErr := csv.GetPhase()
+			if phaseErr != nil || phase != oplmV1alpha1.CSVPhaseSucceeded {
+				continue
+			}
+
+			version := csv.Object.Spec.Version.String()
+			if version != "" {
+				image := fmt.Sprintf("%s:v%s", sbrparams.MustGatherImageRepo, version)
+				GinkgoWriter.Printf("must-gather image resolved from NHC CSV version: %s\n", image)
+
+				return image
+			}
+		}
+	}
+
+	GinkgoWriter.Printf("WARNING: must-gather image using hardcoded fallback tag %s\n",
+		sbrparams.MustGatherDefaultTag)
+
+	return fmt.Sprintf("%s:%s", sbrparams.MustGatherImageRepo, sbrparams.MustGatherDefaultTag)
+}
+
+func createMustGatherDestDir() string {
+	base := os.Getenv("ARTIFACT_DIR")
+	if base == "" {
+		base = GinkgoT().TempDir()
+	}
+
+	dir, mkdirErr := os.MkdirTemp(base, "sbr-must-gather-")
+	ExpectWithOffset(1, mkdirErr).ToNot(HaveOccurred(), "Failed to create must-gather output directory")
+
+	return dir
+}
+
+func runMustGather(ctx context.Context, image, destDir string) {
+	ocTimeout := fmt.Sprintf("%ds", int(sbrparams.MustGatherOCTimeout.Seconds()))
+
+	cmd := exec.CommandContext(ctx, "oc", "adm", "must-gather",
+		"--image="+image,
+		"--dest-dir="+destDir,
+		"--timeout="+ocTimeout,
+	)
+
+	env := os.Environ()
+	if os.Getenv("HOME") == "" {
+		env = append(env, "HOME=/tmp")
+	}
+
+	cmd.Env = env
+
+	output, err := cmd.CombinedOutput()
+
+	logFile := filepath.Join(destDir, "oc-adm-must-gather.log")
+
+	if writeErr := os.WriteFile(logFile, output, 0o644); writeErr != nil {
+		GinkgoWriter.Printf("Warning: failed to write must-gather log to %s: %v\n", logFile, writeErr)
+	}
+
+	GinkgoWriter.Printf("must-gather output saved to %s\n", logFile)
+
+	if ctx.Err() != nil {
+		Fail(fmt.Sprintf("must-gather timed out after %s:\n%s",
+			sbrparams.MustGatherContextTimeout, string(output)))
+	}
+
+	ExpectWithOffset(1, err).ToNot(HaveOccurred(), "must-gather failed:\n%s", string(output))
+}
+
+func collectRelativePaths(root string) ([]string, error) {
+	var paths []string
+
+	err := filepath.WalkDir(root, func(path string, d fs.DirEntry, err error) error {
+		if err != nil {
+			return err
+		}
+
+		rel, relErr := filepath.Rel(root, path)
+		if relErr != nil {
+			return relErr
+		}
+
+		paths = append(paths, filepath.ToSlash(rel))
+
+		return nil
+	})
+
+	return paths, err
+}
+
+func hasMatchingFile(files []string, pattern string) bool {
+	lowerPattern := strings.ToLower(pattern)
+	for _, f := range files {
+		if strings.Contains(strings.ToLower(f), lowerPattern) {
+			return true
+		}
+	}
+
+	return false
+}
+
+func cleanupMustGatherNamespaces(ctx context.Context, testStartTime time.Time) {
+	cleanupCtx, cleanupCancel := context.WithTimeout(ctx, sbrparams.MustGatherCleanupTimeout)
+	defer cleanupCancel()
+
+	out, err := exec.CommandContext(cleanupCtx, "oc", "get", "ns",
+		"-l", "openshift.io/run-level",
+		"-o", "jsonpath={range .items[*]}{.metadata.name} {.metadata.creationTimestamp}{\"\\n\"}{end}",
+	).CombinedOutput()
+	if err != nil {
+		GinkgoWriter.Printf("Warning: failed to list namespaces for must-gather cleanup: %v\n", err)
+
+		return
+	}
+
+	for _, line := range strings.Split(strings.TrimSpace(string(out)), "\n") {
+		fields := strings.Fields(line)
+		if len(fields) < 1 || !strings.HasPrefix(fields[0], "openshift-must-gather-") {
+			continue
+		}
+
+		namespaceName := fields[0]
+
+		if len(fields) >= 2 {
+			createdAt, parseErr := time.Parse(time.RFC3339, fields[1])
+			if parseErr == nil && createdAt.Before(testStartTime) {
+				continue
+			}
+		}
+
+		GinkgoWriter.Printf("Cleaning up leftover must-gather namespace: %s\n", namespaceName)
+
+		cleanupOut, cleanupErr := exec.CommandContext(cleanupCtx, "oc", "delete", "ns", namespaceName,
+			"--ignore-not-found", "--wait=false").CombinedOutput()
+		if cleanupErr != nil {
+			GinkgoWriter.Printf("Warning: failed to delete namespace %s: %v\n%s\n",
+				namespaceName, cleanupErr, string(cleanupOut))
+		}
+	}
+}