Skip to content

[config/syslog]: Resolve per-ASIC-only features to their container on single-ASIC platforms#4479

Open
nsoma-cisco wants to merge 1 commit intosonic-net:masterfrom
nsoma-cisco:nsoma_master
Open

[config/syslog]: Resolve per-ASIC-only features to their container on single-ASIC platforms#4479
nsoma-cisco wants to merge 1 commit intosonic-net:masterfrom
nsoma-cisco:nsoma_master

Conversation

@nsoma-cisco
Copy link
Copy Markdown

What I did

Fix config syslog rate-limit-feature enable/disable <svc> being a silent no-op on single-ASIC platforms for features whose FEATURE entry has has_global_scope=False and has_per_asic_scope=True (e.g. syncd, gbsyncd, and swss on some platforms).

How I did it

  • Root cause: config/syslog.py::get_feature_names_to_proceed only enumerated per-ASIC container names inside an if multi_asic.is_multi_asic(): guard. On a single-ASIC DUT that branch was skipped, the feature list came back empty, and the outer for loop in enable_rate_limit_feature / disable_rate_limit_feature printed nothing and did nothing.
  • Fix: on a single-ASIC platform a feature has exactly one container whose name equals the service name regardless of which scope flag is set, so both scopes are now treated symmetrically in that branch. Multi-ASIC behavior is preserved byte-for-byte.
  • Hardened the flag lookups with .get('flag', '') to avoid a KeyError when a flag is absent.
  • Safety net: enable/disable now emit an explicit "No container resolved … nothing to enable/disable" message when the resolved list is empty, so a future similar misconfiguration cannot silently no-op again.
Behavior matrix after the fix
FEATURE flags Single-ASIC → containers Multi-ASIC (N ASICs) → containers
global=True, per_asic=False [svc] [svc]
global=False, per_asic=True [svc] (was [] — the bug) [svc0, …, svc{N-1}]
global=True, per_asic=True [svc] (was [] when looked up by name) [svc, svc0, …, svc{N-1}]
global=False, per_asic=False [] + "nothing to enable" message [] + "nothing to enable" message

How to verify it

Unit tests:

```bash
python3 -m pytest tests/syslog_test.py tests/syslog_multi_asic_test.py -x -q
```

  • New `tests/syslog_test.py::TestSyslog::test_enable_rate_limit_feature_single_asic_per_asic_scope` asserts that `docker cp .../containercfgd.conf` and `supervisorctl start containercfgd` are invoked for a single-ASIC `syncd`-like feature (`has_global_scope=False, has_per_asic_scope=True`), and the symmetric `stop` + `rm -f` on disable. Would have failed against the pre-fix code.
  • New `tests/syslog_test.py::TestSyslog::test_enable_rate_limit_feature_empty_feature_list` asserts the new "nothing to enable" message and that no shell commands run when the resolved list is empty.
  • Existing `test_enable_syslog_rate_limit_feature`, `test_disable_syslog_rate_limit_feature`, and `tests/syslog_multi_asic_test.py` paths are unaffected.

On a single-ASIC DUT:

  • `sudo config syslog rate-limit-feature enable syncd` prints `Enabling syslog rate limit feature for syncd` and `Enabled syslog rate limit feature for syncd`.
  • `docker exec -i syncd supervisorctl status containercfgd` → `RUNNING`.
  • `sudo config syslog rate-limit-container syncd -i 10 -b 500` → `docker exec -i syncd grep SystemLogRateLimit /etc/rsyslog.conf` shows `10` / `500`, and `grep containercfgd /var/log/syslog` shows the `Configure syslog rate limit interval=10, burst=500` notice.
  • `sudo config syslog rate-limit-feature disable syncd` cleanly tears everything down.

On a multi-ASIC DUT: existing behavior is unchanged (`syncd0`, `syncd1`, … resolve as before; `bgp` global-only still resolves to `bgp`; `teamd` with both flags still resolves to `teamd` + per-ASIC names).

Previous command output (if the output of a command-line utility has changed)

On a single-ASIC DUT, before the fix:

```
admin@sonic:$ sudo config syslog rate-limit-feature enable syncd
admin@sonic:
$
```

(Silent no-op — nothing printed, container untouched.)

New command output (if the output of a command-line utility has changed)

```
admin@sonic:~$ sudo config syslog rate-limit-feature enable syncd
Enabling syslog rate limit feature for syncd
Enabled syslog rate limit feature for syncd
```

And for a feature with neither scope flag set:

```
admin@sonic:~$ sudo config syslog rate-limit-feature enable ghost
No container resolved for service 'ghost' in namespace None, nothing to enable
```

… single-ASIC platforms

config syslog rate-limit-feature enable/disable <svc> was a silent
no-op on single-ASIC platforms when <svc> had has_global_scope=False
and has_per_asic_scope=True (e.g. syncd, gbsyncd, and swss on some
platforms). get_feature_names_to_proceed only enumerated per-ASIC
containers inside an `if multi_asic.is_multi_asic():` guard, so the
single-ASIC case ended up with an empty feature list and the outer
for-loop printed nothing and did nothing.

On a single-ASIC platform a feature has exactly one container whose
name equals the service name, regardless of which scope flag is set;
treat both scopes symmetrically there. Harden the flag lookups with
`.get('flag', '')` to avoid a KeyError when a flag is absent. Also
surface an explicit "nothing to enable/disable" message when the
resolved list is empty so future similar misconfigurations cannot
silently no-op.

Tests:
- tests/syslog_test.py::test_enable_rate_limit_feature_single_asic_per_asic_scope
  asserts docker cp .../containercfgd.conf and supervisorctl start
  containercfgd are invoked for a single-ASIC syncd-like feature
  (has_global_scope=False, has_per_asic_scope=True), plus the symmetric
  stop + rm -f on disable.
- tests/syslog_test.py::test_enable_rate_limit_feature_empty_feature_list
  asserts the new "nothing to enable" message and that no shell
  commands run when the resolved list is empty.
- Existing test_enable_syslog_rate_limit_feature,
  test_disable_syslog_rate_limit_feature, and
  tests/syslog_multi_asic_test.py paths are unaffected.

Signed-off-by: Nageswara Soma <nsoma@cisco.com>
@linux-foundation-easycla
Copy link
Copy Markdown

linux-foundation-easycla Bot commented Apr 21, 2026

CLA Signed
The committers listed above are authorized under a signed CLA.

  • ✅ login: nsoma-cisco / name: Nageswara Soma (78d9772)

@mssonicbld
Copy link
Copy Markdown
Collaborator

/azp run

@azure-pipelines
Copy link
Copy Markdown

Azure Pipelines successfully started running 1 pipeline(s).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants