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
Fix multiple bugs related to modularity #1254
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,37 @@ | ||
import hashlib | ||
import os | ||
|
||
from pulp.plugins.util.metadata_writer import MetadataFileContext | ||
from pulp.server.exceptions import PulpCodedException | ||
|
||
from pulp_rpm.common.constants import CONFIG_DEFAULT_CHECKSUM | ||
from pulp_rpm.plugins import error_codes | ||
from pulp_rpm.plugins.distributors.yum.metadata.metadata import REPO_DATA_DIR_NAME | ||
from pulp_rpm.yum_plugin import util | ||
|
||
|
||
_LOG = util.getLogger(__name__) | ||
|
||
MODULES_FILE_NAME = 'modules.yaml.gz' | ||
|
||
|
||
# Note that this is "MetadataFileContext" from the core plugin API, not from | ||
# pulp_rpm.plugin.distributors.yum.metadata.metadata | ||
class ModulesFileContext(MetadataFileContext): | ||
|
||
def __init__(self, working_dir, checksum_type=CONFIG_DEFAULT_CHECKSUM): | ||
metadata_file_path = os.path.join(working_dir, REPO_DATA_DIR_NAME, MODULES_FILE_NAME) | ||
super(ModulesFileContext, self).__init__(metadata_file_path, checksum_type) | ||
|
||
def initialize(self): | ||
super(ModulesFileContext, self).initialize() | ||
|
||
def finalize(self): | ||
super(ModulesFileContext, self).finalize() | ||
|
||
def add_document(self, document, doc_checksum): | ||
checksum = hashlib.sha256(document).hexdigest() | ||
if checksum == doc_checksum: | ||
self.metadata_file_handle.write(document) | ||
else: | ||
raise PulpCodedException(error_codes.RPM1017) |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,7 +1,5 @@ | ||
import copy | ||
import datetime | ||
import hashlib | ||
import gzip | ||
import itertools | ||
import os | ||
import subprocess | ||
|
@@ -23,20 +21,22 @@ | |
from pulp_rpm.common import constants, ids, file_utils | ||
from pulp_rpm.yum_plugin import util | ||
from pulp_rpm.plugins import error_codes | ||
from pulp_rpm.plugins.importers.yum.repomd import modules | ||
from pulp_rpm.plugins.db import models | ||
from pulp_rpm.plugins.distributors.export_distributor import export_utils | ||
from pulp_rpm.plugins.distributors.export_distributor import generate_iso | ||
from pulp_rpm.plugins.importers.yum.repomd import modules | ||
from pulp_rpm.plugins.importers.yum.parse.treeinfo import KEY_PACKAGEDIR | ||
|
||
from . import configuration | ||
from .metadata.filelists import FilelistsXMLFileContext | ||
from .metadata.metadata import REPO_DATA_DIR_NAME | ||
from .metadata.modules import ModulesFileContext | ||
from .metadata.other import OtherXMLFileContext | ||
from .metadata.prestodelta import PrestodeltaXMLFileContext | ||
from .metadata.package import PackageXMLFileContext | ||
from .metadata.primary import PrimaryXMLFileContext | ||
from .metadata.repomd import RepomdXMLFileContext | ||
from .metadata.updateinfo import UpdateinfoXMLFileContext | ||
from .metadata.package import PackageXMLFileContext | ||
|
||
|
||
logger = util.getLogger(__name__) | ||
|
@@ -511,7 +511,7 @@ def __init__(self): | |
|
||
def process_main(self, item=None): | ||
""" | ||
Copy the metadata file into place and add it tot he repomd file. | ||
Copy the metadata file into place and add it to the repomd file. | ||
|
||
:param item: The unit to process | ||
:type item: pulp.server.db.model.ContentUnit | ||
|
@@ -810,6 +810,7 @@ class PublishErrataStepIncremental(platform_steps.UnitModelPluginStep): | |
""" | ||
Publish all incremental errata | ||
""" | ||
|
||
def __init__(self, **kwargs): | ||
super(PublishErrataStepIncremental, self).__init__(constants.PUBLISH_ERRATA_STEP, | ||
[models.Errata], **kwargs) | ||
|
@@ -834,66 +835,62 @@ def process_main(self, item=None): | |
class PublishModulesStep(platform_steps.UnitModelPluginStep): | ||
""" | ||
Publish modularity artifacts. | ||
|
||
:ivar fp_out: file-like used for writing the modules.yaml. | ||
:type fp_out: file-like | ||
""" | ||
|
||
FILE_NAME = 'modules.yaml.gz' | ||
|
||
def __init__(self): | ||
super(PublishModulesStep, self).__init__( | ||
constants.PUBLISH_MODULES_STEP, | ||
[ | ||
models.Modulemd, | ||
models.ModulemdDefaults | ||
]) | ||
self.context = None | ||
self.description = _('Publishing Modules') | ||
self.fp_out = None | ||
|
||
def is_skipped(self): | ||
""" | ||
Test to find out if the step should be skipped. | ||
|
||
:return: whether or not the step should be skipped | ||
:rtype: bool | ||
""" | ||
# skip if there are no modules files. | ||
if self.get_total() == 0: | ||
return True | ||
|
||
return super(PublishModulesStep, self).is_skipped() | ||
|
||
def initialize(self): | ||
""" | ||
Create the directory and open fp_out for writing. | ||
""" | ||
path = os.path.join( | ||
self.get_working_dir(), | ||
REPO_DATA_DIR_NAME, | ||
self.FILE_NAME) | ||
plugin_misc.mkdir(os.path.dirname(path)) | ||
self.fp_out = gzip.open(path, 'wb') | ||
checksum_type = self.parent.get_checksum_type() | ||
self.context = ModulesFileContext(self.get_working_dir(), checksum_type=checksum_type) | ||
self.context.initialize() | ||
|
||
def process_main(self, item=None): | ||
""" | ||
Append each yaml document to the modules.yaml. | ||
|
||
:param item: Either Modulemd or ModulemdDefaults. | ||
:type item: pulp.server.db.model.FileContent | ||
:type item: pulp.server.db.model.FileContent (baseclass) | ||
""" | ||
with open(item.storage_path) as fp_in: | ||
document = fp_in.read() | ||
checksum = hashlib.sha256(document).hexdigest() | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm not 100% sure why this check is necessary, but given that it was considered important enough to do + assign a coded exception to, I'm leaving it here. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @dalley, is it still a valid comment? or an outdated one? I don't follow what you mean here. It looks like the code which performs a check is removed There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ah I see, you moved it to |
||
if checksum == item.checksum: | ||
self.fp_out.write(document) | ||
else: | ||
raise PulpCodedException(error_codes.RPM1017) | ||
self.context.add_document(document, doc_checksum=item.checksum) | ||
|
||
def finalize(self): | ||
""" | ||
Close the fp_out; rename the file to include the checksum; | ||
and add to the repomd. | ||
""" | ||
if self.fp_out: | ||
self.fp_out.close() | ||
with open(self.fp_out.name, 'rb') as fp_in: | ||
checksum = file_utils.calculate_checksum(fp_in) | ||
_dir, name = os.path.split(self.fp_out.name) | ||
path = os.path.join(_dir, '-'.join([checksum, name])) | ||
os.rename(self.fp_out.name, path) | ||
repomd = self.parent.repomd_file_context | ||
repomd.add_metadata_file_metadata( | ||
data_type=modules.METADATA_FILE_NAME, | ||
file_path=path, | ||
precalculated_checksum=checksum) | ||
if self.context: | ||
self.context.finalize() | ||
self.parent.repomd_file_context.add_metadata_file_metadata( | ||
data_type=modules.METADATA_FILE_NAME, | ||
file_path=self.context.metadata_file_path, | ||
precalculated_checksum=self.context.checksum | ||
) | ||
|
||
|
||
class PublishCompsStep(platform_steps.UnitModelPluginStep): | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@dralley , it seems like that any checksum type except the sha256 raises exception (in the code below), doesn't it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I didn't experience that when I tested w/ checksum-type=sha1. I believe this particular checksum (of the content) is always sha256. It's for internal validation and isn't used in the name of the file or anything.
https://github.com/pulp/pulp_rpm/blob/2-master/plugins/pulp_rpm/plugins/db/models.py#L1808