| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2018-06-11 | |||
| 15:17:22 | dansmith | oh did I forget to do that? | |
| 15:17:26 | smcginnis | mriedem: Yeah, I believe that should be done in lower-constraints. | |
| 15:17:31 | mriedem | dansmith: you did, but that's not your patch | |
| 15:17:35 | dansmith | oh | |
| 15:17:40 | dansmith | I was going to say | |
| 15:18:05 | dansmith | surely the requirements job should check that? | |
| 15:18:25 | mriedem | there is a lower-constraints job | |
| 15:18:39 | mriedem | but i didn't see it run on https://review.openstack.org/#/c/566696/ | |
| 15:19:05 | smcginnis | I'm honestly not sure what the job checks now with different versions set between requirements and lower-constraints. | |
| 15:19:35 | smcginnis | It may just check that requirements isn't less than lower-constraints, but seems like it should also check the other way as well. | |
| 15:19:45 | smcginnis | Or versions should just be removed from requirements. | |
| 15:19:46 | mriedem | lower-constraints just runs unit tests with the lower constraints installed, right? whereas py27 runs with upper-constraints | |
| 15:19:55 | smcginnis | Yeah | |
| 15:20:00 | mriedem | we need more requirements files | |
| 15:20:04 | mriedem | that's all i know | |
| 15:20:06 | openstackgerrit | Curt Moore proposed openstack/nova master: Add ability to download Glance images into the libvirt image cache via RBD https://review.openstack.org/574301 | |
| 15:20:12 | smcginnis | Yeah, half a dozen more and we should be solid. | |
| 15:22:30 | mriedem | dansmith: nvm you have that in htere | |
| 15:22:35 | mriedem | the job didn't run on your patch though for some reason | |
| 15:22:48 | dansmith | it trusts me? | |
| 15:22:55 | mriedem | no, shit failed hard | |
| 15:23:05 | mriedem | openstack-tox-lower-constraints finger://ze02.openstack.org/d6e860451b074239a28aae41f39381c8 : POST_FAILURE in 9m 56s | |
| 15:23:38 | mriedem | the xenapi patch last ran jobs in may before we had the lower-constraints job so that's probably why it wasn't caught there | |
| 15:23:46 | dansmith | ah and doesn't get reported up top I guess? | |
| 15:23:57 | mriedem | i would have expected those POST_FAILURES to get reported up top | |
| 15:24:02 | mriedem | they used to with lots of red | |
| 15:24:09 | mriedem | maybe new gerrit / zuul doesn't show that now? | |
| 15:29:22 | dansmith | idk, seems confusing | |
| 15:29:32 | dansmith | because zuul -1s and reports few jobs, all passing | |
| 15:30:02 | dansmith | efried: left comments on your spec from skimming | |
| 15:31:24 | efried | dansmith: Thanks. Are you going to do a patch set adding words for things you said you were going to compose? | |
| 15:31:53 | dansmith | efried: I commented on that one place, which I think is enough, but I think you have things you need to tweak anyway right? | |
| 15:32:23 | efried | dansmith: I do. Okay, I'll add that in, if you think that's enough. | |
| 15:32:36 | efried | terse guy that you are. | |
| 15:32:45 | dansmith | efried: you covered what it'll do pretty well I think | |
| 15:32:55 | efried | okay, good | |
| 15:34:47 | mriedem | naichuans: is it possible to get a test run for the xenserver ci with image_handler=vdi_remote_stream to make sure this code passes tempest? | |
| 15:36:44 | efried | dansmith: If (found, done) == (!0, 0), it means there are non-migratable records. What am I supposed to do if I get that? | |
| 15:37:05 | dansmith | efried: who is "I" in that case? | |
| 15:37:18 | dansmith | efried: if "I" is the CLI user, I think it's handled in the docs for that command | |
| 15:37:23 | efried | dansmith: I don't know. What does the code do? Does it raise? | |
| 15:37:56 | efried | dansmith: I'm more asking for my edification, not suggesting the docs are inaccurate/inadequate. | |
| 15:38:33 | dansmith | efried: these functions migrate stuff, what they do and why they might be unable to migrate some records depends on what they're migrating | |
| 15:38:48 | dansmith | efried: if I'm migrating instances, I might refuse to touch instances with task_state!=None for example | |
| 15:39:15 | dansmith | efried: so I might say "there are 10 more instances to do, but I could only do 8" and then next I might say "there are 2 more to do but I could only do 0" | |
| 15:40:32 | mriedem | once done is 0 across the board, the CLI returns 0 and yo'ure done | |
| 15:40:58 | efried | dansmith: I guess I'm trying to understand whether there are cases where that means "the migration failed, push the red button and blow yourself up" or "go do XYZ manually" or "meh, don't worry about it, these things happen". | |
| 15:41:06 | mriedem | efried: if this helps https://review.openstack.org/#/c/574268/1/nova/cmd/manage.py@387 | |
| 15:41:57 | mriedem | return ran and 1 or 0 | |
| 15:41:59 | efried | mriedem: Right, that's the sentence that's prompting my question. "We've done all the ones we can do." So... what about the rest of them? | |
| 15:42:02 | mriedem | ran is based on done | |
| 15:42:12 | dansmith | mriedem: not really.. if done is zero it just means you're not able to do more work, but it doesn't mean everything is migrated | |
| 15:42:41 | dansmith | efried: it depends | |
| 15:43:00 | dansmith | efried: in my instance example, it means "try later once those instances are no longer transitioning state" | |
| 15:43:02 | mriedem | dansmith: then that's not clear from the command help https://docs.openstack.org/nova/latest/cli/nova-manage.html#nova-database | |
| 15:43:08 | mriedem | "Perform data migration to update all live data. Return exit code 0 if migrations were successful or exit code 1 for partial updates. This command should be called after upgrading database schema and nova services on all controller nodes. If the command exits with partial updates (exit code 1) the command will need to be called again." | |
| 15:43:35 | dansmith | mriedem: it's a minor loop vs. major loop type of thing | |
| 15:43:45 | mriedem | so, | |
| 15:43:47 | efried | ah, that indicates that (!0, 0) means "keep doing this until (0, 0)" | |
| 15:43:50 | dansmith | mriedem: nonzero means ansible should call again immediately.. zero means there's no more you can do right now | |
| 15:43:59 | dansmith | I can update those words too | |
| 15:44:02 | mriedem | it sounds like what you'd really need, if you were automating this, is rc=2 == total != 0, but done == 0 | |
| 15:44:30 | efried | no, it sounds like rc=1 means that already. | |
| 15:44:41 | mriedem | but rc=1 is only based on "done != 0" | |
| 15:44:54 | efried | oh, okay. So yeah. | |
| 15:44:58 | efried | what you said. | |
| 15:45:20 | dansmith | mriedem: we could, but anyone currently just checking for nonzero would get stuck, because it'd be changing what zero really means | |
| 15:45:23 | mriedem | otherwise if there is no other return code for the total != 0, done == 0 case, i can only know that by manually running things | |
| 15:45:41 | dansmith | early on in this, we didn't have migrations that couldn't complete fully, until we did, so.. | |
| 15:45:45 | mriedem | dansmith: sure, osa uses [0, 4] for things like that | |
| 15:46:00 | mriedem | i realize it would be backward incompatible, and upgrade release note, etc | |
| 15:46:14 | dansmith | mriedem: I don't know what that means | |
| 15:46:26 | mriedem | which part? | |
| 15:46:33 | dansmith | oh you mean [0,4] for either return value, sure, | |
| 15:46:36 | dansmith | but it has to change | |
| 15:46:40 | mriedem | right | |
| 15:46:56 | mriedem | if you're just saying != 0 in your tooling, that's kind of on you | |
| 15:47:06 | mriedem | anyway, i don't mean to open a can of worms | |
| 15:47:13 | dansmith | well, you did | |
| 15:47:16 | dansmith | worms. everywhere. | |
| 15:47:50 | dansmith | although efried is like the child that just starts throwing cans on the floor of the supermarket aisle | |
| 15:47:59 | dansmith | denting cans left and right until one opens | |
| 15:48:10 | efried | lol | |
| 15:48:20 | efried | like a child in my ignorance, at least. | |
| 15:48:51 | efried | My vote is that we merge this as is and (maybe) figure out what to do about that other thing later. | |
| 15:49:14 | mriedem | sure, throw a TODO in there about the rc=2 idea i guess | |
| 15:50:29 | openstackgerrit | Dan Smith proposed openstack/nova master: Document the internal online_migrations function behaviors https://review.openstack.org/574268 | |
| 15:50:34 | mriedem | efried: can you double check https://review.openstack.org/#/c/486475/36/doc/source/admin/configuration/hypervisor-xen-api.rst and if i'm right, i'll update the patch for my comments and respin so we can flush it | |
| 15:50:53 | efried | ... | |
| 15:51:04 | mriedem | flush from the runways spot i mean | |
| 15:53:46 | efried | mriedem: You're right. History there was that they were going to switch the default, but I said nay. They wound it back, but missed a couple spots. Good eye. | |
| 15:54:01 | mriedem | ok, i figured that was the case | |
| 15:54:05 | mriedem | want to default to the new shiny | |
| 15:55:11 | efried | mriedem: Was I right to disallow that? | |
| 15:56:00 | mriedem | shrug | |
| 15:56:05 | mriedem | we do change defaults from time to time | |
| 15:56:21 | mriedem | if you were explicitly using the old thing, it's no upgrade issue, otherwise it is if you don't want that new default | |
| 15:56:30 | mriedem | but new installs get the new thing out of the box | |
| 15:56:37 | efried | mm | |
| 15:56:45 | openstackgerrit | Zack Cornelius proposed openstack/nova master: Refactor libvirt get_memory_used_mb() https://review.openstack.org/571030 | |