Infrastructure/ansible
5
0
Fork 0
Code Issues Pull requests 3 Projects Releases Packages Wiki Activity Actions

copr-be: conditionally redirect backend URLs to Pulp #2437

Merged
ryanlerch merged 1 commit from pulp-redirect into main 2025-01-30 13:54:30 +00:00
Conversation 29 Commits 1 Files changed 7 +71 -7
frostyx commented 2025-01-18 14:33:16 +00:00
Contributor
Copy link

See https://github.com/fedora-copr/copr/issues/3504

See https://github.com/fedora-copr/copr/issues/3504
zuul commented 2025-01-18 14:33:25 +00:00
First-time contributor
Copy link

Merge Failed.

This change or one of its cross-repo dependencies was unable to be automatically merged with the current state of its repository. Please rebase the change and upload a new patchset.
Warning:
Error merging pagure.io/fedora-infra/ansible for 2437,236dd5c8588e7639849595226e82c7413e0688c2

Merge Failed. This change or one of its cross-repo dependencies was unable to be automatically merged with the current state of its repository. Please rebase the change and upload a new patchset. Warning: Error merging pagure.io/fedora-infra/ansible for 2437,236dd5c8588e7639849595226e82c7413e0688c2
frostyx commented 2025-01-18 14:33:27 +00:00
Author
Contributor
Copy link

@praiskup PTAL

@praiskup PTAL
frostyx commented 2025-01-18 14:37:02 +00:00
Author
Contributor
Copy link

rebased onto a0720a8029

rebased onto a0720a802945f4fa6d6850e9221bd83cbd4bb1a8
zuul commented 2025-01-18 14:37:10 +00:00
First-time contributor
Copy link

Merge Failed.

This change or one of its cross-repo dependencies was unable to be automatically merged with the current state of its repository. Please rebase the change and upload a new patchset.
Warning:
Error merging pagure.io/fedora-infra/ansible for 2437,1715dce35dc0deeb1b24392943f7b792a88b9f4c

Merge Failed. This change or one of its cross-repo dependencies was unable to be automatically merged with the current state of its repository. Please rebase the change and upload a new patchset. Warning: Error merging pagure.io/fedora-infra/ansible for 2437,1715dce35dc0deeb1b24392943f7b792a88b9f4c
kevin commented 2025-01-20 23:27:27 +00:00
Contributor
Copy link

rebased onto 9495fde3f4

rebased onto 9495fde3f44c2f31ff643241367c95f1266297d8
kevin commented 2025-01-20 23:27:37 +00:00
Contributor
Copy link

rebased onto 9495fde3f4

rebased onto 9495fde3f44c2f31ff643241367c95f1266297d8
zuul commented 2025-01-20 23:30:43 +00:00
First-time contributor
Copy link

Build failed. More information on how to proceed and troubleshoot errors available at https://fedoraproject.org/wiki/Zuul-based-ci
https://fedora.softwarefactory-project.io/zuul/buildset/22dbb9540d094a13844f188a46faa653

Build failed. More information on how to proceed and troubleshoot errors available at https://fedoraproject.org/wiki/Zuul-based-ci https://fedora.softwarefactory-project.io/zuul/buildset/22dbb9540d094a13844f188a46faa653 - [fi-ansible-lint-diff ](https://fedora.softwarefactory-project.io/zuul/build/7177fa5f56ed4b9ab479a8afd3479c2a) : FAILURE in 3m 00s - [fi-yamllint-diff ](https://fedora.softwarefactory-project.io/zuul/build/ce38e244f8b44e1cb41e94b81612bc15) : SUCCESS in 2m 16s
praiskup commented 2025-01-21 08:27:47 +00:00
Owner
Copy link

Hmpf, is this redirecting even if the file doesn't physically exist?

Hmpf, is this redirecting even if the file doesn't _physically_ exist?
praiskup commented 2025-01-21 08:33:50 +00:00
Owner
Copy link

Just two nits:

  • ansible docs say ad mode: "Adding a leading zero (for example, 0755) works sometimes, but can fail in loops and some other circumstances."
  • people in fedora infra seem to prefer templates to end with .j2, see 5bdfe4b1e5
Just two nits: - ansible docs say ad mode: "Adding a leading zero (for example, 0755) works sometimes, but can fail in loops and some other circumstances." - people in fedora infra seem to prefer templates to end with `.j2`, see 5bdfe4b1e5e4185d66a901496712941116b5488f
praiskup commented 2025-01-21 08:35:01 +00:00
Owner
Copy link

I'd wish to have this hardcoded somewhere in vars, and de-hardcode PULP_CONTENT_URL too.

I'd wish to have this hardcoded somewhere in vars, and de-hardcode PULP_CONTENT_URL too.
praiskup commented 2025-01-21 08:37:07 +00:00
Owner
Copy link

