Repository: percona/percona-xtradb-cluster-operator

Issue:
pxc container's `livenessProbe` can not be changed by PerconaXtraDBCluster's `livenessProbes`
As the title. 

Discussion:
@awlleon: readinessProbes is also like this, but it works fine in version 1.11 of PXC.@spron-in: Hey folks. I was able to reproduce it.

I changed it in cr.yaml:
```
pxc:
...
    readinessProbes:
      timeoutSeconds: 16
      periodSeconds: 31

    livenessProbes:
      timeoutSeconds: 6
      periodSeconds: 11
```

I see it is properly set in the custom resource in k8s.

But it is not reflected in the container. 
```
    livenessProbe:
      exec:
        command:
        - /var/lib/mysql/liveness-check.sh
      failureThreshold: 3
      initialDelaySeconds: 300
      periodSeconds: 10
      successThreshold: 1
      timeoutSeconds: 5
```

At the same time I see env variables are set:
```
   - name: LIVENESS_CHECK_TIMEOUT
      value: "6"
    - name: READINESS_CHECK_TIMEOUT
      value: "16"
```

@egegunes @hors any thoughts before I create a JIRA bug?@hors: @spron-in @hulucc it was fixed under https://perconadev.atlassian.net/browse/K8SPXC-1345 and will be available in the next release.@spron-in: Awesome! Thanks @hors !@johnlaur: Thank you @hors ! When we restore a foreign backup onto a pxc-operator managed cluster the inability to adjust the readiness probes gave a very slim window where we could connect into the pxc container of the pod and get the grants restored before the failing probe took out the container. I discovered this issue attempting to lengthen the timeout.

