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

Windows: Don't abort everything if $HOME is not writable #8042

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
155 changes: 112 additions & 43 deletions pkg/rancher-desktop/integrations/windowsIntegrationManager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,16 @@ type SyncState =
/** The `queued` promise will be resolved after the current sync +1 is complete. */
{ state: SyncStateKey.QUEUED, active: ReturnType<typeof Latch>, queued: ReturnType<typeof Latch> };

/**
* DiagnosticKey limits the `key` argument of the diagnostic events.
*/
type DiagnosticKey =
'docker-plugins' |
'docker-socket' |
'kubeconfig' |
'spin-cli' |
never;

/**
* WindowsIntegrationManager manages various integrations on Windows, for both
* the Win32 host, as well as for each (foreign) WSL distribution.
Expand Down Expand Up @@ -177,7 +187,16 @@ export default class WindowsIntegrationManager implements IntegrationManager {
return this.syncState.queued;
}
try {
const kubeconfigPath = await K3sHelper.findKubeConfigToUpdate('rancher-desktop');
let kubeconfigPath: string | undefined;

try {
kubeconfigPath = await K3sHelper.findKubeConfigToUpdate('rancher-desktop');
this.diagnostic({ key: 'kubeconfig' });
} catch (error) {
console.error(`Could not determine kubeconfig: ${ error } - Kubernetes configuration will not be updated.`);
this.diagnostic({ key: 'kubeconfig', error });
kubeconfigPath = undefined;
}

await Promise.all([
this.syncHostSocketProxy(),
Expand Down Expand Up @@ -208,7 +227,7 @@ export default class WindowsIntegrationManager implements IntegrationManager {
}
}

async syncDistro(distro: string, kubeconfigPath: string): Promise<void> {
async syncDistro(distro: string, kubeconfigPath?: string): Promise<void> {
let state = this.settings.WSL?.integrations?.[distro] === true;

console.debug(`Integration sync: ${ distro } -> ${ state }`);
Expand All @@ -228,6 +247,21 @@ export default class WindowsIntegrationManager implements IntegrationManager {
}
}

/**
* Helper function to trigger a diagnostic report. If a diagnostic should be
* cleared, call this with the error unset.
*/
protected diagnostic(input: {key: DiagnosticKey, distro?: string, error?: unknown}) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you think this needs a check for the input key? e.g.

  if (!input.key) {
    throw new Error('Diagnostic key is required');
  }

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, because key is not optional (distro and error are), per TypeScript annotations. Also, this is a protected method, so we should be able to expect the callers to be type-checked.

const error = input.error instanceof Error ? input.error : input.error ? new Error(`${ input.error }`) : undefined;

mainEvents.emit('diagnostics-event', {
id: 'integrations-windows',
key: input.key,
distro: input.distro,
error,
});
}

#wslExe = '';
/**
* The path to the wsl.exe executable.
Expand Down Expand Up @@ -326,10 +360,15 @@ export default class WindowsIntegrationManager implements IntegrationManager {
const reason = this.dockerSocketProxyReason;

console.debug(`Syncing Win32 socket proxy: ${ reason ? `should not run (${ reason })` : 'should run' }`);
if (!reason) {
this.windowsSocketProxyProcess.start();
} else {
await this.windowsSocketProxyProcess.stop();
try {
if (!reason) {
this.windowsSocketProxyProcess.start();
} else {
await this.windowsSocketProxyProcess.stop();
}
this.diagnostic({ key: 'docker-socket' });
} catch (error) {
this.diagnostic({ key: 'docker-socket', error });
}
}

Expand All @@ -354,7 +393,6 @@ export default class WindowsIntegrationManager implements IntegrationManager {
* distribution is started or stopped, as desired.
* @param distro The distribution to manage.
* @param state Whether integration is enabled for the given distro.
* @note this function must not throw.
*/
protected async syncDistroSocketProxy(distro: string, state: boolean) {
try {
Expand Down Expand Up @@ -393,39 +431,49 @@ export default class WindowsIntegrationManager implements IntegrationManager {
delete this.distroSocketProxyProcesses[distro];
}
}
this.diagnostic({ key: 'docker-socket', distro });
} catch (error) {
console.error(`Error syncing ${ distro } distro socket proxy: ${ error }`);
this.diagnostic({
key: 'docker-socket', distro, error,
});
}
}

