Skip to content
This repository was archived by the owner on Jan 11, 2023. It is now read-only.

update prometheus config for 2.1#2187

Closed
martell wants to merge 1 commit intoAzure:masterfrom
marinelayer:update-pga
Closed

update prometheus config for 2.1#2187
martell wants to merge 1 commit intoAzure:masterfrom
marinelayer:update-pga

Conversation

@martell
Copy link
Contributor

@martell martell commented Feb 2, 2018

What this PR does / why we need it:
fixes #2142, as a follow up to #2183

dtzar
dtzar previously approved these changes Feb 2, 2018
@trstringer
Copy link
Contributor

@martell (or @dtzar if you've tested this pr out), what api-model did you use to test this out successfully? Can you share?

@trstringer
Copy link
Contributor

I'm unsure how this could have been tested successfully without changing this line of code in the fork.

@trstringer
Copy link
Contributor

Update: we are looking into this, please do not merge this yet.

cc // @ritazh

@dtzar
Copy link
Contributor

dtzar commented Feb 2, 2018

There is no way I see this fixing the entire solution since in the example Readme we do not turn off RBAC (which is now enabled by default in acs-engine). I approved just because this gets us closer to working. The best solution IMO would be to have a v1 version of the extension which works with non-RBAC clusters where the helm version will match roughly the supported K8s version, using the 4.x version of the prometheus chart. v2 version of the extension should have a switch for RBAC, but leave it on by default and target 1.8/1.9+ clusters with the 5.X version of prometheus chart.

@dtzar dtzar dismissed their stale review February 2, 2018 18:47

Would be good to have a complete working solution, so taking back my approve.

@dtzar
Copy link
Contributor

dtzar commented Feb 4, 2018

I just tested with a vanilla acs-engine configuration (no prometheus/grafana extension) which provisions K8s 1.7.9 configuration and it uses RBAC. Without specifying rbac.create=true the installation fails, but with this set, the installation succeeds. @martell can you please update this to be true in your PR for the values.yaml file?

@trstringer
Copy link
Contributor

@dtzar you're talking about the manual helm installation of these charts succeeding? This is going to be a different case. This embedded prometheus_values.yaml config file is meant for the v4 version of the prometheus chart. It needs to be updated prior to merging.

Without RBAC set for the cluster, what is the failed error message when you try to install v5 of the Prometheus helm chart?

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Kubernetes cluster deployment ends in error state with prometheus-grafana

4 participants