search

ceph / chacra

A binary/file REST API to aid in multi-distro|arch|release management
9 stars 18 forks source link

nfs-ganesha: update purge rotation to keep nfs-ganesha binaries #229

Closed alimaredia closed 5 years ago

alimaredia commented 7 years ago

Signed-off-by: Ali Maredia amaredia@redhat.com

alimaredia commented 7 years ago

Update: Per a discussion with Alfredo last week, the purge rotation can't filter by flavor, only ref. I plan on making some changes to chacra to be able to update the purge rotation by flavor.

alimaredia commented 7 years ago

@alfredodeza made those minor changes. Do you know why one of the checks is failing?

alimaredia commented 7 years ago

retest this please

alfredodeza commented 7 years ago

jenkins test this please

alimaredia commented 7 years ago

@alfredodeza tests all pass now!

alimaredia commented 6 years ago

@andrewschoen @alfredodeza i remember this got stalled on the situation where either the keep minimum is set to 0 or the days is set to 0 in the purge rotation. As of now this PR purges the repo if either is set to 0, but you wanted two wanted me to only purge the repos if both are set to 0.

I think we should merge since I don't think having either one of those inputs in the purge_rotation is a realistic input.

alfredodeza commented 6 years ago

@alimaredia we aren't expecting things to be set to 0 across the board. We are expecting that if any conditions matches to keep a repo around, to respect that.

Your proposed change worked the other way around: if any conditions matched the delete a repo, regardless of any other setting, it would get deleted. That should not be the case.

One of your tests has this configuration:

conf.purge_rotation = {'nfs-ganesha': {'flavor': {'ceph_master': {'keep_minimum': 0}}, 'ref': {'next': {'days': 70}}}, '__force_dict__': True}

In that case, if the 'flavor' is ceph_master but it is day 1 of the next ref, I would expect this not to be removed.

dmick commented 6 years ago

Seems like maybe this needs a rebase

alimaredia commented 5 years ago

@alfredodeza Here is an overview of the test coverage for the functionality of the PR: https://paste.fedoraproject.org/paste/QvwRqVRN0cBU6MOguldnEw

If there's anything else you want me to add that I missed let me know and I'll put it in.