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

Allow for longer transfers without a constant connection to the pod/k8s api #71

Open
telyn opened this issue Aug 7, 2021 · 5 comments

Comments

@telyn
Copy link

telyn commented Aug 7, 2021

Is your feature request related to a problem? Please describe.
I triggered a migration from my laptop using pv-migrate to migrate a large amount of data (1.5TB) into my cluster-managed storage. Due to the length of time the migration took, my laptop went in-and-out of connectivity and ended up cycling through all the migration strategies.

Describe the solution you'd like
I can see two not mutually-exclusive solutions here:

  • once a migration has been confirmed to start with a given strategy, a more robust reconnection mechanism to prevent unnecessary transfer halts & strategy cycling. After all, it creates a Job, which Kubernetes will manage Pods for until success is achieved.
  • a fire-and-forget mode with no progress output, leaving cleanup of the Job, Pods and Services up to the user
    • stretch goal here could be to have a --cleanup mode that looks for successfully-completed pv-migrate jobs and cleans them up
@telyn telyn changed the title Fire-and-forget mode Allow for longer transfers without a constant connection to the pod/k8s api Aug 7, 2021
@utkuozdemir
Copy link
Owner

Thanks for the feature suggestion. I see the problem, and indeed, it makes sense to address it.

I agree with your suggested solution of a detach mode with a cleanup target to be run afterwards. I thought of the same approach before reading that you suggested the same approach :)

Something like

  • --detach flag to the migrate command that starts the job and terminates immediately - or alternatively, a --no-cleanup flag to disable the behavior of cleanup on SIGINT and on job finish/error
  • a cleanup command that operates on the given contexts/namespaces

About the robust connection mechanism, I agree - makes total sense. However, there can be various failure scenarios that require a different way of handling - some require a reconnect, and others might need jumping to the next strategy. For example, a connection failure can be recovered with a retry.

Differentiating between some "common" errors like a connection timeout and handling them specially might be a good start. I am open for ideas here - let me know if you have any.

If you are interested in submitting PRs for any of these, please let me know so I don't start working on them already :)

@telyn
Copy link
Author

telyn commented Aug 9, 2021

Thanks for the response - I like the phrasing of the '--detach' flag a lot - I was struggling with the wording, hence "fire-and-forget", haha.

I've not yet taken a look through the code (I'm sure it's fairly simple though!), but yeah I'd be happy to start working on the --detach flag. In terms of timing I'm not likely to be able to immediately jump on it - maybe next week sometime?

I'm not yet aware of all the details of when and why you might need to jump to the next strategy that couldn't be detected up-front. From what I can see so far, the logic for choosing is:

  • mnt2 is used when both PVs are in the same namespace,
  • svc when they're not
  • lbsvc presumably when migrating between clusters

I guess that production k8s deployments with complex internal routing & firewalling might have communication blocked between certain namespaces/pods - so you'd need to fallback to lbsvc in those circumstances, and detect them by the rsync pod failing?

@utkuozdemir
Copy link
Owner

Thanks, no need to rush on the feature - take your time :)
But before implementing the --detach flag, need to refine it a bit. I'll write my thoughts on that in the next couple of weeks.

The logic works like the following:

  • Strategies are tried in the order: mnt2,svc,lbsvc
  • Every strategy evaluates the request and tells if it can deliver it or not. For example, for a cross-cluster request, mnt2 and svc will reject the request immediately.
  • If any errors occurs while executing the request, then again, it falls back to the next strategy. For example, if a mnt2 fails during the operation, it will fall back to svc.

I chose to implement it this way with 2 separate reasons of fallback - for "can't do" and "error" situations, because there can be reasons for a migration to fail that I cannot predict. Like you said, NetworkPolicies can be one reason.

@telyn
Copy link
Author

telyn commented Aug 10, 2021

Yep - that makes sense. I wonder if we could detect if a transfer began successfully - if it began I would think we ought to be able to assume that if it can finish, it can finish using the current strategy? Parsing the logs output is already happening (I assume) given the progress bar's existence - so perhaps detect the first finished file?

I also thought about running a test transfer between the PVs and going with the first strategy that completes a test transfer successfully without a fallback mechanism - unsure about that if you want to defend against things changing mid-transfer though.

Would you like to keep this issue for discussing improvements to the fallback mechanism, and I can open a second ticket for the --detach flag since it seems simpler than the fallback stuff in terms of being able to extract our discussion about it?

@utkuozdemir
Copy link
Owner

I think we can proceed on this current ticket.

Thought about this a bit and I think something like this would work:

  • Add a --detach to the migrate command, which will detach from the process as soon as a transfer is started. Here, I think we can exit as soon as a strategy reports that it can do the migration, or alternatively, check the logs to find out if any bytes were transferred.
  • We can leave strategy cycling out of scope - detach mode does not need to support falling back to other strategies since it would require a large refactor and there is a good chance for the migration to complete successfully if it started to transfer bytes.
  • We would add a new command, cleanup with the following flags: --context, --namespace and --id - all optional. This command would remove all resources with the given flags. If an id is specified, it would clean up only the ones with that id, otherwise, it would clean up all pv-migrate resources in the target namespace.
  • If the --detach flag was used, we would log info about:
    • the fact that strategy cycling is not supported
    • user will need to run cleanup commands after the transfer is completed - it is a good idea to print the exact commands for the user to copy/paste.

What do you think? We can iterate on it as we go forward.

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

No branches or pull requests

2 participants