Skip to content

Commit

Permalink
fix: Allow re-provisioning of recently de-provisioned resources (#825)
Browse files Browse the repository at this point in the history 
Provisioning a recently de-provisioned Data Layer would silently fail.
This is due to the fact that de/provisioning tasks are stored in-memory,
keyed by a hash of the config. So if a provisioning task was recently
completed, attempting to re-provision would return that same task.

This PR keys by random UUIDs, instead of hashes, so we can trigger
multiple provisioning jobs for a given Indexer/Data Layer, allowing for
the Provision > De-provision > Provision flow. To protect against
re-provisioning an _existing_ Data Layer, we only start the task after
verifying it doesn't already exist. Also removed cache from
`Provisioner` to ensure we are getting accurate results.
  • Loading branch information
@morgsmccauley
morgsmccauley authored Jun 21, 2024
1 parent 8f7326b commit 7303f7a
Show file tree
Hide file tree
Showing 4 changed files with 57 additions and 102 deletions.
10 changes: 3 additions & 7 deletions runner/src/provisioner/provisioner.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -173,24 +173,21 @@ describe('Provisioner', () => {
it('returns false if datasource doesnt exists', async () => {
hasuraClient.doesSourceExist = jest.fn().mockReturnValueOnce(false);

await expect(provisioner.fetchUserApiProvisioningStatus(indexerConfig)).resolves.toBe(false);
expect(provisioner.isUserApiProvisioned(indexerConfig.accountId, indexerConfig.functionName)).toBe(false);
await expect(provisioner.isProvisioned(indexerConfig)).resolves.toBe(false);
});

it('returns false if datasource and schema dont exists', async () => {
hasuraClient.doesSourceExist = jest.fn().mockReturnValueOnce(false);
hasuraClient.doesSchemaExist = jest.fn().mockReturnValueOnce(false);

await expect(provisioner.fetchUserApiProvisioningStatus(indexerConfig)).resolves.toBe(false);
expect(provisioner.isUserApiProvisioned(indexerConfig.accountId, indexerConfig.functionName)).toBe(false);
await expect(provisioner.isProvisioned(indexerConfig)).resolves.toBe(false);
});

it('returns true if datasource and schema exists', async () => {
hasuraClient.doesSourceExist = jest.fn().mockReturnValueOnce(true);
hasuraClient.doesSchemaExist = jest.fn().mockReturnValueOnce(true);

await expect(provisioner.fetchUserApiProvisioningStatus(indexerConfig)).resolves.toBe(true);
expect(provisioner.isUserApiProvisioned(indexerConfig.accountId, indexerConfig.functionName)).toBe(true);
await expect(provisioner.isProvisioned(indexerConfig)).resolves.toBe(true);
});
});

Expand Down Expand Up @@ -233,7 +230,6 @@ describe('Provisioner', () => {
'delete'
]
);
expect(provisioner.isUserApiProvisioned(accountId, functionName)).toBe(true);
});

