fix(test/rekt): retry address validation until predicate passes by pujitha24 · Pull Request #9407 · knative/eventing

pujitha24 · 2026-09-18T15:35:22Z

Motivation:
test/rekt.TestSinkBindingV1Deployment_BrokerAsSinkTLS's "Broker has HTTPS
address" requirement flakes occasionally in CI (about 1 in 5 runs per
knative-prow-updater-robot's tracking). The same "Broker has HTTPS
address" requirement is also used by the PingSource, ApiServerSource, and
Broker BrokerAsSinkTLS rekt features.

Approach:
addressable.ValidateAddress polled Address() until it returned any
non-nil address, then called the validate predicate (e.g.
AssertHTTPSAddress, which checks addr.URL.Scheme == "https") exactly
once with no retry. A Broker's status.address can be reported before it
satisfies a given predicate, e.g. reported as http:// before the
reconciler flips it to https:// once the Broker's TLS state becomes
ready. That one-shot check would fail immediately instead of waiting for
the address to settle, causing the observed flake.

ValidateAddress now polls (same wait.PollUntilContextTimeout idiom
already used in this file and elsewhere in the codebase, e.g.
broker.WaitForCondition) until the address is both present and passes
validate(), retrying on a validation failure instead of erroring
immediately. On timeout it reports the last validation error if one
occurred, which is more actionable than a generic timeout error.
Address() itself, and its ~8 other callers, are unchanged.

Validation:
Added test/rekt/resources/addressable/addressable_test.go, which seeds a
fake dynamic client with an object whose status.address.url starts as
http://, flips it to https:// after 60ms from a background goroutine
(simulating the real timing race), and asserts ValidateAddress retries
until it observes the https address rather than failing on the initial
http one.

go test ./test/rekt/resources/addressable/... -run TestValidateAddress -v: PASS.
Confirmed the new test FAILS against the pre-fix code (verified via
git stash on only addressable.go, rerunning the test: fails with
"address is not HTTPS"), and PASSES with the fix - a failing-then-passing
reproduction of the race, not a live cluster run.
go test -race -count=20 ./test/rekt/resources/addressable/...: PASS, no
data race.
go build ./... and go vet ./test/rekt/...: clean.
go test ./test/rekt/...: all non-e2e-tagged packages pass.
golangci-lint run ./test/rekt/resources/addressable/...: 0 issues.
gofmt -l on changed files: clean.

This change only affects test code (test/rekt), not production code paths.

Report: #9378
Signed-off-by: Pujitha Paladugu 10557236+pujitha24@users.noreply.github.com
Assisted-by: claude-sonnet-5 (via Claude Code)

Fixes #9378

NONE

Motivation: test/rekt.TestSinkBindingV1Deployment_BrokerAsSinkTLS's "Broker has HTTPS address" requirement flakes occasionally in CI (about 1 in 5 runs per knative-prow-updater-robot's tracking). The same "Broker has HTTPS address" requirement is also used by the PingSource, ApiServerSource, and Broker BrokerAsSinkTLS rekt features. Approach: addressable.ValidateAddress polled Address() until it returned any non-nil address, then called the validate predicate (e.g. AssertHTTPSAddress, which checks addr.URL.Scheme == "https") exactly once with no retry. A Broker's status.address can be reported before it satisfies a given predicate, e.g. reported as http:// before the reconciler flips it to https:// once the Broker's TLS state becomes ready. That one-shot check would fail immediately instead of waiting for the address to settle, causing the observed flake. ValidateAddress now polls (same wait.PollUntilContextTimeout idiom already used in this file and elsewhere in the codebase, e.g. broker.WaitForCondition) until the address is both present and passes validate(), retrying on a validation failure instead of erroring immediately. On timeout it reports the last validation error if one occurred, which is more actionable than a generic timeout error. Address() itself, and its ~8 other callers, are unchanged. Validation: Added test/rekt/resources/addressable/addressable_test.go, which seeds a fake dynamic client with an object whose status.address.url starts as http://, flips it to https:// after 60ms from a background goroutine (simulating the real timing race), and asserts ValidateAddress retries until it observes the https address rather than failing on the initial http one. - go test ./test/rekt/resources/addressable/... -run TestValidateAddress -v: PASS. - Confirmed the new test FAILS against the pre-fix code (verified via git stash on only addressable.go, rerunning the test: fails with "address is not HTTPS"), and PASSES with the fix - a failing-then-passing reproduction of the race, not a live cluster run. - go test -race -count=20 ./test/rekt/resources/addressable/...: PASS, no data race. - go build ./... and go vet ./test/rekt/...: clean. - go test ./test/rekt/...: all non-e2e-tagged packages pass. - golangci-lint run ./test/rekt/resources/addressable/...: 0 issues. - gofmt -l on changed files: clean. This change only affects test code (test/rekt), not production code paths. Report: knative#9378 Signed-off-by: Pujitha Paladugu <10557236+pujitha24@users.noreply.github.com> Assisted-by: claude-sonnet-5 (via Claude Code)

