| openstackgerrit | Paul Belanger proposed zuul/zuul master: Remove support for ansible 2.5 https://review.opendev.org/650431 | 00:01 |
|---|---|---|
| openstackgerrit | Paul Belanger proposed zuul/zuul master: Switch ansible_default to 2.8 https://review.opendev.org/676695 | 00:01 |
| openstackgerrit | Paul Belanger proposed zuul/zuul master: WIP: Support Ansible 2.9 https://review.opendev.org/674854 | 00:01 |
| openstackgerrit | Tristan Cacqueray proposed zuul/zuul-registry master: Add OCI containers/storage support https://review.opendev.org/686512 | 00:05 |
| tristanC | corvus: the zuul-registry api is quite docker centric, though i was able to make it work with skopeo and a local containers/image vfs store with ^ | 00:06 |
| tristanC | (in case we want to use the zuul-registry without docker) | 00:06 |
| tristanC | i meant the Storage layer assume docker paths, thus the OCI drivers needs to decode the path component back to get the repo, tag, blob info | 00:16 |
| openstackgerrit | Ian Wienand proposed zuul/nodepool master: [wip] validate diskimages have top-level labels https://review.opendev.org/686514 | 00:39 |
| mnaser | about ianw patch there, why do we even have top level labels in the nodepool config file? | 00:43 |
| ianw | umm, i guess that's where you set min-ready? | 00:44 |
| mnaser | ianw: i guess maybe then we can assume a default value of min-ready and not hard fail if a label is missing there? | 00:45 |
| ianw | so basically auto-populate it? | 00:49 |
| mnaser | ianw: kinda, i guess | 00:50 |
| ianw | i think that list of labels has to be there (e.g. http://zuul.openstack.org/labels) ... but possibly in the config loader it could see the missing label and auto-add it. but then it might get confusing where values are coming from | 00:51 |
| openstackgerrit | Mohammed Naser proposed zuul/nodepool master: Added failing configuration check https://review.opendev.org/686515 | 01:00 |
| *** CrayZee has joined #zuul | 01:10 | |
| *** jangutter_ has joined #zuul | 01:10 | |
| *** panda|off has quit IRC | 01:12 | |
| *** panda has joined #zuul | 01:12 | |
| *** jangutter has quit IRC | 01:12 | |
| *** shachar has quit IRC | 01:13 | |
| ianw | pabelanger: i think the auto detection will help with the selinux bindings | 01:16 |
| ianw | also note this fix for the streamer maybe related? -> https://review.opendev.org/#/c/682556/ | 01:18 |
| ianw | mnaser: did you want me to rebase on that, or are you still working on something better? | 01:18 |
| *** rfolco has quit IRC | 01:19 | |
| ianw | but also note the auto support i'll re-propose after next zuul release : https://review.opendev.org/#/c/682797/1 | 01:20 |
| *** rfolco has joined #zuul | 01:27 | |
| *** shanemcd has quit IRC | 01:29 | |
| *** bstinson has quit IRC | 01:30 | |
| *** wznoinsk has quit IRC | 01:31 | |
| *** shanemcd has joined #zuul | 01:31 | |
| *** bstinson has joined #zuul | 01:40 | |
| mnaser | ianw: late here and I'm out already, so feel free to squash or whatever. I worked on a test first :p | 01:41 |
| ianw | ok, well i'll have some lunch and think about it ;) ttyl | 01:47 |
| corvus | tristanC: i'm a little confused by that change. it's true that zuul-registry should, eventually, not require docker (though i've only tested it with docker so far). but it's meant to implement the registry protocol, so it should be able to shadow dockerhub, quay.io, gcr.io, etc eventually. as long as it implements the protocol, that should work -- i don't understand why a special storage backend would be | 01:53 |
| corvus | needed | 01:53 |
| corvus | tristanC: there's nothing docker-specific about the backend storage. it's actually not compatible with the docker registry. it's a private backend storage format meant only for this service. it's not meant to share the backend storage with anything else. | 01:56 |
| *** jamesmcarthur has joined #zuul | 02:13 | |
| *** rfolco has quit IRC | 02:45 | |
| tristanC | corvus: couldn't the job building the image without docker start the zuul-registry to let the rest of the buildset pull the eventual built content? | 02:46 |
| openstackgerrit | Tristan Cacqueray proposed zuul/zuul-registry master: Add OCI containers/storage support https://review.opendev.org/686512 | 02:52 |
| *** bhavikdbavishi has joined #zuul | 03:29 | |
| openstackgerrit | Ian Wienand proposed zuul/nodepool master: Validate openstack provider pool labels have top-level labels https://review.opendev.org/686514 | 03:30 |
| *** bhavikdbavishi1 has joined #zuul | 03:32 | |
| *** bhavikdbavishi has quit IRC | 03:33 | |
| *** bhavikdbavishi1 is now known as bhavikdbavishi | 03:33 | |
| *** jamesmcarthur has quit IRC | 03:34 | |
| *** jamesmcarthur has joined #zuul | 03:35 | |
| *** jamesmcarthur has quit IRC | 03:39 | |
| openstackgerrit | Ian Wienand proposed zuul/nodepool master: Validate openstack provider pool labels have top-level labels https://review.opendev.org/686514 | 03:53 |
| openstackgerrit | Ian Wienand proposed zuul/nodepool master: Validate openstack provider pool labels have top-level labels https://review.opendev.org/686514 | 04:00 |
| *** jamesmcarthur has joined #zuul | 04:05 | |
| *** jamesmcarthur has quit IRC | 04:12 | |
| *** gouthamr has quit IRC | 04:15 | |
| *** gouthamr has joined #zuul | 04:16 | |
| *** jamesmcarthur has joined #zuul | 05:08 | |
| *** jamesmcarthur has quit IRC | 05:13 | |
| *** raukadah is now known as chandankumar | 05:13 | |
| *** rlandy|bbl is now known as rlandy | 05:43 | |
| *** jamesmcarthur has joined #zuul | 06:09 | |
| *** jlk has joined #zuul | 06:12 | |
| *** jamesmcarthur has quit IRC | 06:15 | |
| *** spsurya has joined #zuul | 06:33 | |
| *** jamesmcarthur has joined #zuul | 07:11 | |
| *** pcaruana has joined #zuul | 07:15 | |
| *** jamesmcarthur has quit IRC | 07:16 | |
| *** tosky has joined #zuul | 07:18 | |
| *** hashar has joined #zuul | 07:26 | |
| *** jpena|off is now known as jpena | 07:38 | |
| openstackgerrit | Matthieu Huin proposed zuul/zuul master: Add OpenAPI description for enqueue, dequeue, autohold https://review.opendev.org/674257 | 07:53 |
| *** hashar_ has joined #zuul | 07:58 | |
| *** hashar has quit IRC | 08:00 | |
| *** jamesmcarthur has joined #zuul | 08:13 | |
| *** mhu has joined #zuul | 08:17 | |
| *** jamesmcarthur has quit IRC | 08:18 | |
| mhu | morning, I see my change to add sphinx-contrib/openapi to the projects under tenant zuul was merged (https://opendev.org/openstack/project-config/src/branch/master/zuul/main.yaml#L1685) but the project doesn't appear in http://zuul.openstack.org/projects | 08:19 |
| mhu | does zuul need a restart/reconfig? | 08:19 |
| openstackgerrit | Fabien Boucher proposed zuul/zuul master: Pagure - Support for branch creation/deletion https://review.opendev.org/685116 | 08:20 |
| *** zbr is now known as zbr|ruck | 08:23 | |
| *** hashar_ has quit IRC | 09:02 | |
| *** hashar has joined #zuul | 09:02 | |
| frickler | mhu: that's the url for the openstack tenant, check https://zuul.opendev.org/t/zuul/projects | 09:11 |
| mhu | frickler, ah thanks | 09:12 |
| *** jamesmcarthur has joined #zuul | 09:14 | |
| *** jamesmcarthur has quit IRC | 09:19 | |
| openstackgerrit | Matthieu Huin proposed zuul/zuul master: Zuul Web: add /api/user/authorizations endpoint https://review.opendev.org/641099 | 09:23 |
| *** rfolco has joined #zuul | 09:23 | |
| *** panda is now known as panda|bbl | 09:25 | |
| *** bhavikdbavishi has quit IRC | 09:40 | |
| *** hashar_ has joined #zuul | 09:50 | |
| *** hashar has quit IRC | 09:53 | |
| openstackgerrit | Fabien Boucher proposed zuul/zuul master: Gitlab - Basic handling of merge_requests event https://review.opendev.org/685990 | 10:00 |
| *** hashar_ has quit IRC | 10:02 | |
| *** mugsie has quit IRC | 10:03 | |
| *** mugsie has joined #zuul | 10:05 | |
| *** jamesmcarthur has joined #zuul | 10:15 | |
| *** jamesmcarthur has quit IRC | 10:19 | |
| *** saneax has joined #zuul | 10:23 | |
| *** badboy has joined #zuul | 10:26 | |
| *** saneax has quit IRC | 10:32 | |
| *** panda|bbl is now known as panda | 10:39 | |
| *** jamesmcarthur has joined #zuul | 10:47 | |
| *** jamesmcarthur has quit IRC | 10:51 | |
| mordred | tristanC: https://review.opendev.org/#/c/686542/ | 10:57 |
| *** jpena is now known as jpena|lunch | 11:00 | |
| jangutter_ | we've got a local hack on our openstack nodepool driver to bypass cleanupLeakedPorts and cleanupLeakedInstances. (long and convoluted story _why_)... | 11:07 |
| *** jangutter_ is now known as jangutter | 11:07 | |
| jangutter | I'd like to fix this upstream: there are two options. One is just to make it a config option that you can flip to turn off the leak cleanup... | 11:08 |
| jangutter | The second is to expose how long it you wait till you decide the port/instance has leaked (in seconds). | 11:08 |
| jangutter | any preference? I seem to remember there was some wish for the second one to move out the hardcoded values into config... | 11:09 |
| openstackgerrit | Monty Taylor proposed zuul/zuul-registry master: WIP Consume typing from openstacksdk and keystoneauth https://review.opendev.org/686403 | 11:12 |
| *** zbr|ruck is now known as zbr|lunch | 11:15 | |
| *** donnyd has joined #zuul | 11:18 | |
| *** panda is now known as panda|eat | 11:43 | |
| *** jamesmcarthur has joined #zuul | 11:48 | |
| *** hashar has joined #zuul | 11:52 | |
| *** EmilienM has quit IRC | 11:53 | |
| *** jamesmcarthur has quit IRC | 11:53 | |
| *** EmilienM has joined #zuul | 11:54 | |
| *** jpena|lunch is now known as jpena | 12:00 | |
| *** badboy has quit IRC | 12:07 | |
| *** spsurya has quit IRC | 12:10 | |
| tristanC | mordred: nice! Also I wasn't able to make it work speculatively in tox and used this command to test it instead: MYPYPATH=/home/fedora/openstacksdk/ ./.tox/pep8/bin/mypy --strict zuul_registry/ | 12:11 |
| pabelanger | ianw: in our case, auto detection won't work, we want to force python38 for testing reasons. auto will just find any python | 12:12 |
| pabelanger | ianw: cool, will see when next zuul release is for zuul_console is. However, I think I also see some warnings from ansible about callback plugin, maybe not python3 safe for other things | 12:13 |
| *** jamesmcarthur has joined #zuul | 12:15 | |
| tristanC | jangutter: both options could be implemented though, a provider cleanup toggle, and perhaps a cleanup-min-age ttl value | 12:16 |
| pabelanger | jangutter: at first guess, clean-floating-ips is a setting so could see other being one too. But also likey want to understand why in case some other issue | 12:17 |
| jangutter | pabelanger: it's because of ironic, ironically :-p | 12:18 |
| mordred | tristanC: it worked speculatively for me - or, at least - I did ".tox/pep8/bin/pip install -e ../../openstack/openstacksdk" and it worked | 12:19 |
| jangutter | pabelanger: if you don't enable the ironic neutron driver, the ports of baremetal nodes are _always_ marked down. So they helpfully get cleaned up even though the instance is still using them. | 12:19 |
| mordred | tristanC: if we wanted to see it in the gate, we'd need to make a job (or a version of the job) with ksa and openstacksdk in the required-projects list | 12:19 |
| mordred | jangutter: oh that's lovely | 12:20 |
| mordred | jangutter: I can see why that would be displeasing :) | 12:20 |
| jangutter | pabelanger: it's possible for me to clean up all that - but it's going to take me writing an HA tripleo agent for ironic agent.... or swap two lines in the nodepool driver :-p | 12:20 |
| jangutter | morded: it's kinda well known and there are ways around it, but it's also only a mild annoyance, except in this case. | 12:21 |
| jangutter | pabelanger: piggybacking off of clean-floating-ips is exactly what I did :0-p | 12:22 |
| jangutter | pabelanger: apologies for the really weird :-p typO. | 12:22 |
| *** rfolco has quit IRC | 12:24 | |
| *** rfolco has joined #zuul | 12:25 | |
| pabelanger | jangutter: maybe propose your patch, with extra info in commit and see what others thing. But in general, I wouldn't want to disable clean up features in nodepool, as it usually does a good job at keeping tenant / project resources free. I am sure others will likely reply too | 12:25 |
| jangutter | pabelanger: yep, cleanup is pretty useful - I'd actually like to keep the default behaviour the same with the option to tweak it added. | 12:28 |
| fungi | jangutter: for opendev's deployment we ran into nodepool not waiting long enough in one of our providers so was deleting the new ports because it took too long for server instances to get far enough along to be attached to them. after some reevaluation of the port cleanup routine we upped the wait time from 3 minutes to 10 minutes. that went into effect in nodepool 3.8.0. to what vintage of nodepool are you | 12:30 |
| fungi | running? | 12:30 |
| jangutter | fungi: it's the software-factory one - 3.8.0. Haven't checked to see lately if it checks if the ports are assigned to an instance rather than just down. | 12:32 |
| jangutter | fungi: they never go to UP, for the entire lifetime of the baremetal instance managed by ironic. | 12:33 |
| *** rlandy has joined #zuul | 12:34 | |
| *** nhicher has quit IRC | 12:35 | |
| *** nhicher has joined #zuul | 12:36 | |
| jangutter | fungi: ah, yeah, logic's still the same in tip. If the ports are DOWN they are eligible for cleanup. https://opendev.org/zuul/nodepool/src/branch/master/nodepool/driver/openstack/provider.py#L547 | 12:36 |
| *** panda|eat is now known as panda | 12:37 | |
| jangutter | please forgive noobie question: is there a way to crank up the verbosity of the ansible log for the zuul executor? | 12:41 |
| fungi | jangutter: got it, so the ports need to exist but will never be reported as "up" by the api? | 12:41 |
| jangutter | fungi: yeah, if you add the ironic neutron agent, or use the ironic neutron mechanism driver with supported HW, they can go 'up' | 12:42 |
| fungi | and i take it there's some reason not to do that | 12:42 |
| tristanC | jangutter: https://softwarefactory-project.io/docs/operator/zuul_operator.html#troubleshooting-the-executor | 12:42 |
| pabelanger | https://zuul-ci.org/docs/zuul/admin/components.html#id11 too | 12:43 |
| jangutter | tristanC: I tried that... ps -a says ansible-playbook is running with -vvv, but either I can't find the logs or somehow they get sanitized. | 12:44 |
| pabelanger | so, tried python38 on fedora-30 last night for ansible. Ran into issues with libselinux python bindings missing, guess fedora-32 will have them. Time to try out python38 PPA for bionic | 12:44 |
| tristanC | jangutter: oh right, you need to set level: DEBUG for the zuul logger in /etc/zuul/executor-logging.yaml | 12:44 |
| jangutter | fungi: yeah the two options to manage the ports are either I have to build a tripleo HA service for the ironic neutron agent, or we have to get actual supporting switches. | 12:45 |
| jangutter | thanks tristanC, gonna try that now. | 12:45 |
| *** jamesmcarthur has quit IRC | 12:47 | |
| *** jamesmcarthur has joined #zuul | 12:47 | |
| *** rlandy is now known as rlandy|mtg | 13:02 | |
| mnaser | hmm, zuul k8s with in-cluster config seems to not working properly | 13:24 |
| mnaser | i think it has to dowith two facts, k8s passes a few env variables (`KUBERNETES_SERVICE_HOST`, `KUBERNETES_SERVICE_PORT`) | 13:24 |
| mnaser | and also the `/var/run/secrets/kubernetes.io/serviceaccount` needs to be exposed inside bwrap | 13:25 |
| mnaser | i can imagine how the latter can actually be a security concern tho.. | 13:25 |
| mnaser | im not sure what can be a good path for that | 13:25 |
| *** zbr|lunch is now known as zbr|ruck | 13:25 | |
| tristanC | mnaser: fwiw nodepool k8s driver creates a service account, pass the token along with the node object and zuul-executor should setup a standalone (per-build) kubeconfig file | 13:37 |
| mnaser | tristanC: oh, that's not what ended up happening when i tried it the other day | 13:37 |
| tristanC | mnaser: hum, what happened then? :) | 13:38 |
| mnaser | i pretty much got a "cant find a config, au revoir" | 13:38 |
| mnaser | but i didn't dig more into it | 13:38 |
| mnaser | but now im seeing that function | 13:38 |
| tristanC | mnaser: could you share the logs of that behavior? | 13:40 |
| mnaser | i wonder if we're not pointing to the kubeconfig file? | 13:40 |
| mnaser | yeah, ill have to dig them again and rebuild things to use k8s | 13:40 |
| tristanC | mnaser: could it be that kubectl isn't installed in the zuul-executor image? | 13:40 |
| mnaser | it was the upstream one | 13:41 |
| mnaser | maybe that was it | 13:41 |
| mnaser | no but actually i remember cp-ing kubectl into the container with an initcontainer (as a hack) | 13:41 |
| mnaser | tristanC: shouldn't we set `kubectl_kubeconfig` ? | 13:41 |
| mnaser | we''re preparing the file in work_root/.kube/config | 13:42 |
| mnaser | but im not actually seeing us point to the config file we generate | 13:43 |
| tristanC | mnaser: the work_root should be the home of the zuul user inside the bwrap | 13:44 |
| mnaser | ah ok, so that implies things should work then | 13:44 |
| mnaser | ill dig a bit more | 13:44 |
| tristanC | mnaser: i can confirm this work, in some situation at least, here is a job running tox job with a kubectl connection: https://review.opendev.org/682049 | 13:45 |
| Shrews | tristanC: where did we land on the openshift job instability last night? Is https://review.opendev.org/686474 the proposed fix? | 14:24 |
| *** bhavikdbavishi has joined #zuul | 14:24 | |
| tristanC | Shrews: i'm not familiar enough with glean to tell if this is going to fix the issue. | 14:25 |
| pabelanger | I believe the issue was slow nested virt, at least what ianw was looking into | 14:26 |
| *** bhavikdbavishi1 has joined #zuul | 14:27 | |
| Shrews | pabelanger: we have MANY issues, i believe. that being one of them | 14:27 |
| Shrews | i'm hoping we can all put out heads together and wipe them out to get np tests back to stable | 14:28 |
| fungi | pabelanger: current theory there is that because it was a long stack of changes which got rebased, all the jobs those queued resulted in suboptimal performance for booting some of the cirros vms | 14:28 |
| *** bhavikdbavishi has quit IRC | 14:28 | |
| *** bhavikdbavishi1 is now known as bhavikdbavishi | 14:28 | |
| pabelanger | Ah, regarding glean, reading source TYPE does seem option, so unsure if that will have an impact to fix. We should also confirm with centos, I looked at latest fedora | 14:28 |
| tristanC | Shrews: well, it's not related to the openshift job, it's just that fedora instance on fortnebula doesn't have ipv4 address setup in the guest | 14:28 |
| Shrews | tristanC: has anyone spoken with donnyd about this yet? | 14:29 |
| fungi | tristanC: right, in that case it sounds like another instance of the kernel/networkmanager race on v6 autoconfiguration | 14:29 |
| donnyd | hi | 14:29 |
| tristanC | Shrews: well we could, but the cloud metadata are correct and centos do get the ipv4, thus it seems like a dib/glean issue that only affect fedora image | 14:30 |
| donnyd | which fedora | 14:30 |
| donnyd | is it all fedoras or just a particular version? | 14:30 |
| tristanC | donnyd: https://zuul.opendev.org/t/zuul/build/dc0765e1b481464e8d9823f3d5f7e4f0/log/zuul-info/zuul-info.launcher.txt#26 | 14:30 |
| fungi | basically if the kernel receives and acts on a v6 route advertisement before glean fires networkmanager, then nm will see there's already a v6 address assigned and will refuse to touch the interface, even to perform v4 address assignment | 14:30 |
| tristanC | donnyd: should show https://zuul.opendev.org/t/zuul/build/dc0765e1b481464e8d9823f3d5f7e4f0/log/zuul-info/inventory.yaml#32 | 14:31 |
| *** rlandy|mtg is now known as rlandy | 14:32 | |
| fungi | we've already done some tweaking to try and eliminate that race condition in opendev's images, but it's a very-long-standing nm bug which as far as i know has never been resolved | 14:32 |
| donnyd | well I upped the RA interval a while back to fix an issue with the def route being cleared and RA's not coming in fast enough to not fail the job | 14:32 |
| Shrews | tristanC: fungi: can we use an image other than fedora? would that help? | 14:33 |
| Shrews | i'm not sure why one is fedora and one is centos, tbh | 14:33 |
| fungi | i known basically nothing about openshift so i can't begin to guess | 14:33 |
| fungi | s/known/know/ | 14:33 |
| pabelanger | we should try to collect glean log for that job, it should be in /tmp | 14:34 |
| pabelanger | that will give some more info what is happening | 14:34 |
| fungi | i was assuming it was that it needed a newer kernel or some particular userspace tools available only on f29+ | 14:34 |
| donnyd | I can fire some test instances and report back if that is faster | 14:34 |
| tristanC | Shrews: fungi: i've only used openshift on centos, it should work on fedora, but with all the docker issue i don't know how it well | 14:34 |
| pabelanger | we've seen cases where glean doesn't run at the right time, and interfaces don't setup properly in the past | 14:35 |
| tristanC | Shrews: fungi: we are using fedora for the nodepool service which needs python3, perhaps we could switch to ubuntu but i don't know how to install k8s client there... | 14:35 |
| fungi | pabelanger: yeah, we should also collect the kernel log so we see messages from it about slaac autoconfiguration, to be able to compare timestamps and see if it's the same nm race as we've already been struggling with elsewhere | 14:36 |
| pabelanger | ++ | 14:36 |
| Shrews | fungi: yeah, my knowledge is zero as well | 14:37 |
| donnyd | also does this only effect fedora29 | 14:38 |
| donnyd | what about 28 or 30 | 14:38 |
| clarkb | Shrews: fungi jangutter maybe we should put port cleanup behind aflag like floating ip cleanup | 14:38 |
| Shrews | donnyd: i don't think we know the answer to that yet | 14:39 |
| pabelanger | IMO, we should at least bump to fedora-30, given it is the latest and we ually remove older versions from nodepool. | 14:39 |
| clarkb | and re thr NM issue ya the debian bug is like 4 years old | 14:39 |
| openstackgerrit | Fabien Boucher proposed zuul/zuul master: Gitlab - Basic handling of merge_requests event https://review.opendev.org/685990 | 14:39 |
| pabelanger | however, fedora also have changes to NM recently for DIB, so maybe we are dealing with that | 14:39 |
| Shrews | donnyd: i think all we know is that if fails consistently in FN with the current setup. other providers seem to be ok | 14:40 |
| fungi | https://bugs.debian.org/755202 suggests setting net.ipv6.conf.*.{autoconf,accept_ra}=0 as a workaround, so that the kernel won't do autoconf and then configure nm to do that instead, but i have no idea if we've tried it in opendev yet | 14:40 |
| openstack | Debian bug 755202 in network-manager "network-manager: keeps creating and using new connection "eth0" that does not work" [Important,Open] | 14:40 |
| Shrews | so if it was all a glean issue, seems like it would hit across other providers | 14:40 |
| openstackgerrit | Tristan Cacqueray proposed zuul/nodepool master: Switch to fedora-30 for the openshift integration job https://review.opendev.org/686737 | 14:40 |
| donnyd | well they don't use v6 as the primary means of communication | 14:40 |
| Shrews | is FN our only v6 provider? | 14:41 |
| donnyd | I think limestone may be using v6 as well | 14:41 |
| pabelanger | limestone is | 14:41 |
| pabelanger | IIRC | 14:41 |
| donnyd | can you get this job to run there too | 14:41 |
| jangutter | clarkb: yep, that's my idea - got side-tracked by some other shiny things, but should be sending some reviews next week. | 14:41 |
| fungi | nope, but not all providers use the same v6 config distribution, advertisement timings, have the same performance profiles, et cetera | 14:41 |
| fungi | and the bug i linked is a race condition | 14:42 |
| donnyd | fungi: we hit this in the beginning if I am not mistaken | 14:42 |
| pabelanger | we could also setup autohold for job, and inspect the node | 14:42 |
| fungi | donnyd: yeah, but not on the same distros | 14:42 |
| clarkb | fungi: I tested that and it did not work | 14:42 |
| pabelanger | but, logs should also expose the info too | 14:42 |
| clarkb | I believe ianw subsequently testedit tok | 14:42 |
| clarkb | this will be the third time we debug this issue | 14:43 |
| clarkb | quite a bit is known about it | 14:43 |
| fungi | clarkb: any idea if we tried setting that on the per-interface sysctls or only on the all sysctl (which only propagates on interface creation)? | 14:43 |
| pabelanger | clarkb: ack, I've missed that will defer to you :) | 14:44 |
| clarkb | fungi: I think I set it using the all sysctl when testing and not on the per interface sysctl | 14:44 |
| clarkb | it is easy enough to test if you manually boot nodes in fn | 14:44 |
| fungi | that may not have propagated to the actual interfaces then | 14:44 |
| clarkb | you get no ipv6 with that updated sysctl at all | 14:44 |
| clarkb | confirmed via ssh from second nodeoverprivate ipv4 | 14:45 |
| donnyd | http://paste.openstack.org/show/781087/ | 14:45 |
| fungi | clarkb: ahh, but did we configure networkmanager to do v6 autoconf? | 14:45 |
| donnyd | that is the log from the test instance | 14:45 |
| clarkb | fungi: I dont recall if glean is configuring it to explicity configure ipv6 via NM | 14:46 |
| donnyd | where should I look folr the glean log | 14:46 |
| fungi | if i'm reading the workaround there correctly, it's to tell the kernel not to do v6 autoconf and then use nm to do v6 autoconf instead | 14:46 |
| clarkb | donnyd: glean logs to syslog | 14:46 |
| donnyd | http://paste.openstack.org/show/781088/ | 14:46 |
| pabelanger | [ 9.033926] glean.sh[634]: DEBUG:glean:Writing output file : /etc/sysconfig/network-scripts/ifcfg-ens3 | 14:46 |
| pabelanger | that looks right | 14:46 |
| pabelanger | but we need to see the content | 14:46 |
| clarkb | fungi you can grab the ifcfg file ^ and check | 14:47 |
| donnyd | my manually fired instance works correctly | 14:47 |
| donnyd | http://paste.openstack.org/show/781089/ | 14:47 |
| donnyd | clarkb: ^^^^ | 14:47 |
| fungi | donnyd: since it's a race condition, i don't know whether we should expect it to fail consistently | 14:47 |
| Shrews | fungi: i don't think we've seen *any* successful runs on FN, so not too racey ? | 14:48 |
| clarkb | I love that NM_CONTROLLER is completly useless as a flag | 14:48 |
| pabelanger | odd, TYPE=ethernet is set there | 14:49 |
| fungi | Shrews: did they all fail on the missing f29 ipv4 address bug though? | 14:49 |
| pabelanger | but, looks as I would expect | 14:49 |
| fungi | can't rule out the possibility there's more than one problem causing those particular job failures | 14:49 |
| clarkb | IPV6_AUTOCONF defaults to true if IPV6FORWARDING is set to false | 14:52 |
| donnyd | logstash ```node_provider:"fortnebula-regionone" AND filename:"job-output.txt" AND message:"Upload logs to swift" AND build_node:"fedora-29"``` | 14:52 |
| clarkb | but the forwardinf flag doesnt seem to have a documented default | 14:52 |
| donnyd | shows only one success in the last day | 14:52 |
| Shrews | fungi: i'm not sure. tristanC was the one that pointed out the FN connection via https://zuul.opendev.org/t/zuul/builds?job_name=nodepool-functional-openshift&result=FAILURE | 14:53 |
| donnyd | https://75254dc8ba7993915a2c-0839d54e3d76bec33f8faada9b13d241.ssl.cf5.rackcdn.com/686515/1/check/nodepool-functional-openshift/02f127f/job-output.txt | 14:53 |
| donnyd | for example this one was successful | 14:53 |
| fungi | yeah, i think we need to update the job to collect kernel, nm and glean logs and then analyze the ones from a failed build there | 14:54 |
| fungi | and configs for them as well i guess | 14:55 |
| donnyd | fungi: this seems pretty similar to the issues we hit when FN was being brought online | 14:55 |
| fungi | yep | 14:55 |
| donnyd | where it worked... ish... when it wanted to | 14:56 |
| donnyd | I do however apologize for all the hassles FN has created for everyone though | 14:56 |
| fungi | again though, tuning timing parameters to address a race condition is just a workaround, and usually only reduces their frequency doesn't eliminate it | 14:56 |
| fungi | donnyd: i don't think it's creating hassles, just helping us find bugs | 14:57 |
| *** chandankumar is now known as raukadah | 14:57 | |
| donnyd | this one here is limestone | 14:59 |
| donnyd | https://zuul.opendev.org/t/zuul/build/7572c0cc839b4b49b70777b424821719 | 14:59 |
| fungi | anyway, it would be excellent if folks with a personal investment in using rh distros for those jobs would help work out what the solution is... all this started cropping up when we switched glean to use networkmanager, in service of "better" rh distro network configuration | 14:59 |
| jangutter | in some other facepalm-newbie news, we think we might have figured out why zuul isn't reconnecting to the newly reformatted baremetal node in our jobs. | 14:59 |
| donnyd | so could be ipv6 related | 14:59 |
| donnyd | oh but that is a different failure | 15:00 |
| openstackgerrit | James E. Blair proposed zuul/zuul-registry master: Fix merge error in streaming support https://review.opendev.org/686505 | 15:00 |
| jangutter | zuul seems to generate a per-job key and revokes the original ssh key from it's inventory. Unfortunately we didn't back up and restore the .ssh keys between the reformat, so our bet is that zuul has no way of reconnecting. | 15:00 |
| fungi | donnyd: yeah, as i said, we shouldn't rule out the possibility that there's more than one way those jobs are failing | 15:00 |
| clarkb | jangutter: ya that is done to avoid cross talk between jobs | 15:00 |
| jangutter | clarkb: just saving and restoring $ZUUL_HOME/.ssh should do the trick? | 15:01 |
| clarkb | jangutter: likely yes | 15:01 |
| corvus | jangutter: yeah that's probably best, but also that is all done by ansible in roles in the base job, so if you need to change something about that, it's "just" job changes, not zuul code changes. | 15:02 |
| corvus | jangutter: (for instance, depending on when you do your formatting, you might be able to just re-order it so it happens after the key change) | 15:02 |
| jangutter | corvus: yep, I saw that, but I'm also a mite hesitant about effectively not dropping privileges, especially since EVERYTHING has the same .ssh key. | 15:03 |
| clarkb | fungi: and ya we weretold NM is the way of the future on red hat distros so ianw started work early to get ahead of that switvh | 15:03 |
| pabelanger | neat! ansible seems to work under python38 :D | 15:03 |
| clarkb | if anyone at red hat knows how ti make ipv6 and ipv4 worl reliably with NM that input would be greatly appreciated | 15:04 |
| fungi | pabelanger: 3.8.0rc1? | 15:04 |
| clarkb | best I could come up with is "its broken and racey but increasing certain tineouts helps avoid it) | 15:04 |
| * fungi apparently needs to update his 3.8.0b3 build to 3.8.0rc1 now that it's a thing | 15:05 | |
| donnyd | well it would seem centos7 doesn't hit this issue | 15:05 |
| clarkb | donnyd: its all d own to startup timing | 15:06 |
| donnyd | so its not all RH distros, just more recent versions of NM right? | 15:06 |
| clarkb | centos has a different kernel and services that start on boot | 15:06 |
| donnyd | true | 15:06 |
| clarkb | if the timing works out such that NM starts before the kernel processes RAs it is fine | 15:06 |
| clarkb | and we influence that by setting a delay for the kernel to begin procesaing RAs | 15:07 |
| mordred | fungi: WOAH: https://docs.python.org/3.9/whatsnew/3.8.html#assignment-expressions | 15:07 |
| mordred | fungi: I don't know what language this is anymore :) | 15:07 |
| donnyd | can't we just force NM to overwrite even if it does have a current RA | 15:07 |
| clarkb | donnyd: I was unable to determine how to do that but possibly | 15:08 |
| jangutter | mordred: assignment expressions is why Guido left as BDFL. | 15:08 |
| *** arxcruz|ruck is now known as arxcruz|rover | 15:08 | |
| pabelanger | fungi: yah! pulled it from https://launchpad.net/~deadsnakes/+archive/ubuntu/ppa so I don't have to fight selinux | 15:08 |
| mordred | jangutter: mind blown | 15:08 |
| fungi | pabelanger: oh, i just build from source, but i don't do anything with selinux on debian so it's never been a problem for me | 15:09 |
| *** jamesmcarthur has quit IRC | 15:09 | |
| donnyd | Is there anything else I can help to T/S??? | 15:09 |
| fungi | jangutter: it brings back memories of c | 15:09 |
| clarkb | donnyd: I dont think so its aknown issue and needs someone that wants working NM on redhat platforms to dig in | 15:10 |
| clarkb | to figure out how to get NM to ignore the kernels possible config for an interface or some other solution | 15:10 |
| pabelanger | fungi: yah, fedora-30 ships with python3.8 beta1 (I think) everything worked in ansible, except things that needed libselinux. So moved to bionic, it just worked | 15:11 |
| openstackgerrit | Merged zuul/zuul-registry master: Fix merge error in streaming support https://review.opendev.org/686505 | 15:11 |
| tristanC | clarkb: i'm not sure to understand, how ipv6 stuff would prevent the ipv4 address to be assigned? | 15:11 |
| *** jamesmcarthur has joined #zuul | 15:11 | |
| pabelanger | that is good enough for me, until fedora-32 comes out | 15:12 |
| donnyd | maybe a sweet udev rule that will flush the current kernel table when it detects a config drive is attached would work | 15:12 |
| clarkb | tristanC: because NM will not configure an interface if it is already configured by something else | 15:12 |
| clarkb | tristanC: see fungi's debian bug link above | 15:12 |
| jangutter | fungi: I recently watched some C++20 stuff, I'm thinking the half-life of the "idiomatic form" of any language is now probably 2 years or so. | 15:13 |
| tristanC | clarkb: thanks, i'll have a look | 15:13 |
| donnyd | tristanC: I think udev could easily solve this issue. see https://github.com/clearlinux/micro-config-drive/pull/43/files | 15:15 |
| donnyd | I think with a rule that RUNS some sort of flush on the kernel RA's would in turn allow NM to function correctly in the event a config drive is connected to a system | 15:16 |
| donnyd | but it would only do it for things that are using config drive | 15:17 |
| donnyd | just a thought on how to fix it | 15:18 |
| *** donnyd is now known as donnyd_afk | 15:19 | |
| fungi | clarkb: tristanC: no idea if it will help but i'm spending some time looking through the networkmanager upstream defect tracker in hopes of finding a similar report about the problem there | 15:19 |
| fungi | since the networkmanager package maintainer in debian seems to have been unable to reproduce the problem (possibly due to lack of trying?) it doesn't appear anything was done to forward the report to the gnome community's tracker (but also it looks like they switched to gitlab issues a year ago, so maybe they just didn't import old reports?) | 15:21 |
| *** mattw4 has joined #zuul | 15:22 | |
| tristanC | fwiw, on my fedora the ifcfg file contains IPV6INIT=yes IPV6_AUTOCONF=yes IPV6_DEFROUTE=yes and IPV6_FAILURE_FATAL=no which doesn't seem to be set by glean | 15:27 |
| pabelanger | glean for redhat doesn't support ipv6, somebody needs to still add support | 15:29 |
| clarkb | right it relies on the kernel accepting RAs to make that function. Which probably extra complicates things with this NM behavior | 15:31 |
| clarkb | my understanding is this isn't an issue with NM not accepting RAs but instead the problem is NM will ignore any interface it believes is configured by some other entity | 15:32 |
| clarkb | that said we can edit glean to update the config and see if it helps | 15:32 |
| tristanC | also, cloud-init does set NETWORKING_IPV6 and IPV6_AUTOCONF in /etc/sysconfig/network for redhat system | 15:33 |
| tristanC | here is how a cloud-init based system network sysconfig file looks like: https://review.opendev.org/686749 | 15:38 |
| tristanC | perhaps this tricks NetworkManager into managing interface, even if it already has ip addresses? | 15:39 |
| mordred | tristanC: none of those settings look like *bad* ideas | 15:39 |
| clarkb | tristanC: left a comment on that | 15:42 |
| *** saneax has joined #zuul | 15:47 | |
| tristanC | clarkb: hum, but shouldn't we remove that 'if ipv6: continue' condition here: https://opendev.org/opendev/glean/src/branch/master/glean/cmd.py#L270 ? | 15:48 |
| mnaser | "just use ipv6, it solves all of our problems" they said | 15:48 |
| clarkb | tristanC: yes that is what pabelanger was talking about. red hat doesn't have support for this in glean today so those checks will have to be removed and the code updated to address it | 15:49 |
| *** rlandy is now known as rlandy|brb | 15:49 | |
| clarkb | mnaser: well if you don't use network manager it works great :) | 15:49 |
| clarkb | tristanC: also your latest patchset assumes the vlan case I think | 15:49 |
| mnaser | clarkb: i think that statement stands in general for ipv6 or not =P | 15:49 |
| pabelanger | clarkb: tristanC: was on list of things to do when working on openstack-infra, sadly never did finish it :( | 15:50 |
| tristanC | clarkb: i'm not sure to understand why we shouldn't enable ipv6 for every interface? | 15:51 |
| clarkb | tristanC: because the configuration is based on the config drive data. If that doesn't include ipv6 config we don't confiure ipv6 | 15:51 |
| tristanC | clarkb: but the network manager would just set link local address in that case | 15:52 |
| clarkb | tristanC: the whole point is to configure it as per the config drive | 15:53 |
| clarkb | and config drive may specific static addresses too | 15:53 |
| clarkb | or dhcpv6 | 15:53 |
| clarkb | you can't assume this single config | 15:53 |
| *** hashar has quit IRC | 15:56 | |
| tristanC | what happen if an interface has both ipv4 and ipv6? | 16:00 |
| *** donnyd_afk is now known as donnyd | 16:01 | |
| tristanC | i'm not familiar with the config drive format, is this documented somewhere? | 16:01 |
| clarkb | unfortunately I don't know that nova documents their network data json file | 16:01 |
| clarkb | if an interface has both ipv4 and ipv6 there are different entries in the json for it and glean should configure the interface for both | 16:02 |
| tristanC | oh, so we need to edit a single file for two different interfaces when it has both v4 and v6? | 16:03 |
| clarkb | you can share v4 and v6 on a single interface | 16:03 |
| *** tosky has quit IRC | 16:05 | |
| *** jamesmcarthur has quit IRC | 16:06 | |
| fungi | tristanC: clarkb: best doc i can find for network_data.json is https://docs.openstack.org/nova/latest/user/metadata.html#openstack-format-metadata | 16:07 |
| fungi | not exactly a formal specification, merely an example | 16:08 |
| fungi | chatting in #openstack-nova about it now | 16:11 |
| tristanC | what's confusing is that glean is processing an interface list that may contains 2 entries for the same interfaces that needs to be configured through a single file... | 16:11 |
| *** jamesmcarthur has joined #zuul | 16:11 | |
| tristanC | i'm tempted to merge the two entries into one to simplify the file template, but that's quite a refactor... | 16:12 |
| clarkb | tristanC: it may be more than 2 entries too because an interface can have multiple ipv4 and multiple ipv6 entries | 16:12 |
| tristanC | clarkb: multiple ipv4 address for a single interface doesn't seem to be supported by the write_redhat_interfaces procedure | 16:13 |
| clarkb | tristanC: k, we can likely ignore that case for now then | 16:14 |
| tristanC | and do we have to support dhcpv6 or static ipv6 address? | 16:15 |
| clarkb | the other platforms do, but as a first pass we can probably ignore those. None of our ipv6 only clouds currently rely on either method (rax relies on ipv6 + static but also has ipv4) | 16:15 |
| fungi | clarkb: tristanC: closest thing nova has to an api document for that file are these: https://specs.openstack.org/openstack/nova-specs/specs/liberty/implemented/metadata-service-network-info.html#rest-api-impact https://specs.openstack.org/openstack/nova-specs/specs/rocky/approved/multiple-fixed-ips-network-information.html#rest-api-impact | 16:17 |
| fungi | as far as stability of that api, they try to only ever add keys and not remove any, but also it rarely gets anything new added anyway | 16:17 |
| *** michael-beaver has joined #zuul | 16:24 | |
| Shrews | corvus: where is the wait-to-start.sh script the quick-start job uses to wait for mysql. we just hit that same error from yesterday | 16:25 |
| Shrews | it's referenced in docker-compose.yaml, but grep is failing me | 16:25 |
| tristanC | fungi: clarkb: pabelanger: then perhaps the last PS of https://review.opendev.org/#/c/686749/ might do the trick | 16:25 |
| fungi | Shrews: `git ls-files` in zuul/zuul says there's a doc/source/admin/examples/playbooks/wait-to-start.sh | 16:27 |
| fungi | is that what you're looking for? | 16:27 |
| Shrews | fungi: yes! thx | 16:27 |
| *** saneax has quit IRC | 16:27 | |
| corvus | Shrews: which cloud? | 16:27 |
| Shrews | corvus: limestone | 16:28 |
| corvus | Shrews: that's 2/2 on limestone | 16:28 |
| Shrews | \o/ | 16:28 |
| corvus | Shrews: hypothesis: something about limestone causes the inter-container networking in docker to be weird | 16:29 |
| Shrews | corvus: i'm going to set a hold on that job to see if we can explore some more | 16:30 |
| corvus | Shrews: ++ | 16:30 |
| Shrews | also, to use funky new autohold commands | 16:30 |
| clarkb | tristanC: couple of notes there | 16:30 |
| clarkb | tristanC: I think it is close but we need to get the interface type checks correct | 16:31 |
| *** rlandy|brb is now known as rlandy | 16:31 | |
| Shrews | corvus: i set autoholds for both np and zuul. playing the recheck game now | 16:37 |
| *** zbr|ruck has quit IRC | 16:39 | |
| openstackgerrit | Merged zuul/zuul master: web: render log manifest consistently https://review.opendev.org/686307 | 16:44 |
| *** zbr has joined #zuul | 16:50 | |
| *** jangutter has quit IRC | 16:56 | |
| *** jpena is now known as jpena|off | 17:08 | |
| *** bhavikdbavishi has quit IRC | 17:30 | |
| *** bhavikdbavishi has joined #zuul | 17:32 | |
| openstackgerrit | James E. Blair proposed zuul/zuul-registry master: DNM null commit for testing https://review.opendev.org/686791 | 17:40 |
| openstackgerrit | James E. Blair proposed zuul/zuul-registry master: DNM: second null commit for more testing https://review.opendev.org/686793 | 17:42 |
| fungi | clarkb: tristanC: for a bit more history on nm's coexistence with kernel v6 autoconf, the discussion in https://bugzilla.gnome.org/show_bug.cgi?id=682932 seems relevant | 17:47 |
| openstack | Gnome bug 682932 in IP and DNS config "settings: harmonize IPv6 methods with IPv4" [Normal,Assigned] - Assigned to psimerda | 17:47 |
| tristanC | corvus: i'm working on adding skopeo support to the zuul-registry | 17:52 |
| clarkb | fungi: the "no touching" stuff seems particularly related. Its odd that this would be the behavior tough if your intent is a working interface | 17:52 |
| clarkb | but also we do tell NM to touch the device via sysconfig | 17:52 |
| clarkb | but maybe the NM sysconfig plugin isn't a sufficiently strong trigger to override the RAs the kernel got? | 17:53 |
| corvus | tristanC: cool. yesterday i started working on impleminting the proxying needed for shadowing; i may be busy with operational stuff today but will continue that next week. i haven't hit any road-blocks so far. | 17:53 |
| fungi | i also found references to [ipv6] method=ignore for interface configs, but that's not mentioned in the NetworkManager.conf(5) manpage on my modern systems | 17:53 |
| corvus | clarkb: ^ related, i can now answer questions about docker registry authentication. i understand why (and still think it's dumb) | 17:54 |
| corvus | clarkb: (the really short version is that getting a token (ie "authenticating") is required even for anonymous pulls because they designed it that way. it's even true for our intermediate registry, which we can now pull from using an "anonymous" user with no password (anon-ftp forever)) | 17:55 |
| fungi | so there's code in zuul-registry to grant tokens? | 17:58 |
| openstackgerrit | Tristan Cacqueray proposed zuul/zuul-registry master: Add type annotations https://review.opendev.org/686249 | 18:02 |
| openstackgerrit | Tristan Cacqueray proposed zuul/zuul-registry master: Add support for skopeo copy https://review.opendev.org/686803 | 18:02 |
| clarkb | fungi: it is possible the basic auth stuff also just works? | 18:02 |
| tristanC | corvus: and regarding type annotations, could we land it or should i rebase without it? | 18:02 |
| corvus | tristanC: we can land it -- i just left some review comments on ps5 so you can see my thoughts -- i'm still really concerned that i won't be able to contribute effectively (or enjoy contributing) with mypy. it seems very difficult to use -- especially the more you use python for what it's good at. i still feel like i'd be happier using a language designed this way from the start like c++ or rust. but i | 18:06 |
| corvus | promised to give it a real try, so i will +2 it with reservations. :) | 18:06 |
| corvus | tristanC: and thank you for doing this. :) | 18:07 |
| corvus | tristanC: oh wait i spotted something in the types change | 18:08 |
| *** rfolco is now known as rfolco|bbl | 18:12 | |
| openstackgerrit | James E. Blair proposed zuul/zuul-registry master: DNM null commit for testing https://review.opendev.org/686791 | 18:12 |
| openstackgerrit | James E. Blair proposed zuul/zuul-registry master: DNM: second null commit for more testing https://review.opendev.org/686793 | 18:13 |
| mordred | corvus, tristanC: even with rust and c++ there is a trend towards allowing for a greater amount of type inferrance and only using explicit types when needed ... so I think learning when adding annotations is useful and when it's not will be a key to this being enjoyable | 18:15 |
| corvus | mordred: yeah, it's those variable types that bug/worry me the most. i'm assuming they were required since tristanC is using strict? | 18:15 |
| mordred | dunno - and yeah - the function ones feel like an immediate win. the variable ones are ... interesting | 18:16 |
| corvus | tristanC: just to be clear, my -1 on 686249 is for the not_found change; i'll +2 it after that and we can continue playing with it | 18:17 |
| * mordred goes to eat fish | 18:17 | |
| clarkb | corvus: the inline with assignment types? | 18:21 |
| corvus | clarkb: yeah | 18:21 |
| clarkb | I wonder if those become more readable (easy to accept too maybe?) if done in a more C like style of specifying type and declaring the var early before using it? | 18:24 |
| *** jamesmcarthur has quit IRC | 18:25 | |
| tristanC | corvus: understood, i think it's worth a try on this new project, and i'd be happy to help explain how it works. Then if it's too cumbersome, then we could drop the strictness and only use it where it's not confusing | 18:25 |
| tristanC | clarkb: yes, mypy is often confused when new variables are introduced in if then block for example | 18:26 |
| corvus | i don't mind declaring variables that way if that improves things all around | 18:27 |
| tristanC | mordred: right, but because python isn't compiled, i think it's really hard to infer function type. iiuc, only Haskell can infer function types effectively, thus i would push for that language instead :-) | 18:27 |
| corvus | tristanC: your skopeo change will need to install skopeo | 18:34 |
| tristanC | corvus: replied to the not_found thing. The method currently doesn't return anything, thus it shouldn't matter to return None after right? | 18:34 |
| corvus | tristanC: the patch i'm working on extends it to return things | 18:35 |
| corvus | tristanC: right now, because you can't configure shadow, it always raises an exception, but the api is exception or returned data. | 18:35 |
| tristanC | alright, let me check if we can disable that checks, mypy doesn't let you return func() if func returns none | 18:36 |
| corvus | aroo? | 18:36 |
| corvus | "def func(): return None; return func()" is valid | 18:36 |
| Shrews | and totes useful function, too :) | 18:37 |
| tristanC | and it fails with: zuul_registry/main.py:121: error: "not_found" of "RegistryAPI" does not return a value | 18:37 |
| corvus | None is a value :) | 18:37 |
| corvus | tristanC: do you maybe have to say not_found returns Optional something? | 18:38 |
| tristanC | Optional[T] is just sugar for Union[None, T] | 18:38 |
| tristanC | corvus: https://github.com/python/mypy/issues/6549 | 18:39 |
| corvus | okay | 18:40 |
| corvus | so mypy has crossed from type checking into code style | 18:40 |
| corvus | and i *strongly* object to that | 18:40 |
| *** jamesmcarthur has joined #zuul | 18:40 | |
| tristanC | fair enough, then i don't mind abandoning 686249 | 18:42 |
| corvus | a type checker would say "this function returns none, and the function you are calling returns none, therefore, the types match" | 18:42 |
| tristanC | may i reply that to the issue? :) | 18:43 |
| corvus | tristanC: sure :) | 18:44 |
| openstackgerrit | Tristan Cacqueray proposed zuul/zuul-registry master: Add support for skopeo copy https://review.opendev.org/686803 | 18:49 |
| openstackgerrit | Tristan Cacqueray proposed zuul/zuul-registry master: Add type annotations https://review.opendev.org/686249 | 18:49 |
| tristanC | in the meantime i've switch rebase order of the above change to get skopeo test in sooner | 18:49 |
| *** bhavikdbavishi has quit IRC | 18:50 | |
| *** pcaruana has quit IRC | 18:51 | |
| tristanC | facebook/pyre-check does not like # type: ignore comments, google/pytype does not like empty list when a function returns List[str], and Microsoft/pyright needs nodejs :-) | 18:58 |
| openstackgerrit | Tim Burke proposed zuul/zuul-registry master: Rework the stream_blob/stream_object API https://review.opendev.org/686827 | 19:00 |
| SpamapS | corvus: tristanC I'm just catching up, but very curious about this function thing. Is it something in https://review.opendev.org/#/c/686249/ ? | 19:07 |
| tristanC | SpamapS: yes, we have to https://review.opendev.org/#/c/686249/7/zuul_registry/main.py@139 | 19:09 |
| SpamapS | corvus: you say you feel that mypy is doing style checking instead of type checking, but I'm just not seeing it, even though I want to see it. :) | 19:09 |
| clarkb | SpamapS: https://github.com/python/mypy/issues/6549 is related | 19:10 |
| SpamapS | just saw that | 19:10 |
| clarkb | SpamapS: basically they are going the extra step to say "this particular type signature is bad so we won't allow it" rather than simply checking the type signature matches how it is used | 19:10 |
| SpamapS | the comment about "assumes that the return value of methods that return None should not be used" sounds like pure BS | 19:10 |
| tristanC | SpamapS: not necessarly, it depends on how 'None' is currently represented in mypy design | 19:12 |
| SpamapS | https://github.com/python/mypy/issues/6549#issuecomment-538525937 | 19:12 |
| SpamapS | I responded | 19:12 |
| SpamapS | it's crap | 19:12 |
| SpamapS | Saying you can't use None is like saying you can't use any class. Everything in python is an object. Period. | 19:13 |
| *** jamesmcarthur has quit IRC | 19:14 | |
| clarkb | haskell even encodes this into a Maybe because it is so common https://wiki.haskell.org/Maybe | 19:15 |
| SpamapS | Option<T> is the rust equivalent, I think. | 19:17 |
| tristanC | SpamapS: it seems like NoneType is special in python, e.g. "not even defined in the standard library" | 19:18 |
| clarkb | tristanC: types.NoneType exists at least | 19:21 |
| tristanC | clarkb: not on my python-3.7 | 19:24 |
| clarkb | oh interesting did they remove that in 3? | 19:26 |
| SpamapS | Yeah I think because None is a singleton it doesn't make much sense. | 19:26 |
| SpamapS | -> None conveys exactly what is expected | 19:26 |
| *** hashar has joined #zuul | 19:27 | |
| tristanC | it seems like None is special in python, for example on the repl, "None" doesn't display anything. Thus maybe mypy does a special case for it resulting in weird behavior such as the one described in the above issue | 19:28 |
| SpamapS | Yeah I'm suggesting it definitely should never be a special case. | 19:29 |
| SpamapS | And I think that's what corvus was on about too. | 19:29 |
| tristanC | in Haskell, Nothing is a real type, and the repl doesn't swallow it when it is returned | 19:29 |
| tristanC | SpamapS: yeah I agree mypy shouldn't impose such rules that is not related to type check, but perhaps there is something more tricky to make it work... | 19:31 |
| SpamapS | Also Guido doesn't like my candor. ;) | 19:31 |
| corvus | whatever comes of this, it's a successful learning exercise | 19:35 |
| *** saneax has joined #zuul | 19:36 | |
| *** hashar has quit IRC | 19:37 | |
| *** hashar has joined #zuul | 19:37 | |
| *** panda is now known as panda|off | 19:38 | |
| SpamapS | corvus: agreed, and to be fair, I've not had this particular problem, despite mypy'ing a lot of code the last year. | 19:41 |
| corvus | i really like the 'call this function and stop processing' paradigm i guess. but then, i also like to put "if foo is None: continue" on one line because i think it's one thing, so i'm clearly not one of the cool kids. | 19:44 |
| tristanC | corvus: also that particular problem will go away once not_found does return something :-) | 19:45 |
| corvus | tristanC: yes, i'll work on it from that angle :) | 19:45 |
| corvus | SpamapS: is there a bar where python 1.5.2 programmers go to drink? i should be there | 19:46 |
| *** zbr has quit IRC | 19:46 | |
| clarkb | I guess they want you to use exceptions more? | 19:48 |
| corvus | funny story -- i have python code running at home that's been in continuous production for so long that it entirely skipped "new-style" classes. it uses "class foo:" because "class foo(object):" hadn't been invented yet, and now suddenly it's practially py3 ready with no work on my part! | 19:48 |
| clarkb | corvus: I think the lesson there is if we ignore our problems they solve themselves :) | 19:49 |
| corvus | that is absolutely what i have taken from that | 19:50 |
| fungi | if you sit by the river long enough, you will watch the bodies of your enemies float by (proverb mis-attributed to all manner of historical figures) | 19:57 |
| *** saneax has quit IRC | 20:01 | |
| corvus | fungi the great | 20:04 |
| corvus | (a hysterical figure) | 20:04 |
| fungi | the 9 o'clock show is different from the 7 o'clock. tell all your friends, and don't forget to tip your waiters | 20:13 |
| *** EmilienM is now known as EvilienM | 20:18 | |
| *** EvilienM is now known as containerizes_hi | 20:19 | |
| *** containerizes_hi is now known as containerized | 20:19 | |
| SpamapS | corvus: at least you aren't having to watch that code be ported to ruby. ;) | 20:19 |
| *** containerized is now known as EvilienM | 20:19 | |
| openstackgerrit | Tristan Cacqueray proposed zuul/zuul-registry master: Add support for skopeo copy https://review.opendev.org/686803 | 20:29 |
| *** jamesmcarthur has joined #zuul | 20:33 | |
| *** jamesmcarthur has quit IRC | 20:44 | |
| *** jamesmcarthur has joined #zuul | 20:46 | |
| *** hashar has quit IRC | 20:50 | |
| *** tosky has joined #zuul | 21:31 | |
| *** jamesmcarthur has quit IRC | 21:34 | |
| *** rlandy has quit IRC | 21:40 | |
| *** armstrongs has joined #zuul | 21:51 | |
| *** EvilienM is now known as EmilienM | 21:52 | |
| *** armstrongs has quit IRC | 22:03 | |
| *** mattw4 has quit IRC | 22:53 | |
| *** rfolco|bbl has quit IRC | 22:54 | |
| *** tosky has quit IRC | 23:39 | |
Generated by irclog2html.py 2.15.3 by Marius Gedminas - find it at mg.pov.lt!