Skip to content

Add timeout to metrics query and cleanup job failure logs in e2e test #3378

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.

Sign up for GitHub

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

Merged
merged 2 commits into from
Oct 1, 2025
+6 −24
Merged

Add timeout to metrics query and cleanup job failure logs in e2e test #3378

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
aa9e9a4
[test] Add timeout to metrics query
sebsoto Sep 30, 2025
92ec5dc
[test] Cleanup job failure logs
sebsoto Sep 30, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion test/e2e/metrics_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import (
"strconv"
"strings"
"testing"
"time"

"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
Expand Down Expand Up @@ -267,7 +268,7 @@ func makePrometheusQuery(address, query, token string) ([]Result, error) {
// InsecureSkipVerify is required to avoid errors due to bad certificate
Copy link
Contributor

@jrvaldes jrvaldes Sep 30, 2025

Choose a reason for hiding this comment

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

consider usign context.WithTimeout to control the timeout per request in ef239e2#diff-41354c8d32cc5449ea0e5de5e5f28928103f9d2642b034f4b6e8474d487d656fL261

Copy link
Contributor

@jrvaldes jrvaldes Sep 30, 2025

Choose a reason for hiding this comment

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

for example:

    ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second)
    defer cancel()

    req, err := http.NewRequestWithContext(ctx, "GET", url, nil)

Copy link
Contributor Author

@sebsoto sebsoto Sep 30, 2025

Choose a reason for hiding this comment

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

Don't know if that really helps here, theres only one request at the moment, and I can't think of a reason of we would need to tweak individual timeouts for different requests in the case we need to add another.

Copy link
Contributor

@jrvaldes jrvaldes Sep 30, 2025

Choose a reason for hiding this comment

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

Makes sense, disregard.

TLSClientConfig: &tls.Config{InsecureSkipVerify: true},
}
client := &http.Client{Transport: tr}
client := &http.Client{Transport: tr, Timeout: 15 * time.Second}
resp, err := client.Do(req)
if err != nil {
return nil, fmt.Errorf("error making GET request: %w", err)
Expand Down
27 changes: 4 additions & 23 deletions test/e2e/network_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -979,28 +979,11 @@ func (tc *testContext) waitUntilDeploymentScaled(name string) error {
return false, nil
})
if err != nil {
events, _ := tc.getPodEvents(name)
return fmt.Errorf("error waiting for deployment %v to scale: %v: %w", deployment, events, err)
return fmt.Errorf("error waiting for deployment %s to scale: %w", deployment.Name, err)
}
return nil
}

// getPodEvents gets all events for any pod with the input in its name. Used for debugging purposes
func (tc *testContext) getPodEvents(name string) ([]v1.Event, error) {
eventList, err := tc.client.K8s.CoreV1().Events(tc.workloadNamespace).List(context.TODO(), metav1.ListOptions{
FieldSelector: "involvedObject.kind=Pod"})
if err != nil {
return []v1.Event{}, err
}
var podEvents []v1.Event
for _, event := range eventList.Items {
if strings.Contains(event.InvolvedObject.Name, name) {
podEvents = append(podEvents, event)
}
}
return podEvents, nil
}

// createLinuxCurlerJob creates a linux job to curl a specific endpoint. curl must be present in the container image.
func (tc *testContext) createLinuxCurlerJob(jobSuffix, endpoint string, continuous bool) (*batchv1.Job, error) {
// Retries a failed curl attempt once to avoid flakes
Expand Down Expand Up @@ -1125,7 +1108,7 @@ func (tc *testContext) waitUntilJobSucceeds(name string) (string, error) {
for i := 0; i < 60; i++ {
job, err = tc.client.K8s.BatchV1().Jobs(tc.workloadNamespace).Get(context.TODO(), name, metav1.GetOptions{})
if err != nil {
return "", err
return "", fmt.Errorf("error getting job with name %s: %w", name, err)
}
if !slices.ContainsFunc(job.Status.Conditions, func(condition batchv1.JobCondition) bool {
return condition.Type == batchv1.JobComplete && condition.Status == v1.ConditionTrue
Expand All @@ -1143,17 +1126,15 @@ func (tc *testContext) waitUntilJobSucceeds(name string) (string, error) {
return condition.Type == batchv1.JobSuccessCriteriaMet && condition.Status == v1.ConditionTrue
}) {
// Job did not succeed, return error
events, _ := tc.getPodEvents(name)
return logs, fmt.Errorf("job %v failed: %v", job, events)
return logs, fmt.Errorf("job %s failed", name)
}
return logs, nil
}
_, err = tc.gatherPodLogs(labelSelector, true)
if err != nil {
log.Printf("Unable to get logs associated with pod %s: %v", labelSelector, err)
}
events, _ := tc.getPodEvents(name)
return "", fmt.Errorf("job %v timed out: %v", job, events)
return "", fmt.Errorf("job %s timed out", name)
}

// gatherPodLogs writes the logs associated with the label selector of a given pod job or deployment to the Artifacts
Expand Down