kiwix / operations

Kiwix Kubernetes Cluster
http://charts.k8s.kiwix.org/
5 stars 0 forks source link

Cleanup /data/tmp/ci/dev_preview of files older than 30 days and empty directories #144

Closed benoit74 closed 7 months ago

benoit74 commented 7 months ago

Fix #143

Changes:

benoit74 commented 7 months ago

Great, thank you!

I'd prefer we use our maintenance image instead of an nginx one (?!). It allows automatically downloading/installing scripts from this repo which makes maintenance a lot easier.

That's cool, how to use it? Do I get it right that it means you expect the cleanup script to live in this k8s repo? Any recommendation in term of location? (same folder than k8s cronjob?). Btw, the nginx image is the one used by tmp-kiwix, as mentioned, but never mind.

I don't see the point of the “dry-run” commands If the goal is to display the files, appending -print to the find command does it. “dry-run” is misleading in this case as the delete command follows right after so it prevents nothing.

You are right.

Such destructive scripts should be using set -e so that any failing command blocks the rest of the script.

The ; between commands does the same thing.

I'd actually prefer we write this script in Python. We can do it easily with the stdlib and we have access to python so expecting maintainers to master shell scripting in addition to python seems superfluous.

Makes sense.

rgaudin commented 7 months ago

The ; between commands does the same thing.

it does the opposite

rgaudin commented 7 months ago

That's cool, how to use it? Do I get it right that it means you expect the cleanup script to live in this k8s repo? Any recommendation in term of location? (same folder than k8s cronjob?).

Yes, code in job description is hard to read and error-prone ; same goes for in-config maps. With this method, you get a clear, python file on the repo ; with your syntax coloring and linting if you wish. I find this easier to maintain.

Here's the repo for the image: https://github.com/rgaudin/docker-images/tree/main/debian You are invited to port that to kiwix/container-images 🙃. I'm not sure why I kept it separate. Was probably a temp thing and I forgot.

benoit74 commented 7 months ago

NOTA: for now the INSTALL_SCRIPTS is set to use a direct URL referencing a branch since this is not yet merged ... the chicken and egg problem ; I intend to migrate this to a github URL once this is merged and before deploying to cluster

benoit74 commented 7 months ago

Doesn't feel like @benoit74 😂

But is it better or worse? (yes, I was not "alone" to write this ^^)