Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Reset factory hld #1231

Merged
merged 12 commits into from
Apr 13, 2023
Merged

Conversation

Mohammedz93
Copy link
Contributor

@Mohammedz93 Mohammedz93 commented Jan 23, 2023

This PR provides high level design for Reset factory.

PR title state context
[sonic-buildimage] Support Reset factory GitHub issue/pull request detail GitHub pull request check contexts
[sonic-mgmt] Reset factory tests GitHub issue/pull request detail GitHub pull request check contexts

@Mohammedz93 Mohammedz93 mentioned this pull request Jan 23, 2023
Copy link
Member

@skg-net skg-net left a comment

Choose a reason for hiding this comment

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

Thanks for the HLD.

  1. Can you elaborate on the keep-basic? I see you have mentioned few tables, how the dependencies between the tables will be addressed. Is there a guideline which tables are allowed etc?
  2. Can you add details how ssh keys will be handled during factory reset?
  3. I assume the users will also be deleted during factory reset, in that case I'm not sure why to retain the home directory.
  4. Is there a clear logs command in SONiC? if yes, how is keep-all-config different from clear logs command?


## 4 <a name='CLI'></a>CLI

#### reset-factory

Choose a reason for hiding this comment

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

From an implementation/design perspective, can the command be "reset factory" (no hyphen) and implemented like the current cli/click commands (config, show, etc.)? This would allow the ability to add different reset sub-commands in the future.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

"reset" is already utilized in Linux for terminal initialization. please see reset
The idea was to implement it as bash script ( same as config-setup) for better performances, as we are going only to clear files and run Linux shell commands.

```
==============================================================================
root@host:~$ sudo reset-factory --help
Usage: reset-factory < keep-all-config | only-config | keep-basic >
Copy link
Collaborator

Choose a reason for hiding this comment

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

Add 'none' or 'default' option as well.

Copy link
Contributor Author

@Mohammedz93 Mohammedz93 Jan 26, 2023

Choose a reason for hiding this comment

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

I think it would be confusing to print something like:
Usage: reset-factory < none | keep-all-config | only-config | keep-basic >
I will explain about the default flow before the parameters (line 185)

Create factory default configuration and save it to
to ${CONFIG_DB_JSON}.

keep-basic - Preserves basic configurations only.
Copy link
Collaborator

Choose a reason for hiding this comment

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

keep-basic name doesnot look right, is it possible to have a default 'filter-list' with tables (e.g MGMT_PORT,MGMT_INTERFACE,PASSW_HARDENING), hope user can dynamically provide the tables to filter in a file as an option during factory-reset command execution..

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Please see my comment
We need to take into account the user use case.
We want to allow the user to reset the system to factory settings without losing basic configurations.
I feel that "keep-basic" suits the use case and it explains the functionality of the command better than "filter-list".
Vendor can still extend/replace this list by changing "config-setup.conf" as explained in the design.

### 3.1 <a name='Reset-Factory'></a>Reset-Factory

##### Default
It will generate the default config_db.json file by running command:<br/>
Copy link
Contributor

Choose a reason for hiding this comment

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

What is really factory reset means? How is different from brand new device vs the factory reset device?
Ex: Use case - if RMA device or send device back to vendor wanted to delete all the data for security reasons?

  1. Remove /etc/sonic files
  2. users & passwd
  3. dockers overlay filesystem

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There are a few use cases:

  1. System was tested in the vendor's lab prior to shipment to customer. before shipment any configuration should be deleted. but software installation should remain as is.
  2. Customer got to configuration conflict and wishes to start from scratch.
  3. Wiping information for security reason, before moving the system (including the case of shipping back to the vendor).

I would say that we want the system to function as a newly manufactured system.
This does not mean that the file system must be in the same state.
We want to achieve that by:

  • Resetting configurations to defaults (config_db.json)
  • Clear files that may potentially affect the functionality or user experience :
    This is very challenging as sudo user can change almost any file/directory in the system.
    We will try to cover the files that might be changed by the system.
    If the user choose to change the file system directly rather than using the CLI/official APIs, then the responsibility falls on him.
  1. Remove /etc/sonic files:
    I believe we can clear the /etc/sonic directory under the upperdir of overlayfs (e.g. /host/image*/rw/etc/sonic)
    but I need to test it in different scenarios first.

  2. users & passwd:
    This will be handled by User MGMT feature.
    but since Reset Factory might be merged before User MGMT, we will handle this also.
    We will delete non-default users and configure the default password for default users (e.g. admin )

  3. dockers overlay filesystem:
    As discussed in the HLD review meeting, this will be covered by removing all docker containers on system.

I will update this HLD with the latest changes very soon,

#### reset-factory
```
==============================================================================
root@host:~$ sudo reset-factory --help
Copy link
Contributor

Choose a reason for hiding this comment

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

Please support yang model for this new configurations.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Why we need yang model ? isn't only for DB tables ?
We add any new DB table.

