| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2018-04-11 | |||
| 09:48:56 | openstackgerrit | Yikun Jiang (Kero) proposed openstack/nova master: Add microversion to support extra_specs in flavor API. https://review.openstack.org/554159 | |
| 09:49:35 | mdbooth | kashyap: You're mocking driver._conn btw | |
| 09:49:44 | mdbooth | But the test uses driver._host | |
| 09:49:47 | mdbooth | s/test/code/ | |
| 09:51:42 | mdbooth | kashyap: Unfortunately your problem is vastly more interesting than what I'm actually supposed to be doing. | |
| 09:51:57 | kashyap | :-) | |
| 09:52:56 | openstackgerrit | Zhenyu Zheng proposed openstack/nova master: Deduplicate notification samples Rocky - 2 https://review.openstack.org/560299 | |
| 09:53:57 | mdbooth | kashyap: Don't assume that the previous unit test was correct. It may have worked only by coincidence. This isn't uncommon. | |
| 09:54:13 | kashyap | This doesn't fly either: | |
| 09:54:14 | kashyap | + with mock.patch.object(driver.init_host, 'getVersion') \ | |
| 09:54:14 | kashyap | + as mock_getver: | |
| 09:54:31 | kashyap | mdbooth: Sigh, yeah, I recall johnthetubaguy saying our MIN_VERSIONS testing has been a "joke" | |
| 09:54:32 | mdbooth | driver.init_host is a function | |
| 09:54:42 | mdbooth | That doesn't really make sense | |
| 09:55:36 | mdbooth | If you look at Host.has_min_version | |
| 09:56:09 | mdbooth | See that _version_check gets version from conn.getLibVersion | |
| 09:56:18 | kashyap | Aah, I remove that: | |
| 09:56:21 | mdbooth | Where conn is returned by Host.get_connection() | |
| 09:58:25 | mdbooth | You need the return value of Host.get_connection() to return an object whose getLibVersion() method returns your test value | |
| 09:58:38 | mdbooth | You can do that with Mock | |
| 09:59:07 | mdbooth | I wonder if you can also do it with FakeVirtAPI | |
| 09:59:11 | kashyap | (Hmm, yeah: host.has_min_version --> _version_check --> getLibVersion) | |
| 09:59:39 | kashyap | mdbooth: Yeah, if we can do it with the FakeVirtAPI, shouldn't I go that route, instead? | |
| 10:00:36 | mdbooth | It looks like you can just mock fakelibvirt.FAKE_LIBVIRT_VERSION | |
| 10:00:41 | mdbooth | And it will do what you want | |
| 10:00:51 | kashyap | mdbooth: NOte that we'd like to check: MIN_VIRTUOZZO_VERSION is OK. | |
| 10:00:56 | kashyap | But thanks for the walk through | |
| 10:01:24 | kashyap | Hey, this is also "work", but yeah, "someone else's work", though :P | |
| 10:01:26 | mdbooth | ^^^ is how to change the value it's testing against | |
| 10:02:01 | kashyap | mdbooth: Noted; will play a bit more and see where I arrive | |
| 10:08:57 | openstackgerrit | Chen Hanxiao proposed openstack/nova master: trivial: fix a comment typo https://review.openstack.org/560349 | |
| 10:11:28 | openstackgerrit | Stephen Finucane proposed openstack/os-vif master: Add lower-constraints job https://review.openstack.org/556056 | |
| 10:32:14 | kashyap | mdbooth: Thanks for the idea! It seemed to have worked. | |
| 10:44:47 | openstackgerrit | Naichuan Sun proposed openstack/nova master: xenapi: Support live migration in pooled multi-nodes environment https://review.openstack.org/489451 | |
| 10:50:02 | openstackgerrit | Kashyap Chamarthy proposed openstack/nova master: libvirt: Bump MIN_{LIBVIRT,QEMU}_VERSION for "Rocky" https://review.openstack.org/558783 | |
| 10:50:03 | openstackgerrit | Kashyap Chamarthy proposed openstack/nova master: libvirt: Drop MIN_{LIBVIRT_KVM,QEMU}_S390_VERSION https://review.openstack.org/560363 | |
| 10:50:04 | openstackgerrit | Kashyap Chamarthy proposed openstack/nova master: libvirt: Drop MIN_LIBVIRT_KVM_PPC64_VERSION https://review.openstack.org/560364 | |
| 10:50:05 | openstackgerrit | Kashyap Chamarthy proposed openstack/nova master: libvirt: Drop MIN_LIBVIRT_VIRTUOZZO_VERSION https://review.openstack.org/560365 | |
| 11:10:13 | openstackgerrit | Merged openstack/nova master: make metadata doc up to date https://review.openstack.org/559613 | |
| 11:10:25 | openstackgerrit | Merged openstack/nova master: Log a more useful error when neutron isn't configured https://review.openstack.org/559111 | |
| 11:10:42 | openstackgerrit | Merged openstack/nova master: doc: add a link in the install guides about configuring neutron https://review.openstack.org/559115 | |
| 11:11:00 | openstackgerrit | Merged openstack/nova master: Log a more useful error when cinder auth isn't configured https://review.openstack.org/559399 | |
| 11:11:11 | openstackgerrit | Merged openstack/nova master: Move test_report_client out of placement namespace https://review.openstack.org/558911 | |
| 11:11:24 | openstackgerrit | Merged openstack/nova master: Update the Cell filters section of the scheduler docs https://review.openstack.org/559108 | |
| 12:31:07 | efried | mikal: Are you satisfied with the response/rework at https://review.openstack.org/#/c/527658/31/nova/virt/zvm/configdrive.py@44 ? | |
| 12:31:49 | efried | It looks to me like the (guest) format is indeed iso9660, which I think is what you wanted to make sure of. | |
| 12:50:29 | jichen | efried: thanks for raising this question to mikal, yes, as mentioned in the patch, we use tgz as is09660 and consume it later by mounting it to VM as iso9660 format before cloud-init take action | |
| 13:05:24 | openstackgerrit | Merged openstack/nova master: Cleanup tempest-dsvm-cells-rc blacklist https://review.openstack.org/553401 | |
| 13:15:13 | openstackgerrit | Merged openstack/nova master: Cleanup _get_request_spec_for_select_destinations for live migrate https://review.openstack.org/559381 | |
| 13:24:34 | openstackgerrit | Zhenyu Zheng proposed openstack/nova master: nova-manage db archive_deleted_rows is not multi-cell aware https://review.openstack.org/507486 | |
| 13:36:10 | openstackgerrit | jichenjc proposed openstack/nova master: uncap eventlet in nova https://review.openstack.org/560420 | |
| 13:41:15 | bhagyashris | jaypipes: Hi, | |
| 13:41:47 | mriedem | sahid: are you also working on the metadata api changes for https://review.openstack.org/#/q/topic:bp/sriov-trusted-vfs+(status:open+OR+status:merged) ? i don't want to merge too much of that series before the metadata api patch is up | |
| 13:42:31 | jichen | mriedem: not sure https://review.openstack.org/560420 is the right way to do ... but my previous zvm patches failed due to requirement.txt . or we need wait for the automatic process of requirements update? | |
| 13:44:03 | mriedem | jichen: i didn't think there was an automated reqs update anymore | |
| 13:44:09 | mriedem | i haven't tried to follow that thread | |
| 13:44:25 | sahid | mriedem: i'm a bit under pressure with the specs i have to work on and my downstream work... I need to build an on a SRIOV to test what you have indicated on the review then I will work on the metadata thing | |
| 13:44:30 | sahid | but yes it's my plan | |
| 13:44:59 | mriedem | sahid: ok thanks | |
| 13:45:05 | jichen | mriedem: ok , I don't have too much background on that stuff, anyway, I followed their patch and updated nova accordingly | |
| 13:45:21 | jichen | hopefully anyone can take a look | |
| 13:46:04 | mriedem | jichen: https://review.openstack.org/#/c/559367/ is merged | |
| 13:46:08 | mriedem | so it seems we don't need this in nova? | |
| 13:47:31 | jichen | mriedem , no , I guess that's the reason caused my issue http://logs.openstack.org/87/523387/32/check/requirements-check/408e28c/job-output.txt.gz, Doug replied in ML and say that's a correct error ... | |
| 13:48:10 | jichen | so I assume we need some changes but not too much experience here | |
| 13:48:11 | mriedem | ok that's a different change https://review.openstack.org/#/c/523387/ | |
| 13:48:33 | mriedem | ok i can try to sort this out, i know it's late for you | |
| 13:48:46 | jichen | ok, thank you for your help | |
| 13:50:11 | openstackgerrit | Merged openstack/nova master: libvirt: handle DiskNotFound during update_available_resource https://review.openstack.org/553067 | |
| 13:50:26 | openstackgerrit | Merged openstack/nova master: Update ImageMetaProp object to expose traits https://review.openstack.org/557795 | |
| 13:51:00 | jaypipes | bhagyashris: did you need something? | |
| 13:52:26 | efried | mriedem: I noticed your reference to https://wiki.openstack.org/wiki/NovaAPIRef#Parameter_Verification in a patch. I have to say I don't agree that the ordering (required=>optional=>by-microversion) should be strictly followed where it doesn't make sense. For example, here https://review.openstack.org/#/c/517757/7/placement-api-ref/source/allocation_candidates.inc it makes more logical sense to order them this way. If I | |
| 13:52:26 | efried | would be after limits, the granulars would be after member_of... IMO it's better if it's in the most sensible logical order for users of the latest microversion. | |
| 13:53:51 | bhagyashris | jaypipes: Jus want to inform few thing regarding the shared resource provider that we have discussed earlier, I have done some testing and analysis. | |
| 13:54:10 | bhagyashris | jaypipes: s/Jus/just | |
| 13:54:59 | mriedem | efried: you'll have to debate that with sdague | |
| 13:55:17 | mriedem | i assume microversions come last in order because we are trying to write api ref docs for the lowest common denominator | |
| 13:55:32 | efried | mriedem: Maybe placement is different. | |
| 13:55:39 | mriedem | placement is always different | |
| 13:56:06 | efried | mriedem: I'm not necessarily suggesting action (like an edit of the guidelines). Just something I noticed and wanted to get off my chest. | |
| 13:56:14 | efried | Thanks for listening :) | |
| 13:57:48 | bhagyashris | jaypipes: I have tested the scenario and here are the observation http://paste.openstack.org/show/718936/ | |
| 13:58:58 | efried | bhagyashris: Woot! That looks just like it should! | |
| 13:59:50 | jaypipes | efried: indeed, after commenting out the "compute node sets DISK_GB inventory item automatically" code. | |
| 13:59:50 | bhagyashris | efried, jaypipes: Based on the current analysis, it appears that compute node shouldn't report it's own resources in inventory if it's configured to use it from shared resource providers but currently, compute node doesn't have any mechanism to report selective resources in inventory. | |
| 14:00:20 | jaypipes | bhagyashris: correct. the original idea was to use the aggregates information in the ProviderTree to determine whether to report "local disk" or not. | |
| 14:00:41 | openstackgerrit | Steve Noyes proposed openstack/nova master: Enables MySQL Cluster Support for Nova https://review.openstack.org/446643 | |
| 14:00:42 | jaypipes | bhagyashris: efried can share some information with you about the aggregate tracking in the ProviderTree object | |
| 14:01:00 | efried | jaypipes: You're saying that if we find a sharing provider for a given RC, we should *remove* that RC from the compute node's inventory?? | |
| 14:01:19 | jaypipes | efried: I'm saying that was my original idea, yeah. | |
| 14:01:32 | bhagyashris | efried, jaypipes: There is a TODO note here : https://github.com/openstack/nova/blob/master/nova/scheduler/filter_scheduler.py#L212-L216 that the first allocation request object is use to claim resources | |
| 14:01:36 | jaypipes | (that's why I originally added the aggregate tracking in the reportclient to begin with) | |
| 14:02:27 | efried | bhagyashris: jaypipes: Do we have any evidence that the virt driver actually *used* the shared storage to make the disk? | |
| 14:02:31 | jaypipes | bhagyashris: yes, but that's more about the use case where we have both shared and local disk and can't decide which to consume from. what efried and I are discussing is how to signal to the compute node not to report local disk if it sees a sharing provider of disk. | |
| 14:02:34 | efried | or has that piece not been hooked up yet? | |
| 14:02:39 | bhagyashris | efried, jaypipes: yeah that is the best idea and that will not need to make any change at sch side | |
| 14:02:53 | jaypipes | efried: well, that's an entirely different question :) | |
| 14:03:01 | efried | But an important one, you'll agree. | |
| 14:03:08 | jaypipes | efried: and I see no reason why it wouldn't. | |
| 14:03:20 | bhagyashris | jaypipes: yeah totally agree with you point | |