[sophora-ugc] [sophora-ugc-proxy] add ServiceMonitors - #304
Conversation
There was a problem hiding this comment.
Pull request overview
Adds optional Prometheus Operator ServiceMonitor resources to the sophora-ugc and sophora-ugc-proxy Helm charts, including wiring Service port names so the monitors can target the correct port.
Changes:
- Add configurable
serviceMonitorvalues and newServiceMonitortemplates for UGC, UGC multimedia, and UGC proxy. - Name Service ports
httpsoServiceMonitor.spec.endpoints[].port: httpresolves correctly. - Bump chart versions and update Artifact Hub change annotations.
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| charts/sophora-ugc/values.yaml | Adds ugc.serviceMonitor and ugcMultimedia.serviceMonitor configuration defaults. |
| charts/sophora-ugc/templates/webapp-service.yaml | Names the Service port http for ServiceMonitor compatibility. |
| charts/sophora-ugc/templates/servicemonitor.yaml | New ServiceMonitor for UGC webapp metrics scraping. |
| charts/sophora-ugc/templates/ugc-multimedia/multimedia-service.yaml | Names the multimedia Service port http for ServiceMonitor compatibility. |
| charts/sophora-ugc/templates/ugc-multimedia/servicemonitor.yaml | New ServiceMonitor for UGC multimedia metrics scraping. |
| charts/sophora-ugc/templates/deployment.yaml | Renames container port 1694 from jolokia to management. |
| charts/sophora-ugc/Chart.yaml | Bumps chart version and updates Artifact Hub changelog. |
| charts/sophora-ugc-proxy/values.yaml | Adds top-level serviceMonitor configuration defaults. |
| charts/sophora-ugc-proxy/templates/service.yaml | Names the Service port http for ServiceMonitor compatibility. |
| charts/sophora-ugc-proxy/templates/servicemonitor.yaml | New ServiceMonitor for UGC proxy metrics scraping. |
| charts/sophora-ugc-proxy/Chart.yaml | Bumps chart version and updates Artifact Hub changelog. |
…djustable per service
|
as this is outside this PR's scope I mention it separately: Before the next release, please refactor the Helm chart templates as in: prefix template filenames with their Kubernetes resource type instead of suffixing them. Examples: This also applies to all template files (including webapp-service, multimedia-service, logback-configmap, and logback-multimedia-configmap). Goal: Keeps templates naturally grouped and sorted by Kubernetes resource type when browsing the directory on ArtifactHub or here. |
# Conflicts: # charts/sophora-ugc-proxy/Chart.yaml
| ## | ||
| ## Specific for the use with GCP Kubernetes Clusters | ||
|
|
||
| gcp: |
There was a problem hiding this comment.
Da httpRoutes toplevel ist, sollte das hier auch toplevel bleiben. Die GCP-Sachen gehören zu den HTTPRouten und sind somit wie ein Ingress. Außerdem bleibt es dann einheitlich zu den anderen Charts mit "gcp" Values.
Wäre es nicht besser, das ugc-multimedia als eigenes Chart zu machen? Das könnte ja als Subchart dann auch mit UGC zusammen deployed werden. 🤔
Oder es gibt noch ein "ugc-all" Chart, welches dann die beiden Charts "ugc" und "ugcMultimedia" einbindet. Dann bleiben die Values auch ähnlich, da die Subcharts dann auch Subvalues verwenden.
Könnte man sich langfristig mal überlegen.
No description provided.