*** amoralej|off is now known as amoralej | 06:39 | |
opendevreview | Dr. Jens Harbott proposed openstack/project-config master: Stop testing github hosted projects in openstack tenant https://review.opendev.org/c/openstack/project-config/+/884563 | 08:07 |
---|---|---|
*** brunomuniz_ is now known as brunomuniz | 12:01 | |
*** amoralej is now known as amoralej|lunch | 12:08 | |
*** amoralej|lunch is now known as amoralej | 13:09 | |
clarkb | corvus: frickler asks a good question on https://review.opendev.org/c/openstack/project-config/+/884563 why do the other merge modes result in failure but this one does not if it is already broken (via the default)? | 15:32 |
clarkb | frickler: I don't think those jobs are failing to run on the merge issue https://zuul.opendev.org/t/openstack/build/d614292d0e704eaabc361950f6b96ec8/log/job-output.txt#5174 | 15:36 |
clarkb | I'm trying to figure out where those error messages originate in zuul and not immeditaely finding them. Ther are the more generic "github doesn't support this mode" errors but the project specific ones seem more hidden | 15:40 |
clarkb | https://opendev.org/zuul/zuul/src/branch/master/zuul/configloader.py#L2682-L2701 I think that is the source | 15:41 |
clarkb | and ultimately that uses https://opendev.org/zuul/zuul/src/branch/master/zuul/driver/github/githubconnection.py#L1803-L1837 to determine the valid set | 15:46 |
clarkb | I don't understand why merge mode rebase fails but merge mode merge does not if merge is the default | 15:47 |
frickler | clarkb: I was referring to the third party checks https://zuul.opendev.org/t/openstack/builds?project=ansible%2Fansible&skip=0 though they haven't run for even 3y | 15:47 |
frickler | rebase is a different thing in github | 15:47 |
clarkb | yes reabse is different but if merge is a failure as is I would expect setting it explicitly to fail too so I'm a bit confused about that. But now I see the github api request to make in order to see the details | 15:48 |
clarkb | using both a new personal access token and a classic token I am unable to view that merge mode information for either ansible/ansible or a personal repo | 16:05 |
clarkb | I suspect this means I don't have sufficient permissions and wonder if that is the problem with zuul too. Anyway I think I see an update that we can do to log the allowed merge modes to help debugging further | 16:05 |
clarkb | remote: https://review.opendev.org/c/zuul/zuul/+/884904 Add more info to merge mode selection errors | 16:16 |
corvus | clarkb: i think you asked that yesterday and i talked about it in the meeting | 16:35 |
corvus | clarkb: frickler because the error is not new | 16:35 |
frickler | clarkb: I get this when using graphql https://paste.opendev.org/show/br6bim5YUttWRJtM9w2E/ | 16:35 |
clarkb | corvus: oh even if ou explicitly set it in a config update its doing a delta against the old thing and skipping it? | 16:36 |
clarkb | frickler: intersting since an earlier patchset said that squash isn't supported | 16:37 |
corvus | yes, zuul only reports on errors introduced by the change | 16:37 |
corvus | this change sets the current value to the current value, so there is no change to the error condition | 16:37 |
clarkb | I think that 884904 will give us better information for what zuul is getting back. It may be that zuul isn't getting any merge info back | 16:39 |
clarkb | in that case we either need new permissions or a fallback maybe? | 16:40 |
clarkb | but I think having the debugging change will give good info | 16:40 |
fungi | i wouldn't be surprised if zuul lacks permission to see what merge modes are configured for those projects | 16:40 |
clarkb | ya though frickler managed to get permissions for it | 16:41 |
clarkb | maybe the information moved out of the rest api and into graphql | 16:41 |
clarkb | that would be annoying | 16:41 |
clarkb | but that could explain why I don't get that info via the rest api | 16:41 |
frickler | note the paste shows IIUC that "merge" isn't supported, but "squash-merge" is. zuul was erroring when I proposed the latter, so it is either receiving no/wrong information from github or acting wrongly on it | 16:43 |
*** amoralej is now known as amoralej|off | 16:43 | |
clarkb | frickler: yup that is why I think my extra info change will be helpful. Definitely something weird happening | 16:44 |
clarkb | and best to have zuul tell us directly what is going on | 16:45 |
frickler | I'm pretty sure from when I looked at this a year ago or so that you can get the info only via graphql, so if github is using API only, it is bound to fail and likely hasn't worked for a long time at least, if ever | 16:51 |
frickler | s/github/zuul/ | 16:52 |
frickler | so maybe zuul's handling of these errors has changed? | 16:52 |
frickler | clarkb: so I'm pretty sure your debug patch will just show an empty list | 16:53 |
frickler | and I don't think that that will be very helpful | 16:53 |
clarkb | frickler: well it would confirm we aren't getting hte infomration from the api naymore I think that is useful? | 16:53 |
clarkb | https://opendev.org/zuul/zuul/src/branch/master/zuul/driver/github/githubconnection.py#L1803-L1837 is what would be producing the empty list | 16:53 |
clarkb | if that happens then we can investigate further including converting that query to graphql | 16:54 |
clarkb | I don't think deleting the projects is appropriate in that situation either | 16:55 |
clarkb | (we should fix zuul not paper over the config error) | 16:55 |
corvus | if no one is using the projects, why not delete them? | 16:57 |
clarkb | corvus: we (opendev) are using them | 16:57 |
clarkb | we could decide to stop using them if we want to (this is for the ansible development job to try and catch early errors) | 16:58 |
corvus | oh i thought frickler said they weren't run in 3 years | 16:58 |
clarkb | we could also modify those jobs to fetch the code bases externally to Zuul's git caches | 16:58 |
clarkb | corvus: the openstack ansible module integration third party CI hasn't been running. But opendev's future ansible job runs on almost every change to system-config (and I'm pretty sure it installs future testinfra there too) | 16:59 |
frickler | clarkb: but that opendev jobs doesn't depend on what I'm deleting? just on the project being defined in main.yaml? | 17:00 |
clarkb | frickler: I think merge mode always applies? | 17:00 |
clarkb | basically if we remove the project specific definitions we'll use the default merge mode of merge which we believe to be invalid and would need to set it. That involves a project setting? | 17:01 |
frickler | only if running against ansible/ansible changes? | 17:01 |
clarkb | hrm ya maybe since there is no speculative state. I guess that aspect isn't clear to me | 17:01 |
clarkb | what about if we did a depends-on? | 17:01 |
frickler | not sure. but if it mattered, I'd expect to see some config error on curl -L \ -H "Accept: application/vnd.github+json" \ -H "Authorization: Bearer <YOUR-TOKEN>"\ -H "X-GitHub-Api-Version: 2022-11-28" \ https://api.github.com/repos/OWNER/REPO | 17:02 |
frickler | meh, wrong paste | 17:02 |
frickler | https://review.opendev.org/c/openstack/project-config/+/884563 | 17:02 |
clarkb | ya I guess we can merge 884563 and find out through experimental processes | 17:03 |
clarkb | I'm ok with that | 17:03 |
frickler | btw. that accidental paste is the API query you could try with your pat. or is that what you already did? at least from the api ref the info should still be there | 17:04 |
clarkb | frickler: that is what I already did and it didn't include any merge info key value pairs | 17:05 |
clarkb | but I'm not sure if that is a token scoping/access problem or a limitation of the rest api | 17:05 |
clarkb | the rest api docs still show the merge info in the example responses at least | 17:05 |
clarkb | fwiw i we land 884563 and 884904 then we can push a revert for 884563 to see the extra info around merge modes so I don't think the two changes conflict in that way | 17:06 |
frickler | I just tried with a new token, added public_repo access to the token. with that I can see the info on my personal repo, but not for ansible/ansible | 17:09 |
clarkb | frickler: is that a classic token or a pat? | 17:09 |
clarkb | either way maybe they have made that information more private in rest (seems like you can get it via graphql though) | 17:10 |
corvus | if it ends up being a perm issue, then in normal circumstances, we'd ask the repo for the correct perms. being able to set something in the case where you don't want to talk to repo owners would be nice, but if it comes at the cost of missing a config error in the normal case (ie, if we just allow anything if our list is empty, that doesn't let us warn users that the list is, in fact, empty in the case where they aren't expecting it) i don't | 17:14 |
corvus | know if i'm excited about that. | 17:14 |
frickler | clarkb: classic | 17:15 |
frickler | the page says "Personal access tokens (classic)" | 17:16 |
frickler | so may be the same thing? | 17:16 |
clarkb | frickler: ok I only tested with the new beta pats when talking to my own repo. I can retry against my repo using the classic token | 17:17 |
clarkb | corvus: ya a better option may be to use graphql if it is less restricted | 17:17 |
clarkb | then we still get the info but don't need as much coordination | 17:17 |
corvus | clarkb: we're kinda getting into #zuul:opendev.org territory here, but i've been trying to maintain a wide range of github api compatability since github enterprise often lags the public site, and individual installations of GHE might be older still. | 17:19 |
corvus | so any changes like that i think we should be careful about | 17:20 |
clarkb | good point on both fronts. We can probably delay this conversation until we generate more data via the debug patch and take it from there | 17:20 |
corvus | (i mean, it's generally cool to add new stuff that might only be supported by github.com, but we should be careful not to take away stuff from older ghe releases) | 17:20 |
frickler | could be optional if the response from the API is empty | 17:32 |
frickler | after a lot of fiddling with the quoting this works for me now: curl -H "Authorization: bearer TOKEN" -X POST -d '{ "query": "query { repository(name: \"ansible\", owner: \"ansible\") { squashMergeAllowed } } " }' https://api.github.com/graphql | 17:32 |
frickler | but that's it for me today | 17:34 |
tonyb | Somewhat random question ... what version of ansible do we use? | 18:24 |
clarkb | tonyb: I think it is the same as zuul 6. | 18:28 |
clarkb | nope its 7 | 18:28 |
clarkb | system-config/playbooks/roles/install-ansible/tasks/main.yaml has a <8 pin | 18:28 |
tonyb | Thanks. I admit that was a very different answer than I was expecting :) | 18:33 |
fungi | zuul tries to stay current with ansible releases, and keeps us on a bit of a treadmill there since we can't really keep using versions (at least on the executors) that zuul drops support for | 18:34 |
fungi | looks like ansible 8 was supposed to reach ga this month? | 18:35 |
tonyb | I'm asking because I'm trying to run configure-mirrors against a bunch of conatiners and the handler failed because the task invokes command with a 'warn' arg which isn't supported in my ansible (2.14) so is that a thing we need to clean up? | 18:36 |
fungi | and ansible 5 is due to go eol this month | 18:36 |
tonyb | I crated a venv with 2.10 and the handler runs "just fine" | 18:37 |
fungi | any idea when in between there it dropped support for that? | 18:39 |
fungi | just wondering why it's not breaking on us | 18:39 |
tonyb | Yeah I can look, that's why I asked about versions. I don't quite grok how 2.10/2.14 fit in with the 5, 7 and 8 that you're talking about. Seems like they chnaged versioning | 18:41 |
fungi | tonyb: https://docs.ansible.com/ansible/latest/reference_appendices/release_and_maintenance.html | 18:43 |
fungi | ansible 2.10 was the point at which ansible community and ansible core versioning diverged | 18:43 |
fungi | ansible core 2.15 is included in ansible community 7.x | 18:43 |
fungi | right now zuul supports ansible community 6-7 (core 2.13-2.14) and community 8 (core 2.15) is supposed to release any day now | 18:45 |
tonyb | Okay I understand now thanks. | 18:46 |
tonyb | I still don't understand why it isn't breaking everywhere | 18:46 |
tonyb | more resarch needed | 18:46 |
clarkb | tonyb: configure mirrors will run under ansible 6 since that is zuul ansible | 18:48 |
clarkb | fungi: I don't think zuul does ansible 7 | 18:49 |
fungi | oh, i bet we haven't transitioned to ansible 7 on the executors yet | 18:49 |
tonyb | Ahh okay and that means ansibke-core 2.13 | 18:49 |
fungi | does it not? with 6 going eol later this year, i guess it's something we'll need to revisit soon | 18:49 |
tonyb | so it possibly will break upon the switch from 6 -> 7 | 18:50 |
clarkb | fungi: yes its been on my backlog to look into that but with everything else going on... | 18:50 |
fungi | i'm still digging around trying to find where the zuul docs say what versions of ansible it works with | 18:51 |
clarkb | probably under the version selector setting | 18:51 |
fungi | i didn't see it mentioned there | 18:52 |
clarkb | manage-ansible can list them | 18:54 |
clarkb | but ya not seeing it in the docs | 18:54 |
clarkb | and that is determined by zuul/lib/ansible-config.conf | 18:54 |
opendevreview | Tony Breeds proposed zuul/zuul-jobs master: [configure-mirrors] Allow per distribution disabling of mirrors https://review.opendev.org/c/zuul/zuul-jobs/+/884935 | 19:54 |
opendevreview | Tony Breeds proposed opendev/base-jobs master: Disable the use of Fedora mirrors https://review.opendev.org/c/opendev/base-jobs/+/884936 | 19:55 |
clarkb | tonyb: I think the linter didn't like your zuul-jobs change | 20:10 |
tonyb | Ahh of course the linter | 20:20 |
tonyb | I'll fix that | 20:20 |
tonyb | I can also address your base-test comment too (which makes sense) | 20:21 |
corvus | tonyb: there is an unimplemented plan for a very flexible mirror configuration that would be a very natural fit for the use case i think you are addressing: https://zuul-ci.org/docs/zuul-jobs/latest/mirror.html | 20:34 |
corvus | i don't think anyone is working on that; if you're interested in taking it up, i think it would be very welcome :) | 20:35 |
tonyb | Assuming configure-mirrors was updated to use that new structure of variable, OpenDev would just omit fedora from the mirror_info to get the desired outcome? | 20:39 |
clarkb | oh is that still not implemented. I guess finally switching to that is an option. | 20:41 |
clarkb | I had completely forgotten about that | 20:41 |
corvus | tonyb: exactly | 20:44 |
tonyb | it is ceratinly neater | 20:51 |
* tonyb will ponder on a run | 20:51 |
Generated by irclog2html.py 2.17.3 by Marius Gedminas - find it at https://mg.pov.lt/irclog2html/!