Skip to content

Commit

Permalink
fix: avoid crash during kubectl retry (#5098)
Browse files Browse the repository at this point in the history
* fix: use forceRetry when using requestWithRetry with kubectl command

This is needed because toKubernetesError does not know (and should not
know) how to deal with a ChildProcessError.

When using requestWithRetry for anything other than Kubernetes requests,
forceRetry needs to be true.

It's questionable if we should use requestWithRetry at all or we should
rather resort to pRetry directly.

* fix: use correct manifest loglevel in the error message
  • Loading branch information
stefreak committed Sep 16, 2023
1 parent 75d16a3 commit c5a7679
Show file tree
Hide file tree
Showing 2 changed files with 15 additions and 9 deletions.
9 changes: 8 additions & 1 deletion core/src/plugins/kubernetes/api.ts
Expand Up @@ -955,6 +955,13 @@ function getGroupBasePath(apiVersion: string) {
return apiVersion.includes("/") ? `/apis/${apiVersion}` : `/api/${apiVersion}`
}

export const KUBECTL_RETRY_OPTS: RetryOpts = {
maxRetries: 3,
minTimeoutMs: 300,
// forceRetry is important, because shouldRetry cannot handle ChildProcessError.
forceRetry: true,
}

export async function getKubeConfig(log: Log, ctx: PluginContext, provider: KubernetesProvider) {
let kubeConfigStr: string

Expand All @@ -972,7 +979,7 @@ export async function getKubeConfig(log: Log, ctx: PluginContext, provider: Kube
log,
args,
}),
{ forceRetry: true }
KUBECTL_RETRY_OPTS
)
}
return load(kubeConfigStr)!
Expand Down
15 changes: 7 additions & 8 deletions core/src/plugins/kubernetes/kubectl.ts
Expand Up @@ -15,7 +15,7 @@ import { dedent, gardenAnnotationKey } from "../../util/string"
import { getResourceKey, hashManifest } from "./util"
import { PluginToolSpec } from "../../plugin/tools"
import { PluginContext } from "../../plugin-context"
import { KubeApi, KubernetesError } from "./api"
import { KUBECTL_RETRY_OPTS, KubeApi, KubernetesError } from "./api"
import { pathExists } from "fs-extra"
import { ChildProcessError, ConfigurationError } from "../../exceptions"
import { requestWithRetry, RetryOpts } from "./retry"
Expand Down Expand Up @@ -58,8 +58,6 @@ export interface ApplyParams {

export const KUBECTL_DEFAULT_TIMEOUT = 300

const KUBECTL_DEFAULT_RETRY_OPTS: RetryOpts = { maxRetries: 3, minTimeoutMs: 300 }

export async function apply({
log,
ctx,
Expand Down Expand Up @@ -111,7 +109,8 @@ export async function apply({

const input = Buffer.from(encodeYamlMulti(manifests))

log.debug(`Applying Kubernetes manifests:\n${input.toString()}`)
const manifestLogLevel = "debug" as const
log[manifestLogLevel](`Applying Kubernetes manifests:\n${input.toString()}`)

let args = ["apply"]
dryRun && args.push("--dry-run")
Expand All @@ -130,7 +129,7 @@ export async function apply({
args,
input,
}),
KUBECTL_DEFAULT_RETRY_OPTS
KUBECTL_RETRY_OPTS
)
} catch (e) {
if (e instanceof ChildProcessError) {
Expand All @@ -140,7 +139,7 @@ export async function apply({
${e.details.output}
Use the option "--log-level verbose" to see the kubernetes manifests that we attempted to apply through "kubectl apply".
Use the option "--log-level ${manifestLogLevel}" to see the kubernetes manifests that we attempted to apply through "kubectl apply".
`,
})
}
Expand Down Expand Up @@ -199,7 +198,7 @@ export async function deleteResourceKeys({
log,
`kubectl ${args.join(" ")}`,
() => kubectl(ctx, provider).stdout({ namespace, args, log }),
KUBECTL_DEFAULT_RETRY_OPTS
KUBECTL_RETRY_OPTS
)
}

Expand Down Expand Up @@ -228,7 +227,7 @@ export async function deleteObjectsBySelector({
log,
`kubectl ${args.join(" ")}`,
() => kubectl(ctx, provider).stdout({ namespace, args, log }),
KUBECTL_DEFAULT_RETRY_OPTS
KUBECTL_RETRY_OPTS
)
}

Expand Down

0 comments on commit c5a7679

Please sign in to comment.