Adding daily mail review from Issue 839 by akmet · Pull Request #881 · docker-mailserver/docker-mailserver

akmet · 2018-03-07T15:50:12Z

I added the daily summery email as requested in #839.
I have not added any tests, because I never worked with this kind of tests and does not really know how to make them and what exact test for. Hope someone can help here?

Currently the script analyzes the /var/log/mail/mail.log and I think logrotate will cause a loss in information when it gets executed. One way to solve this problem could be running logrotate every day at midnight (what I am using in production system) and analyze the mail.log.1 which represents the day before.
Another way could be to analyze the combination of mail.log.1 and mail.log but this just doesn't feels right. Any thoughts?

johansmitsnl

Could you resolve the conflict in the start script an look at these notes?

johansmitsnl · 2018-03-07T18:38:13Z

+test -x /usr/sbin/pflogsumm || exit 0
+test -x /usr/bin/bsd-mailx || exit 0
+
+/usr/sbin/pflogsumm /var/log/mail/mail.log -d yesterday --problems-first | /usr/bin/bsd-mailx -s "Daily Summery for HOSTNAME"  DAILY_SUMM_EMAIL


Should the variable be: $DAILY_SUMM_EMAIL ?

I got problems with sed when used $. Removing it fixed the error..

Even if you encapsulate the variable with "?

Used the following line and it does not replace the variable:
sed -i -r 's/$HOSTNAME/'$HOSTNAME'/g' /usr/local/bin/postfix-summary
and
sed -i -r 's/$HOSTNAME/'$HOSTNAME'/g' /usr/local/bin/postfix-summary

johansmitsnl · 2018-03-07T18:38:50Z

+function _setup_daily_summery() {
+	notify 'task' 'Setting up daily summery'
+
+	DAILY_SUMM_EMAIL=${DAILY_SUMM_EMAIL:="0"}


The default should be set at the start of the script.

johansmitsnl · 2018-03-07T18:39:55Z

+		sed -i -r 's/HOSTNAME/'$HOSTNAME'/g' /usr/local/bin/dailysummery
+		sed -i -r 's/DAILY_SUMM_EMAIL/'$DAILY_SUMM_EMAIL'/g' /usr/local/bin/dailysummery
+
+		echo "/usr/local/bin/dailysummery" > /etc/cron.daily/dailysummery


In the daily directory should contain executable scripts?
Just to confirm.

due to using logrotate this is obsolet

Starbix · 2018-03-07T20:28:15Z

It'd be great if you could change summery to summary

johansmitsnl · 2018-03-07T21:15:55Z

@17Halbe could you assist with the tests?

17Halbe · 2018-03-08T07:21:23Z

Hey, what a great idea!
I'll be busy the next few days, but I'll give it a try.
What about sendmail instead of bsd-mailx wasn't it working properly?
I don't know pflogsumm, but if we already introduce a new env variable, what about letting the user decide between daily/weekly/monthly(?) mails?
The tests would need to confirm, that a mail is being sent by cron, right? That shouldn't be a problem.

17Halbe · 2018-03-08T07:21:59Z

    amavisd-new \
    arj \
    binutils \
+    bsd-mailx \


Isn't sendmail working for this?

I wasn't able to do so. But maybe it was just my fault.
Could you provide an example on how to use sendmail? Is there something already existing here I could adapt?

https://github.com/tomav/docker-mailserver/blob/a73692cc9fb98849bb0eac72311f4966ecaaca41/test/tests.bats#L1368
I haven't tried using it, but the tests seem to work if one is using it like this..
I don't like the way sendmail works either. It's different on every system, sometimes a symlink to some other mail service and then there are about a couple different implementations, all with different syntaxes... 🙄

akmet · 2018-03-10T15:06:44Z

@17Halbe Just had a look at manpage
It can just analyze today, yesterday or the complete log file.
But we could do this with logrotation and after each rotation we just analyze the old log and send an email.
What do you think?

17Halbe · 2018-03-10T16:02:21Z

I think that would be overkill. ;) In that case, I think it‘s not worth the effort.
Unless you like to do it for fun and practice! :) (Isn‘t that the reason we are all doing this anyway?)

akmet · 2018-03-10T16:23:54Z

When using in production we have to edit logrotate anyway because if not we are skipping one day as I mentioned above..

akmet · 2018-03-12T13:33:38Z

Travis failed cause because the postfix-accounts.cf file keeps removing while the container runs.
I can reproduce this on my desktop (win10 with docker running on Windows and Ubuntu Bash) and I don't understand why. Can someone reproduce this?

