| opendevreview | Merged openstack/nova master: Reject PCI dependent device config https://review.opendev.org/c/openstack/nova/+/846435 | 04:42 |
|---|---|---|
| opendevreview | Merged openstack/nova master: Reject mixed VF rc and trait config https://review.opendev.org/c/openstack/nova/+/846436 | 04:46 |
| opendevreview | Merged openstack/nova master: Ignore PCI devs with physical_network tag https://review.opendev.org/c/openstack/nova/+/846219 | 04:51 |
| opendevreview | Merged openstack/nova master: Reject devname based device_spec config https://review.opendev.org/c/openstack/nova/+/846466 | 04:58 |
| opendevreview | Merged openstack/nova master: Support [pci]device_spec reconfiguration https://review.opendev.org/c/openstack/nova/+/846470 | 04:58 |
| opendevreview | Merged openstack/nova master: Stop if tracking is disable after it was enabled before https://review.opendev.org/c/openstack/nova/+/847009 | 04:59 |
| opendevreview | Merged openstack/nova master: Move provider_tree RP creation to PciResourceProvider https://review.opendev.org/c/openstack/nova/+/850546 | 05:01 |
| opendevreview | Amit Uniyal proposed openstack/nova master: Adds a repoducer for post live migration fail https://review.opendev.org/c/openstack/nova/+/854499 | 05:30 |
| opendevreview | Rajesh Tailor proposed openstack/nova master: Fix rescue volume-based instance https://review.opendev.org/c/openstack/nova/+/852737 | 06:54 |
| opendevreview | Rajesh Tailor proposed openstack/nova master: Fix rescue volume-based instance https://review.opendev.org/c/openstack/nova/+/852737 | 06:59 |
| gibi | good morning | 07:02 |
| gibi | wow all the approved PCI patch merged. the gate is in a pretty good shape then | 07:02 |
| Uggla | good morning | 07:39 |
| sean-k-mooney1 | gibi: by the way you know how cursed the gate is going to be for the rest of the cycle ya | 11:45 |
| *** sean-k-mooney1 is now known as sean-k-mooney | 11:47 | |
| opendevreview | Amit Uniyal proposed openstack/nova master: Adds a repoducer for post live migration fail https://review.opendev.org/c/openstack/nova/+/854499 | 12:14 |
| opendevreview | Amit Uniyal proposed openstack/nova master: [compute] always set instnace.host in post_livemigration https://review.opendev.org/c/openstack/nova/+/791135 | 12:14 |
| gibi | I'm hopefule about the gate :) | 12:14 |
| sean-k-mooney | i reviewed the bfv reuild patches today | 12:15 |
| sean-k-mooney | the last 2 are mostly ok but the last one needs docs for the api change | 12:16 |
| sean-k-mooney | the first needs some changes | 12:16 |
| sean-k-mooney | is it intentional that we did not implemtne this for ironic? | 12:16 |
| gibi | I reviewed a buncs of manila today | 12:16 |
| gibi | bunch | 12:16 |
| sean-k-mooney | which do you think makes sense for me to look at next manilla or encypted storage | 12:17 |
| sean-k-mooney | i was going to look that encypted | 12:17 |
| opendevreview | ribaudr proposed openstack/nova master: Attach Manila shares via virtiofs (db) https://review.opendev.org/c/openstack/nova/+/831193 | 12:21 |
| opendevreview | ribaudr proposed openstack/nova master: Attach Manila shares via virtiofs (objects) https://review.opendev.org/c/openstack/nova/+/839401 | 12:21 |
| opendevreview | ribaudr proposed openstack/nova master: Attach Manila shares via virtiofs (manila abstraction) https://review.opendev.org/c/openstack/nova/+/831194 | 12:21 |
| opendevreview | ribaudr proposed openstack/nova master: Attach Manila shares via virtiofs (drivers and compute manager part) https://review.opendev.org/c/openstack/nova/+/833090 | 12:21 |
| opendevreview | ribaudr proposed openstack/nova master: Attach Manila shares via virtiofs (api) https://review.opendev.org/c/openstack/nova/+/836830 | 12:21 |
| opendevreview | ribaudr proposed openstack/nova master: Bump compute version and check shares support https://review.opendev.org/c/openstack/nova/+/850499 | 12:21 |
| opendevreview | ribaudr proposed openstack/nova master: Add metadata for shares https://review.opendev.org/c/openstack/nova/+/850500 | 12:21 |
| opendevreview | ribaudr proposed openstack/nova master: Add instance.share_attach notification https://review.opendev.org/c/openstack/nova/+/850501 | 12:21 |
| opendevreview | ribaudr proposed openstack/nova master: Add instance.share_detach notification https://review.opendev.org/c/openstack/nova/+/851028 | 12:21 |
| opendevreview | ribaudr proposed openstack/nova master: Add shares to InstancePayload https://review.opendev.org/c/openstack/nova/+/851029 | 12:21 |
| opendevreview | ribaudr proposed openstack/nova master: Add instance.power_on_error notification https://review.opendev.org/c/openstack/nova/+/852084 | 12:21 |
| opendevreview | ribaudr proposed openstack/nova master: Add instance.power_off_error notification https://review.opendev.org/c/openstack/nova/+/852278 | 12:21 |
| opendevreview | ribaudr proposed openstack/nova master: Add helper methods to attach/detach shares https://review.opendev.org/c/openstack/nova/+/852085 | 12:21 |
| opendevreview | ribaudr proposed openstack/nova master: Add libvirt test to ensure metadata are working. https://review.opendev.org/c/openstack/nova/+/852086 | 12:21 |
| opendevreview | ribaudr proposed openstack/nova master: Add virt/libvirt error test cases https://review.opendev.org/c/openstack/nova/+/852087 | 12:21 |
| opendevreview | ribaudr proposed openstack/nova master: Change microversion to 2.93 https://review.opendev.org/c/openstack/nova/+/852088 | 12:21 |
| gibi | sean-k-mooney: encrypted | 12:21 |
| gibi | sean-k-mooney: I can check the bfv after I finish with the manila api patach | 12:21 |
| sean-k-mooney | i kicked the first 4 that are remaining into the queue | 12:22 |
| sean-k-mooney | https://review.opendev.org/c/openstack/nova/+/826752 | 12:22 |
| sean-k-mooney | is where melwitt is adding the encyption options which she has set -w on | 12:22 |
| sean-k-mooney | ill review some of the later patches but that will need ot be updated | 12:22 |
| 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 |
| gibi | sean-k-mooney: nova-api tries to resolve the compute hostname to ip address | 12:26 |
| sean-k-mooney | that sound incorrect | 12:26 |
| gibi | yeah it feels strange | 12:27 |
| 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:27 |
| sean-k-mooney | the api also may not be able to resolve the compute ip in an iolsated networkign config | 12:28 |
| gibi | yes, also this socket call only works for ipv4 | 12:28 |
| 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:28 |
| 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 |
| sean-k-mooney | what is this bieing used for | 12:29 |
| gibi | it is to allow mounting the share to the compute | 12:29 |
| sean-k-mooney | well we dont have to use that grant type | 12:29 |
| sean-k-mooney | i was hoping to move to the cert one | 12:29 |
| gibi | ahh I see | 12:29 |
| sean-k-mooney | i think generating a tls cert for each share attachmetn woudl be the better approch longterm | 12:30 |
| opendevreview | Amit Uniyal proposed openstack/nova master: [compute] always set instnace.host in post_livemigration https://review.opendev.org/c/openstack/nova/+/791135 | 12:31 |
| sean-k-mooney | so for now i guess if we are using ip we shoudl do this in the compute | 12:32 |
| sean-k-mooney | not the api | 12:32 |
| sean-k-mooney | although we might need a new config option | 12:32 |
| sean-k-mooney | a share_access_network_ip | 12:32 |
| sean-k-mooney | which we can default based on a lookup using the hypervioer_hostname | 12:32 |
| sean-k-mooney | but for those that have need to have a seperate network for this they can overried it | 12:33 |
| sean-k-mooney | like we do for the live migratieon inbound adress | 12:33 |
| sean-k-mooney | gibi: Uggla ^ what do you think | 12:33 |
| gibi | so define one IP per compute for manila shares | 12:33 |
| gibi | that make sense | 12:33 |
| sean-k-mooney | yes and default to socket.getIpform host or whatehever | 12:33 |
| sean-k-mooney | there is a socket fucntion that gives your an ip form a hostname | 12:34 |
| sean-k-mooney | so use that by default but allow the operator to provide an alternitive | 12:34 |
| sean-k-mooney | if they have a dedicated storage network | 12:34 |
| 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 |
| gibi | push the issue up in the stack to the deployment engine to provide a proper IP ther | 12:34 |
| gibi | e | 12:35 |
| 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 |
| sean-k-mooney | we have https://docs.openstack.org/nova/latest/configuration/config.html#DEFAULT.my_ip | 12:35 |
| sean-k-mooney | and https://docs.openstack.org/nova/latest/configuration/config.html#DEFAULT.my_block_storage_ip | 12:35 |
| sean-k-mooney | so i guess it should use the blockstorage one by default? | 12:35 |
| sean-k-mooney | hum looks like that is only used in hyperv | 12:37 |
| gibi | hm, that is already an IP, so I'm OK with that | 12:37 |
| sean-k-mooney | so lets reuse that for maniall | 12:37 |
| sean-k-mooney | and just document that its now used by libvirt for this usecase too? | 12:37 |
| sean-k-mooney | it also already has a default | 12:38 |
| sean-k-mooney | https://github.com/openstack/nova/blob/0d84833e9688e0df97f3d24e06025e512bca3ce3/nova/conf/netconf.py#L24-L51 | 12:38 |
| sean-k-mooney | =netutils.get_my_ipv4( | 12:38 |
| gibi | I don't like the defaulting, but I won't block on that | 12:38 |
| sean-k-mooney | well i guess the intent is ot have it work out of the box | 12:39 |
| 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:39 |
| sean-k-mooney | that is not default because we use the hostname if not set | 12:40 |
| 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:42 |
| gibi | but that is just a philosophy I can supress :) | 12:43 |
| 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:43 |
| sean-k-mooney | but ya i understand why you would like it to be explict | 12:44 |
| gibi | OK, I left comment in the API patch and linked to this IRC log for Uggla to look at | 12:44 |
| gibi | sean-k-mooney: thanks | 12:44 |
| Uggla | sorry was looking at something else. I have just quicky read the chat. I think I get the idea. | 12:46 |
| sean-k-mooney | tl;dr to the grant in the compute and read CONF.my_block_storage_ip | 12:48 |
| gibi | cool | 12:48 |
| sean-k-mooney | and in the futre maybe we can use the cert grant instead but not sure how that works | 12:48 |
| 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 |
| 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:49 |
| sean-k-mooney | which is not for this cycle | 12:50 |
| Uggla | sean-k-mooney, agree. | 13:22 |
| gibi | I'm done with the bfv rebuild, I had some additional questions top of what you sean-k-mooney had | 13:35 |
| sean-k-mooney | oh ya i forgot we have not drop the 5.x proxy code yet | 13:51 |
| sean-k-mooney | also good point on the functional test disbaing much of the code | 13:53 |
| gibi | honestly I did not check the unit testts | 13:53 |
| 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 |
| gibi | I think the key there would be to assert cinder interaction | 13:54 |
| gibi | I don't care which driver fixture we use for it | 13:55 |
| sean-k-mooney | right but we should not mock with mock.patch.object(self.compute.manager, | 13:55 |
| sean-k-mooney | '_rebuild_volume_backed_instance'), \ | 13:55 |
| sean-k-mooney | in a functional test | 13:55 |
| sean-k-mooney | we shoudl acutlly create a ocmpute service create an instnace and rebuild it as you said | 13:55 |
| gibi | yes | 13:57 |
| gibi | and assert the cinder fixture state after the rebuild | 13:57 |
| gibi | probably need a some extra state in the cinder fixture to store what image is on th volume | 13:57 |
| *** amorin_ is now known as amorin | 14:00 | |
| *** dasm|off is now known as dasm | 14:04 | |
| 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 |
| dansmith | the functional for testing the failure cases is better though obviously | 14:07 |
| opendevreview | Merged openstack/nova master: virt: Add block_device_info helper to find encrypted disks https://review.opendev.org/c/openstack/nova/+/826529 | 14:08 |
| sean-k-mooney | well its more that the functional test is closer to a unit test as written | 14:11 |
| sean-k-mooney | i agree that the tempest test is going to be more valueable | 14:11 |
| dansmith | sean-k-mooney: you know the tempest test is written and working right? (just in case you didn't see it) | 14:12 |
| gibi | I agree too. I left some error case questing inline that would benefit from a functional test | 14:12 |
| gibi | *questiong | 14:12 |
| gibi | *question | 14:12 |
| gibi | (it is Friday) | 14:12 |
| sean-k-mooney | dansmith: yes i have seen it | 14:12 |
| *** hemna6 is now known as hemna | 14:12 | |
| gibi | like I'm not sure what will happen with the instance if the reimage fails | 14:13 |
| sean-k-mooney | the -1 i left on the last patch was more for the lack of docs by the way | 14:13 |
| sean-k-mooney | specifica the api ref | 14:13 |
| sean-k-mooney | we need to update that note that calls out how rebuidl for bfv is differnt | 14:13 |
| 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 |
| dansmith | I dunno how to fix that | 14:13 |
| sean-k-mooney | dansmith: actuly i didnt think they were responsive | 14:14 |
| dansmith | I spent a lot of time with him on it, but had several interruptions this cycle | 14:14 |
| 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 |
| 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:14 |
| 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 |
| gibi | sorry for not reviewing it earlier but I hope it is better to review it late than never | 14:15 |
| sean-k-mooney | dansmith: the patches are in merge conflict one way or another so they need to actuly be update | 14:16 |
| 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 |
| sean-k-mooney | i tought most of the issue were pretty minor that i pointed too and would not take that long | 14:16 |
| gibi | dansmith: I agree on the more deadlines | 14:17 |
| sean-k-mooney | althoghg is it intentional that they did not implement ironic support | 14:17 |
| dansmith | sean-k-mooney: yeah, we knew it was going to need updates, and I'm not complaining about the comments (at all) | 14:17 |
| dansmith | sean-k-mooney: I think that's probably unintentional and I didn't even catch it | 14:18 |
| 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 |
| sean-k-mooney | ack we can just call that out as a limiation in the docs and fix it next cycle | 14:18 |
| dansmith | gibi: oh I know, I'm certainly not complaining about the amount of reviews being done :) | 14:18 |
| sean-k-mooney | its just because ironic does not use the default implementation of the rebuild fucntion | 14:18 |
| sean-k-mooney | it has its own | 14:18 |
| dansmith | sean-k-mooney: yeah I know, but I also didn't know we had bfv with ironic :) | 14:19 |
| 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:21 |
| 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 |
| gibi | allocation 2) the pci and numa fitting logic is shared between the scheduler and the compute | 14:23 |
| 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 |
| 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:24 |
| gibi | or at least pretty shaky business | 14:25 |
| gibi | fortunately I don't have to think about feature backport in this chat window :D | 14:25 |
| 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 |
| 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 |
| opendevreview | Merged openstack/nova master: libvirt: Consolidate create_cow_image and create_image https://review.opendev.org/c/openstack/nova/+/846246 | 14:26 |
| dansmith | gibi: :) | 14:26 |
| opendevreview | Merged openstack/nova stable/wallaby: add regression test case for bug 1978983 https://review.opendev.org/c/openstack/nova/+/853811 | 14:27 |
| 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:28 |
| gibi | dansmith: https://etherpad.opendev.org/p/nova-zed-microversions-plan | 14:33 |
| dansmith | yeah, thanks | 14:33 |
| gibi | I'm not against to reorder the next to microversion | 14:34 |
| dansmith | is that 2.93 one likely to get the review and attention it needs? | 14:34 |
| gibi | if the rebuild bfv become ready before the user_data update | 14:34 |
| gibi | dansmith: I'm actively helping 2.39 and melwitt too | 14:35 |
| dansmith | looks like it's getting attention.. yeah okay cool | 14:35 |
| 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 | 14:36 |
| dansmith | oh yeah for sure, | 14:36 |
| dansmith | I just wanted to make sure that this was still the ordering before I rebase | 14:36 |
| gibi | I have no reason to reorder now as both feautre needs work | 14:36 |
| dansmith | yup | 14:37 |
| gibi | also don't want to step over our PTL :) | 14:37 |
| dansmith | again, I just wanted to make sure I knew the right thing to rebase on, nothing more :) | 14:38 |
| gibi | no worried :) | 14:38 |
| gmann | gibi: do you know till when bauzas is on PTO? | 14:40 |
| gibi | let me double check | 14:40 |
| gibi | he is back on the 30th | 14:41 |
| gibi | based on the RH internal calendar | 14:41 |
| gmann | ok, just one day before PTL nomination close. let me rebase his nomination patch which he added before going to PTO. | 14:41 |
| gibi | ack | 14:42 |
| dansmith | looks like user_data is pretty far behind master at this point too, so I hope that gets rebased when it is updated | 14:46 |
| gibi | ahh you already commented that to review so I don't need to :) | 14:48 |
| dansmith | yeah just did, typo and all | 14:49 |
| opendevreview | Merged openstack/nova stable/wallaby: For evacuation, ignore if task_state is not None https://review.opendev.org/c/openstack/nova/+/853812 | 14:55 |
| dansmith | sean-k-mooney: your comment here says 64: https://review.opendev.org/c/openstack/nova/+/820368/32/nova/objects/service.py | 14:59 |
| dansmith | ah, nevermind | 15:00 |
| * dansmith gets out his calculator to add 63 and 1 | 15:00 | |
| sean-k-mooney | nice power of 2 | 15:01 |
| gibi | now we can infer that your brain uses 6 bit ints internally | 15:01 |
| dansmith | okay I rebased that stack plus user data on master, but won't push user data of course | 15:02 |
| dansmith | but this should apply cleanly once the user data author does | 15:03 |
| gibi | cool | 15:03 |
| dansmith | some conflicts in exception.py too | 15:04 |
| dansmith | is that author on irc? | 15:04 |
| gibi | the author's IRC nick was jhartkopf in the past | 15:05 |
| gibi | he was on nova before but not at the moment | 15:06 |
| dansmith | ah | 15:06 |
| sean-k-mooney | the os traits release i think is still pending but that should happen soon i hope | 15:07 |
| gibi | sean-k-mooney: I talked about that with elodilles and he expect it to happen today | 15:07 |
| sean-k-mooney | cool | 15:08 |
| sean-k-mooney | dansmith: so the docs change i reqestied in the last patch could be in a followup which can merge after FF | 15:09 |
| * dansmith nods | 15:09 | |
| sean-k-mooney | am i might even be able to jsut write that for them | 15:09 |
| sean-k-mooney | we can call out this does not work for ironic htere too | 15:09 |
| sean-k-mooney | and adress that next cycle | 15:09 |
| sean-k-mooney | if you have fixed the compute service version then i think that was the main thing that need to be adress to resolve the merge conflict | 15:10 |
| dansmith | tbh, I'm not sure we even need to pass that to the virt driver anymore, | 15:10 |
| dansmith | that might be a holdover from a previous approach to this | 15:10 |
| dansmith | oh right, envermind, | 15:11 |
| dansmith | because the default impl is in compute manager | 15:11 |
| sean-k-mooney | yep | 15:11 |
| dansmith | I was eye-grepping for virt/* | 15:11 |
| sean-k-mooney | ironic has an imple of it | 15:11 |
| sean-k-mooney | but i think only it does | 15:11 |
| sean-k-mooney | so it does not fallback | 15:11 |
| dansmith | that bottom patch fails unit tests with a missing trait, but does not depends-on anything else.. I assume that's because we're waiting for the traits release? | 15:12 |
| sean-k-mooney | its why ironic supprot rebuild witout erasing the epmeeral disk while reimaing the root disk | 15:12 |
| sean-k-mooney | yes | 15:12 |
| dansmith | having this behave differently for one virt driver does not seem like a very good user experience, even though ironic is weird | 15:13 |
| sean-k-mooney | right ironic already had specific beahivor | 15:13 |
| dansmith | sigh | 15:13 |
| sean-k-mooney | im not sure it would take much to add ironci supprot but testin gwould be hard | 15:13 |
| sean-k-mooney | the ironic specific behavior is that it has an api parmater that allows the ephemerl disk to not be erased when you rebild with a new image | 15:14 |
| sean-k-mooney | we could support that for libvirt too but we dotn since the default impl does not support it | 15:14 |
| dansmith | SIGH | 15:15 |
| dansmith | so yeah, so much of rebuild is done by ironic, perhaps just refusing to do rebuild on bfv if we're instructed to wipe the root disk is the right approach | 15:16 |
| dansmith | but we'll really need that implemented | 15:16 |
| sean-k-mooney | well we can check that in the api fairly simplely | 15:17 |
| sean-k-mooney | we can check the hypervior type i belive | 15:17 |
| dansmith | which would be terrible | 15:17 |
| sean-k-mooney | and just reject it with a 400 | 15:17 |
| dansmith | we should not have the API behaving differently depending on the virt driver | 15:17 |
| sean-k-mooney | it also should not behave differntly based on if the instnace is boot form volume | 15:18 |
| dansmith | that's compute stuff, and while it sucks to fail late like that, I much prefer that than building virt-specific stuff into the api | 15:18 |
| sean-k-mooney | but that the situration we are in | 15:18 |
| sean-k-mooney | due to the hacky (only update metadat if the image is the same and its BFV) legacy | 15:18 |
| dansmith | the api has to behave differently for bfv because it historically did, but we shouldn't be building *new* stuff that drags virt specifics into the api | 15:19 |
| sean-k-mooney | ack i agree with that | 15:19 |
| sean-k-mooney | the preserve_ephemeral filed looks like it predates microveriosn by the way | 15:19 |
| dansmith | basically all the functionals on that user data patch fail because of that missing trait, | 15:19 |
| sean-k-mooney | i was trying to figure out when we added that | 15:19 |
| dansmith | which makes it hard to work on the functional tests | 15:20 |
| sean-k-mooney | i belvie the pased in the previsous version of the patch that did not have the trait | 15:20 |
| sean-k-mooney | locally we can just pip install the os-traits git repo | 15:20 |
| sean-k-mooney | but ya that why i was hopign to do the release yesterday | 15:20 |
| dansmith | yeah, I'll have to do that | 15:20 |
| sean-k-mooney | elodilles: any chance we can merge https://review.opendev.org/c/openstack/releases/+/854617 | 15:21 |
| sean-k-mooney | to adress ^ | 15:21 |
| elodilles | sean-k-mooney: ack, will look into it | 15:22 |
| opendevreview | Merged openstack/nova master: Add locked_memory extra spec and image property https://review.opendev.org/c/openstack/nova/+/778347 | 15:22 |
| sean-k-mooney | elodilles++ thanks very much :) | 15:22 |
| elodilles | sean-k-mooney: do you need to release it ASAP? I'm asking because if really needed then I'll +W with single +2 (i'm the only release manager on duty today o:)) otherwise we need to wait till Monday | 15:24 |
| sean-k-mooney | am ideally yes. it could wait but we have 2 api changes blocked by it currently. | 15:27 |
| sean-k-mooney | its at your disgression ultimately dansmith or gibi might have a stonger prefernce | 15:28 |
| gibi | I'm fine releasing that today with a single release mgmt +2 | 15:28 |
| dansmith | yeah, it'd be a lot better if it could be today, unfortunately | 15:29 |
| * gibi wondering if we can move the next summer Feature Freeze to Sept | 15:29 | |
| gibi | PTO + FF is not a good combination | 15:29 |
| elodilles | sean-k-mooney gibi dansmith: ack, os-traits release is on the way | 16:06 |
| dansmith | elodilles: thanks! | 16:06 |
| gibi | elodilles: thank oyu | 16:06 |
| gibi | you | 16:06 |
| elodilles | np | 16:06 |
| sean-k-mooney | once that is release we shuld bump our min required version https://github.com/openstack/nova/blob/master/requirements.txt#L56 right in the change that adds the userdata feature | 16:07 |
| sean-k-mooney | like i know we will get it automaticaly but we should raise our dep in the feature that requires the new trait | 16:07 |
| opendevreview | Dan Smith proposed openstack/nova master: DNM: Test for rosmaita https://review.opendev.org/c/openstack/nova/+/854815 | 16:09 |
| opendevreview | Dan Smith proposed openstack/nova master: DNM: Test for rosmaita https://review.opendev.org/c/openstack/nova/+/854815 | 16:10 |
| gibi | the gate queue is 53 patches long, nice | 16:10 |
| elodilles | :-o | 16:12 |
| sean-k-mooney | oh the oauth 2 feature is mergin in keystone | 16:12 |
| sean-k-mooney | thats cool | 16:12 |
| dansmith | sean-k-mooney: I'm getting functional fails on missing api samples for 2.93 | 16:15 |
| dansmith | looks like the author dropped some of them from a previous version? | 16:15 |
| sean-k-mooney | on hte user data patch lets see | 16:15 |
| dansmith | yeah | 16:16 |
| sean-k-mooney | ... i jsut realsied i review version 8 not 9 just now | 16:16 |
| sean-k-mooney | and yes | 16:16 |
| sean-k-mooney | https://review.opendev.org/c/openstack/nova/+/816157/8..9 | 16:16 |
| sean-k-mooney | there were more removed then added | 16:16 |
| sean-k-mooney | im stongly tempeted to say we shoudl swap those two if the author of the user data one has not updated it by monday | 16:17 |
| sean-k-mooney | im tempeted to say we do it now but its kind of a pain to update all the micorversion refrences | 16:17 |
| sean-k-mooney | ya so it passed on 8 https://review.opendev.org/c/openstack/nova/+/816157/9#message-87c965d558c8c7b25b2c4d1ebd3232c2627f444a and likely because of os-traits not being released they did not realise it failed on v9 | 16:20 |
| sean-k-mooney | thats a pian | 16:20 |
| dansmith | yeah | 16:21 |
| opendevreview | Merged openstack/nova master: Retry /reshape at provider generation conflict https://review.opendev.org/c/openstack/nova/+/851358 | 17:07 |
| dansmith | gibi: still around? | 17:36 |
| gibi | dansmith: yes, but with limited brain power | 17:37 |
| dansmith | gibi: on the error path.. if we created the new attachment, saved it to the bdm, but failed to delete the old one, is there any reason not to just keep rolling? | 17:37 |
| dansmith | like what does restoring the old attachment_id and aborting get us there? | 17:38 |
| sean-k-mooney | so the db save failed | 17:38 |
| dansmith | no | 17:38 |
| dansmith | the db save of the new one completed | 17:38 |
| dansmith | the delete of the old one failed (in cinderclient) | 17:38 |
| sean-k-mooney | but the delete in cinder failed | 17:38 |
| sean-k-mooney | ah ok | 17:39 |
| sean-k-mooney | am well we could leak it but other then that i think its ok | 17:39 |
| sean-k-mooney | if we have our side in order | 17:39 |
| dansmith | right, we leak an attachment, but presumably we're already sunk if we fail to delete | 17:39 |
| gibi | dansmith: please point me the case in the code | 17:39 |
| dansmith | gibi: https://review.opendev.org/c/openstack/nova/+/820368/32/nova/compute/manager.py in L3437 | 17:40 |
| sean-k-mooney | dansmith: at that point have we "bound" the new attachmet propelry and got all the connector info so we can update the xml and proceed | 17:40 |
| sean-k-mooney | presuamable deleteing the attcoemnt woudl only fail if cidner was unaviable or something like that | 17:41 |
| dansmith | sean-k-mooney: well this is before the reimage, but yeah, I'm saying if we've recorded the new attachment and can't delete the old one, I'm not sure it gets us much to revert our own db record to the old attachment and then try to delete the new one | 17:41 |
| sean-k-mooney | ah | 17:41 |
| gibi | dansmith: so the root_bdm.attachment_id points to the new attachement_id and we just deleted that in cinder | 17:41 |
| dansmith | reverting to the old one and deleting the new one is a more complicated cleanup, which I can do, but I just want to make sure it's worth it | 17:42 |
| gibi | by L3441 | 17:42 |
| dansmith | gibi: that's what I'm saying.. I think if we hit a cinder error in L3437, we should NOT do L3441 and abort | 17:42 |
| gibi | dansmith: OK, that make sense | 17:43 |
| dansmith | the clientexception will catch errors on L3432 *and* 3437 | 17:43 |
| gibi | keep the bdm to use the new attachment id | 17:43 |
| dansmith | ++ okay, it's much cleaner (on our side) to do that, but just wanted to make sure that was reasonable | 17:43 |
| gibi | but we still need to handle the fact that we might detached the volume from the guest already at 3434 | 17:44 |
| sean-k-mooney | i kind of feel like it migh make more sesen to break up that try | 17:44 |
| dansmith | sean-k-mooney: s'what I'm doing (and said in the review) | 17:45 |
| sean-k-mooney | ack | 17:45 |
| dansmith | gibi: yeah, so I'm going to delete the new attachment if we fail to save, but otherwise, I'm going to log both and the situation and plow on | 17:45 |
| gibi | and we say that the instance can be recovered with a hard reboot from this case? | 17:46 |
| gibi | as at that point when the save fail we removed the volume from the guest | 17:46 |
| dansmith | I don't know that we need even that | 17:47 |
| sean-k-mooney | so the exception.InstanceNotFound is coming form the detac potieintally or can it come form the create too | 17:47 |
| sean-k-mooney | gibi: if the bdms are correct and the atachment we created on line 3432 | 17:47 |
| dansmith | sean-k-mooney: only the bdm save, AFAIK | 17:48 |
| sean-k-mooney | is correct then a hard reboot might work | 17:48 |
| dansmith | why do we need a hard reboot? | 17:48 |
| dansmith | we've fixed the instance to point to the new attachment, we can just move on with the reimage | 17:48 |
| dansmith | we' | 17:48 |
| dansmith | we will have leaked an attachment that cinder didn't let us delete but... that doesn't impact the instance anymore right? | 17:48 |
| gibi | ahh OK | 17:48 |
| gibi | I see now | 17:48 |
| gibi | I thought we would abort | 17:48 |
| dansmith | I wish I could push this up, but I'll rebase the userdata one if I do :/ | 17:49 |
| sean-k-mooney | ya that was what i was workign my way thoguht we abort and go to error | 17:49 |
| dansmith | we abort all cases, except the last delete of the old attachment | 17:49 |
| sean-k-mooney | but if we move on | 17:49 |
| gibi | OK, that is fine | 17:49 |
| dansmith | because it doesn't matter at that point | 17:49 |
| sean-k-mooney | as you suggest no reboot needed | 17:49 |
| sean-k-mooney | actully | 17:49 |
| sean-k-mooney | if we get instance not found form our own db | 17:49 |
| sean-k-mooney | well no form libvirt | 17:50 |
| sean-k-mooney | prefumably that means we raced with a delete? | 17:50 |
| dansmith | on bdm save, yeah | 17:50 |
| sean-k-mooney | ya so nothing to try and recover | 17:50 |
| sean-k-mooney | dansmith: you can do git review -R by the way to avoid a rebase | 17:51 |
| dansmith | sean-k-mooney: no, I've already rebased user data locally so I could get this rebased and ready | 17:52 |
| sean-k-mooney | ah ok well it would not be the end fo the world if it was rebased. it needs to be updated for the api sample issue anyway | 17:52 |
| dansmith | I just don't want to step on the other author if they already have things in the middle | 17:53 |
| sean-k-mooney | ack, but yes it sounds like you can just proceed and leak the attacment and may just log it so an op could clean it up later | 17:54 |
| sean-k-mooney | so whats left is gettin gth image form galnce. using that to calualte the size, setting up the wait for the external event and then calling cinder to do the reimage | 17:56 |
| opendevreview | Balazs Gibizer proposed openstack/nova master: Handle PCI dev reconf with allocations https://review.opendev.org/c/openstack/nova/+/852397 | 17:56 |
| opendevreview | Balazs Gibizer proposed openstack/nova master: Allow enabling PCI tracking in Placement https://review.opendev.org/c/openstack/nova/+/850468 | 17:56 |
| opendevreview | Balazs Gibizer proposed openstack/nova master: Generate request_id for Flavor based InstancePCIRequest https://review.opendev.org/c/openstack/nova/+/853835 | 17:56 |
| opendevreview | Balazs Gibizer proposed openstack/nova master: Create RequestGroups from InstancePCIRequests https://review.opendev.org/c/openstack/nova/+/852771 | 17:56 |
| opendevreview | Balazs Gibizer proposed openstack/nova master: Support resource_class and traits in PCI alias https://review.opendev.org/c/openstack/nova/+/853316 | 17:56 |
| opendevreview | Balazs Gibizer proposed openstack/nova master: Split PCI pools per PF https://review.opendev.org/c/openstack/nova/+/854440 | 17:56 |
| opendevreview | Balazs Gibizer proposed openstack/nova master: Map PCI pools to RP UUIDs https://review.opendev.org/c/openstack/nova/+/854118 | 17:56 |
| opendevreview | Balazs Gibizer proposed openstack/nova master: Make allocation candidates available for scheduler filters https://review.opendev.org/c/openstack/nova/+/854119 | 17:56 |
| opendevreview | Balazs Gibizer proposed openstack/nova master: Filter PCI pools based on Placement allocation https://review.opendev.org/c/openstack/nova/+/854120 | 17:56 |
| opendevreview | Balazs Gibizer proposed openstack/nova master: Store allocated RP in InstancePCIRequest https://review.opendev.org/c/openstack/nova/+/854121 | 17:56 |
| opendevreview | Balazs Gibizer proposed openstack/nova master: Func test for PCI in placement scheduling https://review.opendev.org/c/openstack/nova/+/854122 | 17:56 |
| opendevreview | Balazs Gibizer proposed openstack/nova master: Support cold migrate and resize with PCI tracking in placement https://review.opendev.org/c/openstack/nova/+/854247 | 17:56 |
| opendevreview | Balazs Gibizer proposed openstack/nova master: Support evacuate with PCI in placement https://review.opendev.org/c/openstack/nova/+/854615 | 17:57 |
| opendevreview | Balazs Gibizer proposed openstack/nova master: Support unshelve with PCI in placement https://review.opendev.org/c/openstack/nova/+/854616 | 17:57 |
| opendevreview | Balazs Gibizer proposed openstack/nova master: Support same host resize with PCI in placement https://review.opendev.org/c/openstack/nova/+/854441 | 17:57 |
| opendevreview | Balazs Gibizer proposed openstack/nova master: Test reschedule with PCI in placement https://review.opendev.org/c/openstack/nova/+/854626 | 17:57 |
| opendevreview | Balazs Gibizer proposed openstack/nova master: Support multi create with PCI in placement https://review.opendev.org/c/openstack/nova/+/854663 | 17:57 |
| opendevreview | Balazs Gibizer proposed openstack/nova master: Heal allocation for same host resize https://review.opendev.org/c/openstack/nova/+/854822 | 17:57 |
| dansmith | ugh, that causes another failure later, which may be a quirk of our cinder fixture | 17:58 |
| dansmith | the attachment isn't deleted, so much later it complains that the instance is double-attached | 17:58 |
| sean-k-mooney | well form the cinder side i guess it is | 17:58 |
| sean-k-mooney | we could try and recover at that point and delete the old one again | 17:59 |
| sean-k-mooney | which would possisble stop the leak | 17:59 |
| dansmith | that's what I'm trying to avoid because it ends up a pretty nested mess | 17:59 |
| sean-k-mooney | ah ok | 17:59 |
| dansmith | and it's likely to fail in reality if we just failed to delete | 17:59 |
| sean-k-mooney | we are not doing the attchment delete before the save to prevent the other case right | 18:00 |
| dansmith | I'm also not sure I know why we're detaching and re-attaching here, just to do the reimage | 18:00 |
| sean-k-mooney | where our db is now out of sync if we fail to save | 18:00 |
| dansmith | must be some cinder reason why that happens, to reset the state while the instance is powered off or something | 18:00 |
| dansmith | yeah I think we have to update our db before the delete for races | 18:00 |
| sean-k-mooney | ya i think so too. | 18:01 |
| opendevreview | ribaudr proposed openstack/nova master: Attach Manila shares via virtiofs (db) https://review.opendev.org/c/openstack/nova/+/831193 | 18:01 |
| opendevreview | ribaudr proposed openstack/nova master: Attach Manila shares via virtiofs (objects) https://review.opendev.org/c/openstack/nova/+/839401 | 18:01 |
| opendevreview | ribaudr proposed openstack/nova master: Attach Manila shares via virtiofs (manila abstraction) https://review.opendev.org/c/openstack/nova/+/831194 | 18:01 |
| opendevreview | ribaudr proposed openstack/nova master: Attach Manila shares via virtiofs (drivers and compute manager part) https://review.opendev.org/c/openstack/nova/+/833090 | 18:02 |
| opendevreview | ribaudr proposed openstack/nova master: Attach Manila shares via virtiofs (api) https://review.opendev.org/c/openstack/nova/+/836830 | 18:02 |
| opendevreview | ribaudr proposed openstack/nova master: Bump compute version and check shares support https://review.opendev.org/c/openstack/nova/+/850499 | 18:02 |
| opendevreview | ribaudr proposed openstack/nova master: Add metadata for shares https://review.opendev.org/c/openstack/nova/+/850500 | 18:02 |
| opendevreview | ribaudr proposed openstack/nova master: Add instance.share_attach notification https://review.opendev.org/c/openstack/nova/+/850501 | 18:02 |
| opendevreview | ribaudr proposed openstack/nova master: Add instance.share_detach notification https://review.opendev.org/c/openstack/nova/+/851028 | 18:02 |
| opendevreview | ribaudr proposed openstack/nova master: Add shares to InstancePayload https://review.opendev.org/c/openstack/nova/+/851029 | 18:02 |
| opendevreview | ribaudr proposed openstack/nova master: Add instance.power_on_error notification https://review.opendev.org/c/openstack/nova/+/852084 | 18:02 |
| opendevreview | ribaudr proposed openstack/nova master: Add instance.power_off_error notification https://review.opendev.org/c/openstack/nova/+/852278 | 18:02 |
| opendevreview | ribaudr proposed openstack/nova master: Add helper methods to attach/detach shares https://review.opendev.org/c/openstack/nova/+/852085 | 18:02 |
| opendevreview | ribaudr proposed openstack/nova master: Add libvirt test to ensure metadata are working. https://review.opendev.org/c/openstack/nova/+/852086 | 18:02 |
| opendevreview | ribaudr proposed openstack/nova master: Add virt/libvirt error test cases https://review.opendev.org/c/openstack/nova/+/852087 | 18:02 |
| opendevreview | ribaudr proposed openstack/nova master: Change microversion to 2.93 https://review.opendev.org/c/openstack/nova/+/852088 | 18:02 |
| opendevreview | ribaudr proposed openstack/nova master: Add share_info parameter to reboot method for each driver (driver part) https://review.opendev.org/c/openstack/nova/+/854823 | 18:02 |
| opendevreview | ribaudr proposed openstack/nova master: Support rebooting an instance with shares (compute and API part) https://review.opendev.org/c/openstack/nova/+/854824 | 18:02 |
| sean-k-mooney | i dont recall why but i rememebr lee disucsing this at some point | 18:02 |
| sean-k-mooney | i dont see this dicussed in the ptg | 18:06 |
| sean-k-mooney | ah | 18:07 |
| sean-k-mooney | https://review.opendev.org/c/openstack/nova-specs/+/840155/5/specs/zed/approved/volume-backed-server-rebuild.rst#58 | 18:07 |
| sean-k-mooney | #. Create an empty (no connector) volume attachment for the volume and | 18:07 |
| sean-k-mooney | server. This ensures the volume remains ``reserved`` through the next | 18:07 |
| sean-k-mooney | step. | 18:07 |
| sean-k-mooney | #. Delete the existing volume attachment (the old one). | 18:07 |
| sean-k-mooney | #. Save the new attachment UUID to the BDM. | 18:07 |
| sean-k-mooney | #. The above two steps are needed to keep the volume in ``reserved`` state | 18:07 |
| sean-k-mooney | as a management state which is required by cinder to perform re-image | 18:07 |
| sean-k-mooney | operation on it. | 18:07 |
| sean-k-mooney | #. Call the new ``os-reimage`` cinder API. | 18:07 |
| dansmith | yeah, to reset the state basically I assume | 18:07 |
| sean-k-mooney | we need to move it form in-use to reserved to reimage | 18:08 |
| sean-k-mooney | honestly i feel like the way out of this if everything explodes is just call rebuild again | 18:11 |
| sean-k-mooney | like wait for cinder to be back | 18:11 |
| sean-k-mooney | and then the vm that is precumsble in error if we coudl not comptle the reimage later | 18:11 |
| sean-k-mooney | could just be rebuilt again | 18:11 |
| dansmith | yeah I assume that will work, as long as it's still in a reasonable state | 18:12 |
| sean-k-mooney | kind of like what we do with a failed evacuate we just evacuate again | 18:12 |
| dansmith | so I guess I'm just going to put it somewhat back the way it was, let it fail for all cases, but only call delete if we created the new attachment | 18:13 |
| dansmith | right now it'll call delete(None) which I assume will fail | 18:13 |
| sean-k-mooney | ya presumabley with a type/atibute error | 18:13 |
| sean-k-mooney | None type has no attirbute uuid or something like that | 18:14 |
| gibi | rebuild after a failed rebuild make sense to me too | 18:15 |
| sean-k-mooney | i dont know if the precondtions on rebuild allow that | 18:16 |
| dansmith | yeah, I'm not so concerned about that, I just want to make sure we haven't left things in too bad of a state where that's not possible | 18:16 |
| sean-k-mooney | or if you would have to do reset state first | 18:16 |
| dansmith | sounds to me like we're going to have leaked an attachment regardless | 18:16 |
| sean-k-mooney | yes | 18:16 |
| dansmith | because we have nowhere to stash it and try to delete it later | 18:16 |
| sean-k-mooney | although the user can actully delete that if they wanted too | 18:16 |
| sean-k-mooney | it would be a bit painful however | 18:17 |
| sean-k-mooney | since they have no way to lsit teh bdms today | 18:17 |
| sean-k-mooney | its not part of server detail show | 18:18 |
| sean-k-mooney | i mena they coudl try deleteing all the attachments on a voluem but then nova and cinder are out of sync | 18:18 |
| sean-k-mooney | so i think we woudl want to make sure they coudl juse rebuild again | 18:19 |
| sean-k-mooney | rather then trying to repair it themselves | 18:19 |
| dansmith | I think that will fail because of the double attachment | 18:20 |
| dansmith | the same thing as when I failed the delete | 18:20 |
| dansmith | but I dunno there's much we can do about that | 18:20 |
| opendevreview | Rajat Dhasmana proposed openstack/nova master: add support for updating server's user_data https://review.opendev.org/c/openstack/nova/+/816157 | 18:56 |
| opendevreview | Rajat Dhasmana proposed openstack/nova master: Add support for volume backed server rebuild https://review.opendev.org/c/openstack/nova/+/820368 | 18:56 |
| opendevreview | Rajat Dhasmana proposed openstack/nova master: Add conductor RPC interface for rebuild https://review.opendev.org/c/openstack/nova/+/831219 | 18:56 |
| opendevreview | Rajat Dhasmana proposed openstack/nova master: Add API support for rebuilding BFV instances https://review.opendev.org/c/openstack/nova/+/830883 | 18:56 |
| ozzzo_work | We use the AggregateMultiTenancyIsolation filter for some aggregates, and for those aggregates we specify "filter_tenant_id = '(list of projects)' | 19:00 |
| ozzzo_work | Today I'm hitting that filter for aggregates that do not have filter_tenant_id set | 19:00 |
| ozzzo_work | This aggregate has no properties set at all | 19:01 |
| ozzzo_work | 2022-08-26 17:48:49.879 33 INFO nova.filters [req-f57f505f-b3c8-42f5-acf7-5e5efe10ef83 a9219bb013c944edea6ba612b8fa704adc28d676f3ad1e326094350b8ff0c9c0 efc7139e50db483991df846c69b42477 - 8793b235debf49e6aba6bd1e2bf65360 8793b235debf49e6aba6bd1e2bf65360] Filtering removed all hosts for the request with instance ID 'e9f9292a-f1c0-409b-a47f-9fa596fd0965'. Filter results: ['ComputeFilter: (start: 50, end: 50)', 'RetryFilter: (start: 50, end | 19:01 |
| ozzzo_work | looks like the line is too long | 19:01 |
| ozzzo_work | https://paste.openstack.org/show/b5UFNxMB3oIlduDz1nat/ | 19:02 |
| ozzzo_work | What could be causing that? | 19:02 |
| dansmith | oye | 19:02 |
| dansmith | whoami-rajat: did you make changes to that set or just rebase? | 19:02 |
| dansmith | whoami-rajat: I should have made it more clear -- I was doing some work there, but was trying to avoid rebasing the other person's patch | 19:03 |
| whoami-rajat | dansmith, just rebased to resolve merge conflict, now working on changes | 19:03 |
| dansmith | figured I was safe since you said monday | 19:03 |
| dansmith | whoami-rajat: hold on, I have a number of changes | 19:03 |
| whoami-rajat | dansmith, oh sorry about that | 19:03 |
| dansmith | I might as well push them up now that you rebased it | 19:03 |
| whoami-rajat | ack, will wait then | 19:03 |
| whoami-rajat | this microversion dependency doesn't seem good ... | 19:04 |
| dansmith | ah, whoami-rajat I think you reverted the other person's latest rev with your push just now :( | 19:08 |
| dansmith | yeah | 19:09 |
| whoami-rajat | oh ... | 19:09 |
| dansmith | okay so I'll just push up my stuff with a rebased version of their latest | 19:09 |
| whoami-rajat | i think it's just better to break the dependency chain | 19:10 |
| whoami-rajat | will need to do an extra rebase when their change is merged | 19:10 |
| whoami-rajat | i can see how it happened, their change was not latest and my patches were not rebased on their latest | 19:11 |
| whoami-rajat | should've been more careful there :/ | 19:11 |
| dansmith | whoami-rajat: right, I had it all set locally, | 19:11 |
| dansmith | and breaking the chain just means we'll have to do more version switching if we do that | 19:12 |
| dansmith | just a sec | 19:12 |
| opendevreview | Dan Smith proposed openstack/nova master: add support for updating server's user_data https://review.opendev.org/c/openstack/nova/+/816157 | 19:12 |
| opendevreview | Dan Smith proposed openstack/nova master: Add support for volume backed server rebuild https://review.opendev.org/c/openstack/nova/+/820368 | 19:12 |
| opendevreview | Dan Smith proposed openstack/nova master: Add conductor RPC interface for rebuild https://review.opendev.org/c/openstack/nova/+/831219 | 19:12 |
| opendevreview | Dan Smith proposed openstack/nova master: Add API support for rebuilding BFV instances https://review.opendev.org/c/openstack/nova/+/830883 | 19:12 |
| whoami-rajat | yeah 2.94 after 2.92 will break something surely | 19:12 |
| ozzzo_work | Is this the right channel to ask about nova filters? | 19:12 |
| dansmith | ozzzo_work: this is really for nova development not help, and it's friday afternoon | 19:13 |
| dansmith | whoami-rajat: so in the main patch I broke apart that error handler for the issue we noted there, | 19:13 |
| ozzzo_work | ok I'll try emailing the list | 19:13 |
| dansmith | whoami-rajat: and in the last one I made the test less mock-heavy and added some negative cases | 19:13 |
| dansmith | ozzzo_work: that'd be better | 19:13 |
| whoami-rajat | dansmith, great, i was just thinking about how to break those exception blocks, thanks | 19:15 |
| dansmith | whoami-rajat: I don't think we need to be fully covered in the functional tests, but I included several error paths, so I hope that's good enough, assuming I didn't break any unit tests.. more unit tests for covering all the potential exit paths is still good of course | 19:16 |
| whoami-rajat | ack, will take a look | 19:17 |
| dansmith | all the brought-forward comments are referring to the wrong lines in the latest PS instead of being collected up at the top, that's very weird | 19:18 |
| dansmith | IMHO you can resolve those and let people re-comment if they want | 19:19 |
| * dansmith hates that "feature" of gerrit. | 19:19 | |
| whoami-rajat | yeah that happens, I'm on the same patchset when the comments were left and it looks fine like that | 19:19 |
| whoami-rajat | left view: old PS with comments, right view: new PS | 19:20 |
| dansmith | right, but if you have base on the left and current on the right, it is supposed to show the comments on unchanged lines, or at the top if they've been changed | 19:22 |
| dansmith | and this is showing them on the wrong lines | 19:22 |
| dansmith | actually hard refresh seems to have fixed it, | 19:23 |
| dansmith | so maybe just a caching bug or something | 19:23 |
| whoami-rajat | yep, I've also been struggling with that recently | 19:23 |
| whoami-rajat | ah really? | 19:23 |
| whoami-rajat | yep that fixed it, nice | 19:24 |
| dansmith | haven't seen that before | 19:24 |
| sean-k-mooney | ozzzo_work: quick glance not sure | 19:27 |
| sean-k-mooney | dansmith: ya i often have to go to the patset the comment was left on to make any use out of that feature | 19:29 |
| sean-k-mooney | ok im starting to get hungery so im going to finish there for today. | 19:30 |
| sean-k-mooney | i will keep an eye on those patches | 19:31 |
| sean-k-mooney | i assume at this point however that it will be monday for the base patch to be ready | 19:31 |
| whoami-rajat | sean-k-mooney, if you're still around, I've a question | 19:54 |
| whoami-rajat | sean-k-mooney, regarding your comment here https://review.opendev.org/c/openstack/nova/+/820368/comments/711f86cd_9c3731a4 | 19:54 |
| whoami-rajat | sean-k-mooney, if we want to support it for the ironic use case, we would have to provide a generic driver implementation and also implemented it for the libvirt driver right? | 19:55 |
| dansmith | whoami-rajat: ironic is the only one that provides its own rebuild implementation, AFAIK | 19:55 |
| whoami-rajat | looking | 19:55 |
| dansmith | whoami-rajat: I think what you need to do is modify ironic's rebuild to check for the flag and fail if it's requested (for now) | 19:55 |
| dansmith | and obviously pass the flag to it as sean mentions | 19:55 |
| dansmith | whoami-rajat: getting it to work with ironic will require work on the ironic side, which obviously isn't going to happen | 19:56 |
| dansmith | ironic is already "weird" about rebuilds, so I think we just have to document that and hope we can get it resolved later | 19:56 |
| whoami-rajat | dansmith, yeah, that's what my concern was, if we just need to fail in that case then it's good | 19:56 |
| dansmith | it's not good, but..yeah :) | 19:56 |
| whoami-rajat | cool, that clears things | 19:56 |
| whoami-rajat | was kind of worried there | 19:57 |
| opendevreview | Rajat Dhasmana proposed openstack/nova master: Add support for volume backed server rebuild https://review.opendev.org/c/openstack/nova/+/820368 | 20:59 |
| opendevreview | Rajat Dhasmana proposed openstack/nova master: Add conductor RPC interface for rebuild https://review.opendev.org/c/openstack/nova/+/831219 | 20:59 |
| opendevreview | Rajat Dhasmana proposed openstack/nova master: Add API support for rebuilding BFV instances https://review.opendev.org/c/openstack/nova/+/830883 | 20:59 |
| *** dasm is now known as dasm|off | 21:07 | |
| opendevreview | Rico Lin proposed openstack/nova master: libvirt: Add vIOMMU device to guest https://review.opendev.org/c/openstack/nova/+/830646 | 21:16 |
| opendevreview | Rico Lin proposed openstack/nova master: Add traits for viommu model https://review.opendev.org/c/openstack/nova/+/844507 | 21:16 |
| ricolin | sean-k-mooney: thanks, just update patches, will try to work on traits asap | 21:17 |
Generated by irclog2html.py 2.17.3 by Marius Gedminas - find it at https://mg.pov.lt/irclog2html/!