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

Add AddedAffinity to NodeAffinity Filter and Score plugin #96202

Merged
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
2 changes: 1 addition & 1 deletion pkg/kubelet/lifecycle/predicate.go
Expand Up @@ -237,7 +237,7 @@ func GeneralPredicates(pod *v1.Pod, nodeInfo *schedulerframework.NodeInfo) ([]Pr
}

if !pluginhelper.PodMatchesNodeSelectorAndAffinityTerms(pod, nodeInfo.Node()) {
reasons = append(reasons, &PredicateFailureError{nodeaffinity.Name, nodeaffinity.ErrReason})
reasons = append(reasons, &PredicateFailureError{nodeaffinity.Name, nodeaffinity.ErrReasonPod})
}
if !nodename.Fits(pod, nodeInfo) {
reasons = append(reasons, &PredicateFailureError{nodename.Name, nodename.ErrReason})
Expand Down
1 change: 1 addition & 0 deletions pkg/scheduler/apis/config/register.go
Expand Up @@ -49,6 +49,7 @@ func addKnownTypes(scheme *runtime.Scheme) error {
&VolumeBindingArgs{},
&NodeResourcesLeastAllocatedArgs{},
&NodeResourcesMostAllocatedArgs{},
&NodeAffinityArgs{},
)
scheme.AddKnownTypes(schema.GroupVersion{Group: "", Version: runtime.APIVersionInternal}, &Policy{})
return nil
Expand Down
34 changes: 34 additions & 0 deletions pkg/scheduler/apis/config/scheme/scheme_test.go
Expand Up @@ -87,6 +87,15 @@ profiles:
- name: VolumeBinding
args:
bindTimeoutSeconds: 300
- name: NodeAffinity
args:
addedAffinity:
requiredDuringSchedulingIgnoredDuringExecution:
nodeSelectorTerms:
- matchExpressions:
- key: foo
operator: In
values: ["bar"]
`),
wantProfiles: []config.KubeSchedulerProfile{
{
Expand Down Expand Up @@ -148,6 +157,26 @@ profiles:
BindTimeoutSeconds: 300,
},
},
{
Name: "NodeAffinity",
Args: &config.NodeAffinityArgs{
AddedAffinity: &corev1.NodeAffinity{
RequiredDuringSchedulingIgnoredDuringExecution: &corev1.NodeSelector{
NodeSelectorTerms: []corev1.NodeSelectorTerm{
{
MatchExpressions: []corev1.NodeSelectorRequirement{
{
Key: "foo",
Operator: corev1.NodeSelectorOpIn,
Values: []string{"bar"},
},
},
},
},
},
},
},
},
},
},
},
Expand Down Expand Up @@ -271,6 +300,7 @@ profiles:
- name: VolumeBinding
args:
- name: PodTopologySpread
- name: NodeAffinity
`),
wantProfiles: []config.KubeSchedulerProfile{
{
Expand Down Expand Up @@ -315,6 +345,10 @@ profiles:
DefaultingType: config.SystemDefaulting,
},
},
{
Name: "NodeAffinity",
Args: &config.NodeAffinityArgs{},
},
},
},
},
Expand Down
15 changes: 15 additions & 0 deletions pkg/scheduler/apis/config/types_pluginargs.go
Expand Up @@ -203,3 +203,18 @@ type VolumeBindingArgs struct {
// If this value is nil, the default value will be used.
BindTimeoutSeconds int64
}

// +k8s:deepcopy-gen:interfaces=k8s.io/apimachinery/pkg/runtime.Object

// NodeAffinityArgs holds arguments to configure the NodeAffinity plugin.
type NodeAffinityArgs struct {
metav1.TypeMeta

// AddedAffinity is applied to all Pods additionally to the NodeAffinity
// specified in the PodSpec. That is, Nodes need to satisfy AddedAffinity
// AND .spec.NodeAffinity. AddedAffinity is empty by default (all Nodes
// match).
// When AddedAffinity is used, some Pods with affinity requirements that match
// a specific Node (such as Daemonset Pods) might remain unschedulable.
AddedAffinity *v1.NodeAffinity
}
30 changes: 30 additions & 0 deletions pkg/scheduler/apis/config/v1beta1/zz_generated.conversion.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 4 additions & 0 deletions pkg/scheduler/apis/config/validation/BUILD
Expand Up @@ -18,6 +18,7 @@ go_library(
"//staging/src/k8s.io/apimachinery/pkg/util/validation:go_default_library",
"//staging/src/k8s.io/apimachinery/pkg/util/validation/field:go_default_library",
"//staging/src/k8s.io/component-base/config/validation:go_default_library",
"//staging/src/k8s.io/component-helpers/scheduling/corev1/nodeaffinity:go_default_library",
"//vendor/github.com/google/go-cmp/cmp:go_default_library",
],
)
Expand All @@ -33,7 +34,10 @@ go_test(
"//pkg/scheduler/apis/config:go_default_library",
"//staging/src/k8s.io/api/core/v1:go_default_library",
"//staging/src/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library",
"//staging/src/k8s.io/apimachinery/pkg/util/validation/field:go_default_library",
"//staging/src/k8s.io/component-base/config:go_default_library",
"//vendor/github.com/google/go-cmp/cmp:go_default_library",
"//vendor/github.com/google/go-cmp/cmp/cmpopts:go_default_library",
],
)

Expand Down
27 changes: 27 additions & 0 deletions pkg/scheduler/apis/config/validation/validation_pluginargs.go
Expand Up @@ -23,6 +23,7 @@ import (
metav1validation "k8s.io/apimachinery/pkg/apis/meta/v1/validation"
"k8s.io/apimachinery/pkg/util/sets"
"k8s.io/apimachinery/pkg/util/validation/field"
"k8s.io/component-helpers/scheduling/corev1/nodeaffinity"
"k8s.io/kubernetes/pkg/scheduler/apis/config"
)

Expand Down Expand Up @@ -257,3 +258,29 @@ func validateResources(resources []config.ResourceSpec) error {
}
return nil
}

// ValidateNodeAffinityArgs validates that NodeAffinityArgs are correct.
func ValidateNodeAffinityArgs(args *config.NodeAffinityArgs) error {
if args.AddedAffinity == nil {
return nil
}
affinity := args.AddedAffinity
f := field.NewPath("addedAffinity")
var allErrs field.ErrorList
if ns := affinity.RequiredDuringSchedulingIgnoredDuringExecution; ns != nil {
_, err := nodeaffinity.NewNodeSelector(ns)
if err != nil {
// TODO(#96167): Expand all field.Error(s) returned once constructor use them.
allErrs = append(allErrs, field.Invalid(f.Child("requiredDuringSchedulingIgnoredDuringExecution"), affinity.RequiredDuringSchedulingIgnoredDuringExecution, err.Error()))
}
}
// TODO: Add validation for requiredDuringSchedulingRequiredDuringExecution when it gets added to the API.
if terms := affinity.PreferredDuringSchedulingIgnoredDuringExecution; len(terms) != 0 {
_, err := nodeaffinity.NewPreferredSchedulingTerms(terms)
if err != nil {
// TODO(#96167): Expand all field.Error(s) returned once constructor use them.
allErrs = append(allErrs, field.Invalid(f.Child("preferredDuringSchedulingIgnoredDuringExecution"), affinity.PreferredDuringSchedulingIgnoredDuringExecution, err.Error()))
}
}
return allErrs.ToAggregate()
Copy link
Member

Choose a reason for hiding this comment

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

add a TODO to validate RequiredDuringSchedulingRequiredDuringExecution once implemented

Copy link
Member Author

Choose a reason for hiding this comment

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

Done

}
99 changes: 99 additions & 0 deletions pkg/scheduler/apis/config/validation/validation_pluginargs_test.go
Expand Up @@ -19,11 +19,18 @@ package validation
import (
"testing"

"github.com/google/go-cmp/cmp"
"github.com/google/go-cmp/cmp/cmpopts"
v1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/util/validation/field"
"k8s.io/kubernetes/pkg/scheduler/apis/config"
)

var (
ignoreBadValueDetail = cmpopts.IgnoreFields(field.Error{}, "BadValue", "Detail")
)

func TestValidateDefaultPreemptionArgs(t *testing.T) {
cases := map[string]struct {
args config.DefaultPreemptionArgs
Expand Down Expand Up @@ -553,6 +560,98 @@ func TestValidateNodeResourcesMostAllocatedArgs(t *testing.T) {
}
}

func TestValidateNodeAffinityArgs(t *testing.T) {
cases := []struct {
name string
args config.NodeAffinityArgs
wantErr error
}{
{
name: "empty",
},
{
name: "valid added affinity",
args: config.NodeAffinityArgs{
AddedAffinity: &v1.NodeAffinity{
RequiredDuringSchedulingIgnoredDuringExecution: &v1.NodeSelector{
NodeSelectorTerms: []v1.NodeSelectorTerm{
{
MatchExpressions: []v1.NodeSelectorRequirement{
{
Key: "label-1",
Operator: v1.NodeSelectorOpIn,
Values: []string{"label-1-val"},
},
},
},
},
},
PreferredDuringSchedulingIgnoredDuringExecution: []v1.PreferredSchedulingTerm{
{
Weight: 1,
Preference: v1.NodeSelectorTerm{
MatchFields: []v1.NodeSelectorRequirement{
{
Key: "metadata.name",
Operator: v1.NodeSelectorOpIn,
Values: []string{"node-1"},
},
},
},
},
},
},
},
},
{
name: "invalid added affinity",
args: config.NodeAffinityArgs{
AddedAffinity: &v1.NodeAffinity{
RequiredDuringSchedulingIgnoredDuringExecution: &v1.NodeSelector{
NodeSelectorTerms: []v1.NodeSelectorTerm{
{
MatchExpressions: []v1.NodeSelectorRequirement{
{
Key: "invalid/label/key",
Operator: v1.NodeSelectorOpIn,
Values: []string{"label-1-val"},
},
},
},
},
},
PreferredDuringSchedulingIgnoredDuringExecution: []v1.PreferredSchedulingTerm{
{
Weight: 1,
Preference: v1.NodeSelectorTerm{
MatchFields: []v1.NodeSelectorRequirement{
{
Key: "metadata.name",
Operator: v1.NodeSelectorOpIn,
Values: []string{"node-1", "node-2"},
},
},
},
},
},
},
},
wantErr: field.ErrorList{
field.Invalid(field.NewPath("addedAffinity", "requiredDuringSchedulingIgnoredDuringExecution"), nil, ""),
field.Invalid(field.NewPath("addedAffinity", "preferredDuringSchedulingIgnoredDuringExecution"), nil, ""),
}.ToAggregate(),
},
}
for _, tc := range cases {
t.Run(tc.name, func(t *testing.T) {
err := ValidateNodeAffinityArgs(&tc.args)
if diff := cmp.Diff(err, tc.wantErr, ignoreBadValueDetail); diff != "" {
t.Fatalf("ValidatedNodeAffinityArgs returned err (-want,+got):\n%s", diff)
}
})
}
}

func assertErr(t *testing.T, wantErr string, gotErr error) {
if wantErr == "" {
if gotErr != nil {
Expand Down
30 changes: 30 additions & 0 deletions pkg/scheduler/apis/config/zz_generated.deepcopy.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Expand Up @@ -1284,7 +1284,7 @@ func TestNodesWherePreemptionMightHelp(t *testing.T) {
{
name: "No node should be attempted",
nodesStatuses: framework.NodeToStatusMap{
"node1": framework.NewStatus(framework.UnschedulableAndUnresolvable, nodeaffinity.ErrReason),
"node1": framework.NewStatus(framework.UnschedulableAndUnresolvable, nodeaffinity.ErrReasonPod),
"node2": framework.NewStatus(framework.UnschedulableAndUnresolvable, nodename.ErrReason),
"node3": framework.NewStatus(framework.UnschedulableAndUnresolvable, tainttoleration.ErrReasonNotMatch),
"node4": framework.NewStatus(framework.UnschedulableAndUnresolvable, nodelabel.ErrReasonPresenceViolated),
Expand Down