On 12/19/2018 9:03 AM, Balázs Gibizer wrote:
* the next two patches [6][7] implement rejecting use cases that was decided to out of scope for the feature (interface attach with resource requst, booting with network having resource request)
I really thought these would go *before* any new feature changes since we currently do not support these types of ports, so if a user were to create a port (or network) with QoS rules and attach it to a server in nova, we're just ignoring it, which is essentially broken. I think of this like how we rejected attempts to rebuild a volume-backed server with a new image (we used to just let that slide with no errors but it didn't actually rebuild the root volume) or trying to attach SR-IOV ports (again, not supported in compute). I think of the "reject" patches more as bug fixes for existing broken, or at least misleading, code.
* The next patch [8] add support for removing resource allocation during interface detach if the detached port has resource request
This is one example of how the implementation is backward. This is something that would be necessary plumbing before enabling a new feature in the API (assuming this cleanup happens in nova-compute?).
* The rest of the series up until [9] implement support for selecting VFs form the same PF that the bandwidth was allocated from. This allows handling bandwidth and VF resources properly even if there are more than one PFs on the same compute connected to the same physnet.
This is really kind of news to me. Maybe I glossed over this or didn't understand it in the spec, but the spec doesn't really say much about this either. There is only one mention of VFs in the spec: "When multiple ports, having QoS policy rules towards the same physical network, are attached to the server (e.g. two VFs on the same PF) then the resulting allocation is the sum of the resource amounts of each individual port request." Anyway, it seems like a sub-feature and I just didn't realize it.
What is still missing from the patch series: * Support for any kind of server move operations (resize, migrate, evacuate, live-migrate, unshelve) * Upgrade support: Bound SRIOV ports might already have QoS policy that would need healing resource allocation in placement.
This wouldn't be a problem if we were already rejecting ports with QoS rules applied as mentioned above.
Order of the patches --------------------
One comment I got from Matt is that the patches are in wrong order. The current order allows booting server with bandwidth request_before_ every other server operation is supported (or rejected). This could lead to the violation of the resource view of the system. For example booting a server with a QoS aware port will result in bandwidth allocation on the compute host. But then later on migrating that server to another host does not properly handle moving the allocation to the target host.
In my view reordering the patches to only allow booting the first server with bandwdith after every other operation is supported is not feasible. The current chain is already 17 patches long and it does not touch server moving operations at all. Also if I cannot boot a server with bandwidth then I cannot test that such server can be moved properly. So all the functional test would end up in the last patch of the series.
When I say wrong order, I mostly mean from the bottom up, not from a feature parity perspective. The normal way a long complicated feature series like this that involves the entire stack would be to start plumbing code into nova-compute at the bottom layer and then work your way up to the control layer services (conductor/scheduler) and finally the API where the feature is turned on (usually with a microversion). I am semi OK with deferring move operation support in v1 and building that on later given the complicated nature of this feature, but my real issue in the order is the backward nature of it (I don't want to approve patches that add code to the API for a feature just to leave it fall on the ground if the compute code isn't already merged).
However I think we can use the feature flag approach. We can implement and verify everything in the current order without the end user seeing or causing any kind of inconsistencies. The nova implementation is only activated if a neutron port has a resource_request field. This field is added to the neutron API as an API extension. We can mark this extension experimental while we are working through the missing pieces of the feature. But we can also turn on that extension in our test envirnoment to verify each and every step during the implementation process.
I'm definitely not crazy about the feature flag idea, but I need to read and reply to the other responses in the thread first.
Which RP fulfills the request of a neutron port -----------------------------------------------
Neutron expects a single RP uuid sent by nova in the port binding that tells neutron about which RP providers the resources for that port. This is the famous problem to find which request group is fulfilled by which resource provider in an allocation candidate. There are 3 possible solutions:
* Neutron does the mapping. This would require Neutron to see each port binding for a given server as a single request. This is not the case today and not feasible to hack around in neutron.
Before I started reviewing the code, I read the spec again to get the context in my head and I really thought this is what was proposed - that the allocation candidates that we used to claim resources in the scheduler would be passed through to the port binding request and neutron would sort out whatever it needed to from that. This is the section of the spec I'm thinking of: https://specs.openstack.org/openstack/nova-specs/specs/stein/approved/bandwi... Did something change drastically since the spec was written such that nova needs to do this now, at least temporarily? -- Thanks, Matt