Fix nil pointer panic in npc/namespace.go #3833

gobomb · 2020-07-24T10:28:18Z

Try to solve this issue #3836 .

The *coreapi.Namespace in struct ns may be set to nil in func deleteNamespace before func deletePod called，it will cause a pinic when deletePod use the namespace pointer.

I use a new map to store the lables of namespace, when the pointer is set to nil, it can avoid panic and work well.

DEBU: 2020/07/23 11:09:37.920737 EVENT DeletePod {"metadata":{"creationTimestamp":"2020-07-23T08:30:24Z","deletionGracePeriodSeconds":0,"deletionTimestamp":"2020-07-23T11:09:31Z","generateName":"bp-s2-745c79479b-","labels":{"App":"f43f2b4120f7e4adbb88ede557c74ec35","Blueprint":"bbp-s2","Service":"bp-s2","app":"bp-s2","flowid":"57","pod-template-hash":"745c79479b","version":"v1"},"name":"bp-s2-745c79479b-kq66f","namespace":"f43f2b4120f7e4adbb88ede557c74ec35","resourceVersion":"27784263","selfLink":"/api/v1/namespaces/f43f2b4120f7e4adbb88ede557c74ec35/pods/bp-s2-745c79479b-kq66f","uid":"9eb3ec80-d600-4d07-837f-7662cd6a3e0a"},"spec":{"affinity":{},"containers":[{"image":"harbor.cloud2go.cn/cloudtogo/network-multitool","imagePullPolicy":"Always","name":"bp-s2","ports":[{"containerPort":80,"name":"p80","protocol":"TCP"}],"readinessProbe":{"failureThreshold":3,"initialDelaySeconds":1,"periodSeconds":5,"successThreshold":1,"tcpSocket":{"port":80},"timeoutSeconds":1},"terminationMessagePath":"/dev/termination-log","terminationMessagePolicy":"FallbackToLogsOnError"}],"dnsConfig":{"options":[{"name":"ndots","value":"1"}]},"dnsPolicy":"ClusterFirst","nodeName":"n1.env.lab.io","restartPolicy":"Always","schedulerName":"default-scheduler","securityContext":{},"serviceAccount":"default","serviceAccountName":"default","terminationGracePeriodSeconds":0},"status":{"conditions":[{"lastProbeTime":null,"lastTransitionTime":"2020-07-23T08:30:24Z","status":"True","type":"Initialized"},{"lastProbeTime":null,"lastTransitionTime":"2020-07-23T08:31:48Z","status":"True","type":"Ready"},{"lastProbeTime":null,"lastTransitionTime":"2020-07-23T08:31:48Z","status":"True","type":"ContainersReady"},{"lastProbeTime":null,"lastTransitionTime":"2020-07-23T08:30:24Z","status":"True","type":"PodScheduled"}],"hostIP":"10.10.13.8","phase":"Running","podIP":"10.20.192.88","qosClass":"BestEffort","startTime":"2020-07-23T08:30:24Z"}}
INFO: 2020/07/23 11:09:37.939904 deleting entry 10.20.192.88 from weave-Cz*b([u5~[;?3vXY}8w6UH?f3 of 9eb3ec80-d600-4d07-837f-7662cd6a3e0a
INFO: 2020/07/23 11:09:37.939958 deleting entry 10.20.192.88 from weave-5$otwpJo/[g4!?%N8=f+9t=Au of 9eb3ec80-d600-4d07-837f-7662cd6a3e0a
INFO: 2020/07/23 11:09:37.939981 deleting entry 10.20.192.88 from weave-vfZR:H@W@vf0XI[v.hMddEGJF of 9eb3ec80-d600-4d07-837f-7662cd6a3e0a
INFO: 2020/07/23 11:09:37.940007 deleted entry 10.20.192.88 from weave-vfZR:H@W@vf0XI[v.hMddEGJF of 9eb3ec80-d600-4d07-837f-7662cd6a3e0a
ERROR: logging before flag.Parse: E0723 11:09:37.946420    1331 runtime.go:66] Observed a panic: "invalid memory address or nil pointer dereference" (runtime error: invalid memory address or nil pointer dereference)
/go/src/github.com/weaveworks/weave/vendor/k8s.io/apimachinery/pkg/util/runtime/runtime.go:72
/go/src/github.com/weaveworks/weave/vendor/k8s.io/apimachinery/pkg/util/runtime/runtime.go:65
/go/src/github.com/weaveworks/weave/vendor/k8s.io/apimachinery/pkg/util/runtime/runtime.go:51
/usr/local/go/src/runtime/panic.go:679
/usr/local/go/src/runtime/panic.go:199
/usr/local/go/src/runtime/signal_unix.go:394
/go/src/github.com/weaveworks/weave/npc/namespace.go:336
/go/src/github.com/weaveworks/weave/npc/controller.go:144
/go/src/github.com/weaveworks/weave/npc/controller.go:106
/go/src/github.com/weaveworks/weave/npc/controller.go:143
/go/src/github.com/weaveworks/weave/prog/weave-npc/main.go:287
/go/src/github.com/weaveworks/weave/vendor/k8s.io/client-go/tools/cache/controller.go:209
/go/src/github.com/weaveworks/weave/vendor/k8s.io/client-go/tools/cache/controller.go:320
/go/src/github.com/weaveworks/weave/vendor/k8s.io/client-go/tools/cache/delta_fifo.go:444
/go/src/github.com/weaveworks/weave/vendor/k8s.io/client-go/tools/cache/controller.go:150
/go/src/github.com/weaveworks/weave/vendor/k8s.io/apimachinery/pkg/util/wait/wait.go:133
/go/src/github.com/weaveworks/weave/vendor/k8s.io/apimachinery/pkg/util/wait/wait.go:134
/go/src/github.com/weaveworks/weave/vendor/k8s.io/apimachinery/pkg/util/wait/wait.go:88
/go/src/github.com/weaveworks/weave/vendor/k8s.io/client-go/tools/cache/controller.go:124
/usr/local/go/src/runtime/asm_amd64.s:1357

bboreham · 2020-07-27T15:31:01Z

What exactly gets set to nil? Where?

It's probably a good idea to file the problem as an issue, with this PR referring to it, in case there are different ways to solve it.

bboreham · 2020-07-27T15:31:18Z

Oh, and thanks for the PR.

gobomb · 2020-07-28T02:53:46Z

Do I still need to open an issue and provide more info?

bboreham · 2020-07-28T06:19:33Z

My previous comment stands, yes.

bboreham

Thanks, this is really great. I have a couple of suggestions below.

Did you look into adding a test for this case?

bboreham · 2020-07-27T15:28:29Z

npc/namespace.go

@@ -3,7 +3,6 @@ package npc
 import (
 	"errors"
 	"fmt"
-


Please leave this gap between different classes of imports.

bboreham · 2020-07-29T09:49:01Z

npc/namespace.go

-	policies  map[types.UID]interface{}  // k8s NetworkPolicy objects by UID
+	name            string                     // k8s Namespace name
+	nodeName        string                     // my node name
+	namespace       *coreapi.Namespace         // k8s Namespace object


It looks like the UID is the only other thing that we use from namespace, so I suggest to copy that out and lose namespace, so we don't have to worry about setting it to nil.

bboreham · 2020-08-03T10:59:35Z

npc/namespace.go

@@ -508,23 +511,25 @@ func (ns *ns) addNamespace(obj *coreapi.Namespace) error {
 }

 func (ns *ns) updateNamespace(oldObj, newObj *coreapi.Namespace) error {
-	ns.namespace = newObj
+	ns.namespaceUID = newObj.ObjectMeta.UID


This can't change, can it?

bboreham · 2020-08-03T11:00:29Z

Thanks for making all those changes - it's a bit more than I had thought (sorry!).
I'm just looking at fixing up the merge conflicts which came in with #3726

bboreham · 2020-08-03T11:08:51Z

Merge conflicts resolved at master...fix-nil-ns-pointer

bboreham

Thanks! LGTM

gobomb force-pushed the fix-nil-ns-pointer branch 2 times, most recently from 7b019ab to 82209f2 Compare July 27, 2020 06:51

gobomb mentioned this pull request Jul 29, 2020

weave-npc panic after delete a namespace in k8s cluster #3836

Closed

bboreham reviewed Jul 29, 2020

View reviewed changes

bboreham added this to the 2.7 milestone Jul 29, 2020

add namespaceLabels and namespaceUID, remove namespace in ns struct

0789670

gobomb force-pushed the fix-nil-ns-pointer branch from 82209f2 to 0789670 Compare August 2, 2020 13:08

add fake namespace uid for npc test

2ec3e1e

gobomb force-pushed the fix-nil-ns-pointer branch from e5e9a37 to 2ec3e1e Compare August 2, 2020 15:42

gobomb changed the title ~~Fix nil porinter panic in npc/namespace.go~~ Fix nil pointer panic in npc/namespace.go Aug 3, 2020

bboreham reviewed Aug 3, 2020

View reviewed changes

bboreham approved these changes Aug 3, 2020

View reviewed changes

bboreham merged commit f9e27f7 into weaveworks:master Aug 3, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix nil pointer panic in npc/namespace.go #3833

Fix nil pointer panic in npc/namespace.go #3833

gobomb commented Jul 24, 2020 •

edited

Loading

bboreham commented Jul 27, 2020

bboreham commented Jul 27, 2020

gobomb commented Jul 28, 2020

bboreham commented Jul 28, 2020

bboreham left a comment

bboreham Jul 27, 2020

bboreham Jul 29, 2020

bboreham Aug 3, 2020

bboreham commented Aug 3, 2020

bboreham commented Aug 3, 2020

bboreham left a comment

Fix nil pointer panic in npc/namespace.go #3833

Fix nil pointer panic in npc/namespace.go #3833

Conversation

gobomb commented Jul 24, 2020 • edited Loading

bboreham commented Jul 27, 2020

bboreham commented Jul 27, 2020

gobomb commented Jul 28, 2020

bboreham commented Jul 28, 2020

bboreham left a comment

Choose a reason for hiding this comment

bboreham Jul 27, 2020

Choose a reason for hiding this comment

bboreham Jul 29, 2020

Choose a reason for hiding this comment

bboreham Aug 3, 2020

Choose a reason for hiding this comment

bboreham commented Aug 3, 2020

bboreham commented Aug 3, 2020

bboreham left a comment

Choose a reason for hiding this comment

gobomb commented Jul 24, 2020 •

edited

Loading