Merge release (#387)

* update CI pipeline for gitflow branch names

* PTEUDO-2142 remove validation of DSNName (#370)

Previously, customizing the dsnname would throw a validation error
    and prevent any processing of the databaseclaim. This allows the
    customization but ensures the normalized fields are always
    populated.

* update start-pgbouncer to exit after 60seconds (#379)

* DEVOPS-30046 DEVOPS-30239 dsnexec conversion injects dbproxy (#381)

A bug in the conversion webhook added dbproxy mutating labels
    when only dsnexec was requested. This would cause issues if
    the pod could not handle dbproxy listening on port 5432.

* DEVOPS-30046 optional sidecar probes (#384)

Make the probes optional as a fallback that the liveness probe
    failure issues are not resolved.

cmd/main.go

@@ -95,6 +95,8 @@ func main() {
 	var metricsConfigYamlPath string
 	var enableDBProxyWebhook bool
 	var enableDeprecatedConversionWebhook bool
+	var enableSidecarLivenessProbe bool
+	var enableSidecarReadinessProbe bool
 
 	flag.StringVar(&class, "class", "default", "The class of claims this db-controller instance needs to address.")
 
@@ -105,6 +107,9 @@ func main() {
 	flag.BoolVar(&enableDBProxyWebhook, "enable-db-proxy", false, "Enable DB Proxy webhook. See docs for usage: https://infobloxopen.github.io/db-controller/#quick-start")
 	flag.BoolVar(&enableDeprecatedConversionWebhook, "enable-deprecation-conversion-webhook", false, "Enable conversion of deprecated pods using dbproxy and/or dsnexec annotations")
 
+	flag.BoolVar(&enableSidecarLivenessProbe, "enable-sidecar-liveness-probe", false, "Enable liveness probe for dbproxy and dsnexec sidecars")
+	flag.BoolVar(&enableSidecarReadinessProbe, "enable-sidecar-readiness-probe", false, "Enable readiness probe for dbproxy and dsnexec sidecars")
+
 	opts := zap.Options{
 		Development: true,
 	}
@@ -257,10 +262,12 @@ func main() {
 	if enableDBProxyWebhook {
 
 		if err := mutating.SetupWebhookWithManager(mgr, mutating.SetupConfig{
-			Namespace:  namespace,
-			Class:      class,
-			DBProxyImg: os.Getenv("DBPROXY_IMAGE"),
-			DSNExecImg: os.Getenv("DSNEXEC_IMAGE"),
+			Namespace:      namespace,
+			Class:          class,
+			DBProxyImg:     os.Getenv("DBPROXY_IMAGE"),
+			DSNExecImg:     os.Getenv("DSNEXEC_IMAGE"),
+			EnableReady:    enableSidecarReadinessProbe,
+			EnableLiveness: enableSidecarLivenessProbe,
 		}); err != nil {
 			setupLog.Error(err, "failed to setup webhooks")
 			os.Exit(1)

helm/db-controller/templates/deployment.yaml

@@ -56,6 +56,8 @@ spec:
             - -zap-encoder={{ .Values.zapLogger.encoding }}
             - -zap-log-level={{ .Values.zapLogger.level }}
             - -zap-time-encoding={{ .Values.zapLogger.timeEncoding }}
+            - --enable-sidecar-liveness-probe={{ .Values.probes.liveness.enabled }}
+            - --enable-sidecar-readiness-probe={{ .Values.probes.readiness.enabled }}
           command:
             - /manager
           image: "{{ .Values.image.repository }}:{{ default .Chart.AppVersion .Values.image.tag }}"

helm/db-controller/values.yaml

@@ -187,5 +187,11 @@ controllerConfig:
     ib_env: "{{ .Values.env }}"
     ib_lifecycle: "{{ .Values.lifecycle }}"
 
+probes:
+  liveness:
+    enabled: false
+  readiness:
+    enabled: true
+
 dashboard:
   enabled: false

internal/controller/databaseclaim_controller_test.go

@@ -21,7 +21,6 @@ import (
 	"fmt"
 	"net/url"
 
-	"github.com/infobloxopen/db-controller/internal/dockerdb"
 	. "github.com/onsi/ginkgo/v2"
 	. "github.com/onsi/gomega"
 
@@ -33,7 +32,9 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/reconcile"
 
+	persistancev1 "github.com/infobloxopen/db-controller/api/v1"
 	v1 "github.com/infobloxopen/db-controller/api/v1"
+	"github.com/infobloxopen/db-controller/internal/dockerdb"
 	"github.com/infobloxopen/db-controller/pkg/hostparams"
 )
 
@@ -189,6 +190,42 @@ var _ = Describe("DatabaseClaim Controller", func() {
 			Expect(secret.Data[v1.DSNURIKey]).To(Equal(secret.Data["uri_fixme.txt"]))
 		})
 
+		It("Should have DSN and URIDSN keys populated", func() {
+			By("Reconciling the created resource")
+			_, err := controllerReconciler.Reconcile(ctx, reconcile.Request{NamespacedName: typeNamespacedName})
+			Expect(err).NotTo(HaveOccurred())
+
+			var claim persistancev1.DatabaseClaim
+			err = k8sClient.Get(ctx, typeNamespacedName, &claim)
+			Expect(err).NotTo(HaveOccurred())
+			Expect(claim.Status.Error).To(Equal(""))
+
+			By("Checking the user credentials secret")
+
+			secret := &corev1.Secret{}
+			err = k8sClient.Get(ctx, typeNamespacedSecretName, secret)
+			Expect(err).NotTo(HaveOccurred())
+
+			for _, key := range []string{v1.DSNKey, v1.DSNURIKey, "fixme.txt", "uri_fixme.txt"} {
+				Expect(secret.Data[key]).NotTo(BeNil())
+			}
+			oldKey := secret.Data[v1.DSNKey]
+			Expect(secret.Data[v1.DSNKey]).To(Equal(secret.Data["fixme.txt"]))
+			Expect(secret.Data[v1.DSNURIKey]).To(Equal(secret.Data["uri_fixme.txt"]))
+			// Slow down the test so creds are rotated, 60ns rotation time
+			By("Rotate passwords and verify credentials are updated")
+			_, err = controllerReconciler.Reconcile(ctx, reconcile.Request{NamespacedName: typeNamespacedName})
+			Expect(err).NotTo(HaveOccurred())
+
+			err = k8sClient.Get(ctx, typeNamespacedSecretName, secret)
+			Expect(err).NotTo(HaveOccurred())
+
+			Expect(secret.Data[v1.DSNKey]).NotTo(Equal(oldKey))
+			Expect(secret.Data[v1.DSNKey]).To(Equal(secret.Data["fixme.txt"]))
+			Expect(secret.Data[v1.DSNURIKey]).To(Equal(secret.Data["uri_fixme.txt"]))
+
+		})
+
 		It("Should succeed with no error status to reconcile CR with DBVersion", func() {
 			By("Updating the DatabaseClaim resource with a DB Version 13.3")
 			resource := &v1.DatabaseClaim{}

internal/webhook/dbproxy.go

@@ -13,7 +13,7 @@ var (
 	ContainerNameProxy = "dbproxy"
 )
 
-func mutatePodProxy(ctx context.Context, pod *corev1.Pod, secretName string, dbProxyImg string) error {
+func mutatePodProxy(ctx context.Context, pod *corev1.Pod, secretName string, dbProxyImg string, enableReady, enableLiveness bool) error {
 
 	if pod.Annotations == nil {
 		pod.Annotations = map[string]string{}
@@ -52,20 +52,9 @@ func mutatePodProxy(ctx context.Context, pod *corev1.Pod, secretName string, dbP
 		return nil
 	}
 
-	pod.Spec.Containers = append(pod.Spec.Containers, corev1.Container{
-		Name:            ContainerNameProxy,
-		Image:           dbProxyImg,
-		ImagePullPolicy: corev1.PullIfNotPresent,
-
-		Env: []corev1.EnvVar{
-			{
-				Name:  "DBPROXY_CREDENTIAL",
-				Value: fmt.Sprintf("/dbproxy/%s", SecretKey),
-			},
-		},
-		// Test pgbouncer
-		ReadinessProbe: &corev1.Probe{
-
+	var readinessProbe *corev1.Probe
+	if enableReady {
+		readinessProbe = &corev1.Probe{
 			ProbeHandler: corev1.ProbeHandler{
 				Exec: &corev1.ExecAction{
 					Command: []string{
@@ -78,11 +67,13 @@ func mutatePodProxy(ctx context.Context, pod *corev1.Pod, secretName string, dbP
 			InitialDelaySeconds: 5,
 			PeriodSeconds:       15,
 			TimeoutSeconds:      5,
-		},
-		// FIXME: turn these back on when timeouts can be tuned. It was restarting
-		// the pod too often.
-		// Test connection to upstream database
-		LivenessProbe: &corev1.Probe{
+		}
+	}
+
+	var livenessProbe *corev1.Probe
+
+	if enableLiveness {
+		livenessProbe = &corev1.Probe{
 			ProbeHandler: corev1.ProbeHandler{
 				Exec: &corev1.ExecAction{
 					Command: []string{
@@ -95,7 +86,24 @@ func mutatePodProxy(ctx context.Context, pod *corev1.Pod, secretName string, dbP
 			InitialDelaySeconds: 30,
 			PeriodSeconds:       15,
 			TimeoutSeconds:      5,
+		}
+	}
+
+	pod.Spec.Containers = append(pod.Spec.Containers, corev1.Container{
+		Name:            ContainerNameProxy,
+		Image:           dbProxyImg,
+		ImagePullPolicy: corev1.PullIfNotPresent,
+
+		Env: []corev1.EnvVar{
+			{
+				Name:  "DBPROXY_CREDENTIAL",
+				Value: fmt.Sprintf("/dbproxy/%s", SecretKey),
+			},
 		},
+		// Test connection to upstream database
+		LivenessProbe: livenessProbe,
+		// Test pgbouncer
+		ReadinessProbe: readinessProbe,
 		VolumeMounts: []corev1.VolumeMount{
 			{
 				Name:      VolumeNameProxy,

internal/webhook/dbproxy_test.go

@@ -128,6 +128,9 @@ var _ = Describe("dbproxy defaulting", func() {
 		Expect(sidecar.VolumeMounts[0].Name).To(Equal(VolumeNameProxy))
 		Expect(sidecar.VolumeMounts[0].MountPath).To(Equal(MountPathProxy))
 		Expect(sidecar.VolumeMounts[0].ReadOnly).To(BeTrue())
+		Expect(sidecar.ReadinessProbe).ToNot(BeNil())
+		Expect(sidecar.LivenessProbe).ToNot(BeNil())
+
 	})
 
 	It("pre-mutated pods are not re-mutated", func() {
@@ -147,7 +150,7 @@ var _ = Describe("dbproxy defaulting", func() {
 
 func makeMutatedPodProxy(name, claimName, secretName string) *corev1.Pod {
 	pod := makePodProxy(name, claimName)
-	Expect(mutatePodProxy(context.TODO(), pod, secretName, sidecarImageProxy)).To(Succeed())
+	Expect(mutatePodProxy(context.TODO(), pod, secretName, sidecarImageProxy, true, true)).To(Succeed())
 	return pod
 }
 

internal/webhook/defaulter.go

@@ -33,18 +33,22 @@ var (
 
 // podAnnotator annotates Pods
 type podAnnotator struct {
-	class      string
-	namespace  string
-	dbProxyImg string
-	dsnExecImg string
-	k8sClient  client.Reader
+	class          string
+	namespace      string
+	dbProxyImg     string
+	dsnExecImg     string
+	k8sClient      client.Reader
+	enableReady    bool
+	enableLiveness bool
 }
 
 type SetupConfig struct {
-	Namespace  string
-	Class      string
-	DBProxyImg string
-	DSNExecImg string
+	Namespace      string
+	Class          string
+	DBProxyImg     string
+	DSNExecImg     string
+	EnableReady    bool
+	EnableLiveness bool
 }
 
 func SetupWebhookWithManager(mgr ctrl.Manager, cfg SetupConfig) error {
@@ -68,11 +72,13 @@ func SetupWebhookWithManager(mgr ctrl.Manager, cfg SetupConfig) error {
 	return ctrl.NewWebhookManagedBy(mgr).
 		For(&corev1.Pod{}).
 		WithDefaulter(&podAnnotator{
-			namespace:  cfg.Namespace,
-			class:      cfg.Class,
-			dbProxyImg: cfg.DBProxyImg,
-			dsnExecImg: cfg.DSNExecImg,
-			k8sClient:  mgr.GetClient(),
+			namespace:      cfg.Namespace,
+			class:          cfg.Class,
+			dbProxyImg:     cfg.DBProxyImg,
+			dsnExecImg:     cfg.DSNExecImg,
+			k8sClient:      mgr.GetClient(),
+			enableReady:    cfg.EnableReady,
+			enableLiveness: cfg.EnableLiveness,
 		}).
 		Complete()
 }
@@ -133,13 +139,13 @@ func (p *podAnnotator) Default(ctx context.Context, obj runtime.Object) error {
 
 	if pod.Labels[LabelCheckProxy] == "enabled" &&
 		pod.Annotations[AnnotationInjectedProxy] != "true" {
-		err = mutatePodProxy(ctx, pod, secretName, p.dbProxyImg)
+		err = mutatePodProxy(ctx, pod, secretName, p.dbProxyImg, p.enableReady, p.enableLiveness)
 		if err == nil {
 			log.Info("mutated_pod_dbproxy")
 		}
 	} else if pod.Labels[LabelCheckExec] == "enabled" &&
 		pod.Annotations[AnnotationInjectedExec] != "true" {
-		err = mutatePodExec(ctx, pod, secretName, p.dsnExecImg)
+		err = mutatePodExec(ctx, pod, secretName, p.dsnExecImg, p.enableReady, p.enableLiveness)
 		if err == nil {
 			log.Info("mutated_pod_dsnexec")
 		}

internal/webhook/dsnexec.go

@@ -16,7 +16,7 @@ var (
 	VolumeNameExecConfig = "dsnexec-config"
 )
 
-func mutatePodExec(ctx context.Context, pod *corev1.Pod, secretName string, dsnExecImg string) error {
+func mutatePodExec(ctx context.Context, pod *corev1.Pod, secretName string, dsnExecImg string, enableReady, enableLiveness bool) error {
 
 	if pod.Annotations == nil {
 		pod.Annotations = map[string]string{}
@@ -75,6 +75,44 @@ func mutatePodExec(ctx context.Context, pod *corev1.Pod, secretName string, dsnE
 		return nil
 	}
 
+	var readinessProbe *corev1.Probe
+	if enableReady {
+		readinessProbe = &corev1.Probe{
+
+			ProbeHandler: corev1.ProbeHandler{
+				Exec: &corev1.ExecAction{
+					Command: []string{
+						"/bin/sh",
+						"-c",
+						"psql -h localhost -c \"SELECT 1\"",
+					},
+				},
+			},
+			InitialDelaySeconds: 5,
+			PeriodSeconds:       15,
+			TimeoutSeconds:      5,
+		}
+	}
+
+	var livenessProbe *corev1.Probe
+
+	if enableLiveness {
+		livenessProbe = &corev1.Probe{
+			ProbeHandler: corev1.ProbeHandler{
+				Exec: &corev1.ExecAction{
+					Command: []string{
+						"/bin/sh",
+						"-c",
+						fmt.Sprintf("psql \"$(cat /dbproxy/%s)\" -c \"SELECT 1\"", SecretKey),
+					},
+				},
+			},
+			InitialDelaySeconds: 30,
+			PeriodSeconds:       15,
+			TimeoutSeconds:      5,
+		}
+	}
+
 	pod.Spec.Containers = append(pod.Spec.Containers, corev1.Container{
 		Name:            ContainerNameExec,
 		Image:           dsnExecImg,
@@ -91,20 +129,9 @@ func mutatePodExec(ctx context.Context, pod *corev1.Pod, secretName string, dsnE
 			},
 		},
 		// Test connection to upstream database
-		LivenessProbe: &corev1.Probe{
-			ProbeHandler: corev1.ProbeHandler{
-				Exec: &corev1.ExecAction{
-					Command: []string{
-						"/bin/sh",
-						"-c",
-						fmt.Sprintf("psql \"$(cat %s/%s)\" -c \"SELECT 1\"", MountPathExec, SecretKey),
-					},
-				},
-			},
-			InitialDelaySeconds: 30,
-			PeriodSeconds:       15,
-			TimeoutSeconds:      5,
-		},
+		LivenessProbe: livenessProbe,
+		// Test connection to pgbouncer
+		ReadinessProbe: readinessProbe,
 		VolumeMounts: []corev1.VolumeMount{
 			{
 				Name:      VolumeNameExec,

internal/webhook/dsnexec_test.go

@@ -126,6 +126,8 @@ var _ = Describe("dsnexec defaulting", func() {
 		Expect(sidecar.VolumeMounts[0].Name).To(Equal(VolumeNameExec))
 		Expect(sidecar.VolumeMounts[0].MountPath).To(Equal(MountPathExec))
 		Expect(sidecar.VolumeMounts[0].ReadOnly).To(BeTrue())
+		Expect(sidecar.ReadinessProbe).ToNot(BeNil())
+		Expect(sidecar.LivenessProbe).ToNot(BeNil())
 	})
 
 	It("pre-mutated pods are not re-mutated", func() {
@@ -145,7 +147,7 @@ var _ = Describe("dsnexec defaulting", func() {
 
 func makeMutatedPodExec(name, claimName, secretName string) *corev1.Pod {
 	pod := makePodExec(name, claimName)
-	Expect(mutatePodExec(context.TODO(), pod, secretName, sidecarImageExec)).To(Succeed())
+	Expect(mutatePodExec(context.TODO(), pod, secretName, sidecarImageExec, true, true)).To(Succeed())
 	return pod
 
 }

internal/webhook/suite_test.go

@@ -106,10 +106,12 @@ var _ = BeforeSuite(func() {
 	Expect(err).NotTo(HaveOccurred())
 
 	err = SetupWebhookWithManager(mgr, SetupConfig{
-		DBProxyImg: sidecarImageProxy,
-		DSNExecImg: sidecarImageExec,
-		Namespace:  "default",
-		Class:      "default",
+		DBProxyImg:     sidecarImageProxy,
+		DSNExecImg:     sidecarImageExec,
+		Namespace:      "default",
+		Class:          "default",
+		EnableReady:    true,
+		EnableLiveness: true,
 	})
 	Expect(err).NotTo(HaveOccurred())