From 4fc1d15ad2aee0381e0585072dfb66de86a1e0a9 Mon Sep 17 00:00:00 2001 From: Al Bailey Date: Thu, 21 Apr 2022 20:53:48 +0000 Subject: [PATCH] Remove rpm imports from debian patching code Remove the rpm imports from debian patching code. The dnf imports will be removed in a later commit. The patch code had methods, variables and subprocesses that reference 'rpm'. Most of these have been removed or renamed. The remaining 'rpm' references will be removed as functionality related to those calls is implemented for debian ostree. The code is being converted to ostree, so these changes are not currently runnable, nor were the rpm calls on debian. The createrepo calls are also removed, ostree equivalent calls may (or may not) be added in a followup commit. The subprocess exceptions are made more generic, as any uncaight exception in API handling could make the patch controller non-responsive. Robustness improvements may be investigated in a followup commit. Test Plan: Verify build/install/bootstrap/unlock on Debian. Verify sw-patch upload /delete do not report failures using a signed patch. (Note: used an rpm patch for centos) Story: 2009969 Task: 45192 Signed-off-by: Al Bailey Change-Id: I0590950868805b89dd1e302397d83f1a6f5e244a --- sw-patch/bin/sw-patch-controller-init.sh | 20 +- sw-patch/cgcs-patch/cgcs_patch/exceptions.py | 4 +- .../cgcs-patch/cgcs_patch/patch_controller.py | 205 ++++++++---------- .../cgcs-patch/cgcs_patch/patch_functions.py | 35 +-- .../cgcs_patch/tests/test_basics.py | 8 +- .../cgcs_patch/tests/test_patch_agent.py | 5 +- .../cgcs_patch/tests/test_patch_controller.py | 12 +- sw-patch/cgcs-patch/pylint.rc | 2 +- 8 files changed, 126 insertions(+), 165 deletions(-) diff --git a/sw-patch/bin/sw-patch-controller-init.sh b/sw-patch/bin/sw-patch-controller-init.sh index c9190c9c..3650833b 100644 --- a/sw-patch/bin/sw-patch-controller-init.sh +++ b/sw-patch/bin/sw-patch-controller-init.sh @@ -39,28 +39,14 @@ function LOG_TO_FILE { echo "`date "+%FT%T.%3N"`: $NAME: $*" >> $logfile } -function create_groups { - if [ -f $GROUPS_FILE ]; then - return 0 - fi - - cat >$GROUPS_FILE < - - -EOF -} - function do_setup { # Does the repo exist? if [ ! -d $REPO_DIR ]; then - LOG "Creating repo" + LOG "Creating repo. UNDER CONSTRUCTION for OSTREE" mkdir -p $REPO_DIR - # Setup the groups file - create_groups - - createrepo -g $GROUPS_FILE $REPO_DIR >> $logfile 2>&1 + # The original Centos code would create the groups and call createrepo + # todo(jcasteli): determine if the ostree code needs a setup also fi if [ ! -d $PATCHING_DIR ]; then diff --git a/sw-patch/cgcs-patch/cgcs_patch/exceptions.py b/sw-patch/cgcs-patch/cgcs_patch/exceptions.py index 6c27c253..d6a76fa5 100644 --- a/sw-patch/cgcs-patch/cgcs_patch/exceptions.py +++ b/sw-patch/cgcs-patch/cgcs_patch/exceptions.py @@ -22,8 +22,8 @@ class MetadataFail(PatchError): pass -class RpmFail(PatchError): - """RPM error.""" +class ContentFail(PatchError): + """Content handling error.""" pass diff --git a/sw-patch/cgcs-patch/cgcs_patch/patch_controller.py b/sw-patch/cgcs-patch/cgcs_patch/patch_controller.py index e2153329..71f86019 100644 --- a/sw-patch/cgcs-patch/cgcs_patch/patch_controller.py +++ b/sw-patch/cgcs-patch/cgcs_patch/patch_controller.py @@ -4,23 +4,22 @@ Copyright (c) 2014-2019 Wind River Systems, Inc. SPDX-License-Identifier: Apache-2.0 """ +import gc +import json +import os +import select import shutil +import six +from six.moves import configparser +import socket +import subprocess import tempfile import threading import time -import socket -import json -import select -import subprocess -import six -from six.moves import configparser -import rpm -import os -import gc - -from cgcs_patch.patch_functions import parse_pkgver - from wsgiref import simple_server + +from oslo_config import cfg as oslo_cfg + from cgcs_patch.api import app from cgcs_patch.authapi import app as auth_app from cgcs_patch.patch_functions import configure_logging @@ -28,15 +27,17 @@ from cgcs_patch.patch_functions import BasePackageData from cgcs_patch.patch_functions import avail_dir from cgcs_patch.patch_functions import applied_dir from cgcs_patch.patch_functions import committed_dir +from cgcs_patch.patch_functions import contentCompare from cgcs_patch.patch_functions import PatchFile from cgcs_patch.patch_functions import parse_rpm_filename +from cgcs_patch.patch_functions import parse_pkgver from cgcs_patch.patch_functions import package_dir from cgcs_patch.patch_functions import repo_dir from cgcs_patch.patch_functions import semantics_dir from cgcs_patch.patch_functions import SW_VERSION from cgcs_patch.patch_functions import root_package_dir from cgcs_patch.exceptions import MetadataFail -from cgcs_patch.exceptions import RpmFail +from cgcs_patch.exceptions import ContentFail from cgcs_patch.exceptions import SemanticFail from cgcs_patch.exceptions import PatchError from cgcs_patch.exceptions import PatchFail @@ -52,8 +53,7 @@ from cgcs_patch.base import PatchService import cgcs_patch.config as cfg import cgcs_patch.utils as utils -# noinspection PyUnresolvedReferences -from oslo_config import cfg as oslo_cfg + import cgcs_patch.messages as messages import cgcs_patch.constants as constants @@ -780,7 +780,7 @@ class PatchController(PatchService): continue # Is the installed pkg higher or lower version? - # The rpm.labelCompare takes version broken into 3 components + # OSTREE: this comparison will be different than for RPMs installed_ver = self.hosts[ip].installed[pkg].split('@')[0] if ":" in installed_ver: # Ignore epoch @@ -791,8 +791,8 @@ class PatchController(PatchService): # Ignore epoch patch_ver = patch_ver.split(':')[1] - rc = rpm.labelCompare(parse_pkgver(installed_ver), - parse_pkgver(patch_ver)) + rc = contentCompare(parse_pkgver(installed_ver), + parse_pkgver(patch_ver)) if self.patch_data.metadata[patch_id]["repostate"] == constants.AVAILABLE: # The RPM is not expected to be installed. @@ -869,35 +869,21 @@ class PatchController(PatchService): self.hosts_lock.release() - def get_store_filename(self, patch_sw_version, rpmname): - rpm_dir = package_dir[patch_sw_version] - rpmfile = "%s/%s" % (rpm_dir, rpmname) - return rpmfile + def get_store_filename(self, patch_sw_version, contentname): + """Returns the path of a content file from the store""" + content_dir = package_dir[patch_sw_version] + contentfile = "%s/%s" % (content_dir, contentname) + return contentfile - def get_repo_filename(self, patch_sw_version, rpmname): - rpmfile = self.get_store_filename(patch_sw_version, rpmname) - if not os.path.isfile(rpmfile): - msg = "Could not find rpm: %s" % rpmfile + def get_repo_filename(self, patch_sw_version, contentname): + contentfile = self.get_store_filename(patch_sw_version, contentname) + if not os.path.isfile(contentfile): + msg = "Could not find content: %s" % contentfile LOG.error(msg) return None - repo_filename = None - - try: - # Get the architecture from the RPM - pkgarch = subprocess.check_output(["rpm", - "-qp", - "--queryformat", - "%{ARCH}", - "--nosignature", - rpmfile]) - - repo_filename = "%s/Packages/%s/%s" % (repo_dir[patch_sw_version], pkgarch, rpmname) - except subprocess.CalledProcessError: - msg = "RPM query failed for %s" % rpmfile - LOG.exception(msg) - return None - + # OSTREE: need to determine the actual path for the content ie: Content + repo_filename = "%s/Content/%s" % (repo_dir[patch_sw_version], contentname) return repo_filename def run_semantic_check(self, action, patch_list): @@ -1146,49 +1132,52 @@ class PatchController(PatchService): continue # To allow for easy cleanup, we're going to first iterate - # through the rpm list to determine where to copy the file. + # through the content list to determine where to copy the file. # As a second step, we'll go through the list and copy each file. - # If there are problems querying any RPMs, none will be copied. - rpmlist = {} - for rpmname in self.patch_data.contents[patch_id]: - patch_sw_version = self.patch_data.metadata[patch_id]["sw_version"] - - rpmfile = self.get_store_filename(patch_sw_version, rpmname) - if not os.path.isfile(rpmfile): - msg = "Could not find rpm: %s" % rpmfile + # If there are problems querying the content, none will be copied. + content_dict = {} + patch_sw_version = self.patch_data.metadata[patch_id]["sw_version"] + for contentname in self.patch_data.contents[patch_id]: + # the log can be debug or removed altogether once code is working + msg = "OSTREE applying %s " % contentname + LOG.info(msg) + contentfile = self.get_store_filename(patch_sw_version, contentname) + if not os.path.isfile(contentfile): + msg = "Could not find content file: %s" % contentfile LOG.error(msg) - raise RpmFail(msg) + raise ContentFail(msg) - repo_filename = self.get_repo_filename(patch_sw_version, rpmname) + # todo: should 'repo' be replaced with something ostree related + repo_filename = self.get_repo_filename(patch_sw_version, contentname) if repo_filename is None: - msg = "Failed to determine repo path for %s" % rpmfile + msg = "Failed to determine repo path for %s" % contentfile LOG.exception(msg) - raise RpmFail(msg) + raise ContentFail(msg) repo_pkg_dir = os.path.dirname(repo_filename) if not os.path.exists(repo_pkg_dir): os.makedirs(repo_pkg_dir) - rpmlist[rpmfile] = repo_filename + content_dict[contentfile] = repo_filename - # Copy the RPMs. If a failure occurs, clean up copied files. + # Copy the Content. If a failure occurs, clean up copied files. copied = [] - for rpmfile in rpmlist: - LOG.info("Copy %s to %s", rpmfile, rpmlist[rpmfile]) + for contentfile, repo_filename in content_dict.items(): + LOG.info("Copy %s to %s", contentfile, repo_filename) try: - shutil.copy(rpmfile, rpmlist[rpmfile]) - copied.append(rpmlist[rpmfile]) + shutil.copy(contentfile, repo_filename) + copied.append(repo_filename) except IOError: - msg = "Failed to copy %s" % rpmfile + msg = "Failed to copy %s" % contentfile LOG.exception(msg) # Clean up files for filename in copied: LOG.info("Cleaning up %s", filename) os.remove(filename) - raise RpmFail(msg) + raise ContentFail(msg) try: - # Move the metadata to the applied dir + # Move the patching metadata from avail to applied dir shutil.move("%s/%s-metadata.xml" % (avail_dir, patch_id), "%s/%s-metadata.xml" % (applied_dir, patch_id)) @@ -1215,14 +1204,11 @@ class PatchController(PatchService): self.patch_data.gen_groups_xml() for ver, rdir in repo_dir.items(): try: - output = subprocess.check_output(["createrepo", - "--update", - "-g", - "comps.xml", - rdir], - stderr=subprocess.STDOUT) + # todo(jcasteli) determine if ostree change needs additional actions + # old code was calling 'createrepo' for rpms + output = "OSTREE determined a change occurred rdir=%s" % rdir LOG.info("Repo[%s] updated:\n%s", ver, output) - except subprocess.CalledProcessError: + except Exception: msg = "Failed to update the repo for %s" % ver LOG.exception(msg) raise PatchFail(msg) @@ -1350,26 +1336,26 @@ class PatchController(PatchService): repo_changed = True - for rpmname in self.patch_data.contents[patch_id]: - patch_sw_version = self.patch_data.metadata[patch_id]["sw_version"] - rpmfile = self.get_store_filename(patch_sw_version, rpmname) - if not os.path.isfile(rpmfile): - msg = "Could not find rpm: %s" % rpmfile + patch_sw_version = self.patch_data.metadata[patch_id]["sw_version"] + for contentname in self.patch_data.contents[patch_id]: + contentfile = self.get_store_filename(patch_sw_version, contentname) + if not os.path.isfile(contentfile): + msg = "Could not find content: %s" % contentfile LOG.error(msg) - raise RpmFail(msg) + raise ContentFail(msg) - repo_filename = self.get_repo_filename(patch_sw_version, rpmname) + repo_filename = self.get_repo_filename(patch_sw_version, contentname) if repo_filename is None: - msg = "Failed to determine repo path for %s" % rpmfile + msg = "Failed to determine repo path for %s" % contentfile LOG.exception(msg) - raise RpmFail(msg) + raise ContentFail(msg) try: os.remove(repo_filename) except OSError: - msg = "Failed to remove RPM" + msg = "Failed to remove content %s" % repo_filename LOG.exception(msg) - raise RpmFail(msg) + raise ContentFail(msg) try: # Move the metadata to the available dir @@ -1396,14 +1382,11 @@ class PatchController(PatchService): self.patch_data.gen_groups_xml() for ver, rdir in repo_dir.items(): try: - output = subprocess.check_output(["createrepo", - "--update", - "-g", - "comps.xml", - rdir], - stderr=subprocess.STDOUT) + # todo(jcasteli) determine if ostree change needs additional actions + # old code was calling 'createrepo' for rpms + output = "OSTREE determined a change occurred rdir=%s" % rdir LOG.info("Repo[%s] updated:\n%s", ver, output) - except subprocess.CalledProcessError: + except Exception: msg = "Failed to update the repo for %s" % ver LOG.exception(msg) raise PatchFail(msg) @@ -1456,20 +1439,20 @@ class PatchController(PatchService): # Handle operation for patch_id in patch_list: - for rpmname in self.patch_data.contents[patch_id]: - patch_sw_version = self.patch_data.metadata[patch_id]["sw_version"] - rpmfile = self.get_store_filename(patch_sw_version, rpmname) - if not os.path.isfile(rpmfile): + patch_sw_version = self.patch_data.metadata[patch_id]["sw_version"] + for contentname in self.patch_data.contents[patch_id]: + contentfile = self.get_store_filename(patch_sw_version, contentname) + if not os.path.isfile(contentfile): # We're deleting the patch anyway, so the missing file # doesn't really matter continue try: - os.remove(rpmfile) + os.remove(contentfile) except OSError: - msg = "Failed to remove RPM %s" % rpmfile + msg = "Failed to remove Content %s" % contentfile LOG.exception(msg) - raise RpmFail(msg) + raise ContentFail(msg) for action in constants.SEMANTIC_ACTIONS: action_file = os.path.join(semantics_dir, action, patch_id) @@ -1538,14 +1521,10 @@ class PatchController(PatchService): # Create the repo try: - output = subprocess.check_output(["createrepo", - "--update", - "-g", - "comps.xml", - repo_dir[release]], - stderr=subprocess.STDOUT) + # todo(jcasteli) determine if ostree change needs a createrepo equivalent + output = "UNDER CONSTRUCTION for OSTREE" LOG.info("Repo[%s] updated:\n%s", release, output) - except subprocess.CalledProcessError: + except Exception: msg = "Failed to update the repo for %s" % release LOG.exception(msg) @@ -2036,26 +2015,24 @@ class PatchController(PatchService): raise MetadataFail(msg) # Delete the files - for rpmfile in cleanup_files: + for contentfile in cleanup_files: try: - os.remove(rpmfile) + os.remove(contentfile) except OSError: - msg = "Failed to remove: %s" % rpmfile + msg = "Failed to remove: %s" % contentfile LOG.exception(msg) raise MetadataFail(msg) + # OSTREE: this repo update code needs to be re-examined # Update the repo self.patch_data.gen_groups_xml() for ver, rdir in repo_dir.items(): try: - output = subprocess.check_output(["createrepo", - "--update", - "-g", - "comps.xml", - rdir], - stderr=subprocess.STDOUT) + # todo(jcasteli) determine if ostree change needs additional actions + # old code was calling 'createrepo' for rpms + output = "OSTREE determined a change occurred rdir=%s" % rdir LOG.info("Repo[%s] updated:\n%s", ver, output) - except subprocess.CalledProcessError: + except Exception: msg = "Failed to update the repo for %s" % ver LOG.exception(msg) raise PatchFail(msg) diff --git a/sw-patch/cgcs-patch/cgcs_patch/patch_functions.py b/sw-patch/cgcs-patch/cgcs_patch/patch_functions.py index add102e8..2270214e 100644 --- a/sw-patch/cgcs-patch/cgcs_patch/patch_functions.py +++ b/sw-patch/cgcs-patch/cgcs_patch/patch_functions.py @@ -29,7 +29,6 @@ from cgcs_patch.exceptions import PatchValidationFailure from cgcs_patch.exceptions import PatchMismatchFailure import cgcs_patch.constants as constants -import rpm try: # The tsconfig module is only available at runtime @@ -44,6 +43,7 @@ applied_dir = "%s/metadata/applied" % patch_dir committed_dir = "%s/metadata/committed" % patch_dir semantics_dir = "%s/semantics" % patch_dir +# these next 4 variables may need to change to support ostree repo_root_dir = "/var/www/pages/updates" repo_dir = {SW_VERSION: "%s/rel-%s" % (repo_root_dir, SW_VERSION)} @@ -199,6 +199,13 @@ def parse_pkgver(pkgver): return (epoch, version, release) +# OSTREE: this method may be removed or revisited +# based on comparing SHAs and dependencies +def contentCompare(_list1, _list2): + LOG.info("OSTREE: DEV MODE. contentCompare always returns zero") + return 0 + + def get_release_from_patch(patchfile): rel = "" try: @@ -230,46 +237,46 @@ class PackageVersion(object): def __le__(self, other): """ This function is called by comparison operators to compare - two versions. The rpm.labelCompare() function takes two versions, + two versions. The contentCompare() function takes two versions, specified in a list structure, and returns -1, 0, or 1. """ - out = rpm.labelCompare((self.epoch, self.version, self.release), - (other.epoch, other.version, other.release)) + out = contentCompare((self.epoch, self.version, self.release), + (other.epoch, other.version, other.release)) if out == 1: return False return True def __eq__(self, other): - out = rpm.labelCompare((self.epoch, self.version, self.release), - (other.epoch, other.version, other.release)) + out = contentCompare((self.epoch, self.version, self.release), + (other.epoch, other.version, other.release)) if out == 0: return True return False def __ne__(self, other): - out = rpm.labelCompare((self.epoch, self.version, self.release), - (other.epoch, other.version, other.release)) + out = contentCompare((self.epoch, self.version, self.release), + (other.epoch, other.version, other.release)) if out == 0: return False return True def __gt__(self, other): - out = rpm.labelCompare((self.epoch, self.version, self.release), - (other.epoch, other.version, other.release)) + out = contentCompare((self.epoch, self.version, self.release), + (other.epoch, other.version, other.release)) if out == 1: return True return False def __lt__(self, other): - out = rpm.labelCompare((self.epoch, self.version, self.release), - (other.epoch, other.version, other.release)) + out = contentCompare((self.epoch, self.version, self.release), + (other.epoch, other.version, other.release)) if out == -1: return True return False def __ge__(self, other): - out = rpm.labelCompare((self.epoch, self.version, self.release), - (other.epoch, other.version, other.release)) + out = contentCompare((self.epoch, self.version, self.release), + (other.epoch, other.version, other.release)) if out == -1: return False return True diff --git a/sw-patch/cgcs-patch/cgcs_patch/tests/test_basics.py b/sw-patch/cgcs-patch/cgcs_patch/tests/test_basics.py index 0377381e..1a1fc061 100644 --- a/sw-patch/cgcs-patch/cgcs_patch/tests/test_basics.py +++ b/sw-patch/cgcs-patch/cgcs_patch/tests/test_basics.py @@ -4,14 +4,10 @@ # Copyright (c) 2019 Wind River Systems, Inc. # -import mock import os -import sys import testtools -sys.modules['rpm'] = mock.Mock() - -import cgcs_patch.patch_functions # noqa: E402 +from cgcs_patch import patch_functions class CgcsPatchTestCase(testtools.TestCase): @@ -20,6 +16,6 @@ class CgcsPatchTestCase(testtools.TestCase): md5testfile = os.path.join(os.path.dirname(__file__), 'md5test.txt') expected_result = 0x7179a07a8a5c50a3fc9f1971f1ec317f - md5result = cgcs_patch.patch_functions.get_md5(md5testfile) + md5result = patch_functions.get_md5(md5testfile) self.assertEqual(expected_result, md5result) diff --git a/sw-patch/cgcs-patch/cgcs_patch/tests/test_patch_agent.py b/sw-patch/cgcs-patch/cgcs_patch/tests/test_patch_agent.py index 7e30fc50..f28dbb81 100644 --- a/sw-patch/cgcs-patch/cgcs_patch/tests/test_patch_agent.py +++ b/sw-patch/cgcs-patch/cgcs_patch/tests/test_patch_agent.py @@ -9,7 +9,6 @@ import six # pylint: disable=unused-import import sys import testtools -sys.modules['rpm'] = mock.Mock() sys.modules['dnf'] = mock.Mock() sys.modules['dnf.callback'] = mock.Mock() sys.modules['dnf.comps'] = mock.Mock() @@ -26,5 +25,5 @@ import cgcs_patch.patch_agent # noqa: E402 class CgcsPatchAgentTestCase(testtools.TestCase): def test_cgcs_patch_agent_instantiate(self): - # pylint: disable=unused-variable - pc = cgcs_patch.patch_agent.PatchAgent() # noqa: F841 + pc = cgcs_patch.patch_agent.PatchAgent() + self.assertIsNotNone(pc) diff --git a/sw-patch/cgcs-patch/cgcs_patch/tests/test_patch_controller.py b/sw-patch/cgcs-patch/cgcs_patch/tests/test_patch_controller.py index 1db4b686..d5f2a419 100644 --- a/sw-patch/cgcs-patch/cgcs_patch/tests/test_patch_controller.py +++ b/sw-patch/cgcs-patch/cgcs_patch/tests/test_patch_controller.py @@ -5,18 +5,14 @@ # import mock -import six # pylint: disable=unused-import -import sys import testtools -sys.modules['rpm'] = mock.Mock() - -import cgcs_patch.patch_controller # noqa: E402 +from cgcs_patch.patch_controller import PatchController class CgcsPatchControllerTestCase(testtools.TestCase): @mock.patch('six.moves.builtins.open') - def test_cgcs_patch_controller_instantiate(self, mock_open): # pylint: disable=unused-argument - # pylint: disable=unused-variable - pc = cgcs_patch.patch_controller.PatchController() # noqa: F841 + def test_cgcs_patch_controller_instantiate(self, _mock_open): + pc = PatchController() + self.assertIsNotNone(pc) diff --git a/sw-patch/cgcs-patch/pylint.rc b/sw-patch/cgcs-patch/pylint.rc index 94ad01da..e5286b6b 100644 --- a/sw-patch/cgcs-patch/pylint.rc +++ b/sw-patch/cgcs-patch/pylint.rc @@ -329,7 +329,7 @@ ignored-modules=dnf,libdnf # List of classes names for which member attributes should not be checked # (useful for classes with attributes dynamically set). -ignored-classes=rpm,PKCS1_PSS +ignored-classes=PKCS1_PSS # When zope mode is activated, add a predefined set of Zope acquired attributes # to generated-members.