You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@apisix.apache.org by GitBox <gi...@apache.org> on 2022/08/24 06:27:29 UTC

[GitHub] [apisix-helm-chart] akalittle opened a new issue, #330: bug: ingress serviceMonitor doesnt generated

akalittle opened a new issue, #330:
URL: https://github.com/apache/apisix-helm-chart/issues/330

   ``` shell
   helm pull apisix/apisix -d . 
   ```
   after untar the the files below
   ```markdown
   .
   ├── Chart.lock
   ├── Chart.yaml
   ├── README.md
   ├── charts
   │   ├── apisix-dashboard
   │   │   ├── Chart.yaml
   │   │   ├── README.md
   │   │   ├── templates
   │   │   │   ├── NOTES.txt
   │   │   │   ├── _helpers.tpl
   │   │   │   ├── configmap.yaml
   │   │   │   ├── deployment.yaml
   │   │   │   ├── hpa.yaml
   │   │   │   ├── ingress.yaml
   │   │   │   ├── service.yaml
   │   │   │   ├── serviceaccount.yaml
   │   │   │   └── tests
   │   │   │       └── test-connection.yaml
   │   │   └── values.yaml
   │   ├── apisix-ingress-controller
   │   │   ├── Chart.yaml
   │   │   ├── README.md
   │   │   ├── crds
   │   │   │   └── customresourcedefinitions.yaml
   │   │   ├── templates
   │   │   │   ├── NOTES.txt
   │   │   │   ├── _helpers.tpl
   │   │   │   ├── configmap.yaml
   │   │   │   ├── deployment.yaml
   │   │   │   ├── hpa.yaml
   │   │   │   ├── rbac.yaml
   │   │   │   ├── service-account.yaml
   │   │   │   ├── service.yaml
   │   │   │   └── servicemonitor.yaml
   │   │   └── values.yaml
   │   └── etcd
   │       ├── Chart.lock
   │       ├── Chart.yaml
   │       ├── README.md
   │       ├── charts
   │       │   └── common
   │       │       ├── Chart.yaml
   │       │       ├── README.md
   │       │       ├── templates
   │       │       │   ├── _affinities.tpl
   │       │       │   ├── _capabilities.tpl
   │       │       │   ├── _errors.tpl
   │       │       │   ├── _images.tpl
   │       │       │   ├── _ingress.tpl
   │       │       │   ├── _labels.tpl
   │       │       │   ├── _names.tpl
   │       │       │   ├── _secrets.tpl
   │       │       │   ├── _storage.tpl
   │       │       │   ├── _tplvalues.tpl
   │       │       │   ├── _utils.tpl
   │       │       │   ├── _warnings.tpl
   │       │       │   └── validations
   │       │       │       ├── _cassandra.tpl
   │       │       │       ├── _mariadb.tpl
   │       │       │       ├── _mongodb.tpl
   │       │       │       ├── _mysql.tpl
   │       │       │       ├── _postgresql.tpl
   │       │       │       ├── _redis.tpl
   │       │       │       └── _validations.tpl
   │       │       └── values.yaml
   │       ├── templates
   │       │   ├── NOTES.txt
   │       │   ├── _helpers.tpl
   │       │   ├── configmap.yaml
   │       │   ├── cronjob.yaml
   │       │   ├── extra-list.yaml
   │       │   ├── networkpolicy.yaml
   │       │   ├── pdb.yaml
   │       │   ├── podmonitor.yaml
   │       │   ├── prometheusrule.yaml
   │       │   ├── secrets.yaml
   │       │   ├── serviceaccount.yaml
   │       │   ├── snapshot-pvc.yaml
   │       │   ├── statefulset.yaml
   │       │   ├── svc-headless.yaml
   │       │   ├── svc.yaml
   │       │   └── token-secrets.yaml
   │       └── values.yaml
   ├── templates
   │   ├── NOTES.txt
   │   ├── _helpers.tpl
   │   ├── configmap.yaml
   │   ├── daemonset.yaml
   │   ├── deployment.yaml
   │   ├── hpa.yaml
   │   ├── ingress.yaml
   │   ├── pdb.yaml
   │   ├── service-admin.yaml
   │   ├── service-gateway.yaml
   │   └── service-monitor.yaml
   └── values.yaml


-- 
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: notifications-unsubscribe@apisix.apache.org.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [apisix-helm-chart] tao12345666333 commented on issue #330: bug: ingress serviceMonitor doesnt generated

Posted by "tao12345666333 (via GitHub)" <gi...@apache.org>.
tao12345666333 commented on issue #330:
URL: https://github.com/apache/apisix-helm-chart/issues/330#issuecomment-1611465383

   But if prometheus-operator is not deployed in the cluster, it is obviously worthless to generate ServicrMonitor


-- 
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: notifications-unsubscribe@apisix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [apisix-helm-chart] Zebradil commented on issue #330: bug: ingress serviceMonitor doesnt generated

Posted by "Zebradil (via GitHub)" <gi...@apache.org>.
Zebradil commented on issue #330:
URL: https://github.com/apache/apisix-helm-chart/issues/330#issuecomment-1611410839

   This issue is already a bit old, but sharing additional detail for future lurkers might make sense.
   
   This happens because of the following condition in the helm template: https://github.com/apache/apisix-helm-chart/blob/b33af1a4719049cb0b353fd9118b9421c521eea4/charts/apisix-ingress-controller/templates/servicemonitor.yaml#L17
   
   As you can see, it is not enough to enable ServiceMonitor via `serviceMonitor.enabled = true`. The Kubernetes context must also have the `monitoring.coreos.com/v1` API version available. By default, the `helm template` command checks API versions available in the compiled client. Any custom version won't be available in this case.
   
   The workaround is to use `--api-versions` flag, like in this example:
   
   ```console
   $ helm template \
       --api-versions=monitoring.coreos.com/v1 \
       --release-name apisix-istio \
       -f ./values.yaml apisix/apisix \
       --output-dir ./apisix-deploy
   ```
   
   **In my opinion, this check is redundant and should be removed.** If a user's cluster doesn't have the corresponding custom resource definition installed, the `serviceMonitor.enabled` must simply be set to `false` (which is already the default value).


-- 
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: notifications-unsubscribe@apisix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [apisix-helm-chart] tao12345666333 closed issue #330: bug: ingress serviceMonitor doesnt generated

Posted by "tao12345666333 (via GitHub)" <gi...@apache.org>.
tao12345666333 closed issue #330: bug: ingress serviceMonitor doesnt generated
URL: https://github.com/apache/apisix-helm-chart/issues/330


-- 
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: notifications-unsubscribe@apisix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [apisix-helm-chart] Zebradil commented on issue #330: bug: ingress serviceMonitor doesnt generated

Posted by "Zebradil (via GitHub)" <gi...@apache.org>.
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: notifications-unsubscribe@apisix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [apisix-helm-chart] Zebradil commented on issue #330: bug: ingress serviceMonitor doesnt generated

Posted by "Zebradil (via GitHub)" <gi...@apache.org>.
Zebradil commented on issue #330:
URL: https://github.com/apache/apisix-helm-chart/issues/330#issuecomment-1618335218

   Yes, definitely, I will do it in the next couple of days.


-- 
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: notifications-unsubscribe@apisix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [apisix-helm-chart] tao12345666333 commented on issue #330: bug: ingress serviceMonitor doesnt generated

Posted by "tao12345666333 (via GitHub)" <gi...@apache.org>.
tao12345666333 commented on issue #330:
URL: https://github.com/apache/apisix-helm-chart/issues/330#issuecomment-1612300999

   Thanks. It sounds meaningful. Would you be willing to submit a PR to remove the conditional statement?


-- 
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: notifications-unsubscribe@apisix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org