-
Notifications
You must be signed in to change notification settings - Fork 736
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Check to make sure it is a Pod object #170
Conversation
@@ -208,7 +208,7 @@ func (d *Controller) handlePodUpdate(key string) error { | |||
} | |||
|
|||
pod, ok := obj.(*v1.Pod) | |||
if !ok { | |||
if !ok || pod == nil { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What circumstances cause pod to be nil?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@nckturner it happens once in @rtripat setup once. I am not able to reproduce it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok well it looks to me like it might help prevent #168 but I'm wondering why its receiving nil
in the first place. I am hesitant to approve this because I am afraid it will just help mask an issue.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This implies obj was an object of type *v1.Pod, with a value of nil.
nil != *v1.Pod(nil)
Yes, Go is weird.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this is OK.
A nil interface is != nil but a nil interface converted to struct is nil and pod is a struct.
https://play.golang.org/p/uAZJG2_mUuL
Unless I am missing something.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
I agree with nic that we should investigate further why we can get a nil pod here if exists is true. Somewhere we are storing nil in the datastore.
Issue #168, :
Description of changes:
Check to make sure the object is a Pod type
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.