-
Notifications
You must be signed in to change notification settings - Fork 45
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
Bump prometheus-operator version #2948
Bump prometheus-operator version #2948
Conversation
Hello alexandre-allard-scality,My role is to assist you with the merge of this Status report is not available. |
Waiting for approvalThe following approvals are needed before I can proceed with the merge:
Peer approvals must include at least 1 approval from the following list: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see a mention of Helm 3 in one of your commits... If we need it, then it's something we should at least document in the developer book, and likely also update in CI, I guess?
template = subprocess.check_output([ | ||
'helm', 'template', | ||
'--name', args.name, | ||
'helm', 'template', args.name, | ||
'--namespace', args.namespace, | ||
'--values', args.values, | ||
'--include-crds', | ||
args.path, | ||
]) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we do an if/else here to check if we are with helm2 or helm3 so that the script is compatible with helm2 as well (or we just expect all charts to be compatible with helm3, I don't know if helm3 has this kind of backward compatibility guaranteed)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
On my system, both binaries are named helm
, so I don't see how we can handle the two versions at the same time (except if we ask developers to have both, with specific paths or aliases, i.e.: helm2
and helm3
).
I think we may need to test if all charts are OK with helm 3, especially since helm 2 is now considered as deprecated.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We could easily have both,
But right if all charts are OK with helm3 we should get rid of helm2, I agree
1e99e4e
to
57c6203
Compare
ConflictThere is a conflict between your branch Please resolve the conflict on the feature branch ( $ git fetch
$ git checkout origin/improvement/update-prometheus-operator-addon
$ git merge origin/development/2.7
$ # <intense conflict resolution>
$ git commit
$ git push origin HEAD:improvement/update-prometheus-operator-addon |
57c6203
to
1579ae3
Compare
Waiting for approvalThe following approvals are needed before I can proceed with the merge:
Peer approvals must include at least 1 approval from the following list: |
b726cc9
to
361a45b
Compare
This chart has been renamed into `kube-prometheus-stack`. We update the chart to the version 12.2.3. ``` rm -rf charts/prometheus-operator helm repo add prometheus-community https://prometheus-community.github.io/helm-charts helm repo update helm fetch -d charts --untar prometheus-community/kube-prometheus-stack ```
361a45b
to
4b22969
Compare
Update the customization configuration file for `kube-prometheus-stack`. Also override the prefix name to have the same as before with `prometheus-operator` chart. Add node-exporter extra args to override default behavior ignoring loop devices and other kind of devices from disk stats.
kube-prometheus-stack chart is no longer compatible with helm 2, so we need to use helm 3 to render it correctly. So we slightly adapt the render script to work with helm 3
These alert rules come from the new version of kube-prometheus-stack charts. We drop them so we can add them back in an another Salt formula to allow customization by the users at runtime.
Since chart has been bumped, we need to regen it to apply changes. ``` ./charts/render.py prometheus-operator \ charts/kube-prometheus-stack.yaml \ charts/kube-prometheus-stack/ \ --namespace metalk8s-monitoring \ --service-config grafana \ metalk8s-grafana-config \ metalk8s/addons/prometheus-operator/config/grafana.yaml \ metalk8s-monitoring \ --service-config prometheus \ metalk8s-prometheus-config \ metalk8s/addons/prometheus-operator/config/prometheus.yaml \ metalk8s-monitoring \ --service-config alertmanager \ metalk8s-alertmanager-config \ metalk8s/addons/prometheus-operator/config/alertmanager.yaml \ metalk8s-monitoring \ --service-config dex \ metalk8s-dex-config \ metalk8s/addons/dex/config/dex.yaml.j2 metalk8s-auth \ --drop-prometheus-rules charts/drop-prometheus-rules.yaml \ > salt/metalk8s/addons/prometheus-operator/deployed/chart.sls ```
Update the node-exporter rules with the new alert rules coming from the kube-prometheus-stack version 12.2.3
grafana 6.7.4 to 7.2.1 alertmanager v0.20.0 to v0.21.0 k8s-sidecar 0.1.20 to 1.1.0 kube-state-metrics v1.9.5 to v1.9.7 node-exporter v0.18.1 to v1.0.1 prometheus v2.16.0 to v2.22.1 prometheus-config-reload v0.38.1 to v0.43.2 prometheus-operator v0.38.1 to v0.43.2
We need to regenerate these files as we bumped the Prometheus Operator version. The alerting & recording rules are generated using: ``` $ ./tools/rule_extractor/rule_extractor.py \ -i <control-plane-ip> -p 8443 -t rules ```
4b22969
to
25ef2bc
Compare
/approve |
In the queueThe changeset has received all authorizations and has been added to the The changeset will be merged in:
The following branches will NOT be impacted:
There is no action required on your side. You will be notified here once IMPORTANT Please do not attempt to modify this pull request.
If you need this pull request to be removed from the queue, please contact a The following options are set: approve |
I have successfully merged the changeset of this pull request
The following branches have NOT changed:
Please check the status of the associated issue None. Goodbye alexandre-allard-scality. |
Queue out of orderThe changeset has received all authorizations to enter the merge queue, Please contact a member of release engineering. The following options are set: approve |
In the queueThe changeset has received all authorizations and has been added to the The changeset will be merged in:
The following branches will NOT be impacted:
There is no action required on your side. You will be notified here once IMPORTANT Please do not attempt to modify this pull request.
If you need this pull request to be removed from the queue, please contact a The following options are set: approve |
I have successfully merged the changeset of this pull request
The following branches have NOT changed:
Please check the status of the associated issue None. Goodbye alexandre-allard-scality. |
Queue out of orderThe changeset has received all authorizations to enter the merge queue, Please contact a member of release engineering. The following options are set: approve |
In the queueThe changeset has received all authorizations and has been added to the The changeset will be merged in:
The following branches will NOT be impacted:
There is no action required on your side. You will be notified here once IMPORTANT Please do not attempt to modify this pull request.
If you need this pull request to be removed from the queue, please contact a The following options are set: approve |
I have successfully merged the changeset of this pull request
The following branches have NOT changed:
Please check the status of the associated issue None. Goodbye alexandre-allard-scality. |
Component: charts, salt, build
Context:
Addon versions bump.
Summary:
Replace prometheus-operator which is deprecated by kube-prometheus-stack in version 12.2.3
Acceptance criteria: Currently testing if everything still OK.