Thursday, 2022-01-20

kata-irc-bot<eric.ernst> Usually if you either add a new file, or make significant additions  to a file.00:45
kata-irc-bot<feng.wang> I’m getting a bunch of fieldalignment error from the `src/runtime/pkg/govmm/qemu` pkg in the PR static check, which is merged in today. @fidencio01:11
kata-irc-bot<eric.ernst> that lint is a serious pain01:26
kata-irc-bot<eric.ernst> overoptimize for sake of readability and dev suffering imo01:26
kata-irc-bot<feng.wang> Completely agree:,)01:41
kata-irc-bot<bergwolf> Also we should check why it passed CI in the offending PR where CI is supposed to block it during reviewing instead of after it is merged.02:30
kata-irc-bot<bergwolf> FWIW, I created https://github.com/kata-containers/tests/issues/4388 to track the CI issue itself.02:35
kata-irc-bot<eric.ernst> I'm seeing static checks failing unexpectedly here too: https://github.com/kata-containers/kata-containers/pull/294102:48
kata-irc-bot<eric.ernst> Ie,  ‘’’ 20T01:28:21.951721646Z" level=info msg="Checking file" commit=ce03e521b01a4ad14f91ce9118c6e6de16077839 file=tools/packaging/README.md name=kata-check-markdown pid=4061722 source=check-markdown version=0.0.1 01:28:21 time="2022-01-20T01:28:21.955502462Z" level=fatal msg="found 1 parse error:\nstat tools/packaging/cmd/kata-pkgsync: no such file or directory"  ‘’’02:54
kata-irc-bot<bergwolf> Not sure what the tool does, but it was removed by https://github.com/kata-containers/kata-containers/pull/349402:58
kata-irc-bot<fidencio> So, this is happening because we merged GoVmm into the Kata Containers repo.07:44
kata-irc-bot<fidencio> https://github.com/kata-containers/kata-containers/pull/3496 is the PR fixing that07:45
kata-irc-bot<fidencio> Which was raised a few minutes after that one was merged07:45
kata-irc-bot<fidencio> And https://github.com/kata-containers/kata-containers/issues/3500 is the one tracking all the issues found on s390x07:46
kata-irc-bot<fidencio> I am sorry I ended up causing a breakage of a day or so for the folks involved in the project07:46
kata-irc-bot<fidencio> @eric.ernst, have you tried rebasing your PR atop of `main`?08:30
kata-irc-bot<fidencio> Okay, I just checked with https://github.com/kata-containers/kata-containers/runs/4879521064?check_suite_focus=true and static-checks seem to be passing, and you need a rebase on your side, Eric.09:36
kata-irc-bot<fidencio> Folks, if someone is up for reviewing something, https://github.com/kata-containers/kata-containers/pull/3503 is a simple and good candidate for that, and that would allow me to proceed with kata-deploy tests10:53
gmaglioneHi, Kata dev team. I've a question, currently kata runs virtiofsd without 'announce-submounts' option, Is there any particular reason for this? thanks12:45
kata-irc-bot<fidencio> Hey @gmaglion,  No, no specific reason as far as I can remember. Is "announce-submounts" becoming a default for the rust version of the daemon?14:05
kata-irc-bot<gmaglion> Not yet, but that is what we are discussing now. The thing is that could have implications in how sync()/syn14:16
kata-irc-bot<gmaglion> And this could have a performance impact14:19
kata-irc-bot<fidencio> It's worth an issue opened on https://github.com/kata-containers/kata-containers14:20
kata-irc-bot<fidencio> Robert Krawitz (@rlk), from the Red Hat side was the person working on the performance side and could help with that.14:21
kata-irc-bot<fidencio> @jose.carlos.venegas.m may also be interested on this from Intel side14:22
kata-irc-bot<fidencio> Having as much detail about the performance penalty would be interesting.14:22
kata-irc-bot<rlk> @fidencio I'm not familiar with this functionality.14:22
kata-irc-bot<gmaglion> @fidencio Thanks The possible performance implication is due to a proposed patch set (not already merged). Basically, the proposed solution will sync() on all the submounts if the 'announce-submount' is not present14:26
kata-irc-bot<rlk> I'm not familiar enough with this to understand the implications.  When exactly will sync() be done?  How frequent would it be?14:27
kata-irc-bot<gmaglion> the sync will be done in the guest, how often will depend on the use case.14:30
kata-irc-bot<rlk> Could you describe the exact scenario, please?14:30
kata-irc-bot<fidencio> By the way, I should have expressed myself better here.  Saying Robert can help was an extrapolation I wrongly did. :slightly_smiling_face:  What I wanted to say was mostly that Robert worked on performance evaluation in the past.14:38
kata-irc-bot<gmaglion> I was talking with my team. The syncfs is still in development (sorry for the noise) is not an issue yet.14:46
kata-irc-bot<gmaglion> But, since (correct me if I'm wrong) kata is mounting different fs in a shared dir, so it's possible that two files end up having the same inode number and that will broke som tools like find.14:48
kata-irc-bot<gmaglion> the 'announce-submounts' will take care of the inode number collisions14:49
kata-irc-bot<jakob.naucke> I'm feeling close to my wit's end with the leftover shim processes being in k8s on s390x (qemu & virtiofsd exit, containerd-shim-kata-v2 doesn't). I can't seem to isolate this happening to one condition (some vague steps _seemed_ to do something, but I can't say for sure, like rebuilding the runtime with (apparently?) the same toolchain? reinstantiating k8s?). @eric.ernst may I borrow your k8s/shim expertise -- can you think of a15:04
kata-irc-botcondition where this would happen? See :thread: for backtrace on one of these processes... Also, how should we proceed with the s390x CI (I've currently set the node to offline)? Should we skip the leftover shim processes check for now?15:04
kata-irc-bot<jakob.naucke> > The downside of this is that we run the test with the last merged PR, rather than with the "to-be-approved" PR, but that's a limitation we've always had. Really? :face_with_raised_eyebrow: I didn't know that. Why?15:52
kata-irc-bot<fidencio> Historically `/test_kata_deploy` was made to test the deploy action, not the binaries used.  So, for a long time what we actually did was testing with a known-to-work tarball, just to be sure we didn't break the scripts.15:54
kata-irc-bot<fidencio> That turned out to be a terrible idea, as we hit issues from left and right.15:54
kata-irc-bot<fidencio> Then, at the end, I tried to make it work using the `checkout`  action, which ... to be fair, I think can be used to test the *current* PR, but that won't checkout to any different branch than the `main` one15:55
kata-irc-bot<fidencio> So, when called from the stable branch, that would end up testing the main ... and that's a limitation from the `issue_comment` event15:55
kata-irc-bot<fidencio> Then I switched to this other action, which doesn't seem to work with PRs coming from outside the repo, and here we are.15:56
kata-irc-bot<fidencio> Messy, right?15:56
kata-irc-bot<jakob.naucke> ah yes, I knew about the "default branch" limitation. Messy, but makes sense.15:58
kata-irc-bot<feng.wang> I’m getting another error in the static check. ```time="2022-01-20T17:06:06.978906757Z" level=fatal msg="found 1 parse error:\nstat tools/packaging/cmd/kata-pkgsync: no such file or directory" commit=7244d8c983c024b5ff5b2b64193017d2e4212d9e name=kata-check-markdown pid=28206 source=check-markdown version=0.0.1 make: *** [Makefile:40: static-checks] Error 1 Error: Process completed with exit code 2.``` `cmd/kata-pkgsync` doesn’t exi17:31
kata-irc-botin the upstream main branch though. I only see it in a different branch (https://github.com/kata-containers/kata-containers/tree/0917addea77117d09f4ebbcfbe31bcb035dfc0c1/tools/packaging/cmd/kata-pkgsync).17:31
kata-irc-bot<fidencio> Can you rebase your code atop of `main`?17:33
kata-irc-bot<fidencio> https://katacontainers.slack.com/archives/C879ACQ00/p1642671414067000?thread_ts=1642641062.061900&cid=C879ACQ0017:33
kata-irc-bot<fidencio> This may be more a limitation of the `checkout` action than something actually on us.17:33
kata-irc-bot<eric.ernst> I rebased and it did not resolve for me @fidencio17:34
kata-irc-bot<eric.ernst> Was there a change in static checks recently?17:35
kata-irc-bot<feng.wang> I rebased this morning. I can try again.17:35
kata-irc-bot<eric.ernst> agggh, fud.17:36
kata-irc-bot<eric.ernst> ctrl-R’d the GH page.17:36
kata-irc-bot<eric.ernst> ignore me — updated / rebase tests are still running.17:36
kata-irc-bot<eric.ernst> mea culpa.17:36
kata-irc-bot<fidencio> @feng.wang, what's your PR number?17:37
kata-irc-bot<feng.wang> https://github.com/kata-containers/kata-containers/pull/340617:38
kata-irc-bot<feng.wang> I just rebased again.17:38
kata-irc-bot<eric.ernst> ah, indeed: https://github.com/kata-containers/kata-containers/pull/294117:40
kata-irc-bot<eric.ernst> mine is failing still it seems17:40
kata-irc-bot<fidencio> Oh, that's interesting!17:42
kata-irc-bot<fidencio> Cool, then we may have a real bug to hunt.17:45
kata-irc-bot<eric.ernst> I can also fix the issue static-checker is finding, but, yeah…17:45
kata-irc-bot<eric.ernst> I didn’t see any fishy commits to ./ci17:46
kata-irc-bot<fidencio> I found the issue17:49
kata-irc-bot<fidencio> I still don't understand how and why we didn't hit it before17:49
kata-irc-bot<fidencio> https://github.com/kata-containers/kata-containers/pull/351517:52
kata-irc-bot<fidencio> do your PRs touch any of the files touched on this PR?17:52
kata-irc-bot<feng.wang> No. It doesn’t.17:53
kata-irc-bot<fidencio> I wonder what caused some PRs to hit that issue, and some to just be fine17:53
kata-irc-bot<fidencio> And I closed mine in favour of https://github.com/kata-containers/kata-containers/pull/351417:58
kata-irc-bot<eric.ernst> Hmm, haww, hmph.21:14
kata-irc-bot<eric.ernst> have we recently updated: • the versions in the Dockerfile for aks cli? • how we changed the version of k8s/containerd that we install as part of the cluster configuration?21:15
kata-irc-bot<eric.ernst> Are you able to reproduce that issue when bringing up an AKS cluster manually/on your own?21:16
kata-irc-bot<fidencio> Last update on the Dockerfile for AKS cli was done by you, IIRC, that one a long time ago.21:16
kata-irc-bot<fidencio> (and that was the one that made things reasonably stable for some time)21:16
kata-irc-bot<fidencio> I can deploy a cluster a give it a try Tomorrow21:17
kata-irc-bot<fidencio> And that has helped us immensely to actually catch issues related to that (twice, at least)21:18
kata-irc-bot<eric.ernst> interesting. Gosh, CRI should be pretty stable — I would think updating wouldn’t miss issues, but - :shrug: yeah.21:20
kata-irc-bot<eric.ernst> I think the problem you’ll run into is we’re testing on versions that aren’t supported any longer in AKS.21:20
kata-irc-bot<eric.ernst> (ie, 1.20 I think just got its last update, and is almost EOL)21:21
kata-irc-bot<fidencio> I am fine about switching to 1.21 as soon as 1.20 reaches its EOL21:21
kata-irc-bot<fidencio> At least for containerd, as that's what used to most and where we faced more regressions21:22
kata-irc-bot<eric.ernst> Gut reaction is go to 1.23 why give ourselves more burden, but that’s based on not having data myself on catching issues on 1.21 that wouldn’t exist on 1.2321:22
kata-irc-bot<fidencio> We faced issues related to cri-option, you remember?21:23
kata-irc-bot<eric.ernst> my brain doesn’t hold things well :|. Was that K8S or a cri runtime21:24
kata-irc-bot<fidencio> Let me find it here21:24
kata-irc-bot<fidencio> https://github.com/kata-containers/kata-containers/pull/225721:27
kata-irc-bot<eric.ernst> Yeah, that was fun because of containerd, not k8s.21:29
kata-irc-bot<eric.ernst> either way though, I see your point.21:30
kata-irc-bot<fidencio> Oh, I was the whole time thinking about containerd21:31
kata-irc-bot<fidencio> That's something CRI-O does, but containerd doesn't.21:32
kata-irc-bot<fidencio> Let me do some tests Tomorrow and bump the whole thing to run on 1.2321:32
kata-irc-bot<eric.ernst> let me know if you run into any issues — i’m overdue for my once every 6 months dealing with AKS :)22:25

Generated by irclog2html.py 2.17.3 by Marius Gedminas - find it at https://mg.pov.lt/irclog2html/!