Add retries for Docker APIs (#3643)

Blind attempt to fix #3639 before I get debug logs.

Normally I'd worry that we hit a rate limit, but according to [docs](https://cloud.google.com/container-registry/quotas) the rate limit is 50K requests per 10m. Even with an API this chatty it's unlikely to hit the limit...

Without knowing any more info, we can realize that an API as chatty as Docker is likely to fail just through statistics. Deleting 15 functions would probably require 60-75 API calls. If gcr.io is a 3-9s service, that's a 6% chance of failure.

src/deploy/functions/containerCleaner.ts

@@ -40,6 +40,25 @@ export const SUBDOMAIN_MAPPING: Record<string, string> = {
   "australia-southeast1": "asia",
 };
 
+async function retry<Return>(func: () => Promise<Return>): Promise<Return> {
+  const sleep = (ms: number) => new Promise((resolve) => setTimeout(resolve, ms));
+  const MAX_RETRIES = 3;
+  const INITIAL_BACKOFF = 100;
+  let retry = 0;
+  while (true) {
+    try {
+      return await func();
+    } catch (error) {
+      logger.debug("Failed docker command with error", error);
+      retry += 1;
+      if (retry >= MAX_RETRIES) {
+        throw new FirebaseError("Failed to clean up artifacts", { original: error });
+      }
+      await sleep(Math.pow(INITIAL_BACKOFF, retry - 1));
+    }
+  }
+}
+
 export async function cleanupBuildImages(functions: backend.FunctionSpec[]): Promise<void> {
   utils.logBullet(clc.bold.cyan("functions: ") + "cleaning up build files...");
   const gcrCleaner = new ContainerRegistryCleaner();
@@ -258,7 +277,7 @@ export class DockerHelper {
 
   async ls(path: string): Promise<Stat> {
     if (!this.cache[path]) {
-      const raw = await this.client.listTags(path);
+      const raw = await retry(() => this.client.listTags(path));
       this.cache[path] = {
         tags: raw.tags,
         digests: Object.keys(raw.manifest),
@@ -291,7 +310,7 @@ export class DockerHelper {
     const deleteTags = stat.tags.map((tag) =>
       (async () => {
         try {
-          await this.client.deleteTag(path, tag);
+          await retry(() => this.client.deleteTag(path, tag));
           stat.tags.splice(stat.tags.indexOf(tag), 1);
         } catch (err) {
           logger.debug("Got error trying to remove docker tag:", err);
@@ -304,7 +323,7 @@ export class DockerHelper {
     const deleteImages = stat.digests.map((digest) =>
       (async () => {
         try {
-          await this.client.deleteImage(path, digest);
+          await retry(() => this.client.deleteImage(path, digest));
           stat.digests.splice(stat.digests.indexOf(digest), 1);
         } catch (err) {
           logger.debug("Got error trying to remove docker image:", err);

src/test/deploy/functions/containerCleaner.spec.ts

@@ -12,14 +12,14 @@ describe("DockerHelper", () => {
   let deleteImage: sinon.SinonStub;
   let helper: containerCleaner.DockerHelper;
 
-  before(() => {
+  beforeEach(() => {
     helper = new containerCleaner.DockerHelper("us");
     listTags = sinon.stub(helper.client, "listTags").rejects("Unexpected call");
     deleteTag = sinon.stub(helper.client, "deleteTag").rejects("Unexpected call");
     deleteImage = sinon.stub(helper.client, "deleteImage").rejects("Unexpected call");
   });
 
-  after(() => {
+  afterEach(() => {
     sinon.verifyAndRestore();
   });
 
@@ -43,22 +43,27 @@ describe("DockerHelper", () => {
   };
 
   it("Fetches tags with caching", async () => {
-    listTags.withArgs("foo/bar").resolves(FOO_BAR);
+    listTags
+      .withArgs("foo/bar")
+      .onFirstCall()
+      .rejects("I'm flaky!")
+      .onSecondCall()
+      .resolves(FOO_BAR);
 
     await expect(helper.ls("foo/bar")).to.eventually.deep.equal({
       digests: ["sha256:hash1", "sha256:hash2"],
       tags: ["tag1", "tag2"],
       children: ["baz"],
     });
+    expect(listTags).to.have.been.calledTwice;
+    expect(listTags).to.not.have.been.calledWith("foo");
 
     await expect(helper.ls("foo/bar")).to.eventually.deep.equal({
       digests: ["sha256:hash1", "sha256:hash2"],
       tags: ["tag1", "tag2"],
       children: ["baz"],
     });
-
-    // This also verifies that we haven't called at "/foo" to ls "/foo/bar"
-    expect(listTags).to.have.been.calledOnce;
+    expect(listTags).to.have.been.calledTwice;
   });
 
   it("Deletes recursively", async () => {
@@ -69,13 +74,24 @@ describe("DockerHelper", () => {
       "foo/bar": ["tag1", "tag2"],
       "foo/bar/baz": ["tag3"],
     };
+    let firstDeleteTag = true;
     deleteTag.callsFake((path: string, tag: string) => {
+      if (firstDeleteTag) {
+        firstDeleteTag = false;
+        throw new Error("I'm flaky");
+      }
       if (!remainingTags[path].includes(tag)) {
         throw new Error("Cannot remove tag twice");
       }
       remainingTags[path].splice(remainingTags[path].indexOf(tag), 1);
     });
+
+    let firstDeleteImage = true;
     deleteImage.callsFake((path: string, digest: string) => {
+      if (firstDeleteImage) {
+        firstDeleteImage = false;
+        throw new Error("I'm flaky");
+      }
       if (remainingTags[path].length) {
         throw new Error("Cannot remove image while tags still pin it");
       }
@@ -87,12 +103,12 @@ describe("DockerHelper", () => {
     expect(listTags).to.have.been.calledWith("foo/bar");
     expect(listTags).to.have.been.calledWith("foo/bar/baz");
 
-    expect(deleteTag).to.have.been.calledThrice;
+    expect(deleteTag).to.have.been.callCount(4);
     expect(deleteTag).to.have.been.calledWith("foo/bar/baz", "tag3");
     expect(deleteTag).to.have.been.calledWith("foo/bar", "tag1");
     expect(deleteTag).to.have.been.calledWith("foo/bar", "tag2");
 
-    expect(deleteImage).to.have.been.calledThrice;
+    expect(deleteImage).to.have.been.callCount(4);
     expect(deleteImage).to.have.been.calledWith("foo/bar/baz", "sha256:hash3");
     expect(deleteImage).to.have.been.calledWith("foo/bar", "sha256:hash1");
     expect(deleteImage).to.have.been.calledWith("foo/bar", "sha256:hash2");

templates/init/functions/typescript/_eslintrc

@@ -24,6 +24,7 @@ module.exports = {
     "@typescript-eslint",
   ],
   rules: {
-    quotes: ["error", "double"],
+    "quotes": ["error", "double"],
+    "import/no-unresolved": 0,
   },
 };