search

FNNDSC / pman

A process management system written in python
MIT License
23 stars 33 forks source link

Delete jobs in pman using delete command #108

Closed husky-parul closed 5 years ago

husky-parul commented 6 years ago

Added env variables, volumes, and volumeMount to reaper module. This was missed in previous PR causing reaper to fail.

danmcp commented 5 years ago

@husky-parul There are a couple of nit comments. Also is the PR ready/tested? It still says DO NOT MERGE in the title.

danmcp commented 5 years ago

@husky-parul There are still a couple of unresolved comments on this pull. Let me know when you are ready for another review.

rudolphpienaar commented 5 years ago

Good to merge? @danmcp ?

danmcp commented 5 years ago

@rudolphpienaar There are still a couple of unresolved comments on this one.

husky-parul commented 5 years ago

@danmcp @ravisantoshgudimetla All changes add. Ready for review.

danmcp commented 5 years ago

Seems relevant: https://kubernetes.io/docs/concepts/workloads/controllers/jobs-run-to-completion/#clean-up-finished-jobs-automatically

husky-parul commented 5 years ago

Seems relevant: https://kubernetes.io/docs/concepts/workloads/controllers/jobs-run-to-completion/#clean-up-finished-jobs-automatically This requires adding ttlSecondsAfterFinished in job spec in openshiftmgr.py

danmcp commented 5 years ago

@husky-parul And does it theoretically solve the problem or would something still be missing?

husky-parul commented 5 years ago

@husky-parul And does it theoretically solve the problem or would something still be missing? @danmcp If this works, we can do away with cron jobs to delete old jobs (completed/failed). I am testing it. Should we remove delete command from pman as all the jobs will be deleted eventually when their TTL expires?

danmcp commented 5 years ago

@husky-parul We would still need delete for tests unless the ttl is really short

ravisantoshgudimetla commented 5 years ago

I think this is enabled in kube 1.12(that alpha), so might not work with OpenShift 3.11 yet.(4.0 is based on 1.12 but currently rebase to kube 1.12 is in progress and we cannot see this feature in OpenShift till it graduates to beta in kube upstream.)

husky-parul commented 5 years ago

I think this is enabled in kube 1.12(that alpha), so might not work with OpenShift 3.11 yet.(4.0 is based on 1.12 but currently rebase to kube 1.12 is in progress and we cannot see this feature in OpenShift till it graduates to beta in kube upstream.)

@danmcp @ravisantoshgudimetla setting TTL is not deleting jobs as of now so keeping cron jobs.

husky-parul commented 5 years ago

Added ttlSecondsAfterFinished for Openshift 4.0 support.

danmcp commented 5 years ago

@husky-parul Everything tested and ready to merge?

danmcp commented 5 years ago

LGTM @rudolphpienaar Please review and merge.