fix: correct context cancellation in waitForState to fix flaky test by RokeshVS · Pull Request #7547 · kedacore/keda

RokeshVS · 2026-03-15T21:08:19Z

Description

Fixes the flaky TestWaitForState test in pkg/scalers/external_scaler_test.go.

Change 1 — `external_scaler.go` (bug fix)

waitForState had continue instead of return when the context was cancelled.
This caused the goroutine to spin forever, blocking the test indefinitely.

// Before
if !changeState {
    // ctx is done, return
    continue  // goroutine loops forever when ctx is cancelled
}

// After
if !changeState {
    // ctx is done, return
    return  // goroutine exits cleanly when ctx is cancelled
}

Change 2 — `external_scaler_test.go` (timeout fix)

The select timeout of 1s was too tight. After GracefulStop(), the gRPC
client occasionally takes slightly over 1s to transition to SHUTDOWN state.
Increased to 5s to give sufficient headroom.

// Before
case <-time.After(time.Second * 1):

// After
case <-time.After(time.Second * 5):

Result

Ran the test 10 times consecutively — all pass consistently:

go test ./pkg/scalers/ -run TestWaitForState -v -count=10
=== RUN   TestWaitForState
    external_scaler_test.go:268: close state: SHUTDOWN
--- PASS: TestWaitForState (5.01s)
=== RUN   TestWaitForState
    external_scaler_test.go:268: close state: SHUTDOWN
--- PASS: TestWaitForState (5.03s)
=== RUN   TestWaitForState
    external_scaler_test.go:268: close state: SHUTDOWN
--- PASS: TestWaitForState (5.01s)
=== RUN   TestWaitForState
    external_scaler_test.go:268: close state: SHUTDOWN
--- PASS: TestWaitForState (5.01s)
=== RUN   TestWaitForState
    external_scaler_test.go:268: close state: SHUTDOWN
--- PASS: TestWaitForState (5.01s)
=== RUN   TestWaitForState
    external_scaler_test.go:268: close state: SHUTDOWN
--- PASS: TestWaitForState (5.01s)
=== RUN   TestWaitForState
    external_scaler_test.go:268: close state: SHUTDOWN
--- PASS: TestWaitForState (5.01s)
=== RUN   TestWaitForState
    external_scaler_test.go:268: close state: SHUTDOWN
--- PASS: TestWaitForState (5.01s)
=== RUN   TestWaitForState
    external_scaler_test.go:268: close state: SHUTDOWN
--- PASS: TestWaitForState (5.01s)
=== RUN   TestWaitForState
    external_scaler_test.go:268: close state: SHUTDOWN
--- PASS: TestWaitForState (5.01s)
PASS
ok      github.com/kedacore/keda/v2/pkg/scalers 50.208s

Checklist

Changelog has been updated and is aligned with our changelog requirements, only when the change impacts end users
Commits are signed with Developer Certificate of Origin (DCO - learn more)

Fixes #7542

Signed-off-by: RokeshVS <[email protected]>

github-actions · 2026-03-15T21:08:30Z

Thank you for your contribution! 🙏

Please understand that we will do our best to review your PR and give you feedback as soon as possible, but please bear with us if it takes a little longer as expected.

While you are waiting, make sure to:

Add an entry in our changelog in alphabetical order and link related issue
Update the documentation, if needed
Add unit & e2e tests for your changes
GitHub checks are passing
Is the DCO check failing? Here is how you can fix DCO issues

Once the initial tests are successful, a KEDA member will ensure that the e2e tests are run. Once the e2e tests have been successfully completed, the PR may be merged at a later date. Please be patient.

Learn more about our contribution guide.

snyk-io · 2026-03-15T21:08:33Z

✅ Snyk checks have passed. No issues have been found so far.

Status	Scan Engine	Critical	High	Medium	Low	Total (0)
✅	Open Source Security	0	0	0	0	0 issues

💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse.

Signed-off-by: RokeshVS <[email protected]>

Co-authored-by: Rick Brouwer <[email protected]> Signed-off-by: RokeshVS <[email protected]>

Signed-off-by: RokeshVS <[email protected]>

Co-authored-by: Rick Brouwer <[email protected]> Signed-off-by: RokeshVS <[email protected]>

Signed-off-by: Rick Brouwer <[email protected]>

rickbrouwer · 2026-03-16T09:58:53Z

/run-e2e external
Update: You can check the progress here

Signed-off-by: Rick Brouwer <[email protected]>

rickbrouwer · 2026-03-16T10:05:31Z

/run-e2e external
Update: You can check the progress here

rickbrouwer · 2026-04-05T09:25:29Z

Hi @RokeshVS

The issue still exists, so unfortunately it has not yet been resolved. Do you see a chance to take another look at this? See: https://github.com/kedacore/keda/actions/runs/23997664144/job/69988405523?pr=7606

RokeshVS · 2026-04-09T18:41:59Z

Hi @rickbrouwer ,

Thank you for flagging that the issue still existed after #7547 was merged. I have investigated the root cause further and raised a new PR to address the remaining race condition in TestWaitForState.

The previous fix corrected the continue → return in waitForState and increased the timeout, but the flakiness persisted under -race due to:

grpcClient starting in Idle after grpc.NewClient(), causing waitForState to miss state transitions
context.TODO() providing no cancellation, causing t.Log to be called after the test completed → panic

