Fix multiple ServerClaim references

[lukasfrank]

Proposed Changes

A ServerClaim can update multiple servers.

The claimFirstBestServer function finds, returns, and updates a Server. However there is no back-reference to the Server set.

[aobort]

@lukasfrank Could you describe the situation when it is possible?

As far as I see in the code, there is three branches:

1. ServerClaim has non empty reference to the server - in this case it will try to claim server by reference
2. ServerClaim has non empty selector - in this case it will try to claim server by label selector
3. If both previous conditions are false, then it will try to pick first available server
4. After server picked up, reference to claim object is put to server's spec
5. After that reference to server is put to claim's spec.

Therefore, the claimFirstBestServer function will be called only if there is no server reference or label selector in ServerClaim.Spec. The Server.Spec.ServerClaimRef field is cleaned up by the serverClaim finalizer, so you can't delete serverClaim resource without cleaning up the references. In the same time you can't cleanup ServerClaim.Spec.ServerRef field, bc we have validating hook which prevents it.

So, I can imagine the only scenario when you'll have server referencing to claim and the claim without reference to that server: if you'll turn down operator and perform some manual actions, like cleaning up the serverRef field or recreating the claim object. But in case of claim recreating the UID will change and again the check will make no sense.

[lukasfrank]

I'm considering the following case:

3. If both previous conditions are false, then it will try to pick first available server

If the claimFirstBestServer returns a free Server it will be passed to ensureObjectRefForServer and we set the reference there. If the controller now crashes, we have not an option to "restore" the state since it's not saved to the Claim.

[aobort]

I'm considering the following case:

3. If both previous conditions are false, then it will try to pick first available server

If the claimFirstBestServer returns a free Server it will be passed to ensureObjectRefForServer and we set the reference there. If the controller now crashes, we have not an option to "restore" the state since it's not saved to the Claim.

@lukasfrank I just think that in future we might enable concurrent reconciliation and we'll need another solution rather than straight iterating over all servers. Something like creation of lock object for server when the server is selected and lock deletion when all references are set.

I'd also take a look on how it was done for PV-PVC bounding. Anyways, if you're sure and insist this fix is necessary - let's proceed with this.