-
Notifications
You must be signed in to change notification settings - Fork 1.5k
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # 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>
#### Description for the changelog Add facilities to include arista fwutil into image if deb is in repo. Arista fwutil is proprietary and will be included in private repos <!-- Please make sure you've read and understood our contributing guidelines: https://github.com/Azure/SONiC/blob/gh-pages/CONTRIBUTING.md failure_prs.log skip_prs.log Make sure all your commits include a signature generated with `git commit -s` ** If this is a bug fix, make sure your description includes "fixes #xxxx", or "closes #xxxx" or "resolves #xxxx" Please provide the following information: --> #### Why I did it This utility is required for updating optics firmware on eos->sonic conversion for chassis ##### Work item tracking - Microsoft ADO **(number only)**: #### How I did it Add makefile entries to copy and install the deb if it exists #### How to verify it Build image with the deb in platform/broadcom/extra-debs and see if it is in the image <!-- If PR needs to be backported, then the PR must be tested against the base branch and the earliest backport release branch and provide tested image version on these two branches. For example, if the PR is requested for master, 202211 and 202012, then the requester needs to provide test results on master and 202012. --> #### Which release branch to backport (provide reason below if selected) <!-- - Note we only backport fixes to a release branch, *not* features! - Please also provide a reason for the backporting below. - e.g. - [x] 202006 --> - [ x] 202405 #### Tested branch (Please provide the tested image version) <!-- - Please provide tested image version - e.g. - [x] 20201231.100 -->
No description provided.