fix: do not try to monitor machine when autostarting it

fixes podman-desktop/podman-desktop#10173 Signed-off-by: Florent Benoit <[email protected]>
podman-desktop · Dec 11, 2024 · 5f8f6c0 · 5f8f6c0
1 parent ac8e419
commit 5f8f6c0
Show file tree

Hide file tree

Showing 2 changed files with 137 additions and 40 deletions.
diff --git a/extensions/podman/packages/extension/src/extension.spec.ts b/extensions/podman/packages/extension/src/extension.spec.ts
@@ -3107,3 +3107,85 @@ describe('connectionAuditor', () => {
     );
   });
 });
+
+// https://github.com/podman-desktop/podman-desktop/issues/10173
+test('activate and autostart should not duplicate machines ', async () => {
+  vi.mocked(isMac).mockReturnValue(true);
+  vi.mocked(extensionApi.env).isWindows = false;
+  vi.mocked(isLinux).mockReturnValue(false);
+  vi.spyOn(PodmanInstall.prototype, 'checkForUpdate').mockResolvedValue({
+    hasUpdate: false,
+  } as unknown as UpdateCheck);
+  const contextMock = {
+    subscriptions: [],
+    secrets: {
+      delete: vi.fn(),
+      get: vi.fn(),
+      onDidChange: vi.fn(),
+      store: vi.fn(),
+    },
+  } as unknown as extensionApi.ExtensionContext;
+
+  // mock getSocketCompatibility
+  const disableMock = vi.fn();
+  const enableMock = vi.fn();
+  const isEnabledMock = vi.fn().mockReturnValue(false);
+  const mock = vi.spyOn(compatibilityModeLib, 'getSocketCompatibility');
+  mock.mockReturnValue({
+    isEnabled: isEnabledMock,
+    enable: enableMock,
+    disable: disableMock,
+    details: '',
+    tooltipText: (): string => {
+      return '';
+    },
+  });
+
+  let podmanMachineListCalls = 0;
+
+  vi.mocked(extensionApi.process.exec).mockImplementation(
+    (_command: string, args?: string[], _options?: extensionApi.RunOptions) =>
+      new Promise<extensionApi.RunResult>(resolve => {
+        if (args?.[0] === '--version') {
+          resolve({
+            stderr: '',
+            stdout: '5.0.0',
+            command: '',
+          });
+          return;
+        }
+
+        if (args?.[0] === 'machine' && args?.[1] === 'list') {
+          podmanMachineListCalls++;
+          resolve({
+            stderr: '',
+            stdout: '[]',
+            command: '',
+          });
+        }
+      }),
+  );
+
+  const api = await extension.activate(contextMock);
+  expect(api).toBeDefined();
+
+  // check that we've registered a autostart provider
+  expect(provider.registerAutostart).toBeCalled();
+  const autoStartMethod = vi.mocked(provider.registerAutostart).mock.calls[0][0] as unknown as {
+    start: () => Promise<void>;
+  };
+
+  // call the autostart method
+  const promiseAutoStart = autoStartMethod?.start();
+
+  // call 100 times monitorMachines
+  for (let i = 0; i < 100; i++) {
+    extension.monitorMachines(provider, podmanConfiguration).catch(() => {});
+  }
+
+  await promiseAutoStart;
+
+  // should be only 1 but we allow some more calls (if there is not a check to check during the autostart it would be 100+ calls)
+  expect(podmanMachineListCalls).toBeLessThan(5);
+  expect(promiseAutoStart).toBeDefined();
+});
diff --git a/extensions/podman/packages/extension/src/extension.ts b/extensions/podman/packages/extension/src/extension.ts
@@ -87,6 +87,8 @@ const containerProviderConnections = new Map<string, extensionApi.ContainerProvi
 let isDisguisedPodmanSocket = true;
 let disguisedPodmanSocketWatcher: extensionApi.FileSystemWatcher | undefined;
 
+let autostartInProgress = false;
+
 // Configuration buttons
 const configurationCompatibilityMode = 'setting.dockerCompatibility';
 let telemetryLogger: extensionApi.TelemetryLogger | undefined;
@@ -669,16 +671,19 @@ async function timeout(time: number): Promise<void> {
   });
 }
 
