DAOS-16881 control: Fix daos_server scm prep for single missing ns

Features: control Required-githooks: true Signed-off-by: Tom Nabarro <[email protected]>
daos-stack · Dec 17, 2024 · c6324ee · c6324ee
1 parent b31f634
commit c6324ee
Show file tree

Hide file tree

Showing 4 changed files with 97 additions and 15 deletions.
diff --git a/src/control/server/storage/scm/ipmctl.go b/src/control/server/storage/scm/ipmctl.go
@@ -201,27 +201,24 @@ func (cr *cmdRunner) handleFreeCapacity(sockSelector int, nrNsPerSock uint, regi
 		return nil, nil, errors.Wrap(err, "createNamespaces")
 	}
 
-	numaSelector := sockAny
 	switch len(numaIDs) {
 	case 0:
-		return nil, nil, errors.New("no numa nodes were processed")
+		return nil, nil, errors.New("no namespaces created on regions with free capacity")
 	case 1:
-		numaSelector = numaIDs[0]
 	default:
 		if sockSelector != sockAny {
 			return nil, nil,
 				errors.Errorf("unexpected number of numa nodes processed, want 1 got %d",
 					len(numaIDs))
 		}
 	}
+	cr.log.Tracef("namespaces created on %v numa-nodes, fetching updated region details", numaIDs)
 
-	nss, err := cr.getNamespaces(numaSelector)
+	nss, err := cr.getNamespaces(sockSelector)
 	if err != nil {
 		return nil, nil, errors.Wrap(err, "getNamespaces")
 	}
 
-	cr.log.Debug("namespaces created, fetching updated region details")
-
 	rs, err := cr.getRegions(sockSelector)
 	if err != nil {
 		return nil, nil, errors.Wrap(err, "getRegions")
@@ -279,14 +276,14 @@ func (cr *cmdRunner) processActionableState(req storage.ScmPrepareRequest, state
 		}
 		resp.RebootRequired = true
 	case storage.ScmFreeCap:
-		// Regions exist but no namespaces, create block devices on PMem regions and
-		// populate response with namespace details.
+		// At least one region exists without a namespace so create block devices on those PMem regions
+		// with available capacity and populate response with namespace details.
 		cr.log.Info("Creating PMem namespaces...")
 		nss, sockState, err := cr.handleFreeCapacity(sockSelector, req.NrNamespacesPerSocket, regions)
 		if err != nil {
 			return nil, errors.Wrap(err, "handleFreeCapacity")
 		}
-		resp.Namespaces = nss
+		resp.Namespaces = nss // Return only namespaces created.
 		resp.Socket = sockState
 	case storage.ScmNoFreeCap:
 		// Regions and namespaces exist so no changes to response necessary.

diff --git a/src/control/server/storage/scm/ipmctl_test.go b/src/control/server/storage/scm/ipmctl_test.go
@@ -464,6 +464,35 @@ func TestIpmctl_prep(t *testing.T) {
 				cmdListNamespaces, cmdShowRegions,
 			},
 		},
+		"free capacity; dual regions; single namespace": {
+			scanResp: &storage.ScmScanResponse{
+				Modules: testModules,
+			},
+			runOut: []string{
+				verStr,
+				mockXMLRegions(t, "dual-sock-one-free"),
+				"",
+				ndctlRegionsOneFree,
+				"",
+				ndctlDualNsStr,
+				mockXMLRegions(t, "dual-sock-no-free"),
+			},
+			expCalls: []pmemCmd{
+				cmdShowIpmctlVersion,
+				cmdShowRegions,
+				cmdDeleteGoals,
+				cmdListNdctlRegions,
+				mockCmdCreateNamespace(1, 1082331758592),
+				cmdListNamespaces,
+				cmdShowRegions,
+			},
+			expPrepResp: &storage.ScmPrepareResponse{
+				Namespaces: dualNS,
+				Socket: &storage.ScmSocketState{
+					State: storage.ScmNoFreeCap,
+				},
+			},
+		},
 		"free capacity; two namespaces per socket requested": {
 			prepReq: &storage.ScmPrepareRequest{
 				NrNamespacesPerSocket: 2,
@@ -514,6 +543,7 @@ func TestIpmctl_prep(t *testing.T) {
 			expCalls: []pmemCmd{
 				cmdShowIpmctlVersion, mockCmdShowRegionsWithSock(1),
 				mockCmdDeleteGoalsWithSock(1), cmdListNdctlRegions,
+				// Region0 on NUMA1, iset-id matched.
 				mockCmdCreateNamespace(0, 541165879296),
 				mockCmdCreateNamespace(0, 541165879296),
 				mockCmdListNamespacesWithNUMA(1),
@@ -566,7 +596,7 @@ func TestIpmctl_prep(t *testing.T) {
 				mockCmdDeleteGoalsWithSock(1), cmdListNdctlRegions,
 				mockCmdCreateNamespace(1, 541165879296),
 				mockCmdCreateNamespace(1, 541165879296),
-				mockCmdListNamespacesWithNUMA(0),
+				mockCmdListNamespacesWithNUMA(1),
 				mockCmdShowRegionsWithSock(1),
 			},
 		},

diff --git a/src/control/server/storage/scm/mocks.go b/src/control/server/storage/scm/mocks.go
@@ -80,6 +80,12 @@ func mockXMLRegions(t *testing.T, variant string) string {
 		rl.Regions[1].ID = 2
 		rl.Regions[1].SocketID = 1
 		rl.Regions[1].ISetID++
+	case "dual-sock-one-free":
+		rl.Regions = append(rl.Regions, rl.Regions[0])
+		rl.Regions[1].ID = 2
+		rl.Regions[1].SocketID = 1
+		rl.Regions[1].ISetID++
+		rl.Regions[1].FreeCapacity = rl.Regions[1].Capacity
 	case "dual-sock-isetid-switch":
 		rl.Regions[0].FreeCapacity = rl.Regions[0].Capacity
 		rl.Regions = append(rl.Regions, rl.Regions[0])
@@ -261,6 +267,50 @@ const (
     "persistence_domain":"memory_controller"
   }
 ]
+`
+
+	// JSON output from "ndctl list -Rv" showing single namespace on dual socket.
+	ndctlRegionsOneFree = `[
+    {
+      "dev":"region1",
+      "size":1078036791296,
+      "align":16777216,
+      "available_size":1078036791296,
+      "max_available_extent":1078036791296,
+      "type":"pmem",
+      "numa_node":1,
+      "target_node":3,
+      "iset_id":-3098214067285580732,
+      "persistence_domain":"memory_controller"
+    },
+    {
+      "dev":"region0",
+      "size":1078036791296,
+      "align":16777216,
+      "available_size":0,
+      "max_available_extent":0,
+      "type":"pmem",
+      "numa_node":0,
+      "target_node":2,
+      "iset_id":7845533025580426308,
+      "persistence_domain":"memory_controller",
+      "namespaces":[
+        {
+          "dev":"namespace0.0",
+          "mode":"fsdax",
+          "map":"dev",
+          "size":1061190369280,
+          "uuid":"d7804a1c-954a-48f5-bc4a-1fc76c6b60ee",
+          "raw_uuid":"83c04060-109d-40a3-8867-6664c277c958",
+          "sector_size":512,
+          "align":2097152,
+          "blockdev":"pmem0",
+          "numa_node":0,
+          "target_node":2
+        }
+      ]
+    }
+  ]
 `
 )
 

diff --git a/src/control/server/storage/scm/ndctl.go b/src/control/server/storage/scm/ndctl.go
@@ -78,8 +78,6 @@ func (cr *cmdRunner) createNamespaces(regionPerSocket socketRegionMap, nrNsPerSo
 	if len(sockIDs) == 0 {
 		return nil, errors.New("expected non-zero number of pmem regions in input map")
 	}
-	cr.log.Debugf("creating %d namespaces on each of the following socket(s): %v", nrNsPerSock,
-		sockIDs)
 
 	// As the selector is socket, look up the ndctl region with the same ISetID as the ipmctl
 	// region with specified socket ID. This may not work when the ISetID overflows in ndctl
@@ -134,13 +132,20 @@ func (cr *cmdRunner) createNamespaces(regionPerSocket socketRegionMap, nrNsPerSo
 			regionsToPrep = append(regionsToPrep, &NdctlRegion{
 				Dev:           fmt.Sprintf("region%d", sid),
 				AvailableSize: uint64(regionPerSocket[sid].FreeCapacity),
+				NumaNode:      uint32(sid),
 			})
 		}
 	}
 
+	cr.log.Debugf("attempting to create %d namespaces on each of the following socket(s): %v",
+		nrNsPerSock, sockIDs)
+
 	var numaNodesPrepped []int
 	for _, region := range regionsToPrep {
-		cr.log.Debugf("creating namespaces on %q", region.Dev)
+		if region.AvailableSize == 0 {
+			cr.log.Tracef("skipping namespace creation on full region %q", region.Dev)
+			continue
+		}
 
 		// Check value is 2MiB aligned and (TODO) multiples of interleave width.
 		pmemBytes := uint64(region.AvailableSize) / uint64(nrNsPerSock)
@@ -159,8 +164,8 @@ func (cr *cmdRunner) createNamespaces(regionPerSocket socketRegionMap, nrNsPerSo
 			if _, err := cr.runCmd(cmd); err != nil {
 				return nil, errors.WithMessagef(err, "%s", region.Dev)
 			}
-			cr.log.Debugf("created namespace on %s size %s", region.Dev,
-				humanize.IBytes(pmemBytes))
+			cr.log.Debugf("created namespace on %s size %s (numa %d)", region.Dev,
+				humanize.IBytes(pmemBytes), region.NumaNode)
 		}
 
 		numaNodesPrepped = append(numaNodesPrepped, int(region.NumaNode))