From 384a86caa92bdb7cf9ac96b10a6ef333d2d60519 Mon Sep 17 00:00:00 2001 From: "Bobby (Babak) Salamat" Date: Thu, 25 Jan 2018 13:23:56 -0800 Subject: [PATCH] Add NominatedNodeName to PodStatus --- pkg/apis/core/types.go | 7 + pkg/apis/core/v1/conversion.go | 3 +- pkg/apis/core/validation/validation.go | 6 + pkg/apis/core/validation/validation_test.go | 123 ++++++++++++++++++ pkg/printers/internalversion/describe.go | 3 + pkg/printers/internalversion/describe_test.go | 6 +- pkg/printers/internalversion/printers.go | 3 + pkg/printers/internalversion/printers_test.go | 3 +- pkg/registry/core/pod/strategy.go | 1 + pkg/registry/core/pod/strategy_test.go | 15 ++- staging/src/k8s.io/api/core/v1/types.go | 9 ++ 11 files changed, 175 insertions(+), 4 deletions(-) diff --git a/pkg/apis/core/types.go b/pkg/apis/core/types.go index bcf3661ba4e..72dba8aebb6 100644 --- a/pkg/apis/core/types.go +++ b/pkg/apis/core/types.go @@ -2724,6 +2724,13 @@ type PodStatus struct { // A brief CamelCase message indicating details about why the pod is in this state. e.g. 'Evicted' // +optional Reason string + // nominatedNodeName is set when this pod preempts other pods on the node, but it cannot be + // scheduled right away as preemption victims receive their graceful termination periods. + // This field does not guarantee that the pod will be scheduled on this node. Scheduler may decide + // to place the pod elsewhere if other nodes become available sooner. Scheduler may also decide to + // give the resources on this node to a higher priority pod that is created after preemption. + // +optional + NominatedNodeName string // +optional HostIP string diff --git a/pkg/apis/core/v1/conversion.go b/pkg/apis/core/v1/conversion.go index 8f6e09b673e..ddba7ffcb53 100644 --- a/pkg/apis/core/v1/conversion.go +++ b/pkg/apis/core/v1/conversion.go @@ -163,7 +163,8 @@ func addConversionFuncs(scheme *runtime.Scheme) error { "spec.restartPolicy", "spec.schedulerName", "status.phase", - "status.podIP": + "status.podIP", + "status.nominatedNodeName": return label, value, nil // This is for backwards compatibility with old v1 clients which send spec.host case "spec.host": diff --git a/pkg/apis/core/validation/validation.go b/pkg/apis/core/validation/validation.go index 6dd17757266..16e1cb875ca 100644 --- a/pkg/apis/core/validation/validation.go +++ b/pkg/apis/core/validation/validation.go @@ -3374,6 +3374,12 @@ func ValidatePodStatusUpdate(newPod, oldPod *core.Pod) field.ErrorList { allErrs = append(allErrs, field.Forbidden(fldPath.Child("nodeName"), "may not be changed directly")) } + if newPod.Status.NominatedNodeName != oldPod.Status.NominatedNodeName && len(newPod.Status.NominatedNodeName) > 0 { + for _, msg := range ValidateNodeName(newPod.Status.NominatedNodeName, false) { + allErrs = append(allErrs, field.Invalid(fldPath.Child("nominatedNodeName"), newPod.Status.NominatedNodeName, msg)) + } + } + // If pod should not restart, make sure the status update does not transition // any terminated containers to a non-terminated state. allErrs = append(allErrs, ValidateContainerStateTransition(newPod.Status.ContainerStatuses, oldPod.Status.ContainerStatuses, fldPath.Child("containerStatuses"), oldPod.Spec.RestartPolicy)...) diff --git a/pkg/apis/core/validation/validation_test.go b/pkg/apis/core/validation/validation_test.go index 9c0a2fa6cf9..ae28b107344 100644 --- a/pkg/apis/core/validation/validation_test.go +++ b/pkg/apis/core/validation/validation_test.go @@ -7756,6 +7756,129 @@ func TestValidatePodUpdate(t *testing.T) { } } +func TestValidatePodStatusUpdate(t *testing.T) { + tests := []struct { + new core.Pod + old core.Pod + err string + test string + }{ + { + core.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: "foo", + }, + Spec: core.PodSpec{ + NodeName: "node1", + }, + Status: core.PodStatus{ + NominatedNodeName: "node1", + }, + }, + core.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: "foo", + }, + Spec: core.PodSpec{ + NodeName: "node1", + }, + Status: core.PodStatus{}, + }, + "", + "removed nominatedNodeName", + }, + { + core.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: "foo", + }, + Spec: core.PodSpec{ + NodeName: "node1", + }, + }, + core.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: "foo", + }, + Spec: core.PodSpec{ + NodeName: "node1", + }, + Status: core.PodStatus{ + NominatedNodeName: "node1", + }, + }, + "", + "add valid nominatedNodeName", + }, + { + core.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: "foo", + }, + Spec: core.PodSpec{ + NodeName: "node1", + }, + Status: core.PodStatus{ + NominatedNodeName: "Node1", + }, + }, + core.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: "foo", + }, + Spec: core.PodSpec{ + NodeName: "node1", + }, + }, + "nominatedNodeName", + "Add invalid nominatedNodeName", + }, + { + core.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: "foo", + }, + Spec: core.PodSpec{ + NodeName: "node1", + }, + Status: core.PodStatus{ + NominatedNodeName: "node1", + }, + }, + core.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: "foo", + }, + Spec: core.PodSpec{ + NodeName: "node1", + }, + Status: core.PodStatus{ + NominatedNodeName: "node2", + }, + }, + "", + "Update nominatedNodeName", + }, + } + + for _, test := range tests { + test.new.ObjectMeta.ResourceVersion = "1" + test.old.ObjectMeta.ResourceVersion = "1" + errs := ValidatePodStatusUpdate(&test.new, &test.old) + if test.err == "" { + if len(errs) != 0 { + t.Errorf("unexpected invalid: %s (%+v)\nA: %+v\nB: %+v", test.test, errs, test.new, test.old) + } + } else { + if len(errs) == 0 { + t.Errorf("unexpected valid: %s\nA: %+v\nB: %+v", test.test, test.new, test.old) + } else if actualErr := errs.ToAggregate().Error(); !strings.Contains(actualErr, test.err) { + t.Errorf("unexpected error message: %s\nExpected error: %s\nActual error: %s", test.test, test.err, actualErr) + } + } + } +} + func makeValidService() core.Service { return core.Service{ ObjectMeta: metav1.ObjectMeta{ diff --git a/pkg/printers/internalversion/describe.go b/pkg/printers/internalversion/describe.go index 7239652cb34..dc8ca82d2f0 100644 --- a/pkg/printers/internalversion/describe.go +++ b/pkg/printers/internalversion/describe.go @@ -651,6 +651,9 @@ func describePod(pod *api.Pod, events *api.EventList) (string, error) { if controlledBy := printController(pod); len(controlledBy) > 0 { w.Write(LEVEL_0, "Controlled By:\t%s\n", controlledBy) } + if len(pod.Status.NominatedNodeName) > 0 { + w.Write(LEVEL_0, "NominatedNodeName:\t%s\n", pod.Status.NominatedNodeName) + } if len(pod.Spec.InitContainers) > 0 { describeContainers("Init Containers", pod.Spec.InitContainers, pod.Status.InitContainerStatuses, EnvValueRetriever(pod), w, "") diff --git a/pkg/printers/internalversion/describe_test.go b/pkg/printers/internalversion/describe_test.go index 54947d5974f..83fc2d91a02 100644 --- a/pkg/printers/internalversion/describe_test.go +++ b/pkg/printers/internalversion/describe_test.go @@ -80,7 +80,8 @@ func TestDescribePodNode(t *testing.T) { NodeName: "all-in-one", }, Status: api.PodStatus{ - HostIP: "127.0.0.1", + HostIP: "127.0.0.1", + NominatedNodeName: "nodeA", }, }) c := &describeClient{T: t, Namespace: "foo", Interface: fake} @@ -92,6 +93,9 @@ func TestDescribePodNode(t *testing.T) { if !strings.Contains(out, "all-in-one/127.0.0.1") { t.Errorf("unexpected out: %s", out) } + if !strings.Contains(out, "nodeA") { + t.Errorf("unexpected out: %s", out) + } } func TestDescribePodTolerations(t *testing.T) { diff --git a/pkg/printers/internalversion/printers.go b/pkg/printers/internalversion/printers.go index 26f7b6d74b0..e6db8cfe3c8 100644 --- a/pkg/printers/internalversion/printers.go +++ b/pkg/printers/internalversion/printers.go @@ -609,6 +609,9 @@ func printPod(pod *api.Pod, options printers.PrintOptions) ([]metav1alpha1.Table nodeName = "" } row.Cells = append(row.Cells, podIP, nodeName) + if len(pod.Status.NominatedNodeName) > 0 { + row.Cells = append(row.Cells, pod.Status.NominatedNodeName) + } } return []metav1alpha1.TableRow{row}, nil diff --git a/pkg/printers/internalversion/printers_test.go b/pkg/printers/internalversion/printers_test.go index 8b512251fe5..d828cded6fb 100644 --- a/pkg/printers/internalversion/printers_test.go +++ b/pkg/printers/internalversion/printers_test.go @@ -1704,9 +1704,10 @@ func TestPrintPodwide(t *testing.T) { {Ready: true, RestartCount: 3, State: api.ContainerState{Running: &api.ContainerStateRunning{}}}, {RestartCount: 3}, }, + NominatedNodeName: "node1", }, }, - []metav1alpha1.TableRow{{Cells: []interface{}{"test1", "1/2", "podPhase", 6, "", "1.1.1.1", "test1"}}}, + []metav1alpha1.TableRow{{Cells: []interface{}{"test1", "1/2", "podPhase", 6, "", "1.1.1.1", "test1", "node1"}}}, }, { // Test when the NodeName and PodIP are none diff --git a/pkg/registry/core/pod/strategy.go b/pkg/registry/core/pod/strategy.go index 5cd0eb03cf7..1f2acb41a30 100644 --- a/pkg/registry/core/pod/strategy.go +++ b/pkg/registry/core/pod/strategy.go @@ -243,6 +243,7 @@ func PodToSelectableFields(pod *api.Pod) fields.Set { podSpecificFieldsSet["spec.schedulerName"] = string(pod.Spec.SchedulerName) podSpecificFieldsSet["status.phase"] = string(pod.Status.Phase) podSpecificFieldsSet["status.podIP"] = string(pod.Status.PodIP) + podSpecificFieldsSet["status.nominatedNodeName"] = string(pod.Status.NominatedNodeName) return generic.AddObjectMetaFieldsSet(podSpecificFieldsSet, &pod.ObjectMeta, true) } diff --git a/pkg/registry/core/pod/strategy_test.go b/pkg/registry/core/pod/strategy_test.go index d013292635b..2601083420f 100644 --- a/pkg/registry/core/pod/strategy_test.go +++ b/pkg/registry/core/pod/strategy_test.go @@ -114,7 +114,20 @@ func TestMatchPod(t *testing.T) { fieldSelector: fields.ParseSelectorOrDie("status.podIP=4.3.2.1"), expectMatch: false, }, - } + { + in: &api.Pod{ + Status: api.PodStatus{NominatedNodeName: "node1"}, + }, + fieldSelector: fields.ParseSelectorOrDie("status.nominatedNodeName=node1"), + expectMatch: true, + }, + { + in: &api.Pod{ + Status: api.PodStatus{NominatedNodeName: "node1"}, + }, + fieldSelector: fields.ParseSelectorOrDie("status.nominatedNodeName=node2"), + expectMatch: false, + }} for _, testCase := range testCases { m := MatchPod(labels.Everything(), testCase.fieldSelector) result, err := m.Matches(testCase.in) diff --git a/staging/src/k8s.io/api/core/v1/types.go b/staging/src/k8s.io/api/core/v1/types.go index 920afb05b6c..1974700bad1 100644 --- a/staging/src/k8s.io/api/core/v1/types.go +++ b/staging/src/k8s.io/api/core/v1/types.go @@ -3021,6 +3021,15 @@ type PodStatus struct { // e.g. 'Evicted' // +optional Reason string `json:"reason,omitempty" protobuf:"bytes,4,opt,name=reason"` + // nominatedNodeName is set only when this pod preempts other pods on the node, but it cannot be + // scheduled right away as preemption victims receive their graceful termination periods. + // This field does not guarantee that the pod will be scheduled on this node. Scheduler may decide + // to place the pod elsewhere if other nodes become available sooner. Scheduler may also decide to + // give the resources on this node to a higher priority pod that is created after preemption. + // As a result, this field may be different than PodSpec.nodeName when the pod is + // scheduled. + // +optional + NominatedNodeName string `json:"nominatedNodeName,omitempty" protobuf:"bytes,11,opt,name=nominatedNodeName"` // IP address of the host to which the pod is assigned. Empty if not yet scheduled. // +optional