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

Support for parsing config file at start #7399

Merged
merged 5 commits into from
Jun 21, 2020

Conversation

Harkishen-Singh
Copy link
Contributor

Signed-off-by: Harkishen-Singh harkishensingh@hotmail.com

Fixes: #7322

Signed-off-by: Harkishen-Singh <harkishensingh@hotmail.com>
@Harkishen-Singh
Copy link
Contributor Author

/cc @codesome

Copy link
Member

@codesome codesome left a comment

Choose a reason for hiding this comment

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

Let's move this check above, after the compileCORSRegexString

@@ -427,6 +427,11 @@ func main() {
conntrack.DialWithTracing(),
)

if _, err := config.LoadFile(cfg.configFile); err != nil {
level.Info(logger).Log("msg", fmt.Sprintf("One or more errors exists in configuration file (--config.file=%s)", cfg.configFile))
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
level.Info(logger).Log("msg", fmt.Sprintf("One or more errors exists in configuration file (--config.file=%s)", cfg.configFile))
level.Error(logger).Log("msg", fmt.Sprintf("Error loading config (--config.file=%s)", cfg.configFile), "err", err)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

Signed-off-by: Harkishen-Singh <harkishensingh@hotmail.com>
@Harkishen-Singh
Copy link
Contributor Author

@codesome updated.

@@ -278,6 +278,11 @@ func main() {
os.Exit(2)
}

if _, err := config.LoadFile(cfg.configFile); err != nil {
level.Info(logger).Log("msg", fmt.Sprintf("Error loading config (--config.file=%s)", cfg.configFile), "err", err)
Copy link
Member

Choose a reason for hiding this comment

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

level.Error

@@ -278,6 +278,11 @@ func main() {
os.Exit(2)
}

if _, err := config.LoadFile(cfg.configFile); err != nil {
level.Info(logger).Log("msg", fmt.Sprintf("Error loading config (--config.file=%s)", cfg.configFile), "err", err)
os.Exit(2)
Copy link
Member

Choose a reason for hiding this comment

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

I am not really sure what the exit code should be. TestFailedStartupExitCode expects 1 for wrong file name. Someone aware of this can shed some light.

Signed-off-by: Harkishen-Singh <harkishensingh@hotmail.com>
@Harkishen-Singh
Copy link
Contributor Author

@brian-brazil @roidelapluie any views?

Copy link
Contributor

@brian-brazil brian-brazil left a comment

Choose a reason for hiding this comment

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

Seems fine to me.

@@ -278,6 +278,11 @@ func main() {
os.Exit(2)
}

if _, err := config.LoadFile(cfg.configFile); err != nil {
Copy link
Contributor

Choose a reason for hiding this comment

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

A comment that we're doing this so we'll fail early would be useful.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done. I hope that is satisfactory.

Signed-off-by: Harkishen-Singh <harkishensingh@hotmail.com>
Copy link
Member

@codesome codesome left a comment

Choose a reason for hiding this comment

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

Thanks!

@codesome codesome merged commit 70b0a34 into prometheus:master Jun 21, 2020
beorn7 added a commit that referenced this pull request Oct 12, 2020
In #7399, an early validity check of the config was introduced to
prevent the scenario where an invalid config is only detected after a
possibly very long startup procedure. However, the respective success
metrics are not updated after the initial validation so that the
success metrics suggest an invalid config. If the startup procedure,
like replaying the WAL, really takes very long, alerts about invalid
config will trigger.

This commit sets the succes metrics after initial validation. They
will be set again after the "real" config (re-)load, but that
shouldn't be a problem. The metric now truthfully represents whenever
the config was successfully loaded, no matter if the result was then
thrown away (because it was just for validation) or actually used.

Signed-off-by: beorn7 <beorn@grafana.com>
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.

Parse config before loading WAL
3 participants