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

sdn/eventqueue: handle DeletedFinalStateUnknown delta objects #11792

Merged
merged 1 commit into from Nov 8, 2016

Conversation

dcbw
Copy link
Member

@dcbw dcbw commented Nov 4, 2016

We'll get DeletedFinalStateUnknown objects instead of our expected
dela object when the object used to exist but whatever stocks the
queue calls Replace() because it doesn't have the final object state
at deletion. DeltaFIFO generates a DeletedFinalStateUnknown with
the current object state (which may be stale) but the key function
we were using (MetaNamespaceKeyFunc) and the processing functions
in the SDN modules weren't handling them.

None of our processing functions seem to care that the state may
be stale, so fix up the key function to handle DeletedFinalStateUnknown
and try to extract the expected object out of it when we get one.

Fixes bug 1389770 link

fixes #11794

@openshift/networking

@dcbw
Copy link
Member Author

dcbw commented Nov 4, 2016

[test]

@@ -19,6 +21,266 @@ func testKeyFunc(obj interface{}) (string, error) {
return key, nil
}

func deltaCompress(deltas cache.Deltas, keyFunc cache.KeyFunc) (newDeltas cache.Deltas, paniced bool, msg string) {
Copy link
Contributor

Choose a reason for hiding this comment

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

"panicked"

Copy link
Member Author

Choose a reason for hiding this comment

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

Fixed.

@danwinship
Copy link
Contributor

LGTM. Yay tests.
[test]

@danwinship danwinship closed this Nov 4, 2016
@danwinship
Copy link
Contributor

oops, misclick

@danwinship danwinship reopened this Nov 4, 2016
@dcbw dcbw force-pushed the deleted-final-state-unknown branch from fc2ad1e to 767f700 Compare November 4, 2016 19:14
@dcbw
Copy link
Member Author

dcbw commented Nov 4, 2016

re-[test] due to the mistaken close

@danwinship
Copy link
Contributor

flakes #11750, #11452. [test]

Copy link
Contributor

@knobunc knobunc left a comment

Choose a reason for hiding this comment

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

LGTM

@knobunc
Copy link
Contributor

knobunc commented Nov 7, 2016

[test]

@knobunc
Copy link
Contributor

knobunc commented Nov 7, 2016

[testextended][extended:core(idling)]

We'll get DeletedFinalStateUnknown objects instead of our expected
dela object when the object used to exist but whatever stocks the
queue calls Replace() because it doesn't have the final object state
at deletion.  DeltaFIFO generates a DeletedFinalStateUnknown with
the current object state (which may be stale) but the key function
we were using (MetaNamespaceKeyFunc) and the processing functions
in the SDN modules weren't handling them.

None of our processing functions seem to care that the state may
be stale, so fix up the key function to handle DeletedFinalStateUnknown
and try to extract the expected object out of it when we get one.

Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1389770
@dcbw dcbw force-pushed the deleted-final-state-unknown branch from 767f700 to c040b16 Compare November 7, 2016 19:18
@dcbw
Copy link
Member Author

dcbw commented Nov 7, 2016

Rebased and repsuhed to get fixes for idling tests that keep failing.

@openshift-bot
Copy link
Contributor

Evaluated for origin test up to c040b16

@openshift-bot
Copy link
Contributor

Evaluated for origin testextended up to c040b16

@openshift-bot
Copy link
Contributor

continuous-integration/openshift-jenkins/testextended SUCCESS (https://ci.openshift.redhat.com/jenkins/job/test_pr_origin_extended/759/) (Base Commit: 32708e7) (Extended Tests: core(idling))

@knobunc
Copy link
Contributor

knobunc commented Nov 7, 2016

[merge]

@openshift-bot
Copy link
Contributor

Evaluated for origin merge up to c040b16

@openshift-bot
Copy link
Contributor

continuous-integration/openshift-jenkins/test FAILURE (https://ci.openshift.redhat.com/jenkins/job/test_pr_origin/11228/) (Base Commit: 32708e7)

@openshift-bot
Copy link
Contributor

openshift-bot commented Nov 8, 2016

continuous-integration/openshift-jenkins/merge SUCCESS (https://ci.openshift.redhat.com/jenkins/job/test_pr_origin/11255/) (Base Commit: 07e3ce3) (Image: devenv-rhel7_5336)

@openshift-bot openshift-bot merged commit 7315d33 into openshift:master Nov 8, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

"unkeyable object" in sentry
4 participants