Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2018-03-20
13:52:37 gibi cdent: you can free to forward the ping to other API experts :)
13:52:48 gibi s/can/are/
13:53:32 cdent gibi: hmmm. that is an interesting one. my general rule is capture well here: http://eavesdrop.openstack.org/meetings/nova_scheduler/2018/nova_scheduler.2018-03-19-14.00.log.html#l-178
13:53:53 alex_xu_ jaypipes: one more question, what is the reason you didn't want to add prefered trait in placement
13:54:25 sean-k-mooney[m] efried: damit lost the message. any TL:DR CUSTOM_SEÁNS_TRAIT would be invalid which is not much of a deal for me but some operators may want to create custom traits in there own language in teh future that said we should try to converge in a set of standard traits to cover there needs and cross the non english traits bridge when we really need too
13:56:29 efried alex_xu_: Because placement is a placer, not a weigher
13:57:10 efried alex_xu_: That's my interpretation of jaypipes' opinion, not necessarily my own
13:57:38 sean-k-mooney[m] alex_xu_: placement is fitting hard constratins, a prefereed trait is a best effort request. filtering based on a best effort request would potentially result in no valid host when there were hosts that met the required traits but no the prefered hence why jaypipes wanted to handel prefred in a weigher not in placement
13:58:13 sean-k-mooney[m] what efried said :)
13:58:50 gibi cdent: thanks, does this also means that in the old microversion we have to keep the old behavior which means not to fail the request just ignore the requested resources?
13:58:53 alex_xu_ efried: thanks for the high level explain, sean-k-mooney[m] thanks for the detail explain :)
13:59:03 efried heh
13:59:37 sean-k-mooney interesting. i have been testing out riot.im as an irc bouncer/bridge. looks like there is a 5-10 secodn delay when i use riot vs direct irc at least on posting. i get messages almost instanly on both
14:00:20 sean-k-mooney sean-k-mooney[m] is my riot.im client
14:00:21 cdent gibi: strictly speaking, yes, but it depends on the severity of the bad behavior
14:00:34 cdent I'm not generally a fan of the strict interpretation
14:02:23 gibi cdent: in this particular case the nova boot request was accepted and the requested resources was ignored. After the proposed change such a request would be rejected with an error message
14:02:27 alex_xu_ sean-k-mooney: Does that mean I require TRAIT_A, and prefer TRAIT_B, there is chance to return nothing when there is no TRAIT_B for any RP? not quite understand
14:02:59 cdent gibi: Again, using the strict interpretation, the older microversions should continue to accept
14:04:03 gibi cdent: assume for a second that we don't make it strict and allow failing in the old microversion. This would mean that there would be no difference between the old and the new microversion
14:04:34 gibi cdent: the only difference would be in the documentation
14:04:41 gibi cdent: about the meaning of the accpet
14:04:53 edleafe alex_xu_: "preferred" doesn't mean anything in placement, since it doesn't order anything
14:05:01 gibi cdent: in this case do we need the microversion bump just for the doc?
14:05:13 edleafe "required" implies a filtering, which is what placement does
14:05:54 alex_xu_ edleafe: 'preferred' order the number of traits which the RP has
14:07:15 cdent gibi: assuming that, then I wouldn't think a microverison was required for a doc change, because the behavior is the same everywhered, right? But if there is in fact a behavior change (in the API itself), that seems like a microversion, right?
14:08:11 alex_xu_ or you guys refer to the implement problem
14:09:45 edleafe alex_xu_: if I prefer a trait, I will accept an RP that doesn't have that trait if necessary. So Placement will return all RPs that satisfy the requirements. It is then up to the scheduler to sort them based on various things, and preferred traits would be one of those things
14:10:32 gibi cdent: I'm hesitant. The external behaviour will change as nova will reject a previously accepted the request. But even if nova today accept such request nova actually lies and nova does not fulfill the request properly as nova does not consider the QoS bandwidth policy on the port that is included in the request during the scheduling
14:11:21 gibi cdent: so by rejecting such request nova we clean up a lie
14:11:23 alex_xu_ edleafe: oh, the weigher will change the order again
14:11:29 cdent gibi: and you break working code
14:11:35 gibi cdent: true
14:11:59 cdent gibi: Which is what microversions are supposed to prevent, even though it means broken (in other ways) client code gets to continue existing
14:12:11 cdent I agree that it is a tricky problem and your hesitancy is warranted
14:12:23 cdent Do you feel the "lie" is a security problem?
14:13:05 gibi cdent: no, the lie is not a security issue. It is just a resource allocation issue
14:13:29 edleafe gibi: would you call the "lie" a bug?
14:13:39 edleafe IOW, Nova isn't doing what it's supposed to?
14:14:03 gibi edleafe: Nova missing support for including the port QoS policy in the placement decision
14:14:38 edleafe gibi: understood. What I'm asking is if nova not doing that is a bug
14:14:51 edleafe or is it just a feature that has not yet been implemented
14:15:01 gibi edleafe: it is a missing feature, not a bug.
14:15:09 gibi edleafe: it never worked before
14:15:15 edleafe gibi: ok, then it definitely needs a microversion
14:16:04 gibi edleafe, cdent: thank you for the discussion. I will link this discussion to the spec
14:16:26 mriedem gibi: "The external behaviour will change as nova will reject a previously accepted the request. But even if nova today accept such request nova actually lies and nova does not fulfill the request properly as nova does not consider the QoS bandwidth policy on the port that is included in the request during the scheduling" - same story with rebuilding a volume-backed instance with a new image; we used to accept that and just n
14:16:26 mriedem onor it, but changed it to a 400 in the API in queens
14:16:39 alex_xu_ each microversion discussion is a war
14:17:25 edleafe alex_xu_: https://twitter.com/EdLeafe/status/976100354650791936
14:17:40 mriedem not really
14:17:52 alex_xu_ edleafe: :)
14:18:42 gibi mriedem: so there is precedence not to bump microversion in this case even if it breaks client code
14:19:19 gibi edleafe, alex_xu_: :)
14:20:14 mriedem gibi: https://review.openstack.org/#/c/520660/
14:20:36 mriedem like most things, it's a case by case basis, but in the case of ^ we said it wasn't a microversion because it was a silent failure on the compute side
14:20:50 mriedem you shouldn't have to opt into being not broken
14:21:13 mriedem but i'm sure with enough time and examples we can spin that all ways
14:22:27 openstackgerrit Chris Dent proposed openstack/nova master: DNM: Demo code for microversion parse extraction https://review.openstack.org/550265
14:23:29 edleafe mriedem: yeah, that was why I was asking if it was a bug or a new feature. You shouldn't have to opt into bug fixes.
14:23:58 alex_xu_ mriedem: https://review.openstack.org/#/c/520660/ is a bug, I feel it is different
14:24:11 gibi mriedem, cdent, edleafe: in the bfv rebuild case that rebuild has never worked before
14:24:18 openstackgerrit Silvan Kaiser proposed openstack/nova master: Exec systemd-run with privileges in Quobyte driver https://review.openstack.org/554195
14:24:29 alex_xu_ gibi: the qos policy works with neutron currently, right?
14:25:05 gibi alex_xu_: the qos policy without placement support cannot really work properly even if neutron implemented some data plane enforcement for that policy
14:26:01 mriedem gibi: depends on what you mean by 'worked',
14:26:14 mriedem the api used to accept that kind of request and return a 202, and nothing would fail, but it was wrong
14:26:26 mriedem wrong in that it didn't actually replace the root disk
14:26:38 gibi alex_xu_: imagine that you promised 10G minimum bandwidth for two VMs on the same host and on the same 10G PF. That will cause inconsistency when both VM start using that minimum bandwidth heavily
14:26:53 mriedem today you can create a port with a qos policy and attach it to an instance, right? nova allows that w/o changing any resource consumption for that qos policy.
14:27:02 gibi mriedem: right
14:27:27 mriedem if we followed the volume-backed rebuild + new image path here,
14:27:41 mriedem we'd merge a patch to check if the port (or network) has a qos policy and fail if so
14:27:53 gibi yes
14:27:57 mriedem and then land a microversion that allows supporting those types of ports
14:28:12 gibi mriedem: wait a bit
14:28:18 mriedem which is what https://review.openstack.org/#/c/532407/ is for
14:29:00 mriedem thinking about volume multiattach, we kind of have the same situation - you can't attach a multiattach volume with microversion < 2.60, you have to opt into using those types of volumes
14:29:00 gibi mriedem: we talking about adding microversion bump to the first patch where we start rejecting such requests not just microversion bump when we later start supporting such request
14:29:31 bauzas sean-k-mooney[m]: looking at https://review.openstack.org/#/c/545951/2/specs/rocky/approved/enable-sriov-nic-features.rst
14:29:32 mriedem gibi: if you do that, anyone that doesn't have that code in their cloud can have users still attaching those kinds of ports
14:30:08 mriedem if we consider it a bug that we don't support ports with qos policies, then we should make that a hard failure (bug fix) and then add a new microversion which adds the support for those types of ports
14:31:08 gibi mriedem: so making it a hard failure is a bugfix, I like that
14:31:17 alex_xu_ emm...wait, we don't check the network has qos policy, we check the port has required resource or not. That is something new added in neutron
14:31:33 mriedem gibi: well that's what we did for the bfv rebuild case
14:32:30 gibi alex_xu_: required resources is just another representation of the QoS policy to let neutron do the transformation between QoS policy entity on the API to the resource classes and traits in the Placement
14:32:49 mriedem tbh, we're inconsistent on this type of thing. with the multiattach support in queens, we check if the volume is multiattach and if the microversion is high enough (2.60) and if not, we fail. but i'm pretty sure in pike you could probably attach a multiattach volume to at least one instance at a time without failures.
14:33:40 alex_xu_ gibi: the neutron side add 'resources' field to the port API by extension? or just add directly?
14:33:41 gibi cdent, edleafe, mriedem: I start to get convinced that making the API hardfail instead of accept and lie is a bugfix
14:35:06 mriedem so the bug is today we are potentially over-subscribing the qos bandwidth right?
14:35:10 mriedem for a given network
14:35:15 gibi alex_xu_: that will come from the QoS neutron plugin based on my current understanding
14:35:36 gibi alex_xu_: the neutron spec will describe this in detail (writing is in progress)
14:35:59 mriedem the neutron qos stuff has been around for a long time hasn't it? kind of surprised no one hasn't already reported this as a bug.
14:36:24 gibi mriedem: yes, SRIOV ports with QoS minimum bandwidth policies are not properly enforced and the minimum bandwidth cannot be garanteed
14:36:39 gibi mriedem: only SRIOV ports supporting minimum bandwidth policies
14:37:39 gibi mriedem: and there was a release notes that stated the problem
14:37:47 gibi mriedem: https://github.com/openstack/neutron/blob/49d614895f44c44f9e1735210498facf1886c404/releasenotes/notes/qos-min-egress-bw-rule-b1c80f5675a4c1c3.yaml
14:38:33 gibi mriedem: so maybe the deployers read the release notes and understood that the support for the minimum bandwidth rule is incomplete

Earlier   Later