Skip to content

Commit

Permalink
fix(cloudbuilder): add error handling to fallback to cli install of b…
Browse files Browse the repository at this point in the history
…uildx builder (#6258)

* fix(cloudbuilder): add error handling to fallback to cli install of buildx builder

* chore: use project root instead of cert dir as cwd for docker cmd

* chore: camelcase functions
  • Loading branch information
twelvemo authored Jul 5, 2024
1 parent c3fd790 commit 6f4b120
Show file tree
Hide file tree
Showing 2 changed files with 30 additions and 12 deletions.
8 changes: 8 additions & 0 deletions core/src/plugins/container/build.ts
Original file line number Diff line number Diff line change
Expand Up @@ -196,6 +196,14 @@ async function buildContainerLocally({
Learn more at https://docs.docker.com/go/build-multi-platform/
`,
})
} else if (error.message.includes("failed to push")) {
throw new ConfigurationError({
message: dedent`
The Docker daemon failed to push the image to the registry.
Please make sure that you are logged in and that you
have sufficient permissions on this machine to push to the registry.
`,
})
}
throw error
}
Expand Down
34 changes: 22 additions & 12 deletions core/src/plugins/container/cloudbuilder.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
import type { PluginContext } from "../../plugin-context.js"
import type { Resolved } from "../../actions/types.js"
import type { ContainerBuildAction } from "./config.js"
import { ConfigurationError, InternalError } from "../../exceptions.js"
import { ConfigurationError, InternalError, isErrnoException } from "../../exceptions.js"
import type { ContainerProvider, ContainerProviderConfig } from "./container.js"
import dedent from "dedent"
import { styles } from "../../logger/styles.js"
Expand Down Expand Up @@ -335,8 +335,8 @@ class BuildxBuilder {

try {
if (refCount === 1) {
await this.remove_tmpdir()
await this.remove_builder()
await this.removeBuilder()
await this.removeTmpdir()
}
} finally {
// even decrease refcount if removal failed
Expand All @@ -355,7 +355,7 @@ class BuildxBuilder {

await this.writeCertificates()

const success = this.installDirectly()
const success = await this.installDirectly()
if (!success) {
await this.installUsingCLI()
}
Expand All @@ -367,18 +367,18 @@ class BuildxBuilder {

// private: clean

private async remove_tmpdir() {
private async removeTmpdir() {
this.ctx.log.debug(`Removing ${this.certDir}...`)
await rm(this.certDir, { recursive: true, force: true })
}

private async remove_builder() {
private async removeBuilder() {
try {
await rm(this.buildxInstanceJsonPath)
} catch (e) {
// fall back to docker CLI
const result = await containerHelpers.dockerCli({
cwd: this.certDir,
cwd: this.ctx.projectRoot,
args: ["buildx", "rm", this.name],
ctx: this.ctx,
log: this.ctx.log,
Expand Down Expand Up @@ -435,12 +435,22 @@ class BuildxBuilder {
}

private async installDirectly() {
const statResult = await stat(dirname(this.buildxInstanceJsonPath))
if (statResult.isDirectory()) {
await writeFile(this.buildxInstanceJsonPath, JSON.stringify(this.getBuildxInstanceJson()))
return true
try {
const statResult = await stat(dirname(this.buildxInstanceJsonPath))
if (statResult.isDirectory()) {
await writeFile(this.buildxInstanceJsonPath, JSON.stringify(this.getBuildxInstanceJson()))
return true
}
return false
} catch (e) {
// An error is thrown e.g. if the path does not exist.
// We don't need to handle this error, as we will fall back to the CLI installation.
if (isErrnoException(e) && e.code === "ENOENT") {
this.ctx.log.debug(`Error checking buildx instance path ${this.buildxInstanceJsonPath}: ${e.message}`)
return false
}
throw e
}
return false
}

private getBuildxInstanceJson() {
Expand Down

0 comments on commit 6f4b120

Please sign in to comment.