Skip to content

Commit

Permalink
feat(validator): active instances validation properly filtered for sh…
Browse files Browse the repository at this point in the history
…utdown instances (#156)

adds unit tests for validator groupHasActiveInstances method
  • Loading branch information
aaronkvanmeerten authored May 21, 2024
1 parent 3c840df commit 9c0ea11
Show file tree
Hide file tree
Showing 3 changed files with 126 additions and 2 deletions.
2 changes: 1 addition & 1 deletion src/app.ts
Original file line number Diff line number Diff line change
Expand Up @@ -287,7 +287,7 @@ const h = new Handlers({
scalingManager,
});

const validator = new Validator({ instanceTracker, instanceGroupManager });
const validator = new Validator({ instanceTracker, instanceGroupManager, metricsLoop });
const loggedPaths = ['/sidecar*', '/groups*'];
app.use(loggedPaths, stats.middleware);
app.use('/', context.injectContext);
Expand Down
105 changes: 105 additions & 0 deletions src/test/validator.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,105 @@
/* eslint-disable @typescript-eslint/ban-ts-comment */
// @ts-nocheck

import assert from 'node:assert';
import test, { afterEach, describe, mock } from 'node:test';

import Validator from '../validator';

describe('Validator', () => {
let context = {
logger: {
info: mock.fn(),
debug: mock.fn(),
error: mock.fn(),
warn: mock.fn(),
},
};

const instanceTracker = {
trimCurrent: mock.fn(),
};

const instanceGroupManager = {
getInstanceGroup: mock.fn(),
};

const metricsLoop = {
getCloudInstances: mock.fn(),
};

const groupName = 'group';

const validator = new Validator({ instanceTracker, instanceGroupManager, metricsLoop });

afterEach(() => {
context = {
logger: {
info: mock.fn(),
debug: mock.fn(),
error: mock.fn(),
warn: mock.fn(),
},
};
});

// these tests are for the groupHasActiveInstances method
describe('validator', () => {
test('should return false for a group with no instances', async () => {
instanceTracker.trimCurrent.mock.mockImplementationOnce(() => []);
metricsLoop.getCloudInstances.mock.mockImplementationOnce(() => []);

const result = await validator.groupHasActiveInstances(context, groupName);
assert.strictEqual(result, false);
});

test('should return true for a group with an instance', async () => {
instanceTracker.trimCurrent.mock.mockImplementationOnce(() => [{ instanceId: '1' }]);
metricsLoop.getCloudInstances.mock.mockImplementationOnce(() => []);

const result = await validator.groupHasActiveInstances(context, groupName);
assert.strictEqual(result, true);
});

test('should return false for a group with an instance, shutdown completed', async () => {
instanceTracker.trimCurrent.mock.mockImplementationOnce(() => [
{ instanceId: '1', shutdownComplete: true },
]);
metricsLoop.getCloudInstances.mock.mockImplementationOnce(() => []);

const result = await validator.groupHasActiveInstances(context, groupName);
assert.strictEqual(result, false);
});

test('should return true for a group with one active and one shutdown instance', async () => {
instanceTracker.trimCurrent.mock.mockImplementationOnce(() => [
{ instanceId: '1' },
{ instanceId: '2', shutdownComplete: new Date().toISOString() },
]);
metricsLoop.getCloudInstances.mock.mockImplementationOnce(() => []);

const result = await validator.groupHasActiveInstances(context, groupName);
assert.strictEqual(result, true);
});

test('should return true for a group with cloud status running', async () => {
instanceTracker.trimCurrent.mock.mockImplementationOnce(() => [{ instanceId: '1' }]);
metricsLoop.getCloudInstances.mock.mockImplementationOnce(() => [
{ instanceId: '1', cloudStatus: 'running' },
]);

const result = await validator.groupHasActiveInstances(context, groupName);
assert.strictEqual(result, true);
});

test('should return false for a group with cloud status shutdown', async () => {
instanceTracker.trimCurrent.mock.mockImplementationOnce(() => [{ instanceId: '1' }]);
metricsLoop.getCloudInstances.mock.mockImplementationOnce(() => [
{ instanceId: '1', cloudStatus: 'shutdown' },
]);

const result = await validator.groupHasActiveInstances(context, groupName);
assert.strictEqual(result, false);
});
});
});
21 changes: 20 additions & 1 deletion src/validator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,11 @@ import { Context } from './context';
import { Request } from 'express';
import InstanceGroupManager, { InstanceGroup } from './instance_group';
import { InstanceGroupDesiredValuesRequest } from './handlers';
import MetricsLoop from './metrics_loop';

export interface ValidatorOptions {
instanceTracker: InstanceTracker;
metricsLoop: MetricsLoop;
instanceGroupManager: InstanceGroupManager;
scaleStatus?: string;
cloudStatus?: string;
Expand All @@ -16,17 +18,34 @@ export interface ValidatorOptions {
export default class Validator {
private instanceTracker: InstanceTracker;
private instanceGroupManager: InstanceGroupManager;
private metricsLoop: MetricsLoop;

constructor(options: ValidatorOptions) {
this.instanceTracker = options.instanceTracker;
this.instanceGroupManager = options.instanceGroupManager;
this.metricsLoop = options.metricsLoop;

this.groupHasActiveInstances = this.groupHasActiveInstances.bind(this);
}

async groupHasActiveInstances(context: Context, name: string): Promise<boolean> {
const instanceStates = await this.instanceTracker.trimCurrent(context, name, false);
return instanceStates.length > 0;
const cloudInstances = await this.metricsLoop.getCloudInstances(name);
const shutdownInstances = cloudInstances
.filter((cv, _) => {
return cv.cloudStatus.toLowerCase() == 'shutdown' || cv.cloudStatus.toLowerCase() == 'terminated';
})
.map((cv, _) => cv.instanceId);

return (
instanceStates.filter((v, _) => {
// skip any that have completed shutdown
if (v.shutdownComplete) return false;

// only include instances that are not listed as SHUTDOWN or TERMINATED
return !shutdownInstances.includes(v.instanceId);
}).length > 0
);
}

groupHasValidDesiredValues(minDesired: number, maxDesired: number, desiredCount: number): boolean {
Expand Down

0 comments on commit 9c0ea11

Please sign in to comment.