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 Parallel option for apply-patch #3373

Merged
merged 9 commits into from
Jul 9, 2024
Merged
Changes from 3 commits
Commits
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
31 changes: 28 additions & 3 deletions config/main.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
#!/usr/sbin/env python

import click
import concurrent.futures
import datetime
import ipaddress
import json
@@ -1212,7 +1213,12 @@ def multi_asic_save_config(db, filename):
with open(filename, 'w') as file:
json.dump(all_current_config, file, indent=4)


# Function to apply patch for a single ASIC.
def apply_patch_wrapper(args):
return apply_patch_for_scope(*args)


def apply_patch_for_scope(scope_changes, results, config_format, verbose, dry_run, ignore_non_yang_tables, ignore_path):
scope, changes = scope_changes
# Replace localhost to DEFAULT_NAMESPACE which is db definition of Host
@@ -1459,11 +1465,12 @@ def print_dry_run_message(dry_run):
help='format of config of the patch is either ConfigDb(ABNF) or SonicYang',
show_default=True)
@click.option('-d', '--dry-run', is_flag=True, default=False, help='test out the command without affecting config state')
@click.option('-p', '--parallel', is_flag=True, default=False, help='applying the change to all ASICs parallelly')
@click.option('-n', '--ignore-non-yang-tables', is_flag=True, default=False, help='ignore validation for tables without YANG models', hidden=True)
@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 apply_patch(ctx, patch_file_path, format, dry_run, ignore_non_yang_tables, ignore_path, verbose):
def apply_patch(ctx, patch_file_path, format, dry_run, parallel, ignore_non_yang_tables, ignore_path, verbose):
"""Apply given patch of updates to Config. A patch is a JsonPatch which follows rfc6902.
This command can be used do partial updates to the config with minimum disruption to running processes.
It allows addition as well as deletion of configs. The patch file represents a diff of ConfigDb(ABNF)
@@ -1509,8 +1516,26 @@ def apply_patch(ctx, patch_file_path, format, dry_run, ignore_non_yang_tables, i
changes_by_scope[asic] = []

# Apply changes for each scope
for scope_changes in changes_by_scope.items():
apply_patch_for_scope(scope_changes, results, config_format, verbose, dry_run, ignore_non_yang_tables, ignore_path)
if parallel:
with concurrent.futures.ThreadPoolExecutor() as executor:
# Prepare the argument tuples
arguments = [(scope_changes, results, config_format,
verbose, dry_run, ignore_non_yang_tables, ignore_path)
for scope_changes in changes_by_scope.items()]

# Submit all tasks and wait for them to complete
futures = [executor.submit(apply_patch_wrapper, args) for args in arguments]

# Wait for all tasks to complete
concurrent.futures.wait(futures)
else:
for scope_changes in changes_by_scope.items():
apply_patch_for_scope(scope_changes,
results,
config_format,
verbose, dry_run,
ignore_non_yang_tables,
ignore_path)

# Check if any updates failed
failures = [scope for scope, result in results.items() if not result['success']]
122 changes: 116 additions & 6 deletions tests/config_test.py
Original file line number Diff line number Diff line change
@@ -2965,6 +2965,77 @@ def test_apply_patch_multiasic(self):

@patch('config.main.validate_patch', mock.Mock(return_value=True))
def test_apply_patch_dryrun_multiasic(self):
# Mock open to simulate file reading
with patch('builtins.open', mock_open(read_data=json.dumps(self.patch_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.apply_patch = MagicMock()

# Mock ConfigDBConnector to ensure it's not called during dry-run
with patch('config.main.ConfigDBConnector') as mock_config_db_connector:

print("Multi ASIC: {}".format(multi_asic.is_multi_asic()))
# Invocation of the command with the CliRunner
result = self.runner.invoke(config.config.commands["apply-patch"],
[self.patch_file_path,
"--format", ConfigFormat.SONICYANG.name,
"--dry-run",
xincunli-sonic marked this conversation as resolved.
Show resolved Hide resolved
"--ignore-non-yang-tables",
"--ignore-path", "/ANY_TABLE",
"--ignore-path", "/ANY_OTHER_TABLE/ANY_FIELD",
"--ignore-path", "",
"--verbose"],
catch_exceptions=False)

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

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

# Ensure ConfigDBConnector was never instantiated or called
mock_config_db_connector.assert_not_called()

@patch('config.main.validate_patch', mock.Mock(return_value=True))
def test_apply_patch_parallel_multiasic(self):
# Mock open to simulate file reading
with patch('builtins.open', mock_open(read_data=json.dumps(self.patch_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.apply_patch = MagicMock()

# Mock ConfigDBConnector to ensure it's not called during dry-run
with patch('config.main.ConfigDBConnector') as mock_config_db_connector:

print("Multi ASIC: {}".format(multi_asic.is_multi_asic()))
# Invocation of the command with the CliRunner
result = self.runner.invoke(config.config.commands["apply-patch"],
[self.patch_file_path,
"--format", ConfigFormat.SONICYANG.name,
"--dry-run",
"--parallel",
xincunli-sonic marked this conversation as resolved.
Show resolved Hide resolved
"--ignore-non-yang-tables",
"--ignore-path", "/ANY_TABLE",
"--ignore-path", "/ANY_OTHER_TABLE/ANY_FIELD",
"--ignore-path", "",
"--verbose"],
catch_exceptions=False)

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

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

# Ensure ConfigDBConnector was never instantiated or called
mock_config_db_connector.assert_not_called()

@patch('config.main.validate_patch', mock.Mock(return_value=True))
def test_apply_patch_parallel_with_error_multiasic(self):
# Mock open to simulate file reading
with patch('builtins.open', mock_open(read_data=json.dumps(self.patch_content)), create=True) as mocked_open:
# Mock GenericUpdater to avoid actual patch application
@@ -2979,12 +3050,13 @@ def test_apply_patch_dryrun_multiasic(self):
result = self.runner.invoke(config.config.commands["apply-patch"],
[self.patch_file_path,
"--format", ConfigFormat.SONICYANG.name,
"--dry-run",
"--ignore-non-yang-tables",
"--ignore-path", "/ANY_TABLE",
"--ignore-path", "/ANY_OTHER_TABLE/ANY_FIELD",
"--ignore-path", "",
"--verbose"],
"--dry-run",
"--parallel",
"--ignore-non-yang-tables",
"--ignore-path", "/ANY_TABLE",
"--ignore-path", "/ANY_OTHER_TABLE/ANY_FIELD",
"--ignore-path", "",
"--verbose"],
catch_exceptions=False)

print("Exit Code: {}, output: {}".format(result.exit_code, result.output))
@@ -3062,6 +3134,44 @@ def test_apply_patch_validate_patch_with_badpath_multiasic(self, mock_subprocess
# Verify mocked_open was called as expected
mocked_open.assert_called_with(self.patch_file_path, 'r')

@patch('config.main.subprocess.Popen')
@patch('config.main.SonicYangCfgDbGenerator.validate_config_db_json', mock.Mock(return_value=True))
def test_apply_patch_parallel_badpath_multiasic(self, mock_subprocess_popen):
mock_instance = MagicMock()
mock_instance.communicate.return_value = (json.dumps(self.all_config), 0)
mock_subprocess_popen.return_value = mock_instance

bad_patch = copy.deepcopy(self.patch_content)
bad_patch.append({
"value": {
"policy_desc": "New ACL Table",
"ports": ["Ethernet3", "Ethernet4"],
"stage": "ingress",
"type": "L3"
}
})

# Mock open to simulate file reading
with patch('builtins.open', mock_open(read_data=json.dumps(bad_patch)), 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.apply_patch = MagicMock()

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

print("Exit Code: {}, output: {}".format(result.exit_code, result.output))
# Assertions and verifications
self.assertNotEqual(result.exit_code, 0, "Command should failed.")
self.assertIn("Failed to apply patch", result.output)

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

@patch('config.main.subprocess.Popen')
@patch('config.main.SonicYangCfgDbGenerator.validate_config_db_json', mock.Mock(return_value=True))
def test_apply_patch_validate_patch_with_wrong_fetch_config(self, mock_subprocess_popen):