Reconcile extended resource capacity after kubelet restart. #64784

jiayingz · 2018-06-05T18:41:26Z

What this PR does / why we need it:

Which issue(s) this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close the issue(s) when PR gets merged):
Fixes #64632

Special notes for your reviewer:

Release note:

Kubelet will set extended resource capacity to zero after it restarts. If the extended resource is exported by a device plugin, its capacity will change to a valid value after the device plugin re-connects with the Kubelet. If the extended resource is exported by an external component through direct node status capacity patching, the component should repatch the field after kubelet becomes ready again. During the time gap, pods previously assigned with such resources may fail kubelet admission but their controller should create new pods in response to such failures.

jiayingz · 2018-06-05T18:52:40Z

/sig node

jiayingz · 2018-06-05T18:53:24Z

/assign @vishh

jiayingz · 2018-06-05T20:13:38Z

/cc @ConnorDoyle please let us know asap if you have any concerns on this change.

ConnorDoyle

This seems to be a safe workaround to the problem described in #64632, especially with additional context provided by @vishh here.

Is this true? With this patch, pods that consume extended resources can no longer survive a Kubelet restart because they will fail admission. If so, could we add that to the release note?

ConnorDoyle · 2018-06-05T20:55:46Z

pkg/kubelet/kubelet_node_status.go

+	requiresUpdate := false
+	for k := range node.Status.Capacity {
+		if v1helper.IsExtendedResourceName(k) {
+			node.Status.Capacity[k] = *resource.NewQuantity(int64(0), resource.DecimalSI)


For sanity's sake, should this also set that resource's allocatable value to zero? Otherwise this could temporarily set capacity such that allocatable > capacity. As is, allocatable would be overwritten to zero on the next kubelet sync iteration.

jiayingz · 2018-06-05T21:48:49Z

@ConnorDoyle Thanks a lot for the comment! I updated the release note to clarify the pod failure behavior associated with the change. PTAL.

jiayingz · 2018-06-05T22:04:40Z

/test pull-kubernetes-local-e2e-containerized

RenaudWasTaken · 2018-06-05T22:35:07Z

pkg/kubelet/kubelet_node_status_test.go

+		{
+			name: "no update needed without extended resource",
+			existingNode: &v1.Node{
+				Status: v1.NodeStatus{


Should this test check Allocatable?

Ah my bad. I did extend the test to check allocatable but forgot to push the change.

Ah you just updated it!

vishh · 2018-06-05T22:45:37Z

@jiayingz what will be the behavior after this PR for trivial kubelet restarts? Will the kubelet update capacity/allocatable even though it has a valid checkpoint?

jiayingz · 2018-06-05T22:53:47Z

For device plugin resource, even with a valid checkpoint, we already sets the resource capacity/allocatable to zero since #60856 to make sure no new pods will get assigned to the node till the device plugin re-connects. Existing pods already assigned with the resource can continue though with the valid checkpoint in place. That is the logic covered from device manager Allocate().

vishh · 2018-06-05T23:45:30Z

/lgtm /approve

k8s-ci-robot · 2018-06-05T23:46:16Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: jiayingz, vishh

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~pkg/kubelet/OWNERS~~ [vishh]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

jiayingz · 2018-06-06T00:45:04Z

/kind bug

k8s-github-robot · 2018-06-06T00:51:01Z

[MILESTONENOTIFIER] Milestone Pull Request: Up-to-date for process

@jiayingz @vishh

Pull Request Labels

sig/node: Pull Request will be escalated to these SIGs if needed.
priority/critical-urgent: Never automatically move pull request out of a release milestone; continually escalate to contributor and SIG through all available channels.
kind/bug: Fixes a bug discovered during the current release.

Help

k8s-github-robot · 2018-06-06T08:24:19Z

Automatic merge from submit-queue (batch tested with PRs 63717, 64646, 64792, 64784, 64800). If you want to cherry-pick this change to another branch, please follow the instructions here.

…84-upstream-release-1.10 Automatic merge from submit-queue. Automated cherry pick of #64784: Reconcile extended resource capacity after kubelet restart. Cherry pick of #64784 on release-1.10. #64784: Reconcile extended resource capacity after kubelet restart.

k8s-ci-robot added release-note Denotes a PR that will be considered when it comes time to generate release notes. size/M Denotes a PR that changes 30-99 lines, ignoring generated files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels Jun 5, 2018

k8s-ci-robot requested review from ncdc and sjpotter June 5, 2018 18:42

k8s-ci-robot added the sig/node Categorizes an issue or PR as relevant to SIG Node. label Jun 5, 2018

k8s-ci-robot assigned vishh Jun 5, 2018

jiayingz force-pushed the status-ready branch from 0a6d75c to abb1169 Compare June 5, 2018 19:56

jiayingz mentioned this pull request Jun 5, 2018

From 1.11, an extended resource reported by a device plugin can be left on a node after node upgrade even though its device plugin never registers back #64632

Closed

ConnorDoyle reviewed Jun 5, 2018

View reviewed changes

Reconcile extended resource capacity after kubelet restart.

35efc4f

RenaudWasTaken reviewed Jun 5, 2018

View reviewed changes

jiayingz force-pushed the status-ready branch from abb1169 to 35efc4f Compare June 5, 2018 22:37

k8s-ci-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Jun 5, 2018

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jun 5, 2018

k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jun 5, 2018

vishh added this to the v1.11 milestone Jun 6, 2018

vishh added priority/critical-urgent Highest priority. Must be actively worked on as someone's top priority right now. status/approved-for-milestone labels Jun 6, 2018

k8s-ci-robot added the kind/bug Categorizes issue or PR as related to a bug. label Jun 6, 2018

k8s-github-robot merged commit a32e5b6 into kubernetes:master Jun 6, 2018

jiayingz mentioned this pull request Aug 2, 2018

Automated cherry pick of #64784: Reconcile extended resource capacity after kubelet restart. #66881

Merged

ddongchen mentioned this pull request Mar 29, 2019

make pod with extended resources pass predicate check when kubelet restart #75815

Closed

chardch mentioned this pull request Dec 2, 2019

Jobs failing when a node is preempted kubeflow/training-operator#999

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Reconcile extended resource capacity after kubelet restart. #64784

Reconcile extended resource capacity after kubelet restart. #64784

jiayingz commented Jun 5, 2018 •

edited

Loading

jiayingz commented Jun 5, 2018

jiayingz commented Jun 5, 2018

jiayingz commented Jun 5, 2018

ConnorDoyle left a comment

ConnorDoyle Jun 5, 2018

jiayingz Jun 5, 2018

jiayingz commented Jun 5, 2018

jiayingz commented Jun 5, 2018

RenaudWasTaken Jun 5, 2018

jiayingz Jun 5, 2018

RenaudWasTaken Jun 5, 2018

vishh commented Jun 5, 2018

jiayingz commented Jun 5, 2018

vishh commented Jun 5, 2018 via email

k8s-ci-robot commented Jun 5, 2018

jiayingz commented Jun 6, 2018

k8s-github-robot commented Jun 6, 2018

k8s-github-robot commented Jun 6, 2018

Reconcile extended resource capacity after kubelet restart. #64784

Reconcile extended resource capacity after kubelet restart. #64784

Conversation

jiayingz commented Jun 5, 2018 • edited Loading

jiayingz commented Jun 5, 2018

jiayingz commented Jun 5, 2018

jiayingz commented Jun 5, 2018

ConnorDoyle left a comment

Choose a reason for hiding this comment

ConnorDoyle Jun 5, 2018

Choose a reason for hiding this comment

jiayingz Jun 5, 2018

Choose a reason for hiding this comment

jiayingz commented Jun 5, 2018

jiayingz commented Jun 5, 2018

RenaudWasTaken Jun 5, 2018

Choose a reason for hiding this comment

jiayingz Jun 5, 2018

Choose a reason for hiding this comment

RenaudWasTaken Jun 5, 2018

Choose a reason for hiding this comment

vishh commented Jun 5, 2018

jiayingz commented Jun 5, 2018

vishh commented Jun 5, 2018 via email

k8s-ci-robot commented Jun 5, 2018

jiayingz commented Jun 6, 2018

k8s-github-robot commented Jun 6, 2018

k8s-github-robot commented Jun 6, 2018

jiayingz commented Jun 5, 2018 •

edited

Loading