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

[config][generic-update] Adding apply-patch, rollback, checkpoints commands #1536

Merged
merged 32 commits into from
Apr 26, 2021
Merged
Show file tree
Hide file tree
Changes from 30 commits
Commits
Show all changes
32 commits
Select commit Hold shift + click to select a range
5eaf09e
sekelton
ghooo Mar 29, 2021
8305b67
impl
ghooo Mar 30, 2021
0175885
adding unit-tests and replying to comments
ghooo Apr 5, 2021
f251006
restructure files
ghooo Apr 6, 2021
93e0347
fixing compilation errors/warnings
ghooo Apr 6, 2021
2852f9b
fixing lgtm warnings
ghooo Apr 6, 2021
4399fde
fixing compliation errors according to recent yang models
ghooo Apr 6, 2021
b9c81b8
using assertCountEqual instead of assertListEqual
ghooo Apr 6, 2021
e72407b
catching/raising non-general exceptions
ghooo Apr 14, 2021
30c1265
patch-applier to expect input of ConfigDb patch instead SonicYang patch
ghooo Apr 14, 2021
9aa31df
explaining why sonic_cfggen is loaded as a source
ghooo Apr 14, 2021
0cca529
trying to include sonic_cfggen as a requirement
ghooo Apr 14, 2021
b7ed354
back to source loading sonic-cfggen
ghooo Apr 14, 2021
48448af
adding cli commands
ghooo Apr 16, 2021
a3ca658
removing dry-run option from checkpoint, delete-checkpoint, list-chec…
ghooo Apr 16, 2021
e0d6d43
minor fix
ghooo Apr 16, 2021
99b6796
minor fix
ghooo Apr 16, 2021
d42e16f
adding sonic-cfggen to install_requires
ghooo Apr 16, 2021
ecd6519
fix generic_config_updater path in test file
ghooo Apr 16, 2021
f504980
trying sonic_cfggen instead of sonic-cfggen
ghooo Apr 16, 2021
9e0519d
removing sonic-cfggen from install_requires
ghooo Apr 16, 2021
757d306
make generic_config_updater a pkg
ghooo Apr 16, 2021
038ec67
use generic_config_updater pkg in uts
ghooo Apr 16, 2021
cf6be42
removing load_source of sonic-cfggen
ghooo Apr 17, 2021
4a2a964
unifying exceptions
ghooo Apr 17, 2021
fc026fb
moving common functionalities to common file
ghooo Apr 17, 2021
90f6731
small fixes
ghooo Apr 17, 2021
34ee074
renaming table without yant to TABLE_WITHOUT_YANG
ghooo Apr 18, 2021
5288345
validate running config before taking a checkpoint
ghooo Apr 18, 2021
e5d142c
fixing azure pipeline
ghooo Apr 20, 2021
7ebf3ae
making sonic-config-engine required install_required
ghooo Apr 21, 2021
1a3e37e
err to stderr and format upper case
ghooo Apr 24, 2021
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
6 changes: 5 additions & 1 deletion .azure-pipelines/docker-sonic-vs/Dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -4,4 +4,8 @@ ARG docker_container_name

ADD ["wheels", "/wheels"]

RUN pip3 install --no-deps --force-reinstall /wheels/sonic_utilities-1.2-py3-none-any.whl
# Uninstalls only sonic-utilities and does not impact its dependencies
Copy link
Contributor Author

@ghooo ghooo Apr 20, 2021

Choose a reason for hiding this comment

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

docker-sonic-vs is downloaded then the new sonic-utilities package is installed with most recent modifications. Problem happens if the new package has new external libs or one of the libs is updated as using the command pip3 install with --deps just leave the deps unchanged.

I tried different ideas:
pip3 install /wheels/sonic_utilities-1.2-py3-none-any.whl
does not do anything because the package is already installed in the docker-sonic-vs

pip3 install --force-reinstall /wheels/sonic_utilities-1.2-py3-none-any.whl
does not work because it also forces the deps to be updated but some of the deps such as sonic-py-common are not available in public pip repo and are installed with the sonic image in docker-sonic-vs

pip3 install --update /wheels/sonic_utilities-1.2-py3-none-any.whl
does not update sonic-utilities package and only updates its dependencies

The proposed soln achieved the following:

  • Force install sonic-utilities package
  • Adding missing dependencies
  • Upgrading out-dated dependencies
  • Leaving dependencies provided by the sonic-build process unchanged.

RUN pip3 uninstall -y sonic-utilities

# Installs sonic-utilities, adds missing dependencies, upgrades out-dated depndencies
RUN pip3 install /wheels/sonic_utilities-1.2-py3-none-any.whl
131 changes: 128 additions & 3 deletions config/main.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
import click
import ipaddress
import json
import jsonpatch
import netaddr
import netifaces
import os
Expand All @@ -11,6 +12,7 @@
import sys
import time

from generic_config_updater.generic_updater import GenericUpdater, ConfigFormat
from socket import AF_INET, AF_INET6
from minigraph import parse_device_desc_xml
from portconfig import get_child_ports
Expand Down Expand Up @@ -826,7 +828,7 @@ def cache_arp_entries():
if filter_err:
click.echo("Could not filter FDB entries prior to reloading")
success = False

# If we are able to successfully cache ARP table info, signal SWSS to restore from our cache
# by creating /host/config-reload/needs-restore
if success:
Expand Down Expand Up @@ -986,6 +988,129 @@ def load(filename, yes):
log.log_info("'load' executing...")
clicommon.run_command(command, display_cmd=True)

@config.command('apply-patch')
@click.argument('patch-file-path', type=str, required=True)
@click.option('-f', '--format', type=click.Choice([e.name.lower() for e in ConfigFormat]),
Copy link
Contributor

Choose a reason for hiding this comment

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

format

This option is not in design doc.

Copy link
Contributor Author

@ghooo ghooo Apr 24, 2021

Choose a reason for hiding this comment

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

This option is added to make the code ready to use SonicYang format in the future once our internal services start interacting with SONiC boxes using SonicYang. Do you suggest we add it to the design document, remove it from here, or leave it as is?

Copy link
Contributor

Choose a reason for hiding this comment

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

Better to update the design doc.

default=ConfigFormat.CONFIGDB.name.lower(),
Copy link
Contributor

@qiluo-msft qiluo-msft Apr 23, 2021

Choose a reason for hiding this comment

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

.lower()

Just use the upcases? #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.

updated locally

Copy link
Contributor Author

Choose a reason for hiding this comment

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

updated

help='format of config of the patch is either ConfigDb(ABNF) or SonicYang')
@click.option('-d', '--dry-run', is_flag=True, default=False, help='test out the command without affecting config state')
@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, 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)
format or SonicYang format.

<patch-file-path>: Path to the patch file on the file-system."""
try:
with open(patch_file_path, 'r') as fh:
text = fh.read()
patch_as_json = json.loads(text)
patch = jsonpatch.JsonPatch(patch_as_json)

config_format = ConfigFormat[format.upper()]

GenericUpdater().apply_patch(patch, config_format, verbose, dry_run)

click.secho("Patch applied successfully.", fg="cyan", underline=True)
except Exception as ex:
click.secho("Failed to apply patch", fg="red", underline=True)
Copy link
Contributor

@qiluo-msft qiluo-msft Apr 23, 2021

Choose a reason for hiding this comment

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

secho

secho to stderr if this is a critical error. #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.

updated locally

Copy link
Contributor Author

Choose a reason for hiding this comment

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

updated

ctx.fail(ex)

@config.command()
@click.argument('target-file-path', type=str, required=True)
@click.option('-f', '--format', type=click.Choice([e.name.lower() for e in ConfigFormat]),
default=ConfigFormat.CONFIGDB.name.lower(),
help='format of target config is either ConfigDb(ABNF) or SonicYang')
@click.option('-d', '--dry-run', is_flag=True, default=False, help='test out the command without affecting config state')
@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, 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."""
try:
with open(target_file_path, 'r') as fh:
target_config_as_text = fh.read()
target_config = json.loads(target_config_as_text)

