| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2020-12-16 | |||
| 13:33:00 | openstackgerrit | Balazs Gibizer proposed openstack/nova master: Refactor update_pci_request_spec_with_allocated_interface_name https://review.opendev.org/c/openstack/nova/+/757614 | |
| 13:34:26 | openstackgerrit | Balazs Gibizer proposed openstack/nova master: Support interface attach with qos ports https://review.opendev.org/c/openstack/nova/+/756530 | |
| 13:37:42 | openstackgerrit | Adit Sarfaty proposed openstack/nova master: Retry on vmware create_vm when it fails https://review.opendev.org/c/openstack/nova/+/764586 | |
| 14:43:47 | openstackgerrit | Elod Illes proposed openstack/nova stable/queens: [stable-only] Cap bandit to 1.6.2 https://review.opendev.org/c/openstack/nova/+/767364 | |
| 15:19:33 | openstackgerrit | sean mooney proposed openstack/nova master: only wait for plugtime events in pre-live-migration https://review.opendev.org/c/openstack/nova/+/767368 | |
| 15:22:16 | openstackgerrit | Balazs Gibizer proposed openstack/nova master: Allow extending an existing instance allocation https://review.opendev.org/c/openstack/nova/+/757109 | |
| 15:22:17 | openstackgerrit | Balazs Gibizer proposed openstack/nova master: Make remove allocation symmetric with add allocation https://review.opendev.org/c/openstack/nova/+/757110 | |
| 15:23:46 | openstackgerrit | Balazs Gibizer proposed openstack/nova master: Factor out port resource deallocation https://review.opendev.org/c/openstack/nova/+/757111 | |
| 15:23:46 | openstackgerrit | Balazs Gibizer proposed openstack/nova master: Refactor update_pci_request_spec_with_allocated_interface_name https://review.opendev.org/c/openstack/nova/+/757614 | |
| 15:26:35 | openstackgerrit | Balazs Gibizer proposed openstack/nova master: Support interface attach with qos ports https://review.opendev.org/c/openstack/nova/+/756530 | |
| 15:29:23 | openstackgerrit | Balazs Gibizer proposed openstack/nova master: Enable mypy on scheduler/report.py https://review.opendev.org/c/openstack/nova/+/765152 | |
| 15:35:15 | gibi | stephenfin: fixed your comments in the qos series | |
| 15:35:55 | gibi | also I finished the main patch https://review.opendev.org/c/openstack/nova/+/756530 | |
| 15:37:21 | openstackgerrit | Balazs Gibizer proposed openstack/nova master: Remove unused Instance method https://review.opendev.org/c/openstack/nova/+/757615 | |
| 16:12:04 | openstackgerrit | Balazs Gibizer proposed openstack/nova master: Reproduce bug 1907522 in functional test https://review.opendev.org/c/openstack/nova/+/766791 | |
| 16:12:04 | openstack | bug 1907522 in OpenStack Compute (nova) "test_*_with_qos_min_bw_allocation fails in the nova-multi-cell job with: nova.exception.MigrationPreCheckError: Migration pre-check error: Failed to create port bindings for host |
|
| 16:13:26 | openstackgerrit | Balazs Gibizer proposed openstack/nova master: Fallback to same-cell resize with qos ports https://review.opendev.org/c/openstack/nova/+/766925 | |
| 16:15:35 | gibi | stephenfin: and fixued up this as well ^^ | |
| 16:17:03 | gmann | gibi: for specific budy I mean, rest of the action APIs like lock does not take any parameter in request body so they are allowed everything since legacy time. But in unshelve API, 2.77 version added AZ to be a valid param in request body so in this action API we said if user can pass AZ as valid param then let's allow null or AZ only and block everything else. | |
| 16:17:40 | gibi | gmann: lock takes a lock_reason optionally similarly how unshelve takes AZ optionally | |
| 16:18:23 | gibi | https://github.com/openstack/nova/blob/master/nova/api/openstack/compute/schemas/lock_server.py | |
| 16:21:16 | gmann | gibi: ah yeah, we have not added 'locked_reason' as required. | |
| 16:21:33 | gibi | we not added AZ as required either | |
| 16:22:02 | gmann | gibi: so we can pass anything in cok API like lock: {'unknown': 'unknown'} | |
| 16:22:10 | gibi | no you cannot | |
| 16:22:16 | gibi | as additionalProperties false | |
| 16:22:21 | gmann | https://github.com/openstack/nova/blob/master/nova/api/openstack/compute/schemas/shelve.py#L31 | |
| 16:22:24 | gmann | here it is ^^ | |
| 16:23:26 | gmann | that required is specially to block the empty dict so that we can have more strict shcema. | |
| 16:23:36 | gmann | let me check comments again if any other reason | |
| 16:23:44 | gibi | gmann: but why we need more sticter schema? | |
| 16:24:13 | gibi | the meaning of unshelve: {} and unshelve: null is the same from the impl perspective (except the schema) | |
| 16:24:42 | gmann | gibi: schema main idea is to have a very strict checks so that user will only be able to pass the usable parameters and everything else will be 400 | |
| 16:25:09 | gibi | sure, I'm OK not to allow 'unknown':'unknown' | |
| 16:26:28 | gibi | what I see that our API acceptr "<action>":{} and "<action>": null in mot of the cases but not for unshelve | |
| 16:29:46 | openstackgerrit | Balazs Gibizer proposed openstack/nova master: stabilize unit test asserting log https://review.opendev.org/c/openstack/nova/+/764796 | |
| 16:31:13 | openstackgerrit | Balazs Gibizer proposed openstack/nova master: stabilize set_host libvirt driver unit test https://review.opendev.org/c/openstack/nova/+/764797 | |
| 16:38:58 | gmann | gibi: this was the original discussion on disallow the {} and make it more strict like pass None or AZ. - https://review.opendev.org/c/openstack/nova/+/663851/34/nova/api/openstack/compute/schemas/servers.py#695 | |
| 16:39:08 | gmann | gibi: that time we did not update api-ref for that | |
| 16:39:51 | gmann | I personally did not see passing or allowing {} is more easy or ok than None | |
| 16:42:16 | gibi | but now has two different type of server action API, the unshelve where {} is disallowed and other actions where {} is allowed | |
| 16:42:26 | gibi | for me the meaning of null and {} are the same | |
| 16:43:43 | gibi | so unshelve now seems special, and I guess this was why bauzas was surprised | |
| 16:44:15 | bauzas | gibi: also the fact that we changed the behaviour between 2.1 and 2.77 | |
| 16:44:25 | bauzas | even if that's a microversion | |
| 16:44:48 | gibi | so in pre 2.77 {} is OK but later it is disallowed? | |
| 16:45:10 | gibi | that also feels a bit unnecesserly hars for the users | |
| 16:45:11 | gmann | yeah there is no change for pre-277 | |
| 16:45:53 | gibi | the meaning of {} pre 2.77 is the same as null. And we could keep this meaning after 2.77 but we chose to change it | |
| 16:45:57 | gmann | for all other action except lock afrer 2.52 and unshelve after277, everything is allowed. | |
| 16:46:42 | gmann | bauzas: pre 2.77 there is no change, you can pass {} or any unknown param | |
| 16:48:15 | gmann | bauzas: gibi all action APIs does not use schema so they accept anything in request body and we improved that in lock and unshelve action as they started accepting request body. where two different pattern were selected for both of these APIs. in latest which is 2.77 unshelve we started disallow {} too | |
| 16:48:44 | bauzas | gmann: gibi: sorry, on a meeting | |
| 16:48:57 | bauzas | but yeah, {} was accepted before 2.77 and now not | |
| 16:48:59 | gmann | I cannot say which one is more correct but I think disallowing {} make is more specific | |
| 16:50:38 | gibi | from api user perspective who is upgrading from pre 2.77 to 2.77 the new schema is overly restrictive for no good reason (from the api user perspective) | |
| 16:51:07 | gibi | if we have technical reasons then fine, but I don't see any usability reasons to disallow {} | |
| 16:53:12 | openstackgerrit | Balazs Gibizer proposed openstack/nova master: Optionally prevent nova-compute start with DB credentials https://review.opendev.org/c/openstack/nova/+/762176 | |
| 16:53:37 | gibi | can some core other than stephen look at this simple revert https://review.opendev.org/c/openstack/nova/+/755995 it re-enables a good chunk of test coverage | |
| 16:53:46 | bauzas | gibi: I'll do | |
| 16:53:50 | gibi | thanks | |
| 16:53:51 | bauzas | but then I need to leave | |
| 16:54:37 | bauzas | done | |
| 16:55:12 | gibi | bauzas: thanks. | |
| 16:55:16 | gibi | I need to leave soon too | |
| 16:55:24 | bauzas | (taxi dad) | |
| 16:55:28 | gibi | o/ | |
| 16:57:40 | openstackgerrit | Ghanshyam proposed openstack/placement master: Remove deprecated [placement]/policy_file config option https://review.opendev.org/c/openstack/placement/+/764409 | |
| 16:57:57 | gmann | gibi: ^^ updated this | |
| 16:58:02 | gibi | gmann: looking | |
| 17:02:25 | gibi | gmann: replied about the new init parameter | |
| 17:04:19 | gmann | checking | |
| 17:06:17 | gmann | gibi: yeah, we can remove that as it was just for testing things and pick of policy file is on oslo side now | |
| 17:06:20 | gmann | updating | |
| 17:06:28 | gibi | thanks | |
| 17:09:53 | openstackgerrit | Ghanshyam proposed openstack/placement master: Remove deprecated [placement]/policy_file config option https://review.opendev.org/c/openstack/placement/+/764409 | |
| 17:10:20 | openstackgerrit | Balazs Gibizer proposed openstack/nova master: Use the non polling notification waiter in func test https://review.opendev.org/c/openstack/nova/+/758445 | |
| 17:10:21 | openstackgerrit | Balazs Gibizer proposed openstack/nova master: Create a fixture around fake_notifier https://review.opendev.org/c/openstack/nova/+/758446 | |
| 17:10:54 | gmann | gibi: done ^^ | |
| 17:11:24 | openstackgerrit | Ghanshyam proposed openstack/placement master: [goal] Deprecate the JSON formatted policy file https://review.opendev.org/c/openstack/placement/+/764394 | |
| 17:12:17 | gibi | looking | |
| 17:13:16 | openstackgerrit | Balazs Gibizer proposed openstack/nova master: Use NotificationFixture for legacy notifications too https://review.opendev.org/c/openstack/nova/+/758448 | |
| 17:13:42 | gibi | gmann: I'm +2 on both policy patch now, thanks for proposing them | |
| 17:14:04 | gmann | gibi: thanks. | |
| 17:14:39 | gmann | I am going to write the unit tests also for placement policy like we did in nova so that new policy changes can be validated correctly. but sometime next week | |
| 17:14:41 | openstackgerrit | Balazs Gibizer proposed openstack/nova master: Test the NotificationFixture https://review.opendev.org/c/openstack/nova/+/758450 | |
| 17:16:09 | openstackgerrit | Balazs Gibizer proposed openstack/nova master: Move fake_notifier impl under NotificationFixture https://review.opendev.org/c/openstack/nova/+/758451 | |
| 17:46:38 | sean-k-mooney1 | gibi: gmann so im about to drop for the day but where did we settelon on allowing {} in unshleve | |
| 17:47:32 | sean-k-mooney1 | to me blocking it seams wrong and a bug that shoudl be corrected. { "unkonwn" :"unknown"} != {} the former should be blocked but i think the latter shoudl be treaded the same as null | |
| 17:49:36 | gibi | gmann: ack, thanks | |
| 17:49:48 | gibi | sean-k-mooney1: I don't think we had a final agreement | |
| 17:49:58 | gibi | we just stopped arguing:) | |
| 17:50:22 | sean-k-mooney1 | ok i might be back latter after dinner. but that is my perspective on the topic in anycase | |
| 18:12:32 | gibi | sean-k-mooney1: ack, I share your view | |
| 18:16:26 | melwitt | surely there's precedent for this? I thought a lot of the server actions apis are this way | |
| 18:21:33 | melwitt | if other apis treat null == {} then it seems ok. if they don't, I would make it consistent with the other apis | |
| 18:38:00 | openstackgerrit | Merged openstack/nova master: Revert "Temporarily disable parts of heal port allocation test" https://review.opendev.org/c/openstack/nova/+/755995 | |
| 18:45:38 | gmann | sean-k-mooney2: ack, as gibi mentioned, no final agreement yet we can discuss further | |
| 18:47:21 | gmann | melwitt: all other action API except lock-2.56 and unshelve-2.77 except everything in request, {} or {unknown: unknown} because there is no schema for those as per legacy compatibility. | |
| 18:48:15 | gmann | lock-2.56 and unshelve-2.77 started accepted more param in request body and new schema for those follow different pattern. lock does not block {} but unshelve block {} | |