Wednesday, 2023-05-31

*** amoralej|off is now known as amoralej06:39
opendevreviewDr. Jens Harbott proposed openstack/project-config master: Stop testing github hosted projects in openstack tenant
*** brunomuniz_ is now known as brunomuniz12:01
*** amoralej is now known as amoralej|lunch12:08
*** amoralej|lunch is now known as amoralej13:09
clarkbcorvus: frickler asks a good question on why do the other merge modes result in failure but this one does not if it is already broken (via the default)?15:32
clarkbfrickler: I don't think those jobs are failing to run on the merge issue
clarkbI'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 hidden15:40
clarkb I think that is the source15:41
clarkband ultimately that uses to determine the valid set15:46
clarkbI don't understand why merge mode rebase fails but merge mode merge does not if merge is the default15:47
fricklerclarkb: I was referring to the third party checks though they haven't run for even 3y15:47
fricklerrebase is a different thing in github 15:47
clarkbyes 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 details15:48
clarkbusing 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 repo16:05
clarkbI 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 further16:05
clarkbremote: Add more info to merge mode selection errors16:16
corvusclarkb: i think you asked that yesterday and i talked about it in the meeting16:35
corvusclarkb: frickler because the error is not new16:35
fricklerclarkb: I get this when using graphql
clarkbcorvus: oh even if ou explicitly set it in a config update its doing a delta against the old thing and skipping it?16:36
clarkbfrickler: intersting since an earlier patchset said that squash isn't supported16:37
corvusyes, zuul only reports on errors introduced by the change16:37
corvusthis change sets the current value to the current value, so there is no change to the error condition16:37
clarkbI 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 back16:39
clarkbin that case we either need new permissions or a fallback maybe?16:40
clarkbbut I think having the debugging change will give good info16:40
fungii wouldn't be surprised if zuul lacks permission to see what merge modes are configured for those projects16:40
clarkbya though frickler managed to get permissions for it16:41
clarkbmaybe the information moved out of the rest api and into graphql16:41
clarkbthat would be annoying16:41
clarkbbut that could explain why I don't get that info via the rest api16:41
fricklernote 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 it16:43
*** amoralej is now known as amoralej|off16:43
clarkbfrickler: yup that is why I think my extra info change will be helpful. Definitely something weird happening16:44
clarkband best to have zuul tell us directly what is going on16:45
fricklerI'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 ever16:51
fricklerso maybe zuul's handling of these errors has changed?16:52
fricklerclarkb: so I'm pretty sure your debug patch will just show an empty list16:53
fricklerand I don't think that that will be very helpful16:53
clarkbfrickler: well it would confirm we aren't getting hte infomration from the api naymore I think that is useful?16:53
clarkb is what would be producing the empty list16:53
clarkbif that happens then we can investigate further including converting that query to graphql16:54
clarkbI don't think deleting the projects is appropriate in that situation either16:55
clarkb(we should fix zuul not paper over the config error)16:55
corvusif no one is using the projects, why not delete them?16:57
clarkbcorvus: we (opendev) are using them16:57
clarkbwe 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
corvusoh i thought frickler said they weren't run in 3 years16:58
clarkbwe could also modify those jobs to fetch the code bases externally to Zuul's git caches16:58
clarkbcorvus: 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
fricklerclarkb: but that opendev jobs doesn't depend on what I'm deleting? just on the project being defined in main.yaml?17:00
clarkbfrickler: I think merge mode always applies?17:00
clarkbbasically 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
frickleronly if running against ansible/ansible changes?17:01
clarkbhrm ya maybe since there is no speculative state. I guess that aspect isn't clear to me17:01
clarkbwhat about if we did a depends-on?17:01
fricklernot 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" \
fricklermeh, wrong paste17:02
clarkbya I guess we can merge 884563 and find out through experimental processes17:03
clarkbI'm ok with that17:03
fricklerbtw. 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 there17:04
clarkbfrickler: that is what I already did and it didn't include any merge info key value pairs17:05
clarkbbut I'm not sure if that is a token scoping/access problem or a limitation of the rest api17:05
clarkbthe rest api docs still show the merge info in the example responses at least17:05
clarkbfwiw 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 way17:06
fricklerI 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/ansible17:09
clarkbfrickler: is that a classic token or a pat?17:09
clarkbeither way maybe they have made that information more private in rest (seems like you can get it via graphql though)17:10
corvusif 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't17:14
corvusknow if i'm excited about that.17:14
fricklerclarkb: classic17:15
fricklerthe page says "Personal access tokens (classic)"17:16
fricklerso may be the same thing?17:16
clarkbfrickler: ok I only tested with the new beta pats when talking to my own repo. I can retry against my repo using the classic token17:17
clarkbcorvus: ya a better option may be to use graphql if it is less restricted17:17
clarkbthen we still get the info but don't need as much coordination17:17
corvusclarkb: we're kinda getting into 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
corvusso any changes like that i think we should be careful about17:20
clarkbgood point on both fronts. We can probably delay this conversation until we generate more data via the debug patch and take it from there17:20
corvus(i mean, it's generally cool to add new stuff that might only be supported by, but we should be careful not to take away stuff from older ghe releases)17:20
fricklercould be optional if the response from the API is empty17:32
fricklerafter 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 } } " }'
fricklerbut that's it for me today17:34
tonybSomewhat random question ... what version of ansible do we use?18:24
clarkbtonyb: I think it is the same as zuul 6.18:28
clarkbnope its 718:28
clarkbsystem-config/playbooks/roles/install-ansible/tasks/main.yaml has a <8 pin18:28
tonybThanks.  I admit that was a very different answer than I was expecting :)18:33
fungizuul 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 for18:34
fungilooks like ansible 8 was supposed to reach ga this month?18:35
tonybI'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
fungiand ansible 5 is due to go eol this month18:36
tonybI crated a venv with 2.10 and the handler runs "just fine"18:37
fungiany idea when in between there it dropped support for that?18:39
fungijust wondering why it's not breaking on us18:39
tonybYeah 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 versioning18:41
fungiansible 2.10 was the point at which ansible community and ansible core versioning diverged18:43
fungiansible core 2.15 is included in ansible community 7.x18:43
fungiright now zuul supports ansible community 6-7 (core 2.13-2.14) and community 8 (core 2.15) is supposed to release any day now18:45
tonybOkay I understand now thanks.18:46
tonybI still don't understand why it isn't breaking everywhere18:46
tonybmore resarch needed18:46
clarkbtonyb: configure mirrors will run under ansible 6 since that is zuul ansible18:48
clarkbfungi: I don't think zuul does ansible 718:49
fungioh, i bet we haven't transitioned to ansible 7 on the executors yet18:49
tonybAhh okay and that means ansibke-core 2.1318:49
fungidoes it not? with 6 going eol later this year, i guess it's something we'll need to revisit soon18:49
tonybso it possibly will break upon the switch from 6 -> 718:50
clarkbfungi: yes its been on my backlog to look into that but with everything else going on...18:50
fungii'm still digging around trying to find where the zuul docs say what versions of ansible it works with18:51
clarkbprobably under the version selector setting18:51
fungii didn't see it mentioned there18:52
clarkbmanage-ansible can list them18:54
clarkbbut ya not seeing it in the docs18:54
clarkband that is determined by zuul/lib/ansible-config.conf18:54
opendevreviewTony Breeds proposed zuul/zuul-jobs master: [configure-mirrors] Allow per distribution disabling of mirrors
opendevreviewTony Breeds proposed opendev/base-jobs master: Disable the use of Fedora mirrors
clarkbtonyb: I think the linter didn't like your zuul-jobs change20:10
tonybAhh of course the linter20:20
tonybI'll fix that20:20
tonybI can also address your base-test comment too (which makes sense)20:21
corvustonyb: 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:
corvusi 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
tonybAssuming 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
clarkboh is that still not implemented. I guess finally switching to that is an option.20:41
clarkbI had completely forgotten about that20:41
corvustonyb: exactly20:44
tonybit is ceratinly neater20:51
* tonyb will ponder on a run20:51

Generated by 2.17.3 by Marius Gedminas - find it at!