Zebradil commented on issue #330:
URL: 
https://github.com/apache/apisix-helm-chart/issues/330#issuecomment-1611498764

   I think it's better to leave that up to the user.
   
   This condition is implicit and might lead to confusion (as we can see from 
this issue), the default values file looks like it's enough to just set 
`serviceMonitor.enabled` to `true` to get it rendered, but that's wrong. Apart 
from that, it interferes with some cases of helm usage:
   
   - `helm template` and `helm install` will behave differently, a user has to 
know that they must use the `--api-versions` flag to get similar behavior of 
those two commands. So instead of one configuration interface (helm values) we 
have to use two (helm values and helm cli flags). (Although, this is a design 
issue of helm itself)
   - In my case, `helm` is just a part of a bigger yaml processing pipeline. I 
use a wrapper around it, and now I have to implement support for the 
`--api-versions` flag in this wrapper. (It is not a big deal and it is better 
to be implemented in any case, because, even if the approach of not rendering 
resources depending on presented API versions is changed in this helm chart, 
there could be more helm charts that misuse this feature.) I actually convert 
`ServiceMonitor` resources of the `monitoring.coreos.com/v1` API into 
`monitoring.googleapis.com/v1`'s `PodMonitoring` resources. In this scenario, I 
never have CRDs of the prometheus-operator installed, but I still want to have 
the `ServiceMonitor` to be rendered.
   
   Anyways, for now, I'm going to just keep `serviceMonitor.enabled: false` and 
write the required resource manually in the hope that pod labels will stay 
compatible in the future.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to