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

[DRAFT] Prevent pod restarts on startup #920

Draft
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

pintohutch
Copy link
Collaborator

@pintohutch pintohutch commented Mar 29, 2024

It is tempting to only rely on defaulting webhooks to ensure any
changes to the OperatorConfig updates the .collection.externalLabels
and rules.externalLabels fields with the default project, location, and
cluster labels.

However, for many cases, where an OperatorConfig already exists and the
operator is restarted with this change, a webhook will not be invoked
and the change to the generated Prometheus config's external_labels will
never happen.

So we add the defaulting webhook as well as implicit logic to fill in
the generated config external_labels at runtime.

Another approach could be to omit the mutating webhook entirely, however
if we can reflect the state of the config in our custom resources, we
should. This gives users a more clear and explicit sense of what is
being configured.

@pintohutch pintohutch changed the title Pintohutch/fix restarts [DRAFT] Prevent pod restarts on startup Mar 29, 2024
It is tempting to only rely on defaulting webhooks to ensure any
changes to the OperatorConfig updates the .collection.externalLabels
and rules.externalLabels fields with the default project, location, and
cluster labels.

However, for many cases, where an OperatorConfig already exists and the
operator is restarted with this change, a webhook will not be invoked
and the change to the generated Prometheus config's external_labels will
never happen.

So we add the defaulting webhook as well as implicit logic to fill in
the generated config external_labels at runtime.

Another approach could be to omit the mutating webhook entirely, however
if we can reflect the state of the config in our custom resources, we
should. This gives users a more clear and explicit sense of what is
being configured.
@bwplotka
Copy link
Collaborator

bwplotka commented Apr 25, 2024

I like this approach, but we have to manually (or/and automatically) tested for two cases - new and existing OperatorConfig.

(Not on it, don't have time now, help wanted)

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

2 participants