*** pmannidi is now known as pmannidi|AFK | 01:12 | |
*** pmannidi|AFK is now known as pmannidi | 02:52 | |
*** pmannidi is now known as pmannidi|brb | 04:11 | |
*** pmannidi|brb is now known as pmannidi | 06:08 | |
digitalsimboja | Hi ltomasbo | 07:29 |
---|---|---|
digitalsimboja | Let me bring the house to speed | 07:29 |
ltomasbo | the TLDR is, why the new test is being executed here if the config.py is setting it to false: https://review.opendev.org/c/openstack/kuryr-tempest-plugin/+/803244 | 07:30 |
ltomasbo | gryf, ^ | 07:30 |
digitalsimboja | Yes on Kuryr-kubernetes master, the reconciliation is set to True according to this: https://review.opendev.org/c/openstack/kuryr-kubernetes/+/805033 | 07:49 |
gryf | ltomasbo, hmmmmmm. perhaps the reason is this setting is set to True in here https://review.opendev.org/c/openstack/kuryr-tempest-plugin/+/803244/58/kuryr_tempest_plugin/tests/scenario/test_service.py#188 | 07:49 |
digitalsimboja | gryf, if I set it to False there, the test will be skipped locally | 07:50 |
digitalsimboja | on my local deployment | 07:50 |
digitalsimboja | I mean | 07:50 |
gryf | digitalsimboja, in your local deployment all you don't need to do anything extra, since here https://review.opendev.org/c/openstack/kuryr-kubernetes/+/805033/8/devstack/lib/kuryr_kubernetes#1520 it would be set. | 07:51 |
gryf | on master branch. | 07:51 |
gryf | after this patch would be merged, | 07:51 |
gryf | of course. | 07:52 |
digitalsimboja | Ohh!! I see... | 07:52 |
opendevreview | Sunday Mgbogu proposed openstack/kuryr-tempest-plugin master: Add Kuryr-tempest-plugin test for LoadBalancer Reconciliation https://review.opendev.org/c/openstack/kuryr-tempest-plugin/+/803244 | 07:53 |
ltomasbo | ohh, I did not see that line forcing it to True! | 07:54 |
ltomasbo | that is why, that should be removed, indeed | 07:54 |
digitalsimboja | done already... | 07:54 |
digitalsimboja | Thanks | 07:54 |
ltomasbo | and this patch should include a depends on the kuryr-tempest one (https://review.opendev.org/c/openstack/kuryr-kubernetes/+/805033/) | 07:54 |
gryf | right. | 07:55 |
ltomasbo | so that the test is tested against master | 07:55 |
digitalsimboja | fixing that asa... | 07:55 |
opendevreview | Sunday Mgbogu proposed openstack/kuryr-tempest-plugin master: Add Kuryr-tempest-plugin test for LoadBalancer Reconciliation https://review.opendev.org/c/openstack/kuryr-tempest-plugin/+/803244 | 07:57 |
opendevreview | Itzik Brown proposed openstack/kuryr-tempest-plugin master: Check LB members when scaling a deployment https://review.opendev.org/c/openstack/kuryr-tempest-plugin/+/806124 | 08:40 |
opendevreview | Roman Dobosz proposed openstack/kuryr-kubernetes master: Switch gates to OVN by default. https://review.opendev.org/c/openstack/kuryr-kubernetes/+/802999 | 10:27 |
opendevreview | Roman Dobosz proposed openstack/kuryr-kubernetes master: Switch gates to OVN by default. https://review.opendev.org/c/openstack/kuryr-kubernetes/+/802999 | 10:41 |
*** dmellado_ is now known as dmellado | 10:46 | |
digitalsimboja | ltomasbo, maysams: We are good now on zuul | 11:47 |
maysams | checking | 11:49 |
ltomasbo | digitalsimboja, not yet, if you don't add the dependency on your other patch, then the code is not tested | 11:49 |
digitalsimboja | You mean this: https://review.opendev.org/c/openstack/kuryr-tempest-plugin/+/803244 | 11:50 |
digitalsimboja | Both would have to depend on each other? | 11:51 |
ltomasbo | no, I mean the kuryr-kubernetes patch | 11:52 |
ltomasbo | should have a depends on the kuryr-tempest-plugin one | 11:52 |
ltomasbo | so that the test is executed | 11:52 |
digitalsimboja | This : https://review.opendev.org/c/openstack/kuryr-tempest-plugin/+/803244 depends on this: https://review.opendev.org/c/openstack/kuryr-kubernetes/+/805033 | 11:54 |
digitalsimboja | should I switch it | 11:54 |
ltomasbo | ahh, you added it the other way around... and did the test execute? | 11:55 |
digitalsimboja | sure yeah they did run | 11:55 |
digitalsimboja | check the zuul output | 11:55 |
ltomasbo | ok | 11:55 |
ltomasbo | then I don't care much | 11:56 |
ltomasbo | lets merge the other one first | 11:56 |
ltomasbo | unless maysams gryf have other suggestion/opinion | 11:56 |
digitalsimboja | Perfect | 11:56 |
digitalsimboja | yeah we can await their thoughts though gryf has given his nod on the kuryr-kubernetes patch | 11:57 |
maysams | ltomasbo: yup, it passed https://storage.bhs.cloud.ovh.net/v1/AUTH_dcaab5e32b234d56b626f72581e3644c/zuul_opendev_logs_ac3/803244/60/check/kuryr-kubernetes-tempest-ovn/ac378cd/testr_results.html | 11:58 |
maysams | digitalsimboja: I just added a comment on the patch, please take a look | 11:58 |
digitalsimboja | ok | 11:59 |
opendevreview | Sunday Mgbogu proposed openstack/kuryr-tempest-plugin master: Add Kuryr-tempest-plugin test for LoadBalancer Reconciliation https://review.opendev.org/c/openstack/kuryr-tempest-plugin/+/803244 | 12:04 |
opendevreview | Roman Dobosz proposed openstack/kuryr-kubernetes master: Switch gates to OVN by default. https://review.opendev.org/c/openstack/kuryr-kubernetes/+/802999 | 12:46 |
opendevreview | Merged openstack/kuryr-kubernetes master: Enable reconciliation by default https://review.opendev.org/c/openstack/kuryr-kubernetes/+/805033 | 14:21 |
digitalsimboja | gryf, ltomasbo, maysams please advise here: https://review.opendev.org/c/openstack/kuryr-tempest-plugin/+/803244/61/kuryr_tempest_plugin/tests/scenario/test_service.py#248 | 14:27 |
digitalsimboja | If I remove the if statement and user assertNotEqual, how would I break the loop then? | 14:27 |
gryf | digitalsimboja, I've added the idea I've got, with some remark - please check and let me know if that make sense, or is there a need to have all condition parts fulfilled. | 17:11 |
digitalsimboja | Not really, once new_lb_id is None or does not exist the execution should continue as you stated | 17:20 |
opendevreview | Sunday Mgbogu proposed openstack/kuryr-tempest-plugin master: Add Kuryr-tempest-plugin test for LoadBalancer Reconciliation https://review.opendev.org/c/openstack/kuryr-tempest-plugin/+/803244 | 17:25 |
gryf | digitalsimboja, ok, so the not-exists exception, or the None if returned is the indication, that process of reconciliation is in progress, right? | 17:29 |
gryf | although, current logic is this: if there is no exception, getting id is returning something else than None AND returned ID differ from the one which crd holds, than log will kick in, otherwise it will loop again. | 17:31 |
Generated by irclog2html.py 2.17.2 by Marius Gedminas - find it at https://mg.pov.lt/irclog2html/!