it('skips provisioning the datasource if it already exists', async () => {
Expand Down
24 changes: 3 additions & 21 deletions runner/src/provisioner/provisioner.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,6 @@ const defaultConfig: Config = {

export default class Provisioner {
tracer: Tracer = trace.getTracer('queryapi-runner-provisioner');
#hasBeenProvisioned: Record<string, Record<string, boolean>> = {};

constructor (
private readonly hasuraClient: HasuraClient = new HasuraClient(),
Expand All @@ -70,17 +69,6 @@ export default class Provisioner {
.replace(/\//g, '0');
}

isUserApiProvisioned (accountId: string, functionName: string): boolean {
const accountIndexers = this.#hasBeenProvisioned[accountId];
if (!accountIndexers) { return false; }
return accountIndexers[functionName];
}

private setProvisioned (accountId: string, functionName: string): void {
this.#hasBeenProvisioned[accountId] ??= {};
this.#hasBeenProvisioned[accountId][functionName] = true;
}

async createDatabase (name: string): Promise<void> {
await this.adminDefaultPgClient.query(this.pgFormat('CREATE DATABASE %I', name));
}
Expand Down Expand Up @@ -156,12 +144,8 @@ export default class Provisioner {
);
}

async fetchUserApiProvisioningStatus (indexerConfig: ProvisioningConfig): Promise<boolean> {
async isProvisioned (indexerConfig: ProvisioningConfig): Promise<boolean> {
const checkProvisioningSpan = this.tracer.startSpan('Check if indexer is provisioned');
if (this.isUserApiProvisioned(indexerConfig.accountId, indexerConfig.functionName)) {
checkProvisioningSpan.end();
return true;
}

const databaseName = indexerConfig.databaseName();
const schemaName = indexerConfig.schemaName();
Expand All @@ -172,10 +156,9 @@ export default class Provisioner {
}

const schemaExists = await this.hasuraClient.doesSchemaExist(databaseName, schemaName);
if (schemaExists) {
this.setProvisioned(indexerConfig.accountId, indexerConfig.functionName);
}

checkProvisioningSpan.end();

return schemaExists;
}

Expand Down Expand Up @@ -356,7 +339,6 @@ export default class Provisioner {
await this.trackForeignKeyRelationships(schemaName, databaseName);

await this.addPermissionsToTables(indexerConfig, updatedTableNames, ['select', 'insert', 'update', 'delete']);
this.setProvisioned(indexerConfig.accountId, indexerConfig.functionName);
},
'Failed to provision endpoint'
);
Expand Down
36 changes: 11 additions & 25 deletions runner/src/server/services/data-layer/data-layer-service.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -67,25 +67,27 @@ describe('DataLayerService', () => {
});

describe('StartProvisioningTask', () => {
it('should return the current task if it exists', (done) => {
const tasks: Record<any, any> = {
'8291150845651941809f8f3db28eeb7fd8acdfeb422cb07c10178020070836b8': { pending: false, completed: true, failed: false } as unknown as AsyncTask
};
it('returns FAILED_PRECONDITION if already provisioned', (done) => {
const provisioner = {
isProvisioned: jest.fn().mockResolvedValue(true)
} as unknown as Provisioner;
const tasks = {};
const call = {
request: { accountId: 'testAccount', functionName: 'testFunction', schema: 'schema' }
request: { accountId: 'testAccount', functionName: 'testFunction', schema: 'testSchema' }
} as unknown as ServerUnaryCall<any, any>;
const callback = (_error: any, response: any): void => {
expect(tasks[response.taskId]).toBeDefined();
expect(tasks[response.taskId].completed).toBe(true);
const callback = (error: any): void => {
expect(error.code).toBe(status.FAILED_PRECONDITION);
expect(error.details).toBe('Data Layer is already provisioned');
done();
};

createDataLayerService(undefined, tasks).StartProvisioningTask(call, callback);
createDataLayerService(provisioner, tasks).StartProvisioningTask(call, callback);
});

it('should start a new provisioning task', (done) => {
const tasks: Record<any, any> = {};
const provisioner = {
isProvisioned: jest.fn().mockResolvedValue(false),
provisionUserApi: jest.fn().mockResolvedValue(null)
} as unknown as Provisioner;
const call = {
Expand All @@ -102,22 +104,6 @@ describe('DataLayerService', () => {
});

describe('StartDeprovisioningTask', () => {
it('should return ALREADY_EXISTS if the task exists', (done) => {
const tasks = {
f92a9f97d2609849e6837b483d8210c7b308c6f615a691449087ec00db1eef06: { pending: true, completed: false, failed: false } as unknown as AsyncTask
};
const call = {
request: { accountId: 'testAccount', functionName: 'testFunction', schema: 'schema' }
} as unknown as ServerUnaryCall<any, any>;
const callback = (error: any): void => {
expect(error.code).toBe(status.ALREADY_EXISTS);
expect(error.details).toBe('Deprovisioning task already exists');
done();
};

createDataLayerService(undefined, tasks).StartDeprovisioningTask(call, callback);
});

it('should start a new deprovisioning task', (done) => {
const tasks: Record<any, any> = {};
const provisioner = {
Expand Down
89 changes: 40 additions & 49 deletions runner/src/server/services/data-layer/data-layer-service.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -39,17 +39,6 @@ export class AsyncTask {

type AsyncTasks = Record<string, AsyncTask | undefined>;

enum TaskType {
PROVISION = 'PROVISION',
DEPROVISION = 'DEPROVISION'
}

const hash = (...args: string[]): string => {
const hash = crypto.createHash('sha256');
hash.update(args.join(':'));
return hash.digest('hex');
};

const createLogger = (config: ProvisioningConfig): typeof parentLogger => {
const logger = parentLogger.child({
accountId: config.accountId,
Expand Down Expand Up @@ -98,31 +87,45 @@ export function createDataLayerService (

const logger = createLogger(provisioningConfig);

const taskId = hash(accountId, functionName, schema, TaskType.PROVISION);

const task = tasks[taskId];

if (task) {
callback(null, { taskId });

return;
};

logger.info(`Starting provisioning task: ${taskId}`);

tasks[taskId] = new AsyncTask(
provisioner
.provisionUserApi(provisioningConfig)
.then(() => {
logger.info('Successfully provisioned Data Layer');
})
.catch((err) => {
logger.error('Failed to provision Data Layer', err);
throw err;
})
);

callback(null, { taskId });
provisioner
.isProvisioned(provisioningConfig)
.then((isProvisioned) => {
if (isProvisioned) {
const failedPrecondition = new StatusBuilder()
.withCode(status.FAILED_PRECONDITION)
.withDetails('Data Layer is already provisioned')
.build();

callback(failedPrecondition);

return;
}

const taskId = crypto.randomUUID();

tasks[taskId] = new AsyncTask(
provisioner
.provisionUserApi(provisioningConfig)
.then(() => {
logger.info('Successfully deprovisioned Data Layer');
})
.catch((err) => {
logger.error('Failed to deprovision Data Layer', err);
throw err;
})
);

callback(null, { taskId });
})
.catch((err) => {
logger.error('Failed to check if Data Layer is provisioned', err);

const internal = new StatusBuilder()
.withCode(status.INTERNAL)
.withDetails('Failed to check Data Layer provisioned status')
.build();
callback(internal);
});
},

StartDeprovisioningTask (call: ServerUnaryCall<DeprovisionRequest__Output, StartTaskResponse>, callback: sendUnaryData<StartTaskResponse>): void {
Expand All @@ -132,19 +135,7 @@ export function createDataLayerService (

const logger = createLogger(provisioningConfig);

const taskId = hash(accountId, functionName, TaskType.DEPROVISION);

const task = tasks[taskId];

if (task) {
const exists = new StatusBuilder()
.withCode(status.ALREADY_EXISTS)
.withDetails('Deprovisioning task already exists')
.build();
callback(exists);

return;
};
const taskId = crypto.randomUUID();

logger.info(`Starting deprovisioning task: ${taskId}`);

Expand Down

0 comments on commit 7303f7a

Please sign in to comment.