feat: add prometheusRule

[micborens]

• Adding prometheusRule manifest creation with flexibility for the user to enrich alerts
• Respecting best practice following alert naming convention.
• Alert enrichment logic is based on what is done in official kube prometheus stack

Default alerts are based on what I found in this PR

I'm ok to maintain this part of the helm if needed.

[Vad1mo]

@micborens this is probably related to the PR #1635
I would suggest to sync with @Allex1 so we can get in one version.

[micborens]

@Allex1 I appreciate your first pr but it's not flexible following our usage in my company and any case for the community. That's why I opened this PR.
If you'd like to take a look at my change, you will understand and I covered your change except Grafana dashboard which is from my point of view a specific case.

[Vad1mo]

This generally looks good, I need to test it myself and give it a try, but I support that approach!

[Allex1]

@micborens tbh the extra labels and annotations for each alert group seem unnecessary but it's not my call. If you add Grafana support to this pr I'm willing to close mine in favor of this .

[micborens]

Hello @Allex1 I've included your Grafana dashboard file in my PR.
As you did in your, it's disabled by default to not impact users who don't need this cm.

tbh the extra labels and annotations for each alert group seems unnecessary but it's not my call

Having the ability to enrich or not a resource on Kubernetes is needed when the chart is opensource. And I need it for my usage :)

[micborens]

Hello @Vad1mo do you think we can merge this PR?

[Vad1mo]

IMO, it's good, but you need to address the open points.
Imagine, it should be clear and understandable for 1k .- 10kk of developers by looking at the values.yaml file.

[MinerYang]

Hi @micborens @Allex1 ,

We are aware of many requirements of PrometheusRule for harbor and by having this let it more convenient to allow users to visualizing metrics.
However, what we concern is:

• Harbor helm is not an all-in-one tool kit and we preferred not including CRs other than Kubernetes resources.
• Since it contains CR outside such as PrometheusRule, ServiceMonitor, long-term maintenance would take into our considerations given to limited resources.
• Having all of these rules configurable would somehow too much for values.yaml , however it do necessary for various usage.

So, we are thinking if it's possible to make it as a plugin which should have a standalone repository to co-work on demand, and possibly need a process to move existing metrics-svcmon part along with it.

We will be appreciate if you are willing to continuing contributing this part for our community!

Best,
Miner

[micborens]

Hello

@Vad1mo I understood your point to have a value as clear as posssible. That's why I've put some comments in the files to explain all parts.
To give you another point of view, people who need this feature can just enable it without touching the rest. Of course, I included a lot :D of variable to let community user to custom their alert and deployment.
I think all people that understand helm and/or prometheus operator cr can understand easily this configuration.
btw, different open source projet are using this way to use their helm without default alerts, grafana component (mimir, loki, etc ...)

@MinerYang prometheus operator looks become a standard on kubernetes. however it's secure with check condition (presence of crds).

So, we are thinking if it's possible to make it as a plugin which should have a standalone repository to co-work on demand, and possibly need a process to move existing metrics-svcmon part along with it.

You are probably referring to subchart or libraries. I don't think it is usefull here except you want to reuse it in another chart.

I reiterate my interest in the monitoring part and I am ready to contribute to maintaining the feature.

[github-actions]

This PR is being marked stale due to a period of inactivty. If this PR is still relevant, please comment or remove the stale label. Otherwise, this PR will close in 30 days.