Package CR fails to properly create servicemonitor resources when looping through monitor array

[andygodish]

Environment

Device and OS: Mac (M3) App version: 0.22.2 Kubernetes distro being used: k3d (core-slim-dev) Other:

As part of the effort to package ArgoCD so that it is "Made for UDS", I encountered an issue with the way the pepr operator creates servicemonitor resources by looping through the monitor: field as defined in the uds-argocd-config chart template.

Steps to reproduce

1. Spin up a cluster with uds core-core-slim dev
2. Install the prometheus-stack package from uds-core
   • I could not find a ghcr package for this, I ran a zarf dev deploy from this dir to install.
3. Deploy the ArgoCD zarf package from this repo

Expected result

Given 4 items in the monitor: array of a Package CR manifest, I expect to see 4 service monitors.

Actual Result

I only see a single serviceMonitor resource with spec field values associated with the last item in my monitor array. Despite the Package resource indicating 4 "monitors" in the package resource:

Severity/Priority

Low considering we can use the upstream package in this case to generate serviceMonitors for each ArgoCD component as well as the network field to generate the proper ingress rules. It might not be common for an upstream product to expose multiple metrics endpoints like this.

Additional Context

I think the issue might be the use of the pkgname in generageMonitorName() here , potentially overriding the previous service monitor by creating a new one with the same name.

[mjnagel]

The issue in your case is specific to the use of description with the same value on all monitors. The name generate function will use the description if set, rather than the normal generate which would use selector + port name (which would be unique).

To prevent this situation we can add a validator function to ensure description is unique across monitor items.

[mjnagel]

If you wanted to workaround this for now @andygodish you could make the descriptions unique per item (i.e. "Redis Metrics", "Server Metrics", "Repo Metrics", etc).

[mjnagel]

Going to label as a good first issue, path forward here would just be adding a new piece to our validator. In there we would want a set + loop over the the monitor list to make sure the description is unique if present. It would look similar to the one we have for SSO clients.

I believe the alternative way of constructing names (not using the description) is guaranteed to be unique unless monitor blocks overlap (i.e. selector and port would have to be identical).

Also would be great to add a unit test for this case!