Skip to content
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

Fix panic condition in Pod await logic #998

Merged
merged 1 commit into from
Feb 19, 2020

Conversation

lblackstone
Copy link
Member

Proposed changes

For events received by the Pod awaiter, explicitly check that
the event has a non-nil Object to avoid a possible panic.

Related issues (optional)

Fixes #994

For events received by the Pod awaiter, explicitly check that
the event has a non-nil Object to avoid a possible panic.
@@ -197,6 +197,10 @@ func (pia *podInitAwaiter) Read() error {
}

func (pia *podInitAwaiter) processPodEvent(event watch.Event) {
if event.Object == nil {
glog.V(3).Infof("received event with nil Object: %#v", event)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Under what circumstances does this occur?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the linked issue, it seemed like it was caused by a network interruption.

@pgavlin
Copy link
Member

pgavlin commented Feb 19, 2020

From #994 (comment):

I'm guessing that one of the API's that talks to the K8S cluster returned nil (and possibly an unchecked error)

Did we look for any unchecked errors?

@lblackstone
Copy link
Member Author

@pgavlin That event is coming from a watch channel, so there's no error to check.

@pgavlin
Copy link
Member

pgavlin commented Feb 19, 2020

@pgavlin That event is coming from a watch channel, so there's no error to check.

Hm. Is this due to the way we've written it? Seems like we must be dropping an error somewhere... Can you file an issue for followup?

@lblackstone
Copy link
Member Author

Seems like we must be dropping an error somewhere...

I could switch on the event type to check for an Error type, but I don't know for sure if it would be set properly. The important thing is the check the type cast rather than panicking; if the event is invalid, we should be able to safely ignore it.

@lblackstone lblackstone merged commit 813ceda into master Feb 19, 2020
@pulumi-bot pulumi-bot deleted the lblackstone/fix-unstructured-panic branch February 19, 2020 20:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

panic: interface conversion: runtime.Object is nil, not *unstructured.Unstructured
2 participants