| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2019-11-12 | |||
| 18:43:23 | sean-k-mooney | looks like tis doing the normal multinode setup | |
| 18:44:30 | artom | That's the second attempt | |
| 18:44:40 | sean-k-mooney | yep | |
| 18:44:59 | sean-k-mooney | ill leave it open on one of my monitors and see what hapens | |
| 18:45:49 | sean-k-mooney | haha | |
| 18:45:54 | sean-k-mooney | you have a typo | |
| 18:46:36 | sean-k-mooney | artom: http://paste.openstack.org/show/785994/ | |
| 18:47:45 | sean-k-mooney | you did not include teh whitebox-compute role in the commit | |
| 18:47:52 | sean-k-mooney | or you forgot to remove that line | |
| 18:49:27 | artom | *facepalm* | |
| 18:49:37 | artom | 'git commit -a' isn't magic | |
| 18:49:53 | sean-k-mooney | it speciricly ignore untracted files | |
| 18:52:03 | artom | I know | |
| 18:52:42 | sean-k-mooney | good becaue i apparently cant spell specifcally and im not sure you woudl figure it out if you didnt :P | |
| 18:55:33 | sean-k-mooney | i need to try enabling spell check for irc again but the last time i tried that it was kind of depressing | |
| 18:57:29 | artom | *shrug* I forgive you :) | |
| 18:57:48 | artom | You have other uses besides spelling ;) | |
| 18:58:31 | sean-k-mooney | well if you want a terible way to generate passwords just ask me to spell somthing | |
| 18:59:14 | artom | lulz | |
| 19:04:54 | sean-k-mooney | so looking at the job you are trying to allocate 8G of hugepages on all nodes | |
| 19:05:07 | sean-k-mooney | that will not work with the non expanded flavors | |
| 19:05:11 | sean-k-mooney | *lables | |
| 19:07:17 | artom | derp | |
| 19:46:29 | mriedem | cool, the instance_actions_events table has a detail TEXT column that is never used, and when creating the InstanceActionEvent object we pass the exc_val but it gets passed down to the DB API in a 'message' kwarg rather than 'details' so that column is never populated | |
| 19:48:08 | mriedem | so as a non-admin user you can see a particular event passed or failed but not why, e.g. NoValidHost | |
| 19:48:20 | mriedem | and only admins can see the traceback (like a fault) by default | |
| 19:49:39 | sean-k-mooney | hum ok that sound like the behavior we want. are you going to reuse the colume for something | |
| 19:50:20 | sean-k-mooney | such as storging the traceback? | |
| 19:50:32 | mriedem | the traceback is stored in the traceback column | |
| 19:50:40 | mriedem | the exc_val is not stored at all | |
| 19:50:48 | mriedem | b/c the object is using the wrong column name | |
| 19:51:07 | sean-k-mooney | sure but i was wondering if you planned to chagne that | |
| 19:52:03 | mriedem | i think it would be useful to at least expose to the non-admin owner of the server the exception type, like we do for a fault message | |
| 19:53:37 | sean-k-mooney | ya maybe. as long as its just the type and not the body of the excption its proably safe. although im not sure all operator would be ok with that | |
| 19:53:55 | sean-k-mooney | e.g. it might affect there sla depening on what the falut was | |
| 19:54:13 | mriedem | if the instance goes to ERROR status we're already showing the exception type in the fault message | |
| 19:54:25 | mriedem | if not the formatted message | |
| 19:54:30 | sean-k-mooney | ya that is ture | |
| 19:54:47 | sean-k-mooney | i normally am logged in as root but we get the message in that case at least | |
| 19:54:56 | sean-k-mooney | * admin | |
| 19:55:05 | sean-k-mooney | so im not sure what a non admin sees | |
| 20:10:55 | sean-k-mooney | artom: :) looks like it worked https://openstack.fortnebula.com:13808/v1/AUTH_e8fd161dc34c421a979a9e6421f823e9/zuul_opendev_logs_3ae/691062/59/check/whitebox-multinode-devstack/3ae547c/testr_results.html.gz | |
| 20:11:29 | sean-k-mooney | we still need to adress the skips | |
| 20:12:14 | sean-k-mooney | but the ssh keys worked correctly and it looks liek the hugepages allocation did not cause memory issues | |
| 20:23:15 | mriedem | dansmith: your suggestions here to use the tuples for comparing times doesn't work in all cases https://review.opendev.org/#/c/636224/50/nova/compute/api.py@4956 | |
| 20:23:51 | mriedem | and i don't really want to bake a bunch of conditional logic into unreadable tuples | |
| 20:25:22 | dansmith | aight, well, do what you want | |
| 20:25:32 | dansmith | inlining all that in a closure seems bad, | |
| 20:25:46 | dansmith | especially for testability, which has clearly manifested itself already | |
| 20:26:09 | dansmith | that could be a | |
| 20:26:16 | dansmith | def newer_than(obj) | |
| 20:26:42 | dansmith | method on NovaObject and be usable for other things, as well as easier to test | |
| 20:27:11 | dansmith | is there not some way we could do this without times? | |
| 20:27:22 | dansmith | because that's kinda smelly in general, | |
| 20:27:33 | sean-k-mooney | i was going to suggest a before or after method or something but ya the tuple way would be nice if it was not for the value error | |
| 20:27:37 | dansmith | assuming they're all time-synced, you don't have any delays, delayed transactions, etc | |
| 20:27:45 | sean-k-mooney | *type error | |
| 20:29:01 | sean-k-mooney | oh this is in get_newer_job already | |
| 20:29:31 | sean-k-mooney | you could define a lessthan operator on the migration object | |
| 20:29:45 | sean-k-mooney | although i guess that would have the same proably with None | |
| 20:29:57 | dansmith | no, it wouldn't | |
| 20:30:08 | dansmith | but also, less-than can mean lots of things other than time | |
| 20:30:20 | sean-k-mooney | for a migration object | |
| 20:30:26 | sean-k-mooney | and ya i could | |
| 20:30:42 | mriedem | we could just say f it and do like get_all and filter out by uuid after we've already processed one | |
| 20:31:13 | mriedem | this https://github.com/openstack/nova/blob/master/nova/compute/api.py#L2883 | |
| 20:31:49 | dansmith | mriedem: yeah | |
| 20:32:12 | mriedem | the migration from the first cell processed in the results is always the one that is returned, | |
| 20:32:20 | mriedem | which could be wrong, but i don't really care at this point | |
| 20:32:40 | dansmith | does it matter/ | |
| 20:32:51 | dansmith | you just want to not return dupes right? | |
| 20:33:09 | mriedem | yes, but also figure that since we can know which is "newer" we should return that, | |
| 20:33:29 | mriedem | because the migration in the target cell is the one that's going to be getting updated as the resize progresses | |
| 20:33:42 | mriedem | including when it goes to 'finished' status and the server is in VERIFY_RESIZE status | |
| 20:34:16 | sean-k-mooney | mriedem ya i think the nestest makes sense | |
| 20:34:17 | mriedem | which is kind of important since i think i have functional tests later in the series asserting the migratoins api returns 1 and it's the correct status | |
| 20:34:26 | dansmith | mriedem: once you create the new one, couldn't you set the hidden flag on the old one or something/ | |
| 20:34:48 | mriedem | sure, that's the alternative mentioned in the code and the commit message | |
| 20:34:59 | mriedem | but it has implications for the api because that hidden field has never been set before | |
| 20:35:01 | dansmith | the *newest* only makes sense if we never update the old migration | |
| 20:35:41 | dansmith | mriedem: I don't understand what the problem with that is, as I said.. api ignores migrations with the hidden field already ... | |
| 20:35:50 | mriedem | i don't think we do update the source cell migration unless we revert the resize | |
| 20:36:13 | dansmith | we just leave it pending forever? | |
| 20:36:36 | mriedem | no, when you confirm the resize in the target cell we hard destroy the source cell instance which will hard destroy it's related migratoin in the source cell db, | |
| 20:36:44 | mriedem | same (in reverse) for revert | |
| 20:37:02 | dansmith | what I meant is.. we leave it in the half-finished state until that time comes | |
| 20:37:04 | mriedem | otherwise you can never resize back to that cell b/c of the duplicate instance uuid | |
| 20:37:13 | mriedem | pretty sure yeah | |
| 20:37:21 | sean-k-mooney | so in that case cant we have teh db return the latest version based on update or created time | |
| 20:37:29 | mriedem | sean-k-mooney: different dbs | |
| 20:37:30 | mriedem | so no | |
| 20:38:10 | sean-k-mooney | but we only need to look at the one of the dbs if we only want the latest one right? i gues we would have too look at all of them during a migration | |
| 20:38:18 | sean-k-mooney | ya ok | |
| 20:38:25 | dansmith | sean-k-mooney: maybe you should read the patch we're discussing? | |
| 20:38:43 | sean-k-mooney | i am but im also about to head off for the night | |
| 20:39:51 | sean-k-mooney | the reason i changed my mind half way true was the Verify_resize comment | |
| 20:43:37 | mriedem | so it's not just the active migration that is a duplicate either, it's all migrations linked to the instance - those get copied into the target cell db early in TargetDBSetupTask, because once we flip the instance mapping that's where the api is going to go to get migrations for that server, | |
| 20:43:59 | mriedem | so if we started setting the hidden field on migrations, we'd have to do it on all of them until we're sure where the server is going to ultimately be | |
| 20:44:38 | efried | If I initiate an operation as an admin user, does the context have is_admin set, or does that only happen when nova generates an admin context explicitly? | |
| 20:44:51 | mriedem | is_admin is set | |