| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2019-05-28 | |||
| 15:20:34 | openstackgerrit | Matthew Booth proposed openstack/nova master: Add functional test for bug 1550919 https://review.opendev.org/631294 | |
| 15:20:35 | openstackgerrit | Matthew Booth proposed openstack/nova master: libvirt: Don't delete disks on shared storage during evacuate https://review.opendev.org/578846 | |
| 15:20:35 | openstack | bug 1550919 in OpenStack Compute (nova) "[Libvirt]Evacuate fail may cause disk image be deleted" [Medium,In progress] https://launchpad.net/bugs/1550919 - Assigned to Matthew Booth (mbooth-9) | |
| 15:22:05 | aspiers | kashyap: hope that refactoring is OK ^^^ | |
| 15:27:36 | openstackgerrit | Matt Riedemann proposed openstack/nova master: WIP: Workaround missing RequestSpec.instance_group.uuid https://review.opendev.org/661786 | |
| 15:27:37 | mriedem | zigo: ^ needs tests and to be prettied up but it should workaround the issue given the request spec you showed me | |
| 15:29:00 | kashyap | aspiers: Hiya | |
| 15:31:20 | zigo | mriedem: Is this for nova-conductor ? | |
| 15:31:26 | zigo | In which node should I apply it? | |
| 15:31:52 | mriedem | api and conductor | |
| 15:32:08 | zigo | mriedem: Ok, then restart conductor and API, I guess ... | |
| 15:32:14 | mriedem | yup | |
| 15:34:18 | kashyap | aspiers: Nice; no test to prove that "nothing is broken" with the refactoring, I presume? | |
| 15:35:02 | aspiers | kashyap: the existing tests should cover it. Refactoring => no change in functionality to test :) | |
| 15:35:49 | kashyap | aspiers: Right | |
| 15:36:59 | kashyap | I like the change, it makes things clearer. (And I intend to refactor _get_guest_baseline_cpu_features() in turn later as part of the CPU modelling spec) | |
| 15:38:54 | aspiers | kashyap: the follow-up patch to add the SEV trait will rename _get_cpu_traits() to _get_cpu_feature_traits() and _get_cpu_traits() will wrap around it | |
| 15:40:42 | kashyap | aspiers: Yep, nod. | |
| 15:40:54 | aspiers | kashyap: coming up in a few secs | |
| 15:41:21 | gibi | mriedem: I've fixed your comments in https://review.opendev.org/#/c/652608/ | |
| 15:41:41 | zigo | mriedem: I still get a 500 error ... | |
| 15:43:48 | mriedem | zigo: do you have a paste of the logs? | |
| 15:43:55 | mriedem | note i added some debug logging | |
| 15:44:01 | zigo | mriedem: name "LOG" is undefined ... | |
| 15:44:09 | zigo | Problem with the logging ... :P | |
| 15:44:24 | openstackgerrit | Adam Spiers proposed openstack/nova master: Provide HW_CPU_AMD_SEV trait when SEV is supported https://review.opendev.org/638680 | |
| 15:44:30 | mriedem | ah the LOG didn't exist in rocky... | |
| 15:44:34 | mriedem | i'll remove the logging | |
| 15:44:57 | aspiers | kashyap: https://review.opendev.org/#/c/638680/15/nova/virt/libvirt/driver.py@9638 | |
| 15:45:16 | aspiers | kashyap: keeps the SEV bit nicely separated from the CPU feature code | |
| 15:46:00 | aspiers | efried: this should make you happy too :) | |
| 15:46:07 | openstackgerrit | Matt Riedemann proposed openstack/nova master: WIP: Workaround missing RequestSpec.instance_group.uuid https://review.opendev.org/661786 | |
| 15:46:08 | mriedem | zigo: ^ | |
| 15:46:13 | mriedem | gibi: huh i thought that was approved by now | |
| 15:46:52 | efried | aspiers: Sans click, I'm still gonna say "what about the HW_CPU_X86_AMD_SEV trait?" | |
| 15:47:00 | kashyap | aspiers: A naming nit: "_kernel_supports_amd_sev" --> "_kernel_support_for_amd_sev"? | |
| 15:47:16 | aspiers | efried: haha, thanks for the reminder | |
| 15:47:17 | gibi | mriedem: I would like to get it approved :) | |
| 15:47:25 | aspiers | kashyap: why? | |
| 15:47:31 | efried | aspiers: Maybe kashyap incorporated it in his patch, I haven't looked yet. | |
| 15:48:09 | kashyap | aspiers: Maybe not. It returns either true or false, so as it stands is okay :-) | |
| 15:48:14 | zigo | mriedem: Thanks a lot, unfortunately, I have to go, so I'll try tomorrow. | |
| 15:48:45 | kashyap | efried: Morning; I addressed your remarks and made them part of the older PS; and even added a note in your patch to adjust the Depends-On ID | |
| 15:49:09 | efried | kashyap: ack, saw it go by, haven't looked yet. Will take me a day or two to catch up from "holiday". | |
| 15:49:18 | efried | kashyap: and thank you :) | |
| 15:49:23 | kashyap | efried: Take your sweet time | |
| 15:49:55 | aspiers | efried: OK now here's a reoccurrence of that gerrit limitation we discussed before, but it's worse this time, because I'm not in control of all the reviews involved | |
| 15:50:07 | efried | aspiers: which one? | |
| 15:50:16 | efried | oh, can't branch upside down? | |
| 15:50:19 | aspiers | right | |
| 15:50:26 | mriedem | zigo: ack - also i have triaged how we get here in rocky | |
| 15:50:32 | mriedem | zigo: will have the details in the bug report | |
| 15:50:38 | mriedem | it | |
| 15:50:46 | mriedem | it's the reschedule and such that is the problem as i suspected | |
| 15:50:55 | mriedem | request spec bites me in the ass yet again | |
| 15:50:55 | aspiers | 638680 depends on some of my stuff (e.g. the patch_open helper and the _get_cpu_traits() refactor I just submitted), but now it also needs to depend on kashyap's traits reworking | |
| 15:51:25 | aspiers | efried: which are distinctly orthogonal strands of development | |
| 15:51:46 | efried | aspiers: traits reworking in os-traits or traits reworking in nova? | |
| 15:52:06 | aspiers | efried: ahhh good point! can use Depends-On here | |
| 15:52:15 | efried | aspiers: Well, theoretically yes | |
| 15:52:16 | aspiers | ... I think | |
| 15:52:23 | efried | except I figured out last week that it doesn't bloody work | |
| 15:52:27 | aspiers | :-/ | |
| 15:52:33 | aspiers | I'll give it a go anyway | |
| 15:52:55 | efried | You would have to hack up .zuul.yaml to make os-traits a required project for all the jobs. | |
| 15:53:01 | efried | which is a regal PITA | |
| 15:53:16 | aspiers | why? | |
| 15:53:26 | efried | but go ahead and depends-on it and we'll just get the damn thing merged and released and requirementsed | |
| 15:53:32 | efried | why what? | |
| 15:53:40 | aspiers | why would .zuul.yaml need to be hacked? | |
| 15:53:49 | efried | because otherwise zuul will ignore your depends-on. | |
| 15:53:56 | mriedem | depends-on won't pull in your os-traits change w/o changing required-projects | |
| 15:54:28 | mriedem | we shouldn't do that anyway imo | |
| 15:54:34 | efried | and we don't want to make that ^ permanent, because we generally want to test against released os-traits | |
| 15:54:36 | mriedem | b/c we could forget to release os-traits and bump the min required versions | |
| 15:54:39 | mriedem | correct | |
| 15:55:07 | efried | mriedem: My take is that we should be allowed to do it as long as the dep switches off as soon as it merges | |
| 15:55:45 | mriedem | that's putting a lot of trust into people remembering all of these knobs | |
| 15:55:46 | efried | that is, nova X depends on os-traits Y, so while Y is still open, X will build against it. As soon as Y merges, X goes back to testing against whatever the requirements-specified version is. | |
| 15:55:51 | aspiers | are the zuul folks aware of this issue? | |
| 15:55:58 | mriedem | aspiers: it's by design | |
| 15:56:01 | efried | aspiers: Yes, it's a feature, not a bug. | |
| 15:56:03 | efried | what mriedem said | |
| 15:56:06 | mriedem | it's like how LIBS_FROM_GIT worked in the old days | |
| 15:56:29 | aspiers | without thinking hard or having the experience of you two, it sounds like a deficiency in the design? | |
| 15:56:52 | efried | aspiers: I agree it would be nice if there was a (straightforward) way around it, for just this reason. | |
| 15:57:02 | aspiers | like mriedem says, requiring people to remember sounds like a UX bug to me | |
| 15:57:12 | efried | I want to make sure my nova patch will build against the os-traits I'm about to release, without having to release it first to try it. | |
| 15:57:18 | efried | I can test locally to some extent by hacking up my venv | |
| 15:57:26 | efried | but I can't e.g. test the dsvms locally. | |
| 15:57:52 | efried | or even run all of unit and/or functional locally without the fire department standing by. | |
| 15:58:00 | aspiers | is there a good reason *not* to honour Depends-On if the dependency is missing from required-projects? | |
| 15:58:05 | kashyap | Trying to open nova/tests/unit/virt/libvirt/test_driver.py in GitHub tells me: "(Sorry about that, but we can’t show files that are this big right now.)" | |
| 15:58:44 | kashyap | ["But we can render lots of crap that clutters your view"] | |
| 15:59:01 | efried | aspiers: Point is, Depends-On will normally build against the dep, but block your patch from merging until the dep merges. If we allowed your patch to merge once the dep merges, world breaks because nova is still building against released os-traits and your os-traits patch hasn't been released yet. | |
| 15:59:27 | mriedem | aspiers: this is not a new problem, this is how things have always worked for depending on library changes | |
| 15:59:49 | aspiers | mriedem: OK thanks | |
| 15:59:53 | mriedem | which is why i -1 changes that depend on the library change directly rather than the upper-constraints bump for whatever released version of the library contains that change | |
| 16:00:13 | mriedem | you can test the things in the gate with the required-projects hack | |
| 16:01:15 | openstackgerrit | Balazs Gibizer proposed openstack/nova master: cleanup evacuated instances not on hypervisor https://review.opendev.org/512623 | |