test: SANDBOX-624: TestForceMetricsSynchronization: EnsureMUR

[rsoaresd]

Description

Sporadically, we are hitting this flaky test:

=== RUN   TestForceMetricsSynchronization/tampering_activation-counter_annotations/verify_metrics_did_not_change_after_restarting_pod_without_forcing_recount
    awaitility.go:374: waiting for metric 'sandbox_master_user_records{[domain external]}' to reach '2'
    awaitility.go:382: 
        	Error Trace:	/tmp/toolchain-e2e/testsupport/wait/awaitility.go:382
        	            				/tmp/toolchain-e2e/testsupport/wait/awaitility.go:117
        	            				/tmp/toolchain-e2e/test/metrics/metrics_test.go:702
        	Error:      	Received unexpected error:
        	            	timed out waiting for the condition
        	Test:       	TestForceMetricsSynchronization/tampering_activation-counter_annotations/verify_metrics_did_not_change_after_restarting_pod_without_forcing_recount
        	Messages:   	waited for metric 'sandbox_master_user_records{[domain external]}' to reach '2'. Current value: 1
=== NAME  TestForceMetricsSynchronization
    clean.go:113: deleting UserSignup: multiple-signup-testuser-1 ...
    clean.go:113: deleting UserSignup: multiple-signup-testuser-0 ...
    clean.go:131: waiting until UserSignup: multiple-signup-testuser-0 is completely deleted
    clean.go:131: waiting until UserSignup: multiple-signup-testuser-1 is completely deleted
    clean.go:204: the UserSignup multiple-signup-testuser-1 doesn't have CompliantUsername set
    clean.go:238: the UserSignup multiple-signup-testuser-1 doesn't have CompliantUsername set
    clean.go:184: the related MasterUserRecord: multiple-signup-test is deleted as well

As we can see on the logs, "the UserSignup multiple-signup-testuser-1 doesn't have CompliantUsername set", so at this time, it does not have the MUR, and that's why it failed.

When the test TestForceMetricsSynchronization fails, when it tries to get the metric sandbox_master_user_records, the second UserSignup does not yet have the CompliantUsername set probably because it does not have enough time to generate it between the time that the UserSignup is created and the time the test access the metrics.

Since we are not ensuring MUR in the function CreateMultipleSignup, we are not ensuring that the UserSignup has already generated the CompliantUsername (CompliantUsername is needed for the MUR) before continuing the test flow.

For now, this is just a theory that I hope it will solve it :D

Issue ticket number and link

SANDBOX-624

[mfrancisc]

Looks good 👍 ! Thanks for improving this 🙏

JFYI - there is also this helper:

		userSignup, err = hostAwait.WaitForUserSignup(t, userSignup.Name,
			wait.UntilUserSignupHasCompliantUsername())
		require.NoError(t, err)

which waits for the UserSignup to have the CompliantUsername field set, if that is what we are looking for.

[mfrancisc]

I see the e2e job failing but it doesn't seem related to your change , maybe some other flakyness :)

   user_management_test.go:505: 
        	Error Trace:	/go/src/github.com/codeready-toolchain/toolchain-e2e/test/e2e/user_management_test.go:505
        	Error:      	Should be true
        	Test:       	TestUserManagement/TestUserDeactivation/reactivated_but_unverified_user_reverted_back_to_deactivated_after_timeout
        	Messages:   	usersignup should be deactivated

[sonarcloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
0.0% Duplication on New Code

See analysis details on SonarCloud

[rsoaresd]

wait.UntilUserSignupHasCompliantUsername()

@mfrancisc, thank you so much for the tip!! Can we wait for the CompliantUsername in the Execute function?

[rsoaresd]

I see the e2e job failing but it doesn't seem related to your change , maybe some other flakyness :)

   user_management_test.go:505: 
        	Error Trace:	/go/src/github.com/codeready-toolchain/toolchain-e2e/test/e2e/user_management_test.go:505
        	Error:      	Should be true
        	Test:       	TestUserManagement/TestUserDeactivation/reactivated_but_unverified_user_reverted_back_to_deactivated_after_timeout
        	Messages:   	usersignup should be deactivated

Goinggg to take a look 🏎️

[rsoaresd]

I see the e2e job failing but it doesn't seem related to your change , maybe some other flakyness :)

   user_management_test.go:505: 
        	Error Trace:	/go/src/github.com/codeready-toolchain/toolchain-e2e/test/e2e/user_management_test.go:505
        	Error:      	Should be true
        	Test:       	TestUserManagement/TestUserDeactivation/reactivated_but_unverified_user_reverted_back_to_deactivated_after_timeout
        	Messages:   	usersignup should be deactivated

@mfrancisc, based on this search query, it only happened once for now. I will keep an eye on it. Thanks!!

[alexeykazakov]

Thanks for addressing the comment!

[openshift-ci]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: alexeykazakov, mfrancisc, rsoaresd

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

The pull request process is described here

Needs approval from an approver in each of these files:

• OWNERS [alexeykazakov,mfrancisc]

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

[mfrancisc]

Can we wait for the CompliantUsername in the Execute function?

Maybe there are some cases where we don't want to "wait" until the CompliantUserName is populated , like when we create multiple signups in parallel, thus worth looking deeper at the code for the usage of wait.UntilUserSignupHasCompliantUsername() before trying to add it to the Execute function.