Merge pull request #6832 from mook-as/bats/extensions/wait-for-manager

BATS: Wait for extension manager

background.ts

@@ -552,9 +552,9 @@ async function startK8sManager() {
   }
   await k8smanager.start(cfg);
 
-  const getEM = (await import('@pkg/main/extensions/manager')).default;
+  const { initializeExtensionManager } = await import('@pkg/main/extensions/manager');
 
-  await getEM(k8smanager.containerEngineClient, cfg);
+  await initializeExtensionManager(k8smanager.containerEngineClient, cfg);
   window.send('extensions/changed');
 }
 
@@ -1493,25 +1493,28 @@ class BackgroundCommandWorker implements CommandWorkerInterface {
 
   async listExtensions() {
     const extensionManager = await getExtensionManager();
-    const extensions = await extensionManager?.getInstalledExtensions() ?? [];
+
+    if (!extensionManager) {
+      return undefined;
+    }
+    const extensions = await extensionManager.getInstalledExtensions();
     const entries = await Promise.all(extensions.map(async x => [x.id, {
       version:  x.version,
       metadata: await x.metadata,
       labels:   await x.labels,
     }] as const));
 
-    return Promise.resolve(Object.fromEntries(entries));
+    return Object.fromEntries(entries);
   }
 
   async installExtension(image: string, state: 'install' | 'uninstall'): Promise<{status: number, data?: any}> {
     const em = await getExtensionManager();
-    const extension = await em?.getExtension(image, { preferInstalled: state === 'uninstall' });
 
-    if (!extension) {
-      console.debug(`Failed to install extension ${ image }: could not get extension.`);
-
-      return { status: 503 };
+    if (!em) {
+      return { status: 503, data: 'Extension manager is not ready yet.' };
     }
+    const extension = await em.getExtension(image, { preferInstalled: state === 'uninstall' });
+
     if (state === 'install') {
       console.debug(`Installing extension ${ image }...`);
       try {

bats/tests/extensions/allow-list.bats

@@ -55,6 +55,7 @@ check_extension_installed() { # refute, name
 }
 
 @test 'when no extension allow list is set up, all extensions can install' {
+    wait_for_extension_manager
     write_allow_list ''
     rdctl extension install rd/extension/basic
     check_extension_installed

bats/tests/extensions/containers.bats

@@ -41,6 +41,7 @@ encoded_id() { # variant
 }
 
 @test 'no extensions installed' {
+    wait_for_extension_manager
     run rdctl api /v1/extensions
     assert_success
     assert_output $'\x7b'$'\x7d' # empty JSON dict, {}

bats/tests/extensions/install.bats

@@ -38,6 +38,7 @@ encoded_id() { # variant
 @test 'start container engine' {
     RD_ENV_EXTENSIONS=1 start_container_engine
     wait_for_container_engine
+    wait_for_extension_manager
 }
 
 @test 'no extensions installed' {

bats/tests/helpers/load.bash

@@ -89,6 +89,10 @@ setup_file() {
     if semver_gt 5.0.0 "$(semver "$BASH_VERSION")"; then
         fail "Bash 5.0.0 is required; you have $BASH_VERSION"
     fi
+    # We currently use a submodule that provides BATS 1.10; we do not test
+    # against any other copy of BATS (and therefore only support the version in
+    # that submodule).
+    bats_require_minimum_version 1.10.0
     # Ideally this should be printed only when using the tap formatter,
     # but I don't see a way to check for this.
     echo "# ===== $RD_TEST_FILENAME =====" >&3

bats/tests/helpers/vm.bash

@@ -378,6 +378,23 @@ wait_for_container_engine() {
     try --max 12 --delay 10 get_container_engine_info
 }
 
+# Wait fot the extension manager to be initialized.
+wait_for_extension_manager() {
+    trace "waiting for extension manager to be ready"
+    # We want to match specific error strings, so we can't use try() directly.
+    local count=0 max=30 message
+    while true; do
+        run --separate-stderr rdctl api /extensions
+        if ((status == 0 || ++count >= max)); then
+            break
+        fi
+        message=$(jq_output .message)
+        output="$message" assert_output "503 Service Unavailable"
+        sleep 10
+    done
+    trace "$count/$max tries: wait_for_extension_manager"
+}
+
 # See definition of `State` in
 # pkg/rancher-desktop/backend/backend.ts for an explanation of each state.
 assert_backend_available() {

bats/tests/k8s/traefik.bats

@@ -6,7 +6,6 @@ local_setup() {
         skip "Test does not yet work from inside a WSL distro when using networking tunnel, since it requires WSL integration"
     fi
     needs_port 80
-    bats_require_minimum_version 1.5.0
 }
 
 assert_traefik_pods_are_down() {

bats/tests/k8s/wasm.bats

@@ -16,7 +16,6 @@ assert_traefik_crd_established() {
 # Get Kubernetes RuntimeClasses; sets $output to the JSON list.
 get_runtime_classes() {
     # kubectl may emit warnings here; ensure that we don't fall over.
-    bats_require_minimum_version 1.5.0
     run --separate-stderr kubectl get RuntimeClasses --output json
     assert_success || return
 

pkg/rancher-desktop/assets/specs/command-api.yaml

@@ -141,6 +141,10 @@ paths:
                       type: object
                       additionalProperties:
                         type: string
+        '503':
+          description: >-
+            The extension manager has not been loaded yet.  The client should
+            retry the request at some future point in time.
 
   /v1/extensions/install:
     post:

pkg/rancher-desktop/main/commandServer/httpCommandServer.ts

@@ -698,7 +698,11 @@ export class HttpCommandServer {
   protected async listExtensions(request: express.Request, response: express.Response, context: commandContext): Promise<void> {
     const extensions = await this.commandWorker.listExtensions();
 
-    response.status(200).type('json').send(extensions);
+    if (!extensions) {
+      response.status(503).type('txt').send('Extension manager is not ready yet.');
+    } else {
+      response.status(200).type('json').send(extensions);
+    }
   }
 
   protected async installExtension(request: express.Request, response: express.Response, context: commandContext): Promise<void> {
@@ -898,8 +902,11 @@ export interface CommandWorkerInterface {
   setBackendState: (state: BackendState) => Promise<void>;
 
   // #region extensions
-  /** List the installed extensions with their versions */
-  listExtensions(): Promise<Record<string, {version: string, metadata: ExtensionMetadata, labels: Record<string, string>}>>;
+  /**
+   * List the installed extensions with their versions.
+   * If the extension manager is not ready, returns undefined.
+   */
+  listExtensions(): Promise<Record<string, {version: string, metadata: ExtensionMetadata, labels: Record<string, string>}> | undefined>;
   /**
    * Install or uninstall the given extension, returning an appropriate HTTP status code.
    * @param state Whether to install or uninstall the extension.

pkg/rancher-desktop/main/extensions/index.ts

@@ -1,31 +1 @@
-import { ExtensionManager } from './types';
-
-import { ContainerEngineClient } from '@pkg/backend/containerClient';
-import type { Settings } from '@pkg/config/settings';
-import { RecursiveReadonly } from '@pkg/utils/typeUtils';
-
 export * from './types';
-
-/**
- * Get the extension manager for the given client using the given settings.
- * If the client is not given, return the previously fetched extension manager.
- * It is an error to call this without a client if not previously called with
- * one.
- */
-export async function getExtensionManager(): Promise<ExtensionManager | undefined>;
-export async function getExtensionManager(client: ContainerEngineClient, cfg: RecursiveReadonly<Settings>): Promise<ExtensionManager>;
-export async function getExtensionManager(client?: ContainerEngineClient, cfg?: RecursiveReadonly<Settings>): Promise<ExtensionManager | undefined> {
-  // We do a local import here to ensure we don't pull in everything when this
-  // is just imported for the types.
-  const getEMImpl = (await import('./manager')).default;
-
-  if (client) {
-    if (!cfg) {
-      throw new Error(`getExtensionManager called without configuration`);
-    }
-
-    return getEMImpl(client, cfg);
-  } else {
-    return getEMImpl();
-  }
-}

pkg/rancher-desktop/main/extensions/manager.ts

@@ -576,19 +576,14 @@ export class ExtensionManagerImpl implements ExtensionManager {
   }
 }
 
-async function getExtensionManager(): Promise<ExtensionManager | undefined>;
-async function getExtensionManager(client: ContainerEngineClient, cfg: RecursiveReadonly<Settings>): Promise<ExtensionManager>;
-async function getExtensionManager(client?: ContainerEngineClient, cfg?: RecursiveReadonly<Settings>): Promise<ExtensionManager | undefined> {
-  if (!client || manager?.client === client) {
-    if (!client && !manager) {
-      console.debug(`Warning: cached client missing, returning nothing`);
-    }
-
-    return manager;
-  }
+function getExtensionManager(): Promise<ExtensionManager | undefined> {
+  return Promise.resolve(manager);
+}
 
-  if (!cfg) {
-    throw new Error(`getExtensionManager called without configuration`);
+export async function initializeExtensionManager(client: ContainerEngineClient, cfg: RecursiveReadonly<Settings>): Promise<void> {
+  if (manager?.client === client) {
+    // The manager is already the correct one; do nothing.
+    return;
   }
 
   await manager?.shutdown();
@@ -597,8 +592,6 @@ async function getExtensionManager(client?: ContainerEngineClient, cfg?: Recursi
   manager = new ExtensionManagerImpl(client, cfg.containerEngine.name === ContainerEngine.CONTAINERD);
 
   await manager.init(cfg);
-
-  return manager;
 }
 
 export default getExtensionManager;