Deep-copy CD object only when mutation needed

[jgehrcke]

A side quest from #646 (comment).

I wanted to think this through, and not throw the patch away.

[copy-pr-bot]

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

[klueska]

This needs to be done on the newCD, otherwise we run the risk of this object changing out from under us (it's a pointer).

[jgehrcke]

You're saying that

nodeInfo = node.DeepCopy()

right below won't do the trick because we're just copying a pointer?

Will review.

[klueska]

No, I'm saying we can't loop over cd.Status.Nodes because that might change in the course of the loop because cd is a pointer into an object in the informer cache.

In this particular case it might be fine since there are no pointers in cd.Status.Nodes itself, but in general we would need to (at a minimum) iterate over a deep copy of cd.Status.Nodes to be sure nothing embedded in the object we are looking at gets changed behind our backs.

[jgehrcke]

Some conclusions:

• Of course because we pass the CD in by pointer there could hypothetically be a concurrent execution unit mutating the object underneath us (we know/assume this to not be true right now)
• DeepCopy() doesn't generally protect from concurrent mutation (it might happen during DeepCopy execution, and only locking would be able to guard against that).
• The informer itself is not mutating the CD object underneath us (other callbacks however may, if they run concurrently -- not sure if they ever do).
• Performance of just DeepCopy() is not a concern; it may only become significant when we call this function at pathologically high rate (in which case we should look into changing that).

Let's come back to this later. And when we do: we probably want to adjust all similar places in the same way, for symmetry. Maybe even using common code.