fix: configmgr need wait for all processed avoid tests data race

[zedongh]

configmgr Start test need ensure all queue source be processed then do assert action. avoid call l.called when it write at SyncConfig.

@lowang-bh
/close #3884

[volcano-sh-bot]

Welcome @zedongh!

It looks like this is your first PR to volcano-sh/volcano.

Thank you, and welcome to Volcano. 😃

[Monokaix]

seems add a rwlock is enough: )

[lowang-bh]

seems add a rwlock is enough: )

Agree!

[zedongh]

seems add a rwlock is enough: )

Agree!

Thanks a lot. After review the code of fake source, I make last simple commit in Stop() replace queue.Shutdown with queue.ShutdownWithDrain. Let me to cleaify the reason.

I believe there should be no concurrency here, so it is not suitable to use locks. The only scenario where concurrency occurs is as follows:

Mgr.Start goroutine

[Order 1]   key, quit := queue.Get()  // consume item and queue size -1

[Order 3a]  m.notifyListeners()

Test goroutine

[Order 2]. s.Stop() // queue length is zero, break loop, queue Shutdown, but item not Done yet
          // if f.queue.Len() == 0 {
          //  f.queue.ShutDown()
          //  return
          // }

[Order 3b] assert.Equal(t, tc.expectedNotifyCalled, l.called)

3a and 3b have no order guarantee. may cause data race

Simple to make origin test failed is set a time out in SyncConfig

func (l *FakeListener) SyncConfig(cfg *api.ColocationConfig) error {
        // time out will make this calling after testing assert, make it failed
	// time.Sleep(1*time.Second + 500*time.Millisecond)
	l.called++

[lowang-bh]

As the issue described，l.called++ at line 224 and l.called at line 305 has a read & write conflict.

[zedongh]

As the issue described，l.called++ at line 224 and l.called at line 305 has a read & write conflict.

Yeah, the reason is fakeSource Stop use queue.Shutdown not queue.ShutdownWithDrain cause the last queue item processing data race. So just fixed the Stop rather than add a rwlock.

[lowang-bh]

/ok-to-test

[JesseStutler]

Can we push forward this PR? I also meet this e2e failing frequently @hwdef @Monokaix

[Monokaix]

/approve

[volcano-sh-bot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: Monokaix

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 [Monokaix]

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

[Monokaix]

I think we can merge it first, this is a reasoable change, if it still fails, we can re-work ion that. @lowang-bh

[lowang-bh]

/lgtm