storage: Fix simulation of rebalance removals to actually remove targets

If the first target attempted was rejected due to the simulation
claiming that it would be immediately removed, we would reuse the
modified `rangeInfo.Desc.Replicas` that had the target added to it,
messing with future iterations of the loop.

Also, we weren't properly modifying the `candidates` slice, meaning that
we could end up trying the same replica multiple times.

I have a test for this, but it doesn't pass yet because the code in #18364
actually isn't quite sufficient for fixing cases like #20241. I'll send
that out tomorrow once I have a fix done.

Release note: None

pkg/storage/allocator.go

@@ -556,12 +556,14 @@ func (a Allocator) RebalanceTarget(
 		if removeReplica.StoreID != target.store.StoreID {
 			break
 		}
-		newTargets := candidates.removeCandidate(*target)
-		newTarget := newTargets.selectGood(a.randGen)
-		if newTarget == nil {
+		// Remove the considered target from our modified RangeDescriptor and from
+		// the candidates list, then try again if there are any other candidates.
+		rangeInfo.Desc.Replicas = rangeInfo.Desc.Replicas[:len(rangeInfo.Desc.Replicas)-1]
+		candidates = candidates.removeCandidate(*target)
+		target = candidates.selectGood(a.randGen)
+		if target == nil {
 			return nil, ""
 		}
-		target = newTarget
 	}
 	details, err := json.Marshal(decisionDetails{
 		Target:               target.String(),