From 76ba21812cc439aafb88fbceacbd788d5d309d88 Mon Sep 17 00:00:00 2001
From: zhaonan <zhaonan06@corp.netease.com>
Date: Wed, 23 Aug 2023 20:12:56 +0800
Subject: [PATCH] feat: add doc and fix code

Signed-off-by: zhaonan <zhaonan06@corp.netease.com>
---
 api/config/v1alpha1/envoyproxy_types.go       |  5 +-
 ...ig.gateway.envoyproxy.io_envoyproxies.yaml |  5 +-
 docs/latest/api/config_types.md               |  4 +-
 docs/latest/user/proxy-enhance.md             | 49 +++++++++++++++++++
 .../kubernetes/proxy/resource.go              | 36 +++++++-------
 .../kubernetes/proxy/resource_provider.go     |  4 +-
 .../proxy/testdata/deployments/bootstrap.yaml |  4 +-
 .../testdata/deployments/component-level.yaml |  4 +-
 .../proxy/testdata/deployments/custom.yaml    |  4 +-
 .../testdata/deployments/default-env.yaml     |  4 +-
 .../proxy/testdata/deployments/default.yaml   |  4 +-
 .../testdata/deployments/enable-coredump.yaml | 25 ++++------
 .../deployments/enable-prometheus.yaml        |  4 +-
 .../testdata/deployments/extension-env.yaml   |  4 +-
 .../proxy/testdata/deployments/volumes.yaml   |  4 +-
 .../deployments/with-concurrency.yaml         |  4 +-
 16 files changed, 105 insertions(+), 59 deletions(-)
 create mode 100644 docs/latest/user/proxy-enhance.md

diff --git a/api/config/v1alpha1/envoyproxy_types.go b/api/config/v1alpha1/envoyproxy_types.go
index a2451eea067..85c0cbe075b 100644
--- a/api/config/v1alpha1/envoyproxy_types.go
+++ b/api/config/v1alpha1/envoyproxy_types.go
@@ -64,9 +64,10 @@ type EnvoyProxySpec struct {
 	// +optional
 	Concurrency *int32 `json:"concurrency,omitempty"`
 
-	// Enables core dumps for gateway.
+	// Enables core dumps for the managed Envoy Proxy fleet.
 	//
-	// If set, gateway will have core dumps enabled.
+	// If set, the managed Envoy Proxy fleet will generate core dumps
+	// when they crash or experience a segmentation fault.
 	//
 	// +optional
 	EnableCoreDump bool `json:"enableCoreDump,omitempty"`
diff --git a/charts/gateway-helm/crds/generated/config.gateway.envoyproxy.io_envoyproxies.yaml b/charts/gateway-helm/crds/generated/config.gateway.envoyproxy.io_envoyproxies.yaml
index 00e6e90bb9d..30473af4820 100644
--- a/charts/gateway-helm/crds/generated/config.gateway.envoyproxy.io_envoyproxies.yaml
+++ b/charts/gateway-helm/crds/generated/config.gateway.envoyproxy.io_envoyproxies.yaml
@@ -71,8 +71,9 @@ spec:
                 format: int32
                 type: integer
               enableCoreDump:
-                description: "Enables core dumps for gateway. \n If set, gateway will
-                  have core dumps enabled."
+                description: "Enables core dumps for the managed Envoy Proxy fleet.
+                  \n If set, the managed Envoy Proxy fleet will generate core dumps
+                  when they crash or experience a segmentation fault."
                 type: boolean
               logging:
                 default:
diff --git a/docs/latest/api/config_types.md b/docs/latest/api/config_types.md
index 2531a8e3846..b606dda7361 100644
--- a/docs/latest/api/config_types.md
+++ b/docs/latest/api/config_types.md
@@ -334,8 +334,8 @@ _Appears in:_
 | `telemetry` _[ProxyTelemetry](#proxytelemetry)_ | Telemetry defines telemetry parameters for managed proxies. |
 | `bootstrap` _[ProxyBootstrap](#proxybootstrap)_ | Bootstrap defines the Envoy Bootstrap as a YAML string. Visit https://www.envoyproxy.io/docs/envoy/latest/api-v3/config/bootstrap/v3/bootstrap.proto#envoy-v3-api-msg-config-bootstrap-v3-bootstrap to learn more about the syntax. If set, this is the Bootstrap configuration used for the managed Envoy Proxy fleet instead of the default Bootstrap configuration set by Envoy Gateway. Some fields within the Bootstrap that are required to communicate with the xDS Server (Envoy Gateway) and receive xDS resources from it are not configurable and will result in the `EnvoyProxy` resource being rejected. Backward compatibility across minor versions is not guaranteed. We strongly recommend using `egctl x translate` to generate a `EnvoyProxy` resource with the `Bootstrap` field set to the default Bootstrap configuration used. You can edit this configuration, and rerun `egctl x translate` to ensure there are no validation errors. |
 | `concurrency` _integer_ | Concurrency defines the number of worker threads to run. If unset, it defaults to the number of cpuset threads on the platform. |
-| `enableCoreDump` _boolean_ | Enables core dumps for gateway. 
- If set, gateway will have core dumps enabled. |
+| `enableCoreDump` _boolean_ | Enables core dumps for the managed Envoy Proxy fleet. 
+ If set, the managed Envoy Proxy fleet will generate core dumps when they crash or experience a segmentation fault. |
 
 
 
diff --git a/docs/latest/user/proxy-enhance.md b/docs/latest/user/proxy-enhance.md
new file mode 100644
index 00000000000..6cc165f4db9
--- /dev/null
+++ b/docs/latest/user/proxy-enhance.md
@@ -0,0 +1,49 @@
+# Proxy Enhance
+
+Envoy Gateway provides basic system debugging for the ControlPlane and the underlying EnvoyProxy instances.
+This guide show you how to get a core file for debugging in proxy.
+
+## Prerequisites
+
+Follow the steps from the [Quickstart Guide](quickstart.md) to install Envoy Gateway and the example manifest.
+Before proceeding, you should be able to query the example backend using HTTP.
+
+### Add GatewayClass ParametersRef
+First, you need to add ParametersRef in GatewayClass, and refer to EnvoyProxy Config:
+
+```shell
+cat <<EOF | kubectl apply -f -
+apiVersion: gateway.networking.k8s.io/v1beta1
+kind: GatewayClass
+metadata:
+  name: eg
+spec:
+  controllerName: gateway.envoyproxy.io/gatewayclass-controller
+  parametersRef:
+    group: config.gateway.envoyproxy.io
+    kind: EnvoyProxy
+    name: custom-proxy-config
+    namespace: envoy-gateway-system
+EOF
+```
+
+### Custommize EnvoyProxy CoreDump
+You can customize the EnvoyProxy CoreDump via EnvoyProxy Config like:
+
+```shell
+cat <<EOF | kubectl apply -f -
+apiVersion: config.gateway.envoyproxy.io/v1alpha1
+kind: EnvoyProxy
+metadata:
+  name: custom-proxy-config
+  namespace: envoy-gateway-system
+spec:
+  enableCoreDump: true
+EOF
+```
+
+After you apply the config,you will find the generated core file in the local `/var/gateway/proxy/data/` directory when envoy proxy crashes.
+
+
+## Debug coredump file
+### GDB
diff --git a/internal/infrastructure/kubernetes/proxy/resource.go b/internal/infrastructure/kubernetes/proxy/resource.go
index a0400c035e3..996f9894ace 100644
--- a/internal/infrastructure/kubernetes/proxy/resource.go
+++ b/internal/infrastructure/kubernetes/proxy/resource.go
@@ -39,7 +39,7 @@ const (
 	// envoyPodEnvVar is the name of the Envoy pod name environment variable.
 	envoyPodEnvVar = "ENVOY_POD_NAME"
 	// initContainerName is the name of the init container.
-	initContainerName = "enable-core-dump"
+	initContainerName = "configure-core-dump"
 )
 
 var (
@@ -194,24 +194,24 @@ func expectedProxyContainers(infra *ir.ProxyInfra, deploymentConfig *egcfgv1a1.K
 	return containers, nil
 }
 
-func expectedInitContainers(image string) []corev1.Container {
+func expectedCoreDumpInitContainers(image string) corev1.Container {
 	args := []string{
 		"-c",
-		"sysctl -w kernel.core_pattern=/tmp/core-%e-%p-%t && ulimit -c unlimited",
+		// set the output directory for the core file & increase the core file size limit
+		"sysctl -w kernel.core_pattern=/cores/core-%e-%p-%t && ulimit -c unlimited",
 	}
-	containers := []corev1.Container{
-		{
-			Name:            initContainerName,
-			Image:           *pointer.String(image),
-			ImagePullPolicy: corev1.PullIfNotPresent,
-			Command:         []string{"/bin/sh"},
-			Args:            args,
-			SecurityContext: &corev1.SecurityContext{
-				RunAsUser:    pointer.Int64(0),
-				RunAsGroup:   pointer.Int64(0),
-				RunAsNonRoot: pointer.Bool(false),
-				Privileged:   pointer.Bool(true),
-			},
+
+	containers := corev1.Container{
+		Name:            initContainerName,
+		Image:           *pointer.String(image),
+		ImagePullPolicy: corev1.PullIfNotPresent,
+		Command:         []string{"/bin/sh"},
+		Args:            args,
+		SecurityContext: &corev1.SecurityContext{
+			RunAsUser:    pointer.Int64(0),
+			RunAsGroup:   pointer.Int64(0),
+			RunAsNonRoot: pointer.Bool(false),
+			Privileged:   pointer.Bool(true),
 		},
 	}
 	return containers
@@ -231,7 +231,7 @@ func expectedContainerVolumeMounts(deploymentSpec *egcfgv1a1.KubernetesDeploymen
 		},
 		{
 			Name:      "coredump",
-			MountPath: "/tmp/",
+			MountPath: "/cores/",
 		},
 	}
 
@@ -277,7 +277,7 @@ func expectedDeploymentVolumes(name string, deploymentSpec *egcfgv1a1.Kubernetes
 			Name: "coredump",
 			VolumeSource: corev1.VolumeSource{
 				HostPath: &corev1.HostPathVolumeSource{
-					Path: "/data/log",
+					Path: "/var/gateway/proxy/data/",
 					Type: &createType,
 				},
 			},
diff --git a/internal/infrastructure/kubernetes/proxy/resource_provider.go b/internal/infrastructure/kubernetes/proxy/resource_provider.go
index b1a7ef59e81..388d7d53cf8 100644
--- a/internal/infrastructure/kubernetes/proxy/resource_provider.go
+++ b/internal/infrastructure/kubernetes/proxy/resource_provider.go
@@ -230,8 +230,8 @@ func (r *ResourceRender) Deployment() (*appsv1.Deployment, error) {
 	}
 
 	if r.infra.Config.Spec.EnableCoreDump {
-		initcontainers := expectedInitContainers(*deploymentConfig.Container.Image)
-		deployment.Spec.Template.Spec.InitContainers = initcontainers
+		coreDumpInitContainers := expectedCoreDumpInitContainers(*deploymentConfig.Container.Image)
+		deployment.Spec.Template.Spec.InitContainers = append(deployment.Spec.Template.Spec.InitContainers, coreDumpInitContainers)
 	}
 
 	return deployment, nil
diff --git a/internal/infrastructure/kubernetes/proxy/testdata/deployments/bootstrap.yaml b/internal/infrastructure/kubernetes/proxy/testdata/deployments/bootstrap.yaml
index a557a610638..4ff76e87c0f 100644
--- a/internal/infrastructure/kubernetes/proxy/testdata/deployments/bootstrap.yaml
+++ b/internal/infrastructure/kubernetes/proxy/testdata/deployments/bootstrap.yaml
@@ -81,7 +81,7 @@ spec:
               readOnly: true
             - mountPath: /sds
               name: sds
-            - mountPath: /tmp/
+            - mountPath: /cores/
               name: coredump
       dnsPolicy: ClusterFirst
       restartPolicy: Always
@@ -105,7 +105,7 @@ spec:
           name: sds
         - name: coredump
           hostPath:
-            path: /data/log
+            path: /var/gateway/proxy/data/
             type: DirectoryOrCreate
   revisionHistoryLimit: 10
   progressDeadlineSeconds: 600
diff --git a/internal/infrastructure/kubernetes/proxy/testdata/deployments/component-level.yaml b/internal/infrastructure/kubernetes/proxy/testdata/deployments/component-level.yaml
index ca3e2e5602a..fec49311997 100644
--- a/internal/infrastructure/kubernetes/proxy/testdata/deployments/component-level.yaml
+++ b/internal/infrastructure/kubernetes/proxy/testdata/deployments/component-level.yaml
@@ -82,7 +82,7 @@ spec:
               readOnly: true
             - mountPath: /sds
               name: sds
-            - mountPath: /tmp/
+            - mountPath: /cores/
               name: coredump
       dnsPolicy: ClusterFirst
       restartPolicy: Always
@@ -106,7 +106,7 @@ spec:
           name: sds
         - name: coredump
           hostPath:
-            path: /data/log
+            path: /var/gateway/proxy/data/
             type: DirectoryOrCreate
   revisionHistoryLimit: 10
   progressDeadlineSeconds: 600
diff --git a/internal/infrastructure/kubernetes/proxy/testdata/deployments/custom.yaml b/internal/infrastructure/kubernetes/proxy/testdata/deployments/custom.yaml
index e1b74093d10..61a53def1b2 100644
--- a/internal/infrastructure/kubernetes/proxy/testdata/deployments/custom.yaml
+++ b/internal/infrastructure/kubernetes/proxy/testdata/deployments/custom.yaml
@@ -191,7 +191,7 @@ spec:
               readOnly: true
             - mountPath: /sds
               name: sds
-            - mountPath: /tmp/
+            - mountPath: /cores/
               name: coredump
       dnsPolicy: ClusterFirst
       restartPolicy: Always
@@ -217,7 +217,7 @@ spec:
           name: sds
         - name: coredump
           hostPath:
-            path: /data/log
+            path: /var/gateway/proxy/data/
             type: DirectoryOrCreate
   revisionHistoryLimit: 10
   progressDeadlineSeconds: 600
diff --git a/internal/infrastructure/kubernetes/proxy/testdata/deployments/default-env.yaml b/internal/infrastructure/kubernetes/proxy/testdata/deployments/default-env.yaml
index c6a926d88f1..c3d5065a6f7 100644
--- a/internal/infrastructure/kubernetes/proxy/testdata/deployments/default-env.yaml
+++ b/internal/infrastructure/kubernetes/proxy/testdata/deployments/default-env.yaml
@@ -189,7 +189,7 @@ spec:
               readOnly: true
             - mountPath: /sds
               name: sds
-            - mountPath: /tmp/
+            - mountPath: /cores/
               name: coredump
       dnsPolicy: ClusterFirst
       restartPolicy: Always
@@ -215,7 +215,7 @@ spec:
           name: sds
         - name: coredump
           hostPath:
-            path: /data/log
+            path: /var/gateway/proxy/data/
             type: DirectoryOrCreate
   revisionHistoryLimit: 10
   progressDeadlineSeconds: 600
diff --git a/internal/infrastructure/kubernetes/proxy/testdata/deployments/default.yaml b/internal/infrastructure/kubernetes/proxy/testdata/deployments/default.yaml
index 0be18662307..d478148c38f 100644
--- a/internal/infrastructure/kubernetes/proxy/testdata/deployments/default.yaml
+++ b/internal/infrastructure/kubernetes/proxy/testdata/deployments/default.yaml
@@ -182,7 +182,7 @@ spec:
               readOnly: true
             - mountPath: /sds
               name: sds
-            - mountPath: /tmp/
+            - mountPath: /cores/
               name: coredump
       dnsPolicy: ClusterFirst
       restartPolicy: Always
@@ -206,7 +206,7 @@ spec:
           name: sds
         - name: coredump
           hostPath:
-            path: /data/log
+            path: /var/gateway/proxy/data/
             type: DirectoryOrCreate
   revisionHistoryLimit: 10
   progressDeadlineSeconds: 600
diff --git a/internal/infrastructure/kubernetes/proxy/testdata/deployments/enable-coredump.yaml b/internal/infrastructure/kubernetes/proxy/testdata/deployments/enable-coredump.yaml
index 6bba4d31d02..48f440873ea 100644
--- a/internal/infrastructure/kubernetes/proxy/testdata/deployments/enable-coredump.yaml
+++ b/internal/infrastructure/kubernetes/proxy/testdata/deployments/enable-coredump.yaml
@@ -81,22 +81,17 @@ spec:
               readOnly: true
             - mountPath: /sds
               name: sds
-            - mountPath: /tmp/
+            - mountPath: /cores/
               name: coredump
       initContainers:
-         - name: enable-core-dump
-           image: envoyproxy/envoy-dev:latest
-           command:
-             - /bin/sh
-           args:
-             - -c
-             - sysctl -w kernel.core_pattern=/tmp/core-%e-%p-%t && ulimit -c unlimited
-           imagePullPolicy: IfNotPresent
-           securityContext:
-             privileged: true
-             runAsUser:  0
-             runAsGroup: 0
-             runAsNonRoot: false
+        - name: condig-core-dump
+          image: envoyproxy/envoy-dev:latest
+          command:
+            - /bin/sh
+          args:
+            - -c
+            - sysctl -w kernel.core_pattern=/cores/core-%e-%p-%t && ulimit -c unlimited
+          imagePullPolicy: IfNotPresent
       dnsPolicy: ClusterFirst
       restartPolicy: Always
       schedulerName: default-scheduler
@@ -119,7 +114,7 @@ spec:
           name: sds
         - name: coredump
           hostPath:
-            path: /data/log
+            path: /var/gateway/proxy/data/
             type: DirectoryOrCreate
   revisionHistoryLimit: 10
   progressDeadlineSeconds: 600
diff --git a/internal/infrastructure/kubernetes/proxy/testdata/deployments/enable-prometheus.yaml b/internal/infrastructure/kubernetes/proxy/testdata/deployments/enable-prometheus.yaml
index f051823e829..1d1c5efdfe7 100644
--- a/internal/infrastructure/kubernetes/proxy/testdata/deployments/enable-prometheus.yaml
+++ b/internal/infrastructure/kubernetes/proxy/testdata/deployments/enable-prometheus.yaml
@@ -211,7 +211,7 @@ spec:
               readOnly: true
             - mountPath: /sds
               name: sds
-            - mountPath: /tmp/
+            - mountPath: /cores/
               name: coredump
       dnsPolicy: ClusterFirst
       restartPolicy: Always
@@ -235,7 +235,7 @@ spec:
           name: sds
         - name: coredump
           hostPath:
-            path: /data/log
+            path: /var/gateway/proxy/data/
             type: DirectoryOrCreate
   revisionHistoryLimit: 10
   progressDeadlineSeconds: 600
diff --git a/internal/infrastructure/kubernetes/proxy/testdata/deployments/extension-env.yaml b/internal/infrastructure/kubernetes/proxy/testdata/deployments/extension-env.yaml
index 97a5338e3f7..997938d7d82 100644
--- a/internal/infrastructure/kubernetes/proxy/testdata/deployments/extension-env.yaml
+++ b/internal/infrastructure/kubernetes/proxy/testdata/deployments/extension-env.yaml
@@ -193,7 +193,7 @@ spec:
               readOnly: true
             - mountPath: /sds
               name: sds
-            - mountPath: /tmp/
+            - mountPath: /cores/
               name: coredump
       dnsPolicy: ClusterFirst
       restartPolicy: Always
@@ -219,7 +219,7 @@ spec:
           name: sds
         - name: coredump
           hostPath:
-            path: /data/log
+            path: /var/gateway/proxy/data/
             type: DirectoryOrCreate
   revisionHistoryLimit: 10
   progressDeadlineSeconds: 600
diff --git a/internal/infrastructure/kubernetes/proxy/testdata/deployments/volumes.yaml b/internal/infrastructure/kubernetes/proxy/testdata/deployments/volumes.yaml
index 23f4b1a6ea6..c6906b999c0 100644
--- a/internal/infrastructure/kubernetes/proxy/testdata/deployments/volumes.yaml
+++ b/internal/infrastructure/kubernetes/proxy/testdata/deployments/volumes.yaml
@@ -193,7 +193,7 @@ spec:
               readOnly: true
             - mountPath: /sds
               name: sds
-            - mountPath: /tmp/
+            - mountPath: /cores/
               name: coredump
       dnsPolicy: ClusterFirst
       restartPolicy: Always
@@ -219,7 +219,7 @@ spec:
           name: sds
         - name: coredump
           hostPath:
-            path: /data/log
+            path: /var/gateway/proxy/data/
             type: DirectoryOrCreate
   revisionHistoryLimit: 10
   progressDeadlineSeconds: 600
diff --git a/internal/infrastructure/kubernetes/proxy/testdata/deployments/with-concurrency.yaml b/internal/infrastructure/kubernetes/proxy/testdata/deployments/with-concurrency.yaml
index 5dfef5aeb43..4a0903361c2 100644
--- a/internal/infrastructure/kubernetes/proxy/testdata/deployments/with-concurrency.yaml
+++ b/internal/infrastructure/kubernetes/proxy/testdata/deployments/with-concurrency.yaml
@@ -82,7 +82,7 @@ spec:
               readOnly: true
             - mountPath: /sds
               name: sds
-            - mountPath: /tmp/
+            - mountPath: /cores/
               name: coredump
       dnsPolicy: ClusterFirst
       restartPolicy: Always
@@ -106,7 +106,7 @@ spec:
           name: sds
         - name: coredump
           hostPath:
-            path: /data/log
+            path: /var/gateway/proxy/data/
             type: DirectoryOrCreate
   revisionHistoryLimit: 10
   progressDeadlineSeconds: 600