| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2022-11-30 | |||
| 17:02:33 | gmann | neutron call external event API with service user and then nova will return 404 | |
| 17:02:38 | gmann | 404 for get server | |
| 17:02:53 | dansmith | gmann: do you mean because nova internally does a get_server? | |
| 17:03:04 | gmann | dansmith: sean-k-mooney either we need to access Db for server with hard coded admin inside the API | |
| 17:03:17 | dansmith | are we still checking that at the db layer? | |
| 17:03:17 | gmann | dansmith: yes, get server of the requested project if not admin | |
| 17:03:35 | sean-k-mooney | dansmith: ya i think we are | |
| 17:03:42 | gmann | I think it match the project_id from context unless it is admin | |
| 17:04:11 | dansmith | okay, so external_event should get the server from the db with admin context, but do the usual policy check of "can you see this" on the result? | |
| 17:04:37 | sean-k-mooney | the external events api si doign the server get | |
| 17:04:40 | sean-k-mooney | to get the host | |
| 17:04:45 | sean-k-mooney | so it know where to send the event | |
| 17:04:51 | dansmith | well, also just to make sure it's for a legit server right? | |
| 17:04:57 | gmann | dansmith: if we do with admin then no 'get server policy' come into pic | |
| 17:05:08 | sean-k-mooney | yes also to ensure it exsits | |
| 17:05:28 | sean-k-mooney | so we coudl make that db check supprot the service user too | |
| 17:05:32 | gmann | if we are ok to use admin context inside those then it should work | |
| 17:05:39 | dansmith | we need to make sure we don't leak the existence of servers via the external_event API because someone can call it and get a 403 vs 404, which I presume is why we look up the server with user creds now | |
| 17:05:50 | sean-k-mooney | eventully we proably want to remvoe the db check but as a minimal interim step i think that would be ok | |
| 17:06:08 | gmann | dansmith: yeah that is issue of 403 vs 404 then | |
| 17:06:26 | gmann | and yes leak of server existence | |
| 17:06:27 | dansmith | yeah, just need to be careful about that | |
| 17:06:28 | sean-k-mooney | well the api is admin only now | |
| 17:06:43 | gmann | now is ok, if we make it service only | |
| 17:06:50 | dansmith | yeah, if they get stopped before the server get because they lack service role, then that's fine | |
| 17:06:52 | gmann | I can pass server uuid to know if that exist or not | |
| 17:07:15 | sean-k-mooney | it would only be an issue if you had the service role | |
| 17:07:21 | dansmith | yeah | |
| 17:07:21 | sean-k-mooney | which no human shoudl ever have | |
| 17:07:32 | gmann | dansmith: yes, that policy check will be there for service role before they try getting sevrer | |
| 17:07:39 | dansmith | for this case.. that might not be the case for all of ours, if we have any that are legit for humans and machines | |
| 17:07:43 | dansmith | gmann: cool | |
| 17:07:46 | gmann | sean-k-mooney: yes, only with service role | |
| 17:08:19 | sean-k-mooney | dansmith: woudl you object ot adding the service role to the place where we check for admin in the db | |
| 17:08:30 | dansmith | sean-k-mooney: I think that's a bad idea | |
| 17:08:36 | sean-k-mooney | even if its just an interim step to reventually removing that in the db layer | |
| 17:08:50 | sean-k-mooney | ok so you would prefer we internally escalate to admin context | |
| 17:08:53 | dansmith | sean-k-mooney: not only because it would affect lots of other non-service role things, but also because it expands that check which we probably want to minimize | |
| 17:09:08 | sean-k-mooney | ya that fiar | |
| 17:09:10 | dansmith | I'd prefer we explicitly "elevate" to admin for service role things at the point of access | |
| 17:09:12 | gmann | yeah | |
| 17:09:26 | gmann | ok, let me go with approach 1. policy check for service role at start 2. fetch the things (server etc) with admin context | |
| 17:09:41 | dansmith | ++ | |
| 17:09:48 | sean-k-mooney | yep that works for me | |
| 17:10:01 | sean-k-mooney | the same shoudl apply to swap volume | |
| 17:10:10 | sean-k-mooney | its the same check that is failign right | |
| 17:10:14 | gmann | ok, thanks dansmith sean-k-mooney | |
| 17:10:16 | sean-k-mooney | the get_server | |
| 17:10:36 | gmann | dansmith: sean-k-mooney btw if you have time, placement policy updates are ready to review too https://review.opendev.org/c/openstack/placement/+/865618 | |
| 17:10:39 | gmann | gibi: ^^ | |
| 17:10:45 | gmann | sean-k-mooney: yes | |
| 17:14:59 | sean-k-mooney | gmann: realistically it will be next week before i have time to take a look but i set RP+1 and ill try to come back to itthen | |
| 17:15:33 | gmann | sean-k-mooney: sure, thanks | |
| 17:19:24 | opendevreview | melanie witt proposed openstack/nova stable/yoga: Adapt websocketproxy tests for SimpleHTTPServer fix https://review.opendev.org/c/openstack/nova/+/866192 | |
| 17:20:36 | opendevreview | melanie witt proposed openstack/nova stable/xena: Adapt websocketproxy tests for SimpleHTTPServer fix https://review.opendev.org/c/openstack/nova/+/866193 | |
| 17:21:41 | opendevreview | melanie witt proposed openstack/nova stable/wallaby: Adapt websocketproxy tests for SimpleHTTPServer fix https://review.opendev.org/c/openstack/nova/+/866194 | |
| 17:29:43 | opendevreview | melanie witt proposed openstack/nova stable/victoria: Adapt websocketproxy tests for SimpleHTTPServer fix https://review.opendev.org/c/openstack/nova/+/866195 | |
| 17:31:16 | opendevreview | melanie witt proposed openstack/nova stable/ussuri: Adapt websocketproxy tests for SimpleHTTPServer fix https://review.opendev.org/c/openstack/nova/+/866196 | |
| 18:05:07 | opendevreview | melanie witt proposed openstack/nova stable/train: Adapt websocketproxy tests for SimpleHTTPServer fix https://review.opendev.org/c/openstack/nova/+/866201 | |
| 18:07:24 | opendevreview | Merged openstack/nova-specs master: fixing: allowing target state for evacuate https://review.opendev.org/c/openstack/nova-specs/+/866108 | |
| 19:05:48 | opendevreview | Merged openstack/nova stable/yoga: refactor: remove duplicated logic https://review.opendev.org/c/openstack/nova/+/855022 | |
| 21:07:27 | opendevreview | Ghanshyam proposed openstack/nova master: Enable new defaults and scope checks by default https://review.opendev.org/c/openstack/nova/+/866218 | |
| 21:48:10 | atmark | need help modifying https://github.com/openstack/nova/blob/master/nova/virt/libvirt/driver.py#L4150-L4172 . I'd like to add to check VM property if it contains say a string 'NoRestart' in addition to ignored_states | |
| 21:54:37 | opendevreview | Ghanshyam proposed openstack/nova master: Enable new defaults and scope checks by default https://review.opendev.org/c/openstack/nova/+/866218 | |
| 22:33:37 | opendevreview | melanie witt proposed openstack/nova stable/train: Adapt websocketproxy tests for SimpleHTTPServer fix https://review.opendev.org/c/openstack/nova/+/866201 | |
| #openstack-nova - 2022-12-01 | |||
| 00:02:28 | opendevreview | melanie witt proposed openstack/nova stable/train: Adapt websocketproxy tests for SimpleHTTPServer fix https://review.opendev.org/c/openstack/nova/+/866201 | |
| 06:12:15 | opendevreview | Wenping Song proposed openstack/nova master: Get only resolved arqs instead of filter all arqs https://review.opendev.org/c/openstack/nova/+/866291 | |
| 08:44:47 | opendevreview | Manuel Bentele proposed openstack/nova master: libvirt: Add configuration options to set SPICE compression settings https://review.opendev.org/c/openstack/nova/+/828675 | |
| 08:45:53 | opendevreview | Merged openstack/nova master: Adds regression functional test for 1980720 https://review.opendev.org/c/openstack/nova/+/861357 | |
| 10:25:15 | opendevreview | Merged openstack/nova stable/train: add regression test case for bug 1978983 https://review.opendev.org/c/openstack/nova/+/864168 | |
| 10:54:05 | jsanemet | hello | |
| 10:54:23 | jsanemet | could i get a review for this spec? | |
| 10:54:25 | jsanemet | https://review.opendev.org/c/openstack/nova-specs/+/865432 | |
| 10:54:41 | jsanemet | thanks | |
| 11:37:42 | opendevreview | Merged openstack/nova stable/train: For evacuation, ignore if task_state is not None https://review.opendev.org/c/openstack/nova/+/864169 | |
| 11:49:23 | opendevreview | Rajat Dhasmana proposed openstack/nova stable/wallaby: [stable-only] Use os-brick from source in wallaby https://review.opendev.org/c/openstack/nova/+/866326 | |
| 13:55:49 | opendevreview | Manuel Bentele proposed openstack/nova master: libvirt: Add configuration options to set SPICE compression settings https://review.opendev.org/c/openstack/nova/+/828675 | |
| 14:08:27 | tobias-urdin | do we support live migration of instances with pinned pcpus? perhaps sean-k-mooney can shime in | |
| 14:11:19 | tobias-urdin | imo should be supported since train, but always better to ask i guess | |
| 14:11:51 | sean-k-mooney | tobias-urdin: yes but it depend on the release | |
| 14:12:05 | bauzas | we indeed do | |
| 14:12:24 | sean-k-mooney | https://specs.openstack.org/openstack/nova-specs/specs/train/implemented/numa-aware-live-migration.html | |
| 14:12:45 | sean-k-mooney | the numa aware live migration feature added the ablity to recaluate and update the xml for the destination host | |
| 14:13:19 | sean-k-mooney | tobias-urdin: before that you could only live migrate if the same cpus were free on the dest and that was kind of a hack | |
| 14:13:26 | sean-k-mooney | so before train really only cold migration | |
| 14:13:34 | sean-k-mooney | after train live migration should work properly | |
| 14:14:11 | sean-k-mooney | tobias-urdin: the same is true for hugepages. it was done in the same feautre | |
| 14:14:20 | sean-k-mooney | tobias-urdin: there is still one unfixed bug | |
| 14:14:52 | sean-k-mooney | live migrating between hosts with different vcpu_pin_sets or different cpu_shared_sets | |
| 14:15:04 | sean-k-mooney | for non numa instnace is still technically broken | |
| 14:15:15 | opendevreview | Merged openstack/nova stable/yoga: Record SRIOV PF MAC in the binding profile https://review.opendev.org/c/openstack/nova/+/855023 | |
| 14:15:21 | opendevreview | Merged openstack/nova stable/yoga: Remove double mocking https://review.opendev.org/c/openstack/nova/+/855024 | |
| 14:15:24 | tobias-urdin | ack, so if nodes are identical in terms of config (pin sets) it shouldn't be a problem | |
| 14:15:26 | opendevreview | Merged openstack/nova stable/yoga: Remove double mocking... again https://review.opendev.org/c/openstack/nova/+/855025 | |
| 14:15:32 | opendevreview | Merged openstack/nova stable/yoga: Add compute restart capability for libvirt func tests https://review.opendev.org/c/openstack/nova/+/855026 | |
| 14:15:32 | sean-k-mooney | we do not update the cpus for floating instnaces until you hard reboot the vm | |
| 14:15:42 | opendevreview | Merged openstack/nova stable/victoria: [compute] always set instance.host in post_livemigration https://review.opendev.org/c/openstack/nova/+/863903 | |
| 14:16:20 | sean-k-mooney | tobias-urdin: for pinend vms the pin sets can be differnt and it shoudl not be a problem | |
| 14:16:56 | sean-k-mooney | in trian+ | |
| 14:18:19 | tobias-urdin | ack, not sure I understand the bug tho | |