theforeman / community-templates

A place to share templates for various OSes for Foreman's provisioning
GNU General Public License v3.0
144 stars 182 forks source link

Refs #28782 - updates compare host report #711

Closed lzap closed 4 years ago

lzap commented 4 years ago

This is a rewrite of the original report, unfortunately this is flawed by design - gem version comparison is not compatible with RPM NVRE versioning scheme. It will only work for versions to some degree, RPM versioning is much more complicated. I'd rather remove this template, for now I am putting a warning comment in that report at least. For more info:

https://docs.fedoraproject.org/en-US/packaging-guidelines/Versioning/

To get this implemented correctly, it would need to implement full versioning parser as it's implemented in Pulp.

This fixes https://github.com/theforeman/community-templates/pull/679

lzap commented 4 years ago

Both release and epoch when in use are giving wrong results. Example: 1.0-1 == 1.0-2 according to this code, while its not true for the actual RPM package.

lzap commented 4 years ago

Rebased, based on the gem compare.

I think we should either remove this template (my bad I merged it no doubt) or to keep it in this simple form which often will give incorrect results. I don't think we should be solving this NVRE problem in a template, there should be a feature built-in katello if there is demand for this. Currently, Katello codebase does not offer any method that compares two versions, looks like quite complex problem: https://docs.pulpproject.org/en/2.16/plugins/pulp_rpm/tech-reference/sort-index.html

I don't feel competent to tell or implement this, I believe that version resolution is done in Pulp via RPM libraries it uses.

theforeman-bot commented 4 years ago

@lzap, this pull request is currently not mergeable. Please rebase against the develop branch and push again.

If you have a remote called 'upstream' that points to this repository, you can do this by running:

    $ git pull --rebase upstream develop

This message was auto-generated by Foreman's prprocessor

lzap commented 4 years ago

Rebased, @ezr-ondrej what you think? It's not perfect but very much improved from what we had initially. Maybe if we can squeeze this one into 2.1?

lzap commented 4 years ago

Rebased, thanks.

tbrisker commented 4 years ago

On behalf of the Foreman team, thank you for your contribution to the Foreman community templates repository! This repository is being archived and all templates are migrated to the relevant repositories. If you wish to pursue this change, please re-open the PR against the relevant repository. For more information and background, please click here.