Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix openshift values to default with in-cluster prometheus #3721

Open
wants to merge 8 commits into
base: develop
Choose a base branch
from
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,9 @@ metadata:
labels:
{{ include "cost-analyzer.commonLabels" . | nindent 4 }}
roleRef:
# Grant the kubecost service account the cluster-monitoring-view role to enable it to query OpenShift Prometheus.
# This is necessary for Kubecost to get access and query the in-cluster Prometheus instance using its service account token.
# https://docs.redhat.com/en/documentation/openshift_container_platform/4.2/html/monitoring/cluster-monitoring#monitoring-accessing-prometheus-alerting-ui-grafana-using-the-web-console_accessing-prometheus
Comment on lines +46 to +48
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very helpful comment!

apiGroup: rbac.authorization.k8s.io
kind: ClusterRole
name: cluster-monitoring-view
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ metadata:
subjects:
- kind: ServiceAccount
name: {{ .Values.global.platforms.openshift.monitoringServiceAccountName | quote }}
namespace: openshift-monitoring
namespace: {{ .Values.global.platforms.openshift.monitoringServiceAccountNamespace | quote }}
roleRef:
kind: Role
name: {{ template "cost-analyzer.fullname" . }}-reader
Expand Down
54 changes: 54 additions & 0 deletions cost-analyzer/values-openshift-cluster-prometheus.yaml
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please also remove any excess comments & unused configs which are present in this file.

Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
# This helm values file is a modified version of the values-openshift.yaml file that is used to deploy Kubecost on OpenShift.
# The main difference is that the values-openshift-cluster-prometheus.yaml file is configured to connect to the in-cluster Prometheus instance running on the OpenShift cluster and not use bundled prometheus.
thomasvn marked this conversation as resolved.
Show resolved Hide resolved

global:
prometheus:
enabled: false # Kubecost depends on Prometheus data, it is not optional. When enabled: false, Prometheus will not be installed and you must configure your own Prometheus to scrape kubecost as well as provide the fqdn below. -- Warning: Before changing this setting, please read to understand the risks https://docs.kubecost.com/install-and-configure/install/custom-prom
fqdn: https://prometheus-k8s.openshift-monitoring.svc.cluster.local:9091 # example address of a Prometheus to connect to. Include protocol (http:// or https://) Ignored if enabled: true
insecureSkipVerify: false # If true, kubecost will not check the TLS cert of prometheus
# queryServiceBearerTokenSecretName: mcdbsecret # kubectl create secret generic mcdbsecret -n kubecost --from-file=TOKEN
kubeRBACProxy: true # If true, kubecost will use kube-rbac-proxy to authenticate with in cluster Prometheus for openshift
grafana:
enabled: false # If false, Grafana will not be installed
domainName: grafana.grafana
proxy: false
# Platforms is a higher-level abstraction for platform-specific values and settings.
platforms:
# Deploying to OpenShift (OCP) requires enabling this option.
openshift:
enabled: true # Deploy Kubecost to OpenShift.
createMonitoringClusterRoleBinding: true # Create a ClusterRoleBinding to allow using in-cluster Prometheus or Thanos.
createMonitoringResourceReaderRoleBinding: true # Create a Role and Role Binding to allow in-cluster Prometheus or Thanos to list and watch resources. This will be necessary if you are not using bundled prometheus and need to add scrape config for resources.
monitoringServiceAccountName: prometheus-k8s # Name of the service account to bind to the Resource Reader Role Binding.
monitoringServiceAccountNamespace: openshift-monitoring # Namespace of the service account to bind to the Resource Reader Role Binding.
route:
enabled: false # Create an OpenShift Route.
annotations: {} # Add annotations to the Route.
# host: kubecost.apps.okd4.example.com # Add a custom host for your Route.
# Create Security Context Constraint resources for the DaemonSets requiring additional privileges.
scc:
nodeExporter: false # Creates an SCC for Prometheus Node Exporter. This requires Node Exporter be enabled.
networkCosts: false # Creates an SCC for Kubecost network-costs. This requires network-costs be enabled.
# When OpenShift is enabled, the following securityContext will be applied to all resources unless they define their own.
securityContext:
runAsNonRoot: true
seccompProfile:
type: RuntimeDefault

# networkCosts:
# enabled: false # Enable network costs.
# prometheus:
# nodeExporter:
# enabled: true # Enable Prometheus Node Exporter.
serviceMonitor:
enabled: true
# additionalLabels:
# label-key: label-value
# networkCosts:
# enabled: false
# additionalLabels:
# label-key: label-value
prometheusRule:
enabled: true
# additionalLabels:
# label-key: label-value
9 changes: 5 additions & 4 deletions cost-analyzer/values-openshift.yaml
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Kubecost is always recommended to be run with the helm chart's bundled Prometheus. This is usually the best user experience and results in the fewest issues.

Although we should certainly document how to run with Openshift's pre-installed Prometheus, I don't think that we should default to that experience.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Additionally, when users choose not to deploy Kubecost's bundled Prometheus they will not get the Kubecost-curated extraScrapeConfigs which, among them, newly include targets for NVIDIA's DCGM Exporter. I am willing to bet this discrepancy will be the root cause of many future support cases.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we add create a service monitor with the needed scrape config for DCGM?

Perhaps we should have two value files to avoid requests asking for how to correctly implement using openshift prometheus?

Do we need additional tests to validate the non-bundled prometheus? Are we missing anything today that diagnostics would miss?

Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
global:
prometheus:
enabled: true # Kubecost depends on Prometheus data, it is not optional. When enabled: false, Prometheus will not be installed and you must configure your own Prometheus to scrape kubecost as well as provide the fqdn below. -- Warning: Before changing this setting, please read to understand the risks https://docs.kubecost.com/install-and-configure/install/custom-prom
fqdn: https://prometheus-k8s.openshift-monitoring.svc.cluster.local:9091 # example address of a prometheus to connect to. Include protocol (http:// or https://) Ignored if enabled: true
fqdn: https://prometheus-k8s.openshift-monitoring.svc.cluster.local:9091 # example address of a Prometheus to connect to. Include protocol (http:// or https://) Ignored if enabled: true
# insecureSkipVerify: false # If true, kubecost will not check the TLS cert of prometheus
# queryServiceBearerTokenSecretName: mcdbsecret # kubectl create secret generic mcdbsecret -n kubecost --from-file=TOKEN
# kubeRBACProxy: false # If true, kubecost will use kube-rbac-proxy to authenticate with in cluster Prometheus for openshift
Expand All @@ -11,9 +11,10 @@ global:
# Deploying to OpenShift (OCP) requires enabling this option.
openshift:
enabled: true # Deploy Kubecost to OpenShift.
# createMonitoringClusterRoleBinding: false # Create a Cluster Role Binding to allow using in-cluster prometheus or thanos.
# createMonitoringResourceReaderRoleBinding: false # Create a Role and Role Binding to allow in-cluster prometheus or thanos to list and watch resources. This will be necessary if you are not using bundled prometheus and need to add scrape config for resources.
# createMonitoringClusterRoleBinding: false # Create a ClusterRoleBinding to allow using in-cluster Prometheus or Thanos.
# createMonitoringResourceReaderRoleBinding: false # Create a Role and Role Binding to allow in-cluster Prometheus or Thanos to list and watch resources. This will be necessary if you are not using bundled prometheus and need to add scrape config for resources.
# monitoringServiceAccountName: prometheus-k8s # Name of the service account to bind to the Resource Reader Role Binding.
# monitoringServiceAccountNamespace: openshift-monitoring # Namespace of the service account to bind to the Resource Reader Role Binding.
route:
enabled: false # Create an OpenShift Route.
annotations: {} # Add annotations to the Route.
Expand All @@ -32,4 +33,4 @@ global:
# enabled: true # Enable network costs.
# prometheus:
# nodeExporter:
# enabled: true # Enable Prometheus Node Exporter.
# enabled: true # Enable Prometheus Node Exporter.