It would be beneficial if the operator could detect and possibly remediate the condition where the grants required by the operator are not present in the database; however I do not know if that is already possible by some existing mechanism I have not yet discovered. Is it possible to use a prestop hook on the restore job I wonder?@hors: > Thank you [@hors](https://github.com/hors) ! When we restore a foreign backup onto a pxc-operator managed cluster the inability to adjust the readiness probes gave a very slim window where we could connect into the pxc container of the pod and get the grants restored before the failing probe took out the container. I discovered this issue attempting to lengthen the timeout.
> 
> It would be beneficial if the operator could detect and possibly remediate the condition where the grants required by the operator are not present in the database; however I do not know if that is already possible by some existing mechanism I have not yet discovered. Is it possible to use a prestop hook on the restore job I wonder?

@johnlaur I think this improvement https://perconadev.atlassian.net/browse/K8SPXC-1376 should fix your issue. You can test it using main branch (but please do not use it in production).@hors: @johnlaur The PXCO 1.18.0 was released with this bug fix.  

Solution patch:
diff --git a/Makefile b/Makefile
index 25808619..5af85ea2 100644
--- a/Makefile
+++ b/Makefile
@@ -35,7 +35,7 @@ vet: ## Run go vet against code.
 	go vet ./...
 
 test: manifests generate fmt vet envtest ## Run tests.
-	KUBEBUILDER_ASSETS="$(shell $(ENVTEST) --arch=amd64 use $(ENVTEST_K8S_VERSION) -p path)" go test ./... -coverprofile cover.out
+	DISABLE_TELEMETRY=true KUBEBUILDER_ASSETS="$(shell $(ENVTEST) --arch=amd64 use $(ENVTEST_K8S_VERSION) -p path)" go test ./... -coverprofile cover.out
 
 ##@ Build
 
diff --git a/pkg/apis/pxc/v1/pxc_types.go b/pkg/apis/pxc/v1/pxc_types.go
index 720676ec..281c543f 100644
--- a/pkg/apis/pxc/v1/pxc_types.go
+++ b/pkg/apis/pxc/v1/pxc_types.go
@@ -1054,7 +1054,13 @@ func (cr *PerconaXtraDBCluster) setProbesDefaults() {
 		cr.Spec.PXC.LivenessProbes.TimeoutSeconds = 5
 	}
 
-	cr.Spec.PXC.LivenessProbes.SuccessThreshold = 1
+	if cr.Spec.PXC.LivenessProbes.FailureThreshold == 0 {
+		cr.Spec.PXC.LivenessProbes.FailureThreshold = 3
+	}
+
+	if cr.Spec.PXC.LivenessProbes.SuccessThreshold == 0 {
+		cr.Spec.PXC.LivenessProbes.SuccessThreshold = 1
+	}
 
 	if cr.Spec.PXC.ReadinessInitialDelaySeconds != nil {
 		cr.Spec.PXC.ReadinessProbes.InitialDelaySeconds = *cr.Spec.PXC.ReadinessInitialDelaySeconds
@@ -1069,6 +1075,11 @@ func (cr *PerconaXtraDBCluster) setProbesDefaults() {
 	if cr.Spec.PXC.ReadinessProbes.FailureThreshold == 0 {
 		cr.Spec.PXC.ReadinessProbes.FailureThreshold = 5
 	}
+
+	if cr.Spec.PXC.ReadinessProbes.SuccessThreshold == 0 {
+		cr.Spec.PXC.ReadinessProbes.SuccessThreshold = 1
+	}
+
 	if cr.Spec.PXC.ReadinessProbes.TimeoutSeconds == 0 {
 		cr.Spec.PXC.ReadinessProbes.TimeoutSeconds = 15
 	}
@@ -1087,6 +1098,14 @@ func (cr *PerconaXtraDBCluster) setProbesDefaults() {
 			cr.Spec.HAProxy.ReadinessProbes.TimeoutSeconds = 1
 		}
 
+		if cr.Spec.HAProxy.ReadinessProbes.SuccessThreshold == 0 {
+			cr.Spec.HAProxy.ReadinessProbes.SuccessThreshold = 1
+		}
+
+		if cr.Spec.HAProxy.ReadinessProbes.FailureThreshold == 0 {
+			cr.Spec.HAProxy.ReadinessProbes.FailureThreshold = 3
+		}
+
 		if cr.Spec.HAProxy.LivenessInitialDelaySeconds != nil {
 			cr.Spec.HAProxy.LivenessProbes.InitialDelaySeconds = *cr.Spec.HAProxy.LivenessInitialDelaySeconds
 		} else if cr.Spec.HAProxy.LivenessProbes.InitialDelaySeconds == 0 {
@@ -1103,8 +1122,9 @@ func (cr *PerconaXtraDBCluster) setProbesDefaults() {
 			cr.Spec.HAProxy.LivenessProbes.PeriodSeconds = 30
 		}
 
-		cr.Spec.HAProxy.LivenessProbes.SuccessThreshold = 1
-
+		if cr.Spec.HAProxy.LivenessProbes.SuccessThreshold == 0 {
+			cr.Spec.HAProxy.LivenessProbes.SuccessThreshold = 1
+		}
 	}
 }
 
diff --git a/pkg/pxc/app/statefulset/node.go b/pkg/pxc/app/statefulset/node.go
index a77f289d..37c4d461 100644
--- a/pkg/pxc/app/statefulset/node.go
+++ b/pkg/pxc/app/statefulset/node.go
@@ -254,6 +254,9 @@ func (c *Node) AppContainer(spec *api.PodSpec, secrets string, cr *api.PerconaXt
 			Name:      "mysql-init-file",
 			MountPath: "/etc/mysql/init-file",
 		})
+
+		appc.ReadinessProbe = app.Probe(&cr.Spec.PXC.ReadinessProbes, "/var/lib/mysql/readiness-check.sh")
+		appc.LivenessProbe = app.Probe(&cr.Spec.PXC.LivenessProbes, "/var/lib/mysql/liveness-check.sh")
 	}
 
 	if cr.Spec.PXC != nil && (cr.Spec.PXC.Lifecycle.PostStart != nil || cr.Spec.PXC.Lifecycle.PreStop != nil) {
diff --git a/pkg/pxc/users/users.go b/pkg/pxc/users/users.go
index ce5392a9..e2ad6050 100644
--- a/pkg/pxc/users/users.go
+++ b/pkg/pxc/users/users.go
@@ -255,7 +255,7 @@ func (u *Manager) Update170XtrabackupUser(pass string) (err error) {
 	return nil
 }
 
-// Update1100MonitorUserPrivilege grants system_user privilege for monitor 
+// Update1100MonitorUserPrivilege grants system_user privilege for monitor
 func (u *Manager) Update1100MonitorUserPrivilege() (err error) {
 	if _, err := u.db.Exec("GRANT SYSTEM_USER ON *.* TO 'monitor'@'%'"); err != nil {
 		return errors.Wrap(err, "monitor user")



Test patch:
diff --git a/pkg/controller/pxc/controller_test.go b/pkg/controller/pxc/controller_test.go
index fecfd75d..7ce6772f 100644
--- a/pkg/controller/pxc/controller_test.go
+++ b/pkg/controller/pxc/controller_test.go
@@ -1426,3 +1426,278 @@ var _ = Describe("PostStart/PreStop lifecycle hooks", Ordered, func() {
 		})
 	})
 })
+
+var _ = Describe("Liveness/Readiness Probes", Ordered, func() {
+	ctx := context.Background()
+
+	const ns = "probes"
+	namespace := &corev1.Namespace{
+		ObjectMeta: metav1.ObjectMeta{
+			Name:      ns,
+			Namespace: ns,
+		},
+	}
+
+	BeforeAll(func() {
+		By("Creating the Namespace to perform the tests")
+		err := k8sClient.Create(ctx, namespace)
+		Expect(err).To(Not(HaveOccurred()))
+	})
+
+	AfterAll(func() {
+		By("Deleting the Namespace to perform the tests")
+		_ = k8sClient.Delete(ctx, namespace)
+	})
+
+	defaultReadiness := corev1.Probe{
+		ProbeHandler: corev1.ProbeHandler{
+			Exec: &corev1.ExecAction{
+				Command: []string{
+					"/var/lib/mysql/readiness-check.sh",
+				},
+			},
+		},
+		InitialDelaySeconds: int32(15),
+		TimeoutSeconds:      int32(15),
+		PeriodSeconds:       int32(30),
+		SuccessThreshold:    int32(1),
+		FailureThreshold:    int32(5),
+	}
+	defaultLiveness := corev1.Probe{
+		ProbeHandler: corev1.ProbeHandler{
+			Exec: &corev1.ExecAction{
+				Command: []string{
+					"/var/lib/mysql/liveness-check.sh",
+				},
+			},
+		},
+		InitialDelaySeconds: int32(300),
+		TimeoutSeconds:      int32(5),
+		PeriodSeconds:       int32(10),
+		SuccessThreshold:    int32(1),
+		FailureThreshold:    int32(3),
+	}
+
+	DescribeTable("PXC probes",
+		func(probes func() (corev1.Probe, corev1.Probe)) {
+			const crName = "probes"
+			crNamespacedName := types.NamespacedName{Name: crName, Namespace: ns}
+
+			cr, err := readDefaultCR(crName, ns)
+			Expect(err).NotTo(HaveOccurred())
+
+			cr.ObjectMeta.Finalizers = []string{}
+
+			readiness, liveness := probes()
+			cr.Spec.PXC.ReadinessProbes = readiness
+			cr.Spec.PXC.LivenessProbes = liveness
+
+			Expect(k8sClient.Create(ctx, cr)).Should(Succeed())
+
+			_, err = reconciler().Reconcile(ctx, ctrl.Request{NamespacedName: crNamespacedName})
+			Expect(err).NotTo(HaveOccurred())
+
+			sts := appsv1.StatefulSet{}
+			err = k8sClient.Get(ctx, types.NamespacedName{Name: "probes-pxc", Namespace: ns}, &sts)
+			Expect(err).NotTo(HaveOccurred())
+
+			for _, ct := range sts.Spec.Template.Spec.Containers {
+				if ct.Name != "pxc" {
+					continue
+				}
+
+				Expect(*ct.ReadinessProbe).To(Equal(readiness))
+				Expect(*ct.LivenessProbe).To(Equal(liveness))
+			}
+
+			Expect(k8sClient.Delete(ctx, cr)).Should(Succeed())
+		},
+		Entry("[readiness] custom initial delay seconds", func() (corev1.Probe, corev1.Probe) {
+			readiness := defaultReadiness.DeepCopy()
+			readiness.InitialDelaySeconds = defaultReadiness.InitialDelaySeconds + 10
+
+			return *readiness, defaultLiveness
+		}),
+		Entry("[readiness] custom timeout seconds", func() (corev1.Probe, corev1.Probe) {
+			readiness := defaultReadiness.DeepCopy()
+			readiness.TimeoutSeconds = defaultReadiness.TimeoutSeconds + 10
+
+			return *readiness, defaultLiveness
+		}),
+		Entry("[readiness] custom period seconds", func() (corev1.Probe, corev1.Probe) {
+			readiness := defaultReadiness.DeepCopy()
+			readiness.PeriodSeconds = defaultReadiness.PeriodSeconds + 10
+
+			return *readiness, defaultLiveness
+		}),
+		Entry("[readiness] custom success threshold", func() (corev1.Probe, corev1.Probe) {
+			readiness := defaultReadiness.DeepCopy()
+			readiness.SuccessThreshold = defaultReadiness.SuccessThreshold + 1
+
+			return *readiness, defaultLiveness
+		}),
+		Entry("[readiness] custom failure threshold", func() (corev1.Probe, corev1.Probe) {
+			readiness := defaultReadiness.DeepCopy()
+			readiness.FailureThreshold = defaultReadiness.FailureThreshold + 1
+
+			return *readiness, defaultLiveness
+		}),
+		Entry("[liveness] custom initial delay seconds", func() (corev1.Probe, corev1.Probe) {
+			liveness := defaultLiveness.DeepCopy()
+			liveness.InitialDelaySeconds = defaultLiveness.InitialDelaySeconds + 10
+
+			return defaultReadiness, *liveness
+		}),
+		Entry("[liveness] custom timeout seconds", func() (corev1.Probe, corev1.Probe) {
+			liveness := defaultLiveness.DeepCopy()
+			liveness.TimeoutSeconds = defaultLiveness.TimeoutSeconds + 10
+
+			return defaultReadiness, *liveness
+		}),
+		Entry("[liveness] custom period seconds", func() (corev1.Probe, corev1.Probe) {
+			liveness := defaultLiveness.DeepCopy()
+			liveness.PeriodSeconds = defaultLiveness.PeriodSeconds + 10
+
+			return defaultReadiness, *liveness
+		}),
+		Entry("[liveness] custom success threshold", func() (corev1.Probe, corev1.Probe) {
+			liveness := defaultLiveness.DeepCopy()
+			liveness.SuccessThreshold = defaultLiveness.SuccessThreshold + 1
+
+			return defaultReadiness, *liveness
+		}),
+		Entry("[liveness] custom failure threshold", func() (corev1.Probe, corev1.Probe) {
+			liveness := defaultLiveness.DeepCopy()
+			liveness.FailureThreshold = defaultLiveness.FailureThreshold + 1
+
+			return defaultReadiness, *liveness
+		}),
+	)
+
+	defaultHAProxyReadiness := corev1.Probe{
+		ProbeHandler: corev1.ProbeHandler{
+			Exec: &corev1.ExecAction{
+				Command: []string{
+					"/usr/local/bin/readiness-check.sh",
+				},
+			},
+		},
+		InitialDelaySeconds: int32(15),
+		TimeoutSeconds:      int32(1),
+		PeriodSeconds:       int32(5),
+		SuccessThreshold:    int32(1),
+		FailureThreshold:    int32(3),
+	}
+	defaultHAProxyLiveness := corev1.Probe{
+		ProbeHandler: corev1.ProbeHandler{
+			Exec: &corev1.ExecAction{
+				Command: []string{
+					"/usr/local/bin/liveness-check.sh",
+				},
+			},
+		},
+		InitialDelaySeconds: int32(60),
+		TimeoutSeconds:      int32(5),
+		PeriodSeconds:       int32(30),
+		SuccessThreshold:    int32(1),
+		FailureThreshold:    int32(4),
+	}
+
+	DescribeTable("HAProxy probes",
+		func(probes func() (corev1.Probe, corev1.Probe)) {
+			const crName = "probes"
+			crNamespacedName := types.NamespacedName{Name: crName, Namespace: ns}
+
+			cr, err := readDefaultCR(crName, ns)
+			Expect(err).NotTo(HaveOccurred())
+
+			cr.ObjectMeta.Finalizers = []string{}
+			cr.Spec.HAProxy.Enabled = true
+			cr.Spec.ProxySQL.Enabled = false
+
+			readiness, liveness := probes()
+			cr.Spec.HAProxy.ReadinessProbes = readiness
+			cr.Spec.HAProxy.LivenessProbes = liveness
+
+			Expect(k8sClient.Create(ctx, cr)).Should(Succeed())
+
+			_, err = reconciler().Reconcile(ctx, ctrl.Request{NamespacedName: crNamespacedName})
+			Expect(err).NotTo(HaveOccurred())
+
+			sts := appsv1.StatefulSet{}
+			err = k8sClient.Get(ctx, types.NamespacedName{Name: "probes-haproxy", Namespace: ns}, &sts)
+			Expect(err).NotTo(HaveOccurred())
+
+			for _, ct := range sts.Spec.Template.Spec.Containers {
+				if ct.Name != "haproxy" {
+					continue
+				}
+
+				Expect(*ct.ReadinessProbe).To(Equal(readiness))
+				Expect(*ct.LivenessProbe).To(Equal(liveness))
+			}
+
+			Expect(k8sClient.Delete(ctx, cr)).Should(Succeed())
+		},
+		Entry("[readiness] custom initial delay seconds", func() (corev1.Probe, corev1.Probe) {
+			readiness := defaultHAProxyReadiness.DeepCopy()
+			readiness.InitialDelaySeconds = defaultHAProxyReadiness.InitialDelaySeconds + 10
+
+			return *readiness, defaultHAProxyLiveness
+		}),
+		Entry("[readiness] custom timeout seconds", func() (corev1.Probe, corev1.Probe) {
+			readiness := defaultHAProxyReadiness.DeepCopy()
+			readiness.TimeoutSeconds = defaultHAProxyReadiness.TimeoutSeconds + 10
+
+			return *readiness, defaultHAProxyLiveness
+		}),
+		Entry("[readiness] custom period seconds", func() (corev1.Probe, corev1.Probe) {
+			readiness := defaultHAProxyReadiness.DeepCopy()
+			readiness.PeriodSeconds = defaultHAProxyReadiness.PeriodSeconds + 10
+
+			return *readiness, defaultHAProxyLiveness
+		}),
+		Entry("[readiness] custom success threshold", func() (corev1.Probe, corev1.Probe) {
+			readiness := defaultHAProxyReadiness.DeepCopy()
+			readiness.SuccessThreshold = defaultHAProxyReadiness.SuccessThreshold + 1
+
+			return *readiness, defaultHAProxyLiveness
+		}),
+		Entry("[readiness] custom failure threshold", func() (corev1.Probe, corev1.Probe) {
+			readiness := defaultHAProxyReadiness.DeepCopy()
+			readiness.FailureThreshold = defaultHAProxyReadiness.FailureThreshold + 1
+
+			return *readiness, defaultHAProxyLiveness
+		}),
+		Entry("[liveness] custom initial delay seconds", func() (corev1.Probe, corev1.Probe) {
+			liveness := defaultHAProxyLiveness.DeepCopy()
+			liveness.InitialDelaySeconds = defaultHAProxyLiveness.InitialDelaySeconds + 10
+
+			return defaultHAProxyReadiness, *liveness
+		}),
+		Entry("[liveness] custom timeout seconds", func() (corev1.Probe, corev1.Probe) {
+			liveness := defaultHAProxyLiveness.DeepCopy()
+			liveness.TimeoutSeconds = defaultHAProxyLiveness.TimeoutSeconds + 10
+
+			return defaultHAProxyReadiness, *liveness
+		}),
+		Entry("[liveness] custom period seconds", func() (corev1.Probe, corev1.Probe) {
+			liveness := defaultHAProxyLiveness.DeepCopy()
+			liveness.PeriodSeconds = defaultHAProxyLiveness.PeriodSeconds + 10
+
+			return defaultHAProxyReadiness, *liveness
+		}),
+		Entry("[liveness] custom success threshold", func() (corev1.Probe, corev1.Probe) {
+			liveness := defaultHAProxyLiveness.DeepCopy()
+			liveness.SuccessThreshold = defaultHAProxyLiveness.SuccessThreshold + 1
+
+			return defaultHAProxyReadiness, *liveness
+		}),
+		Entry("[liveness] custom failure threshold", func() (corev1.Probe, corev1.Probe) {
+			liveness := defaultHAProxyLiveness.DeepCopy()
+			liveness.FailureThreshold = defaultHAProxyLiveness.FailureThreshold + 1
+
+			return defaultHAProxyReadiness, *liveness
+		}),
+	)
+})