Skip to content

Commit

Permalink
feat: grab usage data of volumes only on demand
Browse files Browse the repository at this point in the history 
fixes podman-desktop#3376

it is the last PR related to limit the calls to the REST API
here it avoids to call the system/df call that can be expensive
on podman in rootful mode

Signed-off-by: Florent Benoit <[email protected]>
  • Loading branch information
@benoitf @mairin
benoitf authored and mairin committed Aug 29, 2023
1 parent e0ac572 commit fc8f383
Show file tree
Hide file tree
Showing 5 changed files with 79 additions and 46 deletions.
58 changes: 49 additions & 9 deletions packages/renderer/src/lib/volume/VolumesList.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ import { render, screen } from '@testing-library/svelte';
import VolumesList from './VolumesList.svelte';
import { get } from 'svelte/store';
import { providerInfos } from '/@/stores/providers';
import { fetchVolumes, resetInitializationVolumes, volumeListInfos } from '/@/stores/volumes';
import { volumeListInfos, volumesEventStore } from '/@/stores/volumes';

const listVolumesMock = vi.fn();
const getProviderInfosMock = vi.fn();
Expand All @@ -51,6 +51,7 @@ beforeAll(() => {

beforeEach(() => {
vi.resetAllMocks();
vi.clearAllMocks();
onDidUpdateProviderStatusMock.mockImplementation(() => Promise.resolve());
getProviderInfosMock.mockResolvedValue([]);
});
Expand All @@ -63,7 +64,7 @@ async function waitRender(customProperties: object): Promise<void> {
}
}

test('Expect fetching in progress being displayed', async () => {
test('Expect No Container Engine being displayed', async () => {
listVolumesMock.mockResolvedValue([]);
getProviderInfosMock.mockResolvedValue([
{
Expand All @@ -83,9 +84,7 @@ test('Expect fetching in progress being displayed', async () => {
expect(noEngine).toBeInTheDocument();
});

test('Expect no container engines being displayed', async () => {
resetInitializationVolumes();
listVolumesMock.mockResolvedValue([]);
test('Expect volumes being displayed once extensions are started (without size data)', async () => {
getProviderInfosMock.mockResolvedValue([
{
name: 'podman',
Expand All @@ -99,21 +98,57 @@ test('Expect no container engines being displayed', async () => {
],
},
]);

listVolumesMock.mockResolvedValue([
{
Volumes: [
{
Driver: 'local',
Labels: {},
Mountpoint: '/var/lib/containers/storage/volumes/fedora/_data',
Name: '0052074a2ade930338c00aea982a90e4243e6cf58ba920eb411c388630b8c967',
Options: {},
Scope: 'local',
engineName: 'Podman',
engineId: 'podman.Podman Machine',
UsageData: { RefCount: 1, Size: -1 },
containersUsage: [],
},
],
},
]);

window.dispatchEvent(new CustomEvent('extensions-already-started'));
window.dispatchEvent(new CustomEvent('provider-lifecycle-change'));

// ask to fetch the volumes
const volumesEventStoreInfo = volumesEventStore.setup();

await volumesEventStoreInfo.fetch();

// first call is with listing without details
expect(listVolumesMock).toHaveBeenNthCalledWith(1, false);

// wait store are populated
while (get(volumeListInfos).length === 0) {
await new Promise(resolve => setTimeout(resolve, 500));
}
while (get(providerInfos).length === 0) {
await new Promise(resolve => setTimeout(resolve, 500));
}

await waitRender({});

const noEngine = screen.getByRole('heading', { name: 'Fetching volumes...' });
expect(noEngine).toBeInTheDocument();
const volumeName = screen.getByRole('cell', { name: '0052074a2ade' });
// expect size to be N/A
const volumeSize = screen.getByRole('cell', { name: 'N/A' });
expect(volumeName).toBeInTheDocument();
expect(volumeSize).toBeInTheDocument();

expect(volumeName.compareDocumentPosition(volumeSize)).toBe(4);
});

test('Expect volumes being displayed once extensions are started', async () => {
test('Expect volumes being displayed once extensions are started (with size data)', async () => {
getProviderInfosMock.mockResolvedValue([
{
name: 'podman',
Expand Down Expand Up @@ -151,7 +186,12 @@ test('Expect volumes being displayed once extensions are started', async () => {
window.dispatchEvent(new CustomEvent('provider-lifecycle-change'));

// ask to fetch the volumes
await fetchVolumes();
const volumesEventStoreInfo = volumesEventStore.setup();

await volumesEventStoreInfo.fetch('fetchUsage');

// first call is with listing with details
expect(listVolumesMock).toHaveBeenNthCalledWith(1, true);

// wait store are populated
while (get(volumeListInfos).length === 0) {
Expand Down
42 changes: 18 additions & 24 deletions packages/renderer/src/lib/volume/VolumesList.svelte
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import { onDestroy, onMount } from 'svelte';
import { router } from 'tinro';
import type { Unsubscriber } from 'svelte/store';
import type { VolumeInfoUI } from './VolumeInfoUI';
import { fetchVolumes, filtered, searchPattern, volumeListInfos, volumesInitialized } from '../../stores/volumes';
import { fetchVolumesWithData, filtered, searchPattern, volumeListInfos } from '../../stores/volumes';
import { providerInfos } from '../../stores/providers';
import NavPage from '../ui/NavPage.svelte';
import { VolumeUtils } from './volume-utils';
Expand All @@ -16,10 +16,9 @@ import StatusIcon from '../images/StatusIcon.svelte';
import Prune from '../engine/Prune.svelte';
import moment from 'moment';
import type { EngineInfoUI } from '../engine/EngineInfoUI';
import EmptyScreen from '../ui/EmptyScreen.svelte';
import Checkbox from '../ui/Checkbox.svelte';
import Button from '../ui/Button.svelte';
import { faTrash } from '@fortawesome/free-solid-svg-icons';
import { faPieChart, faTrash } from '@fortawesome/free-solid-svg-icons';
let searchTerm = '';
$: searchPattern.set(searchTerm);
Expand All @@ -41,24 +40,8 @@ $: selectedAllCheckboxes = volumes.filter(volume => !volume.inUse).every(volume
const volumeUtils = new VolumeUtils();
let fetchingInProgress = false;
let volumesUnsubscribe: Unsubscriber;
onMount(async () => {
if (!volumesInitialized) {
fetchingInProgress = true;
try {
await fetchVolumes();
} finally {
fetchingInProgress = false;
}
} else {
// fetch in background
fetchVolumes().catch((error: unknown) => {
console.error('unable to fetch the volumes', error);
});
}
volumesUnsubscribe = filtered.subscribe(value => {
const computedVolumes = value
.map(volumeListInfo => volumeListInfo.Volumes)
Expand Down Expand Up @@ -187,12 +170,28 @@ function computeInterval(): number {
// every day
return 60 * 60 * 24 * SECOND;
}
let fetchDataInProgress = false;
async function fetchUsageData() {
fetchDataInProgress = true;
try {
await fetchVolumesWithData();
} finally {
fetchDataInProgress = false;
}
}
</script>

<NavPage bind:searchTerm="{searchTerm}" title="volumes">
<div slot="additional-actions" class="space-x-2 flex flex-nowrap">
{#if $volumeListInfos.map(volumeInfo => volumeInfo.Volumes).flat().length > 0}
<Prune type="volumes" engines="{enginesList}" />

<Button
inProgress="{fetchDataInProgress}"
on:click="{() => fetchUsageData()}"
title="Collect usage data for volumes. It can take a while..."
icon="{faPieChart}">Collect usage data</Button>
{/if}
</div>

Expand Down Expand Up @@ -281,11 +280,6 @@ function computeInterval(): number {

{#if providerConnections.length === 0}
<NoContainerEngineEmptyScreen />
{:else if fetchingInProgress}
<EmptyScreen
icon="{VolumeIcon}"
title="Fetching volumes..."
message="Grabbing volumes from your container engine(s)." />
{:else if $filtered.map(volumeInfo => volumeInfo.Volumes).flat().length === 0}
<VolumeEmptyScreen />
{/if}
Expand Down
4 changes: 4 additions & 0 deletions packages/renderer/src/lib/volume/volume-utils.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,10 @@ export class VolumeUtils {
}

getSize(volumeInfo: VolumeInfo): string {
// handle missing case
if (volumeInfo.UsageData?.Size === -1) {
return 'N/A';
}
if (volumeInfo.UsageData?.Size) {
return `${filesize(volumeInfo.UsageData?.Size, { roundingMethod: 'round' })}`;
}
Expand Down
4 changes: 2 additions & 2 deletions packages/renderer/src/stores/volumes.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ import { get } from 'svelte/store';
import type { Mock } from 'vitest';
import { beforeAll, expect, test, vi } from 'vitest';
import type { VolumeInspectInfo } from '../../../main/src/plugin/api/volume-info';
import { fetchVolumes, volumeListInfos, volumesEventStore } from './volumes';
import { fetchVolumesWithData, volumeListInfos, volumesEventStore } from './volumes';

// first, path window object
const callbacks = new Map<string, any>();
Expand Down Expand Up @@ -70,7 +70,7 @@ test('volumes should be updated in case of a container is removed', async () =>
await callback();

// now ready to fetch volumes
await fetchVolumes();
await fetchVolumesWithData();

// now get list
const volumes = get(volumeListInfos);
Expand Down
17 changes: 6 additions & 11 deletions packages/renderer/src/stores/volumes.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -52,8 +52,10 @@ export async function checkForUpdate(eventName: string): Promise<boolean> {
export const volumeListInfos: Writable<VolumeListInfo[]> = writable([]);

// use helper here as window methods are initialized after the store in tests
const listVolumes = (): Promise<VolumeListInfo[]> => {
return window.listVolumes();
const listVolumes = (...args: unknown[]): Promise<VolumeListInfo[]> => {
const fetchUsage = args?.length > 0 && args[0] === 'fetchUsage';

return window.listVolumes(fetchUsage);
};

export const volumesEventStore = new EventStore<VolumeListInfo[]>(
Expand Down Expand Up @@ -84,13 +86,6 @@ export const filtered = derived([searchPattern, volumeListInfos], ([$searchPatte
});
});

export let volumesInitialized = false;

export const fetchVolumes = async () => {
await volumesEventStoreInfo.fetch();
volumesInitialized = true;
};

export const resetInitializationVolumes = () => {
volumesInitialized = false;
export const fetchVolumesWithData = async () => {
await volumesEventStoreInfo.fetch('fetchUsage');
};

0 comments on commit fc8f383

Please sign in to comment.