-
Notifications
You must be signed in to change notification settings - Fork 182
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: dedupe for events FailedToSchedule nodes #372
Conversation
@engedaam can you kindly add a detailed description to the PR that describes what this fix does? |
Pull Request Test Coverage Report for Build 5281819940
💛 - Coveralls |
Pull Request Test Coverage Report for Build 5284091132
💛 - Coveralls |
6ba46b3
to
13933a3
Compare
13933a3
to
bee0f04
Compare
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, but I'll let the others approve.
bee0f04
to
9b480c5
Compare
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 🚀 Nice work!
@@ -406,6 +406,28 @@ var _ = Describe("Requirements", func() { | |||
Expect(reqs.NodeSelectorRequirements()).To(HaveLen(14)) | |||
}) | |||
}) | |||
Context("Stringify Requirements", func() { |
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.
thanks for adding a UT!
Fixes #
Description
Karpenter was not sorting scheduling requirements strings for it's events, which created the notion that one scheduling requirements was represented in multiple ways.
How was this change tested?
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.