Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2022-08-26
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?
14:34:42 gibi if the rebuild bfv become ready before the user_data update
14:35:02 gibi dansmith: I'm actively helping 2.39 and melwitt too
14:35:18 dansmith looks like it's getting attention.. yeah okay cool
14:36:04 gibi I suggest to make rebuild bfv ready independently from the fact which microversion will it get. and it is ready before the user_data feature then we can switch the order in couple of hours

Earlier   Later