kiwigrid / k8s-sidecar

This is a docker container intended to run inside a kubernetes cluster to collect config maps with a specified label and store the included files in a local folder.
MIT License
613 stars 183 forks source link

Bug: target directory should be tidy #227

Closed cndoit18 closed 2 years ago

cndoit18 commented 2 years ago

Fixes: #59 Signed-off-by: cndoit18 cndoit18@outlook.com

cndoit18 commented 2 years ago

PTAL @jekkel

cndoit18 commented 2 years ago

Sorry for the late reply. I think the approach is sane and it seems to properly track the keys 👍 Unfortunately the code has got a bit to complex now with many calls with huge parameter lists all over the place. Any idea how to improve on that?

Also please add an appropriate test so we catch any regression in the future.

I've tried to simplify it, but I can't think of a suitable way.

cndoit18 commented 2 years ago

Thanks for the contribution. The tests still fail due to bash not available in the image. Maybe you should go with downloading the files before the assertion statement which only looks on local files then as done with all the other assertions and files.

Thx, I'll come by later and fix the test part, unfortunately I don't have a way to automatically trigger the ci to determine if my test can run

cndoit18 commented 2 years ago

Thanks for the contribution. The tests still fail due to bash not available in the image. Maybe you should go with downloading the files before the assertion statement which only looks on local files then as done with all the other assertions and files.

done

cndoit18 commented 2 years ago

@jekkel A few use cases have been successful and we may need to wait longer to update the data.

jekkel commented 2 years ago

[...] unfortunately I don't have a way to automatically trigger the ci to determine if my test can run

Sorry for the inconvenience but for the time being I try to kick-off CI whenever I notice any change on the PR.

cndoit18 commented 2 years ago

[...] unfortunately I don't have a way to automatically trigger the ci to determine if my test can run

Sorry for the inconvenience but for the time being I try to kick-off CI whenever I notice any change on the PR.

Thank you very much, it looks like it passed