diff --git a/pkg/controller/daemon/util/BUILD b/pkg/controller/daemon/util/BUILD index 9a64e9a62e0..3d92cbd88a9 100644 --- a/pkg/controller/daemon/util/BUILD +++ b/pkg/controller/daemon/util/BUILD @@ -14,7 +14,6 @@ go_library( "//pkg/api/v1/pod:go_default_library", "//pkg/apis/core/v1/helper:go_default_library", "//pkg/features:go_default_library", - "//pkg/kubelet/apis:go_default_library", "//pkg/kubelet/types:go_default_library", "//pkg/scheduler/algorithm:go_default_library", "//vendor/k8s.io/api/apps/v1:go_default_library", @@ -45,9 +44,12 @@ go_test( embed = [":go_default_library"], deps = [ "//pkg/api/testapi:go_default_library", + "//pkg/features:go_default_library", "//pkg/kubelet/apis:go_default_library", + "//pkg/scheduler/algorithm:go_default_library", "//vendor/k8s.io/api/core/v1:go_default_library", "//vendor/k8s.io/api/extensions/v1beta1:go_default_library", "//vendor/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library", + "//vendor/k8s.io/apiserver/pkg/util/feature:go_default_library", ], ) diff --git a/pkg/controller/daemon/util/daemonset_util.go b/pkg/controller/daemon/util/daemonset_util.go index e39f1620f17..ffb7033bd3d 100644 --- a/pkg/controller/daemon/util/daemonset_util.go +++ b/pkg/controller/daemon/util/daemonset_util.go @@ -29,7 +29,6 @@ import ( podutil "k8s.io/kubernetes/pkg/api/v1/pod" v1helper "k8s.io/kubernetes/pkg/apis/core/v1/helper" "k8s.io/kubernetes/pkg/features" - kubeletapis "k8s.io/kubernetes/pkg/kubelet/apis" kubelettypes "k8s.io/kubernetes/pkg/kubelet/types" "k8s.io/kubernetes/pkg/scheduler/algorithm" ) @@ -136,19 +135,20 @@ func SplitByAvailablePods(minReadySeconds int32, pods []*v1.Pod) ([]*v1.Pod, []* return availablePods, unavailablePods } -// ReplaceDaemonSetPodHostnameNodeAffinity replaces the 'kubernetes.io/hostname' NodeAffinity term with -// the given "nodeName" in the "affinity" terms. -func ReplaceDaemonSetPodHostnameNodeAffinity(affinity *v1.Affinity, nodename string) *v1.Affinity { +// ReplaceDaemonSetPodNodeNameNodeAffinity replaces the RequiredDuringSchedulingIgnoredDuringExecution +// NodeAffinity of the given affinity with a new NodeAffinity that selects the given nodeName. +// Note that this function assumes that no NodeAffinity conflicts with the selected nodeName. +func ReplaceDaemonSetPodNodeNameNodeAffinity(affinity *v1.Affinity, nodename string) *v1.Affinity { + nodeSelReq := v1.NodeSelectorRequirement{ + Key: algorithm.NodeFieldSelectorKeyNodeName, + Operator: v1.NodeSelectorOpIn, + Values: []string{nodename}, + } + nodeSelector := &v1.NodeSelector{ NodeSelectorTerms: []v1.NodeSelectorTerm{ { - MatchExpressions: []v1.NodeSelectorRequirement{ - { - Key: kubeletapis.LabelHostname, - Operator: v1.NodeSelectorOpIn, - Values: []string{nodename}, - }, - }, + MatchFields: []v1.NodeSelectorRequirement{nodeSelReq}, }, }, } @@ -175,28 +175,12 @@ func ReplaceDaemonSetPodHostnameNodeAffinity(affinity *v1.Affinity, nodename str return affinity } - nodeSelectorTerms := []v1.NodeSelectorTerm{} - - // Removes hostname node selector, as only the target hostname will take effect. - for _, term := range nodeAffinity.RequiredDuringSchedulingIgnoredDuringExecution.NodeSelectorTerms { - exps := []v1.NodeSelectorRequirement{} - for _, exp := range term.MatchExpressions { - if exp.Key != kubeletapis.LabelHostname { - exps = append(exps, exp) - } - } - - if len(exps) > 0 { - term.MatchExpressions = exps - nodeSelectorTerms = append(nodeSelectorTerms, term) - } - } - - // Adds target hostname NodeAffinity term. - nodeSelectorTerms = append(nodeSelectorTerms, nodeSelector.NodeSelectorTerms[0]) - // Replace node selector with the new one. - nodeAffinity.RequiredDuringSchedulingIgnoredDuringExecution.NodeSelectorTerms = nodeSelectorTerms + nodeAffinity.RequiredDuringSchedulingIgnoredDuringExecution.NodeSelectorTerms = []v1.NodeSelectorTerm{ + { + MatchFields: []v1.NodeSelectorRequirement{nodeSelReq}, + }, + } return affinity } @@ -225,3 +209,42 @@ func AppendNoScheduleTolerationIfNotExist(tolerations []v1.Toleration) []v1.Tole return tolerations } + +// GetTargetNodeName get the target node name of DaemonSet pods. If `.spec.NodeName` is not empty (nil), +// return `.spec.NodeName`; otherwise, retrieve node name of pending pods from NodeAffinity. Return error +// if failed to retrieve node name from `.spec.NodeName` and NodeAffinity. +func GetTargetNodeName(pod *v1.Pod) (string, error) { + if len(pod.Spec.NodeName) != 0 { + return pod.Spec.NodeName, nil + } + + // If ScheduleDaemonSetPods was enabled before, retrieve node name of unscheduled pods from NodeAffinity + if pod.Spec.Affinity == nil || + pod.Spec.Affinity.NodeAffinity == nil || + pod.Spec.Affinity.NodeAffinity.RequiredDuringSchedulingIgnoredDuringExecution == nil { + return "", fmt.Errorf("no spec.affinity.nodeAffinity.requiredDuringSchedulingIgnoredDuringExecution for pod %s/%s", + pod.Namespace, pod.Name) + } + + terms := pod.Spec.Affinity.NodeAffinity.RequiredDuringSchedulingIgnoredDuringExecution.NodeSelectorTerms + if len(terms) < 1 { + return "", fmt.Errorf("no nodeSelectorTerms in requiredDuringSchedulingIgnoredDuringExecution of pod %s/%s", + pod.Namespace, pod.Name) + } + + for _, term := range terms { + for _, exp := range term.MatchFields { + if exp.Key == algorithm.NodeFieldSelectorKeyNodeName && + exp.Operator == v1.NodeSelectorOpIn { + if len(exp.Values) != 1 { + return "", fmt.Errorf("the matchFields value of '%s' is not unique for pod %s/%s", + algorithm.NodeFieldSelectorKeyNodeName, pod.Namespace, pod.Name) + } + + return exp.Values[0], nil + } + } + } + + return "", fmt.Errorf("no node name found for pod %s/%s", pod.Namespace, pod.Name) +} diff --git a/pkg/controller/daemon/util/daemonset_util_test.go b/pkg/controller/daemon/util/daemonset_util_test.go index cf23be4015b..5ed8d999021 100644 --- a/pkg/controller/daemon/util/daemonset_util_test.go +++ b/pkg/controller/daemon/util/daemonset_util_test.go @@ -24,8 +24,11 @@ import ( "k8s.io/api/core/v1" extensions "k8s.io/api/extensions/v1beta1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + utilfeature "k8s.io/apiserver/pkg/util/feature" "k8s.io/kubernetes/pkg/api/testapi" + "k8s.io/kubernetes/pkg/features" kubeletapis "k8s.io/kubernetes/pkg/kubelet/apis" + "k8s.io/kubernetes/pkg/scheduler/algorithm" ) func newPod(podName string, nodeName string, label map[string]string) *v1.Pod { @@ -171,7 +174,7 @@ func int64Ptr(i int) *int64 { return &li } -func TestReplaceDaemonSetPodHostnameNodeAffinity(t *testing.T) { +func TestReplaceDaemonSetPodNodeNameNodeAffinity(t *testing.T) { tests := []struct { affinity *v1.Affinity hostname string @@ -181,6 +184,25 @@ func TestReplaceDaemonSetPodHostnameNodeAffinity(t *testing.T) { affinity: nil, hostname: "host_1", expected: &v1.Affinity{ + NodeAffinity: &v1.NodeAffinity{ + RequiredDuringSchedulingIgnoredDuringExecution: &v1.NodeSelector{ + NodeSelectorTerms: []v1.NodeSelectorTerm{ + { + MatchFields: []v1.NodeSelectorRequirement{ + { + Key: algorithm.NodeFieldSelectorKeyNodeName, + Operator: v1.NodeSelectorOpIn, + Values: []string{"host_1"}, + }, + }, + }, + }, + }, + }, + }, + }, + { + affinity: &v1.Affinity{ NodeAffinity: &v1.NodeAffinity{ RequiredDuringSchedulingIgnoredDuringExecution: &v1.NodeSelector{ NodeSelectorTerms: []v1.NodeSelectorTerm{ @@ -197,6 +219,24 @@ func TestReplaceDaemonSetPodHostnameNodeAffinity(t *testing.T) { }, }, }, + hostname: "host_1", + expected: &v1.Affinity{ + NodeAffinity: &v1.NodeAffinity{ + RequiredDuringSchedulingIgnoredDuringExecution: &v1.NodeSelector{ + NodeSelectorTerms: []v1.NodeSelectorTerm{ + { + MatchFields: []v1.NodeSelectorRequirement{ + { + Key: algorithm.NodeFieldSelectorKeyNodeName, + Operator: v1.NodeSelectorOpIn, + Values: []string{"host_1"}, + }, + }, + }, + }, + }, + }, + }, }, { affinity: &v1.Affinity{ @@ -235,9 +275,9 @@ func TestReplaceDaemonSetPodHostnameNodeAffinity(t *testing.T) { RequiredDuringSchedulingIgnoredDuringExecution: &v1.NodeSelector{ NodeSelectorTerms: []v1.NodeSelectorTerm{ { - MatchExpressions: []v1.NodeSelectorRequirement{ + MatchFields: []v1.NodeSelectorRequirement{ { - Key: kubeletapis.LabelHostname, + Key: algorithm.NodeFieldSelectorKeyNodeName, Operator: v1.NodeSelectorOpIn, Values: []string{"host_1"}, }, @@ -254,55 +294,9 @@ func TestReplaceDaemonSetPodHostnameNodeAffinity(t *testing.T) { RequiredDuringSchedulingIgnoredDuringExecution: &v1.NodeSelector{ NodeSelectorTerms: []v1.NodeSelectorTerm{ { - MatchExpressions: []v1.NodeSelectorRequirement{ + MatchFields: []v1.NodeSelectorRequirement{ { - Key: "not-host-label", - Operator: v1.NodeSelectorOpIn, - Values: []string{"label_value_1", "label_value_2"}, - }, - }, - }, - }, - }, - }, - }, - hostname: "host_1", - expected: &v1.Affinity{ - NodeAffinity: &v1.NodeAffinity{ - RequiredDuringSchedulingIgnoredDuringExecution: &v1.NodeSelector{ - NodeSelectorTerms: []v1.NodeSelectorTerm{ - { - MatchExpressions: []v1.NodeSelectorRequirement{ - { - Key: "not-host-label", - Operator: v1.NodeSelectorOpIn, - Values: []string{"label_value_1", "label_value_2"}, - }, - }, - }, - { - MatchExpressions: []v1.NodeSelectorRequirement{ - { - Key: kubeletapis.LabelHostname, - Operator: v1.NodeSelectorOpIn, - Values: []string{"host_1"}, - }, - }, - }, - }, - }, - }, - }, - }, - { - affinity: &v1.Affinity{ - NodeAffinity: &v1.NodeAffinity{ - RequiredDuringSchedulingIgnoredDuringExecution: &v1.NodeSelector{ - NodeSelectorTerms: []v1.NodeSelectorTerm{ - { - MatchExpressions: []v1.NodeSelectorRequirement{ - { - Key: kubeletapis.LabelHostname, + Key: algorithm.NodeFieldSelectorKeyNodeName, Operator: v1.NodeSelectorOpIn, Values: []string{"host_1", "host_2"}, }, @@ -314,13 +308,157 @@ func TestReplaceDaemonSetPodHostnameNodeAffinity(t *testing.T) { }, hostname: "host_1", expected: &v1.Affinity{ + NodeAffinity: &v1.NodeAffinity{ + RequiredDuringSchedulingIgnoredDuringExecution: &v1.NodeSelector{ + NodeSelectorTerms: []v1.NodeSelectorTerm{ + { + MatchFields: []v1.NodeSelectorRequirement{ + { + Key: algorithm.NodeFieldSelectorKeyNodeName, + Operator: v1.NodeSelectorOpIn, + Values: []string{"host_1"}, + }, + }, + }, + }, + }, + }, + }, + }, + { + affinity: nil, + hostname: "host_1", + expected: &v1.Affinity{ + NodeAffinity: &v1.NodeAffinity{ + RequiredDuringSchedulingIgnoredDuringExecution: &v1.NodeSelector{ + NodeSelectorTerms: []v1.NodeSelectorTerm{ + { + MatchFields: []v1.NodeSelectorRequirement{ + { + Key: algorithm.NodeFieldSelectorKeyNodeName, + Operator: v1.NodeSelectorOpIn, + Values: []string{"host_1"}, + }, + }, + }, + }, + }, + }, + }, + }, + { + affinity: &v1.Affinity{ NodeAffinity: &v1.NodeAffinity{ RequiredDuringSchedulingIgnoredDuringExecution: &v1.NodeSelector{ NodeSelectorTerms: []v1.NodeSelectorTerm{ { MatchExpressions: []v1.NodeSelectorRequirement{ { - Key: kubeletapis.LabelHostname, + Key: "hostname", + Operator: v1.NodeSelectorOpIn, + Values: []string{"host_1"}, + }, + }, + }, + { + MatchFields: []v1.NodeSelectorRequirement{ + { + Key: algorithm.NodeFieldSelectorKeyNodeName, + Operator: v1.NodeSelectorOpIn, + Values: []string{"host_2"}, + }, + }, + }, + }, + }, + }, + }, + hostname: "host_1", + expected: &v1.Affinity{ + NodeAffinity: &v1.NodeAffinity{ + RequiredDuringSchedulingIgnoredDuringExecution: &v1.NodeSelector{ + NodeSelectorTerms: []v1.NodeSelectorTerm{ + { + MatchFields: []v1.NodeSelectorRequirement{ + { + Key: algorithm.NodeFieldSelectorKeyNodeName, + Operator: v1.NodeSelectorOpIn, + Values: []string{"host_1"}, + }, + }, + }, + }, + }, + }, + }, + }, + { + affinity: &v1.Affinity{ + NodeAffinity: &v1.NodeAffinity{ + RequiredDuringSchedulingIgnoredDuringExecution: &v1.NodeSelector{ + NodeSelectorTerms: []v1.NodeSelectorTerm{ + { + MatchFields: []v1.NodeSelectorRequirement{ + { + Key: algorithm.NodeFieldSelectorKeyNodeName, + Operator: v1.NodeSelectorOpNotIn, + Values: []string{"host_2"}, + }, + }, + }, + }, + }, + }, + }, + hostname: "host_1", + expected: &v1.Affinity{ + NodeAffinity: &v1.NodeAffinity{ + RequiredDuringSchedulingIgnoredDuringExecution: &v1.NodeSelector{ + NodeSelectorTerms: []v1.NodeSelectorTerm{ + { + MatchFields: []v1.NodeSelectorRequirement{ + { + Key: algorithm.NodeFieldSelectorKeyNodeName, + Operator: v1.NodeSelectorOpIn, + Values: []string{"host_1"}, + }, + }, + }, + }, + }, + }, + }, + }, + { + affinity: &v1.Affinity{ + NodeAffinity: &v1.NodeAffinity{ + RequiredDuringSchedulingIgnoredDuringExecution: &v1.NodeSelector{ + NodeSelectorTerms: []v1.NodeSelectorTerm{ + { + MatchFields: []v1.NodeSelectorRequirement{ + { + // NOTE: Only `metadata.name` is valid key in `MatchFields` in 1.11; + // added this case for compatibility: the feature works as normal + // when new Keys introduced. + Key: "metadata.foo", + Operator: v1.NodeSelectorOpIn, + Values: []string{"bar"}, + }, + }, + }, + }, + }, + }, + }, + hostname: "host_1", + expected: &v1.Affinity{ + NodeAffinity: &v1.NodeAffinity{ + RequiredDuringSchedulingIgnoredDuringExecution: &v1.NodeSelector{ + NodeSelectorTerms: []v1.NodeSelectorTerm{ + { + MatchFields: []v1.NodeSelectorRequirement{ + { + Key: algorithm.NodeFieldSelectorKeyNodeName, Operator: v1.NodeSelectorOpIn, Values: []string{"host_1"}, }, @@ -333,10 +471,129 @@ func TestReplaceDaemonSetPodHostnameNodeAffinity(t *testing.T) { }, } - for _, test := range tests { - got := ReplaceDaemonSetPodHostnameNodeAffinity(test.affinity, test.hostname) + for i, test := range tests { + got := ReplaceDaemonSetPodNodeNameNodeAffinity(test.affinity, test.hostname) if !reflect.DeepEqual(test.expected, got) { - t.Errorf("Failed to append NodeAffinity, got: %v, expected: %v", got, test.expected) + t.Errorf("Failed to append NodeAffinity in case %d, got: %v, expected: %v", + i, got, test.expected) } } } + +func forEachFeatureGate(t *testing.T, tf func(t *testing.T), gates ...utilfeature.Feature) { + for _, fg := range gates { + func() { + enabled := utilfeature.DefaultFeatureGate.Enabled(fg) + defer func() { + utilfeature.DefaultFeatureGate.Set(fmt.Sprintf("%v=%t", fg, enabled)) + }() + + for _, f := range []bool{true, false} { + utilfeature.DefaultFeatureGate.Set(fmt.Sprintf("%v=%t", fg, f)) + t.Run(fmt.Sprintf("%v (%t)", fg, f), tf) + } + }() + } +} + +func TestGetTargetNodeName(t *testing.T) { + testFun := func(t *testing.T) { + tests := []struct { + pod *v1.Pod + nodeName string + expectedErr bool + }{ + { + pod: &v1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: "pod1", + Namespace: "default", + }, + Spec: v1.PodSpec{ + NodeName: "node-1", + }, + }, + nodeName: "node-1", + }, + { + pod: &v1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: "pod2", + Namespace: "default", + }, + Spec: v1.PodSpec{ + Affinity: &v1.Affinity{ + NodeAffinity: &v1.NodeAffinity{ + RequiredDuringSchedulingIgnoredDuringExecution: &v1.NodeSelector{ + NodeSelectorTerms: []v1.NodeSelectorTerm{ + { + MatchFields: []v1.NodeSelectorRequirement{ + { + Key: algorithm.NodeFieldSelectorKeyNodeName, + Operator: v1.NodeSelectorOpIn, + Values: []string{"node-1"}, + }, + }, + }, + }, + }, + }, + }, + }, + }, + nodeName: "node-1", + }, + { + pod: &v1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: "pod3", + Namespace: "default", + }, + Spec: v1.PodSpec{ + Affinity: &v1.Affinity{ + NodeAffinity: &v1.NodeAffinity{ + RequiredDuringSchedulingIgnoredDuringExecution: &v1.NodeSelector{ + NodeSelectorTerms: []v1.NodeSelectorTerm{ + { + MatchFields: []v1.NodeSelectorRequirement{ + { + Key: algorithm.NodeFieldSelectorKeyNodeName, + Operator: v1.NodeSelectorOpIn, + Values: []string{"node-1", "node-2"}, + }, + }, + }, + }, + }, + }, + }, + }, + }, + expectedErr: true, + }, + { + pod: &v1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: "pod4", + Namespace: "default", + }, + Spec: v1.PodSpec{}, + }, + expectedErr: true, + }, + } + + for _, test := range tests { + got, err := GetTargetNodeName(test.pod) + if test.expectedErr != (err != nil) { + t.Errorf("Unexpected error, expectedErr: %v, err: %v", test.expectedErr, err) + } else if !test.expectedErr { + if test.nodeName != got { + t.Errorf("Failed to get target node name, got: %v, expected: %v", got, test.nodeName) + } + } + } + } + + forEachFeatureGate(t, testFun, features.ScheduleDaemonSetPods) +}