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

fix(extensions): make sure periodical full ui reload doesn't happen #6693

Closed
wants to merge 2 commits into from
Closed
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
52 changes: 22 additions & 30 deletions extensions/docker/src/extension.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,11 +30,6 @@ let providerState: extensionApi.ProviderConnectionStatus = 'stopped';
let containerProviderConnection: extensionApi.ContainerProviderConnection;
let containerProviderConnectionDisposable: extensionApi.Disposable;

async function timeout(time: number): Promise<void> {
return new Promise<void>(resolve => {
setTimeout(resolve, time);
});
}
async function isDockerDaemonAlive(socketPath: string): Promise<boolean> {
const pingUrl = {
path: '/_ping',
Expand Down Expand Up @@ -89,24 +84,28 @@ async function isDisguisedPodman(socketPath: string): Promise<boolean> {
});
}

async function monitorDaemon(extensionContext: extensionApi.ExtensionContext): Promise<void> {
// call us again
if (!stopLoop) {
try {
await updateProvider(extensionContext);
} catch (error) {
// ignore the update of machines
}
await timeout(5000);
monitorDaemon(extensionContext).catch((err: unknown) => {
console.error('Error while monitoring docker daemon', err);
if (err instanceof Error) {
extensionApi.env.createTelemetryLogger().logError(err);
} else {
extensionApi.env.createTelemetryLogger().logError(err.toString());
function monitorDaemon(extensionContext: extensionApi.ExtensionContext): void {
const loop = (): NodeJS.Timeout =>
// eslint-disable-next-line @typescript-eslint/no-misused-promises
Copy link
Collaborator

Choose a reason for hiding this comment

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

we should not ignore the misused-promises in the project

setTimeout(async () => {
if (stopLoop) {
return;
}
});
}

try {
await updateProvider(extensionContext);
} catch (err) {
console.error('Error while monitoring docker daemon', err);
if (err instanceof Error) {
extensionApi.env.createTelemetryLogger().logError(err);
} else {
extensionApi.env.createTelemetryLogger().logError(err.toString());
}
}

setTimeout(() => loop(), 5000);
}, 5000);
loop();
}

async function updateProvider(extensionContext: extensionApi.ExtensionContext): Promise<void> {
Expand Down Expand Up @@ -168,14 +167,7 @@ export async function activate(extensionContext: extensionApi.ExtensionContext):
}

// monitor daemon
monitorDaemon(extensionContext).catch((err: unknown) => {
console.error('Error while monitoring docker daemon', err);
if (err instanceof Error) {
extensionApi.env.createTelemetryLogger().logError(err);
} else {
extensionApi.env.createTelemetryLogger().logError(err.toString());
}
});
void monitorDaemon(extensionContext);
Copy link
Collaborator

Choose a reason for hiding this comment

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

hello, here it looks you're bypassing the linter and do not catch error on promises, etc.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

monitorDaemon doesn't return a promise

}

function initProvider(extensionContext: extensionApi.ExtensionContext): void {
Expand Down
132 changes: 56 additions & 76 deletions extensions/podman/src/extension.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ import { compareVersions } from 'compare-versions';

import { getSocketCompatibility } from './compatibility-mode';
import { getDetectionChecks } from './detection-checks';
import { Monitor } from './monitor';
import { PodmanBinaryLocationHelper } from './podman-binary-location-helper';
import { PodmanCleanupMacOS } from './podman-cleanup-macos';
import { PodmanCleanupWindows } from './podman-cleanup-windows';
Expand Down Expand Up @@ -507,10 +508,11 @@ async function initDefaultLinux(provider: extensionApi.Provider): Promise<void>
socketPath,
},
};

monitorPodmanSocket(socketPath).catch((error: unknown) => {
console.error('Error monitoring podman socket', error);
const podmanSocketMonitor = new Monitor(() => monitorPodmanSocket(socketPath), {
stopMonitorPredicate: stopMonitoringPodmanSocket,
name: 'podman socket',
});
podmanSocketMonitor.start();

const disposable = provider.registerContainerProviderConnection(containerProviderConnection);
currentConnections.set('podman', disposable);
Expand Down Expand Up @@ -542,22 +544,11 @@ async function isPodmanSocketAlive(socketPath: string): Promise<boolean> {
}

async function monitorPodmanSocket(socketPath: string, machineName?: string): Promise<void> {
// call us again
if (!stopMonitoringPodmanSocket(machineName)) {
try {
const alive = await isPodmanSocketAlive(socketPath);
if (!alive) {
updateProviderStatus('stopped', machineName);
} else {
updateProviderStatus('started', machineName);
}
} catch (error) {
// ignore the update of machines
}
await timeout(5000);
monitorPodmanSocket(socketPath, machineName).catch((error: unknown) => {
console.error('Error monitoring podman socket', error);
});
const alive = await isPodmanSocketAlive(socketPath);
if (!alive) {
updateProviderStatus('stopped', machineName);
} else {
updateProviderStatus('started', machineName);
}
}

Expand All @@ -582,21 +573,6 @@ async function timeout(time: number): Promise<void> {
});
}

async function monitorMachines(provider: extensionApi.Provider): Promise<void> {
// call us again
if (!stopLoop) {
try {
await updateMachines(provider);
} catch (error) {
// ignore the update of machines
}
await timeout(5000);
monitorMachines(provider).catch((error: unknown) => {
console.error('Error monitoring podman machines', error);
});
}
}

function shouldNotifyQemuMachinesWithV5(installedPodman: InstalledPodman): boolean {
// if on macOS we have some files from qemu it needs to be removed/cleaned
// check if the qemu files are present in ~/.config/containers/podman/machine/qemu or ~/.local/share/containers/podman/machine/qemu
Expand All @@ -612,47 +588,40 @@ function shouldNotifyQemuMachinesWithV5(installedPodman: InstalledPodman): boole
}

async function monitorProvider(provider: extensionApi.Provider): Promise<void> {
// call us again
if (!stopLoop) {
try {
const installedPodman = await getPodmanInstallation();
provider.updateDetectionChecks(getDetectionChecks(installedPodman));

// update version
if (!installedPodman) {
provider.updateStatus('not-installed');
extensionApi.context.setValue('podmanIsNotInstalled', true, 'onboarding');
// if podman is not installed and the OS is linux we show the podman onboarding notification (if it has not been shown earlier)
// this should be limited to Linux as in other OSes the onboarding workflow is enabled based on the podman machine existance
// and the notification is handled by checking the machine
if (isLinux() && shouldNotifySetup) {
// push setup notification
notificationDisposable = extensionApi.window.showNotification(setupPodmanNotification);
shouldNotifySetup = false;
}
} else if (installedPodman.version) {
provider.updateVersion(installedPodman.version);
// update provider status if someone has installed podman externally
if (provider.status === 'not-installed') {
provider.updateStatus('installed');
}
try {
const installedPodman = await getPodmanInstallation();
provider.updateDetectionChecks(getDetectionChecks(installedPodman));

// update version
if (!installedPodman) {
provider.updateStatus('not-installed');
extensionApi.context.setValue('podmanIsNotInstalled', true, 'onboarding');
// if podman is not installed and the OS is linux we show the podman onboarding notification (if it has not been shown earlier)
// this should be limited to Linux as in other OSes the onboarding workflow is enabled based on the podman machine existance
// and the notification is handled by checking the machine
if (isLinux() && shouldNotifySetup) {
// push setup notification
notificationDisposable = extensionApi.window.showNotification(setupPodmanNotification);
shouldNotifySetup = false;
}
} else if (installedPodman.version) {
provider.updateVersion(installedPodman.version);
// update provider status if someone has installed podman externally
if (provider.status === 'not-installed') {
provider.updateStatus('installed');
}

extensionApi.context.setValue('podmanIsNotInstalled', false, 'onboarding');
// if podman has been installed, we reset the notification flag so if podman is uninstalled in future we can show the notification again
if (isLinux()) {
shouldNotifySetup = true;
// notification is no more required
notificationDisposable?.dispose();
}
extensionApi.context.setValue('podmanIsNotInstalled', false, 'onboarding');
// if podman has been installed, we reset the notification flag so if podman is uninstalled in future we can show the notification again
if (isLinux()) {
shouldNotifySetup = true;
// notification is no more required
notificationDisposable?.dispose();
}
} catch (error) {
// ignore the update
}
} catch (error) {
// ignore the update
}
await timeout(8000);
monitorProvider(provider).catch((error: unknown) => {
console.error('Error monitoring podman provider', error);
});
}

function prettyMachineName(machineName: string): string {
Expand Down Expand Up @@ -887,6 +856,7 @@ export function initExtensionContext(extensionContext: extensionApi.ExtensionCon
}

const currentUpdatesDisposables: extensionApi.Disposable[] = [];

export async function initCheckAndRegisterUpdate(
provider: extensionApi.Provider,
podmanInstall: PodmanInstall,
Expand Down Expand Up @@ -1399,15 +1369,25 @@ export async function activate(extensionContext: extensionApi.ExtensionContext):
// Podman Machine support is on macOS, Windows and Linux
// Despite Linux having native container support, Podman Machine is still supported on Linux
// so let's monitor for the machines
monitorMachines(provider).catch((error: unknown) => {
console.error('Error while monitoring machines', error);
});
const machineMonitor = new Monitor(
async () => {
await updateMachines(provider);
},
{
name: 'podman machine',
stopMonitorPredicate: () => stopLoop,
},
);
machineMonitor.start();

// monitor provider
// like version, checks, warnings
monitorProvider(provider).catch((error: unknown) => {
console.error('Error while monitoring provider', error);
const providerMonitor = new Monitor(() => monitorProvider(provider), {
name: 'providers',
intervalMs: 8000,
stopMonitorPredicate: () => stopLoop,
});
providerMonitor.start();

const onboardingCheckInstallationCommand = extensionApi.commands.registerCommand(
'podman.onboarding.checkInstalledCommand',
Expand Down
99 changes: 99 additions & 0 deletions extensions/podman/src/monitor.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,99 @@
/**********************************************************************
* Copyright (C) 2024 Red Hat, Inc.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*
* SPDX-License-Identifier: Apache-2.0
***********************************************************************/
import * as extensionApi from '@podman-desktop/api';
import { afterEach, beforeEach, expect, it, type Mock, vi } from 'vitest';

import { Monitor } from './monitor';

const logErrorTelemetry = vi.fn();

// mock the API
vi.mock('@podman-desktop/api', async () => ({
env: {
createTelemetryLogger: vi.fn(() => ({
logError: logErrorTelemetry,
})),
},
}));

let fakeFn: Mock;
let stopPredicate: Mock;
let monitor: Monitor;
const fakeName = 'testMonitor';

beforeEach(() => {
vi.useFakeTimers();
fakeFn = vi.fn();
stopPredicate = vi.fn().mockReturnValue(false); // Initially don't stop the loop
monitor = new Monitor(fakeFn, {
name: fakeName,
stopMonitorPredicate: stopPredicate,
intervalMs: 1000,
});
});

afterEach(() => {
vi.runOnlyPendingTimers();
vi.useRealTimers();
});

it('Expect monitor to call the function at the specified interval', async () => {
monitor.start();

// Initially, fakeFn should not have been called.
expect(fakeFn).not.toHaveBeenCalled();

// After 1000ms, fakeFn should be called once.
vi.advanceTimersByTime(1000);
expect(fakeFn).toHaveBeenCalledTimes(1);

// Ensure the function is not called again if the stop predicate is true.
stopPredicate.mockReturnValue(true);
vi.advanceTimersByTime(1000);
expect(fakeFn).toHaveBeenCalledTimes(1);
});

it('Expect monitor to stop monitoring when stopLoopPredicate returns true', async () => {
stopPredicate.mockReturnValue(true);
monitor.start();

vi.advanceTimersByTime(1000);
expect(fakeFn).not.toHaveBeenCalled();
});

it('Expect monitor to log an error when fn throws', async () => {
const error = new Error('Test error');
fakeFn.mockRejectedValueOnce(error);
monitor.start();

vi.advanceTimersByTime(1000);
await new Promise(process.nextTick);

expect(extensionApi.env.createTelemetryLogger().logError).toHaveBeenCalledWith(error);
});

it('Expect monitor to log the correct error message for non-Error instances', async () => {
const errorMessage = 'String error';
fakeFn.mockRejectedValueOnce(errorMessage);
monitor.start();

vi.advanceTimersByTime(1000);
await new Promise(process.nextTick);

expect(extensionApi.env.createTelemetryLogger().logError).toHaveBeenCalledWith(errorMessage);
});
Loading