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

Add Checkpoint and Rollback for Multi ASIC. #3299

Merged
merged 48 commits into from
Jun 14, 2024
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
48 commits
Select commit Hold shift + click to select a range
be43d9b
Add Checkpoint and Rollback for Multi ASIC.
xincunli-sonic Apr 30, 2024
a322004
Fix scope
xincunli-sonic May 1, 2024
406f3f9
Fix syntax
xincunli-sonic May 1, 2024
029d0d0
Fix pre commit check
xincunli-sonic May 1, 2024
3aec0f9
fix pre commit test
xincunli-sonic May 1, 2024
b296200
Refactor scope parameter comment
xincunli-sonic May 1, 2024
3eabf79
Add Checkpoint and Rollback for Multi ASIC.
xincunli-sonic Apr 30, 2024
9fe2bd4
Fix scope
xincunli-sonic May 1, 2024
5c1e8fa
Fix syntax
xincunli-sonic May 1, 2024
9cdff48
Fix pre commit check
xincunli-sonic May 1, 2024
6cdd6a2
fix pre commit test
xincunli-sonic May 1, 2024
edfa2dd
Refactor scope parameter comment
xincunli-sonic May 1, 2024
bebbdb5
Merge branch 'master' into xincun/multiasic-checkpoint-rollback
xincunli-sonic May 17, 2024
328a9bb
Merge branch 'xincun/multiasic-checkpoint-rollback' of https://github…
xincunli-sonic May 17, 2024
ab8fc14
Refactor replace with single config file.
xincunli-sonic May 17, 2024
c61344d
Fix pre-commit and replace function
xincunli-sonic May 18, 2024
8016675
Fix precommit
xincunli-sonic May 18, 2024
9c47e38
Refactor checkpoint and rollback.
xincunli-sonic May 22, 2024
e257b78
fix format
xincunli-sonic May 23, 2024
1cd4b88
Fix format
xincunli-sonic May 23, 2024
87760a1
Fix UT
xincunli-sonic May 23, 2024
e9a9971
Merge branch 'sonic-net:master' into xincun/multiasic-checkpoint-roll…
xincunli-sonic May 23, 2024
cf2f399
Fix UT.
xincunli-sonic May 24, 2024
57e9d3a
Add UT.
xincunli-sonic May 24, 2024
4716f4d
Fix format.
xincunli-sonic May 24, 2024
1bfb020
Fix UT
xincunli-sonic May 25, 2024
bcdfb8f
Fix UT.
xincunli-sonic May 25, 2024
c8371eb
Refactor replacer and rollbacker
xincunli-sonic May 29, 2024
7fd927e
Merge branch 'xincun/multiasic-checkpoint-rollback' of https://github…
xincunli-sonic May 29, 2024
e66d9af
fix generic_updater format
xincunli-sonic May 29, 2024
326023c
fix format
xincunli-sonic May 29, 2024
49ceca0
Refactor code.
xincunli-sonic May 31, 2024
33d6801
remove unused part.
xincunli-sonic May 31, 2024
ac85032
fix show import
xincunli-sonic May 31, 2024
d0116e1
fix ut.
xincunli-sonic May 31, 2024
26c84cc
fix format
xincunli-sonic May 31, 2024
4465fdc
fix ut.
xincunli-sonic May 31, 2024
906e903
fix ut
xincunli-sonic Jun 3, 2024
698959b
fix ut
xincunli-sonic Jun 3, 2024
5f2c6a6
fix ut.
xincunli-sonic Jun 3, 2024
e469185
fix scope
xincunli-sonic Jun 3, 2024
3e610da
fix ut.
xincunli-sonic Jun 3, 2024
acf663c
improve coverage.
xincunli-sonic Jun 4, 2024
ff036e7
fix replacer
xincunli-sonic Jun 5, 2024
3569037
fix format.
xincunli-sonic Jun 5, 2024
fbbc34e
Remove unused parts
xincunli-sonic Jun 5, 2024
c4d20c6
add get_config_db_as_text ut
xincunli-sonic Jun 6, 2024
05cb10f
Change get_config_json implementation
xincunli-sonic Jun 12, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
47 changes: 37 additions & 10 deletions config/main.py
Original file line number Diff line number Diff line change
Expand Up @@ -1168,7 +1168,7 @@ def apply_patch_for_scope(scope_changes, results, config_format, verbose, dry_ru
log.log_notice(f"'apply-patch' executed successfully for {scope_for_log} by {changes}")
except Exception as e:
results[scope_for_log] = {"success": False, "message": str(e)}
log.log_error(f"'apply-patch' executed failed for {scope_for_log} by {changes} due to {str(e)}")
log.log_warning(f"'apply-patch' executed failed for {scope_for_log} by {changes} due to {str(e)}")


# This is our main entrypoint - the main 'config' command
Expand Down Expand Up @@ -1397,7 +1397,8 @@ def apply_patch(ctx, patch_file_path, format, dry_run, ignore_non_yang_tables, i
# Empty case to force validate YANG model.
if not changes_by_scope:
asic_list = [multi_asic.DEFAULT_NAMESPACE]
asic_list.extend(multi_asic.get_namespace_list())
if multi_asic.is_multi_asic():
asic_list.extend(multi_asic.get_namespace_list())
for asic in asic_list:
changes_by_scope[asic] = []

Expand All @@ -1420,6 +1421,11 @@ def apply_patch(ctx, patch_file_path, format, dry_run, ignore_non_yang_tables, i

@config.command()
@click.argument('target-file-path', type=str, required=True)
@click.option('-s', '--scope',
help='Specify the namespace for Multi-ASIC environments. For Single-ASIC environments, \
specifying the namespace is not required.',
required=True if multi_asic.is_multi_asic() else False,
type=click.Choice(multi_asic.get_namespace_list() + ['localhost']))
@click.option('-f', '--format', type=click.Choice([e.name for e in ConfigFormat]),
default=ConfigFormat.CONFIGDB.name,
help='format of target config is either ConfigDb(ABNF) or SonicYang',
Expand All @@ -1429,14 +1435,17 @@ def apply_patch(ctx, patch_file_path, format, dry_run, ignore_non_yang_tables, i
@click.option('-i', '--ignore-path', multiple=True, help='ignore validation for config specified by given path which is a JsonPointer', hidden=True)
@click.option('-v', '--verbose', is_flag=True, default=False, help='print additional details of what the operation is doing')
@click.pass_context
def replace(ctx, target_file_path, format, dry_run, ignore_non_yang_tables, ignore_path, verbose):
def replace(ctx, target_file_path, scope, format, dry_run, ignore_non_yang_tables, ignore_path, verbose):
"""Replace the whole config with the specified config. The config is replaced with minimum disruption e.g.
if ACL config is different between current and target config only ACL config is updated, and other config/services
such as DHCP will not be affected.

**WARNING** The target config file should be the whole config, not just the part intended to be updated.

<target-file-path>: Path to the target file on the file-system."""
<target-file-path>: Path to the target file on the file-system.
If the device is a Multi-ASIC environment, please give the namespace of specific ASIC, for instance:
localhost, asic0, asic1, ...
"""
try:
print_dry_run_message(dry_run)

Expand All @@ -1445,8 +1454,14 @@ def replace(ctx, target_file_path, format, dry_run, ignore_non_yang_tables, igno
target_config = json.loads(target_config_as_text)

config_format = ConfigFormat[format.upper()]

GenericUpdater().replace(target_config, config_format, verbose, dry_run, ignore_non_yang_tables, ignore_path)
if multi_asic.is_multi_asic():
if scope not in (multi_asic.get_namespace_list() + ["localhost"]):
raise Exception(f"Failed to replace config due to wrong namespace:{scope}")
scope = scope if scope != "localhost" else multi_asic.DEFAULT_NAMESPACE
else:
scope = multi_asic.DEFAULT_NAMESPACE
GenericUpdater(namespace=scope).replace(target_config, config_format, verbose, dry_run,
ignore_non_yang_tables, ignore_path)

click.secho("Config replaced successfully.", fg="cyan", underline=True)
except Exception as ex:
Expand All @@ -1468,8 +1483,12 @@ def rollback(ctx, checkpoint_name, dry_run, ignore_non_yang_tables, ignore_path,
<checkpoint-name>: The checkpoint name, use `config list-checkpoints` command to see available checkpoints."""
try:
print_dry_run_message(dry_run)

GenericUpdater().rollback(checkpoint_name, verbose, dry_run, ignore_non_yang_tables, ignore_path)
asic_list = [multi_asic.DEFAULT_NAMESPACE]
if multi_asic.is_multi_asic():
asic_list.extend(multi_asic.get_namespace_list())
for asic in asic_list:
GenericUpdater(namespace=asic).rollback(checkpoint_name, verbose, dry_run,
ignore_non_yang_tables, ignore_path)

click.secho("Config rolled back successfully.", fg="cyan", underline=True)
except Exception as ex:
Expand All @@ -1485,7 +1504,11 @@ def checkpoint(ctx, checkpoint_name, verbose):

<checkpoint-name>: The checkpoint name, use `config list-checkpoints` command to see available checkpoints."""
try:
GenericUpdater().checkpoint(checkpoint_name, verbose)
asic_list = [multi_asic.DEFAULT_NAMESPACE]
if multi_asic.is_multi_asic():
asic_list.extend(multi_asic.get_namespace_list())
for asic in asic_list:
GenericUpdater(namespace=asic).checkpoint(checkpoint_name, verbose)

click.secho("Checkpoint created successfully.", fg="cyan", underline=True)
except Exception as ex:
Expand All @@ -1501,7 +1524,11 @@ def delete_checkpoint(ctx, checkpoint_name, verbose):

<checkpoint-name>: The checkpoint name, use `config list-checkpoints` command to see available checkpoints."""
try:
GenericUpdater().delete_checkpoint(checkpoint_name, verbose)
asic_list = [multi_asic.DEFAULT_NAMESPACE]
if multi_asic.is_multi_asic():
asic_list.extend(multi_asic.get_namespace_list())
for asic in asic_list:
GenericUpdater(namespace=asic).delete_checkpoint(checkpoint_name, verbose)

click.secho("Checkpoint deleted successfully.", fg="cyan", underline=True)
except Exception as ex:
Expand Down
13 changes: 8 additions & 5 deletions generic_config_updater/generic_updater.py
Original file line number Diff line number Diff line change
Expand Up @@ -84,14 +84,14 @@ def apply(self, patch, sort=True):
self.config_wrapper.validate_field_operation(old_config, target_config)

# Validate target config does not have empty tables since they do not show up in ConfigDb
self.logger.log_notice(f"{scope}: alidating target config does not have empty tables, " \
"since they do not show up in ConfigDb.")
self.logger.log_notice(f"{scope}: validating target config does not have empty tables, \
since they do not show up in ConfigDb.")
empty_tables = self.config_wrapper.get_empty_tables(target_config)
if empty_tables: # if there are empty tables
empty_tables_txt = ", ".join(empty_tables)
raise EmptyTableError(f"{scope}: given patch is not valid because it will result in empty tables " \
"which is not allowed in ConfigDb. " \
f"Table{'s' if len(empty_tables) != 1 else ''}: {empty_tables_txt}")
raise EmptyTableError(f"{scope}: given patch is not valid because it will result in empty tables \
which is not allowed in ConfigDb. \
Table{'s' if len(empty_tables) != 1 else ''}: {empty_tables_txt}")

# Generate list of changes to apply
if sort:
Expand Down Expand Up @@ -169,6 +169,7 @@ def __init__(self,
self.config_wrapper = config_wrapper if config_wrapper is not None else ConfigWrapper(namespace=self.namespace)

def rollback(self, checkpoint_name):
checkpoint_name = checkpoint_name + self.namespace
self.logger.log_notice("Config rollbacking starting.")
self.logger.log_notice(f"Checkpoint name: {checkpoint_name}.")

Expand All @@ -185,6 +186,7 @@ def rollback(self, checkpoint_name):
self.logger.log_notice("Config rollbacking completed.")

def checkpoint(self, checkpoint_name):
checkpoint_name = checkpoint_name + self.namespace
self.logger.log_notice("Config checkpoint starting.")
self.logger.log_notice(f"Checkpoint name: {checkpoint_name}.")

Expand Down Expand Up @@ -223,6 +225,7 @@ def list_checkpoints(self):
return checkpoint_names

def delete_checkpoint(self, checkpoint_name):
checkpoint_name = checkpoint_name + self.namespace
self.logger.log_notice("Deleting checkpoint starting.")
self.logger.log_notice(f"Checkpoint name: {checkpoint_name}.")

Expand Down
110 changes: 109 additions & 1 deletion tests/config_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -2651,6 +2651,7 @@ def setUp(self):

self.runner = CliRunner()
self.patch_file_path = 'path/to/patch.json'
self.replace_file_path = 'path/to/replace.json'
self.patch_content = [
{
"op": "add",
Expand Down Expand Up @@ -2732,6 +2733,113 @@ def test_apply_patch_dryrun_multiasic(self):
# Ensure ConfigDBConnector was never instantiated or called
mock_config_db_connector.assert_not_called()

def test_repalce_multiasic(self):
# Mock open to simulate file reading
mock_replace_content = "[]"
with patch('builtins.open', mock_open(read_data=json.dumps(mock_replace_content)), create=True) as mocked_open:
# Mock GenericUpdater to avoid actual patch application
with patch('config.main.GenericUpdater') as mock_generic_updater:
mock_generic_updater.return_value.replace = MagicMock()

print("Multi ASIC: {}".format(multi_asic.is_multi_asic()))
# Invocation of the command with the CliRunner
result = self.runner.invoke(config.config.commands["replace"],
[self.replace_file_path,
"--scope", "localhost"],
catch_exceptions=True)

print("Exit Code: {}, output: {}".format(result.exit_code, result.output))
# Assertions and verifications
self.assertEqual(result.exit_code, 0, "Command should succeed")
self.assertIn("Config replaced successfully.", result.output)

# Verify mocked_open was called as expected
mocked_open.assert_called_with(self.replace_file_path, 'r')

def test_repalce_multiasic_missing_scope(self):
# Mock GenericUpdater to avoid actual patch application
with patch('config.main.GenericUpdater') as mock_generic_updater:
mock_generic_updater.return_value.replace = MagicMock()

print("Multi ASIC: {}".format(multi_asic.is_multi_asic()))
# Invocation of the command with the CliRunner
result = self.runner.invoke(config.config.commands["replace"],
[self.replace_file_path],
catch_exceptions=True)

print("Exit Code: {}, output: {}".format(result.exit_code, result.output))
# Assertions and verifications
self.assertEqual(result.exit_code, 2, "Command should failed")
self.assertIn("Missing option \"-s\"", result.output)

def test_repalce_multiasic_with_wrong_scope(self):
# Mock GenericUpdater to avoid actual patch application
with patch('config.main.GenericUpdater') as mock_generic_updater:
mock_generic_updater.return_value.replace = MagicMock()

print("Multi ASIC: {}".format(multi_asic.is_multi_asic()))
# Invocation of the command with the CliRunner
result = self.runner.invoke(config.config.commands["replace"],
[self.replace_file_path,
"--scope", "x"],
catch_exceptions=True)

print("Exit Code: {}, output: {}".format(result.exit_code, result.output))
# Assertions and verifications
self.assertEqual(result.exit_code, 2, "Command should failed")
self.assertIn("Invalid value for \"-s\"", result.output)

def test_checkpoint_multiasic(self):
checkpointname = "checkpointname"
# Mock GenericUpdater to avoid actual patch application
with patch('config.main.GenericUpdater') as mock_generic_updater:
mock_generic_updater.return_value.checkpoint = MagicMock()

print("Multi ASIC: {}".format(multi_asic.is_multi_asic()))
# Invocation of the command with the CliRunner
result = self.runner.invoke(config.config.commands["checkpoint"],
[checkpointname],
catch_exceptions=True)

print("Exit Code: {}, output: {}".format(result.exit_code, result.output))
# Assertions and verifications
self.assertEqual(result.exit_code, 0, "Command should succeed")
self.assertIn("Checkpoint created successfully.", result.output)

def test_rollback_multiasic(self):
checkpointname = "checkpointname"
# Mock GenericUpdater to avoid actual patch application
with patch('config.main.GenericUpdater') as mock_generic_updater:
mock_generic_updater.return_value.rollback = MagicMock()

print("Multi ASIC: {}".format(multi_asic.is_multi_asic()))
# Invocation of the command with the CliRunner
result = self.runner.invoke(config.config.commands["rollback"],
[checkpointname],
catch_exceptions=True)

print("Exit Code: {}, output: {}".format(result.exit_code, result.output))
# Assertions and verifications
self.assertEqual(result.exit_code, 0, "Command should succeed")
self.assertIn("Config rolled back successfully.", result.output)

def test_delete_checkpoint_multiasic(self):
checkpointname = "checkpointname"
# Mock GenericUpdater to avoid actual patch application
with patch('config.main.GenericUpdater') as mock_generic_updater:
mock_generic_updater.return_value.deletecheckpoint = MagicMock()

print("Multi ASIC: {}".format(multi_asic.is_multi_asic()))
# Invocation of the command with the CliRunner
result = self.runner.invoke(config.config.commands["delete-checkpoint"],
[checkpointname],
catch_exceptions=True)

print("Exit Code: {}, output: {}".format(result.exit_code, result.output))
# Assertions and verifications
self.assertEqual(result.exit_code, 0, "Command should succeed")
self.assertIn("Checkpoint deleted successfully.", result.output)

@classmethod
def teardown_class(cls):
print("TEARDOWN")
Expand All @@ -2741,4 +2849,4 @@ def teardown_class(cls):
from .mock_tables import dbconnector
from .mock_tables import mock_single_asic
importlib.reload(mock_single_asic)
dbconnector.load_database_config()
dbconnector.load_database_config()
Loading