:thumbsup:

This adds some more complexity for users who want to enable garbage collection in their Koji instances.

What value do you recommend for the --lock-file option out of the box? I'm thinking like /var/run/user/<uid>/koji-gc.lock

To make this easier for users, can we document the recommended location? Or make it a default?

@ktdreyer I intended to not change original behaviour at all, so --lock-file is only optional and needn't to be used at all. So, that's the reason, why it has no default (so it is disabled) - maybe just creating some documentation (or document all util/*) at all would be more helpful?

Would you please say more about where you intend to store your lock file when you deploy this change?

As gc needs to be run under the root, /var/lock/koji-gc.lock should work.

As gc needs to be run under the root

koji-gc is just a client tool for Koji. It does not need to be run as root, or on any sort of privileged host (it does not access /mnt/koji directly). It can even be run in test mode without admin permissions or authentication.

Ah, thought, that it does some real deletion by itself. So, in such case it is more problematic. I expect people running it via cron/root, where I would use that systemwide location (or /var/lock/koji-gc.lock) or manually. In manual case I wouldn't use lock-file at all (as it can be handled by user) or use /run/user/<uid>/koji-gc.lock

It sounds like we could reduce the complexity of the implementation by always writing to /run/user/<uid>/koji-gc.lock (don't make it optional)

removed --lock-file option

It sounds like we could reduce the complexity of the implementation by always writing to /run/user//koji-gc.lock (don't make it optional)

I might be too late to this party, but I think the possibility of specifying the location of the lockfile is useful. It allows us to put the file on shared storage and make sure only one machine in the cluster is running koji-gc at any one time.

Ok, I've changed it, that option exists and default is /run/user/<uid>/koji-gc.lock.

ping @breilly @julian8628

@jcupova removed testing-ready due to merge conflict.

:thumbsup:

Commit 76caa2ef fixes this pull-request