Skip to content
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

E2E tests: dump failure information #13

Merged
merged 7 commits into from
Jul 3, 2023
Merged

Conversation

fedepaol
Copy link
Member

Dump relevant information when a test fails.

@fedepaol fedepaol force-pushed the dumpfailure branch 4 times, most recently from 2fcd324 to 3a8641d Compare June 30, 2023 15:11

func BGPInfo(testName string, FRRContainers []*frrcontainer.FRR, cs clientset.Interface, f *framework.Framework) {
if ReportPath == "" {
log.Fatalf("ReportPath is not set")
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't we want to use framework.Logf here like we do below?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fatalf comes with an embedded panic. I will revert to use the framework logging and will panic in the line below


const labelSelector = "app=frr-k8s"

// SpeakerPods returns the set of pods running the speakers.
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: need to update theis comment

Comment on lines 27 to 32
speakerPods := make([]*corev1.Pod, 0)
for _, item := range pods.Items {
i := item
speakerPods = append(speakerPods, &i)
}
return speakerPods, nil
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

and the speakerPods var name

}

func K8sInfo(testName string, reporter *k8sreporter.KubernetesReporter) {
reporter.Dump(10*time.Minute, testName)
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can you please pass the 10 minutes as a const for clarity?
we named this as LogsExtractDuration on the cnf-tests.

We dump both the content of FRR and the k8s informations.

Signed-off-by: Federico Paolinelli <[email protected]>
We want to be able to triage why a test failed, so we dump whenever a
failure happens.

Signed-off-by: Federico Paolinelli <[email protected]>
Signed-off-by: Federico Paolinelli <[email protected]>
Signed-off-by: Federico Paolinelli <[email protected]>
@liornoy
Copy link

liornoy commented Jul 3, 2023

lgtm. I validated the reports locally on my end.

@fedepaol fedepaol added this pull request to the merge queue Jul 3, 2023
Merged via the queue into metallb:main with commit 17e6f35 Jul 3, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants