fix(fleet): image override might contain the commitId tag (#3292)

the entire point of image override in fleet is to be able to create a node running a specific commit (for pre-deployment testing or verification purpose) This commit ensures that an image override with a commitId tag is properly handled tested in staging
NangoHQ · Jan 13, 2025 · fef77b7 · fef77b7
1 parent 503c7fa
commit fef77b7
Show file tree

Hide file tree

Showing 2 changed files with 68 additions and 25 deletions.
diff --git a/packages/fleet/lib/supervisor/supervisor.integration.test.ts b/packages/fleet/lib/supervisor/supervisor.integration.test.ts
@@ -107,22 +107,65 @@ describe('Supervisor', () => {
         const nodeAfter = (await nodes.get(dbClient.db, node.id)).unwrap();
         expect(nodeAfter.state).toBe('OUTDATED');
     });
-    it('should mark nodes with config override as OUTDATED', async () => {
+    it('should mark nodes with resource override as OUTDATED', async () => {
         const node = await createNodeWithAttributes(dbClient.db, { state: 'RUNNING', deploymentId: activeDeployment.id });
-        (
-            await nodeConfigOverrides.create(dbClient.db, {
-                routingId: node.routingId,
-                image: 'new-image',
-                cpuMilli: 10000,
-                memoryMb: 1234,
-                storageMb: 567890
-            })
-        ).unwrap();
+        await nodeConfigOverrides.create(dbClient.db, {
+            routingId: node.routingId,
+            image: node.image,
+            cpuMilli: 10000,
+            memoryMb: 1234,
+            storageMb: 567890
+        });
+
+        await supervisor.tick();
+
+        const nodeAfter = (await nodes.get(dbClient.db, node.id)).unwrap();
+        expect(nodeAfter.state).toBe('OUTDATED');
+
+        await supervisor.tick();
+
+        const newNode = (await nodes.search(dbClient.db, { states: ['PENDING'] })).unwrap().nodes.get(node.routingId)?.PENDING[0];
+        expect(newNode).toMatchObject({
+            state: 'PENDING',
+            routingId: node.routingId,
+            deploymentId: activeDeployment.id,
+            image: node.image,
+            cpuMilli: 10000,
+            memoryMb: 1234,
+            storageMb: 567890,
+            error: null
+        });
+    });
+
+    it('should mark nodes with image override as OUTDATED', async () => {
+        const node = await createNodeWithAttributes(dbClient.db, { state: 'RUNNING', deploymentId: activeDeployment.id });
+        const imageOverride = `${mockNodeProvider.defaultNodeConfig.image}:12345`;
+        await nodeConfigOverrides.create(dbClient.db, {
+            routingId: node.routingId,
+            image: imageOverride,
+            cpuMilli: node.cpuMilli,
+            memoryMb: node.memoryMb,
+            storageMb: node.storageMb
+        });
 
         await supervisor.tick();
 
         const nodeAfter = (await nodes.get(dbClient.db, node.id)).unwrap();
         expect(nodeAfter.state).toBe('OUTDATED');
+
+        await supervisor.tick();
+
+        const newNode = (await nodes.search(dbClient.db, { states: ['PENDING'] })).unwrap().nodes.get(node.routingId)?.PENDING[0];
+        expect(newNode).toMatchObject({
+            state: 'PENDING',
+            routingId: node.routingId,
+            deploymentId: activeDeployment.id,
+            image: imageOverride,
+            cpuMilli: node.cpuMilli,
+            memoryMb: node.memoryMb,
+            storageMb: node.storageMb,
+            error: null
+        });
     });
 
     it('should create new nodes if only OUTDATED', async () => {

diff --git a/packages/fleet/lib/supervisor/supervisor.ts b/packages/fleet/lib/supervisor/supervisor.ts
@@ -9,7 +9,7 @@ import { Err, Ok, retryWithBackoff } from '@nangohq/utils';
 import type { Result } from '@nangohq/utils';
 import { FleetError } from '../utils/errors.js';
 import type { Node, NodeConfigOverride } from '../types.js';
-import type { Deployment, NodeConfig } from '@nangohq/types';
+import type { Deployment } from '@nangohq/types';
 import { setTimeout } from 'node:timers/promises';
 import type { NodeProvider } from '../node-providers/node_provider.js';
 import { envs } from '../env.js';
@@ -182,8 +182,11 @@ export class Supervisor {
                     if (!configOverride) {
                         return false;
                     }
+                    // image override might have a commitId
+                    // so we need to check if the image override with commitId is the same as the node's image
+                    // or if the image override (without commitId) + current deployment commitId is the same as the node's image
                     return !(
-                        node.image === `${configOverride.image}:${deployment.commitId}` &&
+                        (configOverride.image === node.image || `${configOverride.image}:${deployment.commitId}` === node.image) &&
                         node.cpuMilli === configOverride.cpuMilli &&
                         node.memoryMb === configOverride.memoryMb &&
                         node.storageMb === configOverride.storageMb
@@ -320,31 +323,28 @@ export class Supervisor {
         db: Knex,
         {
             routingId,
-            deployment,
-            nodeConfig
+            deployment
         }: {
             type: 'CREATE';
             routingId: Node['routingId'];
             deployment: Deployment;
-            nodeConfig?: NodeConfig | undefined;
         }
     ): Promise<Result<Node>> {
         let newNodeConfig = this.nodeProvider.defaultNodeConfig;
-        if (!nodeConfig) {
-            const nodeConfigOverride = await nodeConfigOverrides.search(db, { routingIds: [routingId] });
-            if (nodeConfigOverride.isErr()) {
-                return Err(nodeConfigOverride.error);
-            }
-            const nodeConfigOverrideValue = nodeConfigOverride.value.get(routingId);
-            if (nodeConfigOverrideValue) {
-                newNodeConfig = nodeConfigOverrideValue;
-            }
+
+        const nodeConfigOverride = await nodeConfigOverrides.search(db, { routingIds: [routingId] });
+        if (nodeConfigOverride.isErr()) {
+            return Err(nodeConfigOverride.error);
+        }
+        const nodeConfigOverrideValue = nodeConfigOverride.value.get(routingId);
+        if (nodeConfigOverrideValue) {
+            newNodeConfig = nodeConfigOverrideValue;
         }
 
         return nodes.create(db, {
             routingId,
             deploymentId: deployment.id,
-            image: `${newNodeConfig.image}:${deployment.commitId}`,
+            image: newNodeConfig.image.includes(':') ? newNodeConfig.image : `${newNodeConfig.image}:${deployment.commitId}`,
             cpuMilli: newNodeConfig.cpuMilli,
             memoryMb: newNodeConfig.memoryMb,
             storageMb: newNodeConfig.storageMb