From cae40d14c2aaa4e5d2a31fd2ea8cd562e61a3fb2 Mon Sep 17 00:00:00 2001 From: Don Penney Date: Thu, 14 Jun 2018 16:02:46 -0400 Subject: [PATCH] Fix variable collision in patch apply/remove semantic checks A Day One bug in the patching apply/remove semantic checks around the required patches means the specified list of patches can be truncated in certain scenarios. A local variable used in a for-loop iteration has the same name as another local variable in a slightly broader scope, causing the broader scoped variable to be overwritten in certain scenarios. The APIs have semantics that are intended to do the following checks: - From every patch in the arg list, build up a list of required patches. - if a required patch is in the arg list, it doesn't get added to the "required patches" dict, since it's being operated on - Loop over the required patches dict, checking the states - This is where the bug exists, as the loop inadvertently overwrites the patch list So the following scenarios will work fine and not have an issue: - No patches are applied on the system, all patches being applied in one command. The required patches dict is empty, because all patches are in the argument list, so the patch list does not get corrupted. - Patches are applied one by one on the system. The patch list gets overwritten, but it's overwritten with the same single patch. The following scenario runs into problems: - Some patches applied on the system. Two or more patches, at least one of which has dependencies, and dependency lists are not complete lists of all previous patches. The patch list is corrupted with a partial list of patches, and not all specified patches end up getting applied. Story: 2002859 Task: 22808 Change-Id: I09ce3af9ede39f163a3ddd1fc64fc154716d1d60 Signed-off-by: Don Penney Signed-off-by: Jack Ding --- cgcs-patch/cgcs-patch/cgcs_patch/patch_controller.py | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/cgcs-patch/cgcs-patch/cgcs_patch/patch_controller.py b/cgcs-patch/cgcs-patch/cgcs_patch/patch_controller.py index 66775c02..1541cd03 100644 --- a/cgcs-patch/cgcs-patch/cgcs_patch/patch_controller.py +++ b/cgcs-patch/cgcs-patch/cgcs_patch/patch_controller.py @@ -1014,10 +1014,10 @@ class PatchController(PatchService): # Now verify the state of the required patches req_verification = True - for req_patch, patch_list in required_patches.iteritems(): + for req_patch, iter_patch_list in required_patches.iteritems(): if req_patch not in self.patch_data.metadata \ or self.patch_data.metadata[req_patch]["repostate"] == constants.AVAILABLE: - msg = "%s is required by: %s" % (req_patch, ", ".join(sorted(patch_list))) + msg = "%s is required by: %s" % (req_patch, ", ".join(sorted(iter_patch_list))) msg_error += msg + "\n" LOG.info(msg) req_verification = False @@ -1202,8 +1202,8 @@ class PatchController(PatchService): required_patches[req_patch].append(patch_iter) if len(required_patches) > 0: - for req_patch, patch_list in required_patches.iteritems(): - msg = "%s is required by: %s" % (req_patch, ", ".join(sorted(patch_list))) + for req_patch, iter_patch_list in required_patches.iteritems(): + msg = "%s is required by: %s" % (req_patch, ", ".join(sorted(iter_patch_list))) msg_error += msg + "\n" LOG.info(msg) @@ -1549,8 +1549,8 @@ class PatchController(PatchService): for patch_id in patch_ids: if patch_id in required_patches: - patch_list = required_patches[patch_id] - msg_info += "%s is required by: %s\n" % (patch_id, ", ".join(sorted(patch_list))) + iter_patch_list = required_patches[patch_id] + msg_info += "%s is required by: %s\n" % (patch_id, ", ".join(sorted(iter_patch_list))) else: msg_info += "%s is not required by any patches.\n" % patch_id