| *** auk has joined #kata-dev | 01:50 | |
| *** dklyle has joined #kata-dev | 03:24 | |
| *** dklyle has quit IRC | 04:00 | |
| *** auk has quit IRC | 04:37 | |
| *** jodh has joined #kata-dev | 07:45 | |
| *** sgarzare has joined #kata-dev | 07:47 | |
| *** fgiudici has joined #kata-dev | 08:20 | |
| *** Yarboa has quit IRC | 08:47 | |
| *** davidgiluk has joined #kata-dev | 09:05 | |
| *** Yarboa has joined #kata-dev | 09:21 | |
| *** sameo has joined #kata-dev | 09:37 | |
| fidencio | jodh: morning / ping. May I bother you for a review on https://github.com/kata-containers/tests/pull/3032? I'd like to have it in ASAP, in order to unblock other people facing issues with those tests | 10:06 |
|---|---|---|
| jodh | fidencio: hi - done. | 10:09 |
| fidencio | jodh: thanks a lot! | 10:09 |
| *** Yarboa has quit IRC | 10:19 | |
| *** Yarboa has joined #kata-dev | 10:20 | |
| *** sameo has quit IRC | 10:55 | |
| *** sameo has joined #kata-dev | 11:18 | |
| kata-irc-bot | <fidencio> @bergwolf, may I open a PR reverting ff13bde3c1bd03b5ed0c93e7ea315befd24a75b7 ? | 12:28 |
| kata-irc-bot | <fidencio> I understand we should not block development, but we should treat a revert in a more clear way. Took me a 48 hours to have it solved, I guess it's a fair request for us to wait those 48 hours before reverting something, mainly without a clear understanding of why was causing the regression. Obviously, if the person (me in this case) wouldn't be responsive, then go ahead and revert, but let's try to give this grace period before doing | 12:48 |
| kata-irc-bot | so. | 12:48 |
| *** Yarboa has quit IRC | 12:49 | |
| *** Yarboa has joined #kata-dev | 12:51 | |
| *** sameo has quit IRC | 13:05 | |
| *** devimc has joined #kata-dev | 13:10 | |
| *** sameo has joined #kata-dev | 13:18 | |
| *** fuentess has joined #kata-dev | 14:01 | |
| kata-irc-bot | <bergwolf> hi, my understanding is that the offending commit was merged with the specific test failed but ignored, and it is causing a lot of failures preventing other PRs from being properly tested. So IMO it should be reverted quickly and have the root cause resolved properly before getting merged again. | 14:14 |
| kata-irc-bot | <bergwolf> In which case, I don’t think https://github.com/kata-containers/tests/pull/3032 is a proper solution at all. We have all these tests passing w/ v1.8.3. Now we have identified that it is a CRIO v1.8.4 regression, we should wait for CRIO to fix it before upgrading CRIO dependency. | 14:16 |
| kata-irc-bot | <fidencio> That test was flaky. Sometimes it passed, sometimes it didn't. | 14:16 |
| kata-irc-bot | <fidencio> I would really prefer having the test disabled instead of a whole downgrade of the CRI-O version. | 14:17 |
| kata-irc-bot | <bergwolf> fwics, it was failing a lot w/ v1.8.4 | 14:18 |
| *** Yarboa has quit IRC | 14:19 | |
| kata-irc-bot | <bergwolf> Are there specific reasons/issues that you want to fix by upgrading to v1.8.4? | 14:19 |
| kata-irc-bot | <fidencio> Not 100% of the times, though. That's the reason I'm asking you, as someone also working on CRI-O in order to have it properly fixed there, to just disable the test and stick to the latest one. Latest one has fixes related to pods not being terminated. | 14:20 |
| kata-irc-bot | <fidencio> It has fixes related to logs being properly stored and retrieved | 14:21 |
| *** Yarboa has joined #kata-dev | 14:21 | |
| kata-irc-bot | <fidencio> What I'd like to see, sincerely, is if some issue involving CRI-O is going on, just open an issue and ping folks from Red Hat. We're here to help, sinceely, and then we could debug the issue and figure out what's going on before reverting the patch. | 14:23 |
| kata-irc-bot | <fidencio> That was exactly the approach taken when I figured out that Liu Bin's patch was the one causing the regression on CRI-O. I didn't revert the patch, but rather talked to him and we got to find a solution together. | 14:24 |
| kata-irc-bot | <bergwolf> sorry about that. I was in a hurry as the issue is blocking a cve fix | 14:25 |
| kata-irc-bot | <bergwolf> which we should merge asap but got blocked several days by unrelated issues already | 14:25 |
| kata-irc-bot | <fidencio> I get that, that happens. I'm mentioning this here mainly because: 1. By doing this (regardless of being CRI-O or not), we put some pressure on the back of the developers working on the failing piece; instead of our own back; 2. We can avoid be in such situation in the future. *Please*, don't take this as I'm criticizing what you did. You had to have the patch merged, other people reviewed the changes, that's fine. | 14:27 |
| kata-irc-bot | <bergwolf> however I still think that commit should be reverted because it is causing a lot of failures in CI, not just the CVE one | 14:28 |
| kata-irc-bot | <fidencio> I have a run 100% green here: https://github.com/kata-containers/kata-containers/pull/1093 | 14:29 |
| kata-irc-bot | <fidencio> What I'd prefer is, if we find issues we file the issues and we take care of the issues. | 14:29 |
| kata-irc-bot | <fidencio> Instead of reverting the patch without investigating the issues. | 14:30 |
| kata-irc-bot | <fidencio> Does this sound reasonable? | 14:30 |
| kata-irc-bot | <bergwolf> that’s because you have disabled the test, isn’t it? | 14:30 |
| kata-irc-bot | <fidencio> I have disabled that test, and I have fixed the issue on CRI-O | 14:31 |
| kata-irc-bot | <bergwolf> then why not enable the test and depend on the correct CRIO version? | 14:32 |
| kata-irc-bot | <fidencio> And I will re-enable that test as soon as v1.18.5 is out (if not when the commit is merged, if we could use a specific commit) | 14:32 |
| kata-irc-bot | <bergwolf> ok, I thought we support commit based dependency. but you can decide when to upgrade | 14:32 |
| kata-irc-bot | <fidencio> If we support, then we upgrade cri-o as soon as the commit gets merged | 14:33 |
| kata-irc-bot | <fidencio> IMHO, it's way better to test with the latest official release of the package than with an old specific version. | 14:33 |
| kata-irc-bot | <bergwolf> ATM I don’t object reverting ff13bde3c1bd03b5ed0c93e7ea315befd24a75b7. but we should really take more caution on merging PRs with failed CIs. at least some examination should be done. In this case, the failing test did show up in the original PR but it was merged anyway. Then it caused a lot of CI failures preventing most PRs from being tested/merged. | 14:34 |
| kata-irc-bot | <fidencio> I don't disagree with you, any bit. | 14:35 |
| kata-irc-bot | <fidencio> We should *not* disable tests to bump CRI-O versions. | 14:35 |
| kata-irc-bot | <bergwolf> +1 | 14:35 |
| kata-irc-bot | <fidencio> But if we find a regression after the bump happened, I'd prefer to skip the test shortly, so we can properly investigate the issue and then re-enable once it gets fixed on the faulty component. | 14:36 |
| kata-irc-bot | <fidencio> Do we have an agreement about those two things? | 14:36 |
| kata-irc-bot | <bergwolf> +1, unless there are multiple regressions and I’d prefer to revert directly. This time it was not this case though. | 14:39 |
| kata-irc-bot | <fidencio> Cool, I'm glad we have an agreement. Thanks a lot for the talk, man, very much appreciated! | 14:40 |
| kata-irc-bot | <bergwolf> And I want to add one more thing that we should investigate failed CIs before merging PRs. We should re-enable the fedora CI as `required` , it was disabled as the fedora source was not stable for some time | 14:40 |
| kata-irc-bot | <fidencio> Yep, that's a must! | 14:41 |
| kata-irc-bot | <fidencio> I cannot disagree on that. | 14:41 |
| *** Yarboa has quit IRC | 14:54 | |
| *** crobinso has joined #kata-dev | 14:59 | |
| *** Yarboa has joined #kata-dev | 15:01 | |
| *** devimc has quit IRC | 15:15 | |
| *** Yarboa has quit IRC | 15:29 | |
| *** Yarboa has joined #kata-dev | 15:39 | |
| *** dklyle has joined #kata-dev | 16:02 | |
| *** devimc has joined #kata-dev | 16:39 | |
| *** Yarboa has quit IRC | 17:39 | |
| *** Yarboa has joined #kata-dev | 17:40 | |
| *** crobinso has quit IRC | 17:50 | |
| *** jodh has quit IRC | 18:02 | |
| *** fgiudici has quit IRC | 18:08 | |
| *** Yarboa has quit IRC | 18:49 | |
| *** Yarboa has joined #kata-dev | 18:52 | |
| *** sgarzare has quit IRC | 19:01 | |
| *** Yarboa has quit IRC | 19:09 | |
| *** Yarboa has joined #kata-dev | 19:10 | |
| *** davidgiluk has quit IRC | 19:59 | |
| *** Yarboa has quit IRC | 20:49 | |
| *** Yarboa has joined #kata-dev | 20:51 | |
| *** fuentess has quit IRC | 21:14 | |
| *** fuentess has joined #kata-dev | 21:32 | |
| *** devimc has quit IRC | 21:52 | |
| *** Yarboa has quit IRC | 23:40 | |
| *** Yarboa has joined #kata-dev | 23:40 | |
Generated by irclog2html.py 2.17.2 by Marius Gedminas - find it at https://mg.pov.lt/irclog2html/!