e2e: use delta-only logging in all polling loops, remove event dumps#6027
e2e: use delta-only logging in all polling loops, remove event dumps#6027stevekuznetsov wants to merge 1 commit into
Conversation
There was a problem hiding this comment.
Pull request overview
This PR reduces E2E/verifier polling noise by applying delta-only logging (log only on observed state changes) across remaining polling/retry loops, and by removing verbose Kubernetes event dump blocks on timeouts in the Cilium verifiers.
Changes:
- Add delta-only logging state tracking to ClusterOperator availability polling and various E2E polling/retry loops.
- Remove Kubernetes event-dump diagnostics from Cilium verifier timeout paths (retain terminated container diagnostics).
- Normalize pod-status logging in Cilium polling by comparing against previously-observed pod sets.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| test/util/verifiers/clusteroperator.go | Adds delta-only logging to ClusterOperator availability polling. |
| test/util/verifiers/cilium.go | Adds delta-only logging for Cilium pod readiness polling and removes event dump blocks on timeouts. |
| test/e2e/external_auth_create.go | Adds delta-only logging for MSGraph token acquisition retries. |
| test/e2e/cluster_delayed_role_assignments.go | Adds delta-only logging for cluster GET polling during delayed role assignment scenarios. |
| test/e2e/cluster_authorized_cidrs_connectivity.go | Adds delta-only logging for MSGraph token acquisition retries. |
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: bennerv, stevekuznetsov The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
stevekuznetsov
force-pushed
the
skuznets/better-test-output
branch
from
f2596d7 to
3ebc13a
Compare
|
/label lgtm |
|
New changes are detected. LGTM label has been removed. |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
Comments suppressed due to low confidence (1)
test/util/verifiers/cilium.go:92
- Delta-only logging: lastErrMsg is never cleared after a successful pod list, so the same list/no-pods error message can be suppressed if it reoccurs after an intervening different state (e.g. notRunningPods). Clearing lastErrMsg once we can observe pods ensures state transitions are logged as intended (test/AGENTS.md delta-only logging rule).
}
notRunningPods := []string{}
Polling loops in e2e tests and verifiers were logging the same message on every iteration (every 20-30s), producing noisy output that obscured real state transitions. Apply the delta-only logging pattern already established in poll.go and cilium.go connectivity checks to all remaining pollers: - cilium.go: track lastErrMsg and lastNotRunningPods in the first poll loop, only log when the observed state changes - clusteroperator.go: track lastErrMsg across all three log sites in the ClusterOperator availability poll - cluster_delayed_role_assignments.go: track last error and provisioning state in both Consistently and Eventually blocks - cluster_authorized_cidrs_connectivity.go, external_auth_create.go: track lastTokenErr in GetToken retry loops Additionally, remove event dump blocks from cilium.go on poll timeout. Events are available in Kusto and dumping them inline adds clutter without actionable value. The terminated container diagnostic dump is retained since that information is not available in Kusto. Signed-off-by: Steve Kuznetsov <stekuznetsov@microsoft.com>
stevekuznetsov
force-pushed
the
skuznets/better-test-output
branch
from
3ebc13a to
74c3c32
Compare
|
New changes are detected. LGTM label has been removed. |
|
/label lgtm |
| if !slices.Equal(notRunningPods, lastNotRunningPods) { | ||
| logger.Info("waiting for cilium pods to be running", "notRunning", notRunningPods) | ||
| lastNotRunningPods = notRunningPods | ||
| } |
|
/retest |
|
/retest |
|
/retest |
|
/test all |
1 similar comment
|
/test all |
|
@stevekuznetsov: The following tests failed, say
Full PR test history. Your PR dashboard. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
|
/test all |
Polling loops in e2e tests and verifiers were logging the same message on every iteration (every 20-30s), producing noisy output that obscured real state transitions. Apply the delta-only logging pattern already established in poll.go and cilium.go connectivity checks to all remaining pollers:
Additionally, remove event dump blocks from cilium.go on poll timeout. Events are available in Kusto and dumping them inline adds clutter without actionable value. The terminated container diagnostic dump is retained since that information is not available in Kusto.