-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
Add monit for disk>85% into pmon docker #582
Conversation
RUN sed -i 's/^# allow @users readonly/ allow @users readonly/' /etc/monit/monitrc | ||
|
||
RUN echo "check filesystem root-aufs with path /" >> /etc/monit/monitrc | ||
RUN echo " if space usage > 85% for 5 times within 15 cycles then alert" >> /etc/monit/monitrc |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Instead of all of these sed
and echo
commands, wouldn't it be cleaner to keep our own custom monitrc file in this directory and just copy it into /etc/monit/
here instead? #Resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Technically they are the same. The benefit of sed is reusing the default config, and easy to detect incompatibility issue in future monit config (which may be rare but possible).
In reply to: 115619051 [](ancestors = 115619051)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I do agree that it would be easier to detect future compatibility issues.
RUN sed -i 's/^# allow @users readonly/ allow @users readonly/' /etc/monit/monitrc | ||
|
||
RUN echo "check filesystem root-aufs with path /" >> /etc/monit/monitrc | ||
RUN echo " if space usage > 85% for 5 times within 15 cycles then alert" >> /etc/monit/monitrc |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I do agree that it would be easier to detect future compatibility issues.
can we also add cpu and mem utilization alert? |
we also need to add cpu and mem util too, maybe it also make sense to add it into the base image so that we can monitor some process in the base image too. |
RUN sed -i 's/^# allow localhost/ allow localhost/' /etc/monit/monitrc | ||
RUN sed -i 's/^# allow admin:monit/ allow admin:monit/' /etc/monit/monitrc | ||
RUN sed -i 's/^# allow @monit/ allow @monit/' /etc/monit/monitrc | ||
RUN sed -i 's/^# allow @users readonly/ allow @users readonly/' /etc/monit/monitrc |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
merge the sed RUNs to -ri with one docker command to make it clear. (like https://github.com/Azure/sonic-buildimage/blob/master/dockers/docker-database/Dockerfile.j2)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Personally, I think the separate RUN
commands are pretty clear, plus there's no need to escape every line break.
i think it is better to put monit into pmon docker as the name of the docker implies. |
@stcheng, you cannot monitor the base image process such as ntp, sshd when monit is in pmon |
@stcheng @lguohan: Acually, installing monit in the base image makes more sense than in docker-platform-monitor. Correct me if I'm wrong, but my understanding is that docker-platform-monitor's purpose is for monitoring platform-specific hardware, whereas monit is being used to monitor the state of the OS. Thought of this way, monit actually belongs in the base OS, not in docker-platform-monitor. |
This reverts commit 9cbbf591c08bce4b52a0f68cbbddae102d7fc614.
sudo sed -i ' | ||
s/^# set logfile syslog/set logfile syslog/; | ||
s/^\s*set logfile \/var/# set logfile \/var/; | ||
s/^# set httpd port/set httpd port/; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what is this httpd port?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ref: https://mmonit.com/monit/documentation/#MONIT-HTTPD
It enables cli to check status. The access is granted to local connection, so it should be safe.
* src/sonic-utilities ee56d54...cb0e745 (11): > sonic_utilities: Support for DOM Threshold values for EEPROM dump (#545) > [portstat] Fix portstat show RX_UTIL over 100% for 100G (#563) > sonic_installer: fix read-only filesystem support for firmware update (#565) > Revert "show acl table command output should show binding column correctly even with single port (#447)" (#589) > show acl table command output should show binding column correctly even with single port (#447) > [config] Do no stop or restart dependent services (#582) > sfpshow: prevent 'show int trans eeprom --dom' from crashing (#567) > [warm-reboot] add docker upgrade --warm option and roll back support (#559) > [ecnconfig] Validate input WRED parameters (#579) > [sonic-utilities] Add fstrim to reboot (#535) > Fixing the expected neighbor command due to change in output format under sonic-buildimage/pull/3036 (#584)
Submodule src/sonic-utilities fe2c656..afaedb7: > Revert "[FastReboot]: Send SIGINT to all teamd before stop (sonic-net#633)" (sonic-net#650) > Fixed config Asym PFC CLI. (sonic-net#632) > [config] Do no stop or restart dependent services (sonic-net#582) Signed-off-by: Ying Xie <ying.xie@microsoft.com>
No description provided.