Skip to content

Commit 101c12d

Browse files
Praveenrajmaniwlan0
Praveenrajmani
authored and
wlan0
committed
Introduce a random selection of filtered drives when scheduling
- Run a random index generator over the filter drives which have the same (max)total capacity. - This change to reduce the conflicts while scheduling volumes to drives Signed-off-by: Praveenrajmani <praveen@minio.io>
1 parent c66dabe commit 101c12d

File tree

3 files changed

+196
-45
lines changed

3 files changed

+196
-45
lines changed

‎pkg/controller/controller.go

+1
Original file line numberDiff line numberDiff line change
@@ -255,6 +255,7 @@ func (c *ControllerServer) CreateVolume(ctx context.Context, req *csi.CreateVolu
255255
finalizers = append(finalizers, finalizer)
256256
drive.SetFinalizers(finalizers)
257257

258+
glog.Infof("Reserving DirectCSI drive: (Name: %s, NodeName: %s)", drive.Name, drive.Status.NodeName)
258259
if _, err := dclient.Update(ctx, drive, metav1.UpdateOptions{
259260
TypeMeta: utils.DirectCSIDriveTypeMeta(strings.Join([]string{directcsi.Group, directcsi.Version}, "/")),
260261
}); err != nil {

‎pkg/controller/controller_test.go

+146-32
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,15 @@ const (
4242
mb30 = 30 * MB
4343
)
4444

45-
func TestSelectDriveByTopology(t1 *testing.T) {
45+
func TestSelectDrivesByTopology(t1 *testing.T) {
46+
47+
getDriveNameSet := func(drives []directcsi.DirectCSIDrive) []string {
48+
driveNames := []string{}
49+
for _, drive := range drives {
50+
driveNames = append(driveNames, drive.Name)
51+
}
52+
return driveNames
53+
}
4654

4755
testDriveSet := []directcsi.DirectCSIDrive{
4856
{
@@ -53,6 +61,14 @@ func TestSelectDriveByTopology(t1 *testing.T) {
5361
Topology: map[string]string{"node": "N1", "rack": "RK1", "zone": "Z1", "region": "R1"},
5462
},
5563
},
64+
{
65+
ObjectMeta: metav1.ObjectMeta{
66+
Name: "drive11",
67+
},
68+
Status: directcsi.DirectCSIDriveStatus{
69+
Topology: map[string]string{"node": "N1", "rack": "RK1", "zone": "Z1", "region": "R1"},
70+
},
71+
},
5672
{
5773
ObjectMeta: metav1.ObjectMeta{
5874
Name: "drive2",
@@ -61,6 +77,14 @@ func TestSelectDriveByTopology(t1 *testing.T) {
6177
Topology: map[string]string{"node": "N2", "rack": "RK2", "zone": "Z2", "region": "R2"},
6278
},
6379
},
80+
{
81+
ObjectMeta: metav1.ObjectMeta{
82+
Name: "drive22",
83+
},
84+
Status: directcsi.DirectCSIDriveStatus{
85+
Topology: map[string]string{"node": "N2", "rack": "RK2", "zone": "Z2", "region": "R2"},
86+
},
87+
},
6488
{
6589
ObjectMeta: metav1.ObjectMeta{
6690
Name: "drive3",
@@ -72,56 +96,56 @@ func TestSelectDriveByTopology(t1 *testing.T) {
7296
}
7397

7498
testCases := []struct {
75-
name string
76-
topologyRequest *csi.Topology
77-
errExpected bool
78-
selectedDriveName string
99+
name string
100+
topologyRequest *csi.Topology
101+
errExpected bool
102+
selectedDriveNames []string
79103
}{
80104
{
81-
name: "test1",
82-
topologyRequest: &csi.Topology{Segments: map[string]string{"node": "N2", "rack": "RK2", "zone": "Z2", "region": "R2"}},
83-
errExpected: false,
84-
selectedDriveName: "drive2",
105+
name: "test1",
106+
topologyRequest: &csi.Topology{Segments: map[string]string{"node": "N2", "rack": "RK2", "zone": "Z2", "region": "R2"}},
107+
errExpected: false,
108+
selectedDriveNames: []string{"drive2", "drive22"},
85109
},
86110
{
87-
name: "test2",
88-
topologyRequest: &csi.Topology{Segments: map[string]string{"node": "N3", "rack": "RK3", "zone": "Z3", "region": "R3"}},
89-
errExpected: false,
90-
selectedDriveName: "drive3",
111+
name: "test2",
112+
topologyRequest: &csi.Topology{Segments: map[string]string{"node": "N3", "rack": "RK3", "zone": "Z3", "region": "R3"}},
113+
errExpected: false,
114+
selectedDriveNames: []string{"drive3"},
91115
},
92116
{
93-
name: "test3",
94-
topologyRequest: &csi.Topology{Segments: map[string]string{"node": "N4", "rack": "RK2", "zone": "Z4", "region": "R2"}},
95-
errExpected: true,
96-
selectedDriveName: "",
117+
name: "test3",
118+
topologyRequest: &csi.Topology{Segments: map[string]string{"node": "N4", "rack": "RK2", "zone": "Z4", "region": "R2"}},
119+
errExpected: true,
120+
selectedDriveNames: []string{},
97121
},
98122
{
99-
name: "test4",
100-
topologyRequest: &csi.Topology{Segments: map[string]string{"node": "N3", "rack": "RK3"}},
101-
errExpected: false,
102-
selectedDriveName: "drive3",
123+
name: "test4",
124+
topologyRequest: &csi.Topology{Segments: map[string]string{"node": "N3", "rack": "RK3"}},
125+
errExpected: false,
126+
selectedDriveNames: []string{"drive3"},
103127
},
104128
{
105-
name: "test5",
106-
topologyRequest: &csi.Topology{Segments: map[string]string{"node": "N1", "rack": "RK5"}},
107-
errExpected: true,
108-
selectedDriveName: "",
129+
name: "test5",
130+
topologyRequest: &csi.Topology{Segments: map[string]string{"node": "N1", "rack": "RK5"}},
131+
errExpected: true,
132+
selectedDriveNames: []string{},
109133
},
110134
{
111-
name: "test5",
112-
topologyRequest: &csi.Topology{Segments: map[string]string{"node": "N1"}},
113-
errExpected: false,
114-
selectedDriveName: "drive1",
135+
name: "test5",
136+
topologyRequest: &csi.Topology{Segments: map[string]string{"node": "N1"}},
137+
errExpected: false,
138+
selectedDriveNames: []string{"drive1", "drive11"},
115139
},
116140
}
117141

118142
for _, tt := range testCases {
119143
t1.Run(tt.name, func(t1 *testing.T) {
120-
selectedDrive, err := selectDriveByTopology(tt.topologyRequest, testDriveSet)
144+
selectedDrives, err := selectDrivesByTopology(tt.topologyRequest, testDriveSet)
121145
if tt.errExpected && err == nil {
122146
t1.Fatalf("Test case name %s: Expected error but succeeded", tt.name)
123-
} else if selectedDrive.Name != tt.selectedDriveName {
124-
t1.Errorf("Test case name %s: Expected drive name = %s, got %v", tt.name, tt.selectedDriveName, selectedDrive.Name)
147+
} else if !reflect.DeepEqual(getDriveNameSet(selectedDrives), tt.selectedDriveNames) {
148+
t1.Errorf("Test case name %s: Expected drive names = %s, got %v", tt.name, tt.selectedDriveNames, getDriveNameSet(selectedDrives))
125149
}
126150
})
127151
}
@@ -916,3 +940,93 @@ func TestCreateVolume(t *testing.T) {
916940
}
917941
}
918942
}
943+
944+
func TestSelectDriveByFreeCapacity(t1 *testing.T) {
945+
testCases := []struct {
946+
name string
947+
driveList []directcsi.DirectCSIDrive
948+
expectedDriveNames []string
949+
}{
950+
{
951+
name: "test1",
952+
driveList: []directcsi.DirectCSIDrive{
953+
{
954+
ObjectMeta: metav1.ObjectMeta{
955+
Name: "drive1",
956+
},
957+
Status: directcsi.DirectCSIDriveStatus{
958+
FreeCapacity: 1000,
959+
},
960+
},
961+
{
962+
ObjectMeta: metav1.ObjectMeta{
963+
Name: "drive2",
964+
},
965+
Status: directcsi.DirectCSIDriveStatus{
966+
FreeCapacity: 2000,
967+
},
968+
},
969+
{
970+
ObjectMeta: metav1.ObjectMeta{
971+
Name: "drive3",
972+
},
973+
Status: directcsi.DirectCSIDriveStatus{
974+
FreeCapacity: 3000,
975+
},
976+
},
977+
},
978+
expectedDriveNames: []string{"drive3"},
979+
},
980+
{
981+
name: "test2",
982+
driveList: []directcsi.DirectCSIDrive{
983+
{
984+
ObjectMeta: metav1.ObjectMeta{
985+
Name: "drive1",
986+
},
987+
Status: directcsi.DirectCSIDriveStatus{
988+
FreeCapacity: 4000,
989+
},
990+
},
991+
{
992+
ObjectMeta: metav1.ObjectMeta{
993+
Name: "drive2",
994+
},
995+
Status: directcsi.DirectCSIDriveStatus{
996+
FreeCapacity: 4000,
997+
},
998+
},
999+
{
1000+
ObjectMeta: metav1.ObjectMeta{
1001+
Name: "drive3",
1002+
},
1003+
Status: directcsi.DirectCSIDriveStatus{
1004+
FreeCapacity: 3000,
1005+
},
1006+
},
1007+
},
1008+
expectedDriveNames: []string{"drive1", "drive2"},
1009+
},
1010+
}
1011+
1012+
checkDriveName := func(expectedDriveNames []string, driveName string) bool {
1013+
for _, edName := range expectedDriveNames {
1014+
if edName == driveName {
1015+
return true
1016+
}
1017+
}
1018+
return false
1019+
}
1020+
1021+
for _, tt := range testCases {
1022+
t1.Run(tt.name, func(t1 *testing.T) {
1023+
selectedDrive, err := selectDriveByFreeCapacity(tt.driveList)
1024+
if err != nil {
1025+
t1.Fatalf("Text case name: %s: Error: %v", tt.name, err)
1026+
}
1027+
if !checkDriveName(tt.expectedDriveNames, selectedDrive.Name) {
1028+
t1.Errorf("Test case name %s: Unexpected drive selected. Expected one among %v but got %s", tt.name, tt.expectedDriveNames, selectedDrive.Name)
1029+
}
1030+
})
1031+
}
1032+
}

‎pkg/controller/utils.go

+49-13
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,8 @@
1717
package controller
1818

1919
import (
20+
"crypto/rand"
21+
"math/big"
2022
"sort"
2123

2224
directcsi "github.com/minio/direct-csi/pkg/apis/direct.csi.min.io/v1beta1"
@@ -136,41 +138,75 @@ func FilterDrivesByTopologyRequirements(volReq *csi.CreateVolumeRequest, csiDriv
136138
preferredXs := tReq.GetPreferred()
137139
requisiteXs := tReq.GetRequisite()
138140

139-
// Sort the drives by free capacity [Descending]
140-
sort.SliceStable(csiDrives, func(i, j int) bool {
141-
return csiDrives[i].Status.FreeCapacity > csiDrives[j].Status.FreeCapacity
142-
})
143-
144141
// Try to fullfill the preferred topology request, If not, fallback to requisite list.
145142
// Ref: https://godoc.org/github.com/container-storage-interface/spec/lib/go/csi#TopologyRequirement
146143
for _, preferredTop := range preferredXs {
147-
if selectedDrive, err := selectDriveByTopology(preferredTop, csiDrives); err == nil {
148-
return selectedDrive, nil
144+
if selectedDrives, err := selectDrivesByTopology(preferredTop, csiDrives); err == nil {
145+
return selectDriveByFreeCapacity(selectedDrives)
149146
}
150147
}
151148

152149
for _, requisiteTop := range requisiteXs {
153-
if selectedDrive, err := selectDriveByTopology(requisiteTop, csiDrives); err == nil {
154-
return selectedDrive, nil
150+
if selectedDrives, err := selectDrivesByTopology(requisiteTop, csiDrives); err == nil {
151+
return selectDriveByFreeCapacity(selectedDrives)
155152
}
156153
}
157154

158155
if len(preferredXs) == 0 && len(requisiteXs) == 0 {
159-
return csiDrives[0], nil
156+
return selectDriveByFreeCapacity(csiDrives)
160157
}
161158

162159
return directcsi.DirectCSIDrive{}, status.Error(codes.ResourceExhausted, "Cannot satisfy the topology constraint")
163160
}
164161

165-
func selectDriveByTopology(top *csi.Topology, csiDrives []directcsi.DirectCSIDrive) (directcsi.DirectCSIDrive, error) {
162+
func selectDriveByFreeCapacity(csiDrives []directcsi.DirectCSIDrive) (directcsi.DirectCSIDrive, error) {
163+
// Sort the drives by free capacity [Descending]
164+
sort.SliceStable(csiDrives, func(i, j int) bool {
165+
return csiDrives[i].Status.FreeCapacity > csiDrives[j].Status.FreeCapacity
166+
})
167+
168+
groupByFreeCapacity := func() []directcsi.DirectCSIDrive {
169+
maxFreeCapacity := csiDrives[0].Status.FreeCapacity
170+
groupedDrives := []directcsi.DirectCSIDrive{}
171+
for _, csiDrive := range csiDrives {
172+
if csiDrive.Status.FreeCapacity == maxFreeCapacity {
173+
groupedDrives = append(groupedDrives, csiDrive)
174+
}
175+
}
176+
return groupedDrives
177+
}
178+
179+
pickRandomIndex := func(max int) (int, error) {
180+
rInt, err := rand.Int(rand.Reader, big.NewInt(int64(max)))
181+
if err != nil {
182+
return int(0), err
183+
}
184+
return int(rInt.Int64()), nil
185+
}
186+
187+
selectedDrives := groupByFreeCapacity()
188+
rIndex, err := pickRandomIndex(len(selectedDrives))
189+
if err != nil {
190+
return selectedDrives[rIndex], status.Errorf(codes.Internal, "Error while selecting (random) drive: %v", err)
191+
}
192+
return selectedDrives[rIndex], nil
193+
}
194+
195+
func selectDrivesByTopology(top *csi.Topology, csiDrives []directcsi.DirectCSIDrive) ([]directcsi.DirectCSIDrive, error) {
196+
matchingDriveList := []directcsi.DirectCSIDrive{}
166197
topSegments := top.GetSegments()
167198
for _, csiDrive := range csiDrives {
168199
driveSegments := csiDrive.Status.Topology
169200
if matchSegments(topSegments, driveSegments) {
170-
return csiDrive, nil
201+
matchingDriveList = append(matchingDriveList, csiDrive)
171202
}
172203
}
173-
return directcsi.DirectCSIDrive{}, status.Error(codes.ResourceExhausted, "Cannot satisfy the topology constraint")
204+
return matchingDriveList, func() error {
205+
if len(matchingDriveList) == 0 {
206+
return status.Error(codes.ResourceExhausted, "Cannot satisfy the topology constraint")
207+
}
208+
return nil
209+
}()
174210
}
175211

176212
func matchSegments(topSegments, driveSegments map[string]string) bool {

0 commit comments

Comments
 (0)