Fix some race conditions

[wallrj]

• Wait for controller manager to exit before exiting the test (prevents calls to t.Log after the test has finished)
• Add Lock around the setting and reading from the fake Etcd client (prevents the controller using a MembershipAPI which has been replaced in the test)
• Return a copy of the members list (prevents controller using a members list which might be mutated during the test)

The controller-runtime manager.Start doesn't wait for all its go-routines to finish,
which leads to another race when the controller runtime attempts to log (via t.Log) after a test has finished.
See:

• zaptest: test logger seems susceptible to data races (?) uber-go/zap#687 (comment)
• Manager does not wait for Runnables to stop kubernetes-sigs/controller-runtime#350

Part of: #117

[wallrj]

You can see the leaked goroutine in the test results;

Goroutine 270 (running) created at:
  sigs.k8s.io/controller-runtime/pkg/internal/controller.(*Controller).Start()
      /go/pkg/mod/sigs.k8s.io/[email protected]/pkg/internal/controller/controller.go:157 +0x4ee
  sigs.k8s.io/controller-runtime/pkg/manager.(*controllerManager).startLeaderElectionRunnables.func1()
      /go/pkg/mod/sigs.k8s.io/[email protected]/pkg/manager/internal.go:326 +0x74

--- https://app.circleci.com/jobs/github/improbable-eng/etcd-cluster-operator/668/parallel-runs/0/steps/0-102

[wallrj]

I've removed the -race parameter now.
We will have to enable the race checks once the bug has been fixed in controller-runtime.
Meanwhile this fixes three of the race conditions.

[JamesLaverack]

This new slice doesn't copy the underlying members does it? So if you modified the Members themselves it would still mutate the old slice?

[wallrj]

Yeah, I didn't bother, because I was concentrating on fixing the race conditions, but I've now added a deep copy function to avoid any accidental mutation of the Member list.