Skip to content

Commit

Permalink
plugins: fix nomadTopologyToProto panic on systems that don't support…
Browse files Browse the repository at this point in the history
… NUMA (#23399)

After changes introduced in #23284 we no longer need to make a if
!st.SupportsNUMA() check in the GetNodes() topology method. In fact this check
will now cause panic in nomadTopologyToProto method on systems that don't
support NUMA.
  • Loading branch information
pkazmierczak authored Jul 9, 2024
1 parent 6560a0c commit 7772711
Show file tree
Hide file tree
Showing 4 changed files with 18 additions and 4 deletions.
3 changes: 3 additions & 0 deletions .changelog/23399.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
```release-note:bug
plugins: Fix panic on systems that don't support NUMA
```
3 changes: 2 additions & 1 deletion .github/workflows/test-windows.yml
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,8 @@ jobs:
github.com/hashicorp/nomad/drivers/docker \
github.com/hashicorp/nomad/client/lib/fifo \
github.com/hashicorp/nomad/client/logmon \
github.com/hashicorp/nomad/client/allocrunner/taskrunner/template
github.com/hashicorp/nomad/client/allocrunner/taskrunner/template \
github.com/hashicorp/nomad/plugins/base
- uses: actions/upload-artifact@65462800fd760344b1a7b4382951275a0abb4808 # v4.3.3
with:
name: results.xml
Expand Down
3 changes: 0 additions & 3 deletions client/lib/numalib/topology.go
Original file line number Diff line number Diff line change
Expand Up @@ -164,9 +164,6 @@ func (st *Topology) SupportsNUMA() bool {

// GetNodes returns the set of NUMA Node IDs.
func (st *Topology) GetNodes() *idset.Set[hw.NodeID] {
if !st.SupportsNUMA() {
return nil
}
if st.nodeIDs.Empty() {
st.nodeIDs = idset.From[hw.NodeID](st.Nodes)
}
Expand Down
13 changes: 13 additions & 0 deletions plugins/base/base_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,19 @@ func Test_nomadTopologyToProto(t *testing.T) {
OverrideTotalCompute: 90_000,
OverrideWitholdCompute: 2000,
}, pb)

// make sure we don't panic in case of empty nodes, vide
// https://github.com/hashicorp/nomad/issues/23385
top2 := &numalib.Topology{}

pb2 := nomadTopologyToProto(top2)
must.Eq(t, &proto.ClientTopology{
NodeIds: []uint32{},
Distances: &proto.ClientTopologySLIT{Dimension: 0, Values: []uint32{}},
Cores: nil,
OverrideTotalCompute: 0,
OverrideWitholdCompute: 0,
}, pb2)
}

func Test_nomadTopologyFromProto(t *testing.T) {
Expand Down

0 comments on commit 7772711

Please sign in to comment.