From c81b1cad40a9247eb50d4da48a2f8db447c22db2 Mon Sep 17 00:00:00 2001 From: Qi Luo Date: Sun, 15 Aug 2021 10:15:36 +0000 Subject: [PATCH 1/5] Implement ChangeApplier for ConfigDb format --- generic_config_updater/generic_updater.py | 52 +++++++++++++++++++++-- 1 file changed, 48 insertions(+), 4 deletions(-) diff --git a/generic_config_updater/generic_updater.py b/generic_config_updater/generic_updater.py index 8fd36ced91..aabdd09394 100644 --- a/generic_config_updater/generic_updater.py +++ b/generic_config_updater/generic_updater.py @@ -1,8 +1,10 @@ import json import os from enum import Enum +from jsonpatch import PatchOperation, AddOperation, RemoveOperation, ReplaceOperation +from swsscommon.swsscommon import ConfigDBConnector from .gu_common import GenericConfigUpdaterError, ConfigWrapper, \ - DryRunConfigWrapper, PatchWrapper + DryRunConfigWrapper, PatchWrapper, JsonChange from .patch_sorter import PatchSorter CHECKPOINTS_DIR = "/etc/sonic/checkpoints" @@ -18,9 +20,51 @@ def release_lock(self): pass class ChangeApplier: - def apply(self, change): - # TODO: Implement change applier - raise NotImplementedError("ChangeApplier.apply(change) is not implemented yet") + def __init__(self): + self.config_db = ConfigDBConnector() + self.config_db.connect() + + def _apply_entry(self, op: PatchOperation): + parts = op.pointer.parts + entry = self.config_db.get_entry(parts[0], parts[1]) + oldtree = entry + for part in parts[1::-1]: + oldtree = { part: oldtree } + op.apply(oldtree) + self.config_db.set_entry(parts[0], parts[1], entry) + + def apply(self, change: JsonChange): + patch = change.patch + + # JsonChange is a list of PatchOperation + for op in patch._ops: + parts = op.pointer.parts + nparts = len(parts) + value = op.operation.get('value', None) + + + tree = value + for part in parts[::-1]: + tree = { part: tree } + if isinstance(op, AddOperation): + self.config_db.mod_config(tree) + elif isinstance(op, RemoveOperation): + if nparts == 1: + # Delete a table + self.config_db.delete_table(parts[0]) + elif nparts == 2: + # Delete a key in a table + self.config_db.set_entry(parts[0], parts[1], None) + else: + self._apply_entry(op) + elif isinstance(op, ReplaceOperation): + if nparts <= 2: + raise NotImplementedError("ConfigDb does not support replace a table name or key name") + else: + self._apply_entry(op) + else: + raise NotImplementedError("Not supported PatchOperation type") + class ConfigFormat(Enum): CONFIGDB = 1 From cf383dbec1b527fc7f3934704996989480b898c5 Mon Sep 17 00:00:00 2001 From: Qi Luo Date: Sun, 15 Aug 2021 12:31:38 +0000 Subject: [PATCH 2/5] ChangeApplier: replace services for table name Signed-off-by: Qi Luo --- generic_config_updater/generic_updater.py | 46 ++++++++++++++++++++++- 1 file changed, 45 insertions(+), 1 deletion(-) diff --git a/generic_config_updater/generic_updater.py b/generic_config_updater/generic_updater.py index aabdd09394..9a3493df5f 100644 --- a/generic_config_updater/generic_updater.py +++ b/generic_config_updater/generic_updater.py @@ -1,5 +1,6 @@ import json import os +import subprocess from enum import Enum from jsonpatch import PatchOperation, AddOperation, RemoveOperation, ReplaceOperation from swsscommon.swsscommon import ConfigDBConnector @@ -20,9 +21,46 @@ def release_lock(self): pass class ChangeApplier: - def __init__(self): + def __init__(self, service_metadata_file: str=None): self.config_db = ConfigDBConnector() self.config_db.connect() + service_metadata = {} + if service_metadata_file: + with open(service_metadata_file, 'r') as f: + service_metadata = json.load(f) + self.table_service = service_metadata.get('tables', {}) + self.service_commands = service_metadata.get('services', {}) + self.valiate_metadata() + self.previous_table_name = None + + def valiate_metadata(self): + # TODO: check metadata file syntax + pass + + def exec_command(self, cmd) -> int: + p = subprocess.Popen(cmd) + p.communicate() + return p.returncode + + def on_table_operation(self, table_name): + services = self.table_service.get(table_name, {}) + for service in services: + commands = self.service_commands.get(service, {}) + restart_comand = commands.get('restart-command') + validate_command = commands.get('validate-commands') + if restart_comand: + rc = self.exec_command(restart_comand) + if rc != 0: + raise GenericConfigUpdaterError(f"Restart command failed: {restart_comand}, rc={rc}") + rc = self.exec_command(validate_command) + if rc != 0: + raise GenericConfigUpdaterError(f"Validate command failed: {validate_command}, rc={rc}") + + def on_table_operation_lazy(self, table_name): + # Optimze: reduce the duplicated restarting in a batch + if table_name != self.previous_table_name: + self.on_table_operation(self.previous_table_name) + self.previous_table_name = table_name def _apply_entry(self, op: PatchOperation): parts = op.pointer.parts @@ -36,6 +74,7 @@ def _apply_entry(self, op: PatchOperation): def apply(self, change: JsonChange): patch = change.patch + self.previous_table_name = None # JsonChange is a list of PatchOperation for op in patch._ops: parts = op.pointer.parts @@ -48,13 +87,18 @@ def apply(self, change: JsonChange): tree = { part: tree } if isinstance(op, AddOperation): self.config_db.mod_config(tree) + if nparts == 2: + # Added a key in a table + self.on_table_operation_lazy(parts[0]) elif isinstance(op, RemoveOperation): if nparts == 1: # Delete a table self.config_db.delete_table(parts[0]) + self.on_table_operation_lazy(parts[0]) elif nparts == 2: # Delete a key in a table self.config_db.set_entry(parts[0], parts[1], None) + self.on_table_operation_lazy(parts[0]) else: self._apply_entry(op) elif isinstance(op, ReplaceOperation): From 7861c58a11850a5572f48a8338f8dd9ea5da4f68 Mon Sep 17 00:00:00 2001 From: Qi Luo Date: Wed, 8 Sep 2021 07:44:09 +0000 Subject: [PATCH 3/5] Refine validate_command logic --- generic_config_updater/generic_updater.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/generic_config_updater/generic_updater.py b/generic_config_updater/generic_updater.py index 9a3493df5f..a87cb0914d 100644 --- a/generic_config_updater/generic_updater.py +++ b/generic_config_updater/generic_updater.py @@ -47,11 +47,12 @@ def on_table_operation(self, table_name): for service in services: commands = self.service_commands.get(service, {}) restart_comand = commands.get('restart-command') - validate_command = commands.get('validate-commands') if restart_comand: rc = self.exec_command(restart_comand) if rc != 0: raise GenericConfigUpdaterError(f"Restart command failed: {restart_comand}, rc={rc}") + validate_command = commands.get('validate-commands') + if validate_command: rc = self.exec_command(validate_command) if rc != 0: raise GenericConfigUpdaterError(f"Validate command failed: {validate_command}, rc={rc}") From 727685dcfa1dda9cde396be524d04d2afe8cf765 Mon Sep 17 00:00:00 2001 From: Qi Luo Date: Wed, 15 Sep 2021 11:25:26 +0000 Subject: [PATCH 4/5] Extract class to a new file Signed-off-by: Qi Luo --- generic_config_updater/change_applier.py | 98 +++++++++++++++++++++++ generic_config_updater/generic_updater.py | 92 +-------------------- 2 files changed, 99 insertions(+), 91 deletions(-) create mode 100644 generic_config_updater/change_applier.py diff --git a/generic_config_updater/change_applier.py b/generic_config_updater/change_applier.py new file mode 100644 index 0000000000..f451329455 --- /dev/null +++ b/generic_config_updater/change_applier.py @@ -0,0 +1,98 @@ +import json +from jsonpatch import PatchOperation, AddOperation, RemoveOperation, ReplaceOperation +from swsscommon.swsscommon import ConfigDBConnector +from .gu_common import GenericConfigUpdaterError, JsonChange + +class ChangeApplier: + def __init__(self, service_metadata_file: str=None): + self.config_db = ConfigDBConnector() + self.config_db.connect() + service_metadata = {} + if service_metadata_file: + with open(service_metadata_file, 'r') as f: + service_metadata = json.load(f) + self.table_service = service_metadata.get('tables', {}) + self.service_commands = service_metadata.get('services', {}) + self.valiate_metadata() + self.previous_table_name = None + + def valiate_metadata(self): + # TODO: check metadata file syntax + pass + + def exec_command(self, cmd) -> int: + p = subprocess.Popen(cmd) + p.communicate() + return p.returncode + + def on_table_operation(self, table_name): + services = self.table_service.get(table_name, {}) + for service in services: + commands = self.service_commands.get(service, {}) + restart_comand = commands.get('restart-command') + if restart_comand: + rc = self.exec_command(restart_comand) + if rc != 0: + raise GenericConfigUpdaterError(f"Restart command failed: {restart_comand}, rc={rc}") + validate_command = commands.get('validate-commands') + if validate_command: + rc = self.exec_command(validate_command) + if rc != 0: + raise GenericConfigUpdaterError(f"Validate command failed: {validate_command}, rc={rc}") + + def on_table_operation_lazy(self, table_name): + # Optimze: reduce the duplicated restarting in a batch + if table_name != self.previous_table_name: + self.on_table_operation(self.previous_table_name) + self.previous_table_name = table_name + + def _apply_entry(self, op: PatchOperation): + parts = op.pointer.parts + entry = self.config_db.get_entry(parts[0], parts[1]) + oldtree = entry + for part in parts[1::-1]: + oldtree = { part: oldtree } + op.apply(oldtree) + self.config_db.set_entry(parts[0], parts[1], entry) + + def apply(self, change: JsonChange): + ## Note: ordering of applying the modifications in a JsonChange is arbitrary + + ## TODO: The implementation is not optimized. + ## Please folllow the stages in design doc https://github.com/Azure/SONiC/blob/master/doc/config-generic-update-rollback/Json_Change_Application_Design.md#22-functional-description + patch = change.patch + + self.previous_table_name = None + # JsonChange is a list of PatchOperation + for op in patch._ops: + parts = op.pointer.parts + nparts = len(parts) + value = op.operation.get('value', None) + + + tree = value + for part in parts[::-1]: + tree = { part: tree } + if isinstance(op, AddOperation): + self.config_db.mod_config(tree) + if nparts == 2: + # Added a key in a table + self.on_table_operation_lazy(parts[0]) + elif isinstance(op, RemoveOperation): + if nparts == 1: + # Delete a table + self.config_db.delete_table(parts[0]) + self.on_table_operation_lazy(parts[0]) + elif nparts == 2: + # Delete a key in a table + self.config_db.set_entry(parts[0], parts[1], None) + self.on_table_operation_lazy(parts[0]) + else: + self._apply_entry(op) + elif isinstance(op, ReplaceOperation): + if nparts <= 2: + raise NotImplementedError("ConfigDb does not support replace a table name or key name") + else: + self._apply_entry(op) + else: + raise NotImplementedError("Not supported PatchOperation type") diff --git a/generic_config_updater/generic_updater.py b/generic_config_updater/generic_updater.py index a87cb0914d..cce9d0d36d 100644 --- a/generic_config_updater/generic_updater.py +++ b/generic_config_updater/generic_updater.py @@ -7,6 +7,7 @@ from .gu_common import GenericConfigUpdaterError, ConfigWrapper, \ DryRunConfigWrapper, PatchWrapper, JsonChange from .patch_sorter import PatchSorter +from .change_applier import ChangeApplier CHECKPOINTS_DIR = "/etc/sonic/checkpoints" CHECKPOINT_EXT = ".cp.json" @@ -20,97 +21,6 @@ def release_lock(self): # TODO: Implement ConfigLock pass -class ChangeApplier: - def __init__(self, service_metadata_file: str=None): - self.config_db = ConfigDBConnector() - self.config_db.connect() - service_metadata = {} - if service_metadata_file: - with open(service_metadata_file, 'r') as f: - service_metadata = json.load(f) - self.table_service = service_metadata.get('tables', {}) - self.service_commands = service_metadata.get('services', {}) - self.valiate_metadata() - self.previous_table_name = None - - def valiate_metadata(self): - # TODO: check metadata file syntax - pass - - def exec_command(self, cmd) -> int: - p = subprocess.Popen(cmd) - p.communicate() - return p.returncode - - def on_table_operation(self, table_name): - services = self.table_service.get(table_name, {}) - for service in services: - commands = self.service_commands.get(service, {}) - restart_comand = commands.get('restart-command') - if restart_comand: - rc = self.exec_command(restart_comand) - if rc != 0: - raise GenericConfigUpdaterError(f"Restart command failed: {restart_comand}, rc={rc}") - validate_command = commands.get('validate-commands') - if validate_command: - rc = self.exec_command(validate_command) - if rc != 0: - raise GenericConfigUpdaterError(f"Validate command failed: {validate_command}, rc={rc}") - - def on_table_operation_lazy(self, table_name): - # Optimze: reduce the duplicated restarting in a batch - if table_name != self.previous_table_name: - self.on_table_operation(self.previous_table_name) - self.previous_table_name = table_name - - def _apply_entry(self, op: PatchOperation): - parts = op.pointer.parts - entry = self.config_db.get_entry(parts[0], parts[1]) - oldtree = entry - for part in parts[1::-1]: - oldtree = { part: oldtree } - op.apply(oldtree) - self.config_db.set_entry(parts[0], parts[1], entry) - - def apply(self, change: JsonChange): - patch = change.patch - - self.previous_table_name = None - # JsonChange is a list of PatchOperation - for op in patch._ops: - parts = op.pointer.parts - nparts = len(parts) - value = op.operation.get('value', None) - - - tree = value - for part in parts[::-1]: - tree = { part: tree } - if isinstance(op, AddOperation): - self.config_db.mod_config(tree) - if nparts == 2: - # Added a key in a table - self.on_table_operation_lazy(parts[0]) - elif isinstance(op, RemoveOperation): - if nparts == 1: - # Delete a table - self.config_db.delete_table(parts[0]) - self.on_table_operation_lazy(parts[0]) - elif nparts == 2: - # Delete a key in a table - self.config_db.set_entry(parts[0], parts[1], None) - self.on_table_operation_lazy(parts[0]) - else: - self._apply_entry(op) - elif isinstance(op, ReplaceOperation): - if nparts <= 2: - raise NotImplementedError("ConfigDb does not support replace a table name or key name") - else: - self._apply_entry(op) - else: - raise NotImplementedError("Not supported PatchOperation type") - - class ConfigFormat(Enum): CONFIGDB = 1 SONICYANG = 2 From 75c31aaffe12a208fd0fb12faae3d8ac9553ef8b Mon Sep 17 00:00:00 2001 From: Qi Luo Date: Thu, 16 Sep 2021 02:39:25 +0000 Subject: [PATCH 5/5] Remove unused import --- generic_config_updater/generic_updater.py | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/generic_config_updater/generic_updater.py b/generic_config_updater/generic_updater.py index cce9d0d36d..e2a8542ec1 100644 --- a/generic_config_updater/generic_updater.py +++ b/generic_config_updater/generic_updater.py @@ -1,11 +1,8 @@ import json import os -import subprocess from enum import Enum -from jsonpatch import PatchOperation, AddOperation, RemoveOperation, ReplaceOperation -from swsscommon.swsscommon import ConfigDBConnector from .gu_common import GenericConfigUpdaterError, ConfigWrapper, \ - DryRunConfigWrapper, PatchWrapper, JsonChange + DryRunConfigWrapper, PatchWrapper from .patch_sorter import PatchSorter from .change_applier import ChangeApplier