Skip to content

Commit

Permalink
fix(k8s): ensure generated pod names are always unique
Browse files Browse the repository at this point in the history
Apologies, this was miscalculated when the fix was introduced.
If Pod names are deterministic we can run into conflicts when running
tasks and tests.
  • Loading branch information
edvald authored and thsig committed Jan 29, 2020
1 parent cf18d1e commit 493a787
Show file tree
Hide file tree
Showing 5 changed files with 20 additions and 7 deletions.
2 changes: 1 addition & 1 deletion garden-service/src/plugins/kubernetes/container/build.ts
Original file line number Diff line number Diff line change
Expand Up @@ -327,7 +327,7 @@ async function runKaniko({ provider, log, module, args, outputStream }: RunKanik
const api = await KubeApi.factory(log, provider)
const systemNamespace = await getSystemNamespace(provider, log)

const podName = makePodName("kaniko", module.name, Math.round(new Date().getTime()).toString())
const podName = makePodName("kaniko", module.name)
const registryHostname = getRegistryHostname(provider.config)
const k8sSystemVars = getKubernetesSystemVariables(provider.config)
const syncDataVolumeName = k8sSystemVars["sync-volume-name"]
Expand Down
2 changes: 1 addition & 1 deletion garden-service/src/plugins/kubernetes/helm/run.ts
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ export async function runHelmModule({
}

const api = await KubeApi.factory(log, provider)
const podName = makePodName("run", module.name, Math.round(new Date().getTime()).toString())
const podName = makePodName("run", module.name)

const runner = new PodRunner({
api,
Expand Down
4 changes: 2 additions & 2 deletions garden-service/src/plugins/kubernetes/run.ts
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,7 @@ export async function runAndCopy({
const api = await KubeApi.factory(log, provider)

if (!podName) {
podName = makePodName("run", module.name, Math.round(new Date().getTime()).toString())
podName = makePodName("run", module.name)
}

const runner = new PodRunner({
Expand Down Expand Up @@ -531,7 +531,7 @@ export class PodRunner extends PodRunnerParams {

return [
"run",
this.podName || makePodName("run", this.module.name, Math.round(new Date().getTime()).toString()),
this.podName || makePodName("run", this.module.name),
`--image=${this.image}`,
"--restart=Never",
// Need to attach to get the log output and exit code.
Expand Down
6 changes: 3 additions & 3 deletions garden-service/src/plugins/kubernetes/util.ts
Original file line number Diff line number Diff line change
Expand Up @@ -571,8 +571,8 @@ const maxPodNamePrefixLength = maxPodNameLength - podNameHashLength - 1
* @param moduleName the name of the module associated with the Pod
* @param key the specific key of the task, test etc.
*/
export function makePodName(type: string, moduleName: string, key: string) {
const id = `${type}-${moduleName}-${key}`
const hash = hasha(key, { algorithm: "sha1" })
export function makePodName(type: string, moduleName: string, key?: string) {
const id = `${type}-${moduleName}${key ? "-" + key : ""}`
const hash = hasha(`${id}-${Math.round(new Date().getTime())}`, { algorithm: "sha1" })
return id.slice(0, maxPodNamePrefixLength) + "-" + hash.slice(0, podNameHashLength)
}
13 changes: 13 additions & 0 deletions garden-service/test/unit/src/plugins/kubernetes/util.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import {
} from "../../../../../src/plugins/kubernetes/util"
import { KubernetesServerResource } from "../../../../../src/plugins/kubernetes/types"
import { V1Pod } from "@kubernetes/client-node"
import { sleep } from "../../../../../src/util/util"

describe("deduplicatePodsByLabel", () => {
it("should return a list of pods, unique by label so that the latest pod is kept", () => {
Expand Down Expand Up @@ -311,10 +312,22 @@ describe("getSelectorString", () => {

describe("makePodName", () => {
it("should create a unique pod name with a hash suffix", () => {
const name = makePodName("test", "some-module")
expect(name.slice(0, -7)).to.equal("test-some-module")
})

it("should optionally include a secondary key", () => {
const name = makePodName("test", "some-module", "unit")
expect(name.slice(0, -7)).to.equal("test-some-module-unit")
})

it("should create different pod names at different times for the same inputs", async () => {
const nameA = makePodName("test", "some-module", "unit")
await sleep(1)
const nameB = makePodName("test", "some-module", "unit")
expect(nameA).to.not.equal(nameB)
})

it("should truncate the pod name if necessary", () => {
const name = makePodName("test", "some-module-with-a-really-unnecessarily-long-name", "really-long-test-name-too")
expect(name.length).to.equal(63)
Expand Down

0 comments on commit 493a787

Please sign in to comment.