deploy: Add rate-limiting aware retries for failed requests

Change-type: patch

lib/utils/compose.ts

@@ -20,6 +20,7 @@ import type * as SDK from 'balena-sdk';
 import type Dockerode = require('dockerode');
 import * as path from 'path';
 import type { Composition, ImageDescriptor } from '@balena/compose/dist/parse';
+import type { RetryParametersObj } from 'pinejs-client-core';
 import type {
 	BuiltImage,
 	ComposeOpts,
@@ -94,22 +95,62 @@ export function createProject(
 	};
 }
 
+const getRequestRetryParameters = (): RetryParametersObj => {
+	if (
+		process.env.BALENA_CLI_TEST_TYPE != null &&
+		process.env.BALENA_CLI_TEST_TYPE !== ''
+	) {
+		// We only read the test env vars when in test mode.
+		const { intVar } =
+			require('@balena/env-parsing') as typeof import('@balena/env-parsing');
+		// We use the BALENARCTEST namespace and only parse the env vars while in test mode
+		// since we plan to switch all pinejs clients with the one of the SDK and might not
+		// want to have to support these env vars.
+		return {
+			minDelayMs: intVar('BALENARCTEST_API_RETRY_MIN_DELAY_MS'),
+			maxDelayMs: intVar('BALENARCTEST_API_RETRY_MAX_DELAY_MS'),
+			maxAttempts: intVar('BALENARCTEST_API_RETRY_MAX_ATTEMPTS'),
+		};
+	}
+
+	return {
+		minDelayMs: 1000,
+		maxDelayMs: 60000,
+		maxAttempts: 7,
+	};
+};
+
 export const createRelease = async function (
+	logger: Logger,
 	apiEndpoint: string,
 	auth: string,
 	userId: number,
 	appId: number,
 	composition: Composition,
 	draft: boolean,
-	semver?: string,
+	semver: string | undefined,
-	contract?: string,
+	contract: string | undefined,
 ): Promise<Release> {
 	const _ = require('lodash') as typeof import('lodash');
 	const crypto = require('crypto') as typeof import('crypto');
 	const releaseMod =
 		require('@balena/compose/dist/release') as typeof import('@balena/compose/dist/release');
 
-	const client = releaseMod.createClient({ apiEndpoint, auth });
+	const client = releaseMod.createClient({
+		apiEndpoint,
+		auth,
+		retry: {
+			...getRequestRetryParameters(),
+			onRetry: (err, delayMs, attempt, maxAttempts) => {
+				const code = err?.statusCode ?? 0;
+				logger.logDebug(
+					`API call failed with code ${code}.  Attempting retry ${attempt} of ${maxAttempts} in ${
+						delayMs / 1000
+					} seconds`,
+				);
+			},
+		},
+	});
 
 	const { release, serviceImages } = await releaseMod.create({
 		client,

lib/utils/compose_ts.ts

@@ -1385,6 +1385,7 @@ export async function deployProject(
 		`${prefix}Creating release...`,
 		() =>
 			createRelease(
+				logger,
 				apiEndpoint,
 				auth,
 				userId,

npm-shrinkwrap.json

@@ -12,6 +12,7 @@
       "dependencies": {
         "@balena/compose": "^3.2.0",
         "@balena/dockerignore": "^1.0.2",
+        "@balena/env-parsing": "^1.1.8",
         "@balena/es-version": "^1.0.1",
         "@oclif/core": "^3.14.1",
         "@resin.io/valid-email": "^0.1.0",
@@ -1544,6 +1545,11 @@
       "resolved": "https://registry.npmjs.org/@balena/dockerignore/-/dockerignore-1.0.2.tgz",
       "integrity": "sha512-wMue2Sy4GAVTk6Ic4tJVcnfdau+gx2EnG7S+uAEe+TWJFqE4YoWN4/H8MSLj4eYJKxGg26lZwboEniNiNwZQ6Q=="
     },
+    "node_modules/@balena/env-parsing": {
+      "version": "1.1.8",
+      "resolved": "https://registry.npmjs.org/@balena/env-parsing/-/env-parsing-1.1.8.tgz",
+      "integrity": "sha512-6L9U2LJ5Akov92962+NjjvrfZ1VPVJGZwjb8DIurRXxFIWldA+D0EOgvvmmZtgiRsG3OfZnRK9oBBYVC/bDFxA=="
+    },
     "node_modules/@balena/es-version": {
       "version": "1.0.1",
       "resolved": "https://registry.npmjs.org/@balena/es-version/-/es-version-1.0.1.tgz",
@@ -25831,6 +25837,11 @@
       "resolved": "https://registry.npmjs.org/@balena/dockerignore/-/dockerignore-1.0.2.tgz",
       "integrity": "sha512-wMue2Sy4GAVTk6Ic4tJVcnfdau+gx2EnG7S+uAEe+TWJFqE4YoWN4/H8MSLj4eYJKxGg26lZwboEniNiNwZQ6Q=="
     },
+    "@balena/env-parsing": {
+      "version": "1.1.8",
+      "resolved": "https://registry.npmjs.org/@balena/env-parsing/-/env-parsing-1.1.8.tgz",
+      "integrity": "sha512-6L9U2LJ5Akov92962+NjjvrfZ1VPVJGZwjb8DIurRXxFIWldA+D0EOgvvmmZtgiRsG3OfZnRK9oBBYVC/bDFxA=="
+    },
     "@balena/es-version": {
       "version": "1.0.1",
       "resolved": "https://registry.npmjs.org/@balena/es-version/-/es-version-1.0.1.tgz",

package.json

@@ -196,6 +196,7 @@
   "dependencies": {
     "@balena/compose": "^3.2.0",
     "@balena/dockerignore": "^1.0.2",
+    "@balena/env-parsing": "^1.1.8",
     "@balena/es-version": "^1.0.1",
     "@oclif/core": "^3.14.1",
     "@resin.io/valid-email": "^0.1.0",

tests/commands/deploy.spec.ts

@@ -15,6 +15,7 @@
  * limitations under the License.
  */
 
+import { intVar } from '@balena/env-parsing';
 import type { Request as ReleaseRequest } from '@balena/compose/dist/release';
 import { expect } from 'chai';
 import { promises as fs } from 'fs';
@@ -284,16 +285,25 @@ describe('balena deploy', function () {
 		api.expectPostRelease({});
 		docker.expectGetManifestBusybox();
 
+		let failedImagePatchRequests = 0;
 		// Mock this patch HTTP request to return status code 500, in which case
 		// the release status should be saved as "failed" rather than "success"
+		const maxRequestRetries = intVar('BALENARCTEST_API_RETRY_MAX_ATTEMPTS');
+		expect(
+			maxRequestRetries,
+			'BALENARCTEST_API_RETRY_MAX_ATTEMPTS must be >= 2 for this test',
+		).to.be.greaterThanOrEqual(2);
 		api.expectPatchImage({
 			replyBody: errMsg,
 			statusCode: 500,
+			// b/c failed requests are retried
+			times: maxRequestRetries,
 			inspectRequest: (_uri, requestBody) => {
 				const imageBody = requestBody as Partial<
 					import('@balena/compose/dist/release/models').ImageModel
 				>;
 				expect(imageBody.status).to.equal('success');
+				failedImagePatchRequests++;
 			},
 		});
 		// Check that the CLI patches the release with status="failed"
@@ -324,6 +334,7 @@ describe('balena deploy', function () {
 				responseCode: 200,
 				services: ['main'],
 			});
+			expect(failedImagePatchRequests).to.equal(maxRequestRetries);
 		} finally {
 			await switchSentry(sentryStatus);
 			// @ts-expect-error claims restore does not exist
@@ -331,6 +342,82 @@ describe('balena deploy', function () {
 		}
 	});
 
+	it('should create the expected --build tar stream after retrying failing OData requests (single container)', async () => {
+		const projectPath = path.join(projectsPath, 'no-docker-compose', 'basic');
+		const expectedFiles: ExpectedTarStreamFiles = {
+			'src/.dockerignore': { fileSize: 16, type: 'file' },
+			'src/start.sh': { fileSize: 89, type: 'file' },
+			'src/windows-crlf.sh': {
+				fileSize: isWindows ? 68 : 70,
+				testStream: isWindows ? expectStreamNoCRLF : undefined,
+				type: 'file',
+			},
+			Dockerfile: { fileSize: 88, type: 'file' },
+			'Dockerfile-alt': { fileSize: 30, type: 'file' },
+		};
+		const responseFilename = 'build-POST.json';
+		const responseBody = await fs.readFile(
+			path.join(dockerResponsePath, responseFilename),
+			'utf8',
+		);
+		const expectedResponseLines = [
+			...commonResponseLines[responseFilename],
+			`[Info] No "docker-compose.yml" file found at "${projectPath}"`,
+			`[Info] Creating default composition with source: "${projectPath}"`,
+			...getDockerignoreWarn1(
+				[path.join(projectPath, 'src', '.dockerignore')],
+				'deploy',
+			),
+		];
+		if (isWindows) {
+			const fname = path.join(projectPath, 'src', 'windows-crlf.sh');
+			expectedResponseLines.push(
+				`[Info] Converting line endings CRLF -> LF for file: ${fname}`,
+			);
+		}
+
+		api.expectPostRelease({});
+		docker.expectGetManifestBusybox();
+
+		const maxRequestRetries = intVar('BALENARCTEST_API_RETRY_MAX_ATTEMPTS');
+		expect(
+			maxRequestRetries,
+			'BALENARCTEST_API_RETRY_MAX_ATTEMPTS must be >= 2 for this test',
+		).to.be.greaterThanOrEqual(2);
+		let failedImagePatchRequests = 0;
+		let succesfullImagePatchRequests = 0;
+		api
+			.optPatch(/^\/v6\/image($|[(?])/, { times: maxRequestRetries })
+			.reply((_uri, requestBody) => {
+				const imageBody = requestBody as Partial<
+					import('@balena/compose/dist/release/models').ImageModel
+				>;
+				expect(imageBody.status).to.equal('success');
+				if (failedImagePatchRequests < maxRequestRetries - 1) {
+					failedImagePatchRequests++;
+					return [500, 'Patch Image Error'];
+				}
+				succesfullImagePatchRequests++;
+				return [200, 'OK'];
+			});
+		api.expectPatchRelease({});
+		api.expectPostImageLabel();
+
+		await testDockerBuildStream({
+			commandLine: `deploy testApp --build --source ${projectPath}`,
+			dockerMock: docker,
+			expectedFilesByService: { main: expectedFiles },
+			expectedQueryParamsByService: { main: commonQueryParams },
+			expectedResponseLines,
+			projectPath,
+			responseBody,
+			responseCode: 200,
+			services: ['main'],
+		});
+		expect(failedImagePatchRequests).to.equal(maxRequestRetries - 1);
+		expect(succesfullImagePatchRequests).to.equal(1);
+	});
+
 	it('should create the expected tar stream (docker-compose, --multi-dockerignore)', async () => {
 		const projectPath = path.join(projectsPath, 'docker-compose', 'basic');
 		const service1Dockerfile = (

tests/config-tests.ts

@@ -26,6 +26,11 @@ process.env.BALENARC_NO_SENTRY = '1';
 // Like the global `--unsupported` flag
 process.env.BALENARC_UNSUPPORTED = '1';
 
+// Reduce the api request retry limits to keep the tests fast.
+process.env.BALENARCTEST_API_RETRY_MIN_DELAY_MS = '100';
+process.env.BALENARCTEST_API_RETRY_MAX_DELAY_MS = '1000';
+process.env.BALENARCTEST_API_RETRY_MAX_ATTEMPTS = '2';
+
 import * as tmp from 'tmp';
 tmp.setGracefulCleanup();
 // Use a temporary dir for tests data

tests/nock/balena-api-mock.ts

@@ -35,11 +35,13 @@ export class BalenaAPIMock extends NockMock {
 		notFound = false,
 		optional = false,
 		persist = false,
+		times = undefined as number | undefined,
 		expandArchitecture = false,
 	} = {}) {
 		const interceptor = this.optGet(/^\/v6\/application($|[(?])/, {
 			optional,
 			persist,
+			times,
 		});
 		if (notFound) {
 			interceptor.reply(200, { d: [] });
@@ -105,10 +107,12 @@ export class BalenaAPIMock extends NockMock {
 		notFound = false,
 		optional = false,
 		persist = false,
+		times = undefined as number | undefined,
 	} = {}) {
 		const interceptor = this.optGet(/^\/v6\/release($|[(?])/, {
 			persist,
 			optional,
+			times,
 		});
 		if (notFound) {
 			interceptor.reply(200, { d: [] });
@@ -133,8 +137,9 @@ export class BalenaAPIMock extends NockMock {
 		inspectRequest = this.inspectNoOp,
 		optional = false,
 		persist = false,
+		times = undefined as number | undefined,
 	}) {
-		this.optPatch(/^\/v6\/release($|[(?])/, { optional, persist }).reply(
+		this.optPatch(/^\/v6\/release($|[(?])/, { optional, persist, times }).reply(
 			statusCode,
 			this.getInspectedReplyBodyFunction(inspectRequest, replyBody),
 		);
@@ -148,8 +153,9 @@ export class BalenaAPIMock extends NockMock {
 		inspectRequest = this.inspectNoOp,
 		optional = false,
 		persist = false,
+		times = undefined as number | undefined,
 	}) {
-		this.optPost(/^\/v6\/release($|[(?])/, { optional, persist }).reply(
+		this.optPost(/^\/v6\/release($|[(?])/, { optional, persist, times }).reply(
 			statusCode,
 			this.getInspectedReplyFileFunction(
 				inspectRequest,
@@ -167,8 +173,9 @@ export class BalenaAPIMock extends NockMock {
 		inspectRequest = this.inspectNoOp,
 		optional = false,
 		persist = false,
+		times = undefined as number | undefined,
 	}) {
-		this.optPatch(/^\/v6\/image($|[(?])/, { optional, persist }).reply(
+		this.optPatch(/^\/v6\/image($|[(?])/, { optional, persist, times }).reply(
 			statusCode,
 			this.getInspectedReplyBodyFunction(inspectRequest, replyBody),
 		);

tests/nock/nock-mock.ts

@@ -21,6 +21,7 @@ import * as fs from 'fs';
 export interface ScopeOpts {
 	optional?: boolean;
 	persist?: boolean;
+	times?: number;
 }
 
 /**
@@ -52,36 +53,50 @@ export class NockMock {
 		this.expect = this.scope;
 	}
 
+	public optMethod(
+		method: 'get' | 'delete' | 'patch' | 'post',
+		uri: string | RegExp | ((uri: string) => boolean),
+		{ optional = false, persist = false, times = undefined }: ScopeOpts,
+	) {
+		let scope = this.scope;
+		if (persist) {
+			scope = scope.persist();
+		}
+		let reqInterceptor = scope[method](uri);
+		if (times != null) {
+			reqInterceptor = reqInterceptor.times(times);
+		} else if (optional) {
+			reqInterceptor = reqInterceptor.optionally();
+		}
+		return reqInterceptor;
+	}
+
 	public optGet(
 		uri: string | RegExp | ((uri: string) => boolean),
-		{ optional = false, persist = false }: ScopeOpts,
+		opts: ScopeOpts,
 	): nock.Interceptor {
-		const get = (persist ? this.scope.persist() : this.scope).get(uri);
+		return this.optMethod('get', uri, opts);
-		return optional ? get.optionally() : get;
 	}
 
 	public optDelete(
 		uri: string | RegExp | ((uri: string) => boolean),
-		{ optional = false, persist = false }: ScopeOpts,
+		opts: ScopeOpts,
 	) {
-		const del = (persist ? this.scope.persist() : this.scope).delete(uri);
+		return this.optMethod('delete', uri, opts);
-		return optional ? del.optionally() : del;
 	}
 
 	public optPatch(
 		uri: string | RegExp | ((uri: string) => boolean),
-		{ optional = false, persist = false }: ScopeOpts,
+		opts: ScopeOpts,
 	) {
-		const patch = (persist ? this.scope.persist() : this.scope).patch(uri);
+		return this.optMethod('patch', uri, opts);
-		return optional ? patch.optionally() : patch;
 	}
 
 	public optPost(
 		uri: string | RegExp | ((uri: string) => boolean),
-		{ optional = false, persist = false }: ScopeOpts,
+		opts: ScopeOpts,
 	) {
-		const post = (persist ? this.scope.persist() : this.scope).post(uri);
+		return this.optMethod('post', uri, opts);
-		return optional ? post.optionally() : post;
 	}
 
 	protected inspectNoOp(_uri: string, _requestBody: nock.Body): void {