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

Breaking changes in v0.4.0? #18

Closed
alexlawrence opened this issue Oct 24, 2017 · 8 comments
Closed

Breaking changes in v0.4.0? #18

alexlawrence opened this issue Oct 24, 2017 · 8 comments

Comments

@alexlawrence
Copy link

I downloaded the latest version in order to be able to only check for container existence. However, it seems like alerting does not work at all anymore. Sample config:

---
duration: 100				# duration in ms between docker API calls
iterations: 0				# number of iterations to run (0 = run forever)

containers:
  - name: foo
    expectedRunning: true

  - name: bar
    expectedRunning: true

  - name: baz

email:
  active: true
  smtp: some.example.com
  password: ...
  port: 587
  from: alerting@example.com
  subject: "monitoring alert"
  to:
    - xyz@example.com
    - abc@example.com

When stopping a container expected to be running, nothing happens.
Any idea what could be wrong?

@jeffwillette
Copy link
Owner

@alexlawrence duration of 100 ms and infinite iterations are now default and 0 iterations is a valid value, meaning it loads the config and everything and then never checks the docker daemon therefore no alerts would be triggered.

You should delete those two lines unless you want to change the default values.

I decided to change this part of the API because it is still in the 0.x.x phase of versioning, and using pointers to check for existence of this setting was the correct way to do this. Sorry if it caused any inconvenience.

@alexlawrence
Copy link
Author

Removed both durationand iterations. Still having the same behavior.

Output:

Oct 24 11:25:20 f012 systemd[1]: Started start docker-alertd.
Oct 24 11:25:20 f012 docker-alertd[13281]: Using config file: /home/ansible/.docker-alertd.yaml
Oct 24 11:25:20 f012 docker-alertd[13281]: 2017/10/24 11:25:20 email alerts active
Oct 24 11:25:20 f012 docker-alertd[13281]: 2017/10/24 11:25:20 starting docker-alertd
Oct 24 11:25:20 f012 docker-alertd[13281]: ------------------------------

@jeffwillette
Copy link
Owner

you're right, I have reproduced it and I am working on a fix now

@jeffwillette
Copy link
Owner

@alexlawrence the command line flag defaults were making the value 0 by default so I switched it back to the 0 value for iterations running the monitor indefinitely.

It should be fixed now, let me know if you have any problems

@alexlawrence
Copy link
Author

Thanks for the fast work! Could you please update the binaries?

@jeffwillette
Copy link
Owner

yes, they should be there now

@alexlawrence
Copy link
Author

alexlawrence commented Oct 25, 2017

Sorry to bother again. Alerting works now but the e-mails look a bit weird:

reporting: expected running state: %!t(*bool=0xc4201dd808), current running
state: true: Running check recovered

It seems that it does not convert the boolean (pointer?) correctly to a string.

Edit: This issue is not critical.

jeffwillette added a commit that referenced this issue Oct 25, 2017
- the call to errors.Wrapf was obscured in a function call and caused the linter to miss a
pointer that should have been dereferenced.

- the function that wraps errors now takes a string so the caller must call fmt.Sprintf
which catches any errors

- updated version to 0.4.3
@jeffwillette
Copy link
Owner

right, that should have been caught by my linter, but it was obscured in a function call. Should be fixed now in 0.4.3

jeffwillette added a commit that referenced this issue Nov 15, 2017
- command line flag defautls were overwriting the nil value that was to be read in from
the config file.

- 0 value for `iterations` now signals the monitor to run forever

- changed initconfig default config to reflect 0 value means run forever
jeffwillette added a commit that referenced this issue Nov 15, 2017
- the call to errors.Wrapf was obscured in a function call and caused the linter to miss a
pointer that should have been dereferenced.

- the function that wraps errors now takes a string so the caller must call fmt.Sprintf
which catches any errors

- updated version to 0.4.3
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

No branches or pull requests

2 participants