-
Notifications
You must be signed in to change notification settings - Fork 520
test: print pod logs when errors occur #274
Conversation
Codecov Report
@@ Coverage Diff @@
## master #274 +/- ##
==========================================
- Coverage 53.21% 53.16% -0.05%
==========================================
Files 95 95
Lines 14235 14244 +9
==========================================
- Hits 7575 7573 -2
- Misses 5995 6006 +11
Partials 665 665 |
@@ -612,6 +612,12 @@ var _ = Describe("Azure Container Cluster using the Kubernetes Orchestrator", fu | |||
} | |||
if i > 28 { | |||
log.Printf("Error while running kubectl top nodes:%s\n", err) | |||
pods, err := pod.GetAllByPrefix("metrics-server", "kube-system") | |||
Expect(err).NotTo(HaveOccurred()) |
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.
I would argue we shouldn't return an error for logging debugging data in this "if not successful" block since that will hide the important error which is Expect(success).To(BeTrue())
on line 624. The logs are bonus, not part of the actual test.
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.
Good feedback, addressed.
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
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: CecileRobertMichon, jackfrancis The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
* test: print pod logs in error conditions * test: need to get metrics-server pod by prefix * test: print pod logs for all WaitOnReady errs * test: return real error if print pod logs fails * test: don’t error on fetching metrics pod
Reason for Change:
To help debug flakes (e.g.,
kubectl top nodes
), we should be printing out the pod logs when errors occur)Issue Fixed:
Requirements:
Notes: