Skip to content

Implement ConfigReload on full config SET update#651

Open
donggyu-nexthop wants to merge 5 commits into
sonic-net:masterfrom
nexthop-ai:implement-configreload-on-full-config
Open

Implement ConfigReload on full config SET update#651
donggyu-nexthop wants to merge 5 commits into
sonic-net:masterfrom
nexthop-ai:implement-configreload-on-full-config

Conversation

@donggyu-nexthop

@donggyu-nexthop donggyu-nexthop commented Apr 17, 2026

Copy link
Copy Markdown

Why I did it

Resolves sonic-net/sonic-buildimage#22362

The current "full config update" operation over gNMI silently fails.

This is because the operation relied on GNOI System Reboot to pick up the requested configuration and apply it; however, the proper reboot call implementation ripped out the configuration file handling to better align with the GNOI system specs. This left the config reload operation non-functional.

How I did it

client.MixedDbClient has a new field which can optionally store "post-response callbacks", which are just functions that will be invoked after the gRPC response has been issued back to the client.

During SetFullConfig, this field will be set to a callback that invokes DBusClient::ConfigReload(). Then, at the gNMI server layer, the server will invoke MixedDbClient::RunCallback() to execute the callback (if registered).

The reason this step had to be deferred until the very end of the request lifecycle was because the ConfigReload D-Bus endpoint will restart ALL services, including the gNMI server, which means that calling this function directly from within SetFullConfig leaves the gNMI client hanging with no response.

How to verify it

  • prep a testbed with changes from this branch
  • ssh into testbed, and use gnmic to issue SET commands over gnmi
    • example query:
gnmic -a "<DUT ip>:<DUT port>" --insecure set \
--encoding json_ietf \
--delete "sonic-db:/CONFIG_DB/localhost/" \
--update-path "sonic-db:/CONFIG_DB/localhost/" \
--update-file <path to json with new config db>
  • then on the DUT, verify the changes were successful
# if you modified the description of ethernet 4 port
sonic-db-cli hget "PORT|Ethernet4" description

Which release branch to backport (provide reason below if selected)

  • 201811
  • 201911
  • 202006
  • 202012
  • 202106
  • 202111

Description for the changelog

Implement gNMI config reload without relying on gNOI reboots

Link to config_db schema for YANG module changes

N/A for this PR

A picture of a cute animal (not mandatory but encouraged)

Signed-off-by: donggyu-nexthop <donggyu@nexthop.ai>
@mssonicbld

Copy link
Copy Markdown
Contributor

/azp run

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines successfully started running 1 pipeline(s).

Signed-off-by: Donggyu Kim <donggyu@nexthop.ai>
@donggyu-nexthop
donggyu-nexthop force-pushed the implement-configreload-on-full-config branch from 1df4de5 to d2317d3 Compare April 17, 2026 21:45
@mssonicbld

Copy link
Copy Markdown
Contributor

/azp run

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines successfully started running 1 pipeline(s).

Signed-off-by: Donggyu Kim <donggyu@nexthop.ai>
@mssonicbld

Copy link
Copy Markdown
Contributor

/azp run

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines successfully started running 1 pipeline(s).

Signed-off-by: Donggyu Kim <donggyu@nexthop.ai>
@mssonicbld

Copy link
Copy Markdown
Contributor

/azp run

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines successfully started running 1 pipeline(s).

@mssonicbld

Copy link
Copy Markdown
Contributor

/azp run

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines successfully started running 1 pipeline(s).

Signed-off-by: Donggyu Kim <donggyu@nexthop.ai>
@donggyu-nexthop
donggyu-nexthop force-pushed the implement-configreload-on-full-config branch from 1bf8cdc to e54c500 Compare April 21, 2026 22:20
@mssonicbld

Copy link
Copy Markdown
Contributor

/azp run

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines successfully started running 1 pipeline(s).

@donggyu-nexthop
donggyu-nexthop marked this pull request as ready for review April 22, 2026 00:05
@ronan-nexthop

Copy link
Copy Markdown
Contributor

@donggyu-nexthop can you use git blame and tag people who may be suitable to review here. I think because no reviewers are auto-assigned people may not be activly looking at this.

@sneelam20 perhaps a good example of why the @gnmi-dev alias would be good to have auto-added to this, or alternatively, some form of auto-assigning based on git blame

@donggyu-nexthop

Copy link
Copy Markdown
Author

@ganglyu @abdosi @jipanyang @ndas7 could you give this a look? thank you!

@anders-nexthop

Copy link
Copy Markdown
Contributor

@sneelam20 @hdwhdw can we get some reviewers assigned to look at this? I do not have permissions to add reviewers.

This PR was discussed in the 5/14 UMF meeting. sonic-gnmi used to support a whole-file config replace option, which was invoked by passing the filename to the rpc as an UPDATE. The original implementation relied on an early version of GnoiReboot which did a 'config reload' rather than a hard reboot. This feature broke when an actual GnoiReboot impl was added. This PR plumbs the feature correctly, using DBUS to call 'config reload' directly.

This is not the same usage as the save-on-set feature, which triggers 'config save' after every write to make config changes persistent. This approach is intended for whole-config operations instead, where the user wants to bulk-load a config throw away anything pre-existing, then reload the system so that configuration takes effect.


ctx, cancel := context.WithTimeout(context.Background(), 15*time.Second)
defer cancel()
value, err := redisDb.HGet(ctx, "FEATURE|gnmi", "auto_restart").Result()

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

GetRedisDBClient() routes this lookup to STATE_DB, but FEATURE|gnmi:auto_restart is in CONFIG_DB.

return fmt.Errorf("Yang validation failed!")
}

c.configReloadCallback = func(autoRestartEnabled bool) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Set returns success before ConfigReload runs. If reload fails after CONFIG_DB is flushed, the failure cannot be reported and there is no rollback. Is that an acceptable risk for the first version? If not, a higher-level option is to stage a checkpoint and let the SONiC host service apply or restore it after gNMI shuts down.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

yeah this is generally the trick about this feature. config reload will bring down gnmi - how do we ensure we can send a clean response back to the user if the signal we are waiting for is gnmi container being restarted? we thought we could at least send a response that the command was accepted (passed yang validation and is being triggered). @hdwhdw do you have any ideas of a better way to handle this?

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.

Enhancement: Implement ConfigReload for full config update

5 participants