| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2020-03-25 | |||
| 21:33:48 | melwitt | this stuff confuses the hell out of me | |
| 21:35:31 | melwitt | so, nix what I said earlier rm_work ^ | |
| 21:40:39 | mriedem | "if you have multiple MQs there's no enforcement of affinity for racing requests until affinity support is added to placement" is not quite true, you just need the conductors configured to hit the API DB (compute -> cell conductor -> API DB); devstack doesn't configure the cell conductor with the API DB connection so again, devstack is doing the *ideal* separate setup but not what most (if any) nova deployments are probably d | |
| 21:40:39 | mriedem | , | |
| 21:41:09 | mriedem | pretty sure most nova deployments just have the api db connection configured everywhere | |
| 21:41:22 | openstackgerrit | Merged openstack/nova stable/stein: nova-live-migration: Ensure subnode is fenced during evacuation testing https://review.opendev.org/713962 | |
| 21:41:40 | mriedem | and yeah you have to have reschedules enabled to....reschedule :) | |
| 21:42:28 | mriedem | the only things that do the late affinity check in the computes are server create and evacuate, so you can still violate affinity policy for other moves (unshelve, cold and live migrate) | |
| 21:42:37 | melwitt | yeah ... I was realizing that slowly regarding the difference between database access vs MQ access | |
| 21:42:55 | mriedem | and the only flows that reschedule today from the compute are create and cold migrate/resize | |
| 21:43:10 | mriedem | i think evacuate just fails the operation if you fail the late affinity check | |
| 21:44:12 | melwitt | I can't remember why that "impossible to contact bc MQ" was ever a thing wrt to affinity | |
| 21:45:35 | melwitt | was it before alternate_hosts became a thing maybe? | |
| 21:46:35 | melwitt | sigh ... have to correct my comment yet again | |
| 21:54:22 | openstackgerrit | Ghanshyam Mann proposed openstack/nova master: Add test coverage of existing instance usage log policies https://review.opendev.org/715080 | |
| 21:55:05 | mriedem | alternate hosts just solved the problem of the cell conductor needing to go back to the scheduler and API DB | |
| 21:55:41 | mriedem | there is actually still an upcall bug there which i don't think got fixed, | |
| 21:56:11 | mriedem | during reschedule the conductor will update the instance.availability_zone for the alternate host and to do that it needs to hit the aggregates table which is in the API DB | |
| 21:57:04 | melwitt | yeah, I mean I had thought in the past it was said that the late affinity check would be impossible due to MQ isolation. but as you explained that's not true. so if that was ever said, I wondered why. it might have been before alternate hosts was added | |
| 21:57:29 | mriedem | err i guess i fixed that https://review.opendev.org/#/q/topic:bug/1781286+(status:open+OR+status:merged)+branch:master | |
| 21:58:46 | mriedem | anytime, it's time to social distance myself into the kitchen, o/ | |
| 22:01:36 | dansmith | melwitt: I think what you're thinking of is that we don't currently have a way for the child cell to know about the top-level mq, and I don't think we should | |
| 22:02:04 | dansmith | we do have a separate api_db connection string, so as a hack, child conductors can use that as if they were a top-level conductor to still hit that database, | |
| 22:02:18 | dansmith | which at least reduces the scope of who at the lower level can talk up, | |
| 22:02:41 | dansmith | and since we should be solving this in placement, we can just hang onto the status quo instead of further pollute the model by teaching everyone to call up | |
| 22:03:17 | dansmith | concerned people could give perms to the child conductors to only view the instance_groups and related tables I think, to further limit the scope of what it can see to just what is needed for tht check | |
| 22:04:33 | melwitt | dansmith: thanks ... I think I am thinking of that. but I could have sworn that there was some previously discussed impossibility about it regarding separate MQs, I might have been mixing back before we had alternate hosts, how once you're in the cell you can't call the scheduler again to request a reschedule | |
| 22:05:06 | dansmith | ...because we don't have a way to tell those services about the upper mq like we do for the upper db | |
| 22:05:53 | melwitt | right. yeah, I do understand that. maybe I was thinking back to before we had alternate hosts and became able to reschedule without sharing a MQ | |
| 22:06:29 | dansmith | well, yeah, the alternate hosts thing was the only way we could reschedule without adding a similar upcall | |
| 22:06:34 | melwitt | and had tied that to the late affinity check in my head. I dunno | |
| 22:06:36 | dansmith | for the same reason | |
| 22:06:47 | dansmith | well, it's the same thing of course | |
| 22:07:08 | dansmith | it was just easier to solve that with pre-populating some alternates to chew through, whereas the affinity check is not so easy | |
| 22:07:11 | melwitt | oh ... guh | |
| 22:08:22 | openstackgerrit | Ghanshyam Mann proposed openstack/nova master: Introduce scope_types in os-instance-usage-audit-log https://review.opendev.org/715082 | |
| 22:20:45 | melwitt | dansmith: I think what confused me was that prior to alternate hosts, you needed to be able to access the scheduler's MQ to reschedule (right?) ... so an upcall to another MQ. and I didn't process that the late affinity check does not involve needing to upcall to another MQ to work, it only needs to upcall to the API DB | |
| 22:21:47 | dansmith | melwitt: yes, (re)schedule is an rpc call, whereas the affinity check is just a db operation | |
| 22:22:18 | melwitt | right, ok | |
| 22:37:41 | openstackgerrit | Ghanshyam Mann proposed openstack/nova master: Add new default roles in os-instance-usage-audit-log policies https://review.opendev.org/715085 | |
| 22:48:19 | openstackgerrit | Ghanshyam Mann proposed openstack/nova master: Pass the actual target in os-instance-usage-audit-log policy https://review.opendev.org/715089 | |
| 22:55:34 | openstackgerrit | Ghanshyam Mann proposed openstack/nova master: Add new default roles in os-instance-usage-audit-log policies https://review.opendev.org/715085 | |
| 22:56:52 | openstackgerrit | Ghanshyam Mann proposed openstack/nova master: Pass the actual target in os-instance-usage-audit-log policy https://review.opendev.org/715089 | |
| 23:18:31 | openstackgerrit | melanie witt proposed openstack/nova master: Add info about affinity requests to the troubleshooting doc https://review.opendev.org/715092 | |
| 23:18:52 | openstackgerrit | Merged openstack/nova master: libvirt: Use oslo.utils >= 4.1.0 to fetch format-specific image data https://review.opendev.org/710785 | |
| 23:19:00 | openstackgerrit | Merged openstack/nova master: remove DISTINCT ON SQL instruction that does nothing on MySQL https://review.opendev.org/705850 | |
| 23:20:14 | openstackgerrit | melanie witt proposed openstack/nova master: Add info about affinity requests to the troubleshooting doc https://review.opendev.org/715092 | |
| 23:49:00 | brinzhang_ | gmann: I would like we not only catch 4xx error, maybe be we also need get 500, so I would like to keep the exception name to populate the details if it is a non-nova exception | |
| 23:51:26 | brinzhang_ | gmann: for example: as a non-admin, iuf I created server failed because of NoValidHost(500), if I get this message, that I can try again, Otherwise I cannot get nothing useful message | |
| 23:51:39 | brinzhang_ | s/iuf/if | |
| #openstack-nova - 2020-03-26 | |||
| 00:02:54 | openstackgerrit | melanie witt proposed openstack/nova master: Add info about affinity requests to the troubleshooting doc https://review.opendev.org/715092 | |
| 00:09:28 | brinzhang_ | dansmith: I dont know what you would want to say, you mean we should use update method instead of patch method? We were disscussed in irc meetting, at the beginning I wanted to use `PUT /servers/{server_id}/os-volume_attachments/{volume_id}` to implement this feature, but more people dont agree. | |
| 00:09:35 | brinzhang_ | dansmith: Firstly, we need admin_or_owner role to execute this, but the PUT swap volume API default role is administrative. | |
| 00:09:44 | brinzhang_ | dansmith: Secondly, we should make the `volumeId` to optional in the request body, that change is to big, and many limits and changes will be add this PUT swap volume API. | |
| 00:09:48 | brinzhang_ | dansmith: From talked in the irc meetting before freeze blueprint for Ussuri, we reached a letter of agreement and adopted the alternative scheme[1] of seen-k-mooney to achieve this function. | |
| 00:09:54 | brinzhang_ | [1] https://review.opendev.org/#/c/580336/32/specs/ussuri/approved/destroy-instance-with-datavolume.rst@51 | |
| 00:10:50 | brinzhang_ | dansmith: I know you didn't participate in the irc discussion at the time, but I don't understand why we can't use the PATCH API? | |
| 00:56:37 | alex_xu | sean-k-mooney: I guess we are missing a patch for evacuate | |
| 01:44:04 | brinzhang_ | alex_xu, sean-k-mooney: I think in the libvirt we are lost the accel_info after evacuate, https://review.opendev.org/#/c/631245/62/nova/virt/libvirt/driver.py@5781 | |
| 01:45:32 | brinzhang_ | alex_xu, sean-k-mooney: when we have an evecuate a server, maybe we need try to get the accelerators info for by instance.uuid, rirht? | |
| 03:15:29 | sean-k-mooney | alex_xu: we are not planning to have all ops supported initally so evac can be added later if we block it in the final patch | |
| 03:15:44 | sean-k-mooney | but it would be good to add it sooner rather then later | |
| 06:08:56 | openstackgerrit | Luyao Zhong proposed openstack/nova master: support live migration with vpmems https://review.opendev.org/687856 | |
| 06:08:57 | openstackgerrit | Luyao Zhong proposed openstack/nova master: Track orphan instances and error migrations in resource tracker https://review.opendev.org/714653 | |
| 08:01:28 | gibi | good morning nova | |
| 08:03:48 | brinzhang_ | gibi: good morning ^^ | |
| 08:05:18 | gibi | o/ | |
| 08:05:29 | brinzhang_ | gibi: pls add this PATCH API for implement bp/destroy-instance-with-datavolume to today's agenda, I replied dansmith's comments inline, and above in irc | |
| 08:06:17 | gibi | brinzhang_: OK. Will you be able to participate or I could you update me shortly what the disagreemen is about? | |
| 08:06:33 | gibi | s/I// | |
| 08:06:39 | brinzhang_ | gibi: he seems dont want I use PATCH API to do this, but I dont want to have a change. | |
| 08:07:27 | brinzhang_ | gibi: it's too later for mee, I am not able to participate | |
| 08:07:59 | gibi | brinzhang_: OK thanks. I will raise it to find a way forward | |
| 08:09:06 | brinzhang_ | I think this PATCH reched in the irc meeting, without good reason, I hope it will not stop it from moving forward. | |
| 08:10:09 | brinzhang_ | gibi: you can see the irc log above, at 8:09:27-8:10:50 | |
| 08:10:10 | gibi | I have to, and I will, read back on the disagreement | |
| 08:10:21 | brinzhang_ | gibi: thanks ^^ | |
| 08:13:29 | brinzhang_ | gibi: for bp/action-event-fault-details https://review.opendev.org/#/c/694430/, gmann agreed to add the new policy, but he want we limit it in 4xx error code, I think we also need expose 500 (novalid host exception) to the non-admin if we changed the default policy | |
| 08:13:54 | brinzhang_ | gibi: you can review https://review.opendev.org/#/c/694430/, thanks | |
| 08:19:01 | gibi | brinzhang_: I will | |
| 09:56:45 | gibi | gmann, brinzhang_: responeded in https://review.opendev.org/#/c/694430/ | |
| 10:14:32 | brinzhang_ | gibi: thanks, the new policy mainly facilitates us to expose the 'details' information to non-admin. We can change it by modifying the default policy check_str. This has been agreed with gmann. Last night at 15:00 UTC in openstac-nova channel discussion . | |
| 10:14:44 | openstackgerrit | Marcin Juszkiewicz proposed openstack/nova master: Add default cpu model for AArch64 https://review.opendev.org/709494 | |
| 10:15:23 | hrw | kashyap: took your comments and applied | |
| 10:15:42 | kashyap | hrw: Hiya; will look | |
| 10:15:46 | kashyap | Thanks! | |
| 10:16:10 | gibi | brinzhang_: yeah I'd like to have the new policy to control the exposure of the 'details' field | |
| 10:16:26 | brinzhang_ | if we re-using the BASE_POLICY_NAME% 'events' policy, we will difficult to distinguish whether 'traceback' or 'details' expose to non-admin | |
| 10:16:35 | brinzhang_ | gibi: yeah, thanks | |
| 10:17:11 | brinzhang_ | gibi: now gmann want we shuold just catch 4xx error to populate the 'details' | |
| 10:18:09 | brinzhang_ | gibi: But I think we also need 500 error, that the non-admin can have a try as soon as possible | |
| 10:18:18 | luyao | stephenfin: Hi, I have addressed alex_xu's comments. :) https://review.opendev.org/#/c/687856 | |
| 10:18:47 | gibi | yeah, I want to expose information, including NoValidHost, but I don't want to collect every possible error to whitelist them. | |
| 10:18:59 | brinzhang_ | as you said in commnets, information leakage is inevitable. Since the administrator chooses to modify the default policy, he will accept the change. | |
| 10:19:09 | gibi | brinzhang_: exactly | |
| 10:19:50 | brinzhang_ | gibi: yeah, we are same, wait for gmann check, he has -1 on the patch | |
| 10:20:02 | hrw | kashyap: turns out that we need that patch to get kolla-ansible CI running ;( | |
| 10:22:53 | nightmare_unreal | hey how can i create fake cells and create instance in them for functional test. I am looking at nova/tests/functional/test_nova_manage.py | |
| 10:24:28 | luyao | lyarwood: about vpmem cleanup during live migration, we can check migration_context but not adding a new flag to migrate_data obj, you can look at https://review.opendev.org/#/c/687856/20/nova/compute/manager.py@8197 | |
| 10:24:58 | kashyap | hrw: "That patch" is the above default CPU model thing? | |