PR #3 that fixes channel use and copying if hard-linking is not possible

A note - @ausil is rebasing this and testing it out in Fedora stg. We can do code review after that work is submitted back.

Older versions of this PR:
https://pagure.io/koji/pull-request/37
https://pagure.io/koji/pull-request/52

A few questions and comments...

This PR adds new imports of yum lib. I know koji is guilty of importing yum elsewhere (for the mergerepos script that ext repos rely on, and for importing comps in the cli), but we may soon be forced to drop those. Not to say we can't use yum for this now, but I'd like to at least consider how much work the eventual port to another lib is going to be.

The data model implementation puts these new repos in the regular repo table. Should they really be there? I.e.

• should they be eligible to be used to make buildroots?
• do we expect them to continue to be defined the same way (create_event, tag_id)

More comments pending inline....

what's this doing here?
also, maybe add signed-repo perm?

needs assertHost

The comparison to repoDone is somewhat confusing. This call only covers a single arch,
doesn't have the callbacks that repoDone does, and the calling task also calls repoDone.

No real behavior change here, but the comment is a bit confusing and suggests that
maybe we need some more straightforward logic

the default is the same as for regular repos? Does that fit the intended use case?

That was my intent, I'll correct it. I think I noticed 'image' was missing from long ago, so I added that and forgot about my original intent.

Other methods use:
host = Host()
host.verify()

Is that sufficient?

What's confusing? Are you looking for a change from me?

Having a separate timer for normal repos and signed repos is the important thing. I suspect policies for how long a signed repo will stay around will vary wildly from deployment to deployment.

The rebased branch only differs from the previous (apart from being rebased onto master) by the last two commits that seem to duplicate part of https://pagure.io/koji/pull-request/65
Since this is your master branch, I'm not sure if that was intentional.

Seems like accidental logic. The existing behavior was there to avoid clearing anything koji
didn't put there. Now it's accidentally serving a different purpose. It seems like this ought
to be more explicit. For example, we could skip 'signed' in the outer loop.
It also exposes the namespace overlap issue. repos/signed/1000 could be repo #1000 for
a tag named 'signed' or a directory of signed repos for a tag named 1000.
I wonder if we should consider a different path.
Perhaps repos/sometag/signed/NNN
Or for that matter, do we really need a different path?

Testing the current code the createrepo task appears to have the correct rpms for the arch. the final repo as written out had no rpms in it for armhfp and i386. the issues are partially fixed

My understanding is the aforementioned problem was determined to be the result of a full filesystem in stage, so I have made no changes to address that. However, in IRC it was observed that capital letters were still being used in the repo paths (the single letter directories), so I've pushed a change to address that.

Rebased for the 3rd time >_>

Is there any progress here?

Haven't seen any progress. Since upstream hasn't been involved in 3 months, I'm inclined to withdraw the installation media feature change proposed for Fedora which builds upon this work. I need to see koji-upstream care more before I'll bother continuing.

And if I get asked to rebase this a 5th time, I'm withdrawing this PR too. (currently up to 3 rebases)

We were just discussing merging it. Expect a response early next week.

Here is the branch rebased to current HEAD, with stray commits removed, and one fix applied.

https://github.com/mikem23/koji-playground/tree/signed-repos-54d-rebase

By stray commits, I mean changes unrelated to signed repos that got pulled in over time and did not cleanly disappear in the various rebases.

I did the initial rebase as normal. There were a few conflicts with PR#114, but nothing too bad. I then did a rebase -i, stripping out all the extraneous commits. The diff between the second rebase and the first was a single blank line.

A question about the new yum imports. I already get flack from Fedora for importing yum. I'm /not/ asking you to port this to dnf or anything, but I wonder if you have an idea about where that might need to go in the coming year.

I appreciate you being clear about your expectations. :)
Let me get in contact with some internal folks before I answer that question about the direction.

A few more updates on the rebased branch:
https://github.com/mikem23/koji-playground/commits/signed-repos-54d-rebase

Questions:

• getRepo has a new optional arg 'signed ' that is never used in the code. Can we drop it? It seems like querying for signed repos is going to need to work differently than the way getRepo does. Or is this opt something that Pungi relies on? Maybe we need a more robust repo query call?
• repoDone now runs the arches through canonArch. This isn't technically wrong, but afaict, all the code paths that lead here will have already done that (or am I missing something). I wonder if we should instead assert arch == canonArch(arch).
• why'd you move pkglist in the regular repos?
• cli comment says src is handled specifically, but I don't see where. What am I missing?

I would really like to see this get into Koji so that Bodhi can offload repo generation to Koji instead of using its masher. Would my assistance be helpful?

1. The "signed" arg is for consumers like Bodhi to be specific about details for a signed repository.
   https://taiga.fedorainfracloud.org/project/acarter-fedora-docker-atomic-tooling/us/461?no-milestone=1

2. Any harm in being defensive? I think I did that to eliminate risk of inconsistent arguments... or maybe because I confused myself trying to get koji to pick the right directories. :)

3. I don't think I did that intentionally...

4. Comment is wrong and out of date, sorry about that.

re: Yum vs. DNF: I don't have clear direction internally yet about DNF in a future RHEL, or when. We should have unit test or at least Koji's use cases on Yum written out so that if/when the switch happens, we can fling the same criteria at DNF and have some confidence that it works like we expect.

@jgreguske At the very least, once RHEL 7.3 drops, DNF will be updated in EPEL to match the version in Fedora, as libsolv will be updated enough that DNF 1.1 or DNF 2.0 will function on EL7.

@jgreguske can you please rebase the patch set

I've already rebased it, maybe I should close this and reopen a new one based on that

@mikem and that is where?

@ausil as linked above
https://github.com/mikem23/koji-playground/tree/signed-repos-54d-rebase

@mikem Does your branch also generate a detached signature for repomd.xml automatically?

@ngompa it does not afaict. At least, I have not added it. My changes on the branch are simply cleanup so far.

@mikem Could you publish your new branch as a PR so it can be reviewed against current master?

Yes, I'll try to get a new PR this week

@mikem A week has passed... New PR?

Please ignore this comment. I just want to subscribe to follow this issue but see no option to do so directly.

I also want to follow the progress of this issue - @jflorian for the record I opened an issue against pagure to support subscribing to comments from pull requests: https://pagure.io/pagure/issue/1866

@mikem Have you had a chance to rebase this and prep it for merging?

Updated PR: #318
https://pagure.io/koji/pull-request/318