Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(core): print warning message if docker server version is unparsab… #5288

Merged
merged 1 commit into from
Oct 23, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion core/src/plugins/container/build.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ export const getContainerBuildStatus: BuildActionHandler<"getStatus", ContainerB
}

export const buildContainer: BuildActionHandler<"build", ContainerBuildAction> = async ({ ctx, action, log }) => {
containerHelpers.checkDockerServerVersion(await containerHelpers.getDockerVersion())
containerHelpers.checkDockerServerVersion(await containerHelpers.getDockerVersion(), log)

const buildPath = action.getBuildPath()
const spec = action.getSpec()
Expand Down
27 changes: 19 additions & 8 deletions core/src/plugins/container/helpers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ import {
} from "./moduleConfig"
import { Writable } from "stream"
import { flatten, uniq, fromPairs, reduce } from "lodash"
import { Log } from "../../logger/log-entry"
import { ActionLog, Log } from "../../logger/log-entry"
import chalk from "chalk"
import isUrl from "is-url"
import titleize from "titleize"
Expand All @@ -39,10 +39,10 @@ interface DockerVersion {
server?: string
}

export const minDockerVersion: DockerVersion = {
export const minDockerVersion = {
client: "19.03.0",
server: "17.07.0",
}
} as const

interface ParsedImageId {
host?: string
Expand Down Expand Up @@ -309,15 +309,26 @@ const helpers = {
/**
* Asserts that the specified docker client version meets the minimum requirements.
*/
checkDockerServerVersion(version: DockerVersion) {
checkDockerServerVersion(version: DockerVersion, log: ActionLog) {
if (!version.server) {
throw new RuntimeError({
message: `Failed to check Docker server version: Docker server is not running or cannot be reached.`,
})
} else if (!checkMinDockerVersion(version.server, minDockerVersion.server!)) {
throw new RuntimeError({
message: `Docker server needs to be version ${minDockerVersion.server} or newer (got ${version.server})`,
})
} else {
let hasMinVersion = true
try {
hasMinVersion = checkMinDockerVersion(version.server, minDockerVersion.server)
} catch (err) {
log.warn(
`Failed to parse Docker server version: ${version.server}. Please check your Docker installation. A docker factory reset may be required.`
)
return
}
if (!hasMinVersion) {
throw new RuntimeError({
message: `Docker server needs to be version ${minDockerVersion.server} or newer (got ${version.server})`,
})
}
}
},

Expand Down
21 changes: 17 additions & 4 deletions core/test/integ/src/plugins/container/container.ts
Original file line number Diff line number Diff line change
Expand Up @@ -169,7 +169,7 @@ describe("plugins.container", () => {

describe("checkDockerServerVersion", () => {
it("should return if server version is equal to the minimum version", async () => {
containerHelpers.checkDockerServerVersion(minDockerVersion)
containerHelpers.checkDockerServerVersion(minDockerVersion, log)
})

it("should return if server version is greater than the minimum version", async () => {
Expand All @@ -178,7 +178,20 @@ describe("plugins.container", () => {
server: "99.99",
}

containerHelpers.checkDockerServerVersion(version)
containerHelpers.checkDockerServerVersion(version, log)
})

// see https://github.com/garden-io/garden/issues/5284
it("should print a warning message if server returns an unparsable server version", async () => {
const version = {
client: "99.99",
server: "dev",
}

expect(() => containerHelpers.checkDockerServerVersion(version, log)).to.not.throw()
expect(log.entries[0].msg).to.equal(
"Failed to parse Docker server version: dev. Please check your Docker installation. A docker factory reset may be required."
)
})

it("should throw if server is not reachable (version is undefined)", async () => {
Expand All @@ -188,7 +201,7 @@ describe("plugins.container", () => {
}

await expectError(
() => containerHelpers.checkDockerServerVersion(version),
() => containerHelpers.checkDockerServerVersion(version, log),
(err) => {
expect(err.message).to.equal(
"Failed to check Docker server version: Docker server is not running or cannot be reached."
Expand All @@ -204,7 +217,7 @@ describe("plugins.container", () => {
}

await expectError(
() => containerHelpers.checkDockerServerVersion(version),
() => containerHelpers.checkDockerServerVersion(version, log),
(err) => {
expect(err.message).to.equal("Docker server needs to be version 17.07.0 or newer (got 17.06)")
}
Expand Down
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -103,4 +103,4 @@
"sdk",
"plugins/*"
]
}
}