johansmitsnl · 2018-03-13T07:20:41Z

Is the file gone or is it empty?

akmet · 2018-03-13T11:36:03Z

Like both?
Windows Explorer shows the file, but I cannot open it ("File does not exist. Create File?") and I cannot write to it. The file only disappears when restarting the Docker Daemon.
I have no idea why this happens and I think that I then had to use my notebook for changes and git.

…o issue-839

…ate interval

johansmitsnl · 2018-03-15T06:46:31Z

I don't know why that is, maybe change de filesystem of docker?

Could you also add a test by calling the function and verify the email has been send and it contains the correct subject and to address?

17Halbe · 2018-03-15T06:51:57Z

I‘ll take care of the test. Probably today.

akmet · 2018-03-15T09:09:23Z

Me neither. But I installed Ubuntu next to Windows and started with a fresh master and now it seems to run.
I would also test if the interval is correctly set and if the logrotate config is set with defaults when no variable is set.

fixed logrotate config

…into Issue-839

Issue 839

…config.

…into Issue-839

hope this was the last time we changed the variables names..

johansmitsnl · 2018-03-18T18:52:37Z

Good work!

Release 5.8.0 * Adding daily mail review from Issue 839 (#881) You can enable REPORT_RECIPIENT for REPORT_INTERVAL reports. Default is disabled. * introducing ENABLE_SRS env variable (#906, #852) In v3.2.0 was SRS introduced and enabled by default Now it is disabled by default and can be enabled with the new env variable. * fixed delalias, added additional tests (#909) Fixes to setup where made for deletion and addition.

akmet added 2 commits March 7, 2018 16:26

Added dependencies, binary, startup configuration

9f0c7f1

Added env variable to dist files/readme

56fb121

johansmitsnl suggested changes Mar 7, 2018

View reviewed changes

17Halbe reviewed Mar 8, 2018

View reviewed changes

akmet added 2 commits March 9, 2018 13:09

summery => summary, define daily_summ_email at default_vars

47ba792

Merge branch 'master' into issue-839

6c12939

akmet added 4 commits March 15, 2018 01:08

Merge branch 'master' of git://github.com/tomav/docker-mailserver int…

5516432

…o issue-839

send summary after each logrotate, added env variable for mail/logrot…

870d98c

…ate interval

remove mail.log from rsyslogs logrotate

b4c7ca6

rotate mail.log when no email is set

d5e05ae

akmet and others added 8 commits March 15, 2018 14:14

Added documentation for POSTFIX_LOGROTATE_INTERVAL

6bbed40

Added tests

dc81f85

forgot to return the Makefile to its default...

1c873b5

Removed interval option, since its not being tested for.

7739bbe

changed test to force logrotate to rotate

b850352

fixed logrotate config

removed mistankenly used postconf

b3e38f5

readded setup_environment, made logrotate_setup being called everytime

6aaf932

Merge branch 'issue-839' of https://github.com/akmet/docker-mailserver …

630e464

…into Issue-839

akmet and others added 7 commits March 17, 2018 13:57

Merge pull request #2 from 17Halbe/Issue-839

16bfab6

Issue 839

changed documentation for new variable names - again

be76a3d

Did Documentation, added a default recipient, added test for default …

5ed6121

…config.

Merge branch 'issue-839' of https://github.com/akmet/docker-mailserver …

c75d162

…into Issue-839

layout fix

17dec7f

Merge pull request #4 from 17Halbe/Issue-839

859110b

hope this was the last time we changed the variables names..

changed variable names apposite the documentation

b4b78d4

johansmitsnl approved these changes Mar 18, 2018

View reviewed changes

johansmitsnl merged commit a420b15 into docker-mailserver:master Mar 18, 2018

Uh oh!

Conversation

akmet commented Mar 7, 2018

Uh oh!

johansmitsnl left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Starbix commented Mar 7, 2018

Uh oh!

johansmitsnl commented Mar 7, 2018

Uh oh!

17Halbe commented Mar 8, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

akmet commented Mar 10, 2018

Uh oh!

17Halbe commented Mar 10, 2018

Uh oh!

akmet commented Mar 10, 2018

Uh oh!

akmet commented Mar 12, 2018

Uh oh!

johansmitsnl commented Mar 13, 2018

Uh oh!

akmet commented Mar 13, 2018

Uh oh!

johansmitsnl commented Mar 15, 2018

Uh oh!

17Halbe commented Mar 15, 2018

Uh oh!

akmet commented Mar 15, 2018

Uh oh!

johansmitsnl commented Mar 18, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants