Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2018-04-04
12:07:55 johnthetubaguy kashyap: ah, cool, upgrade sounds correct
12:08:12 kashyap Err, you're right
12:08:23 kashyap I'll do the s/feature/upgrade/
12:15:14 gmann johnthetubaguy: i might not be getting you completely but yes we can adopt the new set of rules at same time based on old rule is overridden or not. If not then check new rule otherwise go for old rules till we remove them after deprecation phase
12:19:20 johnthetubaguy gmann: not sure, do we have a spec for the Admin vs Read vs Write policy roles?
12:19:35 gmann johnthetubaguy: i think not yet.
12:19:53 gmann johnthetubaguy: just read your comment on patch.
12:20:01 johnthetubaguy gmann: I see that as way more important, and the best reason to add the more granular rules, if that makes sense?
12:20:14 gmann johnthetubaguy: i see your point
12:20:20 johnthetubaguy I assume its operators that want those read only roles that want the extra granularity?
12:20:38 johnthetubaguy so we can give them what the want, rather than what they are asking for, maybe?
12:20:40 gmann yea mainly those
12:21:49 gmann but with admin, read and write roles still we need granular rules with right default out of admin, read, write whatever suitable
12:22:20 johnthetubaguy yes, but I think adding admin, read, write is the reason to add the granular rules
12:23:57 johnthetubaguy gmann: I think there is a patch to add the global vs non-global stuff somewhere, I should dig that up too
12:26:11 gmann johnthetubaguy: ok, i ll search it tomorrow. and yea i think i agree on your point. it makes sense of granular rules with those role. otherwise operator keep having complexity of their own defined roles.
12:28:44 johnthetubaguy gmann: cool, that is the way I was thinking anyways, have a good evening/night
12:30:02 kashyap johnthetubaguy: A quick one: I think you meant to use FAKE_LIBVIRT_VERSION throughout consistently: https://review.openstack.org/#/c/558783/1
12:30:24 gmann johnthetubaguy: thanks. ll update based on the role patch.
12:32:31 johnthetubaguy kashyap: yeah, that is what I meant to say
12:33:13 kashyap Okido; should also probably add a constant for QEMU
12:36:01 johnthetubaguy yeah, I thought there was one already, but yeah
12:37:42 openstackgerrit Kashyap Chamarthy proposed openstack/nova master: libvirt: Bump MIN_{LIBVIRT,QEMU}_VERSION for "Rocky" https://review.openstack.org/558783
12:38:18 kashyap johnthetubaguy: Seems like (from a `git grep`) I'm the first user of the constant FAKE_LIBVIRT_VERSION in tests
12:41:49 johnthetubaguy hmm, that is curious
12:58:45 efried johnthetubaguy: Thanks for merging the error codes spec. Do you know who's authorized to approve the blueprint (other than mriedem, who doesn't seem to be around)?
13:05:47 johnthetubaguy efried: I can I think, in theory anyone with nova-specs core should be able to
13:06:16 efried johnthetubaguy: Okay, thanks.
13:06:41 johnthetubaguy that should look better now
13:07:01 efried johnthetubaguy: Nice, thanks! In case you felt like looking at the code while the spec is still fresh, it's ready: https://review.openstack.org/#/c/546177/
13:07:33 efried be nice to have this code in place for edleafe's work on consumer generations.
13:07:58 johnthetubaguy ah, I see your +2 there already, I will try take a look at that today
13:08:13 efried cool, thank you.
13:14:16 mriedem artom_: can you create https://blueprints.launchpad.net/nova/+spec/numa-aware-live-migration please?
13:22:22 artom_ mriedem, ack
13:24:26 artom mriedem, done
13:24:46 mriedem thanks
13:35:30 edleafe jaypipes: around?
13:36:04 edleafe efried: cdent: maybe you guys could help with a question
13:36:28 efried edleafe: I'm listening.
13:36:59 edleafe ok, <1.8, allocations didn't have project_id/user_id
13:37:09 edleafe in those cases, they are None on the object
13:37:44 edleafe the code I'm doing for consumer generations tries to insert a record in the consumer table if one doesn't yet exist
13:38:15 edleafe But there is a NOT NULL constraint on the project_id and user_id columns, so in that case (and many of our tests) the insert fails
13:38:46 edleafe Should I just punt and say that for <1.8, no consumer record can be created?
13:39:20 efried edleafe: tbc, this is because you wanted to be able to put the generation into the consumers table even for earlier microversions, yah?
13:40:15 dansmith mriedem: I was holding this for your approval: https://review.openstack.org/#/c/558059/4
13:40:25 edleafe efried: well, normally I'd put a conditional and skip for <1.8. But that means that later calls can and will overwrite
13:40:38 edleafe efried: It does seem that that's the best we can do here
13:40:44 edleafe Just wanted to get a second opinion
13:41:09 efried edleafe: So backing up for a second: today (before your code) if you try to create a consumer record with microversion <1.8, what happens?
13:41:39 efried ...because presumably the request payload doesn't require the proj/user IDs at <1.8
13:41:46 edleafe efried: the code checks for the presence of those two fields on the allocation object, and if they aren't there, or are None, it skips the creation
13:42:04 efried But how does it ultimately close the loop, then?
13:42:15 edleafe So my feeling is we should continue that behavior, even if it sucks
13:42:15 efried The allocation gets created with... no consumer?
13:42:25 edleafe efried: yep
13:42:49 efried So the consumer in that case is represented only by its UUID in the allocations table; it doesn't truly exist otherwise
13:42:55 edleafe TBC, no consumer *record*
13:43:10 edleafe it still has the instance UUID in the consumer_id field
13:43:27 efried ...of the allocations table
13:43:33 edleafe yes
13:44:46 efried so presumably if you GET an allocation record at 1.8 that was created at <1.8, the proj/user doesn't show up in the response.
13:44:54 efried or it shows up, but with null?
13:45:28 edleafe yeah, they are None >=1.8
13:45:42 efried Seems to me like the right thing would be to make proj/user be nullable.
13:45:55 efried and always create the record, even <1.8
13:46:09 efried ...which isn't a behavior change, because the API will still be doing exactly the same thing.
13:46:43 edleafe It's a teeny behavior change, but a real edge of an edge case
13:47:28 efried How would it be a behavior change? You're just changing the condition from "if the record doesn't exist, assume null/null" to "use what's in the record, which will always be there (oh, and btw, might be null)"
13:47:28 edleafe here's the case:
13:47:41 mriedem dansmith: i saw, but haven't dug into the changes yet
13:47:50 dansmith mriedem: okay just wanted to make sure you did
13:47:56 edleafe allocations are created <1.8. No consumer record in the past; now there is w/generation=0
13:48:15 efried dansmith, mriedem: You'll be delighted to know that I checked our OOT driver, and you don't break it with that change.
13:48:33 efried ...because we use the tuple form already.
13:48:58 efried edleafe: Sorry, still listening.
13:49:01 edleafe np
13:49:19 mriedem efried: i don't know why your out of tree driver would care about that change
13:49:52 edleafe A later attempt to allocate that provides the generation may or may not succeed, depending on the gen. If there were no consumer record, though, it would always succeed
13:50:02 edleafe IMO, that's not a bad change, but it is a change
13:51:27 openstack Launchpad bug 1761197 in OpenStack Compute (nova) "Not defined keypairs in instance_extra cellsV1 DBs" [Undecided,New] - Assigned to Surya Seetharaman (tssurya)
13:51:27 belmoreira dansmith mriedem can you have a look into https://bugs.launchpad.net/nova/+bug/1761197 and give me your opinion?
13:51:31 efried mriedem: https://github.com/openstack/nova-powervm/blob/master/nova_powervm/virt/powervm/tasks/network.py#L183-L186 where _get_vif_events uses the tuple form: https://github.com/openstack/nova-powervm/blob/master/nova_powervm/virt/powervm/tasks/network.py#L226
13:52:16 mriedem efried: that's how it's used in tree today too
13:52:18 efried edleafe: But the "later attempt to allocate that provides the generation" *should not* "always succeed"
13:52:33 efried mriedem: Right, I guess we've ported that code over already.
13:52:44 kashyap dansmith: Hi there, if you get time today, I too (like johnthetubaguy, who +2ed) am going blind on this: https://review.openstack.org/#/c/534384/
13:52:57 efried edleafe: Besides, there's not a behavior change wrt generation handling cause we don't *have* generation handling yet.
13:53:03 efried edleafe: IMO this is all tasty goodness.
13:53:10 edleafe efried: sure, but if we preserve the behavior with allocations <1.8 (who even uses that anymore?), it will always succeed
13:53:19 edleafe As I said, an edge of an edge case
13:53:20 mriedem efried: i guess you're referring to how event_names in that interface can be a list of names, rather than tuples https://github.com/openstack/nova/blob/master/nova/compute/manager.py#L447
13:53:38 efried mriedem: right. Was allowable before dansmith's patch on your change.
13:54:07 dansmith belmoreira: hmm, are you saying that back when you ran the keypair migration they weren't sync'd down to the v1 cell databases and thus nothing happened?
13:55:26 dansmith efried: everyone copied the first implementation in libvirt I imagine, which was luckily done the more explicit way..I checked all the in-tree drivers
13:55:31 efried edleafe: Still not sure I follow. If you're using <1.8, you'll get generations incremented under the covers and you'll never know about it. As soon as you start using the new microversion (the one you're writing), you'll see the generation (whatever it is) when you GET, and you'll push it back when you PUT/POST. That's as it should be. But that behavior ought to be the same regardless of what we do with the proj/user
13:55:37 efried dansmith: nod
13:56:06 efried edleafe: The proj/user isn't required even at 1.8, is it?
13:56:33 efried hm, looks like the user_id isn't marked as optional.

Earlier   Later