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

[generic_config_updater] Logging #1864

Merged
merged 3 commits into from
Oct 14, 2021
Merged
Show file tree
Hide file tree
Changes from all 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
28 changes: 22 additions & 6 deletions generic_config_updater/generic_updater.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
import os
from enum import Enum
from .gu_common import GenericConfigUpdaterError, ConfigWrapper, \
DryRunConfigWrapper, PatchWrapper
DryRunConfigWrapper, PatchWrapper, genericUpdaterLogging
from .patch_sorter import PatchSorter

CHECKPOINTS_DIR = "/etc/sonic/checkpoints"
Expand Down Expand Up @@ -32,38 +32,58 @@ def __init__(self,
changeapplier=None,
config_wrapper=None,
patch_wrapper=None):
self.logger = genericUpdaterLogging.get_logger(title="Patch Applier")
self.config_wrapper = config_wrapper if config_wrapper is not None else ConfigWrapper()
self.patch_wrapper = patch_wrapper if patch_wrapper is not None else PatchWrapper()
self.patchsorter = patchsorter if patchsorter is not None else PatchSorter(self.config_wrapper, self.patch_wrapper)
self.changeapplier = changeapplier if changeapplier is not None else ChangeApplier()

def apply(self, patch):
print_to_console=True
self.logger.log_notice("Patch application starting.", print_to_console)
self.logger.log_notice(f"Patch: {patch}", print_to_console)

# validate patch is only updating tables with yang models
self.logger.log_notice("Validating patch is not making changes to tables without YANG models.", print_to_console)
if not(self.patch_wrapper.validate_config_db_patch_has_yang_models(patch)):
raise ValueError(f"Given patch is not valid because it has changes to tables without YANG models")

# Get old config
self.logger.log_notice("Getting current config db.", print_to_console)
old_config = self.config_wrapper.get_config_db_as_json()

# Generate target config
self.logger.log_notice("Simulating the target full config after applying the patch.", print_to_console)
target_config = self.patch_wrapper.simulate_patch(patch, old_config)

# Validate target config
self.logger.log_notice("Validating target config according to YANG models.", print_to_console)
if not(self.config_wrapper.validate_config_db_config(target_config)):
raise ValueError(f"Given patch is not valid because it will result in an invalid config")

# Generate list of changes to apply
self.logger.log_notice("Sorting patch updates.", print_to_console)
changes = self.patchsorter.sort(patch)
changes_len = len(changes)
self.logger.log_notice(f"The patch was sorted into {changes_len} " \
f"change{'s' if changes_len != 1 else ''}{':' if changes_len > 0 else '.'}",
print_to_console)
for change in changes:
self.logger.log_notice(f" * {change}", print_to_console)

# Apply changes in order
self.logger.log_notice("Applying changes in order.", print_to_console)
for change in changes:
self.changeapplier.apply(change)

# Validate config updated successfully
self.logger.log_notice("Verifying patch updates are reflected on ConfigDB.", print_to_console)
new_config = self.config_wrapper.get_config_db_as_json()
if not(self.patch_wrapper.verify_same_json(target_config, new_config)):
raise GenericConfigUpdaterError(f"After applying patch to config, there are still some parts not updated")

self.logger.log_notice("Patch application completed.", print_to_console)

class ConfigReplacer:
def __init__(self, patch_applier=None, config_wrapper=None, patch_wrapper=None):
self.patch_applier = patch_applier if patch_applier is not None else PatchApplier()
Expand Down Expand Up @@ -293,11 +313,7 @@ def create_config_rollbacker(self, verbose, dry_run=False):
return config_rollbacker

def init_verbose_logging(self, verbose):
# TODO: implement verbose logging
# Usually logs have levels such as: error, warning, info, debug.
# By default all log levels should show up to the user, except debug.
# By allowing verbose logging, debug msgs will also be shown to the user.
pass
genericUpdaterLogging.set_verbose(verbose)

def get_config_wrapper(self, dry_run):
if dry_run:
Expand Down
25 changes: 25 additions & 0 deletions generic_config_updater/gu_common.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,11 @@
import yang as ly
import copy
import re
from sonic_py_common import logger
from enum import Enum

YANG_DIR = "/usr/local/yang-models"
SYSLOG_IDENTIFIER = "GenericConfigUpdater"
Copy link
Contributor

@qiluo-msft qiluo-msft Oct 12, 2021

Choose a reason for hiding this comment

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

GenericConfigUpdater

Is this ident too long? The default will be process name such as config, is it good enough since you have add title to diffentiate. #Closed

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Here is how it look like:

Oct 11 18:15:50.604939 vlab-01 INFO GenericConfigUpdater: Patch Applier: Getting current config db.
Oct 11 18:15:51.494117 vlab-01 INFO GenericConfigUpdater: Patch Applier: Simulating the target full config after applying the patch.
Oct 11 18:15:51.502449 vlab-01 INFO GenericConfigUpdater: Patch Applier: Validating target config according to YANG models.

I think it is OK, this way I can grep all syslogs associated with GenericConfigUpdater i.e. cat /var/log/syslog | grep GenericConfigUpdater

I think we will add more logs to other parts such config replace, rollback checkpoint and the applier.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Also I don't think we should use config since this library might be used with REST APIs like was requested in some of the earlier meetings with SONiC Community


class GenericConfigUpdaterError(Exception):
pass
Expand Down Expand Up @@ -691,3 +693,26 @@ def _get_model(self, model, name):
return submodel

return None

class TitledLogger(logger.Logger):
def __init__(self, syslog_identifier, title, verbose):
super().__init__(syslog_identifier)
self._title = title
if verbose:
self.set_min_log_priority_debug()

def log(self, priority, msg, also_print_to_console=False):
combined_msg = f"{self._title}: {msg}"
super().log(priority, combined_msg, also_print_to_console)

class GenericUpdaterLogging:
def __init__(self):
self.set_verbose(False)

def set_verbose(self, verbose):
self._verbose = verbose

def get_logger(self, title):
return TitledLogger(SYSLOG_IDENTIFIER, title, self._verbose)

genericUpdaterLogging = GenericUpdaterLogging()