Uh, this might only be truth for repeated reads of the same blobs (which we don't, we read just once), right? Seems to me that initially, sqlite needs to read the blobs just the way we do anyway...

Uh, this might only be truth for repeated reads of the same blobs (which we don't, we read just once), right? Seems to me that initially, sqlite needs to read the blobs just the way we do anyway...
praiskup commented 2025-01-21 08:42:50 +00:00
Owner
Copy link

Should we strop :.* from dirnames, too? To make the redirect work, e.g., for https://copr-be.cloud.fedoraproject.org/results/@copr/copr-pull-requests:pr:3527/?

Can you please test that both of these work:

https://copr-be.cloud.fedoraproject.org/results/@copr/copr-pull-requests:pr:3527/
https://copr-be.cloud.fedoraproject.org/results/%40copr/copr-pull-requests:pr:3527/
Should we strop `:.*` from dirnames, too? To make the redirect work, e.g., for `https://copr-be.cloud.fedoraproject.org/results/@copr/copr-pull-requests:pr:3527/`? Can you please test that both of these work: ``` https://copr-be.cloud.fedoraproject.org/results/@copr/copr-pull-requests:pr:3527/ https://copr-be.cloud.fedoraproject.org/results/%40copr/copr-pull-requests:pr:3527/
praiskup commented 2025-01-21 08:45:15 +00:00
Owner
Copy link

Of course, Pagure :-( https://pagure.io/pagure/issue/4468 , replace the whitespace with colon - "pr" - colon.

Of course, Pagure :-( https://pagure.io/pagure/issue/4468 , replace the whitespace with colon - "pr" - colon.
praiskup commented 2025-01-21 08:50:25 +00:00
Owner
Copy link

Super pedantic: the txt variable name is annoying, perhaps filename_redirect_projects?

Super pedantic: the `txt` variable name is annoying, perhaps filename_redirect_projects?
frostyx commented 2025-01-23 09:15:11 +00:00
Author
Contributor
Copy link

I think so, yes.

$ curl -I https://copr-be-dev.cloud.fedoraproject.org/results/frostyx/test-pulp/fedora-40-x86_64/repodata/foo.xml
HTTP/2 301
location: https://pulp.stage.devshift.net/api/pulp-content/copr/frostyx/test-pulp/fedora-40-x86_64/repodata/foo.xml
I think so, yes. ``` $ curl -I https://copr-be-dev.cloud.fedoraproject.org/results/frostyx/test-pulp/fedora-40-x86_64/repodata/foo.xml HTTP/2 301 location: https://pulp.stage.devshift.net/api/pulp-content/copr/frostyx/test-pulp/fedora-40-x86_64/repodata/foo.xml ```
frostyx commented 2025-01-23 09:31:32 +00:00
Author
Contributor
Copy link

Honestly, I don't know. We can do some experiment in python to find out.

Honestly, I don't know. We can do some experiment in python to find out.
frostyx commented 2025-01-23 10:08:47 +00:00
Author
Contributor
Copy link

There is currently some problem with building into CoprDirs so I cannot properly test the redirect but I think it doesn't work

curl -I https://copr-be-dev.cloud.fedoraproject.org/results/@copr/pulp-test:custom:foo/fedora-41-x86_64/repodata/repomd.xml
HTTP/2 404

The "@" in the URL doesn't cause any issues

[jkadlcik@hive doc]$ curl -I https://copr-be-dev.cloud.fedoraproject.org/results/@copr/pulp-test/fedora-41-x86_64/repodata/repomd.xml
HTTP/2 301
location: https://pulp.stage.devshift.net/api/pulp-content/copr/@copr/pulp-test/fedora-41-x86_64/repodata/repomd.xml

[jkadlcik@hive doc]$ curl -I https://copr-be-dev.cloud.fedoraproject.org/results/%40copr/pulp-test/fedora-41-x86_64/repodata/repomd.xml
HTTP/2 301
location: https://pulp.stage.devshift.net/api/pulp-content/copr/@copr/pulp-test/fedora-41-x86_64/repodata/repomd.xml

I was surprised by this, because I don't handle this in the script. Turns out, lighthttpd does that for us, and gives us lighty.env["uri.path"] with %40 already converted to @.

There is currently some problem with building into CoprDirs so I cannot properly test the redirect but I think it doesn't work ``` curl -I https://copr-be-dev.cloud.fedoraproject.org/results/@copr/pulp-test:custom:foo/fedora-41-x86_64/repodata/repomd.xml HTTP/2 404 ``` The "@" in the URL doesn't cause any issues ``` [jkadlcik@hive doc]$ curl -I https://copr-be-dev.cloud.fedoraproject.org/results/@copr/pulp-test/fedora-41-x86_64/repodata/repomd.xml HTTP/2 301 location: https://pulp.stage.devshift.net/api/pulp-content/copr/@copr/pulp-test/fedora-41-x86_64/repodata/repomd.xml [jkadlcik@hive doc]$ curl -I https://copr-be-dev.cloud.fedoraproject.org/results/%40copr/pulp-test/fedora-41-x86_64/repodata/repomd.xml HTTP/2 301 location: https://pulp.stage.devshift.net/api/pulp-content/copr/@copr/pulp-test/fedora-41-x86_64/repodata/repomd.xml ``` I was surprised by this, because I don't handle this in the script. Turns out, lighthttpd does that for us, and gives us `lighty.env["uri.path"]` with `%40` already converted to `@`.
frostyx commented 2025-01-23 10:17:24 +00:00
Author
Contributor
Copy link

rebased onto ce4a415eb0

rebased onto ce4a415eb0a652d69f02ac4d0948bc4220947749
frostyx commented 2025-01-23 10:18:28 +00:00
Author
Contributor
Copy link

I fixed the CoprDirs redirects

I fixed the CoprDirs redirects
frostyx commented 2025-01-23 10:18:44 +00:00
Author
Contributor
Copy link

Thank you for the review @praiskup. All feedback addressed, PTAL.

Thank you for the review @praiskup. All feedback addressed, PTAL.
zuul commented 2025-01-23 10:20:04 +00:00
First-time contributor
Copy link

Build failed. More information on how to proceed and troubleshoot errors available at https://fedoraproject.org/wiki/Zuul-based-ci
https://fedora.softwarefactory-project.io/zuul/buildset/3e8794b97c9143babe945670d92f97c3

Build failed. More information on how to proceed and troubleshoot errors available at https://fedoraproject.org/wiki/Zuul-based-ci https://fedora.softwarefactory-project.io/zuul/buildset/3e8794b97c9143babe945670d92f97c3 - [fi-ansible-lint-diff ](https://fedora.softwarefactory-project.io/zuul/build/e34a33c964cd4a66a1f10ea4edbfe246) : FAILURE in 2m 23s - [fi-yamllint-diff ](https://fedora.softwarefactory-project.io/zuul/build/af58d588ff944837889f4eaa1a336a95) : SUCCESS in 2m 00s
frostyx commented 2025-01-23 10:22:54 +00:00
Author
Contributor
Copy link

But I think we have to do the redirect only for .rpm files which are not .src.rpm.

But I think we have to do the redirect only for `.rpm` files which are not `.src.rpm`.
praiskup commented 2025-01-28 12:25:40 +00:00
Owner
Copy link

I don't think we have to, why? From lighttpd perspective, src.rpm is just RPM as any other..?

I don't think we have to, why? From lighttpd perspective, src.rpm is just RPM as any other..?
praiskup commented 2025-01-28 12:25:52 +00:00
Owner
Copy link

LGTM

LGTM
praiskup commented 2025-01-28 12:25:54 +00:00
Owner
Copy link

LGTM

LGTM
praiskup commented 2025-01-30 13:53:48 +00:00
Owner
Copy link

rebased onto fb06fb5d5c

rebased onto fb06fb5d5c20c70327a06a463c4f00e2516fa86c
praiskup commented 2025-01-30 13:54:00 +00:00
Owner
Copy link

rebased onto fb06fb5d5c

rebased onto fb06fb5d5c20c70327a06a463c4f00e2516fa86c
praiskup commented 2025-01-30 13:54:31 +00:00
Owner
Copy link

Pull-Request has been merged by praiskup

Pull-Request has been merged by praiskup
zuul commented 2025-01-30 13:56:26 +00:00
First-time contributor
Copy link

Build failed. More information on how to proceed and troubleshoot errors available at https://fedoraproject.org/wiki/Zuul-based-ci
https://fedora.softwarefactory-project.io/zuul/buildset/fd14441007b94f37aacd64cdc42a06d2

Build failed. More information on how to proceed and troubleshoot errors available at https://fedoraproject.org/wiki/Zuul-based-ci https://fedora.softwarefactory-project.io/zuul/buildset/fd14441007b94f37aacd64cdc42a06d2 - [fi-ansible-lint-diff ](https://fedora.softwarefactory-project.io/zuul/build/be2ecea616be442c85baf5bbb3bd4e10) : FAILURE in 2m 21s - [fi-yamllint-diff ](https://fedora.softwarefactory-project.io/zuul/build/21b531ee4b1c40e48a89b0652b761daf) : SUCCESS in 2m 07s
Sign in to join this conversation.
Reviewers
No reviewers
Labels
Clear labels   
freeze-break-request

   
post-freeze
No labels
freeze-break-request
post-freeze
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
4 participants
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference: Infrastructure/ansible#2437
No description provided.
Delete branch "pulp-redirect"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?