-async function monitorMachines(
+export async function monitorMachines(
   provider: extensionApi.Provider,
   podmanConfiguration: PodmanConfiguration,
 ): Promise<void> {
   // call us again
   if (!stopLoop) {
-    try {
-      await updateMachines(provider, podmanConfiguration);
-    } catch (error) {
-      // ignore the update of machines
+    // skip the update during the auto start process
+    if (!autostartInProgress) {
+      try {
+        await updateMachines(provider, podmanConfiguration);
+      } catch (error) {
+        // ignore the update of machines
+      }
     }
     await timeout(5000);
     monitorMachines(provider, podmanConfiguration).catch((error: unknown) => {
@@ -1453,45 +1458,55 @@ export async function activate(extensionContext: extensionApi.ExtensionContext):
     extensionContext.subscriptions.push(command);
   }
 
-  provider.registerAutostart({
-    start: async (logger: extensionApi.Logger) => {
-      // If autostart has been enabled for the machine, try to start it.
-      try {
-        await updateMachines(provider, podmanConfiguration);
-      } catch (error) {
-        // ignore the update of machines
-      }
+  const doAutoStart = async (logger: extensionApi.Logger): Promise<void> => {
+    autostartInProgress = true;
+    // If autostart has been enabled for the machine, try to start it.
+    try {
+      await updateMachines(provider, podmanConfiguration);
+    } catch (error) {
+      // ignore the update of machines
+    }
 
-      // do we have a running machine ?
-      const isRunningMachine = Array.from(podmanMachinesStatuses.values()).find(
-        connectionStatus => connectionStatus === 'started' || connectionStatus === 'starting',
-      );
-      if (isRunningMachine) {
-        console.log('Podman extension:', 'Do not start a machine as there is already one starting or started');
-        return;
-      }
+    // do we have a running machine ?
+    const isRunningMachine = Array.from(podmanMachinesStatuses.values()).find(
+      connectionStatus => connectionStatus === 'started' || connectionStatus === 'starting',
+    );
+    if (isRunningMachine) {
+      console.log('Podman extension:', 'Do not start a machine as there is already one starting or started');
+      return;
+    }
 
-      // start the first machine if any
-      const machines = Array.from(podmanMachinesStatuses.entries());
-      if (machines.length > 0) {
-        const [machineName] = machines[0];
-        if (!podmanMachinesInfo.has(machineName)) {
-          console.error('Unable to retrieve machine infos to be autostarted', machineName);
-        } else {
-          console.log('Podman extension:', 'Autostarting machine', machineName);
-          const machineInfo = podmanMachinesInfo.get(machineName);
-          const containerProviderConnection = containerProviderConnections.get(machineName);
-          if (containerProviderConnection && machineInfo) {
-            const context: extensionApi.LifecycleContext = extensionApi.provider.getProviderLifecycleContext(
-              provider.id,
-              containerProviderConnection,
-            );
-            await startMachine(provider, podmanConfiguration, machineInfo, context, logger, undefined, true);
-            autoMachineStarted = true;
-            autoMachineName = machineName;
-          }
+    // start the first machine if any
+    const machines = Array.from(podmanMachinesStatuses.entries());
+    if (machines.length > 0) {
+      const [machineName] = machines[0];
+      if (!podmanMachinesInfo.has(machineName)) {
+        console.error('Unable to retrieve machine infos to be autostarted', machineName);
+      } else {
+        console.log('Podman extension:', 'Autostarting machine', machineName);
+        const machineInfo = podmanMachinesInfo.get(machineName);
+        const containerProviderConnection = containerProviderConnections.get(machineName);
+        if (containerProviderConnection && machineInfo) {
+          const context: extensionApi.LifecycleContext = extensionApi.provider.getProviderLifecycleContext(
+            provider.id,
+            containerProviderConnection,
+          );
+          await startMachine(provider, podmanConfiguration, machineInfo, context, logger, undefined, true);
+          autoMachineStarted = true;
+          autoMachineName = machineName;
         }
       }
+    }
+  };
+
+  provider.registerAutostart({
+    start: async (logger: extensionApi.Logger) => {
+      try {
+        autostartInProgress = true;
+        await doAutoStart(logger);
+      } finally {
+        autostartInProgress = false;
+      }
     },
   });