Earlier  
Posted Nick Remark
#openstack-nova - 2022-08-26
12:21:20 opendevreview ribaudr proposed openstack/nova master: Add helper methods to attach/detach shares https://review.opendev.org/c/openstack/nova/+/852085
12:21:20 opendevreview ribaudr proposed openstack/nova master: Add instance.power_off_error notification https://review.opendev.org/c/openstack/nova/+/852278
12:21:22 opendevreview ribaudr proposed openstack/nova master: Add virt/libvirt error test cases https://review.opendev.org/c/openstack/nova/+/852087
12:21:22 opendevreview ribaudr proposed openstack/nova master: Add libvirt test to ensure metadata are working. https://review.opendev.org/c/openstack/nova/+/852086
12:21:24 opendevreview ribaudr proposed openstack/nova master: Change microversion to 2.93 https://review.opendev.org/c/openstack/nova/+/852088
12:21:31 gibi sean-k-mooney: encrypted
12:21:57 gibi sean-k-mooney: I can check the bfv after I finish with the manila api patach
12:22:00 sean-k-mooney i kicked the first 4 that are remaining into the queue
12:22:02 sean-k-mooney https://review.opendev.org/c/openstack/nova/+/826752
12:22:19 sean-k-mooney is where melwitt is adding the encyption options which she has set -w on
12:22:33 sean-k-mooney ill review some of the later patches but that will need ot be updated
12:26:08 gibi sean-k-mooney: how do you feel about this https://review.opendev.org/c/openstack/nova/+/836830/10/nova/api/openstack/compute/server_shares.py#54 ?
12:26:27 gibi sean-k-mooney: nova-api tries to resolve the compute hostname to ip address
12:26:53 sean-k-mooney that sound incorrect
12:27:01 gibi yeah it feels strange
12:27:43 sean-k-mooney for one there might be multipel interfaces and the compute host might connect to the starge backend via a differnt interface the we use for our inter service managment trafffic
12:28:09 sean-k-mooney the api also may not be able to resolve the compute ip in an iolsated networkign config
12:28:31 gibi yes, also this socket call only works for ipv4
12:28:49 sean-k-mooney yep so we shoudl not do this in the api and im not sure if we should do it in the comptue
12:29:06 gibi unfortunately manila only takes IP as per https://docs.openstack.org/api-ref/shared-file-system/?expanded=grant-access-detail#grant-access
12:29:07 sean-k-mooney what is this bieing used for
12:29:18 gibi it is to allow mounting the share to the compute
12:29:19 sean-k-mooney well we dont have to use that grant type
12:29:30 sean-k-mooney i was hoping to move to the cert one
12:29:50 gibi ahh I see
12:30:23 sean-k-mooney i think generating a tls cert for each share attachmetn woudl be the better approch longterm
12:31:58 opendevreview Amit Uniyal proposed openstack/nova master: [compute] always set instnace.host in post_livemigration https://review.opendev.org/c/openstack/nova/+/791135
12:32:01 sean-k-mooney so for now i guess if we are using ip we shoudl do this in the compute
12:32:03 sean-k-mooney not the api
12:32:17 sean-k-mooney although we might need a new config option
12:32:28 sean-k-mooney a share_access_network_ip
12:32:43 sean-k-mooney which we can default based on a lookup using the hypervioer_hostname
12:33:03 sean-k-mooney but for those that have need to have a seperate network for this they can overried it
12:33:12 sean-k-mooney like we do for the live migratieon inbound adress
12:33:22 sean-k-mooney gibi: Uggla ^ what do you think
12:33:25 gibi so define one IP per compute for manila shares
12:33:43 gibi that make sense
12:33:51 sean-k-mooney yes and default to socket.getIpform host or whatehever
12:34:07 sean-k-mooney there is a socket fucntion that gives your an ip form a hostname
12:34:22 sean-k-mooney so use that by default but allow the operator to provide an alternitive
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 gibi cool
12:48:30 sean-k-mooney tl;dr to the grant in the compute and read CONF.my_block_storage_ip
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 i agree that the tempest test is going to be more valueable
14:11:06 sean-k-mooney well its more that the functional test is closer to a unit test as written
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

Earlier   Later