From 0962bec13a5e56f83dcd51c6fe42905063895064 Mon Sep 17 00:00:00 2001 From: Marianna Ghirardelli Date: Fri, 10 Sep 2021 11:26:58 -0400 Subject: [PATCH 1/4] fix: no sagemaker autostop lag --- .../__tests__/environment-sc-service.test.js | 59 +++++++++++++++++++ .../service-catalog/environment-sc-service.js | 21 ++++--- .../services/lib/__mocks__/db-service.js | 1 + .../env-status-poll-handler/handler.js | 2 +- 4 files changed, 75 insertions(+), 8 deletions(-) diff --git a/addons/addon-base-raas/packages/base-raas-services/lib/environment/service-catalog/__tests__/environment-sc-service.test.js b/addons/addon-base-raas/packages/base-raas-services/lib/environment/service-catalog/__tests__/environment-sc-service.test.js index b0347a2b58..05a0d2a014 100644 --- a/addons/addon-base-raas/packages/base-raas-services/lib/environment/service-catalog/__tests__/environment-sc-service.test.js +++ b/addons/addon-base-raas/packages/base-raas-services/lib/environment/service-catalog/__tests__/environment-sc-service.test.js @@ -1422,4 +1422,63 @@ describe('EnvironmentSCService', () => { expect(currentIngressRules).toMatchObject(expectedOutcome); expect(securityGroupId).toBeUndefined(); }); + + describe('pollAndSyncSageMakerStatus', () => { + const roleArn = 'roleArn'; + const externalId = 'externalId'; + const requestContext = {}; + + const empty = {}; + + it('should finish updating before returning', async () => { + // BUILD + const sagemakerInstances = { 'notebook-instance-name': {} }; + service.pollSageMakerRealtimeStatus = jest.fn(() => { + return { 'notebook-instance-name': 'InService' }; + }); + service.updateStatus = jest.fn(async () => { + // sleep + await new Promise(r => setTimeout(r, 4000)); + // return some non falsey value + return 'Updated'; + }); + + // OPERATE + const sagemakerUpdated = await service.pollAndSyncSageMakerStatus( + roleArn, + externalId, + sagemakerInstances, + requestContext, + ); + + // CHECK + await expect(sagemakerUpdated).not.toEqual(empty); + }); + + it('should finish update all records that need it before returning', async () => { + // BUILD + const sagemakerInstances = { 'notebook-instance-name': {}, 'notebook-instance-name-1': {} }; + service.pollSageMakerRealtimeStatus = jest.fn(() => { + return { 'notebook-instance-name': 'InService', 'notebook-instance-name-1': 'InService' }; + }); + service.updateStatus = jest.fn(async () => { + // sleep + await new Promise(r => setTimeout(r, 4000)); + // return some non falsey value + return 'Updated'; + }); + + // OPERATE + const sagemakerUpdated = await service.pollAndSyncSageMakerStatus( + roleArn, + externalId, + sagemakerInstances, + requestContext, + ); + + // CHECK + await expect(sagemakerUpdated).not.toEqual(empty); + await expect(Object.keys(sagemakerUpdated).length).toEqual(Object.keys(sagemakerInstances).length); + }); + }); }); diff --git a/addons/addon-base-raas/packages/base-raas-services/lib/environment/service-catalog/environment-sc-service.js b/addons/addon-base-raas/packages/base-raas-services/lib/environment/service-catalog/environment-sc-service.js index 21ea7b1388..251034447b 100644 --- a/addons/addon-base-raas/packages/base-raas-services/lib/environment/service-catalog/environment-sc-service.js +++ b/addons/addon-base-raas/packages/base-raas-services/lib/environment/service-catalog/environment-sc-service.js @@ -104,6 +104,7 @@ class EnvironmentScService extends Service { let envs = await this._scanner({ fields: ['id', 'indexId', 'status', 'outputs'] }) // Verified with EC2 support team that EC2 describe instances API can take 10K instanceIds without issue .limit(10000) + .strong() .scan(); envs = _.filter( envs, @@ -226,13 +227,19 @@ class EnvironmentScService extends Service { }; const sagemakerRealtimeStatus = await this.pollSageMakerRealtimeStatus(roleArn, externalId); const sagemakerUpdated = {}; - _.forEach(sagemakerInstances, async (existingEnvRecord, key) => { - const expectedDDBStatus = SageMakerStatusMap[sagemakerRealtimeStatus[key]]; - const updateStatusResult = await this.updateStatus(requestContext, existingEnvRecord, expectedDDBStatus); - if (updateStatusResult) { - sagemakerUpdated[key] = updateStatusResult; - } - }); + + const sagemakerKeys = Object.keys(sagemakerInstances); + await Promise.all( + sagemakerKeys.map(async key => { + const existingEnvRecord = sagemakerInstances[key]; + const expectedDDBStatus = SageMakerStatusMap[sagemakerRealtimeStatus[key]]; + const updateStatusResult = await this.updateStatus(requestContext, existingEnvRecord, expectedDDBStatus); + if (updateStatusResult) { + sagemakerUpdated[key] = updateStatusResult; + } + }), + ); + return sagemakerUpdated; } diff --git a/addons/addon-base/packages/services/lib/__mocks__/db-service.js b/addons/addon-base/packages/services/lib/__mocks__/db-service.js index e0763f4270..88dac5dc88 100644 --- a/addons/addon-base/packages/services/lib/__mocks__/db-service.js +++ b/addons/addon-base/packages/services/lib/__mocks__/db-service.js @@ -59,6 +59,7 @@ class DbService extends Service { add: jest.fn().mockReturnThis(), values: jest.fn().mockReturnThis(), return: jest.fn().mockReturnThis(), + strong: jest.fn().mockReturnThis(), // Following functions are actual calls to dynamo scan: jest.fn(), diff --git a/main/solution/backend/src/lambdas/env-status-poll-handler/handler.js b/main/solution/backend/src/lambdas/env-status-poll-handler/handler.js index 31250d2d3b..8f71655836 100644 --- a/main/solution/backend/src/lambdas/env-status-poll-handler/handler.js +++ b/main/solution/backend/src/lambdas/env-status-poll-handler/handler.js @@ -27,7 +27,7 @@ const handler = async () => { const environmentScService = await container.find('environmentScService'); const userContext = getSystemRequestContext(); const updatedEnvData = await environmentScService.pollAndSyncWsStatus(userContext); - return { statusCode: 200, body: updatedEnvData }; + return { statusCode: 200, body: JSON.stringify(updatedEnvData) }; }; // eslint-disable-next-line import/prefer-default-export From 2e6e0c556236db317adb9cb1fe794715261788e3 Mon Sep 17 00:00:00 2001 From: Marianna Ghirardelli Date: Fri, 10 Sep 2021 12:35:52 -0400 Subject: [PATCH 2/4] bonus fix: reduce lag for EC2 status update --- .../__tests__/environment-sc-service.test.js | 49 +++++++++++++++++++ .../service-catalog/environment-sc-service.js | 36 ++++++++------ 2 files changed, 70 insertions(+), 15 deletions(-) diff --git a/addons/addon-base-raas/packages/base-raas-services/lib/environment/service-catalog/__tests__/environment-sc-service.test.js b/addons/addon-base-raas/packages/base-raas-services/lib/environment/service-catalog/__tests__/environment-sc-service.test.js index 05a0d2a014..2108904ca8 100644 --- a/addons/addon-base-raas/packages/base-raas-services/lib/environment/service-catalog/__tests__/environment-sc-service.test.js +++ b/addons/addon-base-raas/packages/base-raas-services/lib/environment/service-catalog/__tests__/environment-sc-service.test.js @@ -1481,4 +1481,53 @@ describe('EnvironmentSCService', () => { await expect(Object.keys(sagemakerUpdated).length).toEqual(Object.keys(sagemakerInstances).length); }); }); + + describe('pollAndSyncEC2Status', () => { + const roleArn = 'roleArn'; + const externalId = 'externalId'; + const requestContext = {}; + + const empty = {}; + + it('should finish updating before returning', async () => { + // BUILD + const ec2Instances = { 'instance-name': {} }; + service.pollEc2RealtimeStatus = jest.fn(() => { + return { 'instance-name': 'running' }; + }); + service.updateStatus = jest.fn(async () => { + // sleep + await new Promise(r => setTimeout(r, 4000)); + // return some non falsey value + return 'Updated'; + }); + + // OPERATE + const ec2Updated = await service.pollAndSyncEc2Status(roleArn, externalId, ec2Instances, requestContext); + + // CHECK + await expect(ec2Updated).not.toEqual(empty); + }); + + it('should finish update all records that need it before returning', async () => { + // BUILD + const ec2Instances = { 'instance-name': {}, 'instance-name-1': {} }; + service.pollEc2RealtimeStatus = jest.fn(() => { + return { 'instance-name': 'running', 'instance-name-1': 'running' }; + }); + service.updateStatus = jest.fn(async () => { + // sleep + await new Promise(r => setTimeout(r, 4000)); + // return some non falsey value + return 'Updated'; + }); + + // OPERATE + const ec2Updated = await service.pollAndSyncEc2Status(roleArn, externalId, ec2Instances, requestContext); + + // CHECK + await expect(ec2Updated).not.toEqual(empty); + await expect(Object.keys(ec2Updated).length).toEqual(Object.keys(ec2Instances).length); + }); + }); }); diff --git a/addons/addon-base-raas/packages/base-raas-services/lib/environment/service-catalog/environment-sc-service.js b/addons/addon-base-raas/packages/base-raas-services/lib/environment/service-catalog/environment-sc-service.js index 251034447b..40cc801b47 100644 --- a/addons/addon-base-raas/packages/base-raas-services/lib/environment/service-catalog/environment-sc-service.js +++ b/addons/addon-base-raas/packages/base-raas-services/lib/environment/service-catalog/environment-sc-service.js @@ -184,14 +184,9 @@ class EnvironmentScService extends Service { 'terminated': 'TERMINATED', }; const ec2RealtimeStatus = await this.pollEc2RealtimeStatus(roleArn, externalId, ec2Instances); - const ec2Updated = {}; - _.forEach(ec2Instances, async (existingEnvRecord, ec2InstanceId) => { - const expectedDDBStatus = EC2StatusMap[ec2RealtimeStatus[ec2InstanceId]]; - const updateStatusResult = await this.updateStatus(requestContext, existingEnvRecord, expectedDDBStatus); - if (updateStatusResult) { - ec2Updated[ec2InstanceId] = updateStatusResult; - } - }); + + const ec2Updated = await this.updateAllStatuses(ec2Instances, EC2StatusMap, ec2RealtimeStatus, requestContext); + return ec2Updated; } @@ -226,21 +221,32 @@ class EnvironmentScService extends Service { Failed: 'FAILED', }; const sagemakerRealtimeStatus = await this.pollSageMakerRealtimeStatus(roleArn, externalId); - const sagemakerUpdated = {}; - const sagemakerKeys = Object.keys(sagemakerInstances); + const sagemakerUpdated = await this.updateAllStatuses( + sagemakerInstances, + SageMakerStatusMap, + sagemakerRealtimeStatus, + requestContext, + ); + + return sagemakerUpdated; + } + + async updateAllStatuses(instancesList, statusMap, realtimeStatus, requestContext) { + const updated = {}; + const keys = Object.keys(instancesList); await Promise.all( - sagemakerKeys.map(async key => { - const existingEnvRecord = sagemakerInstances[key]; - const expectedDDBStatus = SageMakerStatusMap[sagemakerRealtimeStatus[key]]; + keys.map(async key => { + const existingEnvRecord = instancesList[key]; + const expectedDDBStatus = statusMap[realtimeStatus[key]]; const updateStatusResult = await this.updateStatus(requestContext, existingEnvRecord, expectedDDBStatus); if (updateStatusResult) { - sagemakerUpdated[key] = updateStatusResult; + updated[key] = updateStatusResult; } }), ); - return sagemakerUpdated; + return updated; } async pollSageMakerRealtimeStatus(roleArn, externalId) { From fa2873fdae0fb16128cdea6a33221d0fde85fdd0 Mon Sep 17 00:00:00 2001 From: Marianna Ghirardelli Date: Fri, 10 Sep 2021 14:10:58 -0400 Subject: [PATCH 3/4] fix tests per comments --- .../__tests__/environment-sc-service.test.js | 23 +++++++++---------- 1 file changed, 11 insertions(+), 12 deletions(-) diff --git a/addons/addon-base-raas/packages/base-raas-services/lib/environment/service-catalog/__tests__/environment-sc-service.test.js b/addons/addon-base-raas/packages/base-raas-services/lib/environment/service-catalog/__tests__/environment-sc-service.test.js index 2108904ca8..d000632e2e 100644 --- a/addons/addon-base-raas/packages/base-raas-services/lib/environment/service-catalog/__tests__/environment-sc-service.test.js +++ b/addons/addon-base-raas/packages/base-raas-services/lib/environment/service-catalog/__tests__/environment-sc-service.test.js @@ -1423,13 +1423,11 @@ describe('EnvironmentSCService', () => { expect(securityGroupId).toBeUndefined(); }); - describe('pollAndSyncSageMakerStatus', () => { + describe('pollAndSyncSageMakerStatus function', () => { const roleArn = 'roleArn'; const externalId = 'externalId'; const requestContext = {}; - const empty = {}; - it('should finish updating before returning', async () => { // BUILD const sagemakerInstances = { 'notebook-instance-name': {} }; @@ -1452,7 +1450,7 @@ describe('EnvironmentSCService', () => { ); // CHECK - await expect(sagemakerUpdated).not.toEqual(empty); + await expect(sagemakerUpdated).toEqual({ 'notebook-instance-name': 'Updated' }); }); it('should finish update all records that need it before returning', async () => { @@ -1463,7 +1461,7 @@ describe('EnvironmentSCService', () => { }); service.updateStatus = jest.fn(async () => { // sleep - await new Promise(r => setTimeout(r, 4000)); + await new Promise(r => setTimeout(r, 1000)); // return some non falsey value return 'Updated'; }); @@ -1477,18 +1475,19 @@ describe('EnvironmentSCService', () => { ); // CHECK - await expect(sagemakerUpdated).not.toEqual(empty); + await expect(sagemakerUpdated).toEqual({ + 'notebook-instance-name': 'Updated', + 'notebook-instance-name-1': 'Updated', + }); await expect(Object.keys(sagemakerUpdated).length).toEqual(Object.keys(sagemakerInstances).length); }); }); - describe('pollAndSyncEC2Status', () => { + describe('pollAndSyncEC2Status function', () => { const roleArn = 'roleArn'; const externalId = 'externalId'; const requestContext = {}; - const empty = {}; - it('should finish updating before returning', async () => { // BUILD const ec2Instances = { 'instance-name': {} }; @@ -1506,7 +1505,7 @@ describe('EnvironmentSCService', () => { const ec2Updated = await service.pollAndSyncEc2Status(roleArn, externalId, ec2Instances, requestContext); // CHECK - await expect(ec2Updated).not.toEqual(empty); + await expect(ec2Updated).toEqual({ 'instance-name': 'Updated' }); }); it('should finish update all records that need it before returning', async () => { @@ -1517,7 +1516,7 @@ describe('EnvironmentSCService', () => { }); service.updateStatus = jest.fn(async () => { // sleep - await new Promise(r => setTimeout(r, 4000)); + await new Promise(r => setTimeout(r, 1000)); // return some non falsey value return 'Updated'; }); @@ -1526,7 +1525,7 @@ describe('EnvironmentSCService', () => { const ec2Updated = await service.pollAndSyncEc2Status(roleArn, externalId, ec2Instances, requestContext); // CHECK - await expect(ec2Updated).not.toEqual(empty); + await expect(ec2Updated).toEqual({ 'instance-name': 'Updated', 'instance-name-1': 'Updated' }); await expect(Object.keys(ec2Updated).length).toEqual(Object.keys(ec2Instances).length); }); }); From 2fc21b310c570f40cc25769b11ade7ecf5969a81 Mon Sep 17 00:00:00 2001 From: Marianna Ghirardelli Date: Fri, 10 Sep 2021 14:23:18 -0400 Subject: [PATCH 4/4] adjust all times --- .../service-catalog/__tests__/environment-sc-service.test.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/addons/addon-base-raas/packages/base-raas-services/lib/environment/service-catalog/__tests__/environment-sc-service.test.js b/addons/addon-base-raas/packages/base-raas-services/lib/environment/service-catalog/__tests__/environment-sc-service.test.js index d000632e2e..29b4314bca 100644 --- a/addons/addon-base-raas/packages/base-raas-services/lib/environment/service-catalog/__tests__/environment-sc-service.test.js +++ b/addons/addon-base-raas/packages/base-raas-services/lib/environment/service-catalog/__tests__/environment-sc-service.test.js @@ -1436,7 +1436,7 @@ describe('EnvironmentSCService', () => { }); service.updateStatus = jest.fn(async () => { // sleep - await new Promise(r => setTimeout(r, 4000)); + await new Promise(r => setTimeout(r, 1000)); // return some non falsey value return 'Updated'; }); @@ -1496,7 +1496,7 @@ describe('EnvironmentSCService', () => { }); service.updateStatus = jest.fn(async () => { // sleep - await new Promise(r => setTimeout(r, 4000)); + await new Promise(r => setTimeout(r, 1000)); // return some non falsey value return 'Updated'; });