config-setup factory<br/>
It will clear system logs and files.

##### Keep-basic
Copy link
Contributor

Choose a reason for hiding this comment

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

what are the use cases and the scope addressed by this design?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We want to allow the user to preserve a basic set of configurations that he will likely always want to to preserve and settings necessary for the box to be reachable from the outside.
So user won't need to reconfigure his MGMT interface, ssh servers and users of the system ( in future).

In this design, we are defining the default basic tables that we will preserve in the new "config_db.json" file. but vendors might choose to overwrite/extend this list by changing "config-setup..conf" file.

##### Only-config
It will generate the default config_db.json file by running command:<br/>
config-setup factory only-config<br/>
It won't clear system logs and files.
Copy link
Contributor

Choose a reason for hiding this comment

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

Does design handles bgp-frr-config which stored in BGP docker mounted to /etc/sonic?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Please see my comment
I believe this will cover it.
But I would like to know that exact flow and path of folder, Can you please share the relevant HLD/File ?

- Run reset-factory without parameters
- Run reset-factory with keep-all-config/only-config/keep-basic
- Run config-setup factory without parameters
- Run config-setup factory with keep-basic
Copy link
Contributor

Choose a reason for hiding this comment

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

Can this HLD handle a warm reboot scenario where synd has files mounted to sda3 ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Are you referring to files under "/host/warmboot" ?
If so, I believe we can just clear this directory as we are going also to clear the reboot-cause folder "/host/reboot-cause"

@liat-grozovik
Copy link
Collaborator

@madhupalu @venkatmahalingam please review HLD fixes following your comments.

@liat-grozovik
Copy link
Collaborator

@madhupalu please review comments reply. if you have no further concern please approve the PR.
@venkatmahalingam and further comments from your side?

Note: if no other comments provided by end of this week, the HLD PR will be merged.

@liat-grozovik
Copy link
Collaborator

@venkatmahalingam any further comment? if, not please approve so we can merge the HLD and start with the code PR review.

@liat-grozovik
Copy link
Collaborator

@zhangyanzhao I suggest to merge this HLD as of approval and no additional comments in the last 2 weeks.

@liat-grozovik liat-grozovik merged commit 04c208a into sonic-net:master Apr 13, 2023
@qiluo-msft
Copy link
Contributor

@rajendra-dendukuri Could you help review (even it is already merged)?

qiluo-msft pushed a commit to sonic-net/sonic-buildimage that referenced this pull request Jul 11, 2023
#### Why I did it
Support reset factory in Sonic OS
[Reset Factory HLD](sonic-net/SONiC#1231)
[Sonic-mgmt tests](sonic-net/sonic-mgmt#7652)

#### How I did it
- Added new script "/usr/bin/reset-factory"
   * It generates a new config_db.json files with factory configurations
   * It clears system files and logs
   * It removes all docker containers on system except database
   * It clears non-default users and restores default users password
- Dump the default users info to a new file during build "/etc/sonic/default_users.json"
- Supported new type "Keep-basic" in "config-setup factory"
- Add new conf file for config-setup "/etc/config-setup/config-setup.conf

#### How to verify it
- Run reset-factory script with all types: < none | keep-all-config | only-config | keep-basic >
- Run config-setup factory with parameters < none | keep-basic >

#### Description for the changelog
Support reset factory in Sonic OS

#### Ensure to add label/tag for the feature raised. example - PR#2174 under sonic-utilities repo. where, Generic Config and Update feature has been labelled as GCU.
sonic-otn pushed a commit to sonic-otn/sonic-buildimage that referenced this pull request Sep 20, 2023
#### Why I did it
Support reset factory in Sonic OS
[Reset Factory HLD](sonic-net/SONiC#1231)
[Sonic-mgmt tests](sonic-net/sonic-mgmt#7652)

#### How I did it
- Added new script "/usr/bin/reset-factory"
   * It generates a new config_db.json files with factory configurations
   * It clears system files and logs
   * It removes all docker containers on system except database
   * It clears non-default users and restores default users password
- Dump the default users info to a new file during build "/etc/sonic/default_users.json"
- Supported new type "Keep-basic" in "config-setup factory"
- Add new conf file for config-setup "/etc/config-setup/config-setup.conf

#### How to verify it
- Run reset-factory script with all types: < none | keep-all-config | only-config | keep-basic >
- Run config-setup factory with parameters < none | keep-basic >

#### Description for the changelog
Support reset factory in Sonic OS

#### Ensure to add label/tag for the feature raised. example - PR#2174 under sonic-utilities repo. where, Generic Config and Update feature has been labelled as GCU.
@skg-net
Copy link
Member

skg-net commented Feb 6, 2024

@Mohammedz93 Can you please update the Quality Metric (Alpha/Beta/GA) for the feature either in this PR comments or in HLD itself based on https://github.com/sonic-net/SONiC/blob/master/doc/SONiC%20feature%20quality%20definition.md
Thanks

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.

7 participants