Fix panic when kubelet register if a node object already exists with no Status.Capacity or Status.Allocatable #95269

SataQiu · 2020-10-03T06:22:56Z

What type of PR is this?
/kind bug

What this PR does / why we need it:
Fix panic when kubelet register if a node object already exists with no Status.Capacity or Status.Allocatable

Which issue(s) this PR fixes:

Fixes #95188

Special notes for your reviewer:

Does this PR introduce a user-facing change?:

fix the panic when kubelet registers if a node object already exists with no Status.Capacity or Status.Allocatable

Additional documentation e.g., KEPs (Kubernetes Enhancement Proposals), usage docs, etc.:

SataQiu · 2020-10-03T06:23:48Z

/sig node

odinuge

/lgtm
/retest

This should have a proper changelog entry, and should be backported to previous releases.
/hold

NicolasT · 2020-10-05T12:56:03Z

pkg/kubelet/kubelet_node_status.go

@@ -129,6 +129,24 @@ func (kl *Kubelet) reconcileHugePageResource(initialNode, existingNode *v1.Node)
 	requiresUpdate := false
 	supportedHugePageResources := sets.String{}

+	if existingNode.Status.Capacity == nil {


Could consider factoring out this code (and the block below) which would allow for some extra unit-tests.

Thank you for your review @NicolasT . Is there any more detailed reconstruction plan?

Not sure I understand the question? You basically have two (+-) 12-line code blocks that are roughly the same, parametrized over two Node objects, 'returning' (so to say) some bool. Sounds like a good opportunity to turn that functionality into a function which then you call twice, and for which some unit-test(s) can be written.

Sounds good!

SataQiu · 2020-10-09T05:51:05Z

/assign @derekwaynecarr

SataQiu · 2020-10-13T01:32:37Z

/triage accepted
/priority important-soon

SataQiu · 2020-11-04T12:11:17Z

Hey @thockin!

I am ok with merging this no relnote, so feel free to unhold. Tho., in my experience the patch release managers prefer to have relnotes when backporting, to actually show what changes (and therefore it is nice to have it here as well).

/lgtm

@odinuge Thanks for your reminding. I have added a release note. If you have a better one, please feel free to feedback.

odinuge · 2020-11-04T12:32:01Z

Nice @SataQiu!

/hold cancel

bobbypage · 2020-11-04T20:03:10Z

pkg/kubelet/kubelet_node_status_test.go

+		needsUpdate  bool
+	}{
+		{
+			name: "no update needed when capacity and available of the existing node are not nil",


nit s/available/allocatable in name here and rest of test cases

Thanks a lot @bobbypage ! The nit has been fixed.

bobbypage · 2020-11-04T20:08:27Z

/lgtm

Thanks for the fix! Have you thought about also proposing cherrypicking this to 1.19?

NicolasT · 2020-11-04T20:34:47Z

Such backport would be appreciated: next to our original report, where we discovered the issue in 1.19 (essentially making 1.19 incompatible with our deployment mechanism), there appears to be more interest (#95188 (comment)).

…with no Status.Capacity or Status.Allocatable Signed-off-by: SataQiu <1527062125@qq.com>

SataQiu · 2020-11-06T09:28:06Z

Such backport would be appreciated: next to our original report, where we discovered the issue in 1.19 (essentially making 1.19 incompatible with our deployment mechanism), there appears to be more interest (#95188 (comment)).

A PR for release-1.19 has been filed. (#96297)

bobbypage · 2020-11-20T23:08:34Z

/lgtm

NicolasT · 2020-11-24T15:50:48Z

/assign @dchen1107

derekwaynecarr · 2020-12-01T14:57:15Z

/lgtm
/approve

k8s-ci-robot · 2020-12-01T14:57:48Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: derekwaynecarr, NicolasT, odinuge, SataQiu, TeddyAndrieux

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~~ [derekwaynecarr]

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

cherry pick of #95269: fix the panic when kubelet registers if a node object already exists with no Status.Capacity or Status.Allocatable

…c-1.20 cherry pick of #95269: fix the panic when kubelet registers if a node object already exists with no Status.Capacity or Status.Allocatable

k8s-ci-robot requested review from feiskyer and krmayankk October 3, 2020 06:23

k8s-ci-robot added area/kubelet sig/node Categorizes an issue or PR as relevant to SIG Node. and removed needs-sig Indicates an issue or PR lacks a `sig/foo` label and requires one. labels Oct 3, 2020

odinuge approved these changes Oct 3, 2020

View reviewed changes

k8s-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Oct 3, 2020

k8s-ci-robot assigned odinuge Oct 3, 2020

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Oct 3, 2020

SataQiu force-pushed the kubelet-20201003 branch from edd436a to c3f107f Compare October 3, 2020 10:29

k8s-ci-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Oct 3, 2020

SataQiu force-pushed the kubelet-20201003 branch from c3f107f to 57d2cbd Compare October 3, 2020 14:27

TeddyAndrieux mentioned this pull request Oct 5, 2020

Kubelet: fix reconcileHugePageResource when Node already exists #95295

Closed

NicolasT reviewed Oct 5, 2020

View reviewed changes

k8s-ci-robot assigned derekwaynecarr Oct 9, 2020

SataQiu force-pushed the kubelet-20201003 branch from 57d2cbd to 6776a9d Compare October 12, 2020 04:10

NicolasT approved these changes Oct 12, 2020

View reviewed changes

thockin assigned sjenning and dchen1107 Nov 2, 2020

thockin mentioned this pull request Nov 2, 2020

Implement ServiceSpec.AllocateLoadBalancerNodePorts #92744

Merged

k8s-ci-robot added release-note Denotes a PR that will be considered when it comes time to generate release notes. and removed release-note-none Denotes a PR that doesn't merit a release note. labels Nov 4, 2020

k8s-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Nov 4, 2020

bobbypage reviewed Nov 4, 2020

View reviewed changes

fix the panic when kubelet registers if a node object already exists …

c73ea0a

…with no Status.Capacity or Status.Allocatable Signed-off-by: SataQiu <1527062125@qq.com>

SataQiu force-pushed the kubelet-20201003 branch from 6776a9d to c73ea0a Compare November 6, 2020 09:10

k8s-ci-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Nov 6, 2020

SataQiu mentioned this pull request Nov 6, 2020

cherry pick of #95269: fix the panic when kubelet registers if a node object already exists with no Status.Capacity or Status.Allocatable #96297

Merged

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Nov 20, 2020

k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Dec 1, 2020

justaugustus added this to the v1.21 milestone Dec 1, 2020

NicolasT mentioned this pull request Dec 4, 2020

Kubelet register fail if a node object already exists with no "Status.Capacity" #95188

Closed

k8s-ci-robot merged commit e40cba5 into kubernetes:master Dec 9, 2020

github-actions bot mentioned this pull request Dec 16, 2020

Week Ending December 13, 2020 dev-obs/actus#308

Open

k8s-ci-robot added a commit that referenced this pull request Dec 16, 2020

Merge pull request #96297 from SataQiu/cherry-pick-kubelet-panic-1.19

0ca9e41

cherry pick of #95269: fix the panic when kubelet registers if a node object already exists with no Status.Capacity or Status.Allocatable

rphillips mentioned this pull request Dec 16, 2020

Bug 1908407: UPSTREAM: 95269: Fix panic when kubelet register if a node object already exists with no Status.Capacity or Status.Allocatable openshift/kubernetes#493

Merged

TeddyAndrieux mentioned this pull request Jan 7, 2021

cherry pick of #95269: fix the panic when kubelet registers if a node object already exists with no Status.Capacity or Status.Allocatable #97803

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix panic when kubelet register if a node object already exists with no Status.Capacity or Status.Allocatable #95269

Fix panic when kubelet register if a node object already exists with no Status.Capacity or Status.Allocatable #95269

SataQiu commented Oct 3, 2020 •

edited

Loading

SataQiu commented Oct 3, 2020

odinuge left a comment

NicolasT Oct 5, 2020

SataQiu Oct 9, 2020

NicolasT Oct 9, 2020

SataQiu Oct 10, 2020

SataQiu Oct 12, 2020

SataQiu commented Oct 9, 2020

SataQiu commented Oct 13, 2020

SataQiu commented Nov 4, 2020

odinuge commented Nov 4, 2020

bobbypage Nov 4, 2020 •

edited

Loading

SataQiu Nov 6, 2020

bobbypage commented Nov 4, 2020

NicolasT commented Nov 4, 2020

SataQiu commented Nov 6, 2020 •

edited

Loading

bobbypage commented Nov 20, 2020

NicolasT commented Nov 24, 2020

derekwaynecarr commented Dec 1, 2020

k8s-ci-robot commented Dec 1, 2020

Fix panic when kubelet register if a node object already exists with no Status.Capacity or Status.Allocatable #95269

Fix panic when kubelet register if a node object already exists with no Status.Capacity or Status.Allocatable #95269

Conversation

SataQiu commented Oct 3, 2020 • edited Loading

SataQiu commented Oct 3, 2020

odinuge left a comment

Choose a reason for hiding this comment

NicolasT Oct 5, 2020

Choose a reason for hiding this comment

SataQiu Oct 9, 2020

Choose a reason for hiding this comment

NicolasT Oct 9, 2020

Choose a reason for hiding this comment

SataQiu Oct 10, 2020

Choose a reason for hiding this comment

SataQiu Oct 12, 2020

Choose a reason for hiding this comment

SataQiu commented Oct 9, 2020

SataQiu commented Oct 13, 2020

SataQiu commented Nov 4, 2020

odinuge commented Nov 4, 2020

bobbypage Nov 4, 2020 • edited Loading

Choose a reason for hiding this comment

SataQiu Nov 6, 2020

Choose a reason for hiding this comment

bobbypage commented Nov 4, 2020

NicolasT commented Nov 4, 2020

SataQiu commented Nov 6, 2020 • edited Loading

bobbypage commented Nov 20, 2020

NicolasT commented Nov 24, 2020

derekwaynecarr commented Dec 1, 2020

k8s-ci-robot commented Dec 1, 2020

SataQiu commented Oct 3, 2020 •

edited

Loading

bobbypage Nov 4, 2020 •

edited

Loading

SataQiu commented Nov 6, 2020 •

edited

Loading