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 2021/08/02 21:26:44 UTC

[GitHub] [incubator-yunikorn-k8shim] yangwwei commented on a change in pull request #292: [YUNIKORN-744]Call healthcheck REST API in e2e tests

yangwwei commented on a change in pull request #292:
URL: https://github.com/apache/incubator-yunikorn-k8shim/pull/292#discussion_r681280615



##########
File path: test/e2e/app/app_test.go
##########
@@ -113,6 +113,16 @@ var _ = ginkgo.Describe("App", func() {
 			gomega.Ω(appCRD.Spec.TaskGroups[0].Tolerations[0].Effect).To(gomega.Equal(v1.TaintEffectNoSchedule))
 		})
 
+		ginkgo.It("Verify the Yunikorn Scheduler healthy", func() {
+			ginkgo.By("Call the HealthCheck API")
+			healthCheck, err := yunikorn.GetHealthCheck()
+			gomega.Ω(err).NotTo(gomega.HaveOccurred())
+			gomega.Ω(healthCheck.Healthy).To(gomega.BeTrue())
+			for _, check := range healthCheck.HealthChecks {
+				gomega.Ω(check.Succeeded).To(gomega.BeTrue())
+			}
+		})

Review comment:
       I am not very familiar with ginkgo framework, but since this is the same function call for every test, can we move this to a utility function and reuse the code everywhere? If that is possible.

##########
File path: test/e2e/app/app_test.go
##########
@@ -113,6 +113,16 @@ var _ = ginkgo.Describe("App", func() {
 			gomega.Ω(appCRD.Spec.TaskGroups[0].Tolerations[0].Effect).To(gomega.Equal(v1.TaintEffectNoSchedule))
 		})
 
+		ginkgo.It("Verify the Yunikorn Scheduler healthy", func() {
+			ginkgo.By("Call the HealthCheck API")
+			healthCheck, err := yunikorn.GetHealthCheck()
+			gomega.Ω(err).NotTo(gomega.HaveOccurred())
+			gomega.Ω(healthCheck.Healthy).To(gomega.BeTrue())
+			for _, check := range healthCheck.HealthChecks {
+				gomega.Ω(check.Succeeded).To(gomega.BeTrue())
+			}

Review comment:
       if line 120 returns true, I do not think we need to double-check every check.
   If any check fails, the overall result should be false.
   however, one thing not very clear to me is, when this fails, can we print the detailed messages in the log to help us debug?




-- 
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