DAOS-19747 test: Extend test_dmg_system_reformat to verify multiple r…

src/control/lib/control/system.go

@@ -697,8 +697,14 @@ func (resp *SystemEraseResp) Errors() error {
 
 // checkSystemErase queries system to interrogate membership before deciding
 // whether a system erase is appropriate.
-func checkSystemErase(ctx context.Context, rpcClient UnaryInvoker) error {
-	resp, err := SystemQuery(ctx, rpcClient, &SystemQueryReq{FailOnUnavailable: true})
+func checkSystemErase(ctx context.Context, rpcClient UnaryInvoker, sys string) error {
+	// sys is the request's raw (unversioned) system name; pass it through unmodified so
+	// SystemQuery's own getSystem() resolution (req.Sys, falling back to rpcClient's
+	// configured system name, then the build default) appends the version suffix exactly
+	// once, matching the resolution SystemErase itself performs below.
+	queryReq := &SystemQueryReq{FailOnUnavailable: true}
+	queryReq.SetSystem(sys)
+	resp, err := SystemQuery(ctx, rpcClient, queryReq)
 	if err != nil {
 		// If the AP hasn't been started, it will respond as if it
 		// is not a replica.
@@ -737,7 +743,11 @@ func SystemErase(ctx context.Context, rpcClient UnaryInvoker, req *SystemEraseRe
 		return nil, errors.Errorf("nil %T request", req)
 	}
 
-	if err := checkSystemErase(ctx, rpcClient); err != nil {
+	// Pass through the request's raw system name (not the version-suffixed form) so
+	// checkSystemErase's internal query resolves it through the same fallback chain
+	// (request -> rpcClient config -> build default) without double-appending the
+	// version suffix.
+	if err := checkSystemErase(ctx, rpcClient, req.Sys); err != nil {
 		return nil, err
 	}
 

src/control/lib/control/system_test.go

@@ -1314,6 +1314,7 @@ func TestControl_SystemCleanup(t *testing.T) {
 
 func TestControl_System_checkSystemErase(t *testing.T) {
 	for name, tc := range map[string]struct {
+		sys          string
 		uErr, expErr error
 		members      []*mgmtpb.SystemMember
 	}{
@@ -1328,6 +1329,9 @@ func TestControl_System_checkSystemErase(t *testing.T) {
 			uErr: system.ErrRaftUnavail,
 		},
 		"empty membership": {},
+		"non-empty system name propagated to query": {
+			sys: "daos_nonstd",
+		},
 		"rank not stopped": {
 			members: []*mgmtpb.SystemMember{
 				{Rank: 0, State: system.MemberStateStopped.String()},
@@ -1358,8 +1362,21 @@ func TestControl_System_checkSystemErase(t *testing.T) {
 					&mgmtpb.SystemQueryResp{Members: tc.members}),
 			})
 
-			err := checkSystemErase(test.Context(t), mi)
+			err := checkSystemErase(test.Context(t), mi, tc.sys)
 			test.CmpErr(t, tc.expErr, err)
+
+			if tc.uErr == nil {
+				if len(mi.SentReqs) != 1 {
+					t.Fatalf("expected 1 request to be sent, got %d", len(mi.SentReqs))
+				}
+				queryReq, ok := mi.SentReqs[0].(*SystemQueryReq)
+				if !ok {
+					t.Fatalf("expected *SystemQueryReq, got %T", mi.SentReqs[0])
+				}
+				if diff := cmp.Diff(tc.sys, queryReq.Sys); diff != "" {
+					t.Fatalf("unexpected system name in query request (-want, +got):\n%s\n", diff)
+				}
+			}
 		})
 	}
 }

src/control/security/grpc_authorization.go

@@ -48,7 +48,7 @@ var methodAuthorizations = map[string][]Component{
 	"/mgmt.MgmtSvc/Join":                     {ComponentServer},
 	"/mgmt.MgmtSvc/ClusterEvent":             {ComponentServer},
 	"/mgmt.MgmtSvc/LeaderQuery":              {ComponentAdmin},
-	"/mgmt.MgmtSvc/SystemQuery":              {ComponentAdmin},
+	"/mgmt.MgmtSvc/SystemQuery":              {ComponentAdmin, ComponentServer},
 	"/mgmt.MgmtSvc/SystemErase":              {ComponentAdmin, ComponentServer},
 	"/mgmt.MgmtSvc/SystemStart":              {ComponentAdmin},
 	"/mgmt.MgmtSvc/SystemStop":               {ComponentAdmin},

src/control/security/grpc_authorization_test.go

@@ -65,7 +65,7 @@ func TestSecurity_ComponentHasAccess(t *testing.T) {
 		"/mgmt.MgmtSvc/Join":                     {ComponentServer},
 		"/mgmt.MgmtSvc/ClusterEvent":             {ComponentServer},
 		"/mgmt.MgmtSvc/LeaderQuery":              {ComponentAdmin},
-		"/mgmt.MgmtSvc/SystemQuery":              {ComponentAdmin},
+		"/mgmt.MgmtSvc/SystemQuery":              {ComponentAdmin, ComponentServer},
 		"/mgmt.MgmtSvc/SystemStop":               {ComponentAdmin},
 		"/mgmt.MgmtSvc/SystemErase":              {ComponentAdmin, ComponentServer},
 		"/mgmt.MgmtSvc/SystemStart":              {ComponentAdmin},

src/control/server/mgmt_system.go

@@ -1823,6 +1823,10 @@ func (svc *mgmtSvc) eraseReplicas(ctx context.Context, peers []*net.TCPAddr) err
 		svc.log.Tracef("sending erase RPC to peer %s", peer.String())
 		peerReq := new(control.SystemEraseReq)
 		peerReq.AddHost(peer.String())
+		// Explicitly set the system name rather than relying on svc.rpcClient's
+		// configured default, so the forwarded request is correctly scoped even if
+		// that default is ever left unset.
+		peerReq.SetSystem(svc.sysdb.SystemName())
 
 		if _, err := control.SystemErase(ctx, svc.rpcClient, peerReq); err != nil {
 			// A connection error here can't be distinguished from "peer never
@@ -1934,9 +1938,34 @@ func (svc *mgmtSvc) scheduleControlPlaneRestart(leaderStr string) error {
 	return nil
 }
 
-// execControlPlaneRestart drains the gRPC server via GracefulStop() (falling back to a
-// forced Stop() after svc.gracefulStopTimeout), then exec()s to restart the control
-// plane process.
+// dumpGoroutineStacks logs the stacks of all running goroutines, for diagnosing hangs in
+// gRPC/raft shutdown paths that have no other visibility into what is blocking progress.
+func (svc *mgmtSvc) dumpGoroutineStacks(reason string) {
+	buf := make([]byte, 1<<20)
+	for {
+		n := runtime.Stack(buf, true)
+		if n < len(buf) {
+			buf = buf[:n]
+			break
+		}
+		buf = make([]byte, 2*len(buf))
+	}
+	svc.log.Errorf("System Erase: dumping goroutine stacks (%s):\n%s", reason, buf)
+}
+
+// execControlPlaneRestart drains the gRPC server via GracefulStop(), falling back to
+// exec()ing to restart the control plane process after svc.gracefulStopTimeout if the
+// drain hasn't completed by then.
+//
+// A forced grpcServer.Stop() is deliberately *not* attempted as a fallback here: Stop()
+// and GracefulStop() share the same internal server mutex/condvar/connection bookkeeping
+// (see grpc's Server.stop()), so calling Stop() while GracefulStop() is still draining
+// just queues behind the same lock and blocks on the same "all connections closed"
+// condition rather than forcing anything independently. If GracefulStop() is stuck (e.g.
+// a peer keeps a connection open rather than closing it in response to drain), Stop()
+// will be stuck too, so it only adds a wasted extra wait before exec() proceeds anyway.
+// exec() itself terminates this process image and the kernel closes all of its sockets,
+// which is sufficient to tear down the gRPC server's listeners/connections.
 func (svc *mgmtSvc) execControlPlaneRestart(myPath, leaderStr string) {
 	scheduledAt := time.Now()
 
@@ -1951,9 +1980,9 @@ func (svc *mgmtSvc) execControlPlaneRestart(myPath, leaderStr string) {
 		svc.log.Debugf("System Erase: gRPC server drained cleanly after %s [role=%s]",
 			time.Since(scheduledAt), leaderStr)
 	case <-time.After(svc.gracefulStopTimeout):
-		svc.log.Errorf("System Erase: timed out after %s waiting for gRPC server to drain, forcing stop [role=%s]",
+		svc.log.Errorf("System Erase: timed out after %s waiting for gRPC server to drain, proceeding to exec anyway [role=%s]",
 			svc.gracefulStopTimeout, leaderStr)
-		svc.grpcServer.Stop()
+		svc.dumpGoroutineStacks("GracefulStop() timed out")
 	}
 
 	svc.log.Infof("System Erase: exec'ing %s to restart control plane after %s total drain [role=%s]",

src/control/server/server.go

@@ -244,6 +244,7 @@ func (srv *server) createServices(ctx context.Context) (err error) {
 	// Create rpcClient for inter-server communication.
 	cliCfg := control.DefaultConfig()
 	cliCfg.TransportConfig = srv.cfg.TransportConfig
+	cliCfg.SystemName = srv.cfg.SystemName
 	rpcClient := control.NewClient(
 		control.WithClientComponent(build.ComponentServer),
 		control.WithConfig(cliCfg),

src/tests/ftest/control/dmg_system_reformat.py

@@ -16,8 +16,11 @@
 class DmgSystemReformatTest(TestWithServers):
     """Test Class Description:
 
-    Test to verify dmg storage format reformat option works as expected on a
-    DAOS system after a controlled shutdown.
+    Test to verify dmg system erase and storage format reformat work as expected on a
+    DAOS system after a controlled shutdown. Exercised across PMem and MD-on-SSD tier-0
+    storage configurations (selected via launch.py --nvme) and single/multiple
+    management service (MS) replica configurations (selected via the
+    mgmt_svc_replicas_qty test yaml setup parameter).
 
     :avocado: recursive
     """
@@ -28,19 +31,43 @@ def setUp(self):
         self.update_log_file_names()
         super().setUp()
 
+    def get_replicas(self, dmg):
+        """Get the current set of management service (MS) replica ranks.
+
+        Args:
+            dmg (DmgCommand): dmg command object to use to query the system.
+
+        Returns:
+            list: sorted list of MS replica ranks reported by the leader.
+
+        """
+        leader_info = dmg.system_leader_query()
+        return sorted(leader_info["response"]["replicas"])
+
     def test_dmg_system_reformat(self):
         """
         JIRA ID: DAOS-5415
 
-        Test Description: Test dmg system reformat functionality.
+        Test Description: Test dmg system erase/reformat functionality across both
+        PMem and MD-on-SSD storage tier-0 configurations as well as single and
+        multiple management service (MS) replica configurations. The storage mode
+        (PMem vs. MD-on-SSD) is selected at launch time (e.g. launch.py --nvme=auto
+        vs. --nvme=auto_md_on_ssd); the MS replica count is varied via the
+        mgmt_svc_replicas_qty test yaml setup parameter.
 
         :avocado: tags=all,daily_regression
         :avocado: tags=hw,medium
-        :avocado: tags=control,dmg,system_reformat
+        :avocado: tags=control,dmg,system_reformat,system_erase
         :avocado: tags=DmgSystemReformatTest,test_dmg_system_reformat
         """
         dmg = self.get_dmg_command().copy()
 
+        self.log.info(
+            "Testing with %s configured MS replica(s): %s",
+            len(self.mgmt_svc_replicas), self.mgmt_svc_replicas)
+        pre_erase_replicas = self.get_replicas(dmg)
+        self.log.info("MS replica ranks before erase: %s", pre_erase_replicas)
+
         # Create pool using 90% of the available NVMe capacity
         pools = [self.get_pool(dmg=dmg)]
 
@@ -56,7 +83,7 @@ def test_dmg_system_reformat(self):
         dmg.exit_status_exception = True
 
         self.log.info("Stop running engine instances: 'dmg system stop'")
-        dmg.system_stop(force=True)
+        dmg.system_stop()
         if dmg.result.exit_status != 0:
             self.fail("Detected issues performing a system stop: {}".format(
                 dmg.result.stderr_text))
@@ -75,26 +102,54 @@ def test_dmg_system_reformat(self):
 
         self.log.info("Perform dmg storage format on all system ranks:")
 
-        # Calling storage format after system stop too soon would fail, so
-        # wait 10 sec and retry up to 4 times.
+        # Record the start time before the first format attempt so that engine-start
+        # detection below covers engines that joined during an earlier retry, even if
+        # that attempt's dmg client call locally timed out before the server-side
+        # format/join sequence actually completed (see MD-on-SSD timing note below).
+        start_time = journalctl_time()
+
+        # Calling storage format after system stop too soon would fail, so wait 10 sec
+        # and retry up to 4 times. Use a generous per-call client timeout since
+        # MD-on-SSD NVMe format plus engine startup/join can legitimately take longer
+        # than the previous 30 sec default, especially under loaded CI hardware; a
+        # client-side timeout here does not necessarily mean the server-side format
+        # failed, it may just still be in progress.
         count = 0
         while count < 4:
             try:
-                dmg.storage_format(force=True)
+                dmg.storage_format(timeout=90)
                 if dmg.result.exit_status != 0:
                     self.fail(
-                        "Issues performing storage format --force: {}".format(
+                        "Issues performing storage format: {}".format(
                             dmg.result.stderr_text))
                 break
             except CommandFailure as error:
+                if "code = 509" in str(error) or "invoked on a running system" in str(error):
+                    # A previous attempt's client-side call timed out locally but the
+                    # server-side format/join actually completed in the background;
+                    # this later retry is correctly rejected because the system is
+                    # already up. Treat this as success rather than exhausting retries.
+                    self.log.info(
+                        "Storage format already completed by a prior attempt: %s", error)
+                    break
                 self.log.info("Storage format failed. Wait 10 sec and retry. %s", error)
                 count += 1
                 time.sleep(10)
 
-        # Check that engine starts up again
+        # Check that engine starts up again on every host, not just the last one, since
+        # a multi-replica MS configuration spans more than one host.
         self.log.info("<SERVER> Waiting for the engines to start")
-        self.server_managers[-1].manager.timestamps["start"] = journalctl_time()
-        self.server_managers[-1].detect_engine_start()
+        for server_manager in self.server_managers:
+            server_manager.manager.timestamps["start"] = start_time
+            server_manager.detect_engine_start()
+
+        # Verify the MS replica set survived the erase/reformat cycle intact - this is the
+        # critical guarantee relied upon by the raft database erase-then-restart logic.
+        post_erase_replicas = self.get_replicas(dmg)
+        self.log.info("MS replica ranks after erase/reformat: %s", post_erase_replicas)
+        self.assertEqual(
+            pre_erase_replicas, post_erase_replicas,
+            "MS replica ranks changed as a result of system erase/reformat")
 
         # Check that we have cleared storage by checking pool list
         if dmg.get_pool_list_uuids():

src/tests/ftest/control/dmg_system_reformat.yaml

@@ -1,13 +1,27 @@
 hosts:
-  test_servers: 1
+  test_servers: 3
   test_clients: 1
 timeout: 300
+setup: !mux
+  single_replica:
+    mgmt_svc_replicas_qty: 1
+  multi_replica:
+    mgmt_svc_replicas_qty: 3
 server_config:
-  name: daos_server
   engines_per_host: 1
   engines:
     0:
       storage: auto
   system_ram_reserved: 32
-pool:
-  size: 90%
+  variant: !mux
+    standard:
+      name: daos_server
+    non_standard:
+      name: daos_nonstd
+      port: 10515
+pool: !mux
+  default:
+    size: 90%
+  md_on_ssd_p2:
+    size: 90%
+    mem_ratio: 25

src/tests/ftest/util/test_utils_pool.py

@@ -39,6 +39,7 @@ def get_size_params(pool):
     """
     return {"size": None,
             "tier_ratio": None,
+            "mem_ratio": None,
             "scm_size": pool.scm_per_rank,
             "nvme_size": pool.nvme_per_rank}