Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Route ingress type on OCP should point to web pods instead of api/content pods. #1140

Closed
Denney-tech opened this issue Nov 7, 2023 · 3 comments

Comments

@Denney-tech
Copy link
Contributor

Denney-tech commented Nov 7, 2023

Version
OpenShift: 4.12.33
pulp-operator: 1.0.0-beta.3
galaxy-minimal: 4.8.1
galaxy-web: 4.8.1

Describe the bug
When using the Route ingress type, pulp-operator creates multiple routes that point to the API/Content services with various root urls. While this works to a degree, any page refresh leads to a 404 error. No web pods are created with this ingress type.

To Reproduce
Create a pulp resource with ingress type set to Route.

Expected behavior
A single Route pointed to a web pod service.

Additional context
From the logs, refreshing the page leads to badly formed urls in the GET requests. The current page gets joined to /static/galaxy/index.html{{current_page}} (or similar). I can reproduce and capture log output if required.

Using the LoadBalancer ingress type will create web pods and related service. A route can then be manually created to point to this service, and no 404 errors occur when refreshing pages

From what I can tell, this requires changes to at least the following files, but is a bit out of my comfort zone to make a PR:
controllers/ocp/route.go
controllers/repo_manager/ingress.go
controllers/repo_manager/controller.go

@dkliban
Copy link
Member

dkliban commented Nov 8, 2023

Thank you for opening the issue. We do not recommend using this feature of the operator. Please manually create ingress that rewrites the requests as done here: https://github.com/ansible/galaxy_ng/blob/master/galaxy_ng/app/webserver_snippets/nginx.conf

In the near future, there is going to be a separate repository that will host an Ansible based operator that is specifically for deploying Galaxy. Perhaps this issue could be addressed there. This repository is not public at this time, but once it is, we will notify the community about it.

@git-hyagi
Copy link
Collaborator

Just as a note, this problem is happening because OCP routes does not support regex in path:
https://access.redhat.com/solutions/4830261
https://issues.redhat.com/browse/RFE-2810

if it did, we could do something like:

spec:
  host: pulp.lab
  path: /ui(/|$)(.*)
  port:
    targetPort: api-24817
  tls:
    insecureEdgeTerminationPolicy: Redirect
    termination: edge
  to:
    kind: Service
    name: pulp-api-svc
    weight: 100

As a workaround, to manually configure the route and pulp-web as you described:

HOSTNAME=pulp.lab
PULP_CR=pulp

kubectl patch pulp $PULP_CR --type='json' -p='[{"op": "remove", "path": "/spec/ingress_type"}]'
kubectl patch pulp $PULP_CR --type merge -p '{"spec": {"web": {"replicas": 1}}}'


oc apply -f-<<EOF
apiVersion: route.openshift.io/v1
kind: Route
metadata:
  annotations:
    haproxy.router.openshift.io/timeout: 180s
  name: galaxy
spec:
  host: $HOSTNAME
  port:
    targetPort: web-8080
  tls:
    insecureEdgeTerminationPolicy: Redirect
    termination: edge
  to:
    kind: Service
    name: ${PULP_CR}-web-svc
    weight: 100
  wildcardPolicy: None
EOF

@Denney-tech
Copy link
Contributor Author

In the near future, there is going to be a separate repository that will host an Ansible based operator that is specifically for deploying Galaxy. Perhaps this issue could be addressed there. This repository is not public at this time, but once it is, we will notify the community about it.

In that case, this is all a bit of a moot point. I was suspicious that I was missing something somewhere considering that there are galaxy images in the quay.io/ansible namespace instead of the pulp namespace, but those won't deploy with the pulp-operator (tried lol). Makes sense that there's some other operator out there that is not GA. I'll make do with the workaround until the other operator becomes available.

@Denney-tech Denney-tech closed this as not planned Won't fix, can't repro, duplicate, stale Nov 8, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants