save config / factory reset - #1523
Conversation
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Comment |
|
🚀 Temp artifacts published: |
e4d0952 to
e8056ff
Compare
not yet implemented for Cumulus; nothing calls these for the moment Signed-off-by: Emanuele Di Pascale <emanuele@githedgehog.com>
d7a6d52 to
b20fde6
Compare
There was a problem hiding this comment.
Pull request overview
Adds “save running config” support to the dozer.Processor abstraction and implements initial SONiC-based save/reset behaviors for Broadcom and Celestica+ platforms (with Cumulus left as unsupported for now).
Changes:
- Extend
dozer.Processorwith aSaveConfig(ctx)method. - Implement Broadcom
FactoryResetvia PTY-backed execution and add BroadcomSaveConfig. - Implement Celestica+
FactoryReset/SaveConfig; add CumulusSaveConfigstub.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| pkg/agent/dozer/dozer.go | Adds SaveConfig to the Processor interface. |
| pkg/agent/dozer/bcm/processor.go | Implements Broadcom FactoryReset and introduces SaveConfig via sonic-cli. |
| pkg/agent/cmls/processor.go | Adds SaveConfig stub returning unsupported. |
| pkg/agent/clsp/processor.go | Implements Celestica+ FactoryReset and SaveConfig via sonic-cli. |
Comments suppressed due to low confidence (1)
pkg/agent/clsp/processor.go:122
exec.CommandContextdoes not perform shell parsing; the embedded quotes in"copy running-config startup-config"will be passed literally and can breaksonic-cli -c. Pass the full command as a single argument without embedded quotes.
cmd := exec.CommandContext(ctx, "sonic-cli", "-c", "\"copy running-config startup-config\"")
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
🚀 Temp artifacts published: |
not yet implemented for Cumulus; nothing calls these for the moment
Part of https://github.com/githedgehog/internal/issues/417