The new PR fixes this by calling grpcClient.Connect() to force an active connection, introducing a cancellable waitCtx so the goroutine exits cleanly, and calling grpcClient.Close() explicitly after GracefulStop() for a deterministic Shutdown transition.

Validated with go test ./pkg/scalers/ -run TestWaitForState -v -count=20 -race — all 20 runs pass consistently.

PR - #7635

Would appreciate your review when you get a chance. Thank you!

… external scaler (kedacore#7547) Signed-off-by: RokeshVS <[email protected]> Signed-off-by: Rick Brouwer <[email protected]> Co-authored-by: Rick Brouwer <[email protected]> Signed-off-by: jansworld <[email protected]>

fix: correct context cancellation in waitForState to fix flaky test

48aff47

Signed-off-by: RokeshVS <[email protected]>

RokeshVS requested a review from a team as a code owner March 15, 2026 21:08

keda-automation requested a review from a team March 15, 2026 21:08

rickbrouwer reviewed Mar 16, 2026

View reviewed changes

Comment thread CHANGELOG.md Outdated

fix: update changelog entry per maintainer feedback

094608d

Signed-off-by: RokeshVS <[email protected]>

keda-automation requested a review from a team March 16, 2026 09:05

rickbrouwer requested changes Mar 16, 2026

View reviewed changes

Comment thread CHANGELOG.md Outdated

Comment thread CHANGELOG.md Outdated

Update CHANGELOG.md

d98eb1f

Co-authored-by: Rick Brouwer <[email protected]> Signed-off-by: RokeshVS <[email protected]>

keda-automation requested a review from a team March 16, 2026 09:49

fix: update changelog by alphabetical order as per maintainer feedback

cdfa537

Signed-off-by: RokeshVS <[email protected]>

rickbrouwer requested changes Mar 16, 2026

View reviewed changes

Comment thread CHANGELOG.md Outdated

Update CHANGELOG.md

0737d3f

Co-authored-by: Rick Brouwer <[email protected]> Signed-off-by: RokeshVS <[email protected]>

keda-automation requested a review from a team March 16, 2026 09:57

rickbrouwer requested changes Mar 16, 2026

View reviewed changes

Comment thread CHANGELOG.md Outdated

Update CHANGELOG.md

bc71bd8

Signed-off-by: Rick Brouwer <[email protected]>

keda-automation requested a review from a team March 16, 2026 09:59

rickbrouwer requested changes Mar 16, 2026

View reviewed changes

Comment thread CHANGELOG.md Outdated

Update CHANGELOG.md

25de80e

Signed-off-by: Rick Brouwer <[email protected]>

keda-automation requested a review from a team March 16, 2026 10:05

rickbrouwer approved these changes Mar 16, 2026

View reviewed changes

rickbrouwer added the Awaiting/2nd-approval This PR needs one more approval review label Mar 16, 2026

rickbrouwer enabled auto-merge (squash) March 16, 2026 12:41

JorTurFer approved these changes Mar 19, 2026

View reviewed changes

rickbrouwer merged commit 0286c9c into kedacore:main Mar 19, 2026
26 checks passed

RokeshVS mentioned this pull request Apr 9, 2026

fix: resolve remaining race condition in TestWaitForState flaky test #7635

Merged

2 tasks

rickbrouwer removed the Awaiting/2nd-approval This PR needs one more approval review label Apr 13, 2026

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix: correct context cancellation in waitForState to fix flaky test#7547

fix: correct context cancellation in waitForState to fix flaky test#7547
rickbrouwer merged 7 commits into
kedacore:mainfrom
RokeshVS:fix/flaky-testwait-for-state-7542

RokeshVS commented Mar 15, 2026 •

edited by rickbrouwer

Loading

Uh oh!

github-actions Bot commented Mar 15, 2026

Uh oh!

snyk-io Bot commented Mar 15, 2026 •

edited

Loading

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

rickbrouwer commented Mar 16, 2026 •

edited by github-actions Bot

Loading

Uh oh!

Uh oh!

rickbrouwer commented Mar 16, 2026 •

edited by github-actions Bot

Loading

Uh oh!

Uh oh!

rickbrouwer commented Apr 5, 2026

Uh oh!

RokeshVS commented Apr 9, 2026 •

edited

Loading

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Conversation

RokeshVS commented Mar 15, 2026 • edited by rickbrouwer Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Change 1 — external_scaler.go (bug fix)

Change 2 — external_scaler_test.go (timeout fix)

Result

Checklist

Uh oh!

github-actions Bot commented Mar 15, 2026

Uh oh!

snyk-io Bot commented Mar 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

✅ Snyk checks have passed. No issues have been found so far.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

rickbrouwer commented Mar 16, 2026 • edited by github-actions Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

rickbrouwer commented Mar 16, 2026 • edited by github-actions Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

rickbrouwer commented Apr 5, 2026

Uh oh!

RokeshVS commented Apr 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

RokeshVS commented Mar 15, 2026 •

edited by rickbrouwer

Loading

Change 1 — `external_scaler.go` (bug fix)

Change 2 — `external_scaler_test.go` (timeout fix)

snyk-io Bot commented Mar 15, 2026 •

edited

Loading

rickbrouwer commented Mar 16, 2026 •

edited by github-actions Bot

Loading

rickbrouwer commented Mar 16, 2026 •

edited by github-actions Bot

Loading

RokeshVS commented Apr 9, 2026 •

edited

Loading