knative-prow · 2026-09-18T15:35:33Z

Hi @pujitha24. Thanks for your PR.

I'm waiting for a knative member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work.

Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Details

Instructions 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.

dsimansk · 2026-09-18T16:58:15Z

/ok-to-test

pujitha24 · 2026-09-18T18:32:37Z

/retest

dsimansk · 2026-09-18T20:24:39Z

+		interval, timeout := k8s.PollTimings(ctx, timings)
+		var validateErr error
+		err := wait.PollUntilContextTimeout(ctx, interval, timeout, true, func(ctx context.Context) (bool, error) {


It's going into a good direction. I agree that it should stabilize tests that wait for https URL to be populated.

But this part is almost 1 to 1 copy of the very similar poll loop (L38-L56) a few lines above in Address. We shouldn't just copy it over, but rather refactor to create common polling function to be shared, and testable.

dsimansk · 2026-09-18T20:25:44Z

+		err := wait.PollUntilContextTimeout(ctx, interval, timeout, true, func(ctx context.Context) (bool, error) {
+			addr, err := k8s.Address(ctx, gvr, name)
+			if err != nil {
+				if apierrors.IsNotFound(err) {


We can think of more transient cases, like timeouts or too many requests that are intermittently occur in e2e tests from the cluster, but can be retried. If the motivation is to improve stability.

codecov · 2026-09-18T20:35:29Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 51.26%. Comparing base (07b1e8a) to head (24d1e2d).
⚠️ Report is 5 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #9407      +/-   ##
==========================================
+ Coverage   51.18%   51.26%   +0.08%     
==========================================
  Files         411      411              
  Lines       22176    22179       +3     
==========================================
+ Hits        11351    11371      +20     
+ Misses       9951     9927      -24     
- Partials      874      881       +7

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

pujitha24 · 2026-09-19T01:32:44Z

/retest

pujitha24 · 2026-09-19T07:40:57Z

/retest

dsimansk

Per my last comment, there are code cleanups to be followed to reduce code duplication.

Extract the polling logic shared by Address and ValidateAddress into a single pollAddress helper, and retry timeout/throttling errors from the apiserver in addition to NotFound, per review feedback on knative#9407. Signed-off-by: Pujitha Paladugu <10557236+pujitha24@users.noreply.github.com>

Add tests for the shared pollAddress loop through Address(): it retries NotFound, Timeout, ServerTimeout and TooManyRequests errors until the address is returned, and stops on a non-transient error (Forbidden) instead of waiting for the timeout. Move the fake Broker setup into a helper shared by the new tests and the existing ValidateAddress test. Signed-off-by: Pujitha Paladugu <10557236+pujitha24@users.noreply.github.com> Assisted-by: claude-opus-5-5 (via Claude Code)

ValidateAddress reported the last validation error whenever pollAddress returned one, even if polling had then stopped on a non-transient error (e.g. Forbidden), which hid the real failure. Only prefer the validation error when the poll was interrupted by its timeout, and add tests for both cases. Signed-off-by: Pujitha Paladugu <10557236+pujitha24@users.noreply.github.com> Assisted-by: claude-opus-5-5 (via Claude Code)

pujitha24 · 2026-10-01T06:22:56Z

Both points are handled in 806a1ab. 9203767 and 24d1e2d add tests for the shared loop.

Duplication: Address and ValidateAddress now share a single pollAddress loop. Address passes no validator, and the copied loop is gone.
Transient errors: pollAddress keeps polling on NotFound, Timeout, ServerTimeout and TooManyRequests (see isRetryableError). Any other error still ends the poll immediately.
Tests (9203767):
- TestAddress_RetriesTransientErrors fails the first two gets with each of those four errors and expects the address to come back.
- TestAddress_FailsFastOnNonTransientError expects a Forbidden to be returned well before the timeout.
- The fake Broker setup is now a helper shared with the existing ValidateAddress test.
- If isRetryableError is reduced to NotFound only, the timeout, server-timeout and too-many-requests cases fail.
24d1e2d: fixes a gap I found while adding those tests. If validation failed once and polling then stopped on a non-transient error, ValidateAddress reported the stale validation error and hid the real one. It now prefers the validation error only when the poll timed out. Both cases are covered by tests.

dsimansk · 2026-10-01T13:21:25Z

/test reconciler-tests

dsimansk · 2026-10-09T11:56:51Z

/approve
/lgtm

knative-prow · 2026-10-09T11:57:01Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: dsimansk, pujitha24

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details

Needs approval from an approver in each of these files:

~~test/OWNERS~~ [dsimansk]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

knative-prow Bot added the area/test-and-release Test infrastructure, tests or release label Sep 18, 2026

knative-prow Bot requested review from creydr and evankanderson September 18, 2026 15:35

knative-prow Bot added needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Sep 18, 2026

knative-prow Bot added ok-to-test Indicates a non-member PR verified by an org member that is safe to test. and removed needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Sep 18, 2026

dsimansk reviewed Sep 18, 2026

View reviewed changes

dsimansk requested changes Sep 21, 2026

View reviewed changes

knative-prow Bot assigned dsimansk Sep 21, 2026

pujitha24 added 3 commits September 21, 2026 05:06

knative-prow Bot added the lgtm Indicates that a PR is ready to be merged. label Oct 9, 2026

knative-prow Bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Oct 9, 2026

Navigation Menu

Repository navigation

fix(test/rekt): retry address validation until predicate passes - #9407

fix(test/rekt): retry address validation until predicate passes#9407
pujitha24 wants to merge 4 commits into
knative:mainfrom
pujitha24:auto/issue-9378

pujitha24 commented Sep 18, 2026

Uh oh!

knative-prow Bot commented Sep 18, 2026

Uh oh!

dsimansk commented Sep 18, 2026

Uh oh!

pujitha24 commented Sep 18, 2026

Uh oh!

dsimansk Sep 18, 2026 •

edited

Loading

Uh oh!

dsimansk Sep 18, 2026

Uh oh!

codecov Bot commented Sep 18, 2026 •

edited

Loading

Uh oh!

pujitha24 commented Sep 19, 2026

Uh oh!

pujitha24 commented Sep 19, 2026

Uh oh!

dsimansk left a comment

Uh oh!

pujitha24 commented Oct 1, 2026

Uh oh!

dsimansk commented Oct 1, 2026

Uh oh!

dsimansk commented Oct 9, 2026

Uh oh!

knative-prow Bot commented Oct 9, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

pujitha24 commented Sep 18, 2026

Uh oh!

knative-prow Bot commented Sep 18, 2026

Uh oh!

dsimansk commented Sep 18, 2026

Uh oh!

pujitha24 commented Sep 18, 2026

Uh oh!

dsimansk Sep 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dsimansk Sep 18, 2026

Choose a reason for hiding this comment

Uh oh!

codecov Bot commented Sep 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

pujitha24 commented Sep 19, 2026

Uh oh!

pujitha24 commented Sep 19, 2026

Uh oh!

dsimansk left a comment

Choose a reason for hiding this comment

Uh oh!

pujitha24 commented Oct 1, 2026

Uh oh!

dsimansk commented Oct 1, 2026

Uh oh!

dsimansk commented Oct 9, 2026

Uh oh!

knative-prow Bot commented Oct 9, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

dsimansk Sep 18, 2026 •

edited

Loading

codecov Bot commented Sep 18, 2026 •

edited

Loading