Skip to content
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

fix priority threshold by name alone #1186

Merged
merged 1 commit into from Jul 10, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
75 changes: 74 additions & 1 deletion pkg/descheduler/policyconfig_test.go
Expand Up @@ -862,6 +862,79 @@ strategies:
},
},
},
{
description: "v1alpha1 to internal with priorityThreshold",
policy: []byte(`apiVersion: "descheduler/v1alpha1"
kind: "DeschedulerPolicy"
strategies:
"PodLifeTime":
enabled: true
params:
podLifeTime:
maxPodLifeTimeSeconds: 5
namespaces:
include:
- "testleaderelection-a"
thresholdPriority: null
thresholdPriorityClassName: prioritym
`),
result: &api.DeschedulerPolicy{
Profiles: []api.DeschedulerProfile{
{
Name: fmt.Sprintf("strategy-%s-profile", podlifetime.PluginName),
PluginConfigs: []api.PluginConfig{
{
Name: "DefaultEvictor",
Args: &defaultevictor.DefaultEvictorArgs{
PriorityThreshold: &api.PriorityThreshold{
Value: utilpointer.Int32(0),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is just 0 because in the unit tests there is no prioritym class to parse right? In a cluster this would be a real number?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah,

func GetPriorityFromPriorityClass(ctx context.Context, client clientset.Interface, name string) (int32, error) {

GetPriorityFromPriorityClass would get one with that name.

return 0, err

It is returning 0 since there are no class with that name

},
},
},
{
Name: podlifetime.PluginName,
Args: &podlifetime.PodLifeTimeArgs{
Namespaces: &api.Namespaces{
Include: []string{"testleaderelection-a"},
},
MaxPodLifeTimeSeconds: utilpointer.Uint(5),
},
},
},
Plugins: api.Plugins{
Filter: api.PluginSet{
Enabled: []string{defaultevictor.PluginName},
},
PreEvictionFilter: api.PluginSet{
Enabled: []string{defaultevictor.PluginName},
},
Deschedule: api.PluginSet{
Enabled: []string{podlifetime.PluginName},
},
},
},
},
},
},
{
description: "v1alpha1 to internal with priorityThreshold value and name should return error",
policy: []byte(`apiVersion: "descheduler/v1alpha1"
kind: "DeschedulerPolicy"
strategies:
"PodLifeTime":
enabled: true
params:
podLifeTime:
maxPodLifeTimeSeconds: 5
namespaces:
include:
- "testleaderelection-a"
thresholdPriority: 222
thresholdPriorityClassName: prioritym
`),
result: nil,
err: fmt.Errorf("failed decoding descheduler's policy config \"filename\": priority threshold misconfigured for plugin PodLifeTime"),
},
{
description: "v1alpha2 to internal",
policy: []byte(`apiVersion: "descheduler/v1alpha2"
Expand Down Expand Up @@ -930,7 +1003,7 @@ profiles:
if err != nil {
if tc.err == nil {
t.Errorf("unexpected error: %s.", err.Error())
} else {
} else if err.Error() != tc.err.Error() {
t.Errorf("unexpected error: %s. Was expecting %s", err.Error(), tc.err.Error())
}
}
Expand Down
2 changes: 1 addition & 1 deletion pkg/framework/plugins/defaultevictor/validation.go
Expand Up @@ -22,7 +22,7 @@ import (
func ValidateDefaultEvictorArgs(obj runtime.Object) error {
args := obj.(*DefaultEvictorArgs)

if args.PriorityThreshold != nil && len(args.PriorityThreshold.Name) > 0 {
if args.PriorityThreshold != nil && args.PriorityThreshold.Value != nil && len(args.PriorityThreshold.Name) > 0 {
return fmt.Errorf("priority threshold misconfigured, only one of priorityThreshold fields can be set, got %v", args)
}

Expand Down