-
Notifications
You must be signed in to change notification settings - Fork 36
libmem: Add unit tests for zones #657
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
ozhuraki
wants to merge
1
commit into
containers:main
Choose a base branch
from
ozhuraki:test-zones
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,185 @@ | ||
| // Copyright 2026 Intel Corporation. All Rights Reserved. | ||
| // | ||
| // Licensed under the Apache License, Version 2.0 (the "License"); | ||
| // you may not use this file except in compliance with the License. | ||
| // You may obtain a copy of the License at | ||
| // | ||
| // http://www.apache.org/licenses/LICENSE-2.0 | ||
| // | ||
| // Unless required by applicable law or agreed to in writing, software | ||
| // distributed under the License is distributed on an "AS IS" BASIS, | ||
| // WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
| // See the License for the specific language governing permissions and | ||
| // limitations under the License. | ||
|
|
||
| package libmem_test | ||
|
|
||
| import ( | ||
| "testing" | ||
|
|
||
| "github.com/stretchr/testify/require" | ||
|
|
||
| . "github.com/containers/nri-plugins/pkg/resmgr/lib/memory" | ||
| ) | ||
|
|
||
| // newZoneTestAllocator creates a simple 2-DRAM-node allocator for zone tests. | ||
| func newZoneTestAllocator(t *testing.T) *Allocator { | ||
| t.Helper() | ||
| setup := &testSetup{ | ||
| description: "2 DRAM nodes for zone tests", | ||
| types: []Type{TypeDRAM, TypeDRAM}, | ||
| capacities: []int64{8, 8}, | ||
| movability: []bool{normal, normal}, | ||
| closeCPUs: [][]int{{0, 1}, {2, 3}}, | ||
| distances: [][]int{ | ||
| {10, 21}, | ||
| {21, 10}, | ||
| }, | ||
| } | ||
| a, err := NewAllocator(WithNodes(setup.nodes(t))) | ||
| require.NoError(t, err) | ||
| require.NotNil(t, a) | ||
| return a | ||
| } | ||
|
|
||
| // newFourNodeDRAMAllocator creates a 4-DRAM-node allocator (4 bytes/node) for zone tests. | ||
| // Node distances: 0<->2 and 1<->3 are close (11), cross pairs are far (21). | ||
| func newFourNodeDRAMAllocator(t *testing.T) *Allocator { | ||
| t.Helper() | ||
| setup := &testSetup{ | ||
| description: "4 DRAM nodes for zone tests", | ||
| types: []Type{TypeDRAM, TypeDRAM, TypeDRAM, TypeDRAM}, | ||
| capacities: []int64{4, 4, 4, 4}, | ||
| movability: []bool{normal, normal, normal, normal}, | ||
| closeCPUs: [][]int{{0, 1}, {2, 3}, {4, 5}, {6, 7}}, | ||
| distances: [][]int{ | ||
| {10, 21, 11, 21}, | ||
| {21, 10, 21, 11}, | ||
| {11, 21, 10, 21}, | ||
| {21, 11, 21, 10}, | ||
| }, | ||
| } | ||
| a, err := NewAllocator(WithNodes(setup.nodes(t))) | ||
| require.NoError(t, err) | ||
| require.NotNil(t, a) | ||
| return a | ||
| } | ||
|
|
||
| // TestZoneCapacity verifies that ZoneCapacity returns the total memory of the | ||
| // nodes in the requested zone. | ||
| func TestZoneCapacity(t *testing.T) { | ||
| a := newZoneTestAllocator(t) | ||
|
|
||
| require.Equal(t, int64(8), a.ZoneCapacity(NewNodeMask(0))) | ||
| require.Equal(t, int64(8), a.ZoneCapacity(NewNodeMask(1))) | ||
| require.Equal(t, int64(16), a.ZoneCapacity(NewNodeMask(0, 1))) | ||
| } | ||
|
|
||
| // TestZoneUsageAndFree allocates memory into a zone and checks that ZoneUsage | ||
| // and ZoneFree reflect the allocation correctly. | ||
| func TestZoneUsageAndFree(t *testing.T) { | ||
| a := newZoneTestAllocator(t) | ||
|
|
||
| zone := NewNodeMask(0, 1) | ||
| require.Equal(t, int64(0), a.ZoneUsage(zone), "usage before allocation") | ||
| require.Equal(t, int64(16), a.ZoneFree(zone), "free before allocation") | ||
|
|
||
| _, _, err := a.Allocate(Container("c1", "test", "burstable", 6, NewNodeMask(0))) | ||
| require.NoError(t, err) | ||
|
|
||
| require.Equal(t, int64(6), a.ZoneUsage(zone), "usage after 6-byte allocation") | ||
| require.Equal(t, int64(10), a.ZoneFree(zone), "free after 6-byte allocation") | ||
| } | ||
|
|
||
| // TestZoneNumUsers verifies that ZoneNumUsers counts requests assigned to a zone. | ||
| func TestZoneNumUsers(t *testing.T) { | ||
| a := newZoneTestAllocator(t) | ||
|
|
||
| zone := NewNodeMask(0) | ||
| require.Equal(t, 0, a.ZoneNumUsers(zone), "no users before allocation") | ||
|
|
||
| _, _, err := a.Allocate(Container("c1", "test", "burstable", 2, NewNodeMask(0))) | ||
| require.NoError(t, err) | ||
| require.Equal(t, 1, a.ZoneNumUsers(zone), "one user after first allocation") | ||
|
|
||
| _, _, err = a.Allocate(Container("c2", "test", "burstable", 2, NewNodeMask(0))) | ||
| require.NoError(t, err) | ||
| require.Equal(t, 2, a.ZoneNumUsers(zone), "two users after second allocation") | ||
| } | ||
|
|
||
| // TestZonesSort verifies SortZones with nil/non-nil filters and with a sorter. | ||
| func TestZonesSort(t *testing.T) { | ||
| a := newFourNodeDRAMAllocator(t) | ||
|
|
||
| zone0 := NewNodeMask(0) | ||
| zone1 := NewNodeMask(1) | ||
|
|
||
| // Populate two distinct zones: zone0 gets two requests, zone1 gets one. | ||
| _, _, err := a.Allocate(Container("c1", "test", "burstable", 2, zone0)) | ||
| require.NoError(t, err) | ||
| _, _, err = a.Allocate(Container("c2", "test", "burstable", 2, zone0)) | ||
| require.NoError(t, err) | ||
| _, _, err = a.Allocate(Container("c3", "test", "burstable", 2, zone1)) | ||
| require.NoError(t, err) | ||
|
|
||
| // Positive: nil filter returns all created zones. | ||
| all := a.SortZones(nil) | ||
| require.Equal(t, 2, len(all), "nil filter should return all zones") | ||
| require.ElementsMatch(t, []NodeMask{zone0, zone1}, all, "nil filter should include zone0 and zone1") | ||
|
|
||
| // Positive: filter restricts to a single zone. | ||
| only0 := a.SortZones(func(z NodeMask) bool { return z == zone0 }) | ||
| require.Equal(t, []NodeMask{zone0}, only0, "filter should return only zone0") | ||
|
|
||
| // Positive: sorter orders zone with more users first. | ||
| sorted := a.SortZones(nil, a.ZonesByUsersSubzonesFirst) | ||
| require.Equal(t, zone0, sorted[0], "zone0 (2 users) should sort before zone1 (1 user)") | ||
| require.Equal(t, zone1, sorted[1], "zone1 (1 user) should sort after zone0 (2 users)") | ||
|
|
||
| // Negative: filter that excludes everything returns an empty slice. | ||
| none := a.SortZones(func(NodeMask) bool { return false }) | ||
| require.Empty(t, none, "filter rejecting all zones should return empty slice") | ||
| } | ||
|
|
||
| // TestZonesByUsersSubzonesFirst verifies the comparator used by SortZones. | ||
| func TestZonesByUsersSubzonesFirst(t *testing.T) { | ||
| a := newFourNodeDRAMAllocator(t) | ||
|
|
||
| zone0 := NewNodeMask(0) | ||
| zone1 := NewNodeMask(1) | ||
|
|
||
| // Populate zones: zone0 gets two requests, zone1 gets one. | ||
| _, _, err := a.Allocate(Container("c1", "test", "burstable", 2, zone0)) | ||
| require.NoError(t, err) | ||
| _, _, err = a.Allocate(Container("c2", "test", "burstable", 2, zone0)) | ||
| require.NoError(t, err) | ||
| _, _, err = a.Allocate(Container("c3", "test", "burstable", 2, zone1)) | ||
| require.NoError(t, err) | ||
|
|
||
| // Positive: zone with more users sorts before zone with fewer users. | ||
| // diff = len(z2.users) - len(z1.users) = 1 - 2 = -1 -> zone0 < zone1 | ||
| require.Negative(t, a.ZonesByUsersSubzonesFirst(zone0, zone1), | ||
| "zone0 (2 users) should sort before zone1 (1 user)") | ||
|
|
||
| // Positive: symmetric -- reversed argument order gives a positive result. | ||
| require.Positive(t, a.ZonesByUsersSubzonesFirst(zone1, zone0), | ||
| "zone1 (1 user) should sort after zone0 (2 users)") | ||
|
|
||
| // Positive: a subzone sorts before its superset when user counts are tied. | ||
| // zone0 is a subset of zone{0,1}: (zone0 & zone{0,1}) == zone0 -> returns -1 | ||
| zone01 := NewNodeMask(0, 1) | ||
| require.Negative(t, a.ZonesByUsersSubzonesFirst(zone0, zone01), | ||
| "subzone zone0 should sort before superset zone{0,1}") | ||
|
|
||
| // Positive: symmetric -- superset sorts after subzone -> positive result. | ||
| require.Positive(t, a.ZonesByUsersSubzonesFirst(zone01, zone0), | ||
| "superset zone{0,1} should sort after subzone zone0") | ||
|
|
||
| // Negative: zones not present in the allocator (nil entries) fall back to | ||
| // subset/size logic. Two disjoint phantom zones with the same size sort by | ||
| // their numeric NodeMask value: higher value first (zone2 - zone1 > 0). | ||
| phantom2 := NewNodeMask(2) | ||
| phantom3 := NewNodeMask(3) | ||
| require.Positive(t, a.ZonesByUsersSubzonesFirst(phantom2, phantom3), | ||
| "for disjoint equal-size zones, higher NodeMask value should sort first") | ||
| } | ||
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In reality, not all zones are of equal size, and in unreality NodeCapacity() could have a bug that would assume all sizes to be equal.
Using different capacities (and why not different types, movabilities, different closeCPUs set sizes on different nodes as well), these tests could cover cases that are more difficult with e2e tests where we have to actually simulate such platforms.
Unit tests for libmem could actually stress test the allocator very thoroughly, which I would find quite useful. I mean using a fuzzer that would keep track on the expected state of allocators and zones, too. @ozhuraki, would you be interested in going to that direction? @klihub, do you think it would make sense?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@askervin
Thanks. Yes, I would be interested. I added a very basic one, FuzzZoneUsageAndFree().
I would like to experiment a bit, i.e. to make it more developer-friendly (preferably to parameterize it, add a bit of ASCII visualization, and add record/replay for error cases and document). I could cut it off and add it separately, WDYT?
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@askervin @ozhuraki Since we are talking about fuzzing, I think what would make sense is to have randomized but reproducible fuzzing tests. IOW, write the test so that it can take an externally provided test seed (for instance from an environment variable), and generate (and print) a random one if none is given.
Additionally, it would make sense to write actual tests so that we have tests both with just sequences allocation from 0 till maxed out mem, and sequences of both allocations and releases, with allocations fuzzed wrt. both container type (IOW relative priority) and allocation size, and releases fuzzed wrt. which existing allocation we release, instead of these being fixed/hardcoded.
Also IMO either some capping of max. allocation size or other way of enforcing minimum number of steps till memory is exhausted could make sense, so that with a randomly generated seed we don't degenerate the fuzzing test sequence in reality to just a few steps which already exhaust all the memory, which is then not an interesting case for fuzzing.
With such a setup we should be able to achieve good fuzzing coverage over time and have reproducibility for any bugs we might trigger and uncover.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Cool!
For more complex fuzzing, what I had in mind was a little bit of a research if we could apply a stateful fuzzer like:
https://github.com/askervin/gofmbt
The idea is that the fuzzer understands which allocations are valid, which are invalid. It's able to test them both and cover them in different scenarios (that is states) and orders (that is different subsequences of inputs).
This fuzzer is documented in
https://github.com/askervin/gofmbt/blob/main/gofmbt/doc.go
and there is an example that tests a music player: it knows when the player is paused or playing, and which song in the playlist it is playing, and generates inputs based on that, aiming to cover more and more untested combinations:
https://github.com/askervin/gofmbt/blob/main/examples/player/main.go
Take your time to familiarize yourself with it, tell me if it looks something that could be useful in this scenario, and let's decide whether or not to go with it. I haven't tested myself how well copilot picks up the idea and supports model creation...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@klihub, I'm optimistic that above requirements (all very sensible and definitely needed) could be satisfied with this approach. Should there be new features needed, which I doubt, they'd be easily accepted into the project.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@klihub @askervin Thanks for the input! Let me check what practical approaches (including gofmbt) would make sense on the allocator first, and then let's think where else such might help. I will cut the fuzzying off for now, re-work and add it separately.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@askervin I am totally fine with using gofmbt for more intelligent fuzzing.