Generic changes and new py-common API's for BMC based platforms#26544
Generic changes and new py-common API's for BMC based platforms#26544judyjoseph wants to merge 2 commits intosonic-net:masterfrom
Conversation
|
/azp run Azure.sonic-buildimage |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
@chander-nexthop a related PR to your changes please review |
There was a problem hiding this comment.
Pull request overview
This PR updates sonic-buildimage to better support BMC-based platforms by extending sonic-py-common with Switch-Host/BMC detection and BMC network helpers, and by aligning database/pmon behavior to treat BMC devices more like the standard device flow (removing prior bmcdb special-casing). It also introduces persistent BMC event logging while keeping /var/log on tmpfs for Switch-BMC systems to reduce eMMC wear.
Changes:
- Extend
sonic-py-commonwith platform role/cooling helpers, remote Redis connector helper, and BMC address/config accessors + unit tests. - Add Switch-BMC operational behavior: force
/var/logto tmpfs, create persistent/host/bmc/event.log, and add logrotate policy for it. - Remove
bmcdbbranching from database config/templates and adjust pmon/supervisor behavior for Switch-BMC role.
Reviewed changes
Copilot reviewed 15 out of 15 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| src/sonic-py-common/sonic_py_common/daemon_base.py | Adds db_connect_remote() helper for direct TCP Redis connections. |
| src/sonic-py-common/tests/test_daemon_base.py | Adds unit tests for db_connect_remote(). |
| src/sonic-py-common/sonic_py_common/device_info.py | Adds Switch-Host/BMC + cooling helpers and BMC config/address accessors; adds global BMC file path. |
| src/sonic-py-common/tests/device_info_test.py | Adds unit tests for the new platform/BMC detection and BMC config/address APIs. |
| files/initramfs-tools/union-mount.j2 | Forces logs-in-RAM for Switch-BMC early in boot to reduce eMMC wear. |
| files/image_config/platform/rc.local | Creates persistent /host/bmc/event.log for Switch-BMC devices at boot. |
| files/image_config/logrotate/logrotate.d/bmc-event | Adds logrotate policy for persistent BMC event log. |
| files/image_config/constants/bmc.json | Introduces a global BMC network config file intended for /etc/sonic/bmc.json. |
| files/build_templates/docker_image_ctl.j2 | Removes bmcdb dump guard; derives Switch role vars from platform_env.conf. |
| dockers/docker-platform-monitor/docker_init.j2 | Detects Switch-BMC and passes IS_SWITCH_BMC into cfggen rendering context. |
| dockers/docker-platform-monitor/docker-pmon.supervisord.conf.j2 | Starts bmcctld on Switch-BMC and prevents chassisd on Switch-BMC. |
| dockers/docker-database/docker-database-init.sh | Removes bmcdb export based on IS_BMC_DEVICE. |
| dockers/docker-database/database_config.json.j2 | Removes bmcdb conditional branches so BMP DB follows standard flow (except dpudb). |
| dockers/docker-database/multi_database_config.json.j2 | Same as above for multi-db config. |
| device/nexthop/arm64-nexthop_b27-r0/pmon_daemon_control.json | Adjusts pmon daemon enable/disable set for this arm64 platform profile. |
| { | ||
| "bmc_if_name": "bmc0", | ||
| "bmc_if_addr": "169.254.100.2", | ||
| "bmc_addr": "169.254.100.1", | ||
| "bmc_net_mask": "255.255.255.252" | ||
| } |
There was a problem hiding this comment.
The new global BMC config is added under files/image_config/constants/bmc.json, but the image build currently only copies constants.yml from this directory into /etc/sonic (see files/build_templates/sonic_debian_extension.j2:779). As a result, /etc/sonic/bmc.json may never exist on the built image, and the new device_info GLOBAL_BMC_DATA_FILE lookup will fail unless a platform-specific bmc.json is present. Please add a build/install step to place this file at /etc/sonic/bmc.json (or adjust the runtime path accordingly).
| Checks the SONiC-wide global bmc.json (/etc/sonic/bmc.json) first; if | ||
| that file is absent, falls back to the platform-specific bmc.json in | ||
| the platform directory. A platform bmc.json overrides the global file | ||
| when both are present in the sense that sonic-config-engine will copy | ||
| the platform file over the installed global one during image generation. | ||
|
|
||
| Returns: | ||
| A dict with bmc_if_name, bmc_if_addr, bmc_addr and bmc_net_mask, | ||
| or None if no bmc.json is found. | ||
| """ | ||
| try: | ||
| platform_path = get_path_to_platform_dir() | ||
| json_file = os.path.join(platform_path, BMC_DATA_FILE) | ||
| if os.path.exists(json_file): | ||
| with open(json_file, "r") as f: | ||
| if os.path.exists(GLOBAL_BMC_DATA_FILE): | ||
| with open(GLOBAL_BMC_DATA_FILE, "r") as f: | ||
| return json.load(f) | ||
| platform_path = get_path_to_platform_dir() | ||
| if platform_path: | ||
| json_file = os.path.join(platform_path, BMC_DATA_FILE) | ||
| if os.path.exists(json_file): | ||
| with open(json_file, "r") as f: | ||
| return json.load(f) |
There was a problem hiding this comment.
get_bmc_data() prefers /etc/sonic/bmc.json, but the PR currently adds bmc.json under files/image_config/constants without any verified install/copy into /etc/sonic. Unless the build templates are updated to install it, this function will return None on platforms that rely on the new global file. Please ensure the global bmc.json is actually deployed to /etc/sonic/bmc.json in the image (or change GLOBAL_BMC_DATA_FILE to point to the deployed location).
| # switch_host=1 / switch_bmc=1 may be set by platform_env.conf above. | ||
| # Normalise to 0 when absent so downstream env vars are always defined. | ||
| IS_SWITCH_HOST=${switch_host:-0} | ||
| IS_SWITCH_BMC=${switch_bmc:-0} | ||
|
|
||
| # Derive IS_BMC_DEVICE from switch_bmc so docker-database-init.sh can use | ||
| # the existing IS_BMC_DEVICE mechanism without extra env vars. | ||
| if [[ "$IS_SWITCH_BMC" == "1" ]]; then | ||
| IS_BMC_DEVICE=true | ||
| fi | ||
|
|
||
|
|
There was a problem hiding this comment.
IS_SWITCH_HOST / IS_SWITCH_BMC are computed, and IS_BMC_DEVICE is set when IS_SWITCH_BMC==1, but IS_BMC_DEVICE is no longer consumed anywhere (docker-database-init.sh no longer checks it) and IS_SWITCH_* are not exported/passed to containers. This leaves dead/misleading logic and can also pass an empty IS_BMC_DEVICE env var to the database container. Please either remove this block or make it effective by wiring the intended variables into the downstream scripts/templates (and initialize IS_BMC_DEVICE explicitly when not a BMC).
| # switch_host=1 / switch_bmc=1 may be set by platform_env.conf above. | |
| # Normalise to 0 when absent so downstream env vars are always defined. | |
| IS_SWITCH_HOST=${switch_host:-0} | |
| IS_SWITCH_BMC=${switch_bmc:-0} | |
| # Derive IS_BMC_DEVICE from switch_bmc so docker-database-init.sh can use | |
| # the existing IS_BMC_DEVICE mechanism without extra env vars. | |
| if [[ "$IS_SWITCH_BMC" == "1" ]]; then | |
| IS_BMC_DEVICE=true | |
| fi |
| # the eMMC flash with high-frequency syslog writes. | ||
| _platform=$(grep '^onie_platform=' ${rootmnt}/host/machine.conf 2>/dev/null | cut -d= -f2) | ||
| _penv="${rootmnt}/usr/share/sonic/device/${_platform}/platform_env.conf" | ||
| if [ -f "$_penv" ] && grep -q '^switch_bmc=1' "$_penv" 2>/dev/null; then |
There was a problem hiding this comment.
is there any other construct I can use to check if this is is_bmc() ? not sure if platform_env file is available at this point with intiramfs
Why I did it
Updates in sonic-buildimage changes based on : sonic-net/SONiC#2215
Work item tracking
How I did it
Some of the changes
Extend
sonic-py-commonwith platform APIsbmc.jsondb_connect_remoteDefine global BMC network config in
bmc.jsonAdd BMC event logging and tmpfs
/var/log/hostRemove
DATABASE_TYPE="bmcdb"and related guardsredis_bmpandBMP_STATE_DBto follow standard device flowDetect Switch Host vs Switch BMC via
platform_env.confIS_SWITCH_HOST/IS_SWITCH_BMCUpdate pmon for BMC role
bmcctldand suppresschassisdon Switch BMCIS_SWITCH_BMCinto supervisor configurationAdjust pmon daemon control for arm64 BMC platforms
How to verify it
TODO
Which release branch to backport (provide reason below if selected)
Tested branch (Please provide the tested image version)
Description for the changelog
Link to config_db schema for YANG module changes
A picture of a cute animal (not mandatory but encouraged)