From 21c76683ef8896f726b66eef5c5c926668cfc957 Mon Sep 17 00:00:00 2001 From: Sanket Sudake Date: Wed, 2 Mar 2022 10:58:56 +0530 Subject: [PATCH] [helm chart] Remove prometheus chart dependency from Fission (#2371) * Removing Prometheus dependency from fission chart * User should install their own Prometheus installation * Install Prometheus chart in Github CI workflow * Change Prometheus query logging to debug Signed-off-by: Sanket Sudake --- .github/workflows/push_pr.yaml | 10 +++++++--- charts/fission-all/Chart.lock | 6 ------ charts/fission-all/Chart.yaml | 7 +------ charts/fission-all/README.md | 8 +++++++- charts/fission-all/templates/_helpers.tpl | 4 ---- charts/fission-all/values.yaml | 11 ++++------- hack/generate-helm-manifest.sh | 2 +- pkg/canaryconfigmgr/prometheusClient.go | 3 +-- skaffold.yaml | 8 ++++---- 9 files changed, 25 insertions(+), 34 deletions(-) delete mode 100644 charts/fission-all/Chart.lock diff --git a/.github/workflows/push_pr.yaml b/.github/workflows/push_pr.yaml index 7e2ca03e..bb60556f 100644 --- a/.github/workflows/push_pr.yaml +++ b/.github/workflows/push_pr.yaml @@ -74,9 +74,6 @@ jobs: run: | helm lint charts/fission-all/ - - name: Helm update - run: helm repo add prometheus-community https://prometheus-community.github.io/helm-charts - - name: Install Skaffold run: | curl -Lo skaffold https://storage.googleapis.com/skaffold/releases/latest/skaffold-linux-amd64 @@ -88,6 +85,13 @@ jobs: with: install-only: true + - name: Setup Prometheus Stack + run: | + helm repo add prometheus-community https://prometheus-community.github.io/helm-charts + helm repo update + kubectl create ns monitoring + helm install monitoring prometheus-community/prometheus -n monitoring + - name: Build and Install Fission run: | kubectl create ns fission diff --git a/charts/fission-all/Chart.lock b/charts/fission-all/Chart.lock deleted file mode 100644 index 56619bb9..00000000 --- a/charts/fission-all/Chart.lock +++ /dev/null @@ -1,6 +0,0 @@ -dependencies: -- name: prometheus - repository: https://prometheus-community.github.io/helm-charts - version: 14.11.0 -digest: sha256:1404b25cbdba70279240fead15b765d6945097fed5afc7e184da02dabfaa577a -generated: "2021-10-19T17:20:08.919609+05:30" diff --git a/charts/fission-all/Chart.yaml b/charts/fission-all/Chart.yaml index 65333937..95b74120 100644 --- a/charts/fission-all/Chart.yaml +++ b/charts/fission-all/Chart.yaml @@ -21,9 +21,4 @@ maintainers: - name: Sanket Sudake email: sanket@infracloud.io engine: gotpl -type: application -dependencies: - - name: prometheus - version: 14.11.0 - repository: https://prometheus-community.github.io/helm-charts - condition: prometheus.enabled +type: application \ No newline at end of file diff --git a/charts/fission-all/README.md b/charts/fission-all/README.md index 16ff645a..919051c2 100644 --- a/charts/fission-all/README.md +++ b/charts/fission-all/README.md @@ -93,6 +93,12 @@ _See [helm upgrade](https://helm.sh/docs/helm/helm_upgrade/) for command documen A major chart version change (like v1.2.3 -> v2.0.0) indicates that there is an incompatible breaking change needing manual actions. +### Upgrade from 1.15.x to 1.16.x + +If you have been using `prometheus.enabled=true` in your fission-all chart, you will need to deploy the prometheus using prometheus community supported chart. +We have removed prometheus dependency from fission-all chart. +We would recommend [prometheus-community/prometheus](https://artifacthub.io/packages/helm/prometheus-community/prometheus) or [prometheus-community/kube-prometheus-stack](https://artifacthub.io/packages/helm/prometheus-community/kube-prometheus-stack) chart. + ### Upgrade from 1.14.x to 1.15.x With 1.15.x release, following changes are made: @@ -129,4 +135,4 @@ You may also `helm show values` on this chart's [dependencies](#dependencies) fo ### Multiple releases -The same chart can be used to run multiple Fission instances in the same cluster if required. To disable a dependency during installation, set `prometheus.enabled` to `false`. +The same chart can be used to run multiple Fission instances in the same cluster if required. diff --git a/charts/fission-all/templates/_helpers.tpl b/charts/fission-all/templates/_helpers.tpl index f80ec11d..2dcadcd1 100644 --- a/charts/fission-all/templates/_helpers.tpl +++ b/charts/fission-all/templates/_helpers.tpl @@ -25,11 +25,7 @@ controller/config.go {{- define "config" -}} canary: enabled: {{ .Values.canaryDeployment.enabled }} - {{- if .Values.prometheus.enabled }} - prometheusSvc: "http://{{ .Release.Name }}-prometheus-server.{{ .Release.Namespace }}" - {{- else }} prometheusSvc: {{ .Values.prometheus.serviceEndpoint | default "" | quote }} - {{- end }} {{- printf "\n" -}} auth: enabled: {{ .Values.authentication.enabled | default false }} diff --git a/charts/fission-all/values.yaml b/charts/fission-all/values.yaml index ee3ef666..c68df7cf 100644 --- a/charts/fission-all/values.yaml +++ b/charts/fission-all/values.yaml @@ -551,15 +551,12 @@ postInstallReportImage: fission/reporter ## debugEnv: false -## Prometheus for scrapping service metrics +## Prometheus related configuration to query metrics ## prometheus: - ## set this flag to true if prometheus needs to be deployed along with fission - ## - enabled: false - - ## If enabled is false, please assign the prometheus service URL - ## that is accessible by components. + ## please assign the prometheus service URL + ## that is accessible by Fission components. + ## This is mainly used to enable canary deployment. ## serviceEndpoint: "" diff --git a/hack/generate-helm-manifest.sh b/hack/generate-helm-manifest.sh index 6ec9919e..53095766 100755 --- a/hack/generate-helm-manifest.sh +++ b/hack/generate-helm-manifest.sh @@ -86,7 +86,7 @@ build_yamls() { $command >${c}-"${version}".yaml # for OpenShift - command="$cmdprefix --set analytics=false,analyticsNonHelmInstall=true,logger.enableSecurityContext=true,prometheus.enabled=false" + command="$cmdprefix --set analytics=false,analyticsNonHelmInstall=true,logger.enableSecurityContext=true" echo "$command" $command >${c}-"${version}"-openshift.yaml diff --git a/pkg/canaryconfigmgr/prometheusClient.go b/pkg/canaryconfigmgr/prometheusClient.go index 4fa56e0d..79524ba0 100644 --- a/pkg/canaryconfigmgr/prometheusClient.go +++ b/pkg/canaryconfigmgr/prometheusClient.go @@ -138,8 +138,7 @@ func (promApiClient *PrometheusApiClient) GetTotalFailedRequestsToFuncInWindow(f } func (promApiClient *PrometheusApiClient) executeQuery(queryString string) (float64, error) { - // TODO: Change to debug level once we have a better understanding of what is happening - promApiClient.logger.Info("prometheus executing query", zap.String("query", queryString)) + promApiClient.logger.Debug("executing prometheus query", zap.String("query", queryString)) val, warn, err := promApiClient.client.Query(context.Background(), queryString, time.Now()) if err != nil { diff --git a/skaffold.yaml b/skaffold.yaml index 45c81d89..e86f31f5 100644 --- a/skaffold.yaml +++ b/skaffold.yaml @@ -43,7 +43,6 @@ deploy: preUpgradeChecks.imageTag: "" namespace: fission pprof.enabled: false - prometheus.enabled: false canaryDeployment.enabled: false nats.enabled: false influxdb.enabled: false @@ -53,6 +52,7 @@ deploy: openTracing.enabled: false openTelemetry.otlpCollectorEndpoint: "" openTelemetry.otlpInsecure: true + prometheus.serviceEndpoint: "" priorityClassName: system-cluster-critical # Use /var/log directory for kind logs export terminationMessagePath: /var/log/termination-log @@ -102,15 +102,15 @@ profiles: - op: replace path: /deploy/helm/releases/0/setValues/routerServiceType value: NodePort - - op: replace - path: /deploy/helm/releases/0/setValues/prometheus.enabled - value: true - op: replace path: /deploy/helm/releases/0/setValues/influxdb.enabled value: true - op: replace path: /deploy/helm/releases/0/setValues/canaryDeployment.enabled value: true + - op: replace + path: /deploy/helm/releases/0/setValues/prometheus.serviceEndpoint + value: "http://monitoring-prometheus-server.monitoring.svc.cluster.local" - name: kind-opentelemetry patches: - op: replace