refactor: set defaults in Deployment, else k8s sets them for you, cre…

…ating infinite reconciliation loop (envoyproxy#1594) * fix: envoy proxy resource apply bug. Signed-off-by: qicz <qiczzhu@gmail.com> * update pointer. Signed-off-by: qicz <qiczzhu@gmail.com> * add comment Signed-off-by: qicz <qiczzhu@gmail.com> * update cm cmp logic. Signed-off-by: qicz <qiczzhu@gmail.com> * fix lint Signed-off-by: qicz <qiczzhu@gmail.com> * add probe field default value. Signed-off-by: qicz <qiczzhu@gmail.com> * fix uts Signed-off-by: qicz <qiczzhu@gmail.com> * align probe Signed-off-by: qicz <qiczzhu@gmail.com> * optimize deploy compare logic Signed-off-by: qicz <qiczzhu@gmail.com> * add compare deploy uts Signed-off-by: qicz <qiczzhu@gmail.com> * rm cm binarydata cmp Signed-off-by: qicz <qiczzhu@gmail.com> * rm deploy cmp logic Signed-off-by: qicz <qiczzhu@gmail.com> * fix ut Signed-off-by: qicz <qiczzhu@gmail.com> * fix lint Signed-off-by: qicz <qiczzhu@gmail.com> --------- Signed-off-by: qicz <qiczzhu@gmail.com> Signed-off-by: qi <qiczzhu@gmail.com>
arkodg · Jul 27, 2023 · 9ba9103 · 9ba9103
1 parent b37e2e9
commit 9ba9103
Show file tree

Hide file tree

Showing 25 changed files with 132 additions and 11 deletions.
diff --git a/internal/infrastructure/kubernetes/proxy/resource.go b/internal/infrastructure/kubernetes/proxy/resource.go
@@ -163,10 +163,15 @@ func expectedProxyContainers(infra *ir.ProxyInfra, deploymentConfig *egcfgv1a1.K
 			ReadinessProbe: &corev1.Probe{
 				ProbeHandler: corev1.ProbeHandler{
 					HTTPGet: &corev1.HTTPGetAction{
-						Path: bootstrap.EnvoyReadinessPath,
-						Port: intstr.IntOrString{Type: intstr.Int, IntVal: bootstrap.EnvoyReadinessPort},
+						Path:   bootstrap.EnvoyReadinessPath,
+						Port:   intstr.IntOrString{Type: intstr.Int, IntVal: bootstrap.EnvoyReadinessPort},
+						Scheme: corev1.URISchemeHTTP,
 					},
 				},
+				TimeoutSeconds:   1,
+				PeriodSeconds:    10,
+				SuccessThreshold: 1,
+				FailureThreshold: 3,
 			},
 		},
 	}
@@ -222,7 +227,8 @@ func expectedDeploymentVolumes(name string, deploymentSpec *egcfgv1a1.Kubernetes
 			Name: "certs",
 			VolumeSource: corev1.VolumeSource{
 				Secret: &corev1.SecretVolumeSource{
-					SecretName: "envoy",
+					SecretName:  "envoy",
+					DefaultMode: pointer.Int32(420),
 				},
 			},
 		},
@@ -243,7 +249,7 @@ func expectedDeploymentVolumes(name string, deploymentSpec *egcfgv1a1.Kubernetes
 							Path: SdsCertFilename,
 						},
 					},
-					DefaultMode: pointer.Int32(int32(420)),
+					DefaultMode: pointer.Int32(420),
 					Optional:    pointer.Bool(false),
 				},
 			},

diff --git a/internal/infrastructure/kubernetes/proxy/resource_provider.go b/internal/infrastructure/kubernetes/proxy/resource_provider.go
@@ -224,6 +224,8 @@ func (r *ResourceRender) Deployment() (*appsv1.Deployment, error) {
 					Volumes:                       expectedDeploymentVolumes(r.infra.Name, deploymentConfig),
 				},
 			},
+			RevisionHistoryLimit:    pointer.Int32(10),
+			ProgressDeadlineSeconds: pointer.Int32(600),
 		},
 	}
 

diff --git a/internal/infrastructure/kubernetes/proxy/resource_provider_test.go b/internal/infrastructure/kubernetes/proxy/resource_provider_test.go
@@ -208,7 +208,8 @@ func TestDeployment(t *testing.T) {
 							Name: "certs",
 							VolumeSource: corev1.VolumeSource{
 								Secret: &corev1.SecretVolumeSource{
-									SecretName: "custom-envoy-cert",
+									SecretName:  "custom-envoy-cert",
+									DefaultMode: pointer.Int32(420),
 								},
 							},
 						},

