| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2019-06-10 | |||
| 14:21:49 | stephenfin | Also got https://review.opendev.org/#/c/660950/ working, though that's lower priority (turns out it's 'foo(x=None, *args, **kwargs)' and not 'foo(*args, x=None, **kwargs)', but flake8 doesn't catch that) | |
| 14:21:52 | stephenfin | Heh, jinx | |
| 14:29:05 | efried | stephenfin: bottom one is +A. | |
| 14:29:17 | efried | imo the super one shouldn't be adding *args or **kwargs anywhere they weren't already. | |
| 14:36:16 | openstackgerrit | Matt Riedemann proposed openstack/nova master: Make get_provider_by_name public and remove safe_connect https://review.opendev.org/664062 | |
| 14:38:21 | openstackgerrit | Stephen Finucane proposed openstack/nova master: Ensure controllers all call super https://review.opendev.org/660950 | |
| 14:38:30 | stephenfin | efried: Cool. Fixed the super one by removing all the *args and **kwargs (none of them were necessary, fwict) ^ | |
| 14:53:18 | efried | stephenfin: Cool man, +2. | |
| 14:53:27 | stephenfin | mriedem, sean-k-mooney: What am I missing here? I see only one callers to the revert_resize function and that's from the API https://review.opendev.org/#/c/662522/3/nova/compute/api.py@3420 | |
| 14:53:52 | stephenfin | We wouldn't call "openstack resize --revert" on a failed migration, would we? | |
| 14:54:38 | sean-k-mooney | stephenfin: im pretty sure we use teh revert_resize api endpoint to revert a cold-migration | |
| 14:55:05 | mriedem | stephenfin: cold migrate == resize except same flavor | |
| 14:55:09 | mriedem | it's the same code flow | |
| 14:55:13 | stephenfin | I realize that | |
| 14:55:17 | stephenfin | 100% | |
| 14:55:24 | mriedem | your comment says, "This is the 'revert_resize' function. There's no way we could get here for anything unless we were doing a revert of the resize operation (i.e. no cold migration flow through here), right?" | |
| 14:55:26 | stephenfin | but would a user revert a cold migration | |
| 14:55:32 | mriedem | yes | |
| 14:55:33 | mriedem | sure | |
| 14:55:42 | sean-k-mooney | stephenfin: yes they can | |
| 14:55:53 | mriedem | just because it's called "resize" doesn't mean it's not also used by the cold migrate flow | |
| 14:55:57 | mriedem | same as rebuild is used for evacuate | |
| 14:56:19 | stephenfin | Seriously? Why can't I find any documentation on the internet about this? | |
| 14:56:34 | sean-k-mooney | because its an internal detail of nova implemeation | |
| 14:56:48 | sean-k-mooney | it should be in a dev ref | |
| 14:56:55 | stephenfin | The ability of a user to revert their failed cold migration is a internal detail? | |
| 14:57:07 | stephenfin | How it's implemented, sure, but not the raw ability to do the thing | |
| 14:57:09 | sean-k-mooney | its in the api ref | |
| 14:57:10 | mriedem | for one, users don't cold migrate, admins do | |
| 14:57:22 | mriedem | https://developer.openstack.org/api-ref/compute/?expanded=migrate-server-migrate-action-detail#migrate-server-migrate-action | |
| 14:57:40 | mriedem | "after a successful cold migration and then must be confirmed or reverted." | |
| 14:58:37 | stephenfin | the more you know | |
| 14:58:39 | sean-k-mooney | i think revert does not require admin https://developer.openstack.org/api-ref/compute/?expanded=migrate-server-migrate-action-detail,revert-resized-server-revertresize-action-detail#migrate-server-migrate-action | |
| 14:58:52 | sean-k-mooney | except as indicated in teh precondions | |
| 14:58:53 | mriedem | "Policy defaults enable only users with the administrative role to perform this operation. " | |
| 14:59:01 | mriedem | cold migrate is admin_api by default | |
| 14:59:15 | stephenfin | Hmm, why don't we have an 'openstack server migrate --revert' thing for this? | |
| 14:59:24 | mriedem | we do | |
| 14:59:44 | mriedem | https://docs.openstack.org/python-openstackclient/latest/cli/command-objects/server.html#server-resize | |
| 14:59:46 | mriedem | resize --revert | |
| 15:00:01 | stephenfin | that's 'resize --revert', not 'migrate --revert' | |
| 15:00:15 | mriedem | they hit the same api | |
| 15:00:27 | stephenfin | Then why have an 'openstack migrate' command | |
| 15:00:38 | mriedem | for cold and live migration | |
| 15:00:48 | mriedem | which are not resizes | |
| 15:01:34 | stephenfin | live migration, sure, but not cold migration | |
| 15:01:44 | stephenfin | that's the same thing but with the same image? | |
| 15:01:46 | stephenfin | *flavor | |
| 15:02:06 | sean-k-mooney | stephenfin: we could delete migrate and jsut use resize for the cold migration case but most user dont think a resize and migration are the same thing | |
| 15:02:07 | mriedem | cold migrate == resize but with no flavor change | |
| 15:02:58 | stephenfin | sean-k-mooney: Yeah, that's my point. Why would a user think "oh, my migration failed. Best revert this resize" | |
| 15:03:00 | sean-k-mooney | migrate is just syntactic sugar over resize really. | |
| 15:03:06 | stephenfin | yeah | |
| 15:03:14 | mriedem | note that a revert isn't to recover a failed move | |
| 15:03:25 | mriedem | you can only revert if the resize/cold migration *didn't* fali | |
| 15:03:26 | mriedem | *fail | |
| 15:03:53 | stephenfin | Actually, yeah, I've done this in tests now that I think about | |
| 15:03:54 | stephenfin | *it | |
| 15:04:21 | stephenfin | (Or rather avoided doing it because confirming a resize/migration requires a lot of mocking out of libvirt stuff in functional tests that's not worth the effort) | |
| 15:05:03 | stephenfin | mriedem: I think a '--revert' flag for the 'openstack server migrate' command would be helpful syntactic sugar. Any reason not to? | |
| 15:05:04 | mriedem | if you're concerned about docs, you could update the description on https://docs.openstack.org/python-openstackclient/latest/cli/command-objects/server.html#server-migrate to talk about how after a successful cold migration you need to revert or confirm the migration and which commad does that | |
| 15:05:17 | stephenfin | I can do that too, sure | |
| 15:05:39 | mriedem | idk, ask dean. one reason could be someone thinking they could revert a live migration since that's built into the same command. | |
| 15:05:42 | Nick_A | sorry if this is not the right place - we have a kvm compute with qcow2, but snapshots are being stored as raw. is there a setting we're overlooking to take snapshots as qcow2? | |
| 15:05:51 | mriedem | but that's likely solved with a good description of the parameter and validation in the command. | |
| 15:05:54 | stephenfin | I'm annoyed that I work on this stuff and _I_ wasn't able to find information about this easily. That seems...silly :) | |
| 15:06:05 | stephenfin | Yeah, we already have that issue for other flags so this is just one more | |
| 15:06:17 | mriedem | Nick_A: probably this https://docs.openstack.org/nova/latest/configuration/config.html#DEFAULT.force_raw_images | |
| 15:07:15 | Nick_A | thank you, looking | |
| 15:07:37 | mriedem | Nick_A: I think glance also has an option to convert everything to raw | |
| 15:07:53 | stephenfin | Okay, I'll start on that. Thanks for the clarification, mriedem, sean-k-mooney | |
| 15:08:13 | sean-k-mooney | stephenfin: there are some docs here too https://github.com/openstack/nova/blob/master/doc/source/admin/migration.rst | |
| 15:09:19 | mriedem | Nick_A: I think this https://docs.openstack.org/glance/latest/configuration/glance_api.html#taskflow_executor.conversion_format | |
| 15:09:26 | sean-k-mooney | stephenfin: we show we use "openstack server resize --confrim" to confrim the migration but not how to revert | |
| 15:09:51 | Nick_A | thank you | |
| 15:10:25 | stephenfin | sean-k-mooney: Sweet, I'll fix that too and add some some 'migrate --confirm' syntactic sugar for good measure | |
| 15:15:24 | openstackgerrit | Matt Riedemann proposed openstack/nova stable/stein: Avoid unnecessary joins in InstanceGroup.get_hosts https://review.opendev.org/664271 | |
| 15:40:22 | dansmith | mriedem: are you going to update the pre-filter disabled computes spec? | |
| 15:40:34 | dansmith | I assume gibi is on vacay or something and hasn't circled back, | |
| 15:40:49 | dansmith | but I figure he's probably fine with it as long as we have a plan for that (which we do) | |
| 15:41:11 | mriedem | dansmith: i was waiting for gibi | |
| 15:42:12 | dansmith | okay | |
| 15:43:29 | mriedem | what was the plan again? :) on disable, check if down and if down, return 200 (like if it was an old compute) and let upt sync the trait on restart of the service | |
| 15:44:00 | mriedem | if that's the plan i can write that edge case into the spec | |
| 15:44:01 | dansmith | yeah, always set compute.disabled=True, not make the rpc call if it's down, return 200 | |
| 15:44:29 | mriedem | ok i can wordsmith that after tearing up artom's patch here | |
| 15:44:43 | dansmith | by all means, prioritize tearing up anything of artom's | |
| 15:44:46 | artom | ;_: | |
| 15:45:07 | artom | (That's from both emotional and physical pain) | |
| 15:48:24 | aspiers | stephenfin: ok to add you to reviewers for a couple of SEV patches which need more core attention? they're not particularly big or complex | |
| 15:48:31 | stephenfin | sure | |
| 15:48:35 | aspiers | thanks! | |
| 16:00:48 | mriedem | artom: sean-k-mooney: mdbooth: dansmith: comments in https://review.opendev.org/#/c/644881/ | |
| 16:01:18 | artom | Whoa, you weren't kidding about tearing up | |
| 16:01:37 | mdbooth | Whoa! | |
| 16:01:43 | mdbooth | mriedem: Thanks, btw | |
| 16:02:33 | artom | sean-k-mooney, correct me if I'm wrong, but I don't believe "The bigger issue I have is the concern that we could have a server with both ovs hybrid plugged ports and ports without that" is possible | |
| 16:03:16 | sean-k-mooney | it is only posible if you have ovs +sriov ports | |
| 16:03:43 | sean-k-mooney | e.g. if they are all ovs ports then it would all be hybrid_plug true or false | |
| 16:03:59 | artom | sean-k-mooney, ah, so ports on which the firewall driver that causes hybrid plug to be used doens't apply | |