Skip to content

Commit

Permalink
fix: revisions to command option
Browse files Browse the repository at this point in the history
BREAKING CHANGE:

k8s providers no longer default to `/bin/sh -c` as the entrypoint when
running pods. This applies to tasks, tests and the `run module` command.
  • Loading branch information
thsig authored and edvald committed Jun 13, 2019
1 parent a2fa49e commit 51fc76a
Show file tree
Hide file tree
Showing 9 changed files with 69 additions and 21 deletions.
5 changes: 3 additions & 2 deletions docs/reference/commands.md
Original file line number Diff line number Diff line change
Expand Up @@ -499,21 +499,22 @@ Examples:

##### Usage

garden run module <module> [command] [options]
garden run module <module> [arguments] [options]

##### Arguments

| Argument | Required | Description |
| -------- | -------- | ----------- |
| `module` | Yes | The name of the module to run.
| `command` | No | The command to run in the module.
| `arguments` | No | The arguments to run the module with. Example: &#x27;npm run my-script&#x27;.

##### Options

| Argument | Alias | Type | Description |
| -------- | ----- | ---- | ----------- |
| `--interactive` | | boolean | Set to false to skip interactive mode and just output the command result.
| `--force-build` | | boolean | Force rebuild of module before running.
| `--command` | `-c` | array:string | The base command (a.k.a. entrypoint) to run in the module. For container modules, for example, this overrides the image&#x27;s default command/entrypoint. This option may not be relevant for all module types. Example: &#x27;/bin/sh -c&#x27;.

### garden run service

Expand Down
13 changes: 12 additions & 1 deletion garden-service/src/commands/base.ts
Original file line number Diff line number Diff line change
Expand Up @@ -93,9 +93,20 @@ export class StringOption extends Parameter<string | undefined> {
}
}

export interface StringsConstructor extends ParameterConstructor<string[]> {
delimiter?: string
}

export class StringsParameter extends Parameter<string[] | undefined> {
type = "array:string"
schema = Joi.array().items(Joi.string())
delimiter: string

constructor(args: StringsConstructor) {
super(args)

this.delimiter = args.delimiter || ","
}

// Sywac returns [undefined] if input is empty so we coerce that into undefined.
// This only applies to optional parameters since Sywac would throw if input is empty for a required parameter.
Expand All @@ -108,7 +119,7 @@ export class StringsParameter extends Parameter<string[] | undefined> {
}

parseString(input: string) {
return input.split(",")
return input.split(this.delimiter)
}
}

Expand Down
3 changes: 2 additions & 1 deletion garden-service/src/commands/exec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,8 @@ import {
CommandResult,
CommandParams,
StringParameter,
StringsParameter,
BooleanParameter,
StringsParameter,
} from "./base"
import dedent = require("dedent")
import { getServiceRuntimeContext } from "../types/service"
Expand All @@ -29,6 +29,7 @@ const runArgs = {
command: new StringsParameter({
help: "The command to run.",
required: true,
delimiter: " ",
}),
}

Expand Down
21 changes: 15 additions & 6 deletions garden-service/src/commands/run/module.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,18 +17,19 @@ import {
StringsParameter,
} from "../base"
import { printRuntimeContext, runtimeContextForServiceDeps } from "./run"
import dedent = require("dedent")
import { printHeader } from "../../logger/util"
import { BuildTask } from "../../tasks/build"
import { dedent, deline } from "../../util/string"

const runArgs = {
module: new StringParameter({
help: "The name of the module to run.",
required: true,
}),
// TODO: make this a variadic arg
command: new StringsParameter({
help: "The command to run in the module.",
arguments: new StringsParameter({
help: "The arguments to run the module with. Example: 'npm run my-script'.",
delimiter: " ",
}),
}

Expand All @@ -42,6 +43,13 @@ const runOpts = {
cliOnly: true,
}),
"force-build": new BooleanParameter({ help: "Force rebuild of module before running." }),
"command": new StringsParameter({
help: deline`The base command (a.k.a. entrypoint) to run in the module. For container modules, for example,
this overrides the image's default command/entrypoint. This option may not be relevant for all module types.
Example: '/bin/sh -c'.`,
alias: "c",
delimiter: " ",
}),
}