diff --git a/internal/infrastructure/kubernetes/proxy/testdata/deployments/bootstrap.yaml b/internal/infrastructure/kubernetes/proxy/testdata/deployments/bootstrap.yaml
@@ -67,6 +67,11 @@ spec:
             httpGet:
               path: /ready
               port: 19001
+              scheme: HTTP
+            timeoutSeconds: 1
+            periodSeconds: 10
+            successThreshold: 1
+            failureThreshold: 3
           terminationMessagePath: /dev/termination-log
           terminationMessagePolicy: File
           volumeMounts:
@@ -84,6 +89,7 @@ spec:
         - name: certs
           secret:
             secretName: envoy
+            defaultMode: 420
         - configMap:
             defaultMode: 420
             items:
@@ -94,3 +100,5 @@ spec:
             name: envoy-default-64656661
             optional: false
           name: sds
+  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
@@ -68,6 +68,11 @@ spec:
             httpGet:
               path: /ready
               port: 19001
+              scheme: HTTP
+            timeoutSeconds: 1
+            periodSeconds: 10
+            successThreshold: 1
+            failureThreshold: 3
           terminationMessagePath: /dev/termination-log
           terminationMessagePolicy: File
           volumeMounts:
@@ -85,6 +90,7 @@ spec:
         - name: certs
           secret:
             secretName: envoy
+            defaultMode: 420
         - configMap:
             defaultMode: 420
             items:
@@ -95,3 +101,5 @@ spec:
             name: envoy-default-64656661
             optional: false
           name: sds
+  revisionHistoryLimit: 10
+  progressDeadlineSeconds: 600
diff --git a/internal/infrastructure/kubernetes/proxy/testdata/deployments/custom.yaml b/internal/infrastructure/kubernetes/proxy/testdata/deployments/custom.yaml
@@ -171,6 +171,11 @@ spec:
             httpGet:
               path: /ready
               port: 19001
+              scheme: HTTP
+            timeoutSeconds: 1
+            periodSeconds: 10
+            successThreshold: 1
+            failureThreshold: 3
           terminationMessagePath: /dev/termination-log
           terminationMessagePolicy: File
           securityContext:
@@ -192,6 +197,7 @@ spec:
         - name: certs
           secret:
             secretName: envoy
+            defaultMode: 420
         - configMap:
             defaultMode: 420
             items:
@@ -202,3 +208,5 @@ spec:
             name: envoy-default-64656661
             optional: false
           name: sds
+  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
@@ -169,6 +169,11 @@ spec:
             httpGet:
               path: /ready
               port: 19001
+              scheme: HTTP
+            timeoutSeconds: 1
+            periodSeconds: 10
+            successThreshold: 1
+            failureThreshold: 3
           terminationMessagePath: /dev/termination-log
           terminationMessagePolicy: File
           securityContext:
@@ -190,6 +195,7 @@ spec:
         - name: certs
           secret:
             secretName: envoy
+            defaultMode: 420
         - configMap:
             defaultMode: 420
             items:
@@ -200,3 +206,5 @@ spec:
             name: envoy-default-64656661
             optional: false
           name: sds
+  revisionHistoryLimit: 10
+  progressDeadlineSeconds: 600
diff --git a/internal/infrastructure/kubernetes/proxy/testdata/deployments/default.yaml b/internal/infrastructure/kubernetes/proxy/testdata/deployments/default.yaml
@@ -164,6 +164,11 @@ spec:
             httpGet:
               path: /ready
               port: 19001
+              scheme: HTTP
+            timeoutSeconds: 1
+            periodSeconds: 10
+            successThreshold: 1
+            failureThreshold: 3
           terminationMessagePath: /dev/termination-log
           terminationMessagePolicy: File
           volumeMounts:
@@ -181,6 +186,7 @@ spec:
         - name: certs
           secret:
             secretName: envoy
+            defaultMode: 420
         - configMap:
             defaultMode: 420
             items:
@@ -191,3 +197,5 @@ spec:
             name: envoy-default-64656661
             optional: false
           name: sds
+  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
@@ -190,6 +190,11 @@ spec:
             httpGet:
               path: /ready
               port: 19001
+              scheme: HTTP
+            timeoutSeconds: 1
+            periodSeconds: 10
+            successThreshold: 1
+            failureThreshold: 3
           terminationMessagePath: /dev/termination-log
           terminationMessagePolicy: File
           volumeMounts:
@@ -207,6 +212,7 @@ spec:
         - name: certs
           secret:
             secretName: envoy
+            defaultMode: 420
         - configMap:
             defaultMode: 420
             items:
@@ -217,3 +223,5 @@ spec:
             name: envoy-default-64656661
             optional: false
           name: sds
+  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
@@ -173,6 +173,11 @@ spec:
             httpGet:
               path: /ready
               port: 19001
+              scheme: HTTP
+            timeoutSeconds: 1
+            periodSeconds: 10
+            successThreshold: 1
+            failureThreshold: 3
           terminationMessagePath: /dev/termination-log
           terminationMessagePolicy: File
           securityContext:
@@ -194,6 +199,7 @@ spec:
         - name: certs
           secret:
             secretName: envoy
+            defaultMode: 420
         - configMap:
             defaultMode: 420
             items:
@@ -204,3 +210,5 @@ spec:
             name: envoy-default-64656661
             optional: false
           name: sds
+  revisionHistoryLimit: 10
+  progressDeadlineSeconds: 600
diff --git a/internal/infrastructure/kubernetes/proxy/testdata/deployments/volumes.yaml b/internal/infrastructure/kubernetes/proxy/testdata/deployments/volumes.yaml
@@ -173,6 +173,11 @@ spec:
             httpGet:
               path: /ready
               port: 19001
+              scheme: HTTP
+            timeoutSeconds: 1
+            periodSeconds: 10
+            successThreshold: 1
+            failureThreshold: 3
           terminationMessagePath: /dev/termination-log
           terminationMessagePolicy: File
           securityContext:
@@ -194,6 +199,7 @@ spec:
         - name: certs
           secret:
             secretName: custom-envoy-cert
+            defaultMode: 420
         - configMap:
             defaultMode: 420
             items:
@@ -204,3 +210,5 @@ spec:
             name: envoy-default-64656661
             optional: false
           name: sds
+  revisionHistoryLimit: 10
+  progressDeadlineSeconds: 600
diff --git a/internal/infrastructure/kubernetes/proxy_infra_test.go b/internal/infrastructure/kubernetes/proxy_infra_test.go
@@ -7,8 +7,10 @@ package kubernetes
 
 import (
 	"context"
+	"reflect"
 	"testing"
 
+	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 	appsv1 "k8s.io/api/apps/v1"
 	corev1 "k8s.io/api/core/v1"
@@ -30,6 +32,17 @@ func newTestInfra(t *testing.T) *Infra {
 	return newTestInfraWithClient(t, cli)
 }
 
+func TestCmpBytes(t *testing.T) {
+	m1 := map[string][]byte{}
+	m1["a"] = []byte("aaa")
+	m2 := map[string][]byte{}
+	m2["a"] = []byte("aaa")
+
+	assert.True(t, reflect.DeepEqual(m1, m2))
+	assert.False(t, reflect.DeepEqual(nil, m2))
+	assert.False(t, reflect.DeepEqual(m1, nil))
+}
+
 func newTestInfraWithClient(t *testing.T, cli client.Client) *Infra {
 	cfg, err := config.New()
 	require.NoError(t, err)

diff --git a/internal/infrastructure/kubernetes/ratelimit/resource.go b/internal/infrastructure/kubernetes/ratelimit/resource.go
@@ -12,6 +12,7 @@ import (
 	"strconv"
 
 	corev1 "k8s.io/api/core/v1"
+	"k8s.io/utils/pointer"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 
 	egcfgv1a1 "github.com/envoyproxy/gateway/api/config/v1alpha1"
@@ -161,7 +162,8 @@ func expectedDeploymentVolumes(rateLimit *egcfgv1a1.RateLimit, rateLimitDeployme
 			Name: "redis-certs",
 			VolumeSource: corev1.VolumeSource{
 				Secret: &corev1.SecretVolumeSource{
-					SecretName: string(rateLimit.Backend.Redis.TLS.CertificateRef.Name),
+					SecretName:  string(rateLimit.Backend.Redis.TLS.CertificateRef.Name),
+					DefaultMode: pointer.Int32(420),
 				},
 			},
 		})
@@ -171,7 +173,8 @@ func expectedDeploymentVolumes(rateLimit *egcfgv1a1.RateLimit, rateLimitDeployme
 		Name: "certs",
 		VolumeSource: corev1.VolumeSource{
 			Secret: &corev1.SecretVolumeSource{
-				SecretName: "envoy-rate-limit",
+				SecretName:  "envoy-rate-limit",
+				DefaultMode: pointer.Int32(420),
 			},
 		},
 	})

diff --git a/internal/infrastructure/kubernetes/ratelimit/resource_provider.go b/internal/infrastructure/kubernetes/ratelimit/resource_provider.go
@@ -180,6 +180,8 @@ func (r *ResourceRender) Deployment() (*appsv1.Deployment, error) {
 					Tolerations:                   r.rateLimitDeployment.Pod.Tolerations,
 				},
 			},
