Skip to content

Commit 3c202ba

Browse files
PhilippMatthesPhilippMatthes
committed
WIP: I want to separate filters and weighers in code
1 parent a307312 commit 3c202ba

12 files changed

Lines changed: 79 additions & 103 deletions

‎api/v1alpha1/pipeline_types.go‎

Lines changed: 31 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,18 @@ import (
1313
// valid candidates. Filters are run before weighers are applied, as
1414
// part of a filter-weigher scheduling pipeline.
1515
type FilterSpec struct {
16-
StepSpec `json:",inline"` // Embed common step spec fields.
16+
// The name of the scheduler step in the cortex implementation.
17+
// Must match to a step implemented by the pipeline controller.
18+
Name string `json:"name"`
19+
20+
// Additional configuration for the extractor that can be used
21+
// +kubebuilder:validation:Optional
22+
Opts runtime.RawExtension `json:"opts,omitempty"`
23+
24+
// Additional description of the step which helps understand its purpose
25+
// and decisions made by it.
26+
// +kubebuilder:validation:Optional
27+
Description string `json:"description,omitempty"`
1728

1829
// Filters are not allowed to depend on knowledges, as knowledges can
1930
// be outdated leading to invalid filtering decisions.
@@ -23,7 +34,18 @@ type FilterSpec struct {
2334
// making some hosts more preferable than others. Weighers are run
2435
// after filters are applied, as part of a filter-weigher scheduling pipeline.
2536
type WeigherSpec struct {
26-
StepSpec `json:",inline"` // Embed common step spec fields.
37+
// The name of the scheduler step in the cortex implementation.
38+
// Must match to a step implemented by the pipeline controller.
39+
Name string `json:"name"`
40+
41+
// Additional configuration for the extractor that can be used
42+
// +kubebuilder:validation:Optional
43+
Opts runtime.RawExtension `json:"opts,omitempty"`
44+
45+
// Additional description of the step which helps understand its purpose
46+
// and decisions made by it.
47+
// +kubebuilder:validation:Optional
48+
Description string `json:"description,omitempty"`
2749

2850
// Knowledges this step depends on to be ready.
2951
//
@@ -37,17 +59,6 @@ type WeigherSpec struct {
3759
// These detectors are run after weighers are applied, as part of a
3860
// descheduler scheduling pipeline.
3961
type DetectorSpec struct {
40-
StepSpec `json:",inline"` // Embed common step spec fields.
41-
42-
// Knowledges this step depends on to be ready.
43-
//
44-
// Detectors can depend on knowledges as they don't ensure valid placements
45-
// and therefore are not on the critical path.
46-
// +kubebuilder:validation:Optional
47-
Knowledges []corev1.ObjectReference `json:"knowledges,omitempty"`
48-
}
49-
50-
type StepSpec struct {
5162
// The name of the scheduler step in the cortex implementation.
5263
// Must match to a step implemented by the pipeline controller.
5364
Name string `json:"name"`
@@ -60,6 +71,13 @@ type StepSpec struct {
6071
// and decisions made by it.
6172
// +kubebuilder:validation:Optional
6273
Description string `json:"description,omitempty"`
74+
75+
// Knowledges this step depends on to be ready.
76+
//
77+
// Detectors can depend on knowledges as they don't ensure valid placements
78+
// and therefore are not on the critical path.
79+
// +kubebuilder:validation:Optional
80+
Knowledges []corev1.ObjectReference `json:"knowledges,omitempty"`
6381
}
6482

6583
type PipelineType string

‎api/v1alpha1/zz_generated.deepcopy.go‎

Lines changed: 3 additions & 19 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎internal/scheduling/decisions/cinder/pipeline_controller_test.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -490,12 +490,12 @@ func TestDecisionPipelineController_InitPipeline(t *testing.T) {
490490
name: "unsupported step",
491491
filters: []v1alpha1.FilterSpec{
492492
{
493-
StepSpec: v1alpha1.StepSpec{Name: "test-plugin"},
493+
Name: "test-plugin",
494494
},
495495
},
496496
weighers: []v1alpha1.WeigherSpec{
497497
{
498-
StepSpec: v1alpha1.StepSpec{Name: "test-plugin"},
498+
Name: "test-plugin",
499499
},
500500
},
501501
expectError: true, // Expected because test-plugin is not in supportedSteps

‎internal/scheduling/decisions/machines/pipeline_controller_test.go‎

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -224,18 +224,14 @@ func TestDecisionPipelineController_InitPipeline(t *testing.T) {
224224
{
225225
name: "noop step",
226226
weighers: []v1alpha1.WeigherSpec{
227-
{
228-
StepSpec: v1alpha1.StepSpec{Name: "noop"},
229-
},
227+
{Name: "noop"},
230228
},
231229
expectError: false,
232230
},
233231
{
234232
name: "unsupported step",
235233
filters: []v1alpha1.FilterSpec{
236-
{
237-
StepSpec: v1alpha1.StepSpec{Name: "unsupported"},
238-
},
234+
{Name: "unsupported"},
239235
},
240236
expectError: true,
241237
},

‎internal/scheduling/decisions/manila/pipeline_controller_test.go‎

Lines changed: 4 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -485,11 +485,9 @@ func TestDecisionPipelineController_InitPipeline(t *testing.T) {
485485
name: "supported netapp step",
486486
weighers: []v1alpha1.WeigherSpec{
487487
{
488-
StepSpec: v1alpha1.StepSpec{
489-
Name: "netapp_cpu_usage_balancing",
490-
Opts: runtime.RawExtension{
491-
Raw: []byte(`{"AvgCPUUsageLowerBound": 0, "AvgCPUUsageUpperBound": 90, "MaxCPUUsageLowerBound": 0, "MaxCPUUsageUpperBound": 100}`),
492-
},
488+
Name: "netapp_cpu_usage_balancing",
489+
Opts: runtime.RawExtension{
490+
Raw: []byte(`{"AvgCPUUsageLowerBound": 0, "AvgCPUUsageUpperBound": 90, "MaxCPUUsageLowerBound": 0, "MaxCPUUsageUpperBound": 100}`),
493491
},
494492
},
495493
},
@@ -499,9 +497,7 @@ func TestDecisionPipelineController_InitPipeline(t *testing.T) {
499497
name: "unsupported step",
500498
filters: []v1alpha1.FilterSpec{
501499
{
502-
StepSpec: v1alpha1.StepSpec{
503-
Name: "unsupported-plugin",
504-
},
500+
Name: "unsupported-plugin",
505501
},
506502
},
507503
expectError: true,

‎internal/scheduling/decisions/nova/pipeline_controller_test.go‎

Lines changed: 8 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -278,7 +278,7 @@ func TestDecisionPipelineController_InitPipeline(t *testing.T) {
278278
name: "supported step",
279279
filters: []v1alpha1.FilterSpec{
280280
{
281-
StepSpec: v1alpha1.StepSpec{Name: "filter_status_conditions"},
281+
Name: "filter_status_conditions",
282282
},
283283
},
284284
expectError: false,
@@ -287,7 +287,7 @@ func TestDecisionPipelineController_InitPipeline(t *testing.T) {
287287
name: "unsupported step",
288288
filters: []v1alpha1.FilterSpec{
289289
{
290-
StepSpec: v1alpha1.StepSpec{Name: "unsupported-plugin"},
290+
Name: "unsupported-plugin",
291291
},
292292
},
293293
expectError: true,
@@ -296,11 +296,9 @@ func TestDecisionPipelineController_InitPipeline(t *testing.T) {
296296
name: "step with scoping options",
297297
filters: []v1alpha1.FilterSpec{
298298
{
299-
StepSpec: v1alpha1.StepSpec{
300-
Name: "filter_status_conditions",
301-
Opts: runtime.RawExtension{
302-
Raw: []byte(`{"scope":{"host_capabilities":{"any_of_trait_infixes":["TEST_TRAIT"]}}}`),
303-
},
299+
Name: "filter_status_conditions",
300+
Opts: runtime.RawExtension{
301+
Raw: []byte(`{"scope":{"host_capabilities":{"any_of_trait_infixes":["TEST_TRAIT"]}}}`),
304302
},
305303
},
306304
},
@@ -310,11 +308,9 @@ func TestDecisionPipelineController_InitPipeline(t *testing.T) {
310308
name: "step with invalid scoping options",
311309
filters: []v1alpha1.FilterSpec{
312310
{
313-
StepSpec: v1alpha1.StepSpec{
314-
Name: "filter_status_conditions",
315-
Opts: runtime.RawExtension{
316-
Raw: []byte(`invalid json`),
317-
},
311+
Name: "filter_status_conditions",
312+
Opts: runtime.RawExtension{
313+
Raw: []byte(`invalid json`),
318314
},
319315
},
320316
},

‎internal/scheduling/decisions/pods/pipeline_controller_test.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -200,7 +200,7 @@ func TestDecisionPipelineController_InitPipeline(t *testing.T) {
200200
name: "noop step",
201201
filters: []v1alpha1.FilterSpec{
202202
{
203-
StepSpec: v1alpha1.StepSpec{Name: "noop"},
203+
Name: "noop",
204204
},
205205
},
206206
expectError: false,
@@ -209,7 +209,7 @@ func TestDecisionPipelineController_InitPipeline(t *testing.T) {
209209
name: "unsupported step",
210210
filters: []v1alpha1.FilterSpec{
211211
{
212-
StepSpec: v1alpha1.StepSpec{Name: "unsupported"},
212+
Name: "unsupported",
213213
},
214214
},
215215
expectError: true,

‎internal/scheduling/descheduling/nova/monitor_test.go‎

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -97,7 +97,7 @@ func TestMonitorStep(t *testing.T) {
9797
{VMID: "vm1", Reason: "test"},
9898
},
9999
}
100-
conf := v1alpha1.DetectorSpec{StepSpec: v1alpha1.StepSpec{Name: "test-step"}}
100+
conf := v1alpha1.DetectorSpec{Name: "test-step"}
101101

102102
monitoredStep := monitorStep(step, conf, monitor)
103103

@@ -117,7 +117,7 @@ func TestMonitorStep(t *testing.T) {
117117
func TestStepMonitor_Init(t *testing.T) {
118118
monitor := NewPipelineMonitor()
119119
step := &mockMonitorStep{}
120-
conf := v1alpha1.DetectorSpec{StepSpec: v1alpha1.StepSpec{Name: "test-step"}}
120+
conf := v1alpha1.DetectorSpec{Name: "test-step"}
121121

122122
monitoredStep := monitorStep(step, conf, monitor)
123123

@@ -139,7 +139,7 @@ func TestStepMonitor_Init_WithError(t *testing.T) {
139139
step := &mockMonitorStep{
140140
initError: expectedErr,
141141
}
142-
conf := v1alpha1.DetectorSpec{StepSpec: v1alpha1.StepSpec{Name: "test-step"}}
142+
conf := v1alpha1.DetectorSpec{Name: "test-step"}
143143
monitoredStep := monitorStep(step, conf, monitor)
144144

145145
client := fake.NewClientBuilder().Build()
@@ -159,7 +159,7 @@ func TestStepMonitor_Run(t *testing.T) {
159159
step := &mockMonitorStep{
160160
decisions: decisions,
161161
}
162-
conf := v1alpha1.DetectorSpec{StepSpec: v1alpha1.StepSpec{Name: "test-step"}}
162+
conf := v1alpha1.DetectorSpec{Name: "test-step"}
163163
monitoredStep := monitorStep(step, conf, monitor)
164164

165165
result, err := monitoredStep.Run()
@@ -189,7 +189,7 @@ func TestStepMonitor_Run_WithError(t *testing.T) {
189189
step := &mockMonitorStep{
190190
runError: expectedErr,
191191
}
192-
conf := v1alpha1.DetectorSpec{StepSpec: v1alpha1.StepSpec{Name: "test-step"}}
192+
conf := v1alpha1.DetectorSpec{Name: "test-step"}
193193
monitoredStep := monitorStep(step, conf, monitor)
194194

195195
result, err := monitoredStep.Run()
@@ -214,7 +214,7 @@ func TestStepMonitor_Run_EmptyResult(t *testing.T) {
214214
step := &mockMonitorStep{
215215
decisions: []plugins.Decision{}, // Empty slice
216216
}
217-
conf := v1alpha1.DetectorSpec{StepSpec: v1alpha1.StepSpec{Name: "test-step"}}
217+
conf := v1alpha1.DetectorSpec{Name: "test-step"}
218218
monitoredStep := monitorStep(step, conf, monitor)
219219

220220
result, err := monitoredStep.Run()
@@ -242,7 +242,7 @@ func TestMonitorStep_WithNilMonitor(t *testing.T) {
242242
{VMID: "vm1", Reason: "test"},
243243
},
244244
}
245-
conf := v1alpha1.DetectorSpec{StepSpec: v1alpha1.StepSpec{Name: "test-step"}}
245+
conf := v1alpha1.DetectorSpec{Name: "test-step"}
246246
monitoredStep := monitorStep(step, conf, monitor)
247247

248248
// Should not panic with nil timers/counters

‎internal/scheduling/descheduling/nova/pipeline_controller_test.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,7 @@ func TestDeschedulingsPipelineController_InitPipeline(t *testing.T) {
4848
name: "successful pipeline initialization",
4949
steps: []v1alpha1.DetectorSpec{
5050
{
51-
StepSpec: v1alpha1.StepSpec{Name: "mock-step"},
51+
Name: "mock-step",
5252
},
5353
},
5454
expectError: false,
@@ -57,7 +57,7 @@ func TestDeschedulingsPipelineController_InitPipeline(t *testing.T) {
5757
name: "unsupported step",
5858
steps: []v1alpha1.DetectorSpec{
5959
{
60-
StepSpec: v1alpha1.StepSpec{Name: "unsupported"},
60+
Name: "unsupported",
6161
},
6262
},
6363
expectError: true,

‎internal/scheduling/descheduling/nova/pipeline_test.go‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,7 @@ func TestPipeline_Init(t *testing.T) {
5252
"test-step": &mockPipelineStep{},
5353
},
5454
confedSteps: []v1alpha1.DetectorSpec{{
55-
StepSpec: v1alpha1.StepSpec{Name: "test-step"},
55+
Name: "test-step",
5656
}},
5757
expectedSteps: 1,
5858
},
@@ -62,7 +62,7 @@ func TestPipeline_Init(t *testing.T) {
6262
"test-step": &mockPipelineStep{},
6363
},
6464
confedSteps: []v1alpha1.DetectorSpec{{
65-
StepSpec: v1alpha1.StepSpec{Name: "unsupported-step"},
65+
Name: "unsupported-step",
6666
}},
6767
expectedError: true,
6868
},
@@ -72,7 +72,7 @@ func TestPipeline_Init(t *testing.T) {
7272
"failing-step": &mockPipelineStep{initError: errors.New("init failed")},
7373
},
7474
confedSteps: []v1alpha1.DetectorSpec{{
75-
StepSpec: v1alpha1.StepSpec{Name: "failing-step"},
75+
Name: "failing-step",
7676
}},
7777
expectedError: true,
7878
},
@@ -84,10 +84,10 @@ func TestPipeline_Init(t *testing.T) {
8484
},
8585
confedSteps: []v1alpha1.DetectorSpec{
8686
{
87-
StepSpec: v1alpha1.StepSpec{Name: "step1"},
87+
Name: "step1",
8888
},
8989
{
90-
StepSpec: v1alpha1.StepSpec{Name: "step2"},
90+
Name: "step2",
9191
},
9292
},
9393
expectedSteps: 2,

0 commit comments

Comments
 (0)