[Feature]: Create Idempotent Update APIs for SMD

[erl-hpe]

Describe the feature

During the development of OpenCHAMI on vTDS we encountered a problem (ochami issue #55 ([BUG] Adding a BMC that already exists causes an error) with using ochami to do static discovery of nodes in which the blind use of POST operations to register data with SMD caused conflicts when two or more nodes were managed by the same BMC. This was fixed in ochami by deduplicating BMC information, but this only allowed a single pass through node data to work successfully. Any subsequent addition of a node on the same BMC or in the same group as previously discovered nodes would cause conflicts and fail.

The analysis of this is as follows:

The ochami discover static command parses the input it is given (whether a file containing node data or a JSON structure containing node data) and submits the contents of the file sequentially in a set of per-node operations that do the following:
POST component information about each node to SMD
POST component information about the BMC (Redfish Endpoint) taken from each node to SMD
POST 'rfe' (RedFish Endpoint) information taken from each node to SMD
POST 'iface' (Ethernet Interface) information taken from each node to SMD
POST a 'group' definition comprised of the group name, description, and the list of nodes to SMD
This was causing conflicts and failures when the node data contained the same BMC data in two or more nodes. By deduplicating the incoming BMC data in ochami it was possible to get past this for a single invocation of static discovery.

The limitation of this, however, is that, since ochami blindly POSTs this information to SMD, if a new node is added to an existing BMC or if a new node is added to an existing 'group' within SMD, and ochami discover static is used to make that change, the data, no matter how carefully collated by the command, will conflict with data already POSTed to SMD in the previous invocation, and the new node will not be completely created.

The obvious, but wrong thing to do is to change ochami discover static to pull data from SMD, massage it, and then PUT the data back on SMD. The reason this won't work well is that there is a cache coherency problem within ochami if two or more people are using ochami at the same time.

The alternative is to modify SMD to have idempotent methods of posting node / bmc and group data in a single atomic chunk. As far as I am aware, an API like that does not exist today, so we would need to modify both SMD and ochami to fix this issue.

The requested feature, then, is to create or modify the SMD APIs to provide an idempotent way to present the data necessary to add a node to SMD and then modify ochami to use that mechanism instead of what it uses today.

Why do you want this feature?

This feature is important to any administrator trying to update SMD with additional node data once initial node data have been ingested. As it stands today, subsequent runs of ochami discover static or similar mechanisms after initial node discovery will encounter collisions and fail.

Alternatives you've considered

As described, we addressed the first run collision issue in ochami by deduplicating data. Extending that solution to subsequent runs, however, would require ochami to fetch from SMD then update SMD which presents the specter of cache coherency and concurrency issues that could corrupt the SMD content. At the moment, I do not see a viable alternative to this feature.

Additional context

No response

Code of Conduct

• I agree to follow this project's Code of Conduct

[alexlovelltroy]

Some questions as we contemplate implementation:

1. With POST, PUT, and PATCH available to us, what do the relevant RFCs suggest about how the server should behave in this situation?

• Should the server return a response code indicating CONFLICT on the attempt to re-create a BMC? In that case, should the client recover by requesting the resource and using PUT/PATCH to update it?
• Does SMD already have separate code paths for updating or replacing a resource versus creating it? Should we use those more appropriately?

2. Since this is part of the ochami discover usecase, is there a bulk addition API available that might be more appropriate anyway? Should we consider some pre-processing on the client side before pushing a collection of resources?

[erl-hpe]

Just a guide from my understanding of the canonical definitions of POST, PUT and PATCH, which is that POST is for initially creating a resource and is not, typically idempotent (could return an error indicating conflict), PUT is for replacing a resource entirely with "new" data (could be the same data as before) and is idempotent. Not clear to me whether the behavior when replacing a resource that does not already exists causes it to bring the resource into existence or fails with a "not found" error. Idempotency suggests the former, but kind of makes POST pointless (unless it is to provide a way to be sure you DO NOT update an existing resource). PATCH is for updating parts of an existing resource and is idempotent. Again, idempotency suggests that PATCH and PUT are pretty much the same and that POST only differs by its refusal to replace or update the target resource. PATCH also runs into the standard ambiguity with respect to updating lists: if I supply a list with one element as part of a resource, should the list in the resource be replaced with a one element list? Should the element be prepended to the list? Should the element be appended to the list? How do I delete an element from a list? How do I unambiguously add an element to a list?

The list questions with PATCH are of interest with respect to "group" reosurces in SMD, because "group" resources are treated as lists that can be modified by the client. It would be cleaner to avoid having client modifiable lists within elements in the API. If, instead of being directly modifiable, "group" resources were simply a generated representation of component group membership, the ambiguity would be erased and the API would be cleaner and easier to implement. I suggest moving away from a model in which "group" resources are directly manipulable and toward one where group membership is simply an attribute of "component" resources.

[synackd]

@erl-hpe While I agree we should ensure idempotent APIs exist in SMD in general, you mention a few times that ochami "blindly" POSTs to SMD, yet I see no mention of the --overwrite flag, which uses PUT to overwrite existing data. The point of this flag is so that you don't accidentally overwrite data with a stock ochami discover static call. Are you aware of this flag?

[synackd]

Also, it's worth mentioning that I'm working on adjusting the format of the discovery file to make it so BMCs are separate entities that nodes can be linked with. See OpenCHAMI/ochami#55 (comment)

[erl-hpe]

@synackd I was unaware of the --overwrite flag. Thank you for letting me know about it. I haven't tried it yet, so I will come back and amend this if my understanding is wrong. The trouble I see with --overwrite as I understand it from your description is that PUT presumes you want to replace the existing contents completely with the new contents. That probably works for BMCs, but I don't see how it works for groups. If I add a new node from a nodes.yaml and that node is in the compute group like all the other existing compute nodes, will that mean that the resulting compute group will now have one entry consisting of the newly added node? That is what it sounds like. Of course, this is less of an issue with the idempotency of the API and more of a problem with using mutable lists for groups.

[synackd]

The trouble I see with --overwrite as I understand it from your description is that PUT presumes you want to replace the existing contents completely with the new contents.

Not quite. It assumes that, given a conflict, overwrite the old data with the new data. It does not delete old data and replace it only with the new data.

If I add a new node from a nodes.yaml and that node is in the compute group like all the other existing compute nodes, will that mean that the resulting compute group will now have one entry consisting of the newly added node? That is what it sounds like.

Nope, it will be appended to the existing compute group.

[synackd]

Thinking about it further, I have not tried using ochami discover static with a nodes.yaml containing a singular node you want to add to the compute group, and I'm not sure if it replaces the list or not. Is this what you tried and saw that the compute group was reduced to only that node?

[synackd]

Ultimately, ochami is a client that tries to work around shortcomings of the SMD API to achieve the admin's goals, so logic can be implemented in it to workaround the list issue, but I think focusing on improving the API would be the proper focus here.

The list questions with PATCH are of interest with respect to "group" reosurces in SMD, because "group" resources are treated as lists that can be modified by the client. It would be cleaner to avoid having client modifiable lists within elements in the API. If, instead of being directly modifiable, "group" resources were simply a generated representation of component group membership, the ambiguity would be erased and the API would be cleaner and easier to implement. I suggest moving away from a model in which "group" resources are directly manipulable and toward one where group membership is simply an attribute of "component" resources.

One use case I'm wondering about here is if the admin needs to modify group memberships in bulk. I could see that being difficult if we move away from the list-based mechanism, but I can see your concern for the lack of ability to easily modify a group list instead of replacing it. Is PATCH the proper way to go about this?

[erl-hpe]

As my first comment indicated, I don't actually know what the right semantics are for PATCH and lists. Lists just seem like a bad idea with REST APIs or any API that relies on merging new data into existing data. The demonstrably wrong way to go about it is to pull the list into the client, modify it, and give it back to the server. There is a strong probability of corruption due to concurrent access in that scenario. Given that, the only meaningful semantics for PATCHing a list within a resource would be to append or prepend the new content to the existing list, which means deleting entries in a list would be impossible anyway. Better to update the entities that you want to remove from a group and remove / change the designator of the group relationship from them. Most often you will be either deleting a component, which inherently removes it from the group, or changing its group membership, which also inherently removes it from any group to which it no longer belongs. I could be wrong, but I don't see another scenario in which removing a component from a group makes sense. I could see wanting to remove a group, but I think that will also lead to problems unless group membership is a (possibly empty) list itself for components, in which case, a DELETE operation on a given group would simply (inside SMD) traverse all members of the group and remove that group affiliation from them, but then you are back in the soup finding a way to edit the group list on a component.

[erl-hpe]

From what I have been able to tell, the best practices for lists are either a) to isolate the list elements as their own sub-resources within a resource (something like .../component/<component-id>/groups/compute) and then use PUT, POST or DELETE on that, or to implement a set of operations that are encoded in the payload you send to update a list element and do that with PATCH (something like {"operation": "add", "target": "groups", "element": "compute"}. Neither one looks very apetizing to me, but hard problems are hard. There is nothing that says that the data sent with PATCH has to look anything like the data in the resource. It can simply be a set of instructions for editing the resource.

[erl-hpe]

Okay, now that I have had a chance to try --overwrite I am seeing some interesting behavior. I added a new node (commenting out all the information for the existing nodes) to nodes.yaml and then ran that new file into ochami discover static --overwrite. What I see is that the component was added, the component endpoint was added, the redfish endpoint was unchanged (probably was actually changed but it was the same as it had been since the new data had the same content), the group content was unchanged (the new node was not added to it, nor was it overwritten with only the new node), and the iface entry for the new node was added but with the wrong (and a duplicate within the list) component-id. The new node's component-id was x2000c0s0b0n4 the iface data for it was x2000c0s0b0n0 but the description was Interface 0 for x2000c0s0b0n4. So, that is weird. Just so you don't think I am crazy, here is the new node I added (and the only non-commented out content in the yaml file):

- name: x2000c0s0b0n4
  nid: 5
  xname: x2000c0s0b0n4
  bmc_mac: 52:54:00:62:ec:d6
  bmc_ip: 10.3.1.1
  groups:
    - compute
  interfaces:
  - mac_addr: 52:54:00:71:df:d3
    ip_addrs:
    - name: compnet
      ip_addr: 10.2.1.36

When I uncommented all of the other node entries in the file and re-ran the command, that fixed the iface result with the wrong component-id but still did not add the new node to the compute group. I then wiped the SMD database, restarted everything and did the same thing adding all of the existing nodes plus the new node and got the same group behavior. Everything else was fine but my node never made it into the group.