[controller] Fix bug in TestHandleUpdateClusterCreatesStatefulSets

[jeromefroe]

This commit fixes a bug in the logic of the TestHandleUpdateClusterCreatesStatefulSets test. The test checks that the controller creates all the StatefulSet's in a new cluster. Previously, the test checked that all the expected StatefulSet's were created by checking the length of the map that the test uses to track StatefulSet's that were created:

expectedMu.Lock()
created := len(expectedSetsCreated)
expectedMu.Unlock()
if created != len(test.expCreateStatefulSets) {
    time.Sleep(100 * time.Millisecond)
    continue
}
done = true

However, when the test creates expectedSetsCreated it also inserts into it all the StatefulSet's that it expects so the previous if condition will always be true:

expectedSetsCreated := make(map[string]bool)
for _, name := range test.expCreateStatefulSets {
    expectedSetsCreated[name] = false
}

This commit changes the test so it no longer adds the expected StatefulSet's to expectedSetsCreated after it's created, so the subsequent check on the length of expectedSetsCreated will be valid because only the Reactor function will insert into expectedSetsCreated.

After making this change, however, I noticed that the tests were starting to fail. After some debugging, I realized this was because the Reaction function was true. The documentation for it states:

If "handled" is false, then the test client will ignore the results and continue to the next ReactionFunc.

Since we were returning true, the next function in the chain, which would add it to the "fake" Kubernetes client we're using, never got called, so it would never be returned from subsequent calls to list the StatefulSet's and, as a result, we would continue retrying to create it.

After changing the function to return false, I began encountering one final problem. Occasionally, the call to create a missing StatefulSet would return an error because the StatefulSet we were trying to create already existed. It seemed this was a caching problem caused when the call to list the StatefulSet's in the cluster didn't return the new StatefulSet. To address this problem I updated the error handing after calling Create to check if the error was because the StatefulSet already exists and to log and return nil if so.

[codecov]

Codecov Report

Merging #235 into master will decrease coverage by 0.10%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master     #235      +/-   ##
==========================================
- Coverage   75.69%   75.59%   -0.11%     
==========================================
  Files          30       30              
  Lines        2115     2118       +3     
==========================================
  Hits         1601     1601              
- Misses        391      392       +1     
- Partials      123      125       +2     

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 8cf55a0...6b26b61. Read the comment docs.

[schallert]

Nice catch! Thanks for the helpful write-up.