RPM Support

ttereshc wrote:

• make collection names unique across repos, maybe by adding repo_id to its name

I think this is the best solution, and is something we should be able to do at the distribution step. I also plan to add some protection to prevent re-adding the string we use to make collection names unique in the event a pulp is being synced from another pulp; depending how far down a user goes down that rabbit hole, this protection would prevent, e.g. "collection-name-<repo-id>-<repo-id>-<repo-id>-<repo-id>"

I've been trying to get at this for a while, and after getting my head back into errata-land, peeking at dnf's source to see how it works, etc, put together a real reproducer:

# assumed running in vagrant env, with zoo already added
# serve-http for ssl laziness
$ pulp-admin rpm repo update --repo-id zoo --relative-url zoo --serve-http true --feed https://repos.fedorapeople.org/repos/pulp/pulp/fixtures/rpm/
$ pulp-admin rpm repo create --repo-id zoo2 --relative-url zoo2 --serve-http true --feed https://repos.fedorapeople.org/repos/pulp/pulp/fixtures/rpm/
$ for repo in zoo zoo2; do pulp-admin rpm repo sync run --repo-id $repo; done
# remove a different package in the "Sea_Errata" from each zoo repo
$ pulp-admin rpm repo remove rpm --repo-id zoo --str-eq="name=penguin"
$ pulp-admin rpm repo remove rpm --repo-id zoo2 --str-eq="name=shark"
# republish to ensure removed packages are (not...) represented in repo
$ for repo in zoo zoo2; do pulp-admin rpm repo publish run --repo-id $repo; done
# for fun, check the repoview for zoo and zoo2 to manually verify packages removed
# while you're poking around in there, check to make sure the removed packages are
# also missing from Sea_Erratum in the updateinfo file (they should be)
# create and install penguin and shark 0.0-0, I followed the instructions here:
# http://www.redhat.com/archives/rpm-list/2006-April/msg00015.html
# Set name to penguin or shark, version to 0.0, release to 0
$ sudo dnf -y install /home/vagrant/rpmbuild/RPMS/noarch/*.rpm
# now...verify. Since shark 0 and penging 0 are installed, there are updates available in
# Sea_Erratum, id RHEA-2012:0055 in current fixtures. Add the synced repos and check errata
$ sudo dnf config-manager --add-repo http://localhost/pulp/repos/zoo --add-repo http://localhost/pulp/repos/zoo2
$ dnf updateinfo list
# see shark and penguin updates:
RHEA-2012:0055 security penguin-0.9.1-1.noarch
RHEA-2012:0055 security shark-0.1-1.noarch

So, at least when using our fixtures repo, I'm unable to reproduce this on the latest fedora, indicating to me that it is perhaps a bug in yum/dnf on older platforms. @tteresch, is my reproducer bad and I'm missing something, or is this possibly NOTABUG?

I'm happy to reproduce this again with fedora or rhel repos if desired.

Edit: Forgot to mention that these tests were done with recent (as of yesterday) tips of 2.12-dev on pulp and pulp_rpm

semyers, my understanding of the issue matches pretty well with the way you tried to reproduce it.

I would try to reproduce it on a latest RHEL system since it has yum only and no dnf.
AFAIK, dnf has a codebase different from the yum's one and their logic and handling of various things can be different (including their plugins like updateinfo), that's my assumption at least.

If RHEL test will show no issue as well, maybe one more thing worth checking - the reporter in BZ said that the issue was reproducible always and he specified two repositories which we can try to sync and check (you can't use existing fixtures for that, but you can create locally fixtures you need, though I think it is easier just to use those from BZ).
This is mostly to prove that we reproduced everything correctly and only empty pkglists are messing up with yum's merge logic.

Yeah, that's one of two things I've thought of to dig into a little deeper:

1. Use yum on el7 instead of dnf on recent fedora
2. Update fixtures to make our updateinfos look and act more like "official" ones (so all pkglists have a "short" name, as one example)

dnf is a backward-incompatible fork of yum, and still carries a lot of the same code, but also seems to contain bug fixes that haven't made their way back to yum, such as the ability to correctly merge errata provided by different repos.

So, like you've recommended, I'll work with yum on el7 using the repos from the bz and see how things go.

(Sorta) good news, I'm able to reproduce this on el6 using the steps above, which means our fixtures do replicate the problem as described in the BZ bug referenced in the previous comment.

$ sudo yum updateinfo list|grep 0055
RHEA-2012:0055 security       shark-0.1-1.noarch

So, to test the rename solution, I updated the errata to publish with different collection names:
c.update({'errata_id': 'RHEA-2012:0055'}, {$set: {'pkglist.0.short': 'foo', 'pkglist.1.short': 'bar'}})

Then I republished, clean the yum metadata, and checked again:

$ sudo yum updateinfo list|grep 0055
RHEA-2012:0055 security       penguin-0.9.1-1.noarch
RHEA-2012:0055 security       shark-0.1-1.noarch

So indeed, changing the collection short name fixes this, and should be easy enough, as mentioned in comment #6. There's still a chance that this is a fixable yum bug, and I'm also curious about what happens in el7. I'll get those questions answered shortly if I can.

I've reproduced this in el7 as well. I'm assuming that since this was fixed in dnf but not in yum that it was considered a backward-incompatibile change, and therefore not backported, which in turn means that means I'll be fixing it in pulp.

+1 for your both last comments, semyers

I just tested a fix where I only altered the collection names on publish, and this is still not enough. Pulp does all the right filtering, and ends up making two different package lists.

zoo:

    <pkglist>
      <collection short="zoo">
        <name>1</name>
        <package src="http://www.fedoraproject.org" name="walrus" epoch="0" version="5.21" release="1" arch="noarch">
          <filename>walrus-5.21-1.noarch.rpm</filename>
        </package>
        <package src="http://www.fedoraproject.org" name="shark" epoch="0" version="0.1" release="1" arch="noarch">
          <filename>shark-0.1-1.noarch.rpm</filename>
        </package>
      </collection>
    </pkglist>

zoo2:

    <pkglist>
      <collection short="zoo2">
        <name>1</name>
        <package src="http://www.fedoraproject.org" name="walrus" epoch="0" version="5.21" release="1" arch="noarch">
          <filename>walrus-5.21-1.noarch.rpm</filename>
        </package>
        <package src="http://www.fedoraproject.org" name="penguin" epoch="0" version="0.9.1" release="1" arch="noarch">
          <filename>penguin-0.9.1-1.noarch.rpm</filename>
        </package>
      </collection>
    </pkglist>

It looks like there was an issue with my testing in comment 10, and changing the collection's short name isn't good enough.

semyers wrote:

It looks like there was an issue with my testing in comment 10, and changing the collection's short name isn't good enough.

I'm still unable to reproduce my results in comment 10, which leads me to believe I forgot a step in testing, like I forgot to 'sudo' the 'yum clean metadata' command, or I forgot to republish with --force-full after deleting a package from zoo/zoo2, or some other oversight.

Poking around at the yum source, it's the collection "name" element that matters, not the "short" attribute in the collection element (yay xml). My main problem was that I was focused on "collection name in pkglist" to mean "the 'short' attribute in the pkglist element used to name collections", not "the name element in individual collections that are found under a pkglist element". The difference is that the "short" attr of pkglists, as far as I can tell, is only used when writing updateinfo XML out; it's imported for the sole purpose of being exported, and never actually used. It's this bit of code that does the merging:

https://github.com/rpm-software-management/yum/blob/master/yum/update_md.py#L545-L553

Note that pkg['name'] in that code is actually the value of the "name" element inside a package collection inside a pkglist. You'd think this was maybe a package name, but you would be wrong. Or at least I was wrong.

tl;dr yum talks about "pkglist name" a lot, but the pkglist attr that looks like a name ("short") isn't the pkglist's name. The thing that matters to yum is a collection's <name> tag, which is obvious (now) when re-reading the issue description. pkglists can have multiple collections that don't have to have the same name, so the idea of a "pkglist name" is...ambiguous.

Time to make my short-name uniquifier work on that "name" element instead, and see how that gets yum working but (I assume) breaks dnf. :)

Fun aside: According to the pkglist dtd, "name" is a valid attribute on the collection tag, and a valid subelement. yum clearly uses the subelement, as does pulp.

ttereshc posted a PR that conflicted with mine, and made some more work necessary. I've got all the tests passing again, so now I'm going through and double-checking docstrings an comments to make sure things aren't telling lies after my changes.

I filed https://github.com/PulpQE/pulp-smash/issues/569 as an alternate take for testing that doesn't require using yum/dnf to test.