customDevices update fix#78
Conversation
In execution time, if the controller-manager tries to update the list of customDevices on the powerNode, the update strategy fails with the message: "Operation cannot be fulfilled on powernodes.power.intel.com: the object has been modified; please apply your changes to the latest version and try again." This commit changes the update strategy to a merge-patch command, avoiding an issue during the power node update. Signed-off-by: Eduardo Juliano Alberti <eduardo.alberti@windriver.com>
madalazar
left a comment
madalazar
left a comment
There was a problem hiding this comment.
We'll need to bring this commit internally & have more people review it - for more awareness + transparency.
I think we also need to check the unit tests
| patch := client.MergeFrom(powerNode.DeepCopy()) | ||
| err = r.Client.Status().Patch(context.TODO(), powerNode, patch) |
There was a problem hiding this comment.
As far as I could understand with the way the Patch call is written we won't be running with optimistic locking which means we'd be overriding the customDevices field without trying to reconcile what's in etcd.
I'm a bit torn whether this particular call is the right thing to do.
Powernodes don't seem the powerconfig's main responsibility so I don't think it should be updating the customDevices field like that. I'd prefer to get into conflicts and trigger as many reconciler loops as needed to solve the conflict.
Now, on the other side, this is the only place (for the moment) where we update the powernode's customDevices field so there are no issues of concurrent updating.
2 participants
In execution time, if the controller-manager tries to update the list of customDevices on the powerNode, the update strategy fails with the message: "Operation cannot be fulfilled on powernodes.power.intel.com: the object has been modified; please apply your changes to the latest version and try again."
This commit changes the update strategy to a merge-patch command, avoiding an issue during the power node update.