| *** polls45 has quit IRC | 00:00 | |
| ianw | hrm, arm64 nodepool testing sort of works; it gets to the end of devstack at least ,but it seems to be trying to run the x86 container | 00:01 |
|---|---|---|
| ianw | https://zuul.opendev.org/t/zuul/build/2a38b5ccc73846ce85d9cbbdb4ab5cca/log/docker/nodepool_nodepool-launcher_1.txt | 00:01 |
| clarkb | ianw: do we build the launcher with arm64 too? | 00:01 |
| ianw | i think both, but the builder is the same | 00:02 |
| ianw | https://zuul.opendev.org/t/zuul/build/2a38b5ccc73846ce85d9cbbdb4ab5cca/log/docker/nodepool_nodepool-builder_1.txt | 00:02 |
| ianw | it must have pulled the wrong image from the intermediate registry? | 00:02 |
| corvus | pabelanger: probably so? i think someone will just need to double check the release notes / git log and make sure there aren't any gotchas. but i don't think it's in a holding pattern like zuul. | 00:04 |
| *** rlandy|rover has quit IRC | 00:07 | |
| openstackgerrit | Ian Wienand proposed zuul/nodepool master: [dnm] arm64 testing test https://review.opendev.org/762774 | 00:09 |
| ianw | ^ my fault; the siblings image builds, that pull in dib/openstacksdk/etc from source, weren't doing multiarch builds | 00:09 |
| ianw | i feel like there's a decent chance this will work. devstack support is held up on full tempest runs working, but i should be enough to build/upload/boot/basic networking | 00:10 |
| ianw | anyway, what i *really* want to validate is the containerfile builds | 00:12 |
| ianw | i have them working for ubuntu everything and fedora 33 | 00:13 |
| ianw | on x86, i can't imagine they won't work on arm64 but never know | 00:13 |
| clarkb | probably the biggest arm64 issue is if docker hub has arm64 images for those platforms | 00:14 |
| ianw | clarkb: not now, but do you have any interest on working on similar for suse? | 00:14 |
| clarkb | ya I can probably help poke at that | 00:14 |
| clarkb | assuming there exists an opensuse leap arm64 docker image | 00:14 |
| ianw | fedora era rpm and zypper are probably the most problematic things running non-native | 00:15 |
| ianw | clarkb: we don't have arm64 suse atm? so for that we'd really only need validate x86 | 00:15 |
| clarkb | oh I see | 00:15 |
| clarkb | ya I can help with that | 00:15 |
| clarkb | they do publsih an arm64 image though so that is also fine if we want to try that | 00:16 |
| ianw | yeah, if i can get gate build/boot testing for arm64, that would be awesome to validate it. so far we just mostly hope things work there, with a modest degree of success :) | 00:17 |
| openstackgerrit | Merged zuul/zuul master: Allow to reuse the code handling the branch cache https://review.opendev.org/756725 | 00:24 |
| openstackgerrit | Merged zuul/zuul master: JS: Don't run tests in watch mode https://review.opendev.org/756662 | 00:24 |
| *** Goneri has quit IRC | 00:36 | |
| *** vishalmanchanda has quit IRC | 00:41 | |
| *** hamalq has quit IRC | 00:41 | |
| *** zenkuro has quit IRC | 00:59 | |
| *** y2kenny has quit IRC | 02:38 | |
| openstackgerrit | Jeremy Stanley proposed zuul/zuul-jobs master: validate-host: Options to require v4 and v6 routes https://review.opendev.org/763065 | 03:09 |
| *** bhavikdbavishi has joined #zuul | 03:10 | |
| openstackgerrit | Ian Wienand proposed zuul/nodepool master: [dnm] arm64 testing test https://review.opendev.org/762774 | 03:20 |
| *** bhavikdbavishi1 has joined #zuul | 03:37 | |
| *** bhavikdbavishi has quit IRC | 03:38 | |
| *** bhavikdbavishi1 is now known as bhavikdbavishi | 03:38 | |
| *** bhavikdbavishi has quit IRC | 04:26 | |
| *** bhavikdbavishi has joined #zuul | 04:27 | |
| ianw | thinking about it, it's probably a lot more practical to have the dib tests pull the current images, because really what we're interested in there is does the image boot, not so much testing nodepool changes | 04:41 |
| openstackgerrit | Ian Wienand proposed zuul/zuul master: web: make build/buildset table details more clickable https://review.opendev.org/762588 | 05:27 |
| openstackgerrit | Ian Wienand proposed zuul/zuul master: web: remove search icon from build/buildset page https://review.opendev.org/763126 | 05:27 |
| *** evrardjp has quit IRC | 05:33 | |
| *** evrardjp has joined #zuul | 05:33 | |
| *** vishalmanchanda has joined #zuul | 06:00 | |
| *** bhavikdbavishi1 has joined #zuul | 06:15 | |
| *** bhavikdbavishi has quit IRC | 06:17 | |
| *** bhavikdbavishi1 is now known as bhavikdbavishi | 06:17 | |
| *** saneax has joined #zuul | 06:21 | |
| *** wuchunyang has joined #zuul | 06:28 | |
| *** rpittau|afk is now known as rpittau | 06:49 | |
| *** mach1na has joined #zuul | 07:29 | |
| *** zbr4 has quit IRC | 07:38 | |
| *** bhavikdbavishi has quit IRC | 07:43 | |
| *** slaweq has joined #zuul | 07:43 | |
| *** mach1na has quit IRC | 07:47 | |
| *** hashar has joined #zuul | 07:58 | |
| *** jcapitao has joined #zuul | 07:59 | |
| *** bhavikdbavishi has joined #zuul | 08:07 | |
| *** mach1na has joined #zuul | 08:12 | |
| *** zbr has joined #zuul | 08:25 | |
| *** iurygregory has quit IRC | 08:44 | |
| *** tosky has joined #zuul | 08:45 | |
| *** iurygregory has joined #zuul | 08:50 | |
| *** jpena|off is now known as jpena | 08:55 | |
| *** zenkuro has joined #zuul | 08:57 | |
| *** slaweq has quit IRC | 09:17 | |
| *** slaweq has joined #zuul | 09:20 | |
| *** nils has joined #zuul | 09:49 | |
| *** yolanda__ has joined #zuul | 09:50 | |
| *** yoctozepto has quit IRC | 09:50 | |
| *** yoctozepto has joined #zuul | 09:51 | |
| *** mach1na has quit IRC | 10:13 | |
| *** mach1na has joined #zuul | 10:14 | |
| *** bolg has joined #zuul | 10:25 | |
| *** zenkuro has quit IRC | 10:39 | |
| *** zenkuro has joined #zuul | 10:40 | |
| *** mach1na has quit IRC | 10:40 | |
| *** zenkuro has quit IRC | 10:45 | |
| *** zenkuro has joined #zuul | 10:46 | |
| *** wuchunyang has quit IRC | 10:55 | |
| *** saneax has quit IRC | 10:57 | |
| *** yolanda__ is now known as yolanda | 11:00 | |
| *** saneax has joined #zuul | 11:04 | |
| *** bhavikdbavishi has quit IRC | 11:11 | |
| *** wuchunyang has joined #zuul | 11:27 | |
| *** jcapitao is now known as jcapitao_lunch | 11:39 | |
| *** slaweq has quit IRC | 11:39 | |
| *** slaweq has joined #zuul | 11:45 | |
| *** mach1na has joined #zuul | 11:46 | |
| *** mach1na has quit IRC | 11:58 | |
| *** bhavikdbavishi has joined #zuul | 12:01 | |
| *** wuchunyang has quit IRC | 12:03 | |
| *** bhavikdbavishi1 has joined #zuul | 12:04 | |
| *** bhavikdbavishi has quit IRC | 12:05 | |
| *** bhavikdbavishi1 is now known as bhavikdbavishi | 12:05 | |
| *** zenkuro has quit IRC | 12:19 | |
| *** zenkuro has joined #zuul | 12:24 | |
| *** mach1na has joined #zuul | 12:28 | |
| *** jpena is now known as jpena|lunch | 12:29 | |
| *** mach1na has quit IRC | 12:33 | |
| openstackgerrit | Tobias Henkel proposed zuul/zuul master: Support per branch change queues https://review.opendev.org/718531 | 12:34 |
| openstackgerrit | Tobias Henkel proposed zuul/zuul master: Move queue from pipeline to project https://review.opendev.org/720182 | 12:34 |
| openstackgerrit | Tobias Henkel proposed zuul/zuul master: Add optional support for circular dependencies https://review.opendev.org/685354 | 12:34 |
| openstackgerrit | Tobias Henkel proposed zuul/zuul master: Check cycle items are mergeable before reporting https://review.opendev.org/743450 | 12:34 |
| openstackgerrit | Tobias Henkel proposed zuul/zuul master: Make reporting asynchronous https://review.opendev.org/691253 | 12:34 |
| *** rlandy has joined #zuul | 12:54 | |
| *** rlandy is now known as rlandy|rover | 12:55 | |
| *** mach1na has joined #zuul | 12:56 | |
| *** jfoufas1 has joined #zuul | 12:59 | |
| openstackgerrit | Tobias Henkel proposed zuul/zuul master: Support per branch change queues https://review.opendev.org/718531 | 13:03 |
| openstackgerrit | Tobias Henkel proposed zuul/zuul master: Move queue from pipeline to project https://review.opendev.org/720182 | 13:03 |
| openstackgerrit | Tobias Henkel proposed zuul/zuul master: Add optional support for circular dependencies https://review.opendev.org/685354 | 13:03 |
| openstackgerrit | Tobias Henkel proposed zuul/zuul master: Check cycle items are mergeable before reporting https://review.opendev.org/743450 | 13:03 |
| openstackgerrit | Tobias Henkel proposed zuul/zuul master: Make reporting asynchronous https://review.opendev.org/691253 | 13:03 |
| *** jcapitao_lunch is now known as jcapitao | 13:16 | |
| pabelanger | zuul-maint: I'd appreciate reviews on https://review.opendev.org/763086/ to begin process of zuul-registry release to pypi. We haven't officially tagged a release yet | 13:21 |
| pabelanger | corvus: looking at release notes, it looks like we'd have to tag 3.14.0 | 13:24 |
| pabelanger | for nodepool | 13:24 |
| tristanC | pabelanger: i guess most zuul-registry deployment are using the container image, is there an issue with it? | 13:25 |
| pabelanger | tristanC: our control plane isn't setup for containers right now | 13:26 |
| pabelanger | so I would like to avoid pulling in that dependency in deployment playbooks | 13:26 |
| pabelanger | we'll likely look to move to operator in the future | 13:26 |
| tristanC | pabelanger: the zuul-operator does integrate and deploy the zuul-registry | 13:27 |
| pabelanger | cool, that should make migrating easier in the future | 13:27 |
| *** bhavikdbavishi has quit IRC | 13:27 | |
| *** Goneri has joined #zuul | 13:29 | |
| *** jpena|lunch is now known as jpena | 13:30 | |
| avass | Is there any known memory leaks in the scheduler? ours just got OOM killed after consuming 30Gb of ram | 13:30 |
| tristanC | avass: are you running 3.19.1 ? | 13:30 |
| avass | no latest containers | 13:30 |
| avass | looks like it's been increasing steadily since the last couple of days | 13:37 |
| openstackgerrit | Tobias Henkel proposed zuul/zuul master: Don't query branch protection on pull request events https://review.opendev.org/753571 | 13:41 |
| openstackgerrit | Tristan Cacqueray proposed zuul/zuul-registry master: Add hypothesis based test for the storage module https://review.opendev.org/687118 | 13:59 |
| openstackgerrit | Tristan Cacqueray proposed zuul/zuul-registry master: Add hypothesis rule based state machine test and fix path issue https://review.opendev.org/687157 | 13:59 |
| openstackgerrit | Tristan Cacqueray proposed zuul/zuul-registry master: Add hypothesis rule based state machine test and fix path issue https://review.opendev.org/687157 | 14:02 |
| openstackgerrit | Tobias Henkel proposed zuul/nodepool master: WIP: allow disabling sticky AZs https://review.opendev.org/546175 | 14:10 |
| openstackgerrit | Tobias Henkel proposed zuul/nodepool master: WIP: allow disabling sticky AZs https://review.opendev.org/546175 | 14:12 |
| tobiash | avass: when was your latest update? | 14:17 |
| tobiash | we had oom issues 1-2 months ago but I think the fixes for that all have merged already | 14:18 |
| tobiash | checking again | 14:18 |
| tobiash | avass: the fixes were https://review.opendev.org/751170, https://review.opendev.org/751171 and https://review.opendev.org/751172 | 14:20 |
| tobiash | all merged ~9 weeks ago | 14:20 |
| openstackgerrit | Merged zuul/zuul-registry master: Begin publishing to pypi https://review.opendev.org/763086 | 14:26 |
| avass | tobiash: saturday :) | 14:29 |
| tobiash | avass: hrm, that was a quick oom | 14:30 |
| avass | yep I'm trying to figure out if we're doing something strange | 14:31 |
| tobiash | avass: are you able to get a list of upstream changes that went in between that and the version before? | 14:31 |
| avass | we were running 3.19.1 before that so quit a lot of changes | 14:33 |
| tobiash | oh | 14:33 |
| tobiash | we're currently based on 3.19.0-440-g61100409 and see no unusual memory behavior | 14:33 |
| avass | we're on 3.19.1.dev441 | 14:34 |
| tobiash | which is the exact commit? | 14:34 |
| avass | I guess that's: 7b8f780d | 14:35 |
| avass | or can I check that easily somehow? | 14:36 |
| tobiash | if all components are the same version, the status page reports it | 14:36 |
| avass | yep the dashboard shows: 3.19.1.dev441 7b8f780d | 14:37 |
| avass | and those should be synced | 14:37 |
| tobiash | that's not in my zuul tree | 14:37 |
| tobiash | oh I guess that was a speculative commit in the gate | 14:38 |
| avass | same | 14:38 |
| avass | oh.. | 14:38 |
| tobiash | but you updated to the latest image on saturday? | 14:39 |
| avass | I think we might have another restart + update when we were restarting during the week as well | 14:39 |
| avass | but the oldest possible image should have been saturday | 14:39 |
| tobiash | that would be probably f91445bb | 14:40 |
| tobiash | do you have graphs where you can see when the memleak started? | 14:41 |
| tobiash | the only scheduler touching change that merged since our last update was https://review.opendev.org/756725 | 14:41 |
| tobiash | but that doesn't really look suspicious at first glance | 14:42 |
| tobiash | or it might be use case driven | 14:42 |
| tobiash | our memleak problems were use case driven as well two months ago (first usage of dequeue rest api by users) | 14:43 |
| avass | hmm no, but from what I've seen it has steadily increasing | 14:43 |
| avass | and it seem to be increasing since the crash | 14:44 |
| avass | we hadn't had time to set up any long term monitoring yet | 14:44 |
| tobiash | ours increases pretty fast at first and then increases slower after some time | 14:44 |
| avass | though it seem to be steady below 3gb at the moment | 14:45 |
| avass | the only error we had was a gerrit project that had the wrong name in the tenants config | 14:47 |
| tobiash | when you're near oom you could do a repl session with further analysis by using objgraph e.g. | 14:51 |
| tobiash | avass: do you have "zookeeper connection" related log messages btw? | 14:52 |
| avass | nope | 14:52 |
| tobiash | we saw that when the memory usage is high the garbage collector sometimes can take quite long which makes increasing the zk session timeouts needed | 14:53 |
| avass | or uh, yeah we got debug messages "Zookeeper connection: CONNECTED/SUSPENDED/LOST" | 14:53 |
| tristanC | corvus: it seems like zuul registry storage is still affected by `..` or `/` in components fixed in https://review.opendev.org/#/c/687157/4/zuul_registry/storage.py | 14:53 |
| tobiash | avass: the LOST causes a mass deletion of all nodes | 14:54 |
| avass | it reconnected 200ms after that | 14:55 |
| tobiash | avass: you probably want to increase that: https://zuul-ci.org/docs/zuul/discussion/components.html#attr-zookeeper.session_timeout | 14:55 |
| avass | might be a good idea :) | 14:55 |
| tobiash | we had 40 for a long time but increased further during our memleak problems | 14:56 |
| tobiash | keep in mind that the zk server also has a config option for a max session timeout that limits what zuul can choose | 14:56 |
| avass | right tickTime*maxSessionTimeout ? | 14:59 |
| avass | oh no that property is in ms | 15:00 |
| *** Miouge has quit IRC | 15:03 | |
| avass | tobiash: thanks! | 15:13 |
| *** bhavikdbavishi has joined #zuul | 15:14 | |
| avass | I guess because we're still pretty reliable on static nodes we haven't actually noticed those problems, or at least not directly. | 15:16 |
| avass | since the static nodes doesn't get deleted | 15:16 |
| *** bhavikdbavishi1 has joined #zuul | 15:16 | |
| avass | but my if they still get deleted in zk, nodepool will see them as available and cause extra load on the nodes | 15:17 |
| *** bhavikdbavishi has quit IRC | 15:18 | |
| *** bhavikdbavishi1 is now known as bhavikdbavishi | 15:18 | |
| *** piotrowskim has joined #zuul | 15:21 | |
| corvus | tristanC: +2 with comment. that made me wonder: do you think a hypothesis state machine can handle a test of store_upload? | 15:28 |
| *** zenkuro has quit IRC | 15:29 | |
| *** zenkuro has joined #zuul | 15:30 | |
| avass | it's just an idea, but if I'm right about that maybe the scheduler also should abort jobs using the nodes when it deletes the nodes? | 15:39 |
| avass | unless I'm wrong and it already does that | 15:40 |
| corvus | avass: i don't follow; i don't think the scheduler deletes nodes at all? | 15:43 |
| avass | I could be misunderstanding what tobiash said, but what I understood is that if the scheduler loses connection to zk it will cause mass deletion of nodes | 15:45 |
| fungi | if the scheduler releases its lock on a node then nodepool may delete it (if it's the sort of thing nodepool expects to delete once freed) | 15:45 |
| corvus | avass: yes, but by nodepool | 15:45 |
| avass | ah well, that's a bit harder to avoid then :( | 15:46 |
| corvus | avass: and the scheduler will abort the jobs because it knows that it lost the locks | 15:46 |
| tobiash | avass: more precise, if zuul looses its session in zk (aka no heartbeat arrived at ak during the session timeout) zuul looses all locks and thus nodepool treats all nodes as released and deletes them | 15:46 |
| avass | okay I think I follow | 15:46 |
| avass | we've seen unexpected extra load on some nodes so it was just a theory | 15:47 |
| fungi | but also as corvus points out, the builds which were in progress are going to be considered lost before nodepool starts deleting any of the nodes it had allocated for them anyway | 15:47 |
| corvus | assuming the scheduler actually resumes work and doesn't get oomkilled | 15:48 |
| fungi | yeah, that too | 15:48 |
| tobiash | well I think the scheduler doesn't handle that at all, so the jobs themselves fail with unreachable once the nodes get destroyed which triggers a job retry | 15:48 |
| tobiash | that gets more complicated with static nodes though since in that case the old job might continue to run in parallel with a new job | 15:49 |
| avass | that would explain it | 15:51 |
| corvus | that seems to be a serious omission :( | 15:52 |
| tobiash | yes, zuul should react on this event and abort all jobs that had any node | 15:52 |
| tobiash | we so far just assume that this never happens | 15:52 |
| tobiash | and we should do anything that this ever happens actually since even if we handle that a abort-all-jobs is exceedingly costly | 15:53 |
| tobiash | corvus: I think we should increase the default zk timeout | 15:54 |
| tobiash | especially since we rely more on zk a higher session timeout makes this much more stable | 15:54 |
| tobiash | especially on larger zuul's a full gc run can pause the zk heartbeat thread for longer than 10s | 15:55 |
| tobiash | we run with 120s atm in production (which is a bit excessive but was needed during our oom problems) | 15:56 |
| tobiash | I think we should aim for 40s which is also the default max session timeout in zk server as I think I rememver | 15:56 |
| avass | it is | 15:56 |
| avass | at least with a ticktime of 2000ms | 15:57 |
| avass | by default it's 20*ticktime, but it's unclear if the maxSessionTimeout property is in ms or number of ticks | 15:57 |
| openstackgerrit | Tobias Henkel proposed zuul/zuul master: Increase default zookeeper session timeout https://review.opendev.org/763209 | 16:01 |
| openstackgerrit | Jeremy Stanley proposed zuul/zuul-jobs master: validate-host: Options to require v4 and v6 routes https://review.opendev.org/763065 | 16:02 |
| *** jfoufas1 has quit IRC | 16:05 | |
| *** saneax has quit IRC | 16:17 | |
| *** mach1na has quit IRC | 16:32 | |
| *** bhavikdbavishi has quit IRC | 17:07 | |
| *** jcapitao has quit IRC | 17:20 | |
| pabelanger | has anyone by chance seen the following error before | 17:23 |
| pabelanger | Error: error creating build container: (image name "centos:7" is a short name and no search registries are defined in /etc/containers/registries.conf) | 17:23 |
| pabelanger | this is using podman and build-container-images | 17:24 |
| pabelanger | Before I start to debug | 17:24 |
| pabelanger | https://4ecbd426a5524f19727d-f1df475a3bd31dc5211674e175785e2a.ssl.cf5.rackcdn.com/61/4ee325bfa52926cf585b71d1dcc401d02c55f99b/check/ansible-galaxy-build-container-image/e4daadf/job-output.html#l602 | 17:24 |
| pabelanger | the dockerfile in question is not using namespaces for image | 17:24 |
| clarkb | I think you need to specify a full path for the image | 17:25 |
| clarkb | podman is saying I don't know whic registry you want me to get centos:7 from | 17:25 |
| clarkb | I think | 17:25 |
| pabelanger | yah, I think there is search registries by default however | 17:25 |
| avass | or you need to configure which registries to search in | 17:25 |
| pabelanger | I wonder if we nuke them some how | 17:25 |
| avass | maybe use-intermediate-registry does | 17:26 |
| avass | buildset-registry* | 17:27 |
| clarkb | I want to say the code reads those files and then adds to them and writes them back | 17:27 |
| pabelanger | [registries.search] | 17:27 |
| pabelanger | registries = ['registry.fedoraproject.org', 'registry.access.redhat.com', 'registry.centos.org', 'docker.io'] | 17:27 |
| pabelanger | is what I have local | 17:27 |
| pabelanger | wonder if ubuntu is different | 17:27 |
| clarkb | and it vendors a toml lib in the module to make that possible | 17:28 |
| pabelanger | k | 17:28 |
| avass | yup | 17:29 |
| corvus | pabelanger: i highly recommend fully qualifying all names | 17:30 |
| corvus | pabelanger: i think that's going to be important in order to stop thinking of 'docker' as the default | 17:31 |
| corvus | pabelanger: we switched to fully-qualified in opendev and zuul | 17:32 |
| pabelanger | corvus: Oh, for sure and I agree | 17:32 |
| pabelanger | trying to work the issue on both sides right now | 17:32 |
| corvus | ++ | 17:33 |
| pabelanger | my goal is to by default have zuul-jobs podman ready for ansible | 17:33 |
| avass | it still annoys me every time someone calls containers 'dockers' | 17:34 |
| pabelanger | avass: containers are rpms | 17:36 |
| pabelanger | :) | 17:36 |
| pabelanger | oh interesting /etc/containers/registries.conf is empty by default | 17:40 |
| pabelanger | from upstream package | 17:40 |
| pabelanger | and we don't drop in registry.search | 17:41 |
| pabelanger | that solves that problem | 17:41 |
| *** jpena is now known as jpena|off | 18:05 | |
| *** hamalq has joined #zuul | 18:07 | |
| pabelanger | here is an odd question, which I likely know the answer too. Is there a way, with github connection, to have zuul apply depends-on PR to a different branch? So, this is case, sandbox (master) depends-on galaxy (devel) but galaxy (master) is also a thing. depends-on does not work in this case, since it is raised against the wrong branch. The issue here, is commit are manually merged from devel into | 18:07 |
| pabelanger | master in galaxy, and zuul may not be doing that on master. | 18:07 |
| pabelanger | if that all makes sense | 18:07 |
| pabelanger | I am thinking that override branch may work out | 18:08 |
| pabelanger | but haven't tested yet | 18:08 |
| clarkb | aiui no beacuse it sets up everything per branch | 18:09 |
| pabelanger | sorry, override-checkout | 18:09 |
| clarkb | for example in openstack you can do stable/train depends on stable/stein to test an upgrade fix. Even if you override the branch the code will be applied to stein | 18:09 |
| clarkb | in cases like that it may be more appropriate for the job to do necessary merging | 18:10 |
| pabelanger | darn | 18:10 |
| pabelanger | okay | 18:10 |
| clarkb | another approach is to push the code you need to the correct branch then depends on that | 18:10 |
| pabelanger | clarkb: yah, that is what I am testing now | 18:11 |
| pabelanger | but suspect people will slap me, because I raised it against 'master' vs devel | 18:11 |
| clarkb | well with pre merge testing they can drop their devel into master workflow and just start working on master :P | 18:12 |
| pabelanger | indeed! | 18:13 |
| pabelanger | right now, trying to get 3pci working first | 18:13 |
| pabelanger | having them change workflows is a whole other topic | 18:13 |
| openstackgerrit | Tristan Cacqueray proposed zuul/zuul-registry master: Add hypothesis rule based state machine test and fix path issue https://review.opendev.org/687157 | 18:14 |
| corvus | pabelanger: galaxy@devel does have the code you want to test -- the issue is just that since you're proposing to sandbox@master the job is checking out master by default; in which case, yeah, override-checkout might be the way to go. you could do that just on the sandbox job (ie, when running this job on the sandbox repo, check out galaxy@devel) | 18:14 |
| clarkb | oh is this purely a zuul config issue and not the actual git checkout state? | 18:15 |
| corvus | pabelanger: some of those statements were supposed to have question marks, sorry :) anyway read that whole thing as a "did i get this right? if so, i think this..." kind of statement | 18:15 |
| clarkb | ya if that is the case then my assertion above wouldn't necessarily be the case. I read it as take the commit(s) from $branch and put them on another branch because that is how we want to test it | 18:16 |
| corvus | clarkb: yeah, and it's possible both are right and i'm just introducing a different way of looking at the prob | 18:16 |
| pabelanger | corvus: yes, you are spot on. | 18:16 |
| corvus | iow, "merge this devel PR into master" would be a solution to the prob, but "checkout devel" would also solve it | 18:17 |
| pabelanger | ack, will test soon | 18:18 |
| clarkb | I think it depends on whether or not you're willing to test all the other stuff in devel | 18:20 |
| clarkb | (not sure) | 18:20 |
| corvus | yeah; there's also a lot of variations here: 2 jobs (one for devel, one for master); just doing this as a one-off (ie, dynamic zuul config change to add override-checkout to just the sandbox patch). and if you go the other way (merge instead of checkout), maybe a role to merge devel into master at the start of the job. | 18:25 |
| corvus | many tools in this box | 18:25 |
| openstackgerrit | Jeremy Stanley proposed zuul/zuul-jobs master: DNM: Exercise validate-host IP family assert https://review.opendev.org/763245 | 18:25 |
| fungi | clarkb: so like that ^ | 18:25 |
| corvus | clarkb: you previously reviewed https://review.opendev.org/687157 and parent; would love your opinion when you have a min | 18:26 |
| clarkb | fungi: yup | 18:26 |
| clarkb | corvus: ya I'll take a look in a bit | 18:27 |
| clarkb | I think we may be restarting gerrit soon to double check our recent config updates don't pose a problem then after that I can review | 18:27 |
| -openstackstatus- NOTICE: The Gerrit service at review.opendev.org is being restarted quickly as a pre-upgrade sanity check, estimated downtime is less than 5 minutes. | 18:37 | |
| *** rpittau is now known as rpittau|afk | 18:39 | |
| clarkb | corvus: tristanC I'm having a hard time wrapping around how a Null upload is valid? re https://review.opendev.org/#/c/687118/5 | 18:54 |
| clarkb | what does that mean from a user standpoint? you've just created an image in the registry maybe with a tag but no real content? | 18:55 |
| fungi | clarkb: it seems like it only runs tasks from a role once even if included multiple times in the same play... do i need to create more plays? | 18:58 |
| clarkb | fungi: oh I think you may need to use include role? or ya just copy pasta the hosts: all roles: header for eahc one | 18:59 |
| clarkb | unless it collapses those too. But include_role should rerun it each time | 18:59 |
| fungi | using separate plays makes it a little easier for me to document what each one's doing, since the console view doesn't seem to provide that degree of detail on its own | 19:01 |
| tristanC | clarkb: if this is not valid, then we could return an error, right now this code path cause a 500 undefined error | 19:02 |
| *** openstackgerrit has quit IRC | 19:02 | |
| clarkb | right I think that was my original concern. The old setup errors (which I expect is correct), but the new code succeeds in order to fit arbitrary testing inputs | 19:03 |
| clarkb | I don't think we should make the api match arbitrary testing inputs if the correct behavior is to fail | 19:03 |
| corvus | clarkb: agreed, i don't think i fully understood the previous comment/response but now i do. | 19:08 |
| corvus | clarkb, tristanC: i think upload_chunk should only be called after start_upload | 19:09 |
| tristanC | the spec ( https://github.com/opencontainers/distribution-spec/blob/master/spec.md#push ) states that the location ` SHOULD match exactly the <location> obtained from the POST request ` | 19:14 |
| *** sugaar has quit IRC | 19:15 | |
| pabelanger | corvus: could I ask for zuul-registry release? We've merged the release jobs | 19:19 |
| tristanC | i guess it's ok to interpret this as a `MUST match`, but then the failure code should be 404/400, not 500 | 19:20 |
| fungi | a possible ui regression has been raised in #opendev, apparently this logline is a multi-line entry in the raw version but only the first line is rendered in the html version: https://zuul.opendev.org/t/openstack/build/00ff397ca5374a0b9441036d43ee3416/log/controller/logs/screen-n-cpu.txt#7371 | 19:40 |
| fungi | also i notice something is causing you to have to scroll to the end of the log to get a horizontal scroll bar | 19:41 |
| sean-k-mooney | ya the long line is a libvirt xml we dump https://d3f8c4c85da258632f6c-407789a006c18491d0ad02a611fbe821.ssl.cf2.rackcdn.com/602432/25/check/tempest-integrated-compute/8105b43/controller/logs/screen-n-cpu.txt | 19:41 |
| sean-k-mooney | its in the raw version but you cant link to ines in that | 19:42 |
| sean-k-mooney | if you seach for 'End _get_guest_xml xml=<domain type="qemu">' you will see the full xml | 19:42 |
| ianw | as for the scrolling to the end -- i think it has been like that for quite a while; at least for me | 19:44 |
| sean-k-mooney | i have noviced it for a few weeks but just assumed most people did not have the issue | 19:44 |
| sean-k-mooney | its only an issue when you search and it auto scrolls | 19:44 |
| sean-k-mooney | previously you could use left and right arrow to scroll back | 19:45 |
| ianw | i restarted zuul-web probably on monday to pick-up the log-viewer change that adds the severity in pf4 buttons and felix's breadcrumbs at the top, but i don't believe that has modified the actual log rendering bits in any way | 19:46 |
| sean-k-mooney | this one https://opendev.org/zuul/zuul/commit/072bf45ff8a534aebdbf8a1987d4bc902dd15b7a | 19:46 |
| ianw | that's it | 19:48 |
| sean-k-mooney | im not familar with this code but skiming it nothing obvious is jumping out at me | 19:49 |
| clarkb | could be other changes since the last restart too | 19:49 |
| *** vishalmanchanda has quit IRC | 19:49 | |
| sean-k-mooney | the previous commti is "Enable ANSI rendering via react-ansi" | 19:50 |
| fungi | i didn't think that one touched the log viewer, only summary and console tabs? | 19:51 |
| sean-k-mooney | ya it didnt | 19:51 |
| sean-k-mooney | build output and console | 19:51 |
| sean-k-mooney | the horrizontal scoling was apparently fixed by https://opendev.org/zuul/zuul/commit/e1a7afa9530eb4c2187354f786c73c6907687212 | 19:52 |
| *** melwitt has joined #zuul | 19:53 | |
| sean-k-mooney | ianw: look like you previous fixed page up page down https://opendev.org/zuul/zuul/commit/8d0f440e1bc85b55a2d780769d8ee1df4ae6ce7e | 19:54 |
| sean-k-mooney | but it might have regrssed again | 19:55 |
| ianw | pg-up/down is working for me at lesat | 19:55 |
| sean-k-mooney | yep it is for me too | 19:56 |
| sean-k-mooney | yep it is as is up and down | 19:56 |
| sean-k-mooney | but not left and right | 19:56 |
| sean-k-mooney | home and end also go to the top and bottom which may or may not be intended | 19:56 |
| ianw | i think first thing we should find a sample site pre the log change and see if those lines are missing there. i don't believe i changed the actual mechanism of the filtering, but you never know | 19:57 |
| sean-k-mooney | shift home/end select the start/end fo the line as i would expect | 19:57 |
| sean-k-mooney | ill check my zuul install i havent updated it in a while | 19:57 |
| ianw | https://7c4c2c6decd3eafecbc8-7e70b99d63ad0f32ef21f8f8ea4f17ac.ssl.cf1.rackcdn.com/762682/1/check/zuul-build-dashboard-opendev/572ac58/npm/html/tenants seems to be the 14th | 19:59 |
| sean-k-mooney | https://zuul.seanmooney.info/t/openstack/build/e9b8da4a13c340b28fd5aa81ca2eea0f/log/controller/logs/screen-n-cpu.txt#4318 | 20:00 |
| sean-k-mooney | that is what it used to look like | 20:00 |
| sean-k-mooney | that might be a little slow to load | 20:00 |
| sean-k-mooney | i need to move the static files to a webserver | 20:00 |
| ianw | bummer, i agree @ https://7c4c2c6decd3eafecbc8-7e70b99d63ad0f32ef21f8f8ea4f17ac.ssl.cf1.rackcdn.com/762682/1/check/zuul-build-dashboard-opendev/572ac58/npm/html/t/openstack/build/00ff397ca5374a0b9441036d43ee3416/log/controller/logs/screen-n-cpu.txt | 20:02 |
| ianw | i looks like the lines that don't match aren't coming out | 20:02 |
| sean-k-mooney | wait are each of those like a table row | 20:04 |
| sean-k-mooney | i mean i guess that works i just was not expecting it to be one big table | 20:04 |
| ianw | yes because that's how like the highlighting of ranges work | 20:05 |
| sean-k-mooney | ah ok | 20:05 |
| sean-k-mooney | ianw: so you said the lines that dont match arent coming out | 20:07 |
| sean-k-mooney | did you mean to some regex | 20:07 |
| ianw | that's a guess, i'm trying to remember exactly how it works | 20:07 |
| sean-k-mooney | i wonder is it only logs that have xml that are affected | 20:07 |
| sean-k-mooney | e.g. as a way to prevent html injections or something | 20:08 |
| ianw | https://opendev.org/zuul/zuul/src/commit/072bf45ff8a534aebdbf8a1987d4bc902dd15b7a/web/src/containers/logfile/LogFile.jsx#L210 | 20:09 |
| ianw | i feel like it must be ~ there, with the line not having a severity maybe? | 20:09 |
| sean-k-mooney | yep | 20:10 |
| sean-k-mooney | it has its 0 | 20:10 |
| sean-k-mooney | the ones with debug are 1 but the next lines are 0 | 20:10 |
| ianw | that is set in https://opendev.org/zuul/zuul/src/commit/072bf45ff8a534aebdbf8a1987d4bc902dd15b7a/web/src/actions/logfile.js | 20:10 |
| sean-k-mooney | class="zuul-log-sev-0" | 20:11 |
| ianw | hrm, but 0 >= 0 | 20:11 |
| sean-k-mooney | the issue is the subsequent lines dont inhit the sev | 20:12 |
| sean-k-mooney | https://opendev.org/zuul/zuul/src/commit/072bf45ff8a534aebdbf8a1987d4bc902dd15b7a/web/src/actions/logfile.js#L30-L38 | 20:12 |
| sean-k-mooney | those shoudl have had 1 form the initall line | 20:12 |
| sean-k-mooney | with DEBUG | 20:12 |
| sean-k-mooney | but its getting 0 becaue of https://opendev.org/zuul/zuul/src/commit/072bf45ff8a534aebdbf8a1987d4bc902dd15b7a/web/src/actions/logfile.js#L47 | 20:13 |
| sean-k-mooney | let sev = null im guessing | 20:13 |
| ianw | i don't think subsequent lines inherit | 20:13 |
| ianw | but it should be showing lines with a severity of 0 | 20:14 |
| sean-k-mooney | why? | 20:14 |
| sean-k-mooney | oh all is 0 | 20:15 |
| ianw | i'm just bringing up a dev environment | 20:16 |
| sean-k-mooney | oh weird it jumps form line number 7371 to 7450 | 20:20 |
| sean-k-mooney | so you can see the gaps | 20:20 |
| sean-k-mooney | https://zuul.opendev.org/t/openstack/build/00ff397ca5374a0b9441036d43ee3416/log/controller/logs/screen-n-cpu.txt?severity=0#7371 | 20:20 |
| ianw | if you remove the if statement @ https://opendev.org/zuul/zuul/src/commit/072bf45ff8a534aebdbf8a1987d4bc902dd15b7a/web/src/containers/logfile/LogFile.jsx#L210 it all comes back | 20:21 |
| sean-k-mooney | i dont acully understand " line.severity >= severity && (" | 20:23 |
| sean-k-mooney | what is that && testing | 20:23 |
| sean-k-mooney | shoudl it be (line.severity >= severity) && ( | 20:23 |
| ianw | like in bash i'd say, like "command-that-might-fail && run-if-it-doesn't" | 20:23 |
| sean-k-mooney | ya but im wondering about precidence | 20:24 |
| sean-k-mooney | is it anding first tehn compureing that | 20:24 |
| sean-k-mooney | e.g. is the operator binding like this (line.severity >= (severity && ())) | 20:25 |
| clarkb | googling js operator precedence says now | 20:25 |
| clarkb | *says no | 20:26 |
| sean-k-mooney | ya just tried it in a console | 20:26 |
| sean-k-mooney | 1 >= 0 && (4) prints 4 | 20:26 |
| sean-k-mooney | so it eveluates the condiotnthen does the thing on the right | 20:26 |
| ianw | i think it's that line.severity is null, or none, or something that javascript doesn't think is >= 0 | 20:28 |
| sean-k-mooney | well line ={}; line.severity = null; line.severity >= 0 | 20:29 |
| sean-k-mooney | prints true | 20:29 |
| sean-k-mooney | my dinner i just cooked so got to go | 20:30 |
| sean-k-mooney | its does seam like its something related to that however | 20:30 |
| sean-k-mooney | not really sure why its not working | 20:31 |
| ianw | yeah, if i dump the severity before the line, it's blank | 20:31 |
| ianw | className={`log-message zuul-log-sev-${ | 20:31 |
| ianw | line.severity || 0 | 20:31 |
| ianw | that's why in the css matcher it seems to do this i guess | 20:32 |
| tristanC | in javascript those expressions are all false: [null < 0, null > 0, null == 0] , but those are true: [null <= 0, null >= 0] | 20:33 |
| sean-k-mooney | actully no | 20:33 |
| sean-k-mooney | null <= 0 is true | 20:34 |
| sean-k-mooney | as is null >= 0 | 20:34 |
| sean-k-mooney | null <0 and null > 0 are both false | 20:35 |
| sean-k-mooney | and null == 0 is false | 20:35 |
| sean-k-mooney | but >= and <= are true | 20:35 |
| sean-k-mooney | tristanC: oh sorry that is what you said | 20:35 |
| sean-k-mooney | ya javescript is a very self consitent language ... | 20:36 |
| tristanC | yeah go figure, it seems like '==' behave differently | 20:36 |
| tristanC | when you use ">=", null is converted to the number 0, but when you use '==', null is converted to undefined which doesn't match anything | 20:37 |
| ianw | i have no idea if this is null, blank, or undefined at this point :/ | 20:40 |
| tristanC | ianw: in that situation, the common strategy is to add `|0` to your value, that would make it default to 0 if it is not defined | 20:40 |
| tristanC | [""|0 , null|0, undefined|0, 42|0] == [0, 0, 0, 42] | 20:41 |
| ianw | hrm, that seems to ... not work | 20:42 |
| tristanC | ianw: what's the error? | 20:43 |
| ianw | a blank page with a bunch of 0's randomly on it :) | 20:43 |
| tristanC | at least they are not null :) | 20:44 |
| ianw | when i output {line.severity} it's just "blank", (debug lines are 1, etc) | 20:45 |
| ianw | so is it undefined, null, "" is i guess the question? | 20:45 |
| fungi | does javascript have anything like python's type() function? | 20:48 |
| tristanC | fungi: typeof | 20:48 |
| fungi | hopefully that would be a way to disambiguate them for debugging purposes | 20:49 |
| *** slaweq has quit IRC | 20:58 | |
| ianw | sorry jump-starting a car ... as you do | 21:06 |
| ianw | ok, i'm a dolt | 21:07 |
| fungi | i'm half expecting to need to jump mine sometime soon. it gets driven for a few minutes once every week or two lately | 21:07 |
| ianw | line.severity|0 does seem to work. i had the filtering set to debug | 21:07 |
| ianw | fungi: yep, it's our second car i run kids around in that hasn't moved in forever since my wife is WFH | 21:07 |
| sean-k-mooney | undefined >=0 is false | 21:07 |
| ianw | however, today she is WAFH | 21:08 |
| sean-k-mooney | "" >= 0 is true | 21:08 |
| sean-k-mooney | so its proably undefiend | 21:08 |
| sean-k-mooney | why is undefined|0 >=0 1 | 21:08 |
| sean-k-mooney | and not true or false | 21:08 |
| sean-k-mooney | ... | 21:08 |
| fungi | ideally those lines with no declared severity would inherit the severity of the last line which had one? | 21:09 |
| fungi | (or 0 if there was no prior line with a severity) | 21:09 |
| sean-k-mooney | oh || not | | 21:09 |
| sean-k-mooney | ya | 21:10 |
| sean-k-mooney | can you jsut set the default calue for line.severity to 1 explictly somewhere | 21:10 |
| sean-k-mooney | or do line_severity = line.severity || 0 | 21:11 |
| sean-k-mooney | then use line_severity instead of line.severity | 21:11 |
| clarkb | ianw: fungi: we've used ~weekly takeout as an excuse to run the car for about 30 minutes a week. | 21:16 |
| clarkb | I suggested we sell the car but haven't managed to make that idea stick | 21:16 |
| ianw | if (!line.severity) line.severity = 0 | 21:19 |
| ianw | before works | 21:19 |
| ianw | i'm not quite sure how to encode that in the current magic | 21:19 |
| clarkb | ianw: have a link to the code context? | 21:20 |
| ianw | https://opendev.org/zuul/zuul/src/commit/072bf45ff8a534aebdbf8a1987d4bc902dd15b7a/web/src/containers/logfile/LogFile.jsx#L210 | 21:20 |
| clarkb | ianw: I think everything in the {} starting at line 205 first non whitespace char is js | 21:21 |
| clarkb | the statement above does a variable assignment too | 21:22 |
| clarkb | at line 208 I think you can do your statement | 21:22 |
| ianw | or, the regex matcher should set the severity to 0 if it doesn't match | 21:23 |
| ianw | i think that actually works better | 21:24 |
| sean-k-mooney | ianw: i think on line 209 before the return you can jsut do "line.severity = line.severity || 0" | 21:25 |
| *** openstackgerrit has joined #zuul | 21:28 | |
| openstackgerrit | Ian Wienand proposed zuul/zuul master: web: return 0 as severity when line doesn't match https://review.opendev.org/763264 | 21:28 |
| ianw | i think that might be the best way; although variations on ^^ work too | 21:29 |
| clarkb | ++ to addressing that as close to the source as possible | 21:29 |
| ianw | i think it makes sense for lines to have a value from 0->7, rather than null,1->7 | 21:30 |
| ianw | but, you know, i'd not be surprised to find out it causes some other issue | 21:31 |
| *** slaweq has joined #zuul | 21:31 | |
| ianw | doing school run, hopefully that returns sample site soon | 21:31 |
| clarkb | I went ahead and +2'd it | 21:31 |
| corvus | what's the latest on horizontal scroll there? | 21:37 |
| openstackgerrit | Slawek Kaplonski proposed zuul/zuul-jobs master: [multi-node-bridge] Add script to configure connectivity https://review.opendev.org/762650 | 21:37 |
| corvus | ie, did anyone find out why left/right keys stopped working? | 21:38 |
| clarkb | I have not | 21:39 |
| openstackgerrit | Merged zuul/zuul-jobs master: validate-host: Options to require v4 and v6 routes https://review.opendev.org/763065 | 21:45 |
| sean-k-mooney | corvus: no, that is annoying but getting the content rendering is the main issue i hit | 21:56 |
| sean-k-mooney | left/right would be nice once its visable | 21:57 |
| corvus | i've gone back a bit, and if that is a regression, it's not a recent one. | 21:57 |
| corvus | i also thought left/right worked, but it looks like it hasn't for a long time, if ever. i agree it should work. | 21:57 |
| sean-k-mooney | it used to work before we started using the new app | 21:57 |
| sean-k-mooney | but that was like over a year ago | 21:57 |
| clarkb | ya pf4 changed a lot of that navigation stuff | 21:59 |
| corvus | ah found it. yeah, pf4 broke it | 22:00 |
| corvus | sean-k-mooney, clarkb, ianw, felixedel: i have confirmed that left/right keyboard input on log page works at this commit: https://opendev.org/zuul/zuul/commit/b4b5b9fd58dd5d5aeb9d32331e4882d7ef5faf06 | 22:03 |
| corvus | so it looks like we missed that when we un-reverted | 22:03 |
| ianw | i have to admit i'm still not sure why "null >= 0" wasn't working | 22:06 |
| sean-k-mooney | ianw: i think that just a case of javasciipt is weird | 22:11 |
| sean-k-mooney | it totally fills you with confidence that so much software is now written in it | 22:12 |
| ianw | maybe it's somehow different between react and js? because in a console "null >= 0" returns true | 22:14 |
| clarkb | jsx is just compiling to js and using a very thin shim iirc. But ya it could be | 22:15 |
| *** slaweq has quit IRC | 22:16 | |
| ianw | the lines we are missing have a line.severity typeof() "undefined" | 22:17 |
| sean-k-mooney | ianw: ya so undefiend >=0 is false | 22:17 |
| sean-k-mooney | even in the console | 22:17 |
| sean-k-mooney | so if they were undefined and not null then it would be false | 22:18 |
| ianw | but why is it not "null"? and why did me changing the default value to "0" from null then change this value to 0 instead of "undefined"? | 22:18 |
| corvus | i think the left/right event is broken by the logs being in a tab | 22:22 |
| sean-k-mooney | its broken on even without that change | 22:23 |
| sean-k-mooney | when the logs were in a new page | 22:23 |
| sean-k-mooney | but yes its definetly broken in the tab case | 22:24 |
| corvus | to be clear, this is what i am certain of: left/right scrolling works at this commit (which reverts out the PF4 build page): https://opendev.org/zuul/zuul/commit/b4b5b9fd58dd5d5aeb9d32331e4882d7ef5faf06 | 22:25 |
| corvus | this makes me think that the pf4 build page broke it | 22:25 |
| corvus | i suspect the actual mechanism for the breakage is the tab container consuming the event | 22:25 |
| *** piotrowskim has quit IRC | 22:27 | |
| *** johnsom has quit IRC | 22:27 | |
| sean-k-mooney | ianw: you have seen https://www.youtube.com/watch?v=et8xNAc2ic8 before by the way right | 22:28 |
| *** johnsom has joined #zuul | 22:29 | |
| *** piotrowskim has joined #zuul | 22:29 | |
| *** nils has quit IRC | 22:32 | |
| ianw | ok, i'm still trying to 100% make sure we understand why log lines are missing | 22:32 |
| ianw | to be clear, i'm dumping the json representation of each line, and the lines that are missing don't have a severity field | 22:33 |
| ianw | this is strange to me because it seems every line should have a severity set by https://opendev.org/zuul/zuul/src/branch/master/web/src/actions/logfile.js#L62 | 22:34 |
| sean-k-mooney | ianw: where is the prtotype for line defined | 22:34 |
| sean-k-mooney | the logfile prototype is here which has a default https://opendev.org/zuul/zuul/src/commit/072bf45ff8a534aebdbf8a1987d4bc902dd15b7a/web/src/containers/logfile/LogFile.jsx#L288-L300 | 22:35 |
| sean-k-mooney | but i assume there is a simielr protype for the line | 22:35 |
| ianw | lines are an array in logFileContent | 22:36 |
| sean-k-mooney | so this is lines right https://opendev.org/zuul/zuul/src/commit/072bf45ff8a534aebdbf8a1987d4bc902dd15b7a/web/src/containers/logfile/LogFile.jsx#L65-L69 | 22:37 |
| ianw | i wonder if this is it somehow? https://opendev.org/zuul/zuul/src/commit/072bf45ff8a534aebdbf8a1987d4bc902dd15b7a/web/src/pages/Build.jsx#L196 | 22:38 |
| ianw | severity={severity ? severity : undefined} | 22:38 |
| sean-k-mooney | actully ya its an array https://opendev.org/zuul/zuul/src/commit/072bf45ff8a534aebdbf8a1987d4bc902dd15b7a/web/src/containers/logfile/LogFile.jsx#L290 | 22:38 |
| fungi | that would reset severity of 0 to undefined, right? | 22:39 |
| *** hashar has quit IRC | 22:39 | |
| sean-k-mooney | test = 0; test = test ? test : undefined; test | 22:40 |
| sean-k-mooney | prints undefined | 22:40 |
| ianw | the *first* line has | 22:41 |
| ianw | {"text":"-- Logs begin at Tue 2020-11-10 03:39:48 UTC, end at Tue 2020-11-10 04:33:38 UTC. --","index":1,"severity":null} | 22:41 |
| ianw | that suggests somehow the first line correctly gets a null severity, but then we don't do it correctly as we loop | 22:42 |
| sean-k-mooney | test_val = null; test_val = test_val ? test_val : undefined; test_val | 22:43 |
| sean-k-mooney | also print null | 22:43 |
| sean-k-mooney | sorry | 22:43 |
| sean-k-mooney | undefined | 22:43 |
| sean-k-mooney | so it will set test_val to undefined | 22:43 |
| sean-k-mooney | after the first iteration | 22:43 |
| sean-k-mooney | given null >= 0 is ture but undefined >= 0 is false | 22:45 |
| sean-k-mooney | ianw: so maybe https://opendev.org/zuul/zuul/src/commit/072bf45ff8a534aebdbf8a1987d4bc902dd15b7a/web/src/pages/Build.jsx#L196 should be null and not undefined | 22:46 |
| sean-k-mooney | it does seam however that you default of 0 are not taking effect | 22:47 |
| ianw | ok, this is in the regex matching somehow | 22:49 |
| ianw | god damn it, why did i look | 22:49 |
| ianw | https://opendev.org/zuul/zuul/src/commit/072bf45ff8a534aebdbf8a1987d4bc902dd15b7a/web/src/actions/logfile.js#L49 | 22:50 |
| ianw | m = SYSTEMD_LOGMATCH.exec(line) | 22:50 |
| ianw | should return a "m" value taht can be looked up in severityMap | 22:51 |
| ianw | it's returning crap, that isn't in the map, which gets you a sev of "undefined" | 22:51 |
| sean-k-mooney | right we dont have a value of 0 ther | 22:51 |
| sean-k-mooney | this is the map https://opendev.org/zuul/zuul/src/commit/072bf45ff8a534aebdbf8a1987d4bc902dd15b7a/web/src/actions/logfile.js#L30-L38 | 22:51 |
| ianw | right, it's looking up | 22:52 |
| ianw | "Nov 10 04:15:04.681121 ubuntu-focal-rax-dfw-0021684256 nova-compute[88940]: <value>fdc</value>,Nov 10 04:15:04.681121,.681121,., ubuntu-focal-rax-dfw-0021684256 nova-compute[88940]: <value>fdc</value>,ubuntu-focal-rax-dfw-0021684256,," | 22:52 |
| ianw | in the map | 22:52 |
| ianw | oh, no, hang on, it's lookup up m[7] | 22:53 |
| sean-k-mooney | yes whatervef m[7] is | 22:53 |
| sean-k-mooney | i think that is "" | 22:54 |
| sean-k-mooney | which wont maghe any of the keys | 22:54 |
| ianw | it's looking up "undefined" | 22:54 |
| ianw | so the match is returning, but it's not got a m[7] | 22:54 |
| sean-k-mooney | 7 is the last ,, | 22:55 |
| sean-k-mooney | in that | 22:55 |
| sean-k-mooney | e.g. nothing right | 22:55 |
| sean-k-mooney | so instead of if(m) | 22:55 |
| sean-k-mooney | you should do if(m && m[7]) | 22:55 |
| sean-k-mooney | which will leave sev=null | 22:56 |
| sean-k-mooney | although really you want it set to the value from the previous line | 22:57 |
| ianw | i don't know if setting it to last line is always what you want | 22:59 |
| sean-k-mooney | well it wont be for the first line so you could be consistent and just always set it to null/0 | 22:59 |
| sean-k-mooney | but i htink in generall it would be | 23:00 |
| sean-k-mooney | but i can think of case where it might not be correct | 23:00 |
| sean-k-mooney | to just use the previous lines | 23:00 |
| sean-k-mooney | e.g. if its not in openstack log format | 23:00 |
| ianw | but why does SYSTEMD_LOGMATCH match, when this line doesn't have a status | 23:04 |
| ianw | that's the root cause | 23:04 |
| ianw | ^(\w+\s+\d+\s+\d{2}:\d{2}:\d{2}((\.|\,)\d{3,6})?)( (\S+) \S+\[\d+\]\: ((DEBUG|INFO|WARNING|ERROR|TRACE|AUDIT|CRITICAL))?.*) | 23:05 |
| ianw | is the expanded regex | 23:05 |
| clarkb | ianw: the ? after the levels means it is optional | 23:05 |
| clarkb | looking at journalctl output locally that seems to be the case | 23:06 |
| clarkb | also some services seem to use E: for error | 23:06 |
| ianw | yeah; removing the '?' everything starts getting a null severity again | 23:07 |
| ianw | has that changed in ... forever? | 23:08 |
| clarkb | I don't think so. Is it possible this issue has just been there? | 23:08 |
| clarkb | (I haven't checked git log) | 23:09 |
| ianw | hasn't changed since https://review.opendev.org/#/c/672839/ afaics | 23:10 |
| ianw | that added it | 23:10 |
| *** aluria has quit IRC | 23:11 | |
| ianw | ok, looking more closely at | 23:11 |
| ianw | https://opendev.org/zuul/zuul/commit/072bf45ff8a534aebdbf8a1987d4bc902dd15b7a | 23:11 |
| ianw | {data.map((line) => ( | 23:12 |
| ianw | ((!severity || (line.severity >= severity)) && | 23:12 |
| ianw | so; i think we have about 6 different ways to fix this now | 23:13 |
| fungi | sounds like you should round out your morning with breakfast at milliways | 23:15 |
| ianw | fungi: i feel a bit more like the cow who offers himself to be eaten :) | 23:16 |
| fungi | indeed | 23:17 |
| ianw | i think the ? on the regex must be wrong. i'm worried it cascades into more weirdness though | 23:17 |
| *** aluria has joined #zuul | 23:18 | |
| openstackgerrit | Ian Wienand proposed zuul/zuul master: web: remove optional match on systemd regex https://review.opendev.org/763281 | 23:24 |
| corvus | ianw: you could look into osla to see if there's any history for that '?' | 23:26 |
| corvus | that may be where it originated | 23:27 |
| ianw | corvus: ummm ... what's osla? | 23:27 |
| ianw | oh, os-loganalyze | 23:29 |
| ianw | i have a tree of that somehere... | 23:29 |
| ianw | https://review.opendev.org/#/c/455831/ is maybe where it came from? | 23:30 |
| ianw | "We also have to realize that status might not be in a systemd line, | 23:31 |
| ianw | and that's ok, but still match it, given that otherwise we run the | 23:31 |
| ianw | risk of falling into the syslog match." | 23:31 |
| ianw | that does -- self.status = m.group('status') or "NONE" | 23:32 |
| corvus | maybe we should do something similar? | 23:32 |
| ianw | yeah, though in this case we don't have the concern that the other match might catch it | 23:34 |
| *** rlandy|rover has quit IRC | 23:35 | |
| *** holser has quit IRC | 23:35 | |
| corvus | ianw: true; we might add more later, but i agree that all we care about is the severity so it's moot | 23:46 |
| ianw | ... and that is why we make an effort to write good commit messages :) | 23:47 |
| ianw | if sdague had of just stuck that in, we'd all still be wondering | 23:48 |
Generated by irclog2html.py 2.17.2 by Marius Gedminas - find it at https://mg.pov.lt/irclog2html/!