-
Notifications
You must be signed in to change notification settings - Fork 4
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
feat(prometheus): Deploy Prometheus StatefulSet by default #89
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
jsirianni
changed the title
feat(prometheus): Deploy remote Prometheus StatefulSet by default
feat(prometheus): Deploy Prometheus StatefulSet by default
Feb 19, 2024
jsirianni
force-pushed
the
always-on-prometheus
branch
from
February 19, 2024 22:37
1bd3d53
to
acdbc44
Compare
jsirianni
requested review from
shazlehu and
dpaasman00
and removed request for
a team and
shazlehu
February 20, 2024 21:51
jsirianni
force-pushed
the
epic/always-on-prometheus
branch
from
February 26, 2024 17:55
b78b381
to
d1ca4f9
Compare
jsirianni
force-pushed
the
always-on-prometheus
branch
from
February 26, 2024 17:55
4c63197
to
081b365
Compare
dpaasman00
approved these changes
Feb 26, 2024
3 tasks
jsirianni
added a commit
that referenced
this pull request
Feb 27, 2024
* bump minor version * run on all PRs * feat(prometheus): Use bindplane-prometheus container image (#87) * use bindplane prometheus image and remove configmap * remove args, they are set in the container image already. updaste volume mount to use /prometheus, the correct path for the bindplane-prometheus image * pin sidecar test version * feat(prometheus): Deploy Prometheus StatefulSet by default (#89) * remove sidecar * rename dev prometheus config * add prometheus statefulset * true up securityContext * add remote option * regen docs * remove prom enable from templates * use generated service name when host is not set * fix svc name * re-add sideCar option * revert size change * use localhost when sidecar * Regen docs * resolve test issues by using newer image * use ee image * Remove all test case as it requires a license * skip if dev prometheus is enabled * fix condition * add fsgroup for gke
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description of Changes
BindPlane v1.46.0 will require Prometheus because measurement support for bolt store and postgres have been removed.
This PR implements a Prometheus StatefulSet that is deployed by default. Support for Prometheus side car and self managed Prometheus is retained. This allows the chart to be compatible with all existing deployments. If sidecar or self managed prometheus are in use, the new statefulset will not be deployed.
Changes
prometheus.enable
bool optionprometheus.remote
bool optionprometheus.host
to""
localhost
when Prometheus side car is enabledprometheus.host
whenprometheus.remote
is enabledprometheus.sidecar.resources
toprometheus.resources
prometheus.storageClass
andprometheus.volumeSize
toprometheus.storage
blockWhen this change is released, the release will contain upgrade notes. The same upgrade notes will be added to the 1.46 bindplane release notes.
Testing
I have already performed significant testing. Feel free to run through a few scenario or two.
Test method
Test matrix used to ensure all upgrade paths are possible. See values files used below. GKE tends to be more strict w/ volume mounts, so we test against both.
New deployment.
Brand new deployment.
We expect bindplane and prometheus to be deployed.
Main without Prometheus configured
Previous deployment from main, without Prometheus explicitly enabled.
We expect the upgrade to this branch to deploy prometheus.
Main w/ Sidecar
Previous deployment from main, with Prometheus sidecar explicitly configured.
When upgrading, the
prometheus.enable
field can be removed.Main w/ remote Prometheus
Previous deployment from main, with a remote Prometheus configured. This is for Prometheus that is unmanaged by this chart.
In this case, Prometheus is deployed to my Linux system using the packages released w/ BindPlane.
Before upgrading, I update the values config to look like this:
prometheus.enable --> prometheus.remote
Helm outputs the same config in both cases, you can compare like this. If I write the output to file and check the hash, they are identical.
Please check that the PR fulfills these requirements