[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). |
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>
a1e8158 to
9f2a782
Compare
|
/azp run |
|
Azure Pipelines: Successfully started running 1 pipeline(s). |
Signed-off-by: Natarajan Subbiramani <natarajan.subbiramani@nokia.com>
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
| 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.
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