Skip to content

sonic/config_generator: align syslog and mgmt_port values with schema #2260

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.

Sign up for GitHub

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

Open
berendt wants to merge 1 commit into
base: main
Choose a base branch
from
Open

sonic/config_generator: align syslog and mgmt_port values with schema #2260

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion files/sonic/config_db.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -335,7 +335,7 @@
"MGMT_PORT": {
"eth0": {
"admin_status": "up",
"autoneg": "true",
"autoneg": "on",
Copy link
Copy Markdown
Member

@osfrickler osfrickler May 5, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

again, I'd prefer to stick to how the config is generated and amend the scheme if needed

"description": "Management0",
"mtu": "1500",
"speed": "1000"
Expand Down
15 changes: 14 additions & 1 deletion osism/tasks/conductor/sonic/config_generator.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2074,6 +2074,16 @@ def _add_portchannel_configuration(config, portchannel_info):
)


# Map common syslog severity aliases to the SONiC-accepted set.
# SONiC schema accepts: none, debug, info, notice, warn, error, crit.
_SYSLOG_SEVERITY_ALIASES = {
"informational": "info",
"warning": "warn",
"err": "error",
"critical": "crit",
Copy link
Copy Markdown
Member

@osfrickler osfrickler May 5, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why did you omit the other three? at least debug sounds like it could be useful in certain cases

}


def _add_log_server_configuration(config, device):
"""Add SYSLOG_SERVER configuration to device config.

Expand All @@ -2085,11 +2095,14 @@ def _add_log_server_configuration(config, device):
proto = device.config_context.get("_segment_log_server_proto", "udp")
severity = device.config_context.get("_segment_log_server_severity", "info")
vrf = device.config_context.get("_segment_log_server_vrf", "mgmt")
proto = proto.strip().lower()
Copy link
Copy Markdown
Member

@osfrickler osfrickler May 5, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is uppercase in the generated config, I would prefer to stick to that

severity = severity.strip().lower()
severity = _SYSLOG_SEVERITY_ALIASES.get(severity, severity)
Comment thread
sourcery-ai[bot] marked this conversation as resolved.
config["SYSLOG_SERVER"] = {}
for host in hosts:
config["SYSLOG_SERVER"][host] = {}
config["SYSLOG_SERVER"][host]["message-type"] = "log"
config["SYSLOG_SERVER"][host]["protocol"] = proto.upper()
config["SYSLOG_SERVER"][host]["protocol"] = proto
config["SYSLOG_SERVER"][host]["remote-port"] = "514"
config["SYSLOG_SERVER"][host]["severity"] = severity
config["SYSLOG_SERVER"][host]["vrf_name"] = vrf
Expand Down
36 changes: 32 additions & 4 deletions tests/unit/tasks/conductor/sonic/test_config_generator_log.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,23 +24,51 @@ def test_add_log_server_configuration_defaults():
for host in ("10.1.1.1", "10.1.1.2"):
assert config["SYSLOG_SERVER"][host] == {
"message-type": "log",
"protocol": "UDP",
"protocol": "udp",
"remote-port": "514",
"severity": "info",
"vrf_name": "mgmt",
}


def test_add_log_server_configuration_protocol_uppercased():
@pytest.mark.parametrize(
"proto_in, proto_out",
[("tcp", "tcp"), ("TCP", "tcp"), ("Udp", "udp")],
)
def test_add_log_server_configuration_protocol_lowercased(proto_in, proto_out):
config = {}
device = _device_with_ctx(
_segment_log_server_hosts=["10.1.1.1"],
_segment_log_server_proto="tcp",
_segment_log_server_proto=proto_in,
)

_add_log_server_configuration(config, device)

assert config["SYSLOG_SERVER"]["10.1.1.1"]["protocol"] == "TCP"
assert config["SYSLOG_SERVER"]["10.1.1.1"]["protocol"] == proto_out


@pytest.mark.parametrize(
"severity_in, severity_out",
[
("warning", "warn"),
("WARNING", "warn"),
("informational", "info"),
("err", "error"),
("critical", "crit"),
("warn", "warn"),
("info", "info"),
],
)
def test_add_log_server_configuration_severity_aliases(severity_in, severity_out):
config = {}
device = _device_with_ctx(
_segment_log_server_hosts=["10.1.1.1"],
_segment_log_server_severity=severity_in,
)

_add_log_server_configuration(config, device)

assert config["SYSLOG_SERVER"]["10.1.1.1"]["severity"] == severity_out


def test_add_log_server_configuration_custom_severity_and_vrf():
Expand Down