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

Replace ConfigMap reloader #2698

Merged
merged 1 commit into from
Oct 27, 2023
Merged

Conversation

chipzoller
Copy link
Collaborator

@chipzoller chipzoller commented Oct 27, 2023

Signed-off-by: chipzoller [email protected]

What does this PR change?

Replaces the previous ConfigMap reloader with the Prometheus operator's official reloader.

Does this PR rely on any other PRs?

No

How does this PR impact users? (This is the kind of thing that goes in release notes!)

No user impact. This is an under-the-hood swap out.

Links to Issues or tickets this PR addresses or fixes

GTM-71 (internal Kubecost)

What risks are associated with merging this PR? What is required to fully test this PR?

ConfigMap reloading might not happen upon change of the source.

How was this PR tested?

Tested locally. Changed resource configmap/kubecost-prometheus-server, tailed logs, observed reloader.

Have you made an update to documentation? If so, please provide the corresponding PR.

Copy link
Collaborator

@jessegoodier jessegoodier left a comment

Choose a reason for hiding this comment

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

LGTM! thanks, we'll run normal suite against this @LastJedionEarth

@chipzoller
Copy link
Collaborator Author

I'll leave this open another day or so giving others to weigh in on if there are other considerations prior to merging.

@thomasvn
Copy link
Member

Looks good! This PR only affects the configmap/kubecost-prometheus-server which includes Prometheus's scrapeconfigs, recording rules, and alerting rules.

Thanks @chipzoller for testing by making a change to the configmap and watching a reload! Biggest risk here is if the new image quay.io/prometheus-operator/prometheus-config-reloader eventually reloads Prometheus server incorrectly for some reason. I'm good to merge and monitor this in our Nightly build environments.

@chipzoller chipzoller merged commit 6b7c44a into kubecost:develop Oct 27, 2023
11 checks passed
@chipzoller chipzoller deleted the configmap branch October 27, 2023 18:27
ameijer pushed a commit that referenced this pull request Oct 30, 2023
…etl-utils-pod

* commit 'b675a8cacaa5b6c64cf00d3583df026cfc2ec507': (28 commits)
  exec (#2701)
  swap jimmidyson/configmap-reload for prometheus-operator/prometheus-config-reloader (#2698)
  update savedReports and advancedReports in values.yml to reflect current filter schema
  add systemProxy env vars (#2687)
  feat(cost-analyzer): add StatefulSet as option (#2188)
  [Feature] Development guide and devcontainers (#2680)
  Bump actions/checkout from 4.1.0 to 4.1.1 (#2683)
  remove replicasets from core (#2678)
  networkCosts service discovery (#2677)
  Begin Helm testing (#2674)
  update securityContexts (#2669)
  consistent image name for aggregator (#2676)
  Add version matrix and more tests (#2664)
  label consistency (#2673)
  setting to 50h to match etl retention time (#2667)
  pv sizing proxy for wf
  add missing bracket
  add ability to override cc sa name
  Added /savings/localLowDisks proxy for Aggregator.
  update perms (#2662)
  ...
@thomasvn
Copy link
Member

@cliffcolvin @jessegoodier Do we want this in v1.107? If so we should cherry pick.

@FernandoMiguel
Copy link

FYI #2762

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants