Configure ingressController to use routeSelector and configure external ingress service #157
Conversation
There was a problem hiding this comment.
I've noted one additional change we need to make here.
Since you're going to be making changes anyway, I would suggest making the commit messages slightly more descriptive:
-
For the "Change ingressController to use routeSelector", consider adding to the commit body our reasons for that change.
-
For "Configure address for external ingress service", it's probably worth noting which fields in particular we're adding with this change (and why), since otherwise it's not obvious.
In either or both cases, consider adding a link to the relevant documentation if you think it's appropriate (see e.g. 480edd0 for an example of how I typically do that).
|
I've removed my requested change (apparently I am unable to read), but I would still like to see the commit messages updated. Sorry for the noise! |
| @@ -15,6 +15,6 @@ spec: | |||
| nodeSelector: | |||
| matchLabels: | |||
| nerc.mghpcc.org/external-ingress: 'true' | |||
| namespaceSelector: | |||
| routeSelector: | |||
| matchLabels: | |||
| type: external | |||
There was a problem hiding this comment.
Actually, here is one more change I think we should make. I think that type is probably too generic. Let's (a) pick a more descriptive name, and (b) let's namespace it like we have for other locally relevant labels.
How about:
routerSelector:
matchLabels:
nerc.mghpcc.org/external-ingress: "true"
This is the same label we use to identify nodes that host the external ingress address (and placement for the external ingress service).
There was a problem hiding this comment.
@larsks good points, updated the commit messages and changed the label
To allow for more granularity, we are opting to go with a route selector as opposed to a namespace selector for the ingress controller. The idea here is that we're filtering the traffic serviced by the ingress controller at the route level as opposed to the entire namespace.
This service is used with the external ingressController and connects with our public metallb loadBalancer The important fields to pay attention to here are the: metadata.annotations metallb.universe.tf/address-pool: public spec.loadBalancerIP: 199.94.61.6 This is the ip where we are servicing external traffic
|
@dystewart this looks fine, but there are apparently still missing pieces. See nerc-project/operations#41 for some details. I'd like to hold off merging this until we get things working (because we'll probably need to update the same set of files). |
We already know it's an ingress controller.
Keep all the external-ingress resources together so they're easier to manage.
When we create an IngressController, the IngressController creates a Service. We need to modify that service resource to add both an annotation and a loadBalancerIP, but ArgoCD doesn't allow us to patch existing resources. The best we can do is apply patches using an ArgoCD post-sync hook [1]. This commit adds a post-sync Job that takes care of patching the Service resource. This job will run after every sync operation. [1]: https://argo-cd.readthedocs.io/en/stable/user-guide/resource_hooks/
I'm a little leery of it: it appears to be written by a single person, and I disagree with the security model (see e.g. redhat-cop/patch-operator#47). |
No description provided.