fix(admin): dedupe instances, treat 409 already-suspended as idempotent skip

packages/cli/src/admin/cmd.test.ts

@@ -0,0 +1,100 @@
+import { FetchError } from '@osaas/client-core';
+import { Command } from 'commander';
+import cmdAdmin from './cmd';
+import { getInstancesToRemove, suspendInstanceForTenant } from './instance';
+import { getTenantPlanMap, getTenantTokenCounts } from './money';
+
+jest.mock('./instance', () => ({
+  getInstancesToRemove: jest.fn(),
+  listInstancesForTenant: jest.fn(),
+  removeInstanceForTenant: jest.fn(),
+  suspendInstanceForTenant: jest.fn()
+}));
+jest.mock('./money', () => ({
+  getTenantPlanMap: jest.fn(),
+  getTenantTokenCounts: jest.fn()
+}));
+
+const mockInstances = getInstancesToRemove as jest.Mock;
+const mockSuspend = suspendInstanceForTenant as jest.Mock;
+
+async function run() {
+  const cli = new Command();
+  cli.addCommand(cmdAdmin());
+  await cli.parseAsync([
+    'node',
+    'cli',
+    'admin',
+    'remove-exceeding-tenant-instances',
+    '--apply'
+  ]);
+}
+
+describe('remove-exceeding-tenant-instances', () => {
+  let logSpy: jest.SpyInstance;
+
+  beforeEach(() => {
+    jest.resetAllMocks();
+    process.exitCode = undefined;
+    logSpy = jest.spyOn(console, 'log').mockImplementation(() => undefined);
+    (getTenantPlanMap as jest.Mock).mockResolvedValue({
+      t1: { planType: 'FREE' }
+    });
+    (getTenantTokenCounts as jest.Mock).mockResolvedValue([
+      { tenantId: 't1', remainingTokens: -500 }
+    ]);
+    mockInstances.mockResolvedValue([{ serviceId: 's', instance: 'i' }]);
+  });
+
+  afterEach(() => {
+    logSpy.mockRestore();
+    process.exitCode = undefined;
+  });
+
+  const summary = () =>
+    logSpy.mock.calls
+      .map((c) => String(c[0]))
+      .find((l) => /tenants below/.test(l));
+
+  it('suspends on 200', async () => {
+    mockSuspend.mockResolvedValue(undefined);
+    await run();
+    expect(mockSuspend).toHaveBeenCalledTimes(1);
+    expect(summary()).toContain('suspended 1 instances, 0 instance failures');
+    expect(summary()).toContain(', 0 already suspended');
+    expect(process.exitCode).toBeUndefined();
+  });
+
+  it('treats 409 already suspended as an idempotent skip', async () => {
+    mockSuspend.mockRejectedValue(
+      new FetchError({
+        message: 'Instance is already suspended',
+        httpCode: 409
+      })
+    );
+    await run();
+    expect(summary()).toContain('suspended 0 instances, 0 instance failures');
+    expect(summary()).toContain(', 1 already suspended');
+    expect(process.exitCode).toBeUndefined();
+  });
+
+  it.each([
+    'Instance suspension state could not be verified',
+    'Instance is running under a stale suspension record and was not re-suspended'
+  ])('treats other 409 reason as failure: %s', async (message) => {
+    mockSuspend.mockRejectedValue(new FetchError({ message, httpCode: 409 }));
+    await run();
+    expect(summary()).toContain('1 instance failures');
+    expect(summary()).toContain(', 0 already suspended');
+    expect(process.exitCode).toBe(1);
+  });
+
+  it('treats 500 as failure', async () => {
+    mockSuspend.mockRejectedValue(
+      new FetchError({ message: 'boom', httpCode: 500 })
+    );
+    await run();
+    expect(summary()).toContain('1 instance failures');
+    expect(process.exitCode).toBe(1);
+  });
+});

packages/cli/src/admin/cmd.ts

@@ -1,6 +1,7 @@
 import { Command } from 'commander';
 import { generatePat } from './token';
 import {
+  FetchError,
   Log,
   Platform,
   getOrderIdByName,
@@ -27,6 +28,8 @@ import {
 } from './subscription';
 import { getTenantPlanMap, getTenantTokenCounts } from './money';
 
+const ALREADY_SUSPENDED_REASON = 'Instance is already suspended';
+
 const delay = (ms: number) => new Promise((res) => setTimeout(res, ms));
 
 export default function cmdAdmin() {
@@ -104,6 +107,7 @@ export default function cmdAdmin() {
         );
         let instancesSuspended = 0;
         let instancesFailed = 0;
+        let instancesAlreadySuspended = 0;
         let tenantsFailed = 0;
         const tenantsExceedingCount = tenantsExceeding.length;
         for (const tenant of tenantsExceeding) {
@@ -131,6 +135,17 @@ export default function cmdAdmin() {
                       instancesSuspended++;
                       console.log('Suspended');
                     } catch (err) {
+                      if (
+                        err instanceof FetchError &&
+                        err.httpCode === 409 &&
+                        err.message === ALREADY_SUSPENDED_REASON
+                      ) {
+                        instancesAlreadySuspended++;
+                        console.log(
+                          `Already suspended ${item.serviceId}: ${item.instance}, skipping`
+                        );
+                        continue;
+                      }
                       instancesFailed++;
                       console.log(
                         `Failed to suspend ${item.serviceId}: ${
@@ -157,7 +172,7 @@ export default function cmdAdmin() {
         console.log(
           `${tenantsExceedingCount} / ${
             Object.keys(tenantPlanMap).length
-          } tenants below threshold, suspended ${instancesSuspended} instances, ${instancesFailed} instance failures, ${tenantsFailed} tenant failures`
+          } tenants below threshold, suspended ${instancesSuspended} instances, ${instancesFailed} instance failures, ${tenantsFailed} tenant failures, ${instancesAlreadySuspended} already suspended`
         );
         if (instancesFailed > 0 || tenantsFailed > 0) {
           process.exitCode = 1;

packages/cli/src/admin/instance.test.ts

@@ -0,0 +1,25 @@
+import { createFetch } from '@osaas/client-core';
+import { getInstancesToRemove } from './instance';
+
+jest.mock('@osaas/client-core', () => ({
+  ...jest.requireActual('@osaas/client-core'),
+  createFetch: jest.fn()
+}));
+jest.mock('./token', () => ({ generatePat: jest.fn(), apiKey: () => 'k' }));
+
+describe('getInstancesToRemove', () => {
+  it('dedupes pods of the same instance', async () => {
+    (createFetch as jest.Mock).mockResolvedValue([
+      { tenantId: 't', serviceId: 's', instanceName: 'a', created: '' },
+      { tenantId: 't', serviceId: 's', instanceName: 'a', created: '' },
+      { tenantId: 't', serviceId: 's', instanceName: 'b', created: '' },
+      { tenantId: 't', serviceId: 'x', instanceName: 'a', created: '' }
+    ]);
+    const res = await getInstancesToRemove('t', 'prod');
+    expect(res).toEqual([
+      { serviceId: 's', instance: 'a' },
+      { serviceId: 's', instance: 'b' },
+      { serviceId: 'x', instance: 'a' }
+    ]);
+  });
+});

packages/cli/src/admin/instance.ts

@@ -72,8 +72,14 @@ export async function getInstancesToRemove(
       Authorization: `Bearer ${apiKey()}`
     }
   });
-  return instances.map(({ serviceId, instanceName }) => ({
-    serviceId,
-    instance: instanceName
-  }));
+  // deploy-manager lists every pod of an instance separately, so dedupe
+  const seen = new Set<string>();
+  const unique: { serviceId: string; instance: string }[] = [];
+  for (const { serviceId, instanceName } of instances) {
+    const key = JSON.stringify([serviceId, instanceName]);
+    if (seen.has(key)) continue;
+    seen.add(key);
+    unique.push({ serviceId, instance: instanceName });
+  }
+  return unique;
 }