Allow the operator to wait for CRDs to be installed#8551
Draft
haanhvu wants to merge 3 commits intoprometheus-operator:mainprometheus-operator/prometheus-operator:mainfrom
Draft
Allow the operator to wait for CRDs to be installed#8551haanhvu wants to merge 3 commits intoprometheus-operator:mainprometheus-operator/prometheus-operator:mainfrom
haanhvu wants to merge 3 commits intoprometheus-operator:mainprometheus-operator/prometheus-operator:mainfrom
Conversation
Signed-off-by: haanhvu <haanhvu888@gmail.com>
Signed-off-by: haanhvu <haanhvu888@gmail.com>
Contributor
simonpasquier
left a comment
There was a problem hiding this comment.
Thanks for tackling this!
| fs.Float64Var(&memlimitRatio, "auto-gomemlimit-ratio", defaultMemlimitRatio, "The ratio of reserved GOMEMLIMIT memory to the detected maximum container or system memory. The value should be greater than 0.0 and less than 1.0. Default: 0.0 (disabled).") | ||
| fs.BoolVar(&disableUnmanagedPrometheusConfiguration, "disable-unmanaged-prometheus-configuration", false, "Disable support for unmanaged Prometheus configuration when all resource selectors are nil. As stated in the API documentation, unmanaged Prometheus configuration is a deprecated feature which can be avoided with '.spec.additionalScrapeConfigs' or the ScrapeConfig CRD. Default: false.") | ||
| fs.DurationVar(&crdInstalledWaitingTime, "crd-installed-waiting-time", 0, "The waiting time for a CRD to be installed. During this time, the operator checks every second whether the CRD is installed. Need to be set together with --crds-wait-to-be-installed for the operator to know what CRDs to wait for.") | ||
| fs.Var(&crdsWaitToBeInstalled, "crds-wait-to-be-installed", "CRDs that the operator will wait to be installed. Valid values are storageclass, scrapeconfig, prometheus, prometheusagent, alertmanager, thanosruler. Need to be set with crd-installed-waiting-time so that the operator knows how long to wait for.") |
Contributor
There was a problem hiding this comment.
I'd remove storageclass since it's not managed by the operator. We should also add servicemonitor, podmonitor and probe (or remove scrapeconfigs if we want to limit ourselves to workload resources). I'm not sure we should use the singular or plural form or the CamelCase form.
Also the operator can have a default timeout value.
Suggested change
| fs.Var(&crdsWaitToBeInstalled, "crds-wait-to-be-installed", "CRDs that the operator will wait to be installed. Valid values are storageclass, scrapeconfig, prometheus, prometheusagent, alertmanager, thanosruler. Need to be set with crd-installed-waiting-time so that the operator knows how long to wait for.") | |
| fs.Var(&crdsWaitToBeInstalled, "check-for-crds-on-startup", "Comma-separated list of Custom Resource Definitions that should be present in the cluster and which the operator should have access to. By default, no check is performed. Valid values are scrapeconfigs, prometheuses, prometheusagents, alertmanagers, thanosrulers.") |
|
|
||
| fs.Float64Var(&memlimitRatio, "auto-gomemlimit-ratio", defaultMemlimitRatio, "The ratio of reserved GOMEMLIMIT memory to the detected maximum container or system memory. The value should be greater than 0.0 and less than 1.0. Default: 0.0 (disabled).") | ||
| fs.BoolVar(&disableUnmanagedPrometheusConfiguration, "disable-unmanaged-prometheus-configuration", false, "Disable support for unmanaged Prometheus configuration when all resource selectors are nil. As stated in the API documentation, unmanaged Prometheus configuration is a deprecated feature which can be avoided with '.spec.additionalScrapeConfigs' or the ScrapeConfig CRD. Default: false.") | ||
| fs.DurationVar(&crdInstalledWaitingTime, "crd-installed-waiting-time", 0, "The waiting time for a CRD to be installed. During this time, the operator checks every second whether the CRD is installed. Need to be set together with --crds-wait-to-be-installed for the operator to know what CRDs to wait for.") |
Contributor
There was a problem hiding this comment.
Suggested change
| fs.DurationVar(&crdInstalledWaitingTime, "crd-installed-waiting-time", 0, "The waiting time for a CRD to be installed. During this time, the operator checks every second whether the CRD is installed. Need to be set together with --crds-wait-to-be-installed for the operator to know what CRDs to wait for.") | |
| fs.DurationVar(&crdInstalledWaitingTime, "check-for-crds-on-startup-timeout", time.Minute, "How long the operator will wait for Custom Resource Definitions to be present on startup before exiting with error. Only relevant when --check-for-crds-on-startup is defined.") |
Contributor
Author
There was a problem hiding this comment.
By time.Minute, you want to set the default value to 1min? Why 1min here? I thought that not all users need this feature, so I set the default value as 0. (0 means no wait, but the operator still checks once.)
Contributor
There was a problem hiding this comment.
Indeed it works for me too. I was thinking about the original issue report but I agree that exiting early if the crds aren't present is also fine (the container will keep restarting until the CRDs are present).
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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
Closes: #7459
This PR add flags that users can use to set the waiting time for CRDs to be installed.
--crds-wait-to-be-installedsets the CRDs to wait for.--crd-installed-waiting-timesets the waiting time during which the operator checks every second whether the CRDs are installed. Both flags need to be set together.Type of change
What type of changes does your code introduce to the Prometheus operator? Put an
xin the box that apply.CHANGE(fix or feature that would cause existing functionality to not work as expected)FEATURE(non-breaking change which adds functionality)BUGFIX(non-breaking change which fixes an issue)ENHANCEMENT(non-breaking change which improves existing functionality)NONE(if none of the other choices apply. Example, tooling, build system, CI, docs, etc.)Verification
Unit tests for:
Changelog entry
Allow the operator to wait for CRDs to be installed