From c5a7679cb85106862f53413be9db8a2dabdb8cd5 Mon Sep 17 00:00:00 2001
From: Steffen Neubauer <steffen@garden.io>
Date: Sat, 16 Sep 2023 15:08:40 +0200
Subject: [PATCH] fix: avoid crash during kubectl retry (#5098)

* 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
---
 core/src/plugins/kubernetes/api.ts     |  9 ++++++++-
 core/src/plugins/kubernetes/kubectl.ts | 15 +++++++--------
 2 files changed, 15 insertions(+), 9 deletions(-)

diff --git a/core/src/plugins/kubernetes/api.ts b/core/src/plugins/kubernetes/api.ts
index c33c2533fd..cf4567e2eb 100644
--- a/core/src/plugins/kubernetes/api.ts
+++ b/core/src/plugins/kubernetes/api.ts
@@ -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
 
@@ -972,7 +979,7 @@ export async function getKubeConfig(log: Log, ctx: PluginContext, provider: Kube
             log,
             args,
           }),
-        { forceRetry: true }
+        KUBECTL_RETRY_OPTS
       )
     }
     return load(kubeConfigStr)!
diff --git a/core/src/plugins/kubernetes/kubectl.ts b/core/src/plugins/kubernetes/kubectl.ts
index 958a742988..47296d9ddf 100644
--- a/core/src/plugins/kubernetes/kubectl.ts
+++ b/core/src/plugins/kubernetes/kubectl.ts
@@ -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"
@@ -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,
@@ -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")
@@ -130,7 +129,7 @@ export async function apply({
           args,
           input,
         }),
-      KUBECTL_DEFAULT_RETRY_OPTS
+      KUBECTL_RETRY_OPTS
     )
   } catch (e) {
     if (e instanceof ChildProcessError) {
@@ -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".
           `,
       })
     }
@@ -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
   )
 }
 
@@ -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
   )
 }