Ensure .status.phase Is Saved During Resource Creation
#189
Conversation
|
The comment does a good job of saying what it does, but it doesn't tell me why it's a problem. If it's not even been reconciled yet, then no power phase makes sense to me... |
|
Shouldn’t it default to |
|
Just throwing ideas around! So nothing else works this way is my concern. While not written down anywhere, the assumption is if the resource doesn't have a reconcile status, then it hasn't been seen by the controller yet, and thus every other status element is implicitly invalid. Then there is the whole split brain stuff to consider: the user is in control of the spec via the API, the controller is in control of the status. I've already fixed the field making it optional in the CRD. |
Do you mean you've made the |
|
To be fair, at this point in time, it's Unknown, so perhaps a new state is what we desire? |
…he instance has not yet been reconciled
|
Not sure if I've got this right, but I've just added a new |
| Conditions []unikornv1core.Condition `json:"conditions,omitempty"` | ||
| // Phase is the current lifecycle phase of the server. | ||
| Phase InstanceLifecyclePhase `json:"phase,omitempty"` | ||
| Phase *InstanceLifecyclePhase `json:"phase,omitempty"` |
There was a problem hiding this comment.
Just FYI, as this is really a string, the zero value of "" will trigger nothing to be generated. But mistakes can happen I guess, so the difference between set but empty, and not set may be handy. Although it causes the code to be a bit more messy :/
| // REVIEW_ME: Do we want to track who started the server, and when? | ||
| updated := current.DeepCopy() | ||
| updated.Status.Phase = unikornv1.InstanceLifecyclePhasePending | ||
| updated.Status.Phase = ptr.To(unikornv1.InstanceLifecyclePhasePending) |
There was a problem hiding this comment.
Hah, I thought you'd not use this API, I am pleasantly surprised 💯
Description
This PR fixes an issue where the
.status.phasefield was not persisted when creating a resource usingclient.Create(...). This occurs because the Kubernetes client does not persist the status sub-resource during creation by default. To address this, a separate status patch call is now made after the resource is created. Additionally, to prevent intermediate validation errors, the phase is defaulted toPendingwhen it is empty.