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

Primary ingress domains shoud be labled #149

Open
dasrecht opened this issue Nov 28, 2022 · 8 comments · Fixed by #153 · May be fixed by #266
Open

Primary ingress domains shoud be labled #149

dasrecht opened this issue Nov 28, 2022 · 8 comments · Fixed by #153 · May be fixed by #266

Comments

@dasrecht
Copy link

To make monitoring sites easier and give the possibility to filter for primary domains, we should start adding a label for those ingresses.

Proposal

lagoon.sh/primaryingress=true
@tobybellwood
Copy link
Member

Possibly related 😜 - uselagoon/lagoon#2827

@shreddedbacon
Copy link
Member

First class routes will be good. But this is an easy win for now just extending the already present monitoring enabled check but just adding the primary ingress label (even if monitoring is not enabled) since this label could be used for other things.

@dasrecht
Copy link
Author

dasrecht commented Dec 5, 2022

should we go ahead and fade out the old stakater parts and add a generic

lagoon.sh/monitoring=true

in the process?

@shreddedbacon
Copy link
Member

should we go ahead and fade out the old stakater parts and add a generic

lagoon.sh/monitoring=true

in the process?

Need to think about that, because I don't know if other people use these?

@shreddedbacon
Copy link
Member

I reviewed the code, we only enable monitoring if it is the primary ingress, so I think the primaryingress label is sufficient? No primary label will be added to an autogenerated route, and won't be added to any development environments at all.

If we do end up removing all the stakater parts, this is also fine by me, but maybe need @tobybellwood to check if this is used by anyone else in the community (hopefully not)

@dasrecht
Copy link
Author

dasrecht commented Dec 6, 2022

Oh yes, didn't think about that. The primaryingress would be fine then. we can have a separate label or annotation on our end if we would need to add monitoring for an ingress that wouldn't be a primary.

Regarding removing the IngressMonitoringController (IMC) parts. It should be ok to remove, as the Stakater parts moved away from the annotation, and after k8s 1.22+ the old operator isn't working anymore. https://github.com/stakater/IngressMonitorController#from-controller-to-operator

@shreddedbacon
Copy link
Member

Oh yes, didn't think about that. The primaryingress would be fine then. we can have a separate label or annotation on our end if we would need to add monitoring for an ingress that wouldn't be a primary.

Using the primaryingress label to then add other labels to ingress? I think this would not be ideal way to go. But you could leverage the label for other systems. Adding labels outside of a build, or outside of .lagoon.yml could potentially be lost if not careful.

Regarding removing the IngressMonitoringController (IMC) parts. It should be ok to remove, as the Stakater parts moved away from the annotation, and after k8s 1.22+ the old operator isn't working anymore. stakater/IngressMonitorController#from-controller-to-operator

I think we still support 1.19+ of kubernetes, once we figure out when we want to drop support for 1.22 and earlier then we can drop the stakater stuff no problemo.

@dasrecht
Copy link
Author

I think we should remove the stakater by now as Kubernetes 1.19 is a thing of the past and it would simplify a few things in code.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment