You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@yunikorn.apache.org by GitBox <gi...@apache.org> on 2022/08/04 16:07:10 UTC
[GitHub] [yunikorn-k8shim] craigcondit commented on a diff in pull request #444: [YUNIKORN-1274] Remove FailedScheduling pod events from predicate logic
craigcondit commented on code in PR #444:
URL: https://github.com/apache/yunikorn-k8shim/pull/444#discussion_r937986884
##########
test/e2e/predicates/predicates_test.go:
##########
@@ -251,31 +243,25 @@ var _ = Describe("Predicates", func() {
},
}
- var success bool
initPod, podErr := k8s.InitTestPod(conf)
Ω(podErr).NotTo(HaveOccurred())
- success, err = kClient.WaitForSchedulerAfterAction(kClient.CreateTestPodAction(initPod, ns), ns, podName, false)
- Ω(err).NotTo(HaveOccurred())
- Ω(success).Should(BeTrue())
+ _, podErr = kClient.CreatePod(initPod, ns)
+ Ω(podErr).NotTo(HaveOccurred())
By(fmt.Sprintf("Verify pod:%s is in pending state", podName))
- err = kClient.WaitForPodPending(ns, podName, time.Duration(60)*time.Second)
- Ω(err).NotTo(HaveOccurred())
+ podErr = kClient.WaitForPodPending(ns, podName, time.Duration(60)*time.Second)
+ Ω(podErr).NotTo(HaveOccurred())
- By("Verify the Yunikorn events: failed scheduling")
- var events *v1.EventList
- events, err = kClient.GetEvents(ns)
- Ω(err).NotTo(HaveOccurred())
- Ω(len(events.Items)).NotTo(BeZero(), "Events cant be empty")
- for _, event := range events.Items {
- framework.Logf("Event source is : %s", event.Source.Component)
- if event.Source.Component == configmanager.SchedulerName && event.Type == WARNING {
- framework.Logf("Yunikorn Event Reason: %s", event.Reason)
- Ω(event.Reason).Should(Equal("FailedScheduling"), "Event reason mismatch")
- framework.Logf("Yunikorn Event Message: %s", event.Message)
- Ω(event.Message).Should(MatchRegexp("predicate is not satisfied.*didn't match Pod's node affinity"), "Event message mismatch")
- }
- }
+ By("Verify the YuniKorn request failed scheduling")
+
+ podErr = restClient.WaitForAllocationLog("default", "root."+ns, initPod.ObjectMeta.Labels["applicationId"], podName, 60)
+ Ω(podErr).NotTo(HaveOccurred())
+ log, podErr := restClient.GetAllocationLog("default", "root."+ns, initPod.ObjectMeta.Labels["applicationId"], podName)
+ Ω(podErr).NotTo(HaveOccurred())
+ Ω(len(log)).NotTo(BeZero(), "Log can't be empty")
+
+ Ω(log).Should(ContainElement(HaveKeyWithValue("message", MatchRegexp(".*didn't match Pod's node affinity"))),
Review Comment:
These match against the message the K8s predicates generate. We purposely keep them generic so as to be more flexible when new K8s versions may change the wording slightly. There's not really a risk of false positives, as only messages from predicates are logged to the allocation log.
##########
test/e2e/predicates/predicates_test.go:
##########
@@ -410,31 +396,25 @@ var _ = Describe("Predicates", func() {
NodeSelector: map[string]string{labelKey: labelValue},
}
- var success bool
initPod, podErr := k8s.InitTestPod(conf)
Ω(podErr).NotTo(HaveOccurred())
- success, err = kClient.WaitForSchedulerAfterAction(kClient.CreateTestPodAction(initPod, ns), ns, podNameNoTolerations, false)
+ _, err := kClient.CreatePod(initPod, ns)
Ω(err).NotTo(HaveOccurred())
- Ω(success).Should(BeTrue())
By(fmt.Sprintf("Verify pod:%s is in pending state", podNameNoTolerations))
err = kClient.WaitForPodPending(ns, podNameNoTolerations, time.Duration(60)*time.Second)
Ω(err).NotTo(HaveOccurred())
- By("Verify the Yunikorn events: failed scheduling")
- var events *v1.EventList
- events, err = kClient.GetEvents(ns)
+ By("Verify the YuniKorn request failed scheduling")
+
+ err = restClient.WaitForAllocationLog("default", "root."+ns, initPod.ObjectMeta.Labels["applicationId"], podNameNoTolerations, 60)
Ω(err).NotTo(HaveOccurred())
- Ω(len(events.Items)).NotTo(BeZero(), "Events cant be empty")
- for _, event := range events.Items {
- framework.Logf("Event source is : %s", event.Source.Component)
- if event.Source.Component == configmanager.SchedulerName && event.Type == WARNING && !strings.Contains(event.Message, "node affinity") {
- framework.Logf("Yunikorn Event Reason: %s", event.Reason)
- Ω(event.Reason).Should(Equal("FailedScheduling"), "Event reason mismatch")
- framework.Logf("Yunikorn Event Message: %s", event.Message)
- Ω(event.Message).Should(MatchRegexp("predicate is not satisfied.*taint.*"), "Event message mismatch")
- }
- }
+ log, err := restClient.GetAllocationLog("default", "root."+ns, initPod.ObjectMeta.Labels["applicationId"], podNameNoTolerations)
+ Ω(err).NotTo(HaveOccurred())
+ Ω(len(log)).NotTo(BeZero(), "Log can't be empty")
+
+ Ω(log).Should(ContainElement(HaveKeyWithValue("message", MatchRegexp(".*taint.*"))),
Review Comment:
Per previous comment, we specifically keep these generic to make supporting a wider range of K8s releases earlier. What we match here is the same as the previous e2e tests, which have been working fine.
##########
test/e2e/predicates/predicates_test.go:
##########
@@ -1064,31 +1044,25 @@ var _ = Describe("Predicates", func() {
},
}
- var success bool
initPod, podErr = k8s.InitTestPod(conf)
Ω(podErr).NotTo(HaveOccurred())
- success, err = kClient.WaitForSchedulerAfterAction(kClient.CreateTestPodAction(initPod, anotherNS), anotherNS, labelPodName2, false)
+ _, err = kClient.CreatePod(initPod, anotherNS)
Ω(err).NotTo(HaveOccurred())
- Ω(success).Should(BeTrue())
By(fmt.Sprintf("Verify pod:%s is in pending state", labelPodName2))
err = kClient.WaitForPodPending(anotherNS, labelPodName2, time.Duration(60)*time.Second)
Ω(err).NotTo(HaveOccurred())
- By("Verify the Yunikorn events: failed scheduling")
- var events *v1.EventList
- events, err = kClient.GetEvents(anotherNS)
+ By("Verify the YuniKorn request failed scheduling")
+
+ err = restClient.WaitForAllocationLog("default", "root."+anotherNS, initPod.ObjectMeta.Labels["applicationId"], labelPodName2, 60)
Ω(err).NotTo(HaveOccurred())
- Ω(len(events.Items)).NotTo(BeZero(), "Events cant be empty")
- for _, event := range events.Items {
- framework.Logf("Event source is : %s", event.Source.Component)
- if event.Source.Component == configmanager.SchedulerName && event.Type == WARNING && !strings.Contains(event.Message, "node affinity") {
- framework.Logf("Yunikorn Event Reason: %s", event.Reason)
- Ω(event.Reason).Should(Equal("FailedScheduling"), "Event reason mismatch")
- framework.Logf("Yunikorn Event Message: %s", event.Message)
- Ω(event.Message).Should(MatchRegexp("predicate is not satisfied.*free ports.*"), "Event message mismatch")
- }
- }
+ log, err := restClient.GetAllocationLog("default", "root."+anotherNS, initPod.ObjectMeta.Labels["applicationId"], labelPodName2)
+ Ω(err).NotTo(HaveOccurred())
+ Ω(len(log)).NotTo(BeZero(), "Log can't be empty")
+
+ Ω(log).Should(ContainElement(HaveKeyWithValue("message", MatchRegexp(".*free ports.*"))),
Review Comment:
Same response as above.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@yunikorn.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org