test: SANDBOX-1826 - fix race condition in CreateSpaceRequest

[rsoaresd]

Description

Last Saturday (May 2), periodic-ci-codeready-toolchain-toolchain-e2e-master-ci-daily failed in the test TestRunUserSignupIntegrationTest/TestAutomaticApproval. We are likely facing flakiness in this test due to a race condition between ToolchainConfig CR update and the in-memory config cache refresh in the host-operator process.

The test updates ToolchainConfig to restrict auto-approval domains to anotherdomain.edu, then immediately creates a waitinglist3@redhat.com signup expecting PendingApproval. However, the UserSignup controller reads config from an in-memory cache (toolchain-common/pkg/configuration/cache.go), which is only refreshed when the ToolchainConfig controller reconciles.

If the UserSignup is reconciled before the cache is refreshed, the controller still sees the old config (all domains approved), auto-approves the user, and the test hangs forever waiting for PendingApproval conditions.

We can see in the UserSignup list that waitinglist3 was approved.

Assisted-by: Cursor

Issue ticket number and link

SANDBOX-1826

Summary by CodeRabbit

• Tests
  • Enhanced user signup test to include additional verification for toolchain configuration synchronization completion.

[coderabbitai]

Walkthrough

An end-to-end test for automatic user approval is enhanced to add an additional verification that the toolchain configuration synchronization has completed, alongside existing checks for automatic approval domains and verification state.

Changes

Test Enhancement

Layer / File(s)	Summary
Test Assertion test/e2e/usersignup_test.go	VerifyToolchainConfig now includes an additional assertion via wait.UntilToolchainConfigHasSyncedStatus(wait.ToolchainConfigSyncComplete()) to ensure toolchain config sync reaches completion in the "add user not matching domains" scenario.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 minutes

Suggested labels

test

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Title check	⚠️ Warning	The title mentions 'fix race condition in CreateSpaceRequest', but the actual change is adding a VerifyToolchainConfig check to TestAutomaticApproval in usersignup_test.go, not modifying CreateSpaceRequest code.	Update the title to reflect the actual change, such as: 'test: SANDBOX-1826 - add ToolchainConfig sync verification to TestAutomaticApproval'

✅ Passed checks (4 passed)

Check name	Status	Explanation
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

[MatousJobanek]

Nice, thanks

[openshift-ci]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: MatousJobanek, rsoaresd, xcoulon

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:

• OWNERS [MatousJobanek,rsoaresd,xcoulon]

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

[rsoaresd]

/retest

flaky test in TestAutomaticClusterAssignment

[metlos]

If I'm reading the code of the toolchainconfig controller correctly, there is no "sync in progress" state or something like that, so the sync complete status might be there for the entirety of this test function. Could you please explain in more detail how does this additional check help the situation?

[rsoaresd]

I wrongly thought that wait.ToolchainConfigSyncComplete() proves that the controller reconciled 😭 I will try to think about other solution

[rsoaresd]

Thank you so much for raising that point!