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

[Feature Request] Deployment clean-up #3

Closed
ChrisLahaye opened this issue Apr 4, 2022 · 7 comments · Fixed by #4
Closed

[Feature Request] Deployment clean-up #3

ChrisLahaye opened this issue Apr 4, 2022 · 7 comments · Fixed by #4
Assignees
Labels
enhancement New feature or request

Comments

@ChrisLahaye
Copy link

Hello,

It would be nice if there was an extra flag, e.g. --rm, to delete the deployment when the process exits.

Thank you for creating this plugin.

@knight42
Copy link
Owner

knight42 commented Apr 6, 2022

@ChrisLahaye Hi! I am glad that you like this plugin!

This feature is easy to implement, but before getting to work on it I am curious about why you need to clean up the krelay-server Deployment? This Deployment is to be re-used by design, so that the next time we execute kubectl relay ... we don't need to wait for the creation of krelay-server.

@knight42 knight42 added the question Further information is requested label Apr 6, 2022
@knight42 knight42 self-assigned this Apr 6, 2022
@ChrisLahaye
Copy link
Author

@ChrisLahaye Hi! I am glad that you like this plugin!

This feature is easy to implement, but before getting to work on it I am curious about why you need to clean up the krelay-server Deployment? This Deployment is to be re-used by design, so that the next time we execute kubectl relay ... we don't need to wait for the creation of krelay-server.

We are using krelay-server as a bastion host, so to access a host that is only accessible from the network of the kubernetes cluster. We are doing this very rarely on our cluster and don't want to leave unregistered resources and processes running on the cluster.

I think in general it would be good practice to provide a clean-up option that destroys the resources the tool created, even better if it is done in the same process in a transient way, like docker run --rm.

Thank you for your consideration.

@knight42 knight42 added enhancement New feature or request and removed question Further information is requested labels Apr 8, 2022
@knight42
Copy link
Owner

knight42 commented Apr 8, 2022

@ChrisLahaye Hi! Thanks for your explanation, I will implement this feature in a few days 🚀

@knight42
Copy link
Owner

knight42 commented Apr 8, 2022

@ChrisLahaye Hi! Please try out the latest version v0.0.3 and feel free to report any issues.

@ChrisLahaye
Copy link
Author

Hi @knight42,

I have a few more remarks:

  • The deployment (when running without --rm) makes 3 pods, is this really necessary, or is a single pod sufficient? Maybe the number of replicas should default to one and be configurable.
  • There is still no way to destroy the deployment that has been created when running without --rm.

The --rm for me personally solves all my issues, but I do believe above remarks would be welcome additions, so just leaving it here for thought.

Thank you!

@knight42
Copy link
Owner

@ChrisLahaye

The deployment makes 3 pods, is this really necessary

Technically a single pod is sufficient, but the krelay-server Deployment is meant to be reusable, so I want to make it high available.

There is still no way to destroy the deployment that has been created when running without --rm.

Yes you are right, and the reason is that somebody else might be still using this Deployment, and removing it might break others' connection.

Thanks for your advice!

@knight42
Copy link
Owner

@ChrisLahaye Hi! just in case you might be interested, now that krelay is able to specify the namespace of the krelay-server, so creating Deployment does not make much sense now(the deployment cannot always be reused) and using a single pod could simplify how krelay works. Therefore I decided to switch to a single pod and delete it afterwards, that said, --rm is now a default behavior and that flag has no effect and might be removed later.

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 a pull request may close this issue.

2 participants