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

OpenShift: Allow to configure "host" in the "route" manifest #200

Closed
titou10titou10 opened this issue Dec 25, 2020 · 5 comments · Fixed by #207
Closed

OpenShift: Allow to configure "host" in the "route" manifest #200

titou10titou10 opened this issue Dec 25, 2020 · 5 comments · Fixed by #207
Assignees
Labels
openshift This issue/PR is related to OpenShift deployments only RFE 🙏 Request For Enhancements
Milestone

Comments

@titou10titou10
Copy link

The title says it all

Currently there is no way to configure the "host" variable in the OCPRoute. It is possible when nexus is exposed via anIngress

Maybe the usage of the "host" parameter, mandatory forIngress, could be extended as optionnal forRoute host name?

Would you be able to assist in testing this feature if implemented?
yes

@titou10titou10 titou10titou10 added the RFE 🙏 Request For Enhancements label Dec 25, 2020
@LCaparelli
Copy link
Member

@titou10titou10 thanks for the request! I agree it would be a nice addition.

@ricardozanini should be as simples as setting spec.image on the route definition, any objections? :-)

@LCaparelli LCaparelli added this to the v0.6.0 milestone Dec 26, 2020
@LCaparelli LCaparelli self-assigned this Dec 26, 2020
@LCaparelli LCaparelli added the openshift This issue/PR is related to OpenShift deployments only label Dec 26, 2020
@ricardozanini
Copy link
Member

@LCaparelli not at all, go ahead if you will! :)
@titou10titou10 many thanks for opening this, sorry for the delayed reply. (vacations and so on 🙉)

@bdurrow
Copy link
Contributor

bdurrow commented Jan 20, 2021

@LCaparelli, I don't understand your comment about spec.image, this would be spec.host.

To mention a few more related things that are not quite working properly:

  • When the nexus operator creates an openshift route from a nexus.apps.m88i.io CRD there are several listed "Resources" including the deployment, pod, service etc. The route is not included in this list. I have spent some time trying to figure out how this list is compiled but I haven't found that in the documentation so I'm not sure how to fix this
  • Openshift routes can be directed to a particular router with route labels but there is no way to assign the route a label from the nexus.apps.m88i.io spec. One might think you can simply add the label later but after a route is admitted it is not removed from a router by adding a label that would normally exclude it at creation.
  • Health checks/probes in the spec are incomplete, they do not include the ability to define a startupProbe

I am happy to add new RFEs for these deltas but I thought some of them might have a close enough adjacency to this RFE that they could be added at the same time with little incremental effort.

I plan to submit PRs for at least some of these that are not covered by this RFE. Thank you for your hard work!

@LCaparelli
Copy link
Member

Hey @bdurrow thanks for your interest!

@LCaparelli, I don't understand your comment about spec.image, this would be spec.host.

Absolutely, brain hiccup on my part. Thanks for pointing it out, I did mean spec.host :-)

To mention a few more related things that are not quite working properly:

  • When the nexus operator creates an openshift route from a nexus.apps.m88i.io CRD there are several listed "Resources" including the deployment, pod, service etc. The route is not included in this list. I have spent some time trying to figure out how this list is compiled but I haven't found that in the documentation so I'm not sure how to fix this

Could you please elaborate on this? I'm not sure which list you're mentioning, but it sounds like routes should be there too. Perhaps the exact command and output where you see this could help.

  • Openshift routes can be directed to a particular router with route labels but there is no way to assign the route a label from the nexus.apps.m88i.io spec. One might think you can simply add the label later but after a route is admitted it is not removed from a router by adding a label that would normally exclude it at creation.

I was not aware of this behavior of OCP routers, where even if you change the label of a route, it cannot be bound to another router after its creation. I attempted to reproduce this locally, but I'm facing some issues when attempting to spin up the second router.

It seems rather strange, not very declarative. Have you been able to reproduce this behavior with routes which were not created by the operator?

I'm concerned that the problem here is that once you change the route's label it initiates a reconciliation, which in turn causes the operator to overwrite your labels with the default one it generates, thus undoing your configuration. After some discussion we're considering making the routes "immutable" by the operator, with it simply facilitating its existence and never making any subsequent changes to the route itself (leaving it open to the user's changes).

Though this would not fix the problem you're describing if this overwrite is not the underlying cause and indeed routes cannot be reassigned to different routers by changing its labels. This would require extending the existing CRD API to provide means to assign the labels at creation time.

  • Health checks/probes in the spec are incomplete, they do not include the ability to define a startupProbe

Fair point, we should definitely add a startupProbe.

I am happy to add new RFEs for these deltas but I thought some of them might have a close enough adjacency to this RFE that they could be added at the same time with little incremental effort.

I plan to submit PRs for at least some of these that are not covered by this RFE. Thank you for your hard work!

I believe we can manage it all through this issue, just me let know your thoughts on the points I raised above. :-)

@ricardozanini
Copy link
Member

@bdurrow @titou10titou10, like @LCaparelli, said we are working on the possibility of not updating the created Routes during the reconciliation loop. This is because the Route object can be highly customizable and cumbersome to keep track of all the user changes in our CR/controller. It would be better to let the Route controllers take care of it.

That said, our controller will:

  1. Create the "default" route if needed on OpenShift environments
  2. Will check if it exists during the reconciliation loop. If not, we will create it again

This way, a user can either let the Nexus Operator keep the default route's state. Any customizations can be done later by an external process, such as a CI/CD.

And of course, a user can always turn off the Route creation in our CRD and create it by themselves.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
openshift This issue/PR is related to OpenShift deployments only RFE 🙏 Request For Enhancements
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants