Thursday, 2021-09-09

*** pmannidi is now known as pmannidi|AFK01:12
*** pmannidi|AFK is now known as pmannidi02:52
*** pmannidi is now known as pmannidi|brb04:11
*** pmannidi|brb is now known as pmannidi06:08
digitalsimbojaHi ltomasbo07:29
digitalsimbojaLet me bring the house to speed07:29
ltomasbothe 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/+/80324407:30
ltomasbogryf, ^07:30
digitalsimbojaYes on Kuryr-kubernetes master, the reconciliation is set to True according to this: https://review.opendev.org/c/openstack/kuryr-kubernetes/+/80503307:49
gryfltomasbo, 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#18807:49
digitalsimbojagryf, if I set it to False there, the test will be skipped locally07:50
digitalsimbojaon my local deployment07:50
digitalsimbojaI mean07:50
gryfdigitalsimboja, 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
gryfon master branch.07:51
gryfafter this patch would be merged,07:51
gryfof course.07:52
digitalsimbojaOhh!! I see...07:52
opendevreviewSunday Mgbogu proposed openstack/kuryr-tempest-plugin master: Add Kuryr-tempest-plugin test for LoadBalancer Reconciliation  https://review.opendev.org/c/openstack/kuryr-tempest-plugin/+/80324407:53
ltomasboohh, I did not see that line forcing it to True!07:54
ltomasbothat is why, that should be removed, indeed07:54
digitalsimbojadone already...07:54
digitalsimbojaThanks07:54
ltomasboand this patch should include a depends on the kuryr-tempest one (https://review.opendev.org/c/openstack/kuryr-kubernetes/+/805033/)07:54
gryfright.07:55
ltomasboso that the test is tested against master07:55
digitalsimbojafixing that asa...07:55
opendevreviewSunday Mgbogu proposed openstack/kuryr-tempest-plugin master: Add Kuryr-tempest-plugin test for LoadBalancer Reconciliation  https://review.opendev.org/c/openstack/kuryr-tempest-plugin/+/80324407:57
opendevreviewItzik Brown proposed openstack/kuryr-tempest-plugin master: Check LB members when scaling a deployment  https://review.opendev.org/c/openstack/kuryr-tempest-plugin/+/80612408:40
opendevreviewRoman Dobosz proposed openstack/kuryr-kubernetes master: Switch gates to OVN by default.  https://review.opendev.org/c/openstack/kuryr-kubernetes/+/80299910:27
opendevreviewRoman Dobosz proposed openstack/kuryr-kubernetes master: Switch gates to OVN by default.  https://review.opendev.org/c/openstack/kuryr-kubernetes/+/80299910:41
*** dmellado_ is now known as dmellado10:46
digitalsimbojaltomasbo, maysams: We are good now on zuul11:47
maysamschecking11:49
ltomasbodigitalsimboja, not yet, if you don't add the dependency on your other patch, then the code is not tested11:49
digitalsimbojaYou mean this: https://review.opendev.org/c/openstack/kuryr-tempest-plugin/+/80324411:50
digitalsimbojaBoth would have to depend on each other?11:51
ltomasbono, I mean the kuryr-kubernetes patch11:52
ltomasboshould have a depends on the kuryr-tempest-plugin one11:52
ltomasboso that the test is executed11:52
digitalsimbojaThis : https://review.opendev.org/c/openstack/kuryr-tempest-plugin/+/803244 depends on this: https://review.opendev.org/c/openstack/kuryr-kubernetes/+/80503311:54
digitalsimbojashould I switch it11:54
ltomasboahh, you added it the other way around... and did the test execute?11:55
digitalsimbojasure yeah they did run11:55
digitalsimbojacheck the zuul output11:55
ltomasbook11:55
ltomasbothen I don't care much11:56
ltomasbolets merge the other one first11:56
ltomasbounless maysams gryf have other suggestion/opinion11:56
digitalsimbojaPerfect11:56
digitalsimbojayeah we can await their thoughts though gryf has given his nod on the kuryr-kubernetes patch11:57
maysamsltomasbo: 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.html11:58
maysamsdigitalsimboja: I just added a comment on the patch, please take a look11:58
digitalsimbojaok11:59
opendevreviewSunday Mgbogu proposed openstack/kuryr-tempest-plugin master: Add Kuryr-tempest-plugin test for LoadBalancer Reconciliation  https://review.opendev.org/c/openstack/kuryr-tempest-plugin/+/80324412:04
opendevreviewRoman Dobosz proposed openstack/kuryr-kubernetes master: Switch gates to OVN by default.  https://review.opendev.org/c/openstack/kuryr-kubernetes/+/80299912:46
opendevreviewMerged openstack/kuryr-kubernetes master: Enable reconciliation by default  https://review.opendev.org/c/openstack/kuryr-kubernetes/+/80503314:21
digitalsimbojagryf, ltomasbo, maysams please advise here: https://review.opendev.org/c/openstack/kuryr-tempest-plugin/+/803244/61/kuryr_tempest_plugin/tests/scenario/test_service.py#24814:27
digitalsimbojaIf I remove the if statement and user assertNotEqual, how would I break the loop then?14:27
gryfdigitalsimboja, 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
digitalsimbojaNot really, once new_lb_id is None or does not exist the execution should continue as you stated17:20
opendevreviewSunday Mgbogu proposed openstack/kuryr-tempest-plugin master: Add Kuryr-tempest-plugin test for LoadBalancer Reconciliation  https://review.opendev.org/c/openstack/kuryr-tempest-plugin/+/80324417:25
gryfdigitalsimboja, ok, so the not-exists exception, or the None if returned is the indication, that process of reconciliation is in progress, right?17:29
gryfalthough, 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/!