+			RevisionHistoryLimit:    pointer.Int32(10),
+			ProgressDeadlineSeconds: pointer.Int32(600),
 		},
 	}
 

diff --git a/internal/infrastructure/kubernetes/ratelimit/resource_provider_test.go b/internal/infrastructure/kubernetes/ratelimit/resource_provider_test.go
@@ -444,7 +444,8 @@ func TestDeployment(t *testing.T) {
 							Name: "certs",
 							VolumeSource: corev1.VolumeSource{
 								Secret: &corev1.SecretVolumeSource{
-									SecretName: "custom-cert",
+									SecretName:  "custom-cert",
+									DefaultMode: pointer.Int32(420),
 								},
 							},
 						},

diff --git a/internal/infrastructure/kubernetes/ratelimit/testdata/deployments/affinity.yaml b/internal/infrastructure/kubernetes/ratelimit/testdata/deployments/affinity.yaml
@@ -115,3 +115,6 @@ spec:
       - name: certs
         secret:
           secretName: envoy-rate-limit
+          defaultMode: 420
+  revisionHistoryLimit: 10
+  progressDeadlineSeconds: 600
diff --git a/internal/infrastructure/kubernetes/ratelimit/testdata/deployments/custom.yaml b/internal/infrastructure/kubernetes/ratelimit/testdata/deployments/custom.yaml
@@ -106,3 +106,6 @@ spec:
       - name: certs
         secret:
           secretName: envoy-rate-limit
+          defaultMode: 420
+  revisionHistoryLimit: 10
+  progressDeadlineSeconds: 600
diff --git a/internal/infrastructure/kubernetes/ratelimit/testdata/deployments/default-env.yaml b/internal/infrastructure/kubernetes/ratelimit/testdata/deployments/default-env.yaml
@@ -106,3 +106,6 @@ spec:
       - name: certs
         secret:
           secretName: envoy-rate-limit
+          defaultMode: 420
+  revisionHistoryLimit: 10
+  progressDeadlineSeconds: 600
diff --git a/internal/infrastructure/kubernetes/ratelimit/testdata/deployments/default.yaml b/internal/infrastructure/kubernetes/ratelimit/testdata/deployments/default.yaml
@@ -97,3 +97,6 @@ spec:
       - name: certs
         secret:
           secretName: envoy-rate-limit
+          defaultMode: 420
+  revisionHistoryLimit: 10
+  progressDeadlineSeconds: 600
diff --git a/internal/infrastructure/kubernetes/ratelimit/testdata/deployments/extension-env.yaml b/internal/infrastructure/kubernetes/ratelimit/testdata/deployments/extension-env.yaml
@@ -110,3 +110,6 @@ spec:
       - name: certs
         secret:
           secretName: envoy-rate-limit
+          defaultMode: 420
+  revisionHistoryLimit: 10
+  progressDeadlineSeconds: 600
diff --git a/internal/infrastructure/kubernetes/ratelimit/testdata/deployments/override-env.yaml b/internal/infrastructure/kubernetes/ratelimit/testdata/deployments/override-env.yaml
@@ -106,3 +106,6 @@ spec:
       - name: certs
         secret:
           secretName: envoy-rate-limit
+          defaultMode: 420
+  revisionHistoryLimit: 10
+  progressDeadlineSeconds: 600
diff --git a/internal/infrastructure/kubernetes/ratelimit/testdata/deployments/redis-tls-settings.yaml b/internal/infrastructure/kubernetes/ratelimit/testdata/deployments/redis-tls-settings.yaml
@@ -117,6 +117,10 @@ spec:
       - name: redis-certs
         secret:
           secretName: ratelimit-cert
+          defaultMode: 420
       - name: certs
         secret:
           secretName: envoy-rate-limit
+          defaultMode: 420
+  revisionHistoryLimit: 10
+  progressDeadlineSeconds: 600