type Args = typeof runArgs
Expand Down Expand Up @@ -73,8 +81,8 @@ export class RunModuleCommand extends Command<Args, Opts> {
const graph = await garden.getConfigGraph()
const module = await graph.getModule(moduleName)

const msg = args.command
? `Running command ${chalk.white(args.command.join(" "))} in module ${chalk.white(moduleName)}`
const msg = args.arguments
? `Running module ${chalk.white(moduleName)} with arguments ${chalk.white(args.arguments.join(" "))}`
: `Running module ${chalk.white(moduleName)}`

printHeader(headerLog, msg, "runner")
Expand All @@ -94,7 +102,8 @@ export class RunModuleCommand extends Command<Args, Opts> {
const result = await actions.runModule({
log,
module,
args: args.command || [],
command: opts.command,
args: args.arguments || [],
runtimeContext,
interactive: opts.interactive,
})
Expand Down
1 change: 0 additions & 1 deletion garden-service/src/plugins/container/helpers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@ import { ModuleConfig } from "../../config/module"
import { ContainerModule, ContainerRegistryConfig, defaultTag, defaultNamespace, ContainerModuleConfig } from "./config"

export const minDockerVersion = "17.07.0"
export const defaultContainerCommand = ["/bin/sh", "-c"]

interface ParsedImageId {
host?: string
Expand Down
3 changes: 2 additions & 1 deletion garden-service/src/plugins/kubernetes/helm/test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ export async function testHelmModule(
TestModuleParams<HelmModule>,
): Promise<TestResult> {
const testName = testConfig.name
const args = testConfig.spec.args
const { command, args } = testConfig.spec
runtimeContext.envVars = { ...runtimeContext.envVars, ...testConfig.spec.env }
const timeout = testConfig.timeout || DEFAULT_TEST_TIMEOUT

Expand All @@ -40,6 +40,7 @@ export async function testHelmModule(
namespace,
module,
envVars: runtimeContext.envVars,
command,
args,
image,
interactive,
Expand Down
10 changes: 6 additions & 4 deletions garden-service/src/plugins/kubernetes/run.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ import { kubectl } from "./kubectl"
import { PrimitiveMap } from "../../config/common"
import { Module } from "../../types/module"
import { LogEntry } from "../../logger/log-entry"
import { defaultContainerCommand } from "../container/helpers"

interface RunPodParams {
context: string,
Expand All @@ -37,12 +36,11 @@ export async function runPod(
): Promise<RunResult> {
const envArgs = Object.entries(envVars).map(([k, v]) => `--env=${k}=${v}`)

const cmd = (command && command.length) ? command : defaultContainerCommand
const cmd = (command && command.length) ? command : []

const opts = [
`--image=${image}`,
"--restart=Never",
"--command",
"--quiet",
"--rm",
// Need to attach to get the log output and exit code.
Expand All @@ -57,13 +55,17 @@ export async function runPod(
opts.push("--tty")
}

if (cmd.length) {
opts.push("--command")
}

const kubecmd = [
"run", `run-${module.name}-${Math.round(new Date().getTime())}`,
...opts,
...envArgs,
"--",
...cmd,
args.join(" "),
...args,
]

log.verbose(`Running ${cmd.join(" ")} '${args.join(" ")}'`)
Expand Down
2 changes: 1 addition & 1 deletion garden-service/test/helpers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ async function runModule(params: RunModuleParams): Promise<RunResult> {

return {
moduleName: params.module.name,
command: params.args,
command: [...(params.command || []), ...params.args],
completedAt: testNow,
output: "OK",
version,
Expand Down
32 changes: 28 additions & 4 deletions garden-service/test/unit/src/commands/run/module.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,14 +21,14 @@ describe("RunModuleCommand", () => {
log = garden.log
})

it("should run a module without a command param", async () => {
it("should run a module without an arguments param", async () => {
const cmd = new RunModuleCommand()
const { result } = await cmd.action({
garden,
log,
headerLog: log,
footerLog: log,
args: { module: "module-a", command: [] },
args: { module: "module-a", arguments: [] },
opts: withDefaultGlobalOpts({ "interactive": false, "force-build": false }),
})

Expand All @@ -45,14 +45,14 @@ describe("RunModuleCommand", () => {
expect(result).to.eql(expected)
})

it("should run a module with a command param", async () => {
it("should run a module with an arguments param", async () => {
const cmd = new RunModuleCommand()
const { result } = await cmd.action({
garden,
log,
headerLog: log,
footerLog: log,
args: { module: "module-a", command: ["my", "command"] },
args: { module: "module-a", arguments: ["my", "command"] },
opts: withDefaultGlobalOpts({ "interactive": false, "force-build": false }),
})

Expand All @@ -68,4 +68,28 @@ describe("RunModuleCommand", () => {

expect(result).to.eql(expected)
})

it("should run a module with a command option", async () => {
const cmd = new RunModuleCommand()
const { result } = await cmd.action({
garden,
log,
headerLog: log,
footerLog: log,
args: { module: "module-a", arguments: ["my", "command"] },
opts: withDefaultGlobalOpts({ "interactive": false, "force-build": false, "command": ["/bin/sh", "-c"] }),
})

const expected: RunResult = {
moduleName: "module-a",
command: ["/bin/sh", "-c", "my", "command"],
completedAt: testNow,
output: "OK",
version: testModuleVersion,
startedAt: testNow,
success: true,
}

expect(result).to.eql(expected)
})
})

0 comments on commit 51fc76a

Please sign in to comment.