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.

Sign up for GitHub

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

Improve error message when podman is present but podman machine isn't #3408

Merged
merged 2 commits into from
Feb 14, 2024
Merged

Improve error message when podman is present but podman machine isn't #3408

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
dc9c140
Improve error message when podman is present but podman machine isn't
datho7561 Oct 6, 2023
e306126
Check for podman machine on CLI instead of relying on odo
datho7561 Jan 10, 2024
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
14 changes: 0 additions & 14 deletions src/odo/odoWrapper.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -359,20 +359,6 @@ export class Odo {
);
}

public async isPodmanPresent(): Promise<boolean> {
try {
const result: CliExitData = await this.execute(
new CommandText('odo', 'version -o json'),
);
if ('podman' in JSON.parse(result.stdout)) {
return true;
}
} catch {
//ignore
}
return false;
}

/**
* Bind a component to a bindable service by modifying the devfile
*
Expand Down
39 changes: 31 additions & 8 deletions src/openshift/component.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,9 @@

import * as fs from 'fs/promises';
import * as JSYAML from 'js-yaml';
import { platform } from 'os';
import * as path from 'path';
import { which } from 'shelljs';
import { commands, debug, DebugConfiguration, DebugSession, Disposable, EventEmitter, extensions, ProgressLocation, Uri, window, workspace } from 'vscode';
import { Oc } from '../oc/ocWrapper';
import { Command } from '../odo/command';
Expand All @@ -14,6 +16,7 @@
import { Odo } from '../odo/odoWrapper';
import { ComponentWorkspaceFolder } from '../odo/workspace';
import sendTelemetry, { NewComponentCommandProps } from '../telemetry';
import { ChildProcessUtil, CliExitData } from '../util/childProcessUtil';
import { Progress } from '../util/progress';
import { vsCommand, VsCommandError } from '../vscommand';
import AddServiceBindingViewLoader, { ServiceBindingFormResponse } from '../webview/add-service-binding/addServiceBindingViewLoader';
Expand Down Expand Up @@ -216,15 +219,35 @@
}

private static async checkForPodman(): Promise<boolean> {
if (await Odo.Instance.isPodmanPresent()) {
return true;
}
void window.showErrorMessage('Podman is not present in the system, please install podman on your machine and try again.', 'Install podman')
.then(async (result) => {
if (result === 'Install podman') {
await commands.executeCommand('vscode.open', Uri.parse('https://podman.io/'));
const podmanPath = which('podman');
if (podmanPath) {
if (platform() === 'linux') {
return true;

Check warning on line 225 in src/openshift/component.ts

View check run for this annotation

Codecov / codecov/patch

src/openshift/component.ts#L222-L225 

Added lines #L222 - L225 were not covered by tests
}
try {
const resultRaw: CliExitData = await ChildProcessUtil.Instance.execute(`"${podmanPath}" machine list --format json`);
const resultObj: { Running: boolean }[] = JSON.parse(resultRaw.stdout);
if (resultObj.length === 1 && resultObj[0].Running) {
return true;

Check warning on line 231 in src/openshift/component.ts

View check run for this annotation

Codecov / codecov/patch

src/openshift/component.ts#L227-L231 

Added lines #L227 - L231 were not covered by tests
}
});
} catch (e) {
// do nothing; something is wrong with the podman setup
}
const SETUP_INSTRUCTIONS = 'Open setup instructions';
void window.showErrorMessage('Podman is present on the system, but is not fully set up yet.', SETUP_INSTRUCTIONS)

Check warning on line 237 in src/openshift/component.ts

View check run for this annotation

Codecov / codecov/patch

src/openshift/component.ts#L236-L237 

Added lines #L236 - L237 were not covered by tests
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@jeffmaury what do you think about this wording?

Copy link
Member

Choose a reason for hiding this comment

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

I don't think it's gonna fix the issue as Component.odo.isPodmanPresent would return true and code won't be executed

Copy link
Contributor Author

Choose a reason for hiding this comment

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

odo version only seems to report podman as installed if the podman machine is present, so in the case that the binary is on the path but the machine is not set up, it will go to this case. I've tested it on my Windows machine.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@jeffmaury if you have time, could you please tell what you think of the wording of this message? I'll double check that it shows up under the right circumstances.

.then(result => {
if (result === SETUP_INSTRUCTIONS) {
void commands.executeCommand('vscode.open', Uri.parse('https://podman.io/docs/installation'));

Check warning on line 240 in src/openshift/component.ts

View check run for this annotation

Codecov / codecov/patch

src/openshift/component.ts#L239-L240 

Added lines #L239 - L240 were not covered by tests
}
});
} else {
void window.showErrorMessage('Podman is not present in the system, please install podman on your machine and try again.', 'Install podman')
.then(async (result) => {
if (result === 'Install podman') {
await commands.executeCommand('vscode.open', Uri.parse('https://podman.io/'));

Check warning on line 247 in src/openshift/component.ts

View check run for this annotation

Codecov / codecov/patch

src/openshift/component.ts#L244-L247 

Added lines #L244 - L247 were not covered by tests
}
});
}
return false;
}

Expand Down
7 changes: 0 additions & 7 deletions test/integration/odoWrapper.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ import { expect } from 'chai';
import * as fs from 'fs/promises';
import { suite, suiteSetup } from 'mocha';
import * as path from 'path';
import { which } from 'shelljs';
import * as tmp from 'tmp';
import { promisify } from 'util';
import { Uri, workspace } from 'vscode';
Expand Down Expand Up @@ -172,12 +171,6 @@ suite('./odo/odoWrapper.ts', function () {
expect(componentDetails.starterProjects).is.not.empty;
});

test('isPodmanPresent()', async function() {
const isPodmanPresent = await Odo.Instance.isPodmanPresent();
const whichImplementationPodmanPresent = which('podman') !== null;
expect(isPodmanPresent).to.equal(whichImplementationPodmanPresent);
});

test('getStarterProjects()', async function() {
const starterProjects = await Odo.Instance.getStarterProjects({
registryName: 'DefaultDevfileRegistry',
Expand Down
Loading