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/03 18:17:24 UTC
[GitHub] [yunikorn-k8shim] craigcondit opened a new pull request, #444: [YUNIKORN-1274] Remove FailedScheduling pod events from predicate logic
craigcondit opened a new pull request, #444:
URL: https://github.com/apache/yunikorn-k8shim/pull/444
### What is this PR for?
Removes a confusing warning emitted as Pod event when predicates don't match a pod with a node. As this is a normal, expected occurrence, these warnings are confusing.
### What type of PR is it?
* [ ] - Bug Fix
* [x] - Improvement
* [ ] - Feature
* [ ] - Documentation
* [ ] - Hot Fix
* [ ] - Refactoring
### Todos
* [ ] - Task
### What is the Jira issue?
https://issues.apache.org/jira/browse/YUNIKORN-1274
### How should this be tested?
No functional changes, but e2e tests have been updated to use the REST API to retrieve these events.
### Screenshots (if appropriate)
### Questions:
* [ ] - The licenses files need update.
* [ ] - There is breaking changes for older versions.
* [ ] - It needs documentation.
--
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
[GitHub] [yunikorn-k8shim] wilfred-s commented on a diff in pull request #444: [YUNIKORN-1274] Remove FailedScheduling pod events from predicate logic
Posted by GitBox <gi...@apache.org>.
wilfred-s commented on code in PR #444:
URL: https://github.com/apache/yunikorn-k8shim/pull/444#discussion_r937430815
##########
test/e2e/basic_scheduling/basic_scheduling_test.go:
##########
@@ -96,7 +96,6 @@ var _ = ginkgo.Describe("", func() {
gomega.Ω(allocations["partition"]).NotTo(gomega.BeNil())
gomega.Ω(allocations["uuid"]).NotTo(gomega.BeNil())
gomega.Ω(allocations["applicationId"]).To(gomega.Equal(sleepRespPod.ObjectMeta.Labels["applicationId"]))
- gomega.Ω(allocations["queueName"]).To(gomega.ContainSubstring(sleepRespPod.ObjectMeta.Namespace))
Review Comment:
This is a follow up on YUNIKORN-1269 in which we removed the queueName from object and REST
--
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
[GitHub] [yunikorn-k8shim] craigcondit closed pull request #444: [YUNIKORN-1274] Remove FailedScheduling pod events from predicate logic
Posted by GitBox <gi...@apache.org>.
craigcondit closed pull request #444: [YUNIKORN-1274] Remove FailedScheduling pod events from predicate logic
URL: https://github.com/apache/yunikorn-k8shim/pull/444
--
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
[GitHub] [yunikorn-k8shim] wilfred-s commented on a diff in pull request #444: [YUNIKORN-1274] Remove FailedScheduling pod events from predicate logic
Posted by GitBox <gi...@apache.org>.
wilfred-s commented on code in PR #444:
URL: https://github.com/apache/yunikorn-k8shim/pull/444#discussion_r937430815
##########
test/e2e/basic_scheduling/basic_scheduling_test.go:
##########
@@ -96,7 +96,6 @@ var _ = ginkgo.Describe("", func() {
gomega.Ω(allocations["partition"]).NotTo(gomega.BeNil())
gomega.Ω(allocations["uuid"]).NotTo(gomega.BeNil())
gomega.Ω(allocations["applicationId"]).To(gomega.Equal(sleepRespPod.ObjectMeta.Labels["applicationId"]))
- gomega.Ω(allocations["queueName"]).To(gomega.ContainSubstring(sleepRespPod.ObjectMeta.Namespace))
Review Comment:
This is a follow up on YUNIKORN-1269 in which we removed the queueName from object and REST.
With the k8shim dependency update we now need this cleanup here.
--
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
[GitHub] [yunikorn-k8shim] craigcondit commented on a diff in pull request #444: [YUNIKORN-1274] Remove FailedScheduling pod events from predicate logic
Posted by GitBox <gi...@apache.org>.
craigcondit commented on code in PR #444:
URL: https://github.com/apache/yunikorn-k8shim/pull/444#discussion_r937987962
##########
pkg/plugin/predicates/predicate_manager.go:
##########
@@ -75,12 +74,7 @@ func (p *predicateManagerImpl) predicatesReserve(pod *v1.Pod, node *framework.No
func (p *predicateManagerImpl) predicatesAllocate(pod *v1.Pod, node *framework.NodeInfo) (plugin string, error error) {
ctx := context.Background()
state := framework.NewCycleState()
- plugin, err := p.podFitsNode(ctx, state, *p.allocationPreFilters, *p.allocationFilters, pod, node)
- if err != nil {
- events.GetRecorder().Eventf(pod.DeepCopy(), nil, v1.EventTypeWarning,
- "FailedScheduling", "FailedScheduling", "predicate is not satisfied, error: %s", err.Error())
- }
- return plugin, err
+ return p.podFitsNode(ctx, state, *p.allocationPreFilters, *p.allocationFilters, pod, node)
Review Comment:
We don't bother to log this for reservations, I don't really see a need to log here either. This can quickly get overwhelming as this can happen multiple times per second and is of little value.
--
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
[GitHub] [yunikorn-k8shim] craigcondit commented on pull request #444: [YUNIKORN-1274] Remove FailedScheduling pod events from predicate logic
Posted by GitBox <gi...@apache.org>.
craigcondit commented on PR #444:
URL: https://github.com/apache/yunikorn-k8shim/pull/444#issuecomment-1205461764
Hi @manirajv06, can you re-review? I've responded to your comments and will fix the unnecessary comment modification in predicates_test.go during final commit. Thanks!
--
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
[GitHub] [yunikorn-k8shim] codecov[bot] commented on pull request #444: [YUNIKORN-1274] Remove FailedScheduling pod events from predicate logic
Posted by GitBox <gi...@apache.org>.
codecov[bot] commented on PR #444:
URL: https://github.com/apache/yunikorn-k8shim/pull/444#issuecomment-1204326468
# [Codecov](https://codecov.io/gh/apache/yunikorn-k8shim/pull/444?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#444](https://codecov.io/gh/apache/yunikorn-k8shim/pull/444?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (3399dca) into [master](https://codecov.io/gh/apache/yunikorn-k8shim/commit/f676c05d3fbfb6e25e7fff2dc4766434a76456e0?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (f676c05) will **decrease** coverage by `0.02%`.
> The diff coverage is `100.00%`.
```diff
@@ Coverage Diff @@
## master #444 +/- ##
==========================================
- Coverage 67.34% 67.32% -0.03%
==========================================
Files 41 41
Lines 6744 6739 -5
==========================================
- Hits 4542 4537 -5
Misses 2032 2032
Partials 170 170
```
| [Impacted Files](https://codecov.io/gh/apache/yunikorn-k8shim/pull/444?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [pkg/plugin/predicates/predicate\_manager.go](https://codecov.io/gh/apache/yunikorn-k8shim/pull/444/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGtnL3BsdWdpbi9wcmVkaWNhdGVzL3ByZWRpY2F0ZV9tYW5hZ2VyLmdv) | `50.19% <100.00%> (-0.96%)` | :arrow_down: |
:mega: Codecov can now indicate which changes are the most critical in Pull Requests. [Learn more](https://about.codecov.io/product/feature/runtime-insights/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
--
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
[GitHub] [yunikorn-k8shim] craigcondit commented on a diff in pull request #444: [YUNIKORN-1274] Remove FailedScheduling pod events from predicate logic
Posted by GitBox <gi...@apache.org>.
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
[GitHub] [yunikorn-k8shim] craigcondit commented on a diff in pull request #444: [YUNIKORN-1274] Remove FailedScheduling pod events from predicate logic
Posted by GitBox <gi...@apache.org>.
craigcondit commented on code in PR #444:
URL: https://github.com/apache/yunikorn-k8shim/pull/444#discussion_r937988272
##########
test/e2e/predicates/predicates_test.go:
##########
@@ -78,20 +77,19 @@ func runTestPod(k *k8s.KubeCtl, conf k8s.SleepPodConfig) *v1.Pod {
var _ = Describe("Predicates", func() {
var kClient k8s.KubeCtl
- //var restClient yunikorn.RClient
+ var restClient yunikorn.RClient
var err error
var ns, anotherNS string
var nodeList *v1.NodeList
const LABELVALUE = "testing-label-value"
- const WARNING = "Warning"
- //var sleepRespPod *v1.Pod
+ // var sleepRespPod *v1.Pod
Review Comment:
Will fix during commit.
--
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
[GitHub] [yunikorn-k8shim] manirajv06 commented on a diff in pull request #444: [YUNIKORN-1274] Remove FailedScheduling pod events from predicate logic
Posted by GitBox <gi...@apache.org>.
manirajv06 commented on code in PR #444:
URL: https://github.com/apache/yunikorn-k8shim/pull/444#discussion_r937355002
##########
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:
Like commented above
##########
test/e2e/predicates/predicates_test.go:
##########
@@ -78,20 +77,19 @@ func runTestPod(k *k8s.KubeCtl, conf k8s.SleepPodConfig) *v1.Pod {
var _ = Describe("Predicates", func() {
var kClient k8s.KubeCtl
- //var restClient yunikorn.RClient
+ var restClient yunikorn.RClient
var err error
var ns, anotherNS string
var nodeList *v1.NodeList
const LABELVALUE = "testing-label-value"
- const WARNING = "Warning"
- //var sleepRespPod *v1.Pod
+ // var sleepRespPod *v1.Pod
Review Comment:
can be removed
##########
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:
Can we check by matching with earlier message? I am assuming allocation log contains same message.
##########
pkg/plugin/predicates/predicate_manager.go:
##########
@@ -75,12 +74,7 @@ func (p *predicateManagerImpl) predicatesReserve(pod *v1.Pod, node *framework.No
func (p *predicateManagerImpl) predicatesAllocate(pod *v1.Pod, node *framework.NodeInfo) (plugin string, error error) {
ctx := context.Background()
state := framework.NewCycleState()
- plugin, err := p.podFitsNode(ctx, state, *p.allocationPreFilters, *p.allocationFilters, pod, node)
- if err != nil {
- events.GetRecorder().Eventf(pod.DeepCopy(), nil, v1.EventTypeWarning,
- "FailedScheduling", "FailedScheduling", "predicate is not satisfied, error: %s", err.Error())
- }
- return plugin, err
+ return p.podFitsNode(ctx, state, *p.allocationPreFilters, *p.allocationFilters, pod, node)
Review Comment:
Should we log this as debug statement?
##########
test/e2e/basic_scheduling/basic_scheduling_test.go:
##########
@@ -96,7 +96,6 @@ var _ = ginkgo.Describe("", func() {
gomega.Ω(allocations["partition"]).NotTo(gomega.BeNil())
gomega.Ω(allocations["uuid"]).NotTo(gomega.BeNil())
gomega.Ω(allocations["applicationId"]).To(gomega.Equal(sleepRespPod.ObjectMeta.Labels["applicationId"]))
- gomega.Ω(allocations["queueName"]).To(gomega.ContainSubstring(sleepRespPod.ObjectMeta.Namespace))
Review Comment:
Is this related to the jira? or some other general cleanup?
##########
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:
Please see my earlier comment. Just depending only on ".*taint.*" may mislead us sometimes.
--
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