Skip to content

fix: reduce http proxy e2e flakiness - #9032

Open
fcher wants to merge 6 commits into
mainfrom
facheruk/https-proxy
Open

fix: reduce http proxy e2e flakiness#9032
fcher wants to merge 6 commits into
mainfrom
facheruk/https-proxy

Conversation

@fcher

@fcher fcher commented Jul 23, 2026

Copy link
Copy Markdown
Contributor

What this PR does / why we need it:
Fixes AgentBaker Gate PR flakiness caused by Test_Ubuntu2204_HTTPSProxy_PrivateDNS E2E test

Which issue(s) this PR fixes:

ADO link: https://msazure.visualstudio.com/CloudNativeCompute/_workitems/edit/38383391

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This PR aims to reduce E2E flakiness (notably Test_Ubuntu2204_HTTPSProxy_PrivateDNS) by ensuring the in-cluster proxy DaemonSet is only considered “ready” once it is actually accepting TCP connections, rather than immediately upon container start.

Changes:

  • Add a TCP readiness probe to the proxy DaemonSet container so PodReady reflects the proxy port being reachable.
  • Add a TCP liveness probe to restart the proxy container if it stops accepting connections.

Comment thread e2e/kube.go Outdated
Comment on lines +599 to +604
// Restart the container if probe fails
LivenessProbe: &corev1.Probe{
ProbeHandler: corev1.ProbeHandler{
TCPSocket: &corev1.TCPSocketAction{Port: intstr.FromInt(proxyPort)},
},
},
Copilot AI review requested due to automatic review settings July 24, 2026 10:05
@github-actions

github-actions Bot commented Jul 24, 2026

Copy link
Copy Markdown
Contributor

Windows Unit Test Results

  3 files   11 suites   49s ⏱️
381 tests 381 ✅ 0 💤 0 ❌
384 runs  384 ✅ 0 💤 0 ❌

Results for commit 69869d7.

♻️ This comment has been updated with latest results.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

Comments suppressed due to low confidence (1)

e2e/kube.go:604

  • The readiness/liveness TCP probes rely on default timings (InitialDelaySeconds=0, TimeoutSeconds=1, FailureThreshold=3). With a cold start/image pull or transient scheduling delays, the liveness probe can trigger restarts before the proxy has a chance to bind the port, which may increase (not reduce) e2e flakiness. Set explicit probe timings (especially an initial delay for liveness) tuned for this test DaemonSet.
						// Gate readiness on the proxy actually accepting TCP connections on :8888.
						ReadinessProbe: &corev1.Probe{
							ProbeHandler: corev1.ProbeHandler{
								TCPSocket: &corev1.TCPSocketAction{Port: intstr.FromInt(proxyPort)},
							},
						},
						// Restart the container if it stops serving on :8888.
						LivenessProbe: &corev1.Probe{
							ProbeHandler: corev1.ProbeHandler{
								TCPSocket: &corev1.TCPSocketAction{Port: intstr.FromInt(proxyPort)},
							},
						},

Comment thread e2e/kube.go
Comment on lines +648 to +659
if len(pods.Items) == 0 {
lastPodStatuses = []string{"no proxy pods found"}
}
// Self-heal once if no proxy pod becomes ready within the grace period
if !selfHealed && time.Since(start) >= selfHealDelay {
selfHealed = true
if rerr := k.recreateProxyPods(ctx); rerr != nil {
toolkit.Logf(ctx, "failed to recreate proxy pods after %s: %v", selfHealDelay, rerr)
} else {
toolkit.Logf(ctx, "recreated proxy pods after %s without a ready proxy", selfHealDelay)
}
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@fcher maybe worth addressing

Also modifies self healing conditionm to handle when at least
one proxy pod is available
Copilot AI review requested due to automatic review settings July 24, 2026 19:21

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

Comment thread e2e/kube.go Outdated
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings July 24, 2026 19:25

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

Comments suppressed due to low confidence (2)

e2e/kube.go:604

  • ReadinessProbe also uses Kubernetes defaults, which can make readiness changes slow to reflect and provide less deterministic behavior in tests. Setting explicit probe timing (period/timeout/failureThreshold) will make readiness gating more predictable for the E2E harness.
						ReadinessProbe: &corev1.Probe{
							ProbeHandler: corev1.ProbeHandler{
								TCPSocket: &corev1.TCPSocketAction{Port: intstr.FromInt(proxyPort)},
							},
						},

e2e/kube.go:610

  • LivenessProbe defaults restart the container after ~30s of TCP check failures. For an E2E helper daemonset this can introduce additional churn during transient node/network conditions; consider explicitly increasing the failure budget so the proxy isn’t restarted too aggressively.
						LivenessProbe: &corev1.Probe{
							ProbeHandler: corev1.ProbeHandler{
								TCPSocket: &corev1.TCPSocketAction{Port: intstr.FromInt(proxyPort)},
							},
						},

Comment thread e2e/kube.go
Comment on lines +594 to +598
StartupProbe: &corev1.Probe{
ProbeHandler: corev1.ProbeHandler{
TCPSocket: &corev1.TCPSocketAction{Port: intstr.FromInt(proxyPort)},
},
},
@fcher fcher changed the title fix: add readiness and liveness probe to proxy fix: reduce http proxy e2e flakiness Jul 24, 2026
Comment thread e2e/kube.go
} else {
toolkit.Logf(ctx, "recreated proxy pods after %s without a ready proxy", selfHealDelay)
}
}

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

once the pods are deleted , what are we doing after ? should we be waiting for them to come online ?

If they still fail to come online, we should describe the daemonset to get the failling events, or the pod events. to understand why pods are not running

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Copilot-Session: 2b080a1f-5eb3-42f9-b320-40567ba8d81e
Copilot AI review requested due to automatic review settings July 29, 2026 21:26

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.

Copilot AI review requested due to automatic review settings July 29, 2026 22:51

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (1)

e2e/kube.go:595

  • The startup probe timing comment doesn't match the configured values: InitialDelaySeconds=5 and FailureThreshold=12 with PeriodSeconds=5 yields 65s (5s + 12×5s), not 60s. This can confuse future tuning/debugging of flakiness.
						// Check whether proxy has started before starting the readiness and liveness probes.
						// Allow up to 60s total (5 + 12×5) for a slow-starting proxy before giving up.
						StartupProbe: &corev1.Probe{

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 49af9358-3981-4111-9e8b-ddfb977a1df1
Copilot AI review requested due to automatic review settings July 29, 2026 23:01
@fcher
fcher force-pushed the facheruk/https-proxy branch from ff07340 to 69869d7 Compare July 29, 2026 23:01

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (3)

e2e/kube.go:672

  • The self-heal grace period is measured from start := time.Now() (function entry), not from when proxy pods first appear. If scheduling/image pulls delay pod creation close to selfHealDelay, this can delete newly created pods almost immediately, potentially increasing flakiness. Consider starting the timer when the first proxy pod is observed (or when the DaemonSet is created) before allowing the recreate path to trigger.
		// Self-heal once: if pods exist but none became ready within the grace
		// period, delete them so the DaemonSet reschedules fresh ones
		if !selfHealed && len(pods.Items) > 0 && time.Since(start) >= selfHealDelay {
			selfHealed = true
			if rerr := k.recreateProxyPods(ctx); rerr != nil {

e2e/kube.go:595

  • The timing math in this comment is off by one probe period: with InitialDelaySeconds=5, PeriodSeconds=5, FailureThreshold=12, the worst-case time to startup-probe failure is ~60s (5 + 11×5), not 65s (5 + 12×5). Keeping this accurate helps when tuning flake mitigation.
						// Check whether proxy has started before starting the readiness and liveness probes.
						// Allow up to 60s total (5 + 12×5) for a slow-starting proxy before giving up.
						StartupProbe: &corev1.Probe{

e2e/kube.go:614

  • This comment's timing math is slightly off: with InitialDelaySeconds=30, PeriodSeconds=10, FailureThreshold=3, liveness restart happens after ~50s (30 + 2×10) of sustained failure, not 60s (30 + 3×10).
						// Restart the container if it stops serving on :8888.
						// Only restart after sustained failure (30s delay + 3×10s) to avoid restart loops on transient blips.
						LivenessProbe: &corev1.Probe{

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.

4 participants