gNOI: Add ConfigSave and ConfigReload RPCs on SonicService - #728
Open
Verma-Anukul wants to merge 1 commit into
Open
Verma-Anukul wants to merge 1 commit into
Verma-Anukul wants to merge 1 commit into
Conversation
…t#55) * MIGSOFTWAR-41331: Add gNOI ConfigSave and ConfigReload RPCs Expose `config save` (running -> startup) and `config reload` over gNOI on the SONiC-specific `gnoi.sonic.SonicService`. The handlers call the existing host-service D-Bus methods directly (no translib indirection): - SonicService.ConfigSave -> org.SONiC.HostService.config.save - SonicService.ConfigReload -> org.SONiC.HostService.config.reload ConfigSave is parameterless and always persists the running CONFIG_DB to /etc/sonic/config_db.json. ConfigReload accepts an optional inline JSON payload; when empty the host service reloads from the startup file, otherwise the JSON is validated and piped to `config reload -y /dev/stdin`. Both RPCs require write-level gNOI authentication and surface backend failures as gRPC error codes. The raw reload payload is never logged (CONFIG_DB may carry credentials); only an inline=<bool> marker is. Also wires both RPCs into the gnoi_client CLI under -module Sonic (`configSave`, `configReload`) and adds reachability tests covering the empty-payload and inline-JSON branches. Backend D-Bus methods already exist in upstream sonic-host-services, so no host-side changes are required. * MIGSOFTWAR-41331: Address review feedback on gNOI ConfigSave/ConfigReload Style and structure refinements following review: - gnmi_server/gnoi.go * Drop the locally-introduced defaultStartupConfigPath constant; no such constant exists elsewhere in sonic-buildimage and the rest of sonic-gnmi (server.go SaveOnSetEnabled, mixed_db_client.go) uses the literal "/etc/sonic/config_db.json". Use the same literal for consistency. * Trim the multi-line doc comments on ConfigSave / ConfigReload to the 1-2 line style used by sibling RPCs in this file. * Drop the filename from the ConfigSave log line; sibling SonicService RPCs log only "gNOI: Sonic <Name>". - gnmi_server/gnoi_config_test.go (new) * Move tests out of clear_neighbor_dummy_test.go (which is marked for removal) into a dedicated gnoi_config_test.go. * Replace the dummy reachability tests with proper unit tests modeled on gnoi_reset_test.go: gomonkey-patch ssc.NewDbusClient and ssc.DbusApi to cover success, DBus-client-creation failure, DBus-call failure, and (for ConfigReload) the empty-payload, inline-JSON, and InvalidArgument paths. - gnmi_server/clear_neighbor_dummy_test.go * Reverted to the upstream baseline (the ConfigSave/ConfigReload dummy tests have moved to gnoi_config_test.go). The error-code conventions, no-defer-Close, and SonicOutput response shape continue to follow the closest analog (factory_reset.Start in gnoi_reset.go) which is the only other gNOI RPC in this codebase that calls a host-service D-Bus method directly rather than going through translib. * MIGSOFTWAR-41331: Extract defaultConfigDBPath const and rename test file - gnmi_server/gnoi.go * Add defaultConfigDBPath const ("/etc/sonic/config_db.json") to the existing const block (alongside stateDB, mirroring its style). * Use the const in ConfigSave instead of repeating the literal. - gnmi_server/{gnoi_config_test.go => sonic_config_test.go} * Rename via git mv (content unchanged). The old name suggested a standard gNOI service test (sibling to gnoi_reset_test.go, gnoi_os_test.go, gnoi_file_test.go, ...), but ConfigSave/ ConfigReload are extensions on the SONiC-specific SonicService. The new name follows the existing SonicService test convention (cf. clear_neighbor_dummy_test.go for SonicService.ClearNeighbors) of using a feature-based name without the gnoi_ prefix. * MIGSOFTWAR-41331: Rename test file to sonic_config_service_test.go The previous name (sonic_config_test.go) was ambiguous and could be misread as testing SONiC configuration in general. The new name makes it explicit that the file tests RPCs on the gnoi.sonic SonicService (currently ConfigSave and ConfigReload). Signed-off-by: Verma-Anukul <anukulverma2013@gmail.com>
Contributor
|
/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). |
Verma-Anukul
changed the title
Contributor
Author
|
@hdwhdw |
Contributor
|
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
|
Contributor
Author
|
@hdwhdw |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Expose SONiC
config saveandconfig reloadover gNOI ongnoi.sonic.SonicService.Before
No gNOI RPC existed for persisting or reloading configuration. Operators had to use the CLI or SSH.
After
Two new RPCs call existing host-service D-Bus methods directly:
ConfigSave→ persists running CONFIG_DB to/etc/sonic/config_db.jsonConfigReload→ reloads from startup file, or from inline JSON when providedLogs
Test plan
go test ./gnmi_server/... -run 'TestConfigSave|TestConfigReload'gnoi_client -module Sonic -rpc configSavegnoi_client -module Sonic -rpc configReload(empty and inline JSON)