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

Don't generate WSL distributions for Rancher Desktop #12757

Closed
AlexHunterCodes opened this issue Mar 25, 2022 · 6 comments · Fixed by #15166
Closed

Don't generate WSL distributions for Rancher Desktop #12757

AlexHunterCodes opened this issue Mar 25, 2022 · 6 comments · Fixed by #15166
Labels
Area-Settings Issues related to settings and customizability, for console or terminal good first issue This is a fix that might be easier for someone to do as a first contribution Help Wanted We encourage anyone to jump in on these. In-PR This issue has a related PR Issue-Task It's a feature request, but it doesn't really need a major design. Needs-Tag-Fix Doesn't match tag requirements Product-Terminal The new Windows Terminal. Product-WSL Issue that should probably go to Microsoft/WSL
Milestone

Comments

@AlexHunterCodes
Copy link

AlexHunterCodes commented Mar 25, 2022

Description of the new feature/enhancement

Feature request #3556 suggested blocking Terminal from auto-generating profiles for Docker Desktop's service WSL2 machines, and this was accepted and implemented.

Rancher Desktop is a new alternative to Docker for Windows developed by Rancher/SUSE and optimised for containerd and Kubernetes; and it uses a similar WSL2-based backend with two managed service machines: rancher-desktop and rancher-desktop-data. This leads to Terminal auto-generating profiles for these machines, even if there is no practical reason to ever use them.

A workaround for this is to hide the two entries from the profile list with the Settings GUI or in settings.json:

{
    "guid": "{1cd79813-5972-56cb-badb-ba5ee12693c5}",
    "hidden": true,
    "name": "rancher-desktop",
    "source": "Windows.Terminal.Wsl"
},
{
    "guid": "{1bdae7ec-8ca6-5caa-8453-8b6fe78b2748}",
    "hidden": true,
    "name": "rancher-desktop-data",
    "source": "Windows.Terminal.Wsl"
},

Terminal should extend this blocking feature to include Rancher Desktop's service machines too, with the likelihood of future apps bundling more WSL service machines in mind.

Proposed technical implementation details (optional)

This could be achieved in a few different ways:

  1. The quick fix, add more hard-coded rules to extend the code added in Don't generate WSL distributions for docker-desktop* #4156, and hope that loads more apps don't start shipping bundled WSL distros.
  2. Suggestion from Blacklist Docker for Windows WSL2 Service Machines #3556, use metadata to differentiate service machines from user machines during auto-generation. This should probably include an allowlist config option to override this automatic detection when necessary. I'm not sure how feasible this is, as I don't think there's any suitable existing metadata provided by WSL. Maybe the distributions themselves would have to provide some flag for Terminal to detect?
  3. Provide a blocklist config option, which would allow:
    • Apps providing WSL distros to programmatically insert their service machine's names into settings.json during app installation
    • Users to manually specify machines to skip, which also allows service machine developers (like me) greater flexibility in general.

Main benefit of options 2 and 3 is eliminating the burden on the Terminal team of maintaining a hard-coded blocklist, and dealing with further requests like these in future. I could see both 2 and 3 in combination being the ideal solution in the long term.

@AlexHunterCodes AlexHunterCodes added the Issue-Feature Complex enough to require an in depth planning process and actual budgeted, scheduled work. label Mar 25, 2022
@ghost ghost added Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting Needs-Tag-Fix Doesn't match tag requirements labels Mar 25, 2022
@zadjii-msft
Copy link
Member

@benhillis & @craigloewen-msft as an FYI.

Was there ever any further progress on enabling WSL dristros to mark themselves as service machines vs actual user-facing ones? That was a agood idea that I don't believe ever really got traction.

Manually suppressing these distros is easy, but that's just a game of wack-a-mole. I feel like if two different entities have done this now, it's only a matter of time till more copy the pattern. There needs to be a more official way to clearmy markup these distros.

@zadjii-msft zadjii-msft added the Product-WSL Issue that should probably go to Microsoft/WSL label Mar 25, 2022
@zadjii-msft
Copy link
Member

(I may also suggest posting a similar issue on the WSL repo to make sure this stays on their triage queue 😝)

@DHowett
Copy link
Member

DHowett commented Mar 25, 2022

Fragment extensions may be a good distro-driven way to handle this! It's worth checking out whether fragments can set hidden when updating a profile.

It's slightly worse than us not generating them, as they'll show up in the settings UI.

@AlexHunterCodes
Copy link
Author

(I may also suggest posting a similar issue on the WSL repo to make sure this stays on their triage queue 😝)

Doesn't look like a feature request was ever created on the WSL repo following up on the original metadata idea from #3556, so I'm not sure its even in their triage queue (unless there were off-GitHub discussions about it).

@zadjii-msft About posting a similar issue to WSL, were you talking to me or the WSL devs you tagged?

@zadjii-msft
Copy link
Member

zadjii-msft commented Jul 6, 2022

We may want to follow up on this discussion upstream in rancher-sandbox/rancher-desktop#818. I don't totally think it's tenable for us to continually manually exclude applications that are doing weird things with WSL distros like this. A better way would be having some property that apps can set to mark their distros themselves as not user-facing, but I suspect that'll never happen (@craigloewen-msft can correct me if I'm wrong).

Admittedly though, https://github.com/microsoft/terminal/pull/4156/files was super trivial and would not take a lot of work for someone to add additional distros there. As much as I'd prefer pursuing the technically correct solution, just monkey-patching that would be the quickest way to the best results.

Note

Walkthrough

Literally just copy #4156, for these two distros.

@zadjii-msft zadjii-msft added Help Wanted We encourage anyone to jump in on these. Product-Terminal The new Windows Terminal. Issue-Task It's a feature request, but it doesn't really need a major design. good first issue This is a fix that might be easier for someone to do as a first contribution and removed Issue-Feature Complex enough to require an in depth planning process and actual budgeted, scheduled work. labels Jul 6, 2022
@zadjii-msft zadjii-msft added this to the Backlog milestone Jul 6, 2022
@zadjii-msft zadjii-msft added Area-Settings Issues related to settings and customizability, for console or terminal and removed Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting labels Jul 6, 2022
@ghost ghost removed the Needs-Tag-Fix Doesn't match tag requirements label Jul 6, 2022
@craigloewen-msft
Copy link
Member

@zadjii-msft we definitely have it on our backlog to have a way for distros to mark themselves as non-user facing, but it isn't on our short term roadmap yet.

@microsoft-github-policy-service microsoft-github-policy-service bot added the In-PR This issue has a related PR label Apr 11, 2023
@microsoft-github-policy-service microsoft-github-policy-service bot added the Needs-Tag-Fix Doesn't match tag requirements label Apr 12, 2023
DHowett pushed a commit that referenced this issue Apr 12, 2023
Don't generate a profile for rancher-desktop utility WSL distro.

Adds a check for rancher-desktop as well as docker. As mentioned in the
discussion of this issue. This becomes much more difficult to maintain
once other folks inevitably start to follow this pattern. But the easy
win was up for grabs so I took it :)

Closes #12757
DHowett pushed a commit that referenced this issue Apr 25, 2023
Don't generate a profile for rancher-desktop utility WSL distro.

Adds a check for rancher-desktop as well as docker. As mentioned in the
discussion of this issue. This becomes much more difficult to maintain
once other folks inevitably start to follow this pattern. But the easy
win was up for grabs so I took it :)

Closes #12757

(cherry picked from commit 10bdadf)
Service-Card-Id: 89001998
Service-Version: 1.17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-Settings Issues related to settings and customizability, for console or terminal good first issue This is a fix that might be easier for someone to do as a first contribution Help Wanted We encourage anyone to jump in on these. In-PR This issue has a related PR Issue-Task It's a feature request, but it doesn't really need a major design. Needs-Tag-Fix Doesn't match tag requirements Product-Terminal The new Windows Terminal. Product-WSL Issue that should probably go to Microsoft/WSL
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

4 participants