-
Notifications
You must be signed in to change notification settings - Fork 273
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
Move away from Mutagen fork #5420
Labels
Comments
github-merge-queue bot
pushed a commit
that referenced
this issue
Feb 19, 2024
…5551) **What this PR does / why we need it**: Garden currently uses a custom fork of Mutagen for its `kubectl exec` transport. This is the last remaining blocker to get Garden back on the official Mutagen releases. At the moment, Mutagen doesn't support custom transports, but it does allow a custom ssh executable to be specified, which presents an opportunity for using a faux SSH command that calls down to `kubectl exec` using parameters smuggled through the SSH hostname specification. In this case, they are smuggled in a Base64-encoded JSON object (with some slight encoding tweaks to pass through Mutagen's local path detection heuristic). The faux SSH implementation added in the original PR #3102 will almost certainly be useable (with minor tweaks) as a custom transport implementation once support exists in Mutagen. The faux SSH implementation has been hosted in its own repo at https://github.com/garden-io/mutagen-faux-ssh. **Which issue(s) this PR fixes**: This is the rebased version of #3102 and it allows to move away from the Mutagen fork. Fixes #5420 **Special notes for your reviewer**: Some questions from the original PR (#3102): > This PR is fairly close to complete, but there are a few remaining questions: > > 1. Is this approach satisfactory overall? > 2. Should the faux `ssh` command be rewritten in TypeScript? > 3. How should the faux `ssh` command be source-controlled and distributed? > 4. Are there any transition considerations for active `garden dev` commands? > > Regarding (1), I think this is the best approach because it gets Garden back on the official Mutagen releases with the minimal amount of effort and the implementation strategy is very similar to what custom transports will eventually look like in Mutagen, meaning that the faux SSH command will probably be convertible to a `mutagen-transport-garden` (name TBD) command later on. > > Regarding (2), that's a policy decision I'd leave to Garden. The critical aspect is just the proper signal and standard input closure forwarding. > > Regarding (3), that's probably a good point for discussion. I would imagine it would be distributed/downloaded in a similar manner to the `mutagen` executable, but I don't know the best location to perform its build or serve its binaries. There's also the consideration of its versioning w.r.t. Garden, especially if the format of the JSON changes, but that's likely to see very little churn once implemented, so manually managing its versioning as some external project might be suitable. Once this is decided and implemented, the `MUTAGEN_SSH_PATH` environment variable will need to be set for `mutagen daemon run`; a TODO has been indicated for this in the code. > > Regarding (4), the simple answer is that the project would probably just need to be restarted entirely. If the same `MUTAGEN_DATA_DIRECTORY` is used, then the standard Mutagen daemon will just reject and ignore the prior `exec`-based sessions, but testing of this would be good before shipping. Comments from @vvagaytsev: 1. Yes, that approach was good and simple. 2. No, the original implementation in Go worked fine. 3. It's now hosted at https://github.com/garden-io/mutagen-faux-ssh. 4. Agree, all running Mutagen processes must be killed before using the new transport and original Mutagen: 4.1. All sync must be stopped. 4.2. All sync monitors must be terminated. 4.3. Mutagen daemon must be stopped. --------- Co-authored-by: Jacob Howard <jacob@mutagen.io>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Right now we use a Mutagen fork to enable syncing to Kubernetes Pods.
This prevents us from updating to the latest version of Mutagen, which means that users might be exposed to bugs that are already fixed in the latest version of Mutagen.
See also #3102
The text was updated successfully, but these errors were encountered: