diff --git a/charts/topograph/values.slinky.block-example.yaml b/charts/topograph/values.slinky.block-example.yaml index d5232454..f9ea2b79 100644 --- a/charts/topograph/values.slinky.block-example.yaml +++ b/charts/topograph/values.slinky.block-example.yaml @@ -19,7 +19,7 @@ global: matchLabels: app.kubernetes.io/component: compute plugin: topology/block - block_sizes: 4 + blockSizes: [4] topologyConfigPath: topology.conf topologyConfigmapName: slurm-config reportingMode: staticNodes diff --git a/docs/api.md b/docs/api.md index 45358f18..da4dea4d 100644 --- a/docs/api.md +++ b/docs/api.md @@ -85,13 +85,13 @@ Topograph exposes three endpoints for interacting with the service. Below are th - **slurm parameters**: - **topologyConfigPath**: (optional) A string specifying the file path for the topology configuration. If omitted, the topology config content is returned in the HTTP response. - **plugin**: (optional) A string specifying topology plugin: `topology/tree` (default) or `topology/block`. - - **block_sizes**: (optional) A string specifying block size for `topology/block` plugin. + - **blockSizes**: (optional) A string specifying block size for `topology/block` plugin. - **reconfigure**: (optional) If `true`, invoke `scontrol reconfigure` after topology config is generated. Default `false` - **slinky parameters**: - **namespace**: A string specifying namespace where SLURM cluster is running. - **podSelector**: A standard Kubernetes label selector for pods running SLURM nodes. - **plugin**: (optional) A string specifying topology plugin: `topology/tree` (default) or `topology/block`. - - **block_sizes**: (optional) A string specifying block size for `topology/block` plugin. + - **blockSizes**: (optional) A string specifying block size for `topology/block` plugin. - **topologyConfigPath**: A string specifying the key for the topology config in the ConfigMap. - **topologyConfigmapName**: A string specifying the name of the ConfigMap containing the topology config. - **nodes**: (optional) An array of regions mapping instance IDs to node names. @@ -111,7 +111,7 @@ Topograph exposes three endpoints for interacting with the service. Below are th "name": "slurm", "params": { "plugin": "topology/block", - "block_sizes": "30,120" + "blockSizes": [30,120] } }, "nodes": [ diff --git a/docs/engines/slinky.md b/docs/engines/slinky.md index 00342434..91030134 100644 --- a/docs/engines/slinky.md +++ b/docs/engines/slinky.md @@ -31,14 +31,14 @@ global: provider: aws engine: slinky engineParams: - namespace: ns-slinky # Namespace where Slinky is running - podSelector: # Label selector for pods running SLURM nodes + namespace: ns-slinky # Namespace where Slinky is running + podSelector: # Label selector for pods running SLURM nodes matchLabels: app.kubernetes.io/component: compute - plugin: topology/block # Name of the topology plugin - block_sizes: 4 # (Optional) Block size for the block topology plugin - topologyConfigmapName: slurm-config # Name of the ConfigMap containing the topology config - topologyConfigPath: topology.conf # Key in the ConfigMap for the topology config + plugin: topology/block # Name of the topology plugin + blockSizes: [4] # (Optional) Block size for the block topology plugin + topologyConfigmapName: slurm-config # Name of the ConfigMap containing the topology config + topologyConfigPath: topology.conf # Key in the ConfigMap for the topology config ``` ### Per-partition topologies @@ -160,7 +160,7 @@ curl -X POST -H "Content-Type: application/json" \ "topologyConfigPath": "topology.conf", "topologyConfigmapName": "slurm-config", "plugin": "topology/block", - "block_sizes": "8,16,32" + "blockSizes": [8,16,32] } } }' \ diff --git a/docs/engines/slurm.md b/docs/engines/slurm.md index b03b431a..d3a99509 100644 --- a/docs/engines/slurm.md +++ b/docs/engines/slurm.md @@ -58,7 +58,7 @@ id=$(curl -s -X POST -H "Content-Type: application/json" -d '{"provider":{"param And if you want to view the block topology (with specified block sizes), use the command: ```bash -id=$(curl -s -X POST -H "Content-Type: application/json" -d '{"provider":{"params":{"modelFileName":"/usr/local/bin/tests/models/.yaml"}},"engine":{"params":{"plugin":"topology/block", "block_sizes": "4,8"}}}' http://localhost:49021/v1/generate) +id=$(curl -s -X POST -H "Content-Type: application/json" -d '{"provider":{"params":{"modelFileName":"/usr/local/bin/tests/models/.yaml"}},"engine":{"params":{"plugin":"topology/block", "blockSizes": [4,8]}}}' http://localhost:49021/v1/generate) ``` You can query the results of either topology request with: diff --git a/docs/get-started/quickstart-k8s.md b/docs/get-started/quickstart-k8s.md index 42496c22..574a24cd 100644 --- a/docs/get-started/quickstart-k8s.md +++ b/docs/get-started/quickstart-k8s.md @@ -76,7 +76,7 @@ Engine-specific install flags point the `slinky` engine at the Slinky deployment --set global.engineParams.topologyConfigPath=topology.conf ``` -The full engine-parameter shape (`podSelector`, `plugin`, `block_sizes`, per-partition topologies, …) is documented in the [chart README](https://github.com/NVIDIA/topograph/blob/main/charts/topograph/README.md). Example values files for common Slinky scenarios ship in the chart directory: +The full engine-parameter shape (`podSelector`, `plugin`, `blockSizes`, per-partition topologies, …) is documented in the [chart README](https://github.com/NVIDIA/topograph/blob/main/charts/topograph/README.md). Example values files for common Slinky scenarios ship in the chart directory: - [`values.slinky.tree-example.yaml`](https://github.com/NVIDIA/topograph/blob/main/charts/topograph/values.slinky.tree-example.yaml) — tree topology - [`values.slinky.block-example.yaml`](https://github.com/NVIDIA/topograph/blob/main/charts/topograph/values.slinky.block-example.yaml) — block topology diff --git a/pkg/engines/slinky/engine.go b/pkg/engines/slinky/engine.go index 88a87dd5..00fbf03c 100644 --- a/pkg/engines/slinky/engine.go +++ b/pkg/engines/slinky/engine.go @@ -22,6 +22,8 @@ import ( "fmt" "maps" "net/http" + "strconv" + "strings" "time" corev1 "k8s.io/api/core/v1" @@ -258,7 +260,7 @@ func (eng *SlinkyEngine) generateConfigMapAnnotations() map[string]string { annotations[topology.KeyConfigMapPlugin] = eng.params.Plugin } if len(eng.params.BlockSizes) != 0 { - annotations[topology.KeyConfigMapBlockSizes] = eng.params.BlockSizes + annotations[topology.KeyConfigMapBlockSizes] = intToStr(eng.params.BlockSizes) } return annotations @@ -545,3 +547,11 @@ func (eng *SlinkyEngine) updateNodeAnnotation(ctx context.Context, node *corev1. return nil } + +func intToStr(input []int) string { + strs := make([]string, len(input)) + for i, n := range input { + strs[i] = strconv.Itoa(n) + } + return strings.Join(strs, ",") +} diff --git a/pkg/engines/slinky/engine_test.go b/pkg/engines/slinky/engine_test.go index be084747..c9628986 100644 --- a/pkg/engines/slinky/engine_test.go +++ b/pkg/engines/slinky/engine_test.go @@ -143,14 +143,14 @@ func TestGetParameters(t *testing.T) { topology.KeyPodSelector: podSelector, topology.KeyNodeSelector: nodeSelector, topology.KeyPlugin: topology.TopologyBlock, - topology.KeyBlockSizes: "16", + topology.KeyBlockSizes: []int{16}, topology.KeyTopoConfigPath: "path", topology.KeyTopoConfigmapName: "name", }, ret: &Params{ BaseParams: slurm.BaseParams{ Plugin: topology.TopologyBlock, - BlockSizes: "16", + BlockSizes: []int{16}, }, Namespace: "namespace", PodSelector: labelSelector, @@ -343,7 +343,7 @@ func TestConfigMapAnnotationsAndMetadata(t *testing.T) { name: "with block sizes only", params: &Params{ BaseParams: slurm.BaseParams{ - BlockSizes: "8,16,32", + BlockSizes: []int{8, 16, 32}, }, Namespace: "test-namespace", PodSelector: labelSelector, @@ -357,7 +357,7 @@ func TestConfigMapAnnotationsAndMetadata(t *testing.T) { params: &Params{ BaseParams: slurm.BaseParams{ Plugin: topology.TopologyBlock, - BlockSizes: "8,16,32", + BlockSizes: []int{8, 16, 32}, }, Namespace: "test-namespace", PodSelector: labelSelector, @@ -388,19 +388,19 @@ func TestConfigMapAnnotationsAndMetadata(t *testing.T) { require.NotContains(t, annotations, topology.KeyConfigMapPlugin) } if tc.wantBlock { - requireAnnotation(t, annotations, topology.KeyConfigMapBlockSizes, tc.params.BlockSizes) + requireAnnotation(t, annotations, topology.KeyConfigMapBlockSizes, intToStr(tc.params.BlockSizes)) } else { require.NotContains(t, annotations, topology.KeyConfigMapBlockSizes) } // Metadata logic (simulate GenerateOutput) tree := &topology.Vertex{Name: "root"} - setMetadata(tree, tc.params.Plugin, tc.params.BlockSizes) + setMetadata(tree, tc.params.Plugin, intToStr(tc.params.BlockSizes)) if tc.wantPlugin { require.Equal(t, tc.params.Plugin, tree.Metadata[topology.KeyPlugin]) } if tc.wantBlock { - require.Equal(t, tc.params.BlockSizes, tree.Metadata[topology.KeyBlockSizes]) + require.Equal(t, intToStr(tc.params.BlockSizes), tree.Metadata[topology.KeyBlockSizes]) } }) } diff --git a/pkg/engines/slurm/slurm.go b/pkg/engines/slurm/slurm.go index 09df81c4..0bb2a925 100644 --- a/pkg/engines/slurm/slurm.go +++ b/pkg/engines/slurm/slurm.go @@ -23,7 +23,6 @@ import ( "fmt" "net/http" "regexp" - "strconv" "strings" "k8s.io/klog/v2" @@ -34,7 +33,6 @@ import ( "github.com/NVIDIA/topograph/internal/files" "github.com/NVIDIA/topograph/internal/httperr" "github.com/NVIDIA/topograph/pkg/engines" - "github.com/NVIDIA/topograph/pkg/metrics" "github.com/NVIDIA/topograph/pkg/topology" "github.com/NVIDIA/topograph/pkg/translate" ) @@ -51,10 +49,8 @@ const NAME = "slurm" type SlurmEngine struct{} type BaseParams struct { - Plugin string `mapstructure:"plugin"` - BlockSizes string `mapstructure:"block_sizes"` - FakeNodesEnabled bool `mapstructure:"fakeNodesEnabled"` - FakeNodePool string `mapstructure:"fake_node_pool"` + Plugin string `mapstructure:"plugin"` + BlockSizes []int `mapstructure:"blockSizes"` } type Topology struct { @@ -184,34 +180,6 @@ func GetNodeList(ctx context.Context) ([]string, error) { return nodes, nil } -func GetFakeNodes(ctx context.Context) (string, error) { - args := []string{"show", "partition", "fake"} - stdout, err := exec.Exec(ctx, "scontrol", args, nil) - if err != nil { - return "", err - } - out := stdout.String() - klog.V(4).Infof("stdout: %s", out) - - return parseFakeNodes(out) -} - -func parseFakeNodes(data string) (string, error) { - prefix := "Nodes=" - scanner := bufio.NewScanner(strings.NewReader(data)) - for scanner.Scan() { - if line := strings.TrimSpace(scanner.Text()); strings.HasPrefix(line, prefix) { - return line[len(prefix):], nil - } - } - - if err := scanner.Err(); err != nil { - return "", fmt.Errorf("failed to scan fake nodes partition: %v", err) - } - - return "", fmt.Errorf("fake partition has no nodes") -} - func getPartitionNodes(ctx context.Context, partition string, _ []any) (string, error) { args := []string{"show", "partition", partition} stdout, err := exec.Exec(ctx, "scontrol", args, nil) @@ -307,22 +275,7 @@ func GenerateOutputParams(ctx context.Context, root *topology.Vertex, params *Pa func GetTranslateConfig(ctx context.Context, params *BaseParams, topologies map[string]*Topology, f *TopologyNodeFinder) (*translate.Config, error) { cfg := &translate.Config{ Plugin: params.Plugin, - BlockSizes: getBlockSizes(params.BlockSizes), - } - - // set fake nodes - if params.Plugin == topology.TopologyBlock && params.FakeNodesEnabled { - var fakeNodes string - var err error - if len(params.FakeNodePool) > 0 { - fakeNodes = params.FakeNodePool - } else { - fakeNodes, err = GetFakeNodes(ctx) - if err != nil { - return nil, err - } - } - cfg.FakeNodePool = fakeNodes + BlockSizes: params.BlockSizes, } // set per-partition topologies @@ -361,24 +314,6 @@ func getParams(params map[string]any) (*Params, error) { return &p, err } -func getBlockSizes(str string) []int { - if len(str) == 0 { - return nil - } - parts := strings.Split(str, ",") - blockSizes := make([]int, 0, len(parts)) - for _, part := range parts { - sz, err := strconv.Atoi(part) - if err != nil { - metrics.AddValidationError("BlockSize parsing error") - klog.Warningf("Failed to parse blockSize %v: %v. Ignoring admin blockSizes.", part, err) - return nil - } - blockSizes = append(blockSizes, sz) - } - return blockSizes -} - func reconfigure(ctx context.Context) error { stdout, err := exec.Exec(ctx, "scontrol", []string{"reconfigure"}, nil) if err != nil { diff --git a/pkg/engines/slurm/slurm_test.go b/pkg/engines/slurm/slurm_test.go index aca257a3..4e397fd9 100644 --- a/pkg/engines/slurm/slurm_test.go +++ b/pkg/engines/slurm/slurm_test.go @@ -83,45 +83,6 @@ func TestAggregateComputeInstances(t *testing.T) { } } -func TestParseFakeNodes(t *testing.T) { - testCases := []struct { - name string - in string - out string - err string - }{ - { - name: "Case 1: no nodes", - err: "fake partition has no nodes", - }, - { - name: "Case 2: valid input", - in: `PartitionName=fake - AllowQos=ALL - DefaultTime=NONE DisableRootJobs=NO ExclusiveUser=NO GraceTime=0 Hidden=NO - MaxNodes=UNLIMITED MaxTime=08:00:00 MinNodes=1 LLN=NO MaxCPUsPerNode=UNLIMITED MaxCPUsPerSocket=UNLIMITED - Nodes=fake-[01-16] - OverTimeLimit=NONE PreemptMode=OFF - JobDefaults=(null) - DefMemPerNode=UNLIMITED MaxMemPerNode=UNLIMITED -`, - out: "fake-[01-16]", - }, - } - - for _, tc := range testCases { - t.Run(tc.name, func(t *testing.T) { - out, err := parseFakeNodes(tc.in) - if len(tc.err) != 0 { - require.EqualError(t, err, tc.err) - } else { - require.NoError(t, err) - require.Equal(t, tc.out, out) - } - }) - } -} - func TestParsePartitionNodes(t *testing.T) { testCases := []struct { name string @@ -197,7 +158,6 @@ func TestGetParams(t *testing.T) { in: ` { "plugin": "123", - "fakeNodesEnabled": true, "topologies": { "topo1": { "plugin": "topology/block", @@ -217,8 +177,7 @@ func TestGetParams(t *testing.T) { `, params: &Params{ BaseParams: BaseParams{ - Plugin: "123", - FakeNodesEnabled: true, + Plugin: "123", }, Topologies: map[string]*Topology{ "topo1": { @@ -280,20 +239,10 @@ func TestGetTranslateConfig(t *testing.T) { }, }, { - name: "Case 2: invalid blocksize", - params: &BaseParams{ - Plugin: topology.TopologyBlock, - BlockSizes: "bad", - }, - cfg: &translate.Config{ - Plugin: topology.TopologyBlock, - }, - }, - { - name: "Case 3: valid blocksize", + name: "Case 2: valid blocksize", params: &BaseParams{ Plugin: topology.TopologyBlock, - BlockSizes: "2,4,8", + BlockSizes: []int{2, 4, 8}, }, cfg: &translate.Config{ Plugin: topology.TopologyBlock, @@ -301,21 +250,7 @@ func TestGetTranslateConfig(t *testing.T) { }, }, { - name: "Case 4: with fake nodes", - params: &BaseParams{ - Plugin: topology.TopologyBlock, - BlockSizes: "2,4,8", - FakeNodesEnabled: true, - FakeNodePool: "fake[001-100]", - }, - cfg: &translate.Config{ - Plugin: topology.TopologyBlock, - BlockSizes: []int{2, 4, 8}, - FakeNodePool: "fake[001-100]", - }, - }, - { - name: "Case 5: with invalid partition topology", + name: "Case 3: with invalid partition topology", params: &BaseParams{}, topologies: map[string]*Topology{ "topo1": { @@ -329,7 +264,7 @@ func TestGetTranslateConfig(t *testing.T) { err: "missing partition name", }, { - name: "Case 6: with valid partition topology", + name: "Case 4: with valid partition topology", params: &BaseParams{}, topologies: map[string]*Topology{ "default": { @@ -394,7 +329,13 @@ SwitchName=S3 Nodes=Node[304-306] { name: "Case 2: invalid blocksize", vertex: v, - params: map[string]any{"block_sizes": "bad"}, + params: map[string]any{"blockSizes": "bad"}, + err: "could not decode configuration: 1 error(s) decoding:\n\n* 'blockSizes': source data must be an array or slice, got string", + code: http.StatusBadRequest, + }, + { + name: "Case 3: valid input", + vertex: v, cfg: cfg, }, } diff --git a/pkg/server/http_server_test.go b/pkg/server/http_server_test.go index 59fade72..699e9c58 100644 --- a/pkg/server/http_server_test.go +++ b/pkg/server/http_server_test.go @@ -144,7 +144,7 @@ SwitchName=no-topology Nodes=n-CPU "name": "slurm", "params": { "plugin": "topology/block", - "block_sizes": "8,16,32" + "blockSizes": [8,16,32] } } } diff --git a/pkg/topology/request_test.go b/pkg/topology/request_test.go index 0add4f23..bc15807d 100644 --- a/pkg/topology/request_test.go +++ b/pkg/topology/request_test.go @@ -75,7 +75,7 @@ func TestPayload(t *testing.T) { "name": "slurm", "params": { "plugin": "topology/block", - "block_sizes": "30,120", + "blockSizes": [30,120], "reconfigure": true } }, @@ -112,7 +112,7 @@ func TestPayload(t *testing.T) { Name: "slurm", Params: map[string]any{ KeyPlugin: TopologyBlock, - KeyBlockSizes: "30,120", + KeyBlockSizes: []any{float64(30), float64(120)}, "reconfigure": true, }, }, @@ -140,7 +140,7 @@ func TestPayload(t *testing.T) { Credentials: [accessKeyId:*** secretAccessKey:***] Parameters: [trimTiers:2] Engine: slurm - Parameters: [block_sizes:30,120 plugin:topology/block reconfigure:true] + Parameters: [blockSizes:[30 120] plugin:topology/block reconfigure:true] Nodes: region1: [instance1:node1 instance2:node2 instance3:node3] region2: [instance4:node4 instance5:node5 instance6:node6] `, }, diff --git a/pkg/topology/topology.go b/pkg/topology/topology.go index c2e3c2bb..eaef1043 100644 --- a/pkg/topology/topology.go +++ b/pkg/topology/topology.go @@ -31,7 +31,7 @@ const ( KeyTopologies = "topologies" KeyTopoConfigPath = "topologyConfigPath" KeyTopoConfigmapName = "topologyConfigmapName" - KeyBlockSizes = "block_sizes" + KeyBlockSizes = "blockSizes" KeyTrimTiers = "trimTiers" KeyPlugin = "plugin" diff --git a/pkg/translate/block.go b/pkg/translate/block.go index 163b2dfa..a75df388 100644 --- a/pkg/translate/block.go +++ b/pkg/translate/block.go @@ -12,11 +12,8 @@ import ( "net/http" "strings" - "k8s.io/klog/v2" - "github.com/NVIDIA/topograph/internal/cluset" "github.com/NVIDIA/topograph/internal/httperr" - "github.com/NVIDIA/topograph/pkg/metrics" ) func findMinDomainSize(blocks []*blockInfo) int { @@ -30,54 +27,20 @@ func findMinDomainSize(blocks []*blockInfo) int { return minDomainSize } -// getBlockSize returns blocksize for each possible level. -// Admin provided blocksize is validated and is overriden with default blocksizes if validation fails. -func getBlockSize(blocks []*blockInfo, requestedBlockSizes []int, useFake bool) []int { - // get smallest domain size - var minDomainSize int - if useFake && len(requestedBlockSizes) != 0 { - minDomainSize = requestedBlockSizes[0] - } else { - minDomainSize = findMinDomainSize(blocks) - } - maxnumbs := int(math.Log2(float64(len(blocks)))) - outputbs := []int{} - - // validate requested block sizes +// getBlockSizes returns the BlockSizes list for Slurm's block topology. +// If requestedBlockSizes is non-empty it is returned unchanged. Otherwise the +// result is [D, 2D, 4D, ..., 2^k*D], where D is the smallest block's node +// count and k = floor(log2(N)) for N blocks: the base size matches the +// smallest accelerator domain and each successive level doubles, up to the +// largest power-of-two multiple that fits the block count. +func getBlockSizes(blocks []*blockInfo, requestedBlockSizes []int) []int { if len(requestedBlockSizes) != 0 { - // validate minimal block size - var candidate int - possiblebs := make(map[int]bool) - for i, bs := range requestedBlockSizes { - if i == 0 { - if bs <= 0 || bs > minDomainSize { - metrics.AddValidationError("bad admin blockSize") - klog.Warningf("Overriding admin blockSizes. Planning blockSize %v does not meet criteria, should be > 0 & <= %v.", bs, minDomainSize) - break - } - candidate = bs - // get possible blocksizes with the planningBS - for l := 0; l <= maxnumbs; l++ { - levelblocksize := int(math.Pow(2, float64(l))) * candidate - possiblebs[levelblocksize] = true - } - } - - if _, exists := possiblebs[bs]; !exists { - metrics.AddValidationError("bad admin blockSize") - klog.Warningf("Overriding admin blockSizes. BlockSize %v should follow the pattern (2^n) * %v, with n <= %v", bs, candidate, maxnumbs) - break - } - outputbs = append(outputbs, bs) - } - - if len(outputbs) == len(requestedBlockSizes) { - return outputbs - } + return requestedBlockSizes } - - // reset outputbs - outputbs = []int{minDomainSize} + // get smallest domain size + minDomainSize := findMinDomainSize(blocks) + outputbs := []int{minDomainSize} + maxnumbs := int(math.Log2(float64(len(blocks)))) for i := 1; i <= maxnumbs; i++ { levelblocksize := int(math.Pow(2, float64(i))) * minDomainSize @@ -88,15 +51,7 @@ func getBlockSize(blocks []*blockInfo, requestedBlockSizes []int, useFake bool) } func (nt *NetworkTopology) toBlockTopology(wr io.Writer, skeletonOnly bool) *httperr.Error { - var fnc *fakeNodeConfig - if len(nt.config.FakeNodePool) != 0 { - fnc = getFakeNodeConfig(nt.config.FakeNodePool) - } - - finalBlockSizes := getBlockSize(nt.blocks, nt.config.BlockSizes, fnc != nil) - if fnc != nil { - fnc.baseBlockSize = finalBlockSizes[0] - } + blockSizes := getBlockSizes(nt.blocks, nt.config.BlockSizes) for _, bInfo := range nt.blocks { var comment string @@ -105,13 +60,6 @@ func (nt *NetworkTopology) toBlockTopology(wr io.Writer, skeletonOnly bool) *htt } outputNodeNames := strings.Join(cluset.Compact(bInfo.nodes), ",") - if fnc != nil && len(bInfo.nodes) < fnc.baseBlockSize { - fakeNodeNames, err := fnc.getFreeFakeNodes(fnc.baseBlockSize - len(bInfo.nodes)) - if err != nil { - return httperr.NewError(http.StatusBadGateway, err.Error()) - } - outputNodeNames = fmt.Sprintf("%s,%s", outputNodeNames, fakeNodeNames) - } var err error if skeletonOnly { @@ -124,8 +72,8 @@ func (nt *NetworkTopology) toBlockTopology(wr io.Writer, skeletonOnly bool) *htt } } - bss := make([]string, 0, len(finalBlockSizes)) - for _, bs := range finalBlockSizes { + bss := make([]string, 0, len(blockSizes)) + for _, bs := range blockSizes { bss = append(bss, fmt.Sprintf("%d", bs)) } diff --git a/pkg/translate/block_test.go b/pkg/translate/block_test.go index 6934b8d7..e4fe7840 100644 --- a/pkg/translate/block_test.go +++ b/pkg/translate/block_test.go @@ -22,7 +22,7 @@ func TestBlockTopology(t *testing.T) { err string }{ { - name: "a block without name", + name: "Case 1: a block without name", nt: &NetworkTopology{ config: &Config{ BlockSizes: []int{2}, @@ -41,7 +41,7 @@ func TestBlockTopology(t *testing.T) { }, "\n"), }, { - name: "a block with name", + name: "Case 2: a block with name", nt: &NetworkTopology{ config: &Config{ BlockSizes: []int{2}, @@ -60,54 +60,33 @@ BlockSizes=2 `, }, { - name: "fake nodes added to meet base block size", + name: "Case 3: multiple blocks with mixed settings with blockSizes", nt: &NetworkTopology{ config: &Config{ - BlockSizes: []int{3}, - FakeNodePool: "fake[1-6]", - }, - blocks: []*blockInfo{ - { - id: "block001", - nodes: []string{"n1"}, - }, - { - id: "block002", - nodes: []string{"n2"}, - }, - }, - }, - output: `BlockName=block001 Nodes=n1,fake[1-2] -BlockName=block002 Nodes=n2,fake[3-4] -BlockSizes=3 -`, - }, - { - name: "not enough fake nodes to meet base block size", - nt: &NetworkTopology{ - config: &Config{ - BlockSizes: []int{3}, - FakeNodePool: "fake1", + BlockSizes: []int{2, 4}, }, blocks: []*blockInfo{ { id: "b1", - nodes: []string{"n1"}, + nodes: []string{"n1", "n2"}, }, { id: "b2", - nodes: []string{"n2"}, + name: "block2", + nodes: []string{"n3"}, }, }, }, - err: errNotEnoughFakeNodes.Error(), + output: `BlockName=b1 Nodes=n[1-2] +# b2=block2 +BlockName=b2 Nodes=n3 +BlockSizes=2,4 +`, }, { - name: "multiple blocks with mixed settings", + name: "Case 4: multiple blocks with mixed settings without blockSizes", nt: &NetworkTopology{ - config: &Config{ - BlockSizes: []int{2, 4}, - }, + config: &Config{}, blocks: []*blockInfo{ { id: "b1", @@ -142,16 +121,15 @@ BlockSizes=1,2 } } -func TestGetBlockSize(t *testing.T) { +func TestGetBlockSizes(t *testing.T) { testCases := []struct { name string blocks map[string]int blockSize []int - useFake bool expectedOutput []int }{ { - name: "Case 1: #nodes/block same, #blocks power of 2, admin !provided base block size", + name: "Case 1: #nodes/block same, #blocks power of 2, blockSizes not requested", blocks: map[string]int{ "nvl1": 2, "nvl2": 2, @@ -159,7 +137,7 @@ func TestGetBlockSize(t *testing.T) { expectedOutput: []int{2, 4}, }, { - name: "Case 2: #nodes/block different, #blocks power of 2, admin !provided base block size", + name: "Case 2: #nodes/block different, #blocks power of 2, blockSizes not requested", blocks: map[string]int{ "nvl1": 2, "nvl2": 3, @@ -167,7 +145,7 @@ func TestGetBlockSize(t *testing.T) { expectedOutput: []int{2, 4}, }, { - name: "Case 3: #nodes/block same, #blocks !power of 2, admin !provided base block size", + name: "Case 3: #nodes/block same, #blocks !power of 2, blockSizes not requested", blocks: map[string]int{ "nvl1": 2, "nvl2": 2, @@ -176,7 +154,7 @@ func TestGetBlockSize(t *testing.T) { expectedOutput: []int{2, 4}, }, { - name: "Case 4: #nodes/block same, #blocks power of 2, admin provided base block size", + name: "Case 4: #nodes/block same, #blocks power of 2, blockSizes requested", blocks: map[string]int{ "nvl1": 2, "nvl2": 2, @@ -185,7 +163,7 @@ func TestGetBlockSize(t *testing.T) { expectedOutput: []int{2}, }, { - name: "Case 5: #nodes/block different, #blocks power of 2, admin provided base block size", + name: "Case 5: #nodes/block different, #blocks power of 2, blockSizes requested", blocks: map[string]int{ "nvl1": 2, "nvl2": 3, @@ -194,7 +172,7 @@ func TestGetBlockSize(t *testing.T) { expectedOutput: []int{2}, }, { - name: "Case 6: #nodes/block same, #blocks !power of 2, admin provided base block size", + name: "Case 6: #nodes/block same, #blocks !power of 2, blockSizes requested", blocks: map[string]int{ "nvl1": 2, "nvl2": 2, @@ -204,7 +182,7 @@ func TestGetBlockSize(t *testing.T) { expectedOutput: []int{2}, }, { - name: "Case 7: #nodes/block same, #blocks power of 2, admin provided blocksizes", + name: "Case 7: #nodes/block same, #blocks power of 2, blockSizes requested", blocks: map[string]int{ "nvl1": 3, "nvl2": 3, @@ -214,102 +192,11 @@ func TestGetBlockSize(t *testing.T) { blockSize: []int{3, 6, 12}, expectedOutput: []int{3, 6, 12}, }, - { - name: "Case 8: #nodes/block different, #blocks power of 2, admin provided wrong base blocksize", - blocks: map[string]int{ - "nvl1": 3, - "nvl2": 4, - "nvl3": 3, - "nvl4": 4, - }, - blockSize: []int{4}, - expectedOutput: []int{3, 6, 12}, - }, - { - name: "Case 9: #nodes/block different, #blocks !power of 2, admin provided wrong blocksizes", - blocks: map[string]int{ - "nvl1": 3, - "nvl2": 4, - "nvl3": 3, - }, - blockSize: []int{3, 4}, - expectedOutput: []int{3, 6}, - }, - { - name: "Case 10: #nodes/block same, #blocks power of 2, admin provided larger base blocksize", - blocks: map[string]int{ - "nvl1": 4, - "nvl2": 4, - "nvl3": 4, - "nvl4": 4, - }, - blockSize: []int{10}, - expectedOutput: []int{4, 8, 16}, - }, - { - name: "Case 11: #nodes/block different, #blocks power of 2, admin provided smaller base blocksize", - blocks: map[string]int{ - "nvl1": 3, - "nvl2": 4, - "nvl3": 3, - "nvl4": 4, - }, - blockSize: []int{2}, - expectedOutput: []int{2}, - }, - { - name: "Case 12: with fake nodes, #nodes = base block size, no requested block sizes", - useFake: true, - blocks: map[string]int{ - "nvl1": 18, - "nvl2": 18, - }, - expectedOutput: []int{18, 36}, - }, - { - name: "Case 13: with fake nodes, mixed #nodes, no requested block sizes", - useFake: true, - blocks: map[string]int{ - "nvl1": 12, - "nvl2": 18, - }, - expectedOutput: []int{12, 24}, - }, - { - name: "Case 14: with fake nodes, requested base block size > #nodes", - useFake: true, - blocks: map[string]int{ - "nvl1": 12, - "nvl2": 12, - }, - blockSize: []int{18, 36}, - expectedOutput: []int{18, 36}, - }, - { - name: "Case 15: with fake nodes, requested base block size > #nodes", - useFake: true, - blocks: map[string]int{ - "nvl1": 12, - "nvl2": 12, - }, - blockSize: []int{18}, - expectedOutput: []int{18}, - }, - { - name: "Case 16: with fake nodes, requested base block size < #nodes", - useFake: true, - blocks: map[string]int{ - "nvl1": 18, - "nvl2": 18, - }, - blockSize: []int{15}, - expectedOutput: []int{15}, - }, } for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { - blockSize := getBlockSize(populateBlockInfo(tc.blocks), tc.blockSize, tc.useFake) + blockSize := getBlockSizes(populateBlockInfo(tc.blocks), tc.blockSize) require.Equal(t, tc.expectedOutput, blockSize) }) } diff --git a/pkg/translate/fake.go b/pkg/translate/fake.go deleted file mode 100644 index 2cba1d7f..00000000 --- a/pkg/translate/fake.go +++ /dev/null @@ -1,39 +0,0 @@ -/* - * Copyright 2025 NVIDIA CORPORATION - * SPDX-License-Identifier: Apache-2.0 - */ - -package translate - -import ( - "errors" - "strings" - - "github.com/NVIDIA/topograph/internal/cluset" -) - -var errNotEnoughFakeNodes = errors.New("not enough fake nodes available") - -type fakeNodeConfig struct { - baseBlockSize int - index int - nodes []string -} - -func getFakeNodeConfig(fakeNodeData string) *fakeNodeConfig { - return &fakeNodeConfig{ - nodes: cluset.Expand([]string{fakeNodeData}), - index: 0, - } -} - -// getFreeFakeNodes generates fake nodes names. -func (fnc *fakeNodeConfig) getFreeFakeNodes(count int) (string, error) { - start := fnc.index - end := fnc.index + count - if end > len(fnc.nodes) { - return "", errNotEnoughFakeNodes - } - fnc.index = end - return strings.Join(cluset.Compact(fnc.nodes[start:end]), ","), nil -} diff --git a/pkg/translate/fake_test.go b/pkg/translate/fake_test.go deleted file mode 100644 index 06230b34..00000000 --- a/pkg/translate/fake_test.go +++ /dev/null @@ -1,64 +0,0 @@ -/* - * Copyright 2025 NVIDIA CORPORATION - * SPDX-License-Identifier: Apache-2.0 - */ - -package translate - -import ( - "testing" - - "github.com/stretchr/testify/require" -) - -func TestFakeNodeConfig(t *testing.T) { - testCases := []struct { - name string - input string - counts []int - outputs []any - }{ - { - name: "Case 1: not enough fake nodes", - input: "fake[001-010]", - counts: []int{11}, - outputs: []any{errNotEnoughFakeNodes}, - }, - { - name: "Case 2: exact fake nodes in one step", - input: "fake[12-17]", - counts: []int{6}, - outputs: []any{"fake[12-17]"}, - }, - { - name: "Case 3: exact fake nodes in two steps", - input: "fake[12-17]", - counts: []int{4, 2}, - outputs: []any{"fake[12-15]", "fake[16-17]"}, - }, - { - name: "Case 4: not enough fake nodes", - input: "fake[1-10]", - counts: []int{4, 2, 1, 4}, - outputs: []any{"fake[1-4]", "fake[5-6]", "fake7", errNotEnoughFakeNodes}, - }, - } - - for _, tc := range testCases { - t.Run(tc.name, func(t *testing.T) { - fnc := getFakeNodeConfig(tc.input) - - for i, count := range tc.counts { - output := tc.outputs[i] - nodes, err := fnc.getFreeFakeNodes(count) - - if e, ok := output.(error); ok { - require.EqualError(t, err, e.Error()) - } else { - require.NoError(t, err) - require.Equal(t, output, nodes) - } - } - }) - } -} diff --git a/pkg/translate/topology.go b/pkg/translate/topology.go index a591358f..ac4d13eb 100644 --- a/pkg/translate/topology.go +++ b/pkg/translate/topology.go @@ -19,10 +19,9 @@ import ( ) type Config struct { - Plugin string // topology plugin (cluster-wide) - BlockSizes []int - FakeNodePool string - Topologies map[string]*TopologySpec // per-partiton topology settings + Plugin string // topology plugin (cluster-wide) + BlockSizes []int + Topologies map[string]*TopologySpec // per-partiton topology settings } // TopologySpec define topology for a partition diff --git a/pkg/translate/topology_test.go b/pkg/translate/topology_test.go index 65b66e05..ddeca9e2 100644 --- a/pkg/translate/topology_test.go +++ b/pkg/translate/topology_test.go @@ -18,7 +18,6 @@ package translate import ( "bytes" - "fmt" "testing" "github.com/stretchr/testify/require" @@ -72,12 +71,6 @@ SwitchName=switch.2.2 Switches=switch.1.2 SwitchName=switch.1.1 Nodes=node-1 # switch.1.2=local-block-2 SwitchName=switch.1.2 Nodes=node-2 -` - - testBlockConfigFakeNodes = `BlockName=B2 Nodes=Node[104-105],fake100 -BlockName=B1 Nodes=Node202,fake[101-102] -BlockName=B3 Nodes=Node205,fake[103-104] -BlockSizes=3 ` ) @@ -282,34 +275,6 @@ func TestToBlockDFSIBTopology(t *testing.T) { require.Equal(t, testBlockConfigDFS, buf.String()) } -func TestBlockFakeNodes(t *testing.T) { - // Test Fake node config - fakeNodeData := "fake[100-998]" - fnc := getFakeNodeConfig(fakeNodeData) - - expectedFnc := &fakeNodeConfig{ - nodes: []string{}, - index: 0, - } - for i := 100; i <= 998; i++ { - expectedFnc.nodes = append(expectedFnc.nodes, fmt.Sprintf("fake%d", i)) - } - require.Equal(t, expectedFnc, fnc) - - // Test Fake node output - v, _ := getBlockWithDFSIBTestSet() - cfg := &Config{ - Plugin: topology.TopologyBlock, - FakeNodePool: fakeNodeData, - BlockSizes: []int{3}, - } - nt, _ := NewNetworkTopology(v, cfg) - buf := &bytes.Buffer{} - err := nt.Generate(buf) - require.Nil(t, err) - require.Equal(t, testBlockConfigFakeNodes, buf.String()) -} - func TestToSlurmNameShortener(t *testing.T) { v := &topology.Vertex{ Vertices: map[string]*topology.Vertex{ diff --git a/pkg/translate/yaml.go b/pkg/translate/yaml.go index 126a3740..cea1bc58 100644 --- a/pkg/translate/yaml.go +++ b/pkg/translate/yaml.go @@ -191,7 +191,7 @@ func (nt *NetworkTopology) getBlockTopologyUnit(topoName string, topoSpec *Topol } tu.Block = &BlockTopo{ - BlockSizes: getBlockSize(bInfos, topoSpec.BlockSizes, false), + BlockSizes: getBlockSizes(bInfos, topoSpec.BlockSizes), Blocks: blocks, parents: parents, } diff --git a/tests/payloads/test-aws-sim-block.json b/tests/payloads/test-aws-sim-block.json index 69bc419a..1761781d 100644 --- a/tests/payloads/test-aws-sim-block.json +++ b/tests/payloads/test-aws-sim-block.json @@ -9,7 +9,7 @@ "name": "slurm", "params": { "plugin": "topology/block", - "block_sizes": "2,4", + "blockSizes": [2,4], "topologyConfigPath": "topology.conf" } } diff --git a/tests/payloads/test-aws-sim-large-block-fake.json b/tests/payloads/test-aws-sim-large-block-fake.json deleted file mode 100644 index 7caf4f7d..00000000 --- a/tests/payloads/test-aws-sim-large-block-fake.json +++ /dev/null @@ -1,18 +0,0 @@ -{ - "provider": { - "name": "aws-sim", - "params": { - "modelFileName": "tests/models/nvl72.yaml" - } - }, - "engine": { - "name": "slurm", - "params": { - "plugin": "topology/block", - "topologyConfigPath": "topology_block_fake.conf", - "fakeNodesEnabled": true, - "block_sizes": "16,32", - "fake_node_pool": "fake[100-200]" - } - } -} \ No newline at end of file diff --git a/tests/payloads/test-gcp-sim-block.json b/tests/payloads/test-gcp-sim-block.json index c6ba70e7..668074b3 100644 --- a/tests/payloads/test-gcp-sim-block.json +++ b/tests/payloads/test-gcp-sim-block.json @@ -9,7 +9,7 @@ "name": "slurm", "params": { "plugin": "topology/block", - "block_sizes": "2,4", + "blockSizes": [2,4], "topologyConfigPath": "topology.conf" } } diff --git a/tests/payloads/test-gcp-sim-large-block.json b/tests/payloads/test-gcp-sim-large-block.json index c443fab0..8fc0783e 100644 --- a/tests/payloads/test-gcp-sim-large-block.json +++ b/tests/payloads/test-gcp-sim-large-block.json @@ -9,7 +9,7 @@ "name": "slurm", "params": { "plugin": "topology/block", - "block_sizes": "8,16,32", + "blockSizes": [8,16,32], "topologyConfigPath": "topology.conf" } } diff --git a/tests/payloads/test-oci-sim-block.json b/tests/payloads/test-oci-sim-block.json index 088974b7..9ce37961 100644 --- a/tests/payloads/test-oci-sim-block.json +++ b/tests/payloads/test-oci-sim-block.json @@ -9,7 +9,7 @@ "name": "slurm", "params": { "plugin": "topology/block", - "block_sizes": "2,4", + "blockSizes": [2,4], "topologyConfigPath": "topology.conf" } } diff --git a/tests/payloads/test-oci-sim-large-block-fake.json b/tests/payloads/test-oci-sim-large-block-fake.json deleted file mode 100644 index 5645b1cd..00000000 --- a/tests/payloads/test-oci-sim-large-block-fake.json +++ /dev/null @@ -1,18 +0,0 @@ -{ - "provider": { - "name": "oci-sim", - "params": { - "modelFileName": "tests/models/large.yaml" - } - }, - "engine": { - "name": "slurm", - "params": { - "plugin": "topology/block", - "topologyConfigPath": "topology_block_fake.conf", - "fakeNodesEnabled": true, - "fake_node_pool": "fake[100-1000]" - } - } - } - \ No newline at end of file diff --git a/tests/payloads/test-oci-sim-large-block.json b/tests/payloads/test-oci-sim-large-block.json index ce5ae2b9..6448c2ca 100644 --- a/tests/payloads/test-oci-sim-large-block.json +++ b/tests/payloads/test-oci-sim-large-block.json @@ -9,7 +9,7 @@ "name": "slurm", "params": { "plugin": "topology/block", - "block_sizes": "8,16,32", + "blockSizes": [8,16,32], "topologyConfigPath": "topology.conf" } } diff --git a/tests/payloads/test-toposim-block-fake.json b/tests/payloads/test-toposim-block-fake.json deleted file mode 100644 index e98cfbfb..00000000 --- a/tests/payloads/test-toposim-block-fake.json +++ /dev/null @@ -1,18 +0,0 @@ -{ - "provider": { - "name": "test", - "params": { - "modelFileName": "nvl72.yaml" - } - }, - "engine": { - "name": "slurm", - "params": { - "plugin": "topology/block", - "fakeNodesEnabled" : "true", - "fake_node_pool": "fake[1-80]", - "topologyConfigPath": "topology_block_fake.conf" - } - } - } - \ No newline at end of file diff --git a/tests/payloads/test-toposim-block.json b/tests/payloads/test-toposim-block.json index 730e4ded..79ac5bd3 100644 --- a/tests/payloads/test-toposim-block.json +++ b/tests/payloads/test-toposim-block.json @@ -9,7 +9,7 @@ "name": "slurm", "params": { "plugin": "topology/block", - "block_sizes": "2,4" + "blockSizes": [2,4] } } }