| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2022-08-26 | |||
| 12:34:29 | sean-k-mooney | if they have a dedicated storage network | |
| 12:34:30 | gibi | yeah, although I might avoid that defaulting to avoid FQDN issues. and just document that if you need manila you need to define an IP there | |
| 12:34:57 | gibi | push the issue up in the stack to the deployment engine to provide a proper IP ther | |
| 12:35:00 | gibi | e | |
| 12:35:01 | sean-k-mooney | ok i was suggesting default ing becuase i think that is what we do for the live migration one but lets check | |
| 12:35:37 | sean-k-mooney | we have https://docs.openstack.org/nova/latest/configuration/config.html#DEFAULT.my_ip | |
| 12:35:49 | sean-k-mooney | and https://docs.openstack.org/nova/latest/configuration/config.html#DEFAULT.my_block_storage_ip | |
| 12:35:58 | sean-k-mooney | so i guess it should use the blockstorage one by default? | |
| 12:37:03 | sean-k-mooney | hum looks like that is only used in hyperv | |
| 12:37:05 | gibi | hm, that is already an IP, so I'm OK with that | |
| 12:37:27 | sean-k-mooney | so lets reuse that for maniall | |
| 12:37:40 | sean-k-mooney | and just document that its now used by libvirt for this usecase too? | |
| 12:38:04 | sean-k-mooney | it also already has a default | |
| 12:38:06 | sean-k-mooney | https://github.com/openstack/nova/blob/0d84833e9688e0df97f3d24e06025e512bca3ce3/nova/conf/netconf.py#L24-L51 | |
| 12:38:12 | sean-k-mooney | =netutils.get_my_ipv4( | |
| 12:38:54 | gibi | I don't like the defaulting, but I won't block on that | |
| 12:39:17 | sean-k-mooney | well i guess the intent is ot have it work out of the box | |
| 12:39:52 | sean-k-mooney | https://docs.openstack.org/nova/latest/configuration/config.html#libvirt.live_migration_inbound_addr was what i was thinking of orginally | |
| 12:40:03 | sean-k-mooney | that is not default because we use the hostname if not set | |
| 12:42:51 | gibi | yeah I know we have these defaulting already, hence I'm accepting it, I just don't like that we guess what would be good instead of asking the deployer to provide what is good | |
| 12:43:02 | gibi | but that is just a philosophy I can supress :) | |
| 12:43:32 | sean-k-mooney | i generally like things to work out of the box so i dont need to look at ooo to figure out how to set things | |
| 12:44:16 | sean-k-mooney | but ya i understand why you would like it to be explict | |
| 12:44:42 | gibi | OK, I left comment in the API patch and linked to this IRC log for Uggla to look at | |
| 12:44:45 | gibi | sean-k-mooney: thanks | |
| 12:46:12 | Uggla | sorry was looking at something else. I have just quicky read the chat. I think I get the idea. | |
| 12:48:30 | sean-k-mooney | tl;dr to the grant in the compute and read CONF.my_block_storage_ip | |
| 12:48:30 | gibi | cool | |
| 12:48:54 | sean-k-mooney | and in the futre maybe we can use the cert grant instead but not sure how that works | |
| 12:49:23 | sean-k-mooney | the cert grant would limit the connection to that speicic vm and would not need to be updated if we move the vm | |
| 12:49:56 | sean-k-mooney | so more secure and less work in the long rune but we woudl need to figure out how to create the certs and set it up | |
| 12:50:03 | sean-k-mooney | which is not for this cycle | |
| 13:22:50 | Uggla | sean-k-mooney, agree. | |
| 13:35:56 | gibi | I'm done with the bfv rebuild, I had some additional questions top of what you sean-k-mooney had | |
| 13:51:09 | sean-k-mooney | oh ya i forgot we have not drop the 5.x proxy code yet | |
| 13:53:15 | sean-k-mooney | also good point on the functional test disbaing much of the code | |
| 13:53:57 | gibi | honestly I did not check the unit testts | |
| 13:54:20 | sean-k-mooney | while this techincaly coudl be libvirt indepentite it proably woudl be simmpler to use use the libvrit functional tests to test this more compeltely | |
| 13:54:48 | gibi | I think the key there would be to assert cinder interaction | |
| 13:55:02 | gibi | I don't care which driver fixture we use for it | |
| 13:55:16 | sean-k-mooney | right but we should not mock with mock.patch.object(self.compute.manager, | |
| 13:55:18 | sean-k-mooney | '_rebuild_volume_backed_instance'), \ | |
| 13:55:26 | sean-k-mooney | in a functional test | |
| 13:55:51 | sean-k-mooney | we shoudl acutlly create a ocmpute service create an instnace and rebuild it as you said | |
| 13:57:06 | gibi | yes | |
| 13:57:14 | gibi | and assert the cinder fixture state after the rebuild | |
| 13:57:34 | gibi | probably need a some extra state in the cinder fixture to store what image is on th volume | |
| 14:07:18 | dansmith | gibi: sean-k-mooney: I think the tempest test for the happy path is better than a functional, especially given the way it's verified, | |
| 14:07:38 | dansmith | the functional for testing the failure cases is better though obviously | |
| 14:08:37 | opendevreview | Merged openstack/nova master: virt: Add block_device_info helper to find encrypted disks https://review.opendev.org/c/openstack/nova/+/826529 | |
| 14:11:06 | sean-k-mooney | well its more that the functional test is closer to a unit test as written | |
| 14:11:06 | sean-k-mooney | i agree that the tempest test is going to be more valueable | |
| 14:12:04 | dansmith | sean-k-mooney: you know the tempest test is written and working right? (just in case you didn't see it) | |
| 14:12:16 | gibi | I agree too. I left some error case questing inline that would benefit from a functional test | |
| 14:12:28 | gibi | *questiong | |
| 14:12:29 | gibi | *question | |
| 14:12:33 | gibi | (it is Friday) | |
| 14:12:34 | sean-k-mooney | dansmith: yes i have seen it | |
| 14:13:00 | gibi | like I'm not sure what will happen with the instance if the reimage fails | |
| 14:13:07 | sean-k-mooney | the -1 i left on the last patch was more for the lack of docs by the way | |
| 14:13:16 | sean-k-mooney | specifica the api ref | |
| 14:13:44 | sean-k-mooney | we need to update that note that calls out how rebuidl for bfv is differnt | |
| 14:13:55 | dansmith | I just hate that we do this to people.. they're fairly responsive for two cycles and then a week before the deadline, we finally review and play them the sad trombone | |
| 14:13:59 | dansmith | I dunno how to fix that | |
| 14:14:22 | sean-k-mooney | dansmith: actuly i didnt think they were responsive | |
| 14:14:31 | dansmith | I spent a lot of time with him on it, but had several interruptions this cycle | |
| 14:14:51 | sean-k-mooney | its one of the reason i was not really looking at the patch beccause the comment i left on the tempset one had not been adress i did not end up looking at the nova ones | |
| 14:14:55 | dansmith | sean-k-mooney: really? for also being the PTL of cinder I think he did pretty well and did everything I asked of him for a pretty complicated feature | |
| 14:15:34 | sean-k-mooney | baisicaly i check back on the tempest one a few times and didnt see much change and never got around to looking at the nova ones | |
| 14:15:45 | gibi | sorry for not reviewing it earlier but I hope it is better to review it late than never | |
| 14:16:36 | sean-k-mooney | dansmith: the patches are in merge conflict one way or another so they need to actuly be update | |
| 14:16:55 | dansmith | gibi: yes of course and definitely appreciated, it just sucks and I wish we (all) could do better.. more deadlines instead of fewer, I always say | |
| 14:16:57 | sean-k-mooney | i tought most of the issue were pretty minor that i pointed too and would not take that long | |
| 14:17:09 | gibi | dansmith: I agree on the more deadlines | |
| 14:17:13 | sean-k-mooney | althoghg is it intentional that they did not implement ironic support | |
| 14:17:28 | dansmith | sean-k-mooney: yeah, we knew it was going to need updates, and I'm not complaining about the comments (at all) | |
| 14:18:04 | dansmith | sean-k-mooney: I think that's probably unintentional and I didn't even catch it | |
| 14:18:12 | gibi | dansmith: I think I could do better if there would be clear and agreed priority which feature to review first. As I think i filled my time with plenty of reviews in general | |
| 14:18:25 | sean-k-mooney | ack we can just call that out as a limiation in the docs and fix it next cycle | |
| 14:18:44 | dansmith | gibi: oh I know, I'm certainly not complaining about the amount of reviews being done :) | |
| 14:18:52 | sean-k-mooney | its just because ironic does not use the default implementation of the rebuild fucntion | |
| 14:18:55 | sean-k-mooney | it has its own | |
| 14:19:15 | dansmith | sean-k-mooney: yeah I know, but I also didn't know we had bfv with ironic :) | |
| 14:21:50 | JayF | Good morning folks o/. Just wanted to bump my three outstanding stable ironic driver patches for review. https://review.opendev.org/c/openstack/nova/+/853546 https://review.opendev.org/c/openstack/nova/+/821351 https://review.opendev.org/c/openstack/nova/+/854257 all three are clean backports, and all but one already have one +2 | |
| 14:23:53 | gibi | dansmith: I slept on your comment about split the PCI feautre at the point where all the compute related part is ready. I figured that the current patch order does not really matches with that. I currently I have the other i) inventory healing, ii) allocation healing, ii) scheduling. But the scheduling part of the feauture needs compute side changes: 1) to driver the PCI claim based on the placment | |
| 14:23:59 | gibi | allocation 2) the pci and numa fitting logic is shared between the scheduler and the compute | |
| 14:24:49 | dansmith | gibi: I wasn't really suggesting a reorder, I was more just commenting on what seams are flexible for backports and which aren't :) | |
| 14:24:55 | gibi | so even if we could merge only the inventory and allocation healing without the scheduling support, backporting the scheduling support later is not really feasible | |
| 14:25:18 | gibi | or at least pretty shaky business | |
| 14:25:43 | gibi | fortunately I don't have to think about feature backport in this chat window :D | |
| 14:26:33 | opendevreview | Merged openstack/nova master: blockinfo: Add encryption details to the disk_info mappings when provided https://review.opendev.org/c/openstack/nova/+/772272 | |
| 14:26:42 | opendevreview | Merged openstack/nova master: imagebackend: Add disk_info_mapping as an optional attribute of Image https://review.opendev.org/c/openstack/nova/+/826530 | |
| 14:26:50 | opendevreview | Merged openstack/nova master: libvirt: Consolidate create_cow_image and create_image https://review.opendev.org/c/openstack/nova/+/846246 | |
| 14:26:59 | dansmith | gibi: :) | |
| 14:27:00 | opendevreview | Merged openstack/nova stable/wallaby: add regression test case for bug 1978983 https://review.opendev.org/c/openstack/nova/+/853811 | |
| 14:28:05 | dansmith | gibi: sean-k-mooney: isn't there a planned train for service/microversions somewhere? I wonder if I could at least rebase this on the right thing to get it lined up for whoami-rajat | |
| 14:33:10 | gibi | dansmith: https://etherpad.opendev.org/p/nova-zed-microversions-plan | |
| 14:33:49 | dansmith | yeah, thanks | |
| 14:34:18 | gibi | I'm not against to reorder the next to microversion | |
| 14:34:37 | dansmith | is that 2.93 one likely to get the review and attention it needs? | |