protected async syncHostDockerPluginConfig() {
const configPath = path.join(os.homedir(), '.docker', 'config.json');
let config: { cliPluginsExtraDirs?: string[] } = {};

try {
config = JSON.parse(await fs.promises.readFile(configPath, 'utf-8'));
} catch (ex) {
if (ex && typeof ex === 'object' && 'code' in ex && ex.code === 'ENOENT') {
// If the file does not exist, create it.
} else {
console.error(`Could not set up docker plugins:`, ex);
const configPath = path.join(os.homedir(), '.docker', 'config.json');
let config: { cliPluginsExtraDirs?: string[] } = {};

return;
try {
config = JSON.parse(await fs.promises.readFile(configPath, 'utf-8'));
} catch (error) {
if (error && typeof error === 'object' && 'code' in error && error.code === 'ENOENT') {
// If the file does not exist, create it.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are we supposed to create a file here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We do on line 472. (This is just the old code with an extra layer of indenting.)

} else {
console.error(`Could not set up docker plugins:`, error);
this.diagnostic({ key: 'docker-plugins', error });

return;
}
}
}

// All of the docker plugins are in the `docker-cli-plugins` directory.
const binDir = path.join(paths.resources, process.platform, 'docker-cli-plugins');
// All of the docker plugins are in the `docker-cli-plugins` directory.
const binDir = path.join(paths.resources, process.platform, 'docker-cli-plugins');

if (config.cliPluginsExtraDirs?.includes(binDir)) {
// If it's already configured, no need to do so again.
return;
}
if (config.cliPluginsExtraDirs?.includes(binDir)) {
// If it's already configured, no need to do so again.
return;
}

config.cliPluginsExtraDirs ??= [];
config.cliPluginsExtraDirs.push(binDir);
config.cliPluginsExtraDirs ??= [];
config.cliPluginsExtraDirs.push(binDir);

await fs.promises.writeFile(configPath, JSON.stringify(config), 'utf-8');
await fs.promises.writeFile(configPath, JSON.stringify(config), 'utf-8');
this.diagnostic({ key: 'docker-plugins' });
} catch (error) {
this.diagnostic({ key: 'docker-plugins', error });
}
}

