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

Allow changes in remote write api endpoint secret #1209

Merged

Conversation

QuentinBisson
Copy link
Contributor

It is currently impossible to edit the remote write api endpoint secret because we made it immutable in the past because the remote write password value could not be changed in every reconciliation.

The manual workaround consists in deletint the secrets manually on all installations which is error-prone and cumbersome.

As the current instability issues we have with the agent will most likely make us need to tune our current configuration of the prometheus remote write, this PR ensure we can change the already existing values for each agent without the need of a manual intervention.

With this PR, we can now tune the agent remote write configuration while keeping the previously generated password, hence not causing any remote write errors due to a password mismatch between the one configured in the agent (that can take up to 5 minutes to be updated) and the one configured in the MC basic auth ingress (which is updated whenener PMO runs).

Please note thatthis PR will also regenerate the existing secret if it is immutable upon release. This means the aforementioned remote write errors will happen for 5-10 minutes after the release but will then not be an issue anymore.

Checklist

I have:

  • Described why this change is being introduced
  • Separated out refactoring/reformatting in a dedicated PR
  • Updated changelog in CHANGELOG.md

@QuentinBisson QuentinBisson self-assigned this Mar 18, 2023
@tityosbot
Copy link
Collaborator

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

@QuentinBisson QuentinBisson force-pushed the allow-changes-in-remote-write-api-endpoint-secret branch 11 times, most recently from f2bccfb to 11f027c Compare March 20, 2023 11:13
@QuentinBisson QuentinBisson force-pushed the allow-changes-in-remote-write-api-endpoint-secret branch from 11f027c to 3a4c44c Compare March 20, 2023 12:30
@QuentinBisson QuentinBisson marked this pull request as ready for review March 20, 2023 12:38
@QuentinBisson QuentinBisson requested a review from a team as a code owner March 20, 2023 12:38
@QuentinBisson
Copy link
Contributor Author

Tested successfully on Gremlin, Just had some issues with marshalling and nil pointers :(

@QuantumEnigmaa
Copy link
Contributor

Do those issues would prevent the changes induced by this PR to work on some installations ?

@QuentinBisson
Copy link
Contributor Author

Nope that was me not knowing how to code

@marieroque marieroque self-requested a review March 21, 2023 15:36
@marieroque
Copy link
Contributor

Would it have been harder to split the current secret into an immutable secret and a configmap ?

@QuentinBisson
Copy link
Contributor Author

harder, no, but the migration would have been harder and took longer. This allows us to update the config now, whereas moving to a configmap would require a new observability bundle release.

@marieroque
Copy link
Contributor

harder, no, but the migration would have been harder and took longer. This allows us to update the config now, whereas moving to a configmap would require a new observability bundle release.

Ok I looked at the code. It's hard to see possible mistake!

Did you test side case like the one with a new cluster with no remote write secret ? Is the secret created correctly ?

@QuentinBisson
Copy link
Contributor Author

Yes it's created fine, I tested on a working cluster and non working cluster

@marieroque
Copy link
Contributor

Yes it's created fine, I tested on a working cluster and non working cluster

LGTM

@QuentinBisson
Copy link
Contributor Author

For the migration to a CM and secret, we would basically have to first create a CM with same data as the secret, add it to the bundle, wait for it to be deployed everywhere, then remove the duplicate section from the secret. That will take a long time :(

@QuentinBisson
Copy link
Contributor Author

We need to make this happen eventually but I do not have the bandwith and I think doing the promtail stuff will help us figure out a clean solution

@QuentinBisson QuentinBisson merged commit ab265d8 into master Mar 21, 2023
1 check passed
@QuentinBisson QuentinBisson deleted the allow-changes-in-remote-write-api-endpoint-secret branch March 21, 2023 16:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants