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

[GCU] Add RemoveCreateOnlyDependency Validator/Generator #2500

Merged
merged 3 commits into from
Dec 2, 2022
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
3 changes: 1 addition & 2 deletions generic_config_updater/gu_common.py
Original file line number Diff line number Diff line change
Expand Up @@ -54,8 +54,7 @@ def get_config_db_as_json(self):
return json.loads(text)

def _get_config_db_as_text(self):
# TODO: Getting configs from CLI is very slow, need to get it from sonic-cffgen directly
cmd = "show runningconfiguration all"
cmd = "sonic-cfggen -d --print-data"
wen587 marked this conversation as resolved.
Show resolved Hide resolved
result = subprocess.Popen(cmd, shell=True, text=True, stdout=subprocess.PIPE, stderr=subprocess.PIPE)
text, err = result.communicate()
return_code = result.returncode
Expand Down
108 changes: 107 additions & 1 deletion generic_config_updater/patch_sorter.py
Original file line number Diff line number Diff line change
Expand Up @@ -544,6 +544,66 @@ def _get_default_value_from_settings(self, parent_path, field_name):

return None

class LaneReplacementMoveValidator:
wen587 marked this conversation as resolved.
Show resolved Hide resolved
def __init__(self, path_addressing):
self.path_addressing = path_addressing

def validate(self, move, diff):
current_config = diff.current_config
target_config = diff.target_config # Final config after applying whole patch

if "PORT" not in current_config:
return True

current_ports = current_config["PORT"]
if not current_ports:
return True

if "PORT" not in target_config:
return True

target_ports = target_config["PORT"]
if not target_ports:
return True

simulated_config = move.apply(current_config) # Config after applying just this move

for port_name in current_ports:
if port_name not in target_ports:
continue

if not self._validate_port(port_name, current_config, target_config, simulated_config):
return False

return True

def _validate_port(self, port_name, current_config, target_config, simulated_config):
current_lanes = self._get_lanes(current_config, port_name)
target_lanes = self._get_lanes(target_config, port_name)

if current_lanes == target_lanes:
return True

simulated_port = self.path_addressing.get_from_path(simulated_config, f"/PORT/{port_name}")

if simulated_port is None:
return True

current_admin_status = self.path_addressing.get_from_path(current_config, f"/PORT/{port_name}/admin_status")
simulated_admin_status = self.path_addressing.get_from_path(simulated_config, f"/PORT/{port_name}/admin_status")
if current_admin_status != simulated_admin_status and current_admin_status != "up":
return False
wen587 marked this conversation as resolved.
Show resolved Hide resolved

port_path = f"/PORT/{port_name}"
for ref_path in self.path_addressing.find_ref_paths(port_path, simulated_config):
if not self.path_addressing.has_path(current_config, ref_path):
return False

return True

def _get_lanes(self, config, port_name):
return config["PORT"][port_name].get("lanes", None)

class DeleteWholeConfigMoveValidator:
"""
A class to validate not deleting whole config as it is not supported by JsonPatch lib.
Expand Down Expand Up @@ -921,6 +981,8 @@ def validate(self, move, diff):
for required_path, required_value in data[path]:
current_value = self.identifier.get_value_or_default(current_config, required_path)
simulated_value = self.identifier.get_value_or_default(simulated_config, required_path)
if simulated_value is None: # Simulated config does not have this value at all.
continue
if current_value != simulated_value and simulated_value != required_value:
return False

Expand Down Expand Up @@ -1000,6 +1062,46 @@ def generate(self, diff):
for move in single_run_generator.generate():
yield move

class LaneReplacementMoveGenerator:
def __init__(self, path_addressing):
self.path_addressing = path_addressing

def generate(self, diff):
current_config = diff.current_config
target_config = diff.target_config # Final config after applying whole patch

if "PORT" not in current_config:
return

current_ports = current_config["PORT"]
if not current_ports:
return

if "PORT" not in target_config:
return

target_ports = target_config["PORT"]
if not target_ports:
return

for port_name in current_ports:
if port_name not in target_ports:
continue

current_lanes = self._get_lanes(current_config, port_name)
target_lanes = self._get_lanes(target_config, port_name)

if current_lanes == target_lanes:
continue

port_path = f"/PORT/{port_name}"

for ref_path in self.path_addressing.find_ref_paths(port_path, current_config):
yield JsonMove(diff, OperationType.REMOVE, self.path_addressing.get_path_tokens(ref_path))

def _get_lanes(self, config, port_name):
return config["PORT"][port_name].get("lanes", None)

class SingleRunLowLevelMoveGenerator:
"""
A class that can only run once to assist LowLevelMoveGenerator with generating the moves.
Expand Down Expand Up @@ -1271,6 +1373,8 @@ def extend(self, move, diff):
for required_path, required_value in data[path]:
current_value = self.identifier.get_value_or_default(current_config, required_path)
simulated_value = self.identifier.get_value_or_default(simulated_config, required_path)
if simulated_value is None: # Simulated config does not have this value at all.
continue
if current_value != simulated_value and simulated_value != required_value:
flip_path_value_tuples.add((required_path, required_value))

Expand Down Expand Up @@ -1473,7 +1577,8 @@ def __init__(self, operation_wrapper, config_wrapper, path_addressing):
self.path_addressing = path_addressing

def create(self, algorithm=Algorithm.DFS):
move_generators = [LowLevelMoveGenerator(self.path_addressing)]
move_generators = [LaneReplacementMoveGenerator(self.path_addressing),
LowLevelMoveGenerator(self.path_addressing)]
# TODO: Enable TableLevelMoveGenerator once it is confirmed whole table can be updated at the same time
move_non_extendable_generators = [KeyLevelMoveGenerator()]
move_extenders = [RequiredValueMoveExtender(self.path_addressing, self.operation_wrapper),
Expand All @@ -1485,6 +1590,7 @@ def create(self, algorithm=Algorithm.DFS):
NoDependencyMoveValidator(self.path_addressing, self.config_wrapper),
CreateOnlyMoveValidator(self.path_addressing),
RequiredValueMoveValidator(self.path_addressing),
LaneReplacementMoveValidator(self.path_addressing),
NoEmptyTableMoveValidator(self.path_addressing)]

move_wrapper = MoveWrapper(move_generators, move_non_extendable_generators, move_extenders, move_validators)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,23 @@
"lanes": "41,42,43,44",
"pfc_asym": "off",
"speed": "40000"
},
"Ethernet28": {
"alias": "fortyGigE0/28",
"description": "Servers5:eth0",
"index": "6",
"lanes": "53,54,55,56",
"pfc_asym": "off",
"speed": "40000"
},
"Ethernet32": {
"alias": "fortyGigE0/32",
"description": "Servers6:eth0",
"index": "7",
"lanes": "57,58,59,60",
"mtu": "9100",
"pfc_asym": "off",
"speed": "40000"
}
},
"BUFFER_PG": {
Expand All @@ -44,6 +61,9 @@
},
"Ethernet12|0": {
"profile": "ingress_lossy_profile"
},
"Ethernet28|0": {
"profile": "ingress_lossy_profile"
}
}
}
Loading