[console-monitor] Implement Console Logging feature - #409
Conversation
|
/azp run |
|
Azure Pipelines: There may be pipelines that require an authorized user to comment /azp run to run. |
|
Azure Pipelines: Successfully started running 1 pipeline(s). |
|
Hi, there are workflow run(s) waiting for approval, you may be first-time contributor. I will notify maintainers to help approve once PR is approved. Thanks! ---Powered by SONiC BuildBot
|
ae0dc70 to
a1e8158
Compare
|
/azp run |
|
Azure Pipelines: Successfully started running 1 pipeline(s). |
a1e8158 to
9f2a782
Compare
|
/azp run |
|
Azure Pipelines: Successfully started running 1 pipeline(s). |
9f2a782 to
12e3332
Compare
|
/azp run |
|
Azure Pipelines: Successfully started running 1 pipeline(s). |
| for link_id in self._logrotate_links - desired_links: | ||
| conf_path = logrotate_conf_path(link_id) | ||
| try: | ||
| if os.path.exists(conf_path): | ||
| os.remove(conf_path) | ||
| log.info(f"DCE: [{link_id}] Removed logrotate config: {conf_path}") | ||
| except OSError as e: | ||
| log.error(f"DCE: [{link_id}] Failed to remove logrotate config {conf_path}: {e}") | ||
|
|
||
| self._logrotate_links = desired_links |
There was a problem hiding this comment.
The current logic only removes configs tracked in the in-memory self._logrotate_links set, which is empty after a restart. The fix is to reconcile against what's actually on disk. After a DCE restart, existing_links is rebuilt from the actual /etc/logrotate.d/console-* files, so any file for a port that has since been disabled or deleted from CONSOLE_PORT gets removed, regardless of whether this process ever tracked it in memory. One caveat to note: the glob-based approach reconciles the entire console-* namespace, so it assumes console-monitor owns every LOGROTATE_CONF_PREFIX file in LOGROTATE_DIR.
I would suggest the change to _sync_logrotate_configs:
import glob
...
# Reconcile against configs actually present on disk, not just the
# in-memory set, so stale files left by a previous DCE run are cleaned up.
existing_links = set()
pattern = os.path.join(LOGROTATE_DIR, f"{LOGROTATE_CONF_PREFIX}*")
for conf_path in glob.glob(pattern):
existing_links.add(os.path.basename(conf_path)[len(LOGROTATE_CONF_PREFIX):])
for link_id in existing_links - desired_links:
conf_path = logrotate_conf_path(link_id)
try:
os.remove(conf_path)
log.info(f"DCE: [{link_id}] Removed logrotate config: {conf_path}")
except OSError as e:
log.error(f"DCE: [{link_id}] Failed to remove logrotate config {conf_path}: {e}")
self._logrotate_links = desired_links
There was a problem hiding this comment.
Though the actual log file would not be written in case of logging is disabled. It is good to remove the stale logrotate conf file, if present.
Addressed this issue. Thank you
| self.log_fd = os.open( | ||
| self.log_file_path, | ||
| os.O_WRONLY | os.O_CREAT | os.O_APPEND, | ||
| 0o644, |
There was a problem hiding this comment.
Console logs capture everything typed/echoed at the console, including credentials entered at login/enable prompts. 0o644 lets any local user read them. Consider 0o640 or 0o600.
There was a problem hiding this comment.
sure, corrected the file permission to 0o640. Thank you.
Signed-off-by: Natarajan Subbiramani <natarajan.subbiramani@nokia.com>
modified logrotate default values size as 10M, count as 10 Added more test coverage Signed-off-by: Natarajan Subbiramani <natarajan.subbiramani@nokia.com>
Signed-off-by: Natarajan Subbiramani <natarajan.subbiramani@nokia.com>
Signed-off-by: Natarajan Subbiramani <natarajan.subbiramani@nokia.com>
12e3332 to
1101b6f
Compare
|
/azp run |
|
Azure Pipelines: Successfully started running 1 pipeline(s). |
3 participants
console-monitor DCE reads console logging filename, logrotate parameters from CONSOLE_PORT table and creates logrotate conf for corresponding line number. If filename and logrotate are not provided, default values will be used.
console-monitor proxy service opens the log file name in append mode and keep logging the characters read from console device, if logging is enabled.
Related PR from other sub modules:
sonic-net/sonic-buildimage#28411
sonic-net/sonic-utilities#4685