Consolidate saga forward and undo actions which create more than one item

[bnaecker]

As part of the instance create saga, we create a variable number of network interfaces for the guest. In the saga, that's done by first creating all the UUIDs for the NICs, and then a single action which creates all the NICs. Because that single creation action performs multiple fallible operations, we attached the corresponding undo action to the preceding saga node, the one which creates the UUIDs. This is because the undo action for a node is only performed if the action completes successfully. Should the NIC-creation action fail partway through creating a list of NICs, they would not be unwound if the undo action were attached to that node. By attaching it to the previous UUID-creation node, which has a no-op undo, we can be sure to unwind any NICs we created, even if we failed partway through. This is all done here, with a detailed explanation.

I used the same strategy in #1458, when creating multiple ephemeral IP addresses for guests, since that has the same properties. During review, it became clear that this is pretty confusing, and definitely bad for refactoring since one could change the forward action, but not realize that it's strongly coupled to the preceding action. This issue tracks moving both these to the strategy used in the disk-creation steps. There, the loop is "unrolled" in to a set of action-undo pairs, all of which are in separate nodes in the saga. Each creates one disk, and each undo removes that disk. This requires that we can statically limit the number of possible nodes, which I think applies in all these cases. It also means we need to have logic inside each node for conditionally running it, if the number of items requested is less than the static limit.

[davepacheco]

oxidecomputer/steno#29 / oxidecomputer/steno#30 (not yet landed) can also improve this. With that change, we'll create a new DAG for each instance-create saga, and that DAG can have exactly the right number of nodes for creating NICs, and you can attach the appropriate undo action to each node. It's basically the same as the disk-creation example you mentioned but you don't have to statically limit the number of them or create extra ones to fill out that limit.

[bnaecker]

Thanks @davepacheco that's very good to know. Is it worth waiting for those to land before fixing any of this? The fix is pretty small, but it'd still be good to avoid churn.

[davepacheco]

I expect those Steno changes to become ready this week or next, but then it may be a bunch of work to update Omicron to use them. I'm not sure how urgent this is.

If I understand right, the change you're proposing here is a necessary part of using the new functionality anyway. Maybe that's a reason not to wait?

[bnaecker]

Ok, thanks Dave. It's not urgent in that the current implementation is not wrong, just confusing. But it will be harder to maintain. I might unravel some of it now, which maybe will help the integration with these Steno changes too, as you point out. Thanks!

[bnaecker]

Closed by #1474