config_format = ConfigFormat[format.upper()]

GenericUpdater().replace(target_config, config_format, verbose, dry_run)

click.secho("Config replaced successfully.", fg="cyan", underline=True)
except Exception as ex:
click.secho("Failed to replace config", fg="red", underline=True)
ctx.fail(ex)

@config.command()
@click.argument('checkpoint-name', type=str, required=True)
@click.option('-d', '--dry-run', is_flag=True, default=False, help='test out the command without affecting config state')
@click.option('-v', '--verbose', is_flag=True, default=False, help='print additional details of what the operation is doing')
@click.pass_context
def rollback(ctx, checkpoint_name, dry_run, verbose):
"""Rollback the whole config to the specified checkpoint. The config is rolled back with minimum disruption e.g.
if ACL config is different between current and checkpoint config only ACL config is updated, and other config/services
such as DHCP will not be affected.

<checkpoint-name>: The checkpoint name, use `config list-checkpoints` command to see available checkpoints."""
try:
GenericUpdater().rollback(checkpoint_name, verbose, dry_run)

click.secho("Config rolled back successfully.", fg="cyan", underline=True)
except Exception as ex:
click.secho("Failed to rollback config", fg="red", underline=True)
ctx.fail(ex)

@config.command()
@click.argument('checkpoint-name', type=str, required=True)
@click.option('-v', '--verbose', is_flag=True, default=False, help='print additional details of what the operation is doing')
@click.pass_context
def checkpoint(ctx, checkpoint_name, verbose):
"""Take a checkpoint of the whole current config with the specified checkpoint name.

<checkpoint-name>: The checkpoint name, use `config list-checkpoints` command to see available checkpoints."""
try:
GenericUpdater().checkpoint(checkpoint_name, verbose)

click.secho("Checkpoint created successfully.", fg="cyan", underline=True)
except Exception as ex:
click.secho("Failed to create a config checkpoint", fg="red", underline=True)
ctx.fail(ex)

@config.command('delete-checkpoint')
@click.argument('checkpoint-name', type=str, required=True)
@click.option('-v', '--verbose', is_flag=True, default=False, help='print additional details of what the operation is doing')
@click.pass_context
def delete_checkpoint(ctx, checkpoint_name, verbose):
"""Delete a checkpoint with the specified checkpoint name.

<checkpoint-name>: The checkpoint name, use `config list-checkpoints` command to see available checkpoints."""
try:
GenericUpdater().delete_checkpoint(checkpoint_name, verbose)

click.secho("Checkpoint deleted successfully.", fg="cyan", underline=True)
except Exception as ex:
click.secho("Failed to delete config checkpoint", fg="red", underline=True)
ctx.fail(ex)

@config.command('list-checkpoints')
@click.option('-v', '--verbose', is_flag=True, default=False, help='print additional details of what the operation is doing')
@click.pass_context
def list_checkpoints(ctx, verbose):
"""List the config checkpoints available."""
try:
checkpoints_list = GenericUpdater().list_checkpoints(verbose)
formatted_output = json.dumps(checkpoints_list, indent=4)
click.echo(formatted_output)
except Exception as ex:
click.secho("Failed to list config checkpoints", fg="red", underline=True)
ctx.fail(ex)

@config.command()
@click.option('-y', '--yes', is_flag=True)
Expand Down Expand Up @@ -2580,8 +2705,8 @@ def add(ctx, interface_name, ip_addr, gw):
if interface_name is None:
ctx.fail("'interface_name' is None!")

# Add a validation to check this interface is not a member in vlan before
# changing it to a router port
# Add a validation to check this interface is not a member in vlan before
# changing it to a router port
vlan_member_table = config_db.get_table('VLAN_MEMBER')
if (interface_is_in_vlan(vlan_member_table, interface_name)):
click.echo("Interface {} is a member of vlan\nAborting!".format(interface_name))
Expand Down
Empty file.
Loading