| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2017-09-06 | |||
| 20:56:19 | melwitt | doing that would remove the instance fault message which explains the quota check failed | |
| 20:56:37 | mriedem | melwitt: can't the build request have a fault on it? | |
| 20:56:40 | dansmith | and we have the instance in the cell db, that's better than a buildreq, so we should use it | |
| 20:56:47 | melwitt | I didn't know it can | |
| 20:56:53 | dansmith | mriedem: I don't think we ever update that instance do we? | |
| 20:56:57 | dansmith | because it's fake right? | |
| 20:57:17 | mriedem | build request doesn't have a fault field, so yeah, can't do that | |
| 20:57:35 | melwitt | it has an instance field which would have a fault field I guess | |
| 20:57:47 | dansmith | no, I think the instance is fake and constructed on the fly | |
| 20:57:54 | melwitt | oh, okay | |
| 20:58:05 | mriedem | the BuildRequest.instance is persisted | |
| 20:58:10 | mriedem | it's just serialized json | |
| 20:59:29 | mriedem | but, that starts to get weird... | |
| 20:59:31 | dansmith | yeah, I guess so | |
| 20:59:36 | mriedem | managing state on an instance nested in a build requst | |
| 20:59:38 | dansmith | I'm not sure why that's better though | |
| 20:59:41 | dansmith | yeah | |
| 20:59:56 | mriedem | it's only better if we want to keep the 2 loops b/c we're afraid of changing them | |
| 20:59:56 | dansmith | this makes a bunch of other regular code just be used for anything else you do to this dead instance | |
| 21:00:15 | dansmith | I ain't skeered | |
| 21:00:20 | mriedem | dude, | |
| 21:00:28 | mriedem | i shit my pants everytime we touch nova these days | |
| 21:00:33 | dansmith | hah | |
| 21:01:03 | mnaser | (which is totally why i thought putting a tiny little call/fixup in the exception handling = less scary | |
| 21:01:05 | mnaser | :p | |
| 21:01:12 | melwitt | well, doesn't moving the quota check to the end and having only one loop address the issue fine since we know the BDMs and tags will get cleaned up at delete time? | |
| 21:01:20 | mriedem | melwitt: i think so | |
| 21:01:38 | melwitt | so the fake instance thing isn't the only way to get rid of the two loop thing | |
| 21:01:40 | dansmith | mnaser: yeah, but we already have lots of "clean this up in this snowflake way" calls | |
| 21:01:50 | mriedem | a test that creates an instance with a bdm and tag and asserts those are removed from the db would make me feel safer | |
| 21:01:54 | dansmith | melwitt: the fake instance is the way to keep the two loops he said | |
| 21:02:07 | dansmith | melwitt: fearing change | |
| 21:02:15 | mriedem | i fear the landmines | |
| 21:02:22 | mriedem | i have literally 2 toes left | |
| 21:02:37 | melwitt | oh. well, my thinking is the two loops are new and it's probably better to put it back to one now that we know we can | |
| 21:02:40 | dansmith | the easier way to do that is just do what mnaser wanted to do in the first place | |
| 21:02:45 | dansmith | melwitt: I agree | |
| 21:03:08 | dansmith | melwitt: and also, the fake instance in a build request is a special case and we should have that oooonly in the case where we haven't created the instance yet or can't, | |
| 21:03:19 | mnaser | for me it's the fact that it makes it more comfortable to backport this sort of thing (which i hope we can do to stable/pike) | |
| 21:03:21 | dansmith | so putting more things in that basket when we already have an instance created in a real cell seems wrong to me | |
| 21:03:26 | dansmith | mnaser: right, so, | |
| 21:03:29 | dansmith | I was going to suggest: | |
| 21:03:42 | dansmith | how about we let mnaser do the simpler but uglier thing of only mapping if we hit that quota check, | |
| 21:03:44 | melwitt | dansmith: yeah, agreed | |
| 21:03:54 | mriedem | and then collapse the loops in queens only | |
| 21:03:56 | mriedem | i like that | |
| 21:03:57 | dansmith | backport that mofo' and then do this cleanup separately with a functional test to get rid of the two loop | |
| 21:04:00 | dansmith | yeah | |
| 21:04:24 | mnaser | i think that would be best: so that stable can remain stable and you have plenty of time to review a much bigger change like that :> | |
| 21:04:28 | mnaser | and you're not worried about stable breaking | |
| 21:04:42 | mriedem | mnaser: btw, how is it that vexxhost is already on pike code? | |
| 21:04:47 | dansmith | I think that's reasonable | |
| 21:05:05 | dansmith | mriedem: you mean "thank you for already being on pike" right? | |
| 21:05:06 | mnaser | mriedem puppet+rdo packages, trusting the openstack CI and puppet-openstack-integration we use | |
| 21:05:07 | mnaser | aha | |
| 21:05:12 | mriedem | dansmith: yes | |
| 21:05:26 | dansmith | thought so | |
| 21:05:27 | dansmith | :) | |
| 21:05:29 | mriedem | i'm just impressed and happy i mean | |
| 21:05:35 | mnaser | honestly it was an easy upgrade | |
| 21:05:37 | melwitt | yeah, it's awesome ppl are running pike | |
| 21:05:42 | mnaser | newton->ocata was a bit of uh a mess | |
| 21:05:56 | mnaser | 400,000 records being mapped in the new default cell? | |
| 21:05:59 | mriedem | well, placement and cells v2 | |
| 21:06:01 | mnaser | takes a little while :p | |
| 21:06:18 | mnaser | i backported the placement puppet manifests so we had placement running in newton so it was *one less thing* to worry about | |
| 21:06:27 | mriedem | smart | |
| 21:07:02 | dansmith | https://twitter.com/get_offmylawn/status/905537805903941633 | |
| 21:07:22 | mnaser | :> | |
| 21:07:30 | melwitt | f yeah | |
| 21:08:02 | mnaser | also well | |
| 21:08:10 | mnaser | running vanilla openstack means we don't much around a lot :) | |
| 21:09:17 | jaypipes | mnaser: ++ :) | |
| 21:09:28 | mnaser | so do we come to quorum to leave things as is except call set cell mapping in the exception handling code? | |
| 21:09:37 | melwitt | running vanilla openstack is the way to go. I have been part of running custom openstack and regretted doing it that way | |
| 21:09:39 | dansmith | mnaser: yeah | |
| 21:10:24 | mnaser | or i'll add a note as well | |
| 21:11:11 | mriedem | TODO i'd say | |
| 21:11:19 | mnaser | damn | |
| 21:11:21 | mnaser | um | |
| 21:11:21 | mriedem | you'll also have to keep track of which cells each instance goes into | |
| 21:11:23 | mnaser | i dont know what cell | |
| 21:11:24 | mnaser | yeah | |
| 21:11:32 | mnaser | i'll have to figure out a clean way to do that | |
| 21:11:36 | mriedem | just save that into a dict in the first loop | |
| 21:11:44 | mnaser | alright | |
| 21:12:47 | mriedem | everything already goes into that instances list variable, and is sometimes None for things that failed, but for things that did get mapped, i think you could map the instance uuid to the cell in a dict and then use those 2 variables to do the mapping in the failure block | |
| 21:12:57 | mriedem | s/mapped/created/ | |
| 21:13:57 | mriedem | hmm, i think i just realized another bug here | |
| 21:14:03 | dansmith | um | |
| 21:14:06 | dansmith | hah yesh | |
| 21:14:12 | mriedem | we're always mapping to the last cell | |
| 21:14:14 | dansmith | yeah | |
| 21:14:17 | mriedem | :) | |
| 21:14:27 | dansmith | oof | |
| 21:15:27 | dansmith | mnaser: so you might want to stack your change on top of the fix for that bug | |
| 21:15:35 | dansmith | which I can cook up | |
| 21:15:54 | mnaser | okay cool, ill work on the stuff to set mappings | |
| 21:17:45 | dansmith | mriedem: I'm guessing that this would have uncovered that if I had ever finished it: https://review.openstack.org/#/c/452006/ | |
| 21:18:41 | dansmith | ah, no, | |