Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added flags to control output structure & another flag for flat dumps #29

Merged
merged 3 commits into from
Feb 9, 2022

Conversation

CollinShoop
Copy link
Contributor

@CollinShoop CollinShoop commented Feb 7, 2022

Hey @WoozyMasta 👋🏻 Very cool project, I was looking at making something just like this and figured I should contribute instead.

For my own use, having better control over the output structure would be ideal. I personally do not require resources to be broken up per file, and I also sometimes need all the detailed fields which were being removed via jq for debugging purposes (as opposed to backups, which I presume is the primary use case for this tool), so I've added a few new flags.

@CollinShoop CollinShoop changed the title Added flags to give better control over output structure Added flags to control output structure Feb 7, 2022
@CollinShoop CollinShoop changed the title Added flags to control output structure Added flags to control output structure & flat dump Feb 7, 2022
@CollinShoop CollinShoop changed the title Added flags to control output structure & flat dump Added flags to control output structure & another flag for flat dumps Feb 7, 2022
@WoozyMasta
Copy link
Owner

Hey
Looks cool, I've been wanting to make the --output-by-type mode for a long time, special thanks for that.

I'll have to find some time to check it all out. And I think these edits will be included in the new version.

@WoozyMasta WoozyMasta added the enhancement New feature or request label Feb 8, 2022
@WoozyMasta
Copy link
Owner

Do you have time to update the "Commands and Flags" section in the README.md and the .env file with the new options? It would be great.

@WoozyMasta
Copy link
Owner

@CollinShoop, I researched the solution a little.

Shouldn't the --output-by-type and --flat keys be checked for the presence of a resource. When saving all available namespace resources (a couple of deployments), a large number of empty directories or files are created, especially if there are a large number of CRDs in the cluster.

Do you think it will not be difficult to implement it, and is it worth it?

After all, when I specify specific resources, only they are saved.

For example:

./kube-dump ns -n dev-app -r deployment,svc --output-by-type 

@WoozyMasta WoozyMasta merged commit adfe3b4 into WoozyMasta:master Feb 9, 2022
@WoozyMasta
Copy link
Owner

In any case, I accepted the changes and released the new version 1.1.0.

Thank you so much for the work done.

@CollinShoop
Copy link
Contributor Author

CollinShoop commented Feb 11, 2022

@WoozyMasta Thanks for the quick review & merge 🥇

@CollinShoop, I researched the solution a little.

Shouldn't the --output-by-type and --flat keys be checked for the presence of a resource. When saving all available namespace resources (a couple of deployments), a large number of empty directories or files are created, especially if there are a large number of CRDs in the cluster.

Do you think it will not be difficult to implement it, and is it worth it?

After all, when I specify specific resources, only they are saved.

For example:

./kube-dump ns -n dev-app -r deployment,svc --output-by-type 

I had the same observation when testing this, but my reasoning for keeping as-is is that it may be better to positively indicate that there are none of a particular resource with an empty file than to omit it which could be ambiguous (ie did the dump even check that resource type?). It's a bit annoying in the case of empty namespaces, though, but I'm not sure of a simple way to avoid that now.

Do you have time to update the "Commands and Flags" section in the README.md and the .env file with the new options? It would be great.

I'll add this to my TODO 👍🏻

@CollinShoop
Copy link
Contributor Author

Actually, I see you've already updated the README.md and .env 💯

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants