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

amtool check-config #978

Merged
merged 1 commit into from
Sep 7, 2017
Merged

Conversation

iksaif
Copy link
Contributor

@iksaif iksaif commented Sep 5, 2017

This is similar to promtool check-config and allows one
to validate the alertmanager configuration (as a git presubmit for example).

govendor fetch github.com/spf13/{cobra,pflag} was needed to
have support for Args.

This fixes #333

$ ./amtool check-config alertmanager.yml ./doc/examples/simple.yml 
Checking 'alertmanager.yml'  SUCCESS
Found 1 templates:   FAILED: template: foo.tmpl:3: unexpected {{end}}

Checking './doc/examples/simple.yml'  SUCCESS
Found 1 templates:   SUCCESS

Error: Failed to validate 1 file(s).

@iksaif iksaif force-pushed the check-config branch 3 times, most recently from c90f7cf to ab677d6 Compare September 5, 2017 08:43
@mxinden
Copy link
Member

mxinden commented Sep 5, 2017

@iksaif Thanks a lot for the contribution. This was requested very often.

I am not familiar with the amtool testing story (//CC @stuartnelson3), but I think we should have a basic test. @iksaif would you be able to add one for the configuration check?

@stuartnelson3
Copy link
Contributor

Ah I forgot to comment! I tested this manually this morning, worked great.

A really high level smoke test should be enough if it's not too much trouble -- does it recognize a good config.yml and an bad one.

@iksaif
Copy link
Contributor Author

iksaif commented Sep 6, 2017

Added tests

Copy link
Member

@mxinden mxinden left a comment

Choose a reason for hiding this comment

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

Thanks for the test. Just some small comments below. Other than that this looks good to me. @stuartnelson3 please merge if you feel the same.


err = CheckConfig([]string{"testdata/conf.bad.yml"})
if err == nil {
t.Fatalf("Failed to detect missing file")
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 confused by the word missing here. How about failed to detect invalid file?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Forgot to change this when I changed the test

func TestCheckConfig(t *testing.T) {
err := CheckConfig([]string{"testdata/conf.good.yml"})
if err != nil {
t.Fatalf("error: %v", err)
Copy link
Member

Choose a reason for hiding this comment

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

Instead of error: how about checking valid config file failed with: ?

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

@stuartnelson3
Copy link
Contributor

couple small wording changes on the test failure output and then we'll merge

This is similar to `promtool check-config` and allows one
to validate the alertmanager configuration (as a git presubmit for example).

`govendor fetch github.com/spf13/{cobra,pflag}` was needed to
have support for `Args`.
@iksaif
Copy link
Contributor Author

iksaif commented Sep 7, 2017

done

@stuartnelson3 stuartnelson3 merged commit 25e4bb9 into prometheus:master Sep 7, 2017
iksaif added a commit to criteo-forks/alertmanager that referenced this pull request Sep 8, 2017
This is similar to `promtool check-config` and allows one
to validate the alertmanager configuration (as a git presubmit for example).

`govendor fetch github.com/spf13/{cobra,pflag}` was needed to
have support for `Args`.
iksaif added a commit to criteo-forks/alertmanager that referenced this pull request Sep 12, 2017
This is similar to `promtool check-config` and allows one
to validate the alertmanager configuration (as a git presubmit for example).

`govendor fetch github.com/spf13/{cobra,pflag}` was needed to
have support for `Args`.
hh pushed a commit to ii/alertmanager that referenced this pull request Aug 6, 2018
* Replace supervisord xmlrpc library
* Use `github.com/mattn/go-xmlrpc` that doesn't leak goroutines.
* Fix uptime metric

* Use Prometheus best practices for uptime metric.
  * Use "start time" rather than "uptime".
  * Don't emit a start time if the process is down.
* Add changelog entry.
* Add example compatibility rules.

Signed-off-by: Ben Kochie <[email protected]>
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.

Validate config
3 participants