/**
Expand All @@ -449,8 +497,12 @@ export default class WindowsIntegrationManager implements IntegrationManager {
}

await this.execCommand({ distro }, wslHelper, ...args);
this.diagnostic({ key: 'docker-plugins', distro });
} catch (error) {
console.error(`Failed to set up ${ distro } docker plugins: ${ error }`.trim());
this.diagnostic({
key: 'docker-plugins', distro, error,
});
}
}

Expand Down Expand Up @@ -490,7 +542,13 @@ export default class WindowsIntegrationManager implements IntegrationManager {
console.debug(`Verified kubeconfig in the following distro: ${ distro }`);
}

protected async syncDistroKubeconfig(distro: string, kubeconfigPath: string, state: boolean) {
protected async syncDistroKubeconfig(distro: string, kubeconfigPath: string | undefined, state: boolean) {
if (!kubeconfigPath) {
console.debug(`Skipping syncing ${ distro } kubeconfig: no kubeconfig found`);
this.diagnostic({ key: 'kubeconfig', distro });

return 'Error setting up integration';
}
try {
console.debug(`Syncing ${ distro } kubeconfig`);
await this.execCommand(
Expand All @@ -506,6 +564,7 @@ export default class WindowsIntegrationManager implements IntegrationManager {
'kubeconfig',
`--enable=${ state && this.settings.kubernetes?.enabled }`,
);
this.diagnostic({ key: 'kubeconfig', distro });
} catch (error: any) {
if (typeof error?.stdout === 'string') {
error.stdout = error.stdout.replace(/\0/g, '');
Expand All @@ -514,28 +573,38 @@ export default class WindowsIntegrationManager implements IntegrationManager {
error.stderr = error.stderr.replace(/\0/g, '');
}
console.error(`Could not set up kubeconfig integration for ${ distro }:`, error);
this.diagnostic({
key: 'kubeconfig', distro, error,
});

return `Error setting up integration`;
}
console.log(`kubeconfig integration for ${ distro } set to ${ state }`);
}

protected async syncDistroSpinCLI(distro: string, state: boolean) {
if (state && this.settings.experimental?.containerEngine?.webAssembly) {
const version = semver.parse(DEPENDENCY_VERSIONS.spinCLI);
const env = {
KUBE_PLUGIN_VERSION: DEPENDENCY_VERSIONS.spinKubePlugin,
SPIN_TEMPLATE_BRANCH: (version ? `v${ version.major }.${ version.minor }` : 'main'),
};
const wslenv = Object.keys(env).join(':');

// wsl-exec is needed to correctly resolve DNS names
await this.execCommand({
distro,
env: {
...process.env, ...env, WSLENV: wslenv,
},
}, await this.getLinuxToolPath(distro, executable('setup-spin')));
try {
if (state && this.settings.experimental?.containerEngine?.webAssembly) {
const version = semver.parse(DEPENDENCY_VERSIONS.spinCLI);
const env = {
KUBE_PLUGIN_VERSION: DEPENDENCY_VERSIONS.spinKubePlugin,
SPIN_TEMPLATE_BRANCH: (version ? `v${ version.major }.${ version.minor }` : 'main'),
};
const wslenv = Object.keys(env).join(':');

// wsl-exec is needed to correctly resolve DNS names
await this.execCommand({
distro,
env: {
...process.env, ...env, WSLENV: wslenv,
},
}, await this.getLinuxToolPath(distro, executable('setup-spin')));
}
this.diagnostic({ key: 'spin-cli', distro });
} catch (error) {
this.diagnostic({
key: 'spin-cli', distro, error,
});
}
}

Expand Down
1 change: 1 addition & 0 deletions pkg/rancher-desktop/main/diagnostics/diagnostics.ts
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ export class DiagnosticsManager {
const imports = (await Promise.all([
import('./connectedToInternet'),
import('./dockerCliSymlinks'),
import('./integrationsWindows'),
import('./kubeConfigSymlink'),
import('./kubeContext'),
import('./kubeVersionsAvailable'),
Expand Down
45 changes: 45 additions & 0 deletions pkg/rancher-desktop/main/diagnostics/integrationsWindows.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
import { DiagnosticsCategory, DiagnosticsChecker, DiagnosticsCheckerResult, DiagnosticsCheckerSingleResult } from './types';

import mainEvents from '@pkg/main/mainEvents';

const cachedResults: Record<string, DiagnosticsCheckerResult> = {};

const CheckWindowsIntegrations: DiagnosticsChecker = {
id: 'WINDOWS_INTEGRATIONS',
category: DiagnosticsCategory.ContainerEngine,
applicable() {
return Promise.resolve(process.platform === 'win32');
},
check(): Promise<DiagnosticsCheckerSingleResult[]> {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's a little hard to see what's happening here should it be broken down to:

const mapResults = (results: Record<string, DiagnosticsCheckerResult>) => {
  return Object.entries(results).map(([id, result]) => ({ ...result, id }));
};

check(): Promise<DiagnosticsCheckerSingleResult[]> {
  return Promise.resolve(mapResults(cachedResults));
}

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 6e1d40a (I factored out the inner map function instead, and Nino seems okay with that).

const resultMapper = ([id, result]: [string, DiagnosticsCheckerResult]) => {
return ({ ...result, id });
};

return Promise.resolve(Object.entries(cachedResults).map(resultMapper));
},
};

mainEvents.on('diagnostics-event', (payload) => {
if (payload.id !== 'integrations-windows') {
return;
}
const { distro, key, error } = payload;
const message = error?.message ?? error?.toString();

cachedResults[`${ distro || '<main>' }-${ key }`] = {
passed: false,
fixes: [],
...(() => {
if (!error) {
return { passed: true, description: `${ distro }/${ key } passed` };
}
if (distro) {
return { description: `Error managing distribution ${ distro }: ${ key }: ${ message }` };
}

return { description: `Error managing ${ key }: ${ message }` };
})(),
};
});

export default CheckWindowsIntegrations;
1 change: 1 addition & 0 deletions pkg/rancher-desktop/main/mainEvents.ts
Original file line number Diff line number Diff line change
Expand Up @@ -166,6 +166,7 @@ interface MainEventNames {
* 'diagnostics-event' event.
*/
type DiagnosticsEventPayload =
{ id: 'integrations-windows', distro?: string, key: string, error?: Error } |
{ id: 'kube-versions-available', available: boolean } |
{ id: 'path-management', fileName: string; error: Error | undefined };

Expand Down
Loading