From ac7fd321c3305e518fc9129c785bd1f236f5956c Mon Sep 17 00:00:00 2001 From: George Kechagias Date: Thu, 23 Oct 2025 14:21:58 +0300 Subject: [PATCH 01/11] K8SPG-882 determine patroni version without using the special patroni version check pod --- percona/controller/pgcluster/controller.go | 234 +----------- .../controller/pgcluster/controller_test.go | 225 ------------ .../controller/pgcluster/patroniversion.go | 332 ++++++++++++++++++ .../pgcluster/patroniversion_test.go | 242 +++++++++++++ 4 files changed, 575 insertions(+), 458 deletions(-) create mode 100644 percona/controller/pgcluster/patroniversion.go create mode 100644 percona/controller/pgcluster/patroniversion_test.go diff --git a/percona/controller/pgcluster/controller.go b/percona/controller/pgcluster/controller.go index 87720b05f..a7685ecde 100644 --- a/percona/controller/pgcluster/controller.go +++ b/percona/controller/pgcluster/controller.go @@ -1,7 +1,6 @@ package pgcluster import ( - "bytes" "context" "crypto/md5" "fmt" @@ -11,21 +10,17 @@ import ( "strings" "time" - gover "github.com/hashicorp/go-version" "github.com/pkg/errors" "go.opentelemetry.io/otel/trace" batchv1 "k8s.io/api/batch/v1" corev1 "k8s.io/api/core/v1" k8serrors "k8s.io/apimachinery/pkg/api/errors" - "k8s.io/apimachinery/pkg/api/resource" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/labels" "k8s.io/apimachinery/pkg/types" - "k8s.io/apimachinery/pkg/util/wait" "k8s.io/client-go/tools/record" "k8s.io/client-go/util/retry" "k8s.io/client-go/util/workqueue" - "k8s.io/utils/ptr" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/builder" "sigs.k8s.io/controller-runtime/pkg/client" @@ -39,11 +34,9 @@ import ( "sigs.k8s.io/controller-runtime/pkg/source" "github.com/percona/percona-postgresql-operator/v2/internal/controller/runtime" - "github.com/percona/percona-postgresql-operator/v2/internal/initialize" "github.com/percona/percona-postgresql-operator/v2/internal/logging" "github.com/percona/percona-postgresql-operator/v2/internal/naming" "github.com/percona/percona-postgresql-operator/v2/internal/postgres" - "github.com/percona/percona-postgresql-operator/v2/percona/clientcmd" perconaController "github.com/percona/percona-postgresql-operator/v2/percona/controller" "github.com/percona/percona-postgresql-operator/v2/percona/extensions" "github.com/percona/percona-postgresql-operator/v2/percona/k8s" @@ -272,7 +265,7 @@ func (r *PGClusterReconciler) Reconcile(ctx context.Context, request reconcile.R return reconcile.Result{}, errors.Wrap(err, "ensure finalizers") } - if err := r.reconcilePatroniVersionCheck(ctx, cr); err != nil { + if err := r.reconcilePatroniVersion(ctx, cr); err != nil { if errors.Is(err, errPatroniVersionCheckWait) { return reconcile.Result{ RequeueAfter: 5 * time.Second, @@ -366,231 +359,6 @@ func (r *PGClusterReconciler) Reconcile(ctx context.Context, request reconcile.R return ctrl.Result{}, nil } -var errPatroniVersionCheckWait = errors.New("waiting for pod to initialize") - -func (r *PGClusterReconciler) reconcilePatroniVersionCheck(ctx context.Context, cr *v2.PerconaPGCluster) error { - if cr.Annotations == nil { - cr.Annotations = make(map[string]string) - } - - if patroniVersion, ok := cr.Annotations[pNaming.AnnotationCustomPatroniVersion]; ok { - patroniVersionUpdateFunc := func() error { - cluster := &v2.PerconaPGCluster{} - if err := r.Client.Get(ctx, types.NamespacedName{ - Name: cr.Name, - Namespace: cr.Namespace, - }, cluster); err != nil { - return errors.Wrap(err, "get PerconaPGCluster") - } - - orig := cluster.DeepCopy() - - cluster.Status.Patroni.Version = patroniVersion - cluster.Status.PatroniVersion = patroniVersion - - if err := r.Client.Status().Patch(ctx, cluster.DeepCopy(), client.MergeFrom(orig)); err != nil { - return errors.Wrap(err, "failed to patch patroni version") - } - - err := r.patchPatroniVersionAnnotation(ctx, cr, patroniVersion) - if err != nil { - return errors.Wrap(err, "failed to patch patroni version annotation") - } - - return nil - } - - // To ensure that the update was done given that conflicts can be caused by - // other code making unrelated updates to the same resource at the same time. - if err := retry.RetryOnConflict(retry.DefaultRetry, patroniVersionUpdateFunc); err != nil { - return errors.Wrap(err, "failed to patch patroni version") - } - return nil - } - - imageIDs, err := r.instanceImageIDs(ctx, cr) - if err != nil { - return errors.Wrap(err, "get image IDs") - } - - // If the imageIDs slice contains the imageID from the status, we skip checking the Patroni version. - // This ensures that the Patroni version is only checked after all pods have been updated. - if cr.CompareVersion("2.8.0") >= 0 { - if (len(imageIDs) == 0 || slices.Contains(imageIDs, cr.Status.Postgres.ImageID)) && cr.Status.Patroni.Version != "" { - err = r.patchPatroniVersionAnnotation(ctx, cr, cr.Status.Patroni.Version) - if err != nil { - return errors.Wrap(err, "failed to patch patroni version annotation") - } - return nil - } - } else { - if (len(imageIDs) == 0 || slices.Contains(imageIDs, cr.Status.Postgres.ImageID)) && cr.Status.PatroniVersion != "" { - err = r.patchPatroniVersionAnnotation(ctx, cr, cr.Status.PatroniVersion) - if err != nil { - return errors.Wrap(err, "failed to patch patroni version annotation") - } - return nil - } - } - - meta := metav1.ObjectMeta{ - Name: cr.Name + "-patroni-version-check", - Namespace: cr.Namespace, - } - - p := &corev1.Pod{ - ObjectMeta: meta, - } - - err = r.Client.Get(ctx, client.ObjectKeyFromObject(p), p) - if client.IgnoreNotFound(err) != nil { - return errors.Wrap(err, "failed to get patroni version check pod") - } - if k8serrors.IsNotFound(err) { - if len(cr.Spec.InstanceSets) == 0 { - return errors.New(".spec.instances is a required value") // shouldn't happen as the value is required in the crd.yaml - } - - // Using minimal resources since the patroni version check pod is performing a very simple - // operation i.e. "patronictl version" - resources := corev1.ResourceRequirements{ - Limits: corev1.ResourceList{ - corev1.ResourceCPU: resource.MustParse("100m"), - corev1.ResourceMemory: resource.MustParse("64Mi"), - }, - Requests: corev1.ResourceList{ - corev1.ResourceCPU: resource.MustParse("50m"), - corev1.ResourceMemory: resource.MustParse("32Mi"), - }, - } - - p = &corev1.Pod{ - ObjectMeta: meta, - Spec: corev1.PodSpec{ - Containers: []corev1.Container{ - { - Name: pNaming.ContainerPatroniVersionCheck, - Image: cr.PostgresImage(), - Command: []string{ - "bash", - }, - Args: []string{ - "-c", "sleep 60", - }, - Resources: resources, - SecurityContext: initialize.RestrictedSecurityContext(cr.CompareVersion("2.8.0") >= 0), - }, - }, - SecurityContext: cr.Spec.InstanceSets[0].SecurityContext, - Affinity: cr.Spec.InstanceSets[0].Affinity, - TerminationGracePeriodSeconds: ptr.To(int64(5)), - ImagePullSecrets: cr.Spec.ImagePullSecrets, - Resources: &resources, - }, - } - - if err := controllerutil.SetControllerReference(cr, p, r.Client.Scheme()); err != nil { - return errors.Wrap(err, "set controller reference") - } - if err := r.Client.Create(ctx, p); client.IgnoreAlreadyExists(err) != nil { - return errors.Wrap(err, "failed to create pod to check patroni version") - } - - return errPatroniVersionCheckWait - } - - if p.Status.Phase != corev1.PodRunning { - return errPatroniVersionCheckWait - } - - var stdout, stderr bytes.Buffer - execCli, err := clientcmd.NewClient() - if err != nil { - return errors.Wrap(err, "failed to create exec client") - } - b := wait.Backoff{ - Duration: 5 * time.Second, - Factor: 1.0, - Steps: 12, - Cap: time.Minute, - } - if err := retry.OnError(b, func(err error) bool { return err != nil && strings.Contains(err.Error(), "container not found") }, func() error { - return execCli.Exec(ctx, p, pNaming.ContainerPatroniVersionCheck, nil, &stdout, &stderr, "patronictl", "version") - }); err != nil { - return errors.Wrap(err, "exec") - } - - patroniVersion := strings.TrimSpace(strings.TrimPrefix(stdout.String(), "patronictl version ")) - - if _, err := gover.NewVersion(patroniVersion); err != nil { - return errors.Wrap(err, "failed to validate patroni version") - } - - orig := cr.DeepCopy() - - cr.Status.Patroni.Version = patroniVersion - cr.Status.PatroniVersion = patroniVersion - cr.Status.Postgres.Version = cr.Spec.PostgresVersion - cr.Status.Postgres.ImageID = getImageIDFromPod(p, pNaming.ContainerPatroniVersionCheck) - - if err := r.Client.Status().Patch(ctx, cr.DeepCopy(), client.MergeFrom(orig)); err != nil { - return errors.Wrap(err, "failed to patch patroni version") - } - - err = r.patchPatroniVersionAnnotation(ctx, cr, patroniVersion) - if err != nil { - return errors.Wrap(err, "failed to patch patroni version annotation") - } - - if err := r.Client.Delete(ctx, p); err != nil { - return errors.Wrap(err, "failed to delete patroni version check pod") - } - - return nil -} - -func (r *PGClusterReconciler) patchPatroniVersionAnnotation(ctx context.Context, cr *v2.PerconaPGCluster, patroniVersion string) error { - orig := cr.DeepCopy() - cr.Annotations[pNaming.AnnotationPatroniVersion] = patroniVersion - if err := r.Client.Patch(ctx, cr.DeepCopy(), client.MergeFrom(orig)); err != nil { - return errors.Wrap(err, "failed to patch the pg cluster") - } - return nil -} - -func (r *PGClusterReconciler) instanceImageIDs(ctx context.Context, cr *v2.PerconaPGCluster) ([]string, error) { - pods := new(corev1.PodList) - instances, err := naming.AsSelector(naming.ClusterInstances(cr.Name)) - if err != nil { - return nil, errors.Wrap(err, "failed to create a selector for instance pods") - } - if err = r.Client.List(ctx, pods, client.InNamespace(cr.Namespace), client.MatchingLabelsSelector{Selector: instances}); err != nil { - return nil, errors.Wrap(err, "failed to list instances") - } - - // Collecting all image IDs from instance pods. Under normal conditions, this slice will contain a single image ID, as all pods typically use the same image. - // During an image update, it may contain multiple different image IDs as the update progresses. - var imageIDs []string - for _, pod := range pods.Items { - imageID := getImageIDFromPod(&pod, naming.ContainerDatabase) - if imageID != "" && !slices.Contains(imageIDs, imageID) { - imageIDs = append(imageIDs, imageID) - } - } - - return imageIDs, nil -} - -func getImageIDFromPod(pod *corev1.Pod, containerName string) string { - idx := slices.IndexFunc(pod.Status.ContainerStatuses, func(s corev1.ContainerStatus) bool { - return s.Name == containerName - }) - if idx == -1 { - return "" - } - return pod.Status.ContainerStatuses[idx].ImageID -} - func (r *PGClusterReconciler) reconcileTLS(ctx context.Context, cr *v2.PerconaPGCluster) error { if err := r.validateTLS(ctx, cr); err != nil { return errors.Wrap(err, "validate TLS") diff --git a/percona/controller/pgcluster/controller_test.go b/percona/controller/pgcluster/controller_test.go index 5e39b2457..1ba73e632 100644 --- a/percona/controller/pgcluster/controller_test.go +++ b/percona/controller/pgcluster/controller_test.go @@ -2025,231 +2025,6 @@ var _ = Describe("ServiceAccount early creation", Ordered, func() { }) }) -var _ = Describe("patroni version check", Ordered, func() { - ctx := context.Background() - - const crName = "patroni-version-test" - const ns = crName - crNamespacedName := types.NamespacedName{Name: crName, Namespace: ns} - - namespace := &corev1.Namespace{ - ObjectMeta: metav1.ObjectMeta{ - Name: crName, - 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) - }) - - Context("With custom patroni version annotation", func() { - cr, err := readDefaultCR(crName, ns) - It("should read default cr.yaml", func() { - Expect(err).NotTo(HaveOccurred()) - }) - - It("should create PerconaPGCluster with custom patroni version", func() { - if cr.Annotations == nil { - cr.Annotations = make(map[string]string) - } - cr.Annotations[pNaming.AnnotationCustomPatroniVersion] = "3.2.1" - - status := cr.Status - Expect(k8sClient.Create(ctx, cr)).Should(Succeed()) - cr.Status = status - Expect(k8sClient.Status().Update(ctx, cr)).Should(Succeed()) - }) - - It("should successfully reconcile patroni version check", func() { - reconcilerInstance := reconciler(cr) - err := reconcilerInstance.reconcilePatroniVersionCheck(ctx, cr) - Expect(err).NotTo(HaveOccurred()) - }) - - It("should copy custom patroni version to status", func() { - updatedCR := &v2.PerconaPGCluster{} - Expect(k8sClient.Get(ctx, crNamespacedName, updatedCR)).Should(Succeed()) - - Expect(updatedCR.Status.Patroni.Version).To(Equal("3.2.1")) - Expect(updatedCR.Status.PatroniVersion).To(Equal("3.2.1")) - Expect(updatedCR.Annotations[pNaming.AnnotationPatroniVersion]).To(Equal("3.2.1")) - }) - }) - - Context("Without custom patroni version annotation", func() { - const crName2 = "patroni-version-test-2" - const ns2 = crName2 - crNamespacedName2 := types.NamespacedName{Name: crName2, Namespace: ns2} - - namespace2 := &corev1.Namespace{ - ObjectMeta: metav1.ObjectMeta{ - Name: crName2, - Namespace: ns2, - }, - } - - BeforeAll(func() { - By("Creating the second namespace") - err := k8sClient.Create(ctx, namespace2) - Expect(err).To(Not(HaveOccurred())) - }) - - AfterAll(func() { - By("Deleting the second namespace") - _ = k8sClient.Delete(ctx, namespace2) - }) - - cr2, err := readDefaultCR(crName2, ns2) - It("should read default cr.yaml", func() { - Expect(err).NotTo(HaveOccurred()) - }) - - It("should create PerconaPGCluster without custom patroni version annotation", func() { - if cr2.Annotations == nil { - cr2.Annotations = make(map[string]string) - } - delete(cr2.Annotations, pNaming.AnnotationCustomPatroniVersion) - - uid := int64(1001) - cr2.Spec.InstanceSets[0].SecurityContext = &corev1.PodSecurityContext{ - RunAsUser: &uid, - } - cr2.Spec.InstanceSets[0].Affinity = &corev1.Affinity{ - NodeAffinity: &corev1.NodeAffinity{ - PreferredDuringSchedulingIgnoredDuringExecution: []corev1.PreferredSchedulingTerm{ - { - Weight: int32(1), - }, - }, - }, - } - cr2.Spec.ImagePullSecrets = []corev1.LocalObjectReference{ - {Name: "test-pull-secret"}, - } - - cr2.Status.Patroni.Version = "3.1.0" - cr2.Status.PatroniVersion = "3.1.0" - cr2.Status.Postgres.ImageID = "some-image-id" - cr2.Annotations[pNaming.AnnotationPatroniVersion] = "3.1.0" - - status := cr2.Status - Expect(k8sClient.Create(ctx, cr2)).Should(Succeed()) - cr2.Status = status - Expect(k8sClient.Status().Update(ctx, cr2)).Should(Succeed()) - - pod := &corev1.Pod{ - ObjectMeta: metav1.ObjectMeta{ - Name: cr2.Name + "-instance-pod", - Namespace: cr2.Namespace, - Labels: map[string]string{ - "postgres-operator.crunchydata.com/cluster": cr2.Name, - "postgres-operator.crunchydata.com/instance": "instance", - }, - }, - Spec: corev1.PodSpec{ - Containers: []corev1.Container{ - { - Name: "database", - Image: "postgres:16", - }, - }, - }, - } - Expect(k8sClient.Create(ctx, pod)).Should(Succeed()) - - pod.Status = corev1.PodStatus{ - Phase: corev1.PodRunning, - ContainerStatuses: []corev1.ContainerStatus{ - { - Name: "database", - ImageID: "postgres:16", - }, - }, - } - Expect(k8sClient.Status().Update(ctx, pod)).Should(Succeed()) - }) - - It("should create patroni version check pod and return errPatroniVersionCheckWait", func() { - reconcilerInstance := reconciler(cr2) - err := reconcilerInstance.reconcilePatroniVersionCheck(ctx, cr2) - Expect(err).To(HaveOccurred()) - Expect(err.Error()).To(ContainSubstring("waiting for pod to initialize")) - }) - - It("should have created patroni version check pod with correct configuration", func() { - podName := cr2.Name + "-patroni-version-check" - pod := &corev1.Pod{} - err = k8sClient.Get(ctx, types.NamespacedName{Name: podName, Namespace: cr2.Namespace}, pod) - Expect(err).NotTo(HaveOccurred()) - - Expect(pod.Spec.Containers).To(HaveLen(1)) - Expect(pod.Spec.Containers[0].Name).To(Equal(pNaming.ContainerPatroniVersionCheck)) - Expect(pod.Spec.Containers[0].Image).To(Equal(cr2.Spec.Image)) - Expect(pod.Spec.Containers[0].Command).To(Equal([]string{"bash"})) - Expect(pod.Spec.Containers[0].Args).To(Equal([]string{"-c", "sleep 60"})) - Expect(pod.Spec.Containers[0].Resources).To(Equal(corev1.ResourceRequirements{ - Limits: corev1.ResourceList{ - corev1.ResourceCPU: resource.MustParse("100m"), - corev1.ResourceMemory: resource.MustParse("64Mi"), - }, - Requests: corev1.ResourceList{ - corev1.ResourceCPU: resource.MustParse("50m"), - corev1.ResourceMemory: resource.MustParse("32Mi"), - }, - })) - Expect(pod.Spec.Resources).To(Equal(&corev1.ResourceRequirements{ - Limits: corev1.ResourceList{ - corev1.ResourceCPU: resource.MustParse("100m"), - corev1.ResourceMemory: resource.MustParse("64Mi"), - }, - Requests: corev1.ResourceList{ - corev1.ResourceCPU: resource.MustParse("50m"), - corev1.ResourceMemory: resource.MustParse("32Mi"), - }, - })) - - uid := int64(1001) - expectedSecurityContext := &corev1.PodSecurityContext{ - RunAsUser: &uid, - } - expectedImagePullSecrets := []corev1.LocalObjectReference{ - {Name: "test-pull-secret"}, - } - expectedAffinity := &corev1.Affinity{ - NodeAffinity: &corev1.NodeAffinity{ - PreferredDuringSchedulingIgnoredDuringExecution: []corev1.PreferredSchedulingTerm{ - { - Weight: int32(1), - }, - }, - }, - } - - Expect(pod.Spec.SecurityContext).To(Equal(expectedSecurityContext)) - Expect(pod.Spec.TerminationGracePeriodSeconds).To(Equal(ptr.To(int64(5)))) - Expect(pod.Spec.ImagePullSecrets).To(Equal(expectedImagePullSecrets)) - Expect(pod.Spec.Affinity).To(Equal(expectedAffinity)) - }) - - It("should preserve existing patroni version in annotation", func() { - updatedCR := &v2.PerconaPGCluster{} - Expect(k8sClient.Get(ctx, crNamespacedName2, updatedCR)).Should(Succeed()) - - Expect(updatedCR.Status.Patroni.Version).To(Equal("3.1.0")) - Expect(updatedCR.Status.PatroniVersion).To(Equal("3.1.0")) - Expect(updatedCR.Annotations[pNaming.AnnotationPatroniVersion]).To(Equal("3.1.0")) - }) - }) -}) - var _ = Describe("CR Validations", Ordered, func() { ctx := context.Background() const crName = "cr-validation" diff --git a/percona/controller/pgcluster/patroniversion.go b/percona/controller/pgcluster/patroniversion.go new file mode 100644 index 000000000..2433c847d --- /dev/null +++ b/percona/controller/pgcluster/patroniversion.go @@ -0,0 +1,332 @@ +package pgcluster + +import ( + "bytes" + "context" + "io" + "slices" + "strings" + "time" + + "github.com/pkg/errors" + corev1 "k8s.io/api/core/v1" + k8serrors "k8s.io/apimachinery/pkg/api/errors" + "k8s.io/apimachinery/pkg/api/resource" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/types" + "k8s.io/apimachinery/pkg/util/wait" + "k8s.io/client-go/util/retry" + "k8s.io/utils/ptr" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" + logf "sigs.k8s.io/controller-runtime/pkg/log" + + gover "github.com/hashicorp/go-version" + "github.com/percona/percona-postgresql-operator/v2/internal/initialize" + "github.com/percona/percona-postgresql-operator/v2/internal/naming" + "github.com/percona/percona-postgresql-operator/v2/percona/clientcmd" + pNaming "github.com/percona/percona-postgresql-operator/v2/percona/naming" + v2 "github.com/percona/percona-postgresql-operator/v2/pkg/apis/pgv2.percona.com/v2" +) + +var errPatroniVersionCheckWait = errors.New("waiting for pod to initialize") + +// ExecClient is an interface for executing commands in pods +type ExecClient interface { + Exec(ctx context.Context, pod *corev1.Pod, containerName string, stdin io.Reader, stdout, stderr io.Writer, command ...string) error +} + +// execClientFactoryFunc is a function that creates an ExecClient +type execClientFactoryFunc func() (ExecClient, error) + +// defaultExecClientFactory is the default factory that creates a real clientcmd.Client +var defaultExecClientFactory execClientFactoryFunc = func() (ExecClient, error) { + return clientcmd.NewClient() +} + +func (r *PGClusterReconciler) reconcilePatroniVersion(ctx context.Context, cr *v2.PerconaPGCluster) error { + log := logf.FromContext(ctx) + + if cr.Annotations == nil { + cr.Annotations = make(map[string]string) + } + + log.Info("reconciling PatroniVersion") + + if patroniVersion, ok := cr.Annotations[pNaming.AnnotationCustomPatroniVersion]; ok { + patroniVersionUpdateFunc := func() error { + cluster := &v2.PerconaPGCluster{} + if err := r.Client.Get(ctx, types.NamespacedName{ + Name: cr.Name, + Namespace: cr.Namespace, + }, cluster); err != nil { + return errors.Wrap(err, "get PerconaPGCluster") + } + + orig := cluster.DeepCopy() + + cluster.Status.Patroni.Version = patroniVersion + cluster.Status.PatroniVersion = patroniVersion + + if err := r.Client.Status().Patch(ctx, cluster.DeepCopy(), client.MergeFrom(orig)); err != nil { + return errors.Wrap(err, "failed to patch patroni version") + } + + err := r.patchPatroniVersionAnnotation(ctx, cr, patroniVersion) + if err != nil { + return errors.Wrap(err, "failed to patch patroni version annotation") + } + + return nil + } + + // To ensure that the update was done given that conflicts can be caused by + // other code making unrelated updates to the same resource at the same time. + if err := retry.RetryOnConflict(retry.DefaultRetry, patroniVersionUpdateFunc); err != nil { + return errors.Wrap(err, "failed to patch patroni version") + } + return nil + } + + if cr.CompareVersion("2.8.0") >= 0 { + log := logf.FromContext(ctx) + + pods, err := r.getInstancePods(ctx, cr) + if err != nil { + return errors.Wrap(err, "failed to get instance pods") + } + if len(pods.Items) == 0 { + return errors.Wrap(err, "instance pods not available") + } + + p := pods.Items[0] + + if p.Status.Phase != corev1.PodRunning { + return errPatroniVersionCheckWait + } + + patroniVersion, err := r.getPatroniVersion(ctx, &p, naming.ContainerDatabase) + if err != nil { + return errors.Wrap(err, "failed to get patroni version") + } + + log.Info("patroni version detected", "version", patroniVersion) + + orig := cr.DeepCopy() + + cr.Status.Patroni.Version = patroniVersion + cr.Status.PatroniVersion = patroniVersion + cr.Status.Postgres.Version = cr.Spec.PostgresVersion + cr.Status.Postgres.ImageID = getImageIDFromPod(&p, naming.ContainerDatabase) + + if err := r.Client.Status().Patch(ctx, cr.DeepCopy(), client.MergeFrom(orig)); err != nil { + return errors.Wrap(err, "failed to patch patroni version") + } + + err = r.patchPatroniVersionAnnotation(ctx, cr, patroniVersion) + if err != nil { + return errors.Wrap(err, "failed to patch patroni version annotation") + } + + return nil + } + + imageIDs, err := r.instanceImageIDs(ctx, cr) + if err != nil { + return errors.Wrap(err, "get image IDs") + } + + // If the imageIDs slice contains the imageID from the status, we skip checking the Patroni version. + // This ensures that the Patroni version is only checked after all pods have been updated. + if cr.CompareVersion("2.8.0") >= 0 { + if (len(imageIDs) == 0 || slices.Contains(imageIDs, cr.Status.Postgres.ImageID)) && cr.Status.Patroni.Version != "" { + err = r.patchPatroniVersionAnnotation(ctx, cr, cr.Status.Patroni.Version) + if err != nil { + return errors.Wrap(err, "failed to patch patroni version annotation") + } + return nil + } + } else { + if (len(imageIDs) == 0 || slices.Contains(imageIDs, cr.Status.Postgres.ImageID)) && cr.Status.PatroniVersion != "" { + err = r.patchPatroniVersionAnnotation(ctx, cr, cr.Status.PatroniVersion) + if err != nil { + return errors.Wrap(err, "failed to patch patroni version annotation") + } + return nil + } + } + + meta := metav1.ObjectMeta{ + Name: cr.Name + "-patroni-version-check", + Namespace: cr.Namespace, + } + + p := &corev1.Pod{ + ObjectMeta: meta, + } + + err = r.Client.Get(ctx, client.ObjectKeyFromObject(p), p) + if client.IgnoreNotFound(err) != nil { + return errors.Wrap(err, "failed to get patroni version check pod") + } + if k8serrors.IsNotFound(err) { + if len(cr.Spec.InstanceSets) == 0 { + return errors.New(".spec.instances is a required value") // shouldn't happen as the value is required in the crd.yaml + } + + // Using minimal resources since the patroni version check pod is performing a very simple + // operation i.e. "patronictl version" + resources := corev1.ResourceRequirements{ + Limits: corev1.ResourceList{ + corev1.ResourceCPU: resource.MustParse("100m"), + corev1.ResourceMemory: resource.MustParse("64Mi"), + }, + Requests: corev1.ResourceList{ + corev1.ResourceCPU: resource.MustParse("50m"), + corev1.ResourceMemory: resource.MustParse("32Mi"), + }, + } + + p = &corev1.Pod{ + ObjectMeta: meta, + Spec: corev1.PodSpec{ + Containers: []corev1.Container{ + { + Name: pNaming.ContainerPatroniVersionCheck, + Image: cr.PostgresImage(), + Command: []string{ + "bash", + }, + Args: []string{ + "-c", "sleep 60", + }, + Resources: resources, + SecurityContext: initialize.RestrictedSecurityContext(cr.CompareVersion("2.8.0") >= 0), + }, + }, + SecurityContext: cr.Spec.InstanceSets[0].SecurityContext, + Affinity: cr.Spec.InstanceSets[0].Affinity, + TerminationGracePeriodSeconds: ptr.To(int64(5)), + ImagePullSecrets: cr.Spec.ImagePullSecrets, + Resources: &resources, + }, + } + + if err := controllerutil.SetControllerReference(cr, p, r.Client.Scheme()); err != nil { + return errors.Wrap(err, "set controller reference") + } + if err := r.Client.Create(ctx, p); client.IgnoreAlreadyExists(err) != nil { + return errors.Wrap(err, "failed to create pod to check patroni version") + } + + return errPatroniVersionCheckWait + } + + if p.Status.Phase != corev1.PodRunning { + return errPatroniVersionCheckWait + } + + patroniVersion, err := r.getPatroniVersion(ctx, p, pNaming.ContainerPatroniVersionCheck) + if err != nil { + return errors.Wrap(err, "failed to get patroni version") + } + + orig := cr.DeepCopy() + + cr.Status.Patroni.Version = patroniVersion + cr.Status.PatroniVersion = patroniVersion + cr.Status.Postgres.Version = cr.Spec.PostgresVersion + cr.Status.Postgres.ImageID = getImageIDFromPod(p, pNaming.ContainerPatroniVersionCheck) + + if err := r.Client.Status().Patch(ctx, cr.DeepCopy(), client.MergeFrom(orig)); err != nil { + return errors.Wrap(err, "failed to patch patroni version") + } + + err = r.patchPatroniVersionAnnotation(ctx, cr, patroniVersion) + if err != nil { + return errors.Wrap(err, "failed to patch patroni version annotation") + } + + if err := r.Client.Delete(ctx, p); err != nil { + return errors.Wrap(err, "failed to delete patroni version check pod") + } + + return nil +} + +func (r *PGClusterReconciler) getPatroniVersion(ctx context.Context, pod *corev1.Pod, containerName string) (string, error) { + var stdout, stderr bytes.Buffer + execCli, err := clientcmd.NewClient() + if err != nil { + return "", errors.Wrap(err, "failed to create exec client") + } + b := wait.Backoff{ + Duration: 5 * time.Second, + Factor: 1.0, + Steps: 12, + Cap: time.Minute, + } + if err := retry.OnError(b, func(err error) bool { return err != nil && strings.Contains(err.Error(), "container not found") }, func() error { + return execCli.Exec(ctx, pod, containerName, nil, &stdout, &stderr, "patronictl", "version") + }); err != nil { + return "", errors.Wrap(err, "exec") + } + + patroniVersion := strings.TrimSpace(strings.TrimPrefix(stdout.String(), "patronictl version ")) + + if _, err := gover.NewVersion(patroniVersion); err != nil { + return "", errors.Wrap(err, "failed to validate patroni version") + } + + return patroniVersion, nil +} + +func (r *PGClusterReconciler) patchPatroniVersionAnnotation(ctx context.Context, cr *v2.PerconaPGCluster, patroniVersion string) error { + orig := cr.DeepCopy() + cr.Annotations[pNaming.AnnotationPatroniVersion] = patroniVersion + if err := r.Client.Patch(ctx, cr.DeepCopy(), client.MergeFrom(orig)); err != nil { + return errors.Wrap(err, "failed to patch the pg cluster") + } + return nil +} + +func (r *PGClusterReconciler) instanceImageIDs(ctx context.Context, cr *v2.PerconaPGCluster) ([]string, error) { + pods, err := r.getInstancePods(ctx, cr) + if err != nil { + return nil, errors.Wrap(err, "failed to get instance pods") + } + + // Collecting all image IDs from instance pods. Under normal conditions, this slice will contain a single image ID, as all pods typically use the same image. + // During an image update, it may contain multiple different image IDs as the update progresses. + var imageIDs []string + for _, pod := range pods.Items { + imageID := getImageIDFromPod(&pod, naming.ContainerDatabase) + if imageID != "" && !slices.Contains(imageIDs, imageID) { + imageIDs = append(imageIDs, imageID) + } + } + + return imageIDs, nil +} + +func (r *PGClusterReconciler) getInstancePods(ctx context.Context, cr *v2.PerconaPGCluster) (*corev1.PodList, error) { + pods := new(corev1.PodList) + instances, err := naming.AsSelector(naming.ClusterInstances(cr.Name)) + if err != nil { + return nil, errors.Wrap(err, "failed to create a selector for instance pods") + } + if err = r.Client.List(ctx, pods, client.InNamespace(cr.Namespace), client.MatchingLabelsSelector{Selector: instances}); err != nil { + return nil, errors.Wrap(err, "failed to list instances") + } + return pods, nil +} + +func getImageIDFromPod(pod *corev1.Pod, containerName string) string { + idx := slices.IndexFunc(pod.Status.ContainerStatuses, func(s corev1.ContainerStatus) bool { + return s.Name == containerName + }) + if idx == -1 { + return "" + } + return pod.Status.ContainerStatuses[idx].ImageID +} diff --git a/percona/controller/pgcluster/patroniversion_test.go b/percona/controller/pgcluster/patroniversion_test.go new file mode 100644 index 000000000..16bdde7a6 --- /dev/null +++ b/percona/controller/pgcluster/patroniversion_test.go @@ -0,0 +1,242 @@ +package pgcluster + +import ( + "context" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + corev1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/api/resource" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/types" + "k8s.io/utils/ptr" + + pNaming "github.com/percona/percona-postgresql-operator/v2/percona/naming" + v2 "github.com/percona/percona-postgresql-operator/v2/pkg/apis/pgv2.percona.com/v2" +) + +var _ = Describe("patroni version check", Ordered, func() { + ctx := context.Background() + + const crName = "patroni-version-test" + const ns = crName + crNamespacedName := types.NamespacedName{Name: crName, Namespace: ns} + + namespace := &corev1.Namespace{ + ObjectMeta: metav1.ObjectMeta{ + Name: crName, + 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) + }) + + Context("With custom patroni version annotation", func() { + cr, err := readDefaultCR(crName, ns) + It("should read default cr.yaml", func() { + Expect(err).NotTo(HaveOccurred()) + }) + + It("should create PerconaPGCluster with custom patroni version", func() { + if cr.Annotations == nil { + cr.Annotations = make(map[string]string) + } + cr.Annotations[pNaming.AnnotationCustomPatroniVersion] = "3.2.1" + + status := cr.Status + Expect(k8sClient.Create(ctx, cr)).Should(Succeed()) + cr.Status = status + Expect(k8sClient.Status().Update(ctx, cr)).Should(Succeed()) + }) + + It("should successfully reconcile patroni version check", func() { + reconcilerInstance := reconciler(cr) + err := reconcilerInstance.reconcilePatroniVersion(ctx, cr) + Expect(err).NotTo(HaveOccurred()) + }) + + It("should copy custom patroni version to status", func() { + updatedCR := &v2.PerconaPGCluster{} + Expect(k8sClient.Get(ctx, crNamespacedName, updatedCR)).Should(Succeed()) + + Expect(updatedCR.Status.Patroni.Version).To(Equal("3.2.1")) + Expect(updatedCR.Status.PatroniVersion).To(Equal("3.2.1")) + Expect(updatedCR.Annotations[pNaming.AnnotationPatroniVersion]).To(Equal("3.2.1")) + }) + }) + + Context("Without custom patroni version annotation for cr version <=2.7", func() { + const crName2 = "patroni-version-test-2" + const ns2 = crName2 + crNamespacedName2 := types.NamespacedName{Name: crName2, Namespace: ns2} + + namespace2 := &corev1.Namespace{ + ObjectMeta: metav1.ObjectMeta{ + Name: crName2, + Namespace: ns2, + }, + } + + BeforeAll(func() { + By("Creating the second namespace") + err := k8sClient.Create(ctx, namespace2) + Expect(err).To(Not(HaveOccurred())) + }) + + AfterAll(func() { + By("Deleting the second namespace") + _ = k8sClient.Delete(ctx, namespace2) + }) + + cr2, err := readDefaultCR(crName2, ns2) + cr2.Spec.CRVersion = "2.7.0" + It("should read default cr.yaml", func() { + Expect(err).NotTo(HaveOccurred()) + }) + + It("should create PerconaPGCluster without custom patroni version annotation", func() { + if cr2.Annotations == nil { + cr2.Annotations = make(map[string]string) + } + delete(cr2.Annotations, pNaming.AnnotationCustomPatroniVersion) + + uid := int64(1001) + cr2.Spec.InstanceSets[0].SecurityContext = &corev1.PodSecurityContext{ + RunAsUser: &uid, + } + cr2.Spec.InstanceSets[0].Affinity = &corev1.Affinity{ + NodeAffinity: &corev1.NodeAffinity{ + PreferredDuringSchedulingIgnoredDuringExecution: []corev1.PreferredSchedulingTerm{ + { + Weight: int32(1), + }, + }, + }, + } + cr2.Spec.ImagePullSecrets = []corev1.LocalObjectReference{ + {Name: "test-pull-secret"}, + } + + cr2.Status.Patroni.Version = "3.1.0" + cr2.Status.PatroniVersion = "3.1.0" + cr2.Status.Postgres.ImageID = "some-image-id" + cr2.Annotations[pNaming.AnnotationPatroniVersion] = "3.1.0" + + status := cr2.Status + Expect(k8sClient.Create(ctx, cr2)).Should(Succeed()) + cr2.Status = status + Expect(k8sClient.Status().Update(ctx, cr2)).Should(Succeed()) + + pod := &corev1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: cr2.Name + "-instance-pod", + Namespace: cr2.Namespace, + Labels: map[string]string{ + "postgres-operator.crunchydata.com/cluster": cr2.Name, + "postgres-operator.crunchydata.com/instance": "instance", + }, + }, + Spec: corev1.PodSpec{ + Containers: []corev1.Container{ + { + Name: "database", + Image: "postgres:16", + }, + }, + }, + } + Expect(k8sClient.Create(ctx, pod)).Should(Succeed()) + + pod.Status = corev1.PodStatus{ + Phase: corev1.PodRunning, + ContainerStatuses: []corev1.ContainerStatus{ + { + Name: "database", + ImageID: "postgres:16", + }, + }, + } + Expect(k8sClient.Status().Update(ctx, pod)).Should(Succeed()) + }) + + It("should create patroni version check pod and return errPatroniVersionCheckWait", func() { + reconcilerInstance := reconciler(cr2) + err := reconcilerInstance.reconcilePatroniVersion(ctx, cr2) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("waiting for pod to initialize")) + }) + + It("should have created patroni version check pod with correct configuration", func() { + podName := cr2.Name + "-patroni-version-check" + pod := &corev1.Pod{} + err = k8sClient.Get(ctx, types.NamespacedName{Name: podName, Namespace: cr2.Namespace}, pod) + Expect(err).NotTo(HaveOccurred()) + + Expect(pod.Spec.Containers).To(HaveLen(1)) + Expect(pod.Spec.Containers[0].Name).To(Equal(pNaming.ContainerPatroniVersionCheck)) + Expect(pod.Spec.Containers[0].Image).To(Equal(cr2.Spec.Image)) + Expect(pod.Spec.Containers[0].Command).To(Equal([]string{"bash"})) + Expect(pod.Spec.Containers[0].Args).To(Equal([]string{"-c", "sleep 60"})) + Expect(pod.Spec.Containers[0].Resources).To(Equal(corev1.ResourceRequirements{ + Limits: corev1.ResourceList{ + corev1.ResourceCPU: resource.MustParse("100m"), + corev1.ResourceMemory: resource.MustParse("64Mi"), + }, + Requests: corev1.ResourceList{ + corev1.ResourceCPU: resource.MustParse("50m"), + corev1.ResourceMemory: resource.MustParse("32Mi"), + }, + })) + Expect(pod.Spec.Resources).To(Equal(&corev1.ResourceRequirements{ + Limits: corev1.ResourceList{ + corev1.ResourceCPU: resource.MustParse("100m"), + corev1.ResourceMemory: resource.MustParse("64Mi"), + }, + Requests: corev1.ResourceList{ + corev1.ResourceCPU: resource.MustParse("50m"), + corev1.ResourceMemory: resource.MustParse("32Mi"), + }, + })) + + uid := int64(1001) + expectedSecurityContext := &corev1.PodSecurityContext{ + RunAsUser: &uid, + } + expectedImagePullSecrets := []corev1.LocalObjectReference{ + {Name: "test-pull-secret"}, + } + expectedAffinity := &corev1.Affinity{ + NodeAffinity: &corev1.NodeAffinity{ + PreferredDuringSchedulingIgnoredDuringExecution: []corev1.PreferredSchedulingTerm{ + { + Weight: int32(1), + }, + }, + }, + } + + Expect(pod.Spec.SecurityContext).To(Equal(expectedSecurityContext)) + Expect(pod.Spec.TerminationGracePeriodSeconds).To(Equal(ptr.To(int64(5)))) + Expect(pod.Spec.ImagePullSecrets).To(Equal(expectedImagePullSecrets)) + Expect(pod.Spec.Affinity).To(Equal(expectedAffinity)) + }) + + It("should preserve existing patroni version in annotation", func() { + updatedCR := &v2.PerconaPGCluster{} + Expect(k8sClient.Get(ctx, crNamespacedName2, updatedCR)).Should(Succeed()) + + Expect(updatedCR.Status.Patroni.Version).To(Equal("3.1.0")) + Expect(updatedCR.Status.PatroniVersion).To(Equal("3.1.0")) + Expect(updatedCR.Annotations[pNaming.AnnotationPatroniVersion]).To(Equal("3.1.0")) + }) + }) +}) From f0bb582701f8591fcbfe30aa0efe67f8efd6ab91 Mon Sep 17 00:00:00 2001 From: George Kechagias Date: Mon, 27 Oct 2025 12:29:21 +0200 Subject: [PATCH 02/11] improve primary pod --- .../controller/pgcluster/patroniversion.go | 36 +- percona/postgres/common.go | 30 +- percona/postgres/common_test.go | 380 ++++++++++++++++++ 3 files changed, 410 insertions(+), 36 deletions(-) create mode 100644 percona/postgres/common_test.go diff --git a/percona/controller/pgcluster/patroniversion.go b/percona/controller/pgcluster/patroniversion.go index 2433c847d..e8d97d45c 100644 --- a/percona/controller/pgcluster/patroniversion.go +++ b/percona/controller/pgcluster/patroniversion.go @@ -3,11 +3,16 @@ package pgcluster import ( "bytes" "context" - "io" "slices" "strings" "time" + gover "github.com/hashicorp/go-version" + "github.com/percona/percona-postgresql-operator/v2/internal/initialize" + "github.com/percona/percona-postgresql-operator/v2/internal/naming" + "github.com/percona/percona-postgresql-operator/v2/percona/clientcmd" + pNaming "github.com/percona/percona-postgresql-operator/v2/percona/naming" + v2 "github.com/percona/percona-postgresql-operator/v2/pkg/apis/pgv2.percona.com/v2" "github.com/pkg/errors" corev1 "k8s.io/api/core/v1" k8serrors "k8s.io/apimachinery/pkg/api/errors" @@ -19,40 +24,15 @@ import ( "k8s.io/utils/ptr" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" - logf "sigs.k8s.io/controller-runtime/pkg/log" - - gover "github.com/hashicorp/go-version" - "github.com/percona/percona-postgresql-operator/v2/internal/initialize" - "github.com/percona/percona-postgresql-operator/v2/internal/naming" - "github.com/percona/percona-postgresql-operator/v2/percona/clientcmd" - pNaming "github.com/percona/percona-postgresql-operator/v2/percona/naming" - v2 "github.com/percona/percona-postgresql-operator/v2/pkg/apis/pgv2.percona.com/v2" ) var errPatroniVersionCheckWait = errors.New("waiting for pod to initialize") -// ExecClient is an interface for executing commands in pods -type ExecClient interface { - Exec(ctx context.Context, pod *corev1.Pod, containerName string, stdin io.Reader, stdout, stderr io.Writer, command ...string) error -} - -// execClientFactoryFunc is a function that creates an ExecClient -type execClientFactoryFunc func() (ExecClient, error) - -// defaultExecClientFactory is the default factory that creates a real clientcmd.Client -var defaultExecClientFactory execClientFactoryFunc = func() (ExecClient, error) { - return clientcmd.NewClient() -} - func (r *PGClusterReconciler) reconcilePatroniVersion(ctx context.Context, cr *v2.PerconaPGCluster) error { - log := logf.FromContext(ctx) - if cr.Annotations == nil { cr.Annotations = make(map[string]string) } - log.Info("reconciling PatroniVersion") - if patroniVersion, ok := cr.Annotations[pNaming.AnnotationCustomPatroniVersion]; ok { patroniVersionUpdateFunc := func() error { cluster := &v2.PerconaPGCluster{} @@ -88,8 +68,8 @@ func (r *PGClusterReconciler) reconcilePatroniVersion(ctx context.Context, cr *v return nil } + // Starting from version 2.8.0, the patroni version check pod should not be executed. if cr.CompareVersion("2.8.0") >= 0 { - log := logf.FromContext(ctx) pods, err := r.getInstancePods(ctx, cr) if err != nil { @@ -110,8 +90,6 @@ func (r *PGClusterReconciler) reconcilePatroniVersion(ctx context.Context, cr *v return errors.Wrap(err, "failed to get patroni version") } - log.Info("patroni version detected", "version", patroniVersion) - orig := cr.DeepCopy() cr.Status.Patroni.Version = patroniVersion diff --git a/percona/postgres/common.go b/percona/postgres/common.go index f0a8c05b5..a6857f02e 100644 --- a/percona/postgres/common.go +++ b/percona/postgres/common.go @@ -9,21 +9,23 @@ import ( "k8s.io/apimachinery/pkg/labels" "sigs.k8s.io/controller-runtime/pkg/client" + pNaming "github.com/percona/percona-postgresql-operator/v2/percona/naming" v2 "github.com/percona/percona-postgresql-operator/v2/pkg/apis/pgv2.percona.com/v2" ) +const ( + patroniVersion4 = "4.0.0" +) + +// GetPrimaryPod returns the primary pod. +// K8SPG-882 func GetPrimaryPod(ctx context.Context, cli client.Client, cr *v2.PerconaPGCluster) (*corev1.Pod, error) { podList := &corev1.PodList{} // K8SPG-648: patroni v4.0.0 deprecated "master" role. // We should use "primary" instead role := "primary" - patroniVersion := cr.Status.PatroniVersion - if cr.CompareVersion("2.8.0") >= 0 { - patroniVersion = cr.Status.Patroni.Version - } - - patroniVer, err := gover.NewVersion(patroniVersion) + patroniVer, err := gover.NewVersion(determineVersion(cr)) if err != nil { return nil, errors.Wrap(err, "failed to get patroni version") } @@ -39,7 +41,7 @@ func GetPrimaryPod(ctx context.Context, cli client.Client, cr *v2.PerconaPGClust }), }) if err != nil { - return nil, err + return nil, errors.Wrap(err, "failed to list pods") } if len(podList.Items) == 0 { @@ -52,3 +54,17 @@ func GetPrimaryPod(ctx context.Context, cli client.Client, cr *v2.PerconaPGClust return &podList.Items[0], nil } + +func determineVersion(cr *v2.PerconaPGCluster) string { + if cr.CompareVersion("2.7.0") <= 0 { + return cr.Status.PatroniVersion + } + patroniVersion, ok := cr.Annotations[pNaming.AnnotationPatroniVersion] + if !ok { + // If the annotation is non-existing, the operator is assuming version 4.x.x by default + // in order to enforce the use of "primary" role. Patroni version after 4.x.x will also + // use "primary", so the operator will be compatible with them as well. + return patroniVersion4 + } + return patroniVersion +} diff --git a/percona/postgres/common_test.go b/percona/postgres/common_test.go new file mode 100644 index 000000000..72a7f0d0a --- /dev/null +++ b/percona/postgres/common_test.go @@ -0,0 +1,380 @@ +package perconaPG + +import ( + "context" + "testing" + + "github.com/percona/percona-postgresql-operator/v2/percona/version" + "gotest.tools/v3/assert" + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + "sigs.k8s.io/controller-runtime/pkg/client/fake" + + pNaming "github.com/percona/percona-postgresql-operator/v2/percona/naming" + v2 "github.com/percona/percona-postgresql-operator/v2/pkg/apis/pgv2.percona.com/v2" +) + +func TestGetPrimaryPod(t *testing.T) { + ctx := context.Background() + + tests := map[string]struct { + cr *v2.PerconaPGCluster + pods []corev1.Pod + expectedError string + expectedPod string + }{ + "patroni 4.1.0 with annotation": { + cr: &v2.PerconaPGCluster{ + Spec: v2.PerconaPGClusterSpec{ + CRVersion: version.Version(), + }, + ObjectMeta: metav1.ObjectMeta{ + Name: "test-cluster", + Namespace: "test-namespace", + Annotations: map[string]string{ + pNaming.AnnotationPatroniVersion: "4.1.0", + }, + }, + Status: v2.PerconaPGClusterStatus{ + Patroni: v2.Patroni{ + Version: "4.0.0", + }, + }, + }, + pods: []corev1.Pod{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: "test-cluster-primary-0", + Namespace: "test-namespace", + Labels: map[string]string{ + "app.kubernetes.io/instance": "test-cluster", + "postgres-operator.crunchydata.com/role": "primary", + }, + }, + }, + { + ObjectMeta: metav1.ObjectMeta{ + Name: "test-cluster-primary-1", + Namespace: "test-namespace", + Labels: map[string]string{ + "app.kubernetes.io/instance": "test-cluster", + "postgres-operator.crunchydata.com/role": "something", + }, + }, + }, + }, + expectedPod: "test-cluster-primary-0", + }, + "patroni 4.0.0 without annotation": { + cr: &v2.PerconaPGCluster{ + Spec: v2.PerconaPGClusterSpec{ + CRVersion: version.Version(), + }, + ObjectMeta: metav1.ObjectMeta{ + Name: "test-cluster", + Namespace: "test-namespace", + }, + Status: v2.PerconaPGClusterStatus{ + Patroni: v2.Patroni{ + Version: "4.0.0", + }, + }, + }, + pods: []corev1.Pod{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: "test-cluster-primary-0", + Namespace: "test-namespace", + Labels: map[string]string{ + "app.kubernetes.io/instance": "test-cluster", + "postgres-operator.crunchydata.com/role": "primary", + }, + }, + }, + { + ObjectMeta: metav1.ObjectMeta{ + Name: "test-cluster-primary-1", + Namespace: "test-namespace", + Labels: map[string]string{ + "app.kubernetes.io/instance": "test-cluster", + "postgres-operator.crunchydata.com/role": "something", + }, + }, + }, + }, + expectedPod: "test-cluster-primary-0", + }, + "patroni 3.x with master role": { + cr: &v2.PerconaPGCluster{ + Spec: v2.PerconaPGClusterSpec{ + CRVersion: "2.7.0", + }, + ObjectMeta: metav1.ObjectMeta{ + Name: "test-cluster", + Namespace: "test-namespace", + }, + Status: v2.PerconaPGClusterStatus{ + PatroniVersion: "3.0.0", + }, + }, + pods: []corev1.Pod{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: "test-cluster-master-0", + Namespace: "test-namespace", + Labels: map[string]string{ + "app.kubernetes.io/instance": "test-cluster", + "postgres-operator.crunchydata.com/role": "master", + }, + }, + }, + }, + expectedPod: "test-cluster-master-0", + }, + "patroni version from annotation overrides status for version >= 2.8.0": { + cr: &v2.PerconaPGCluster{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-cluster", + Namespace: "test-namespace", + Annotations: map[string]string{ + pNaming.AnnotationPatroniVersion: "4.1.0", + }, + }, + Spec: v2.PerconaPGClusterSpec{ + PostgresVersion: 16, + CRVersion: version.Version(), + }, + Status: v2.PerconaPGClusterStatus{ + PatroniVersion: "3.0.0", + Postgres: v2.PostgresStatus{ + Version: 16, + }, + }, + }, + pods: []corev1.Pod{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: "test-cluster-primary-0", + Namespace: "test-namespace", + Labels: map[string]string{ + "app.kubernetes.io/instance": "test-cluster", + "postgres-operator.crunchydata.com/role": "primary", + }, + }, + }, + }, + expectedPod: "test-cluster-primary-0", + }, + "patroni version from status used for version < 2.8.0": { + cr: &v2.PerconaPGCluster{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-cluster", + Namespace: "test-namespace", + Annotations: map[string]string{ + pNaming.AnnotationPatroniVersion: "4.0.0", + }, + }, + Spec: v2.PerconaPGClusterSpec{ + PostgresVersion: 14, + CRVersion: "2.7.0", + }, + Status: v2.PerconaPGClusterStatus{ + PatroniVersion: "3.0.0", + Postgres: v2.PostgresStatus{ + Version: 14, + }, + }, + }, + pods: []corev1.Pod{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: "test-cluster-master-0", + Namespace: "test-namespace", + Labels: map[string]string{ + "app.kubernetes.io/instance": "test-cluster", + "postgres-operator.crunchydata.com/role": "master", + }, + }, + }, + }, + expectedPod: "test-cluster-master-0", + }, + "no primary pod found": { + cr: &v2.PerconaPGCluster{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-cluster", + Namespace: "test-namespace", + Annotations: map[string]string{ + pNaming.AnnotationPatroniVersion: "4.1.0", + }, + }, + Spec: v2.PerconaPGClusterSpec{ + PostgresVersion: 14, + CRVersion: version.Version(), + }, + }, + pods: []corev1.Pod{}, + expectedError: "no primary pod found", + }, + "multiple primary pods found": { + cr: &v2.PerconaPGCluster{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-cluster", + Namespace: "test-namespace", + }, + Status: v2.PerconaPGClusterStatus{ + PatroniVersion: "4.0.0", + }, + Spec: v2.PerconaPGClusterSpec{ + CRVersion: version.Version(), + }, + }, + pods: []corev1.Pod{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: "test-cluster-primary-0", + Namespace: "test-namespace", + Labels: map[string]string{ + "app.kubernetes.io/instance": "test-cluster", + "postgres-operator.crunchydata.com/role": "primary", + }, + }, + }, + { + ObjectMeta: metav1.ObjectMeta{ + Name: "test-cluster-primary-1", + Namespace: "test-namespace", + Labels: map[string]string{ + "app.kubernetes.io/instance": "test-cluster", + "postgres-operator.crunchydata.com/role": "primary", + }, + }, + }, + }, + expectedError: "multiple primary pods found", + }, + "invalid patroni version returns the default primary": { + cr: &v2.PerconaPGCluster{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-cluster", + Namespace: "test-namespace", + }, + Status: v2.PerconaPGClusterStatus{ + PatroniVersion: "invalid-version", + }, + Spec: v2.PerconaPGClusterSpec{ + CRVersion: version.Version(), + }, + }, + pods: []corev1.Pod{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: "test-cluster-primary-0", + Namespace: "test-namespace", + Labels: map[string]string{ + "app.kubernetes.io/instance": "test-cluster", + "postgres-operator.crunchydata.com/role": "primary", + }, + }, + }, + { + ObjectMeta: metav1.ObjectMeta{ + Name: "test-cluster-master-1", + Namespace: "test-namespace", + Labels: map[string]string{ + "app.kubernetes.io/instance": "test-cluster", + "postgres-operator.crunchydata.com/role": "master", + }, + }, + }, + }, + expectedPod: "test-cluster-primary-0", + }, + "patroni 4.1.0-beta with primary role": { + cr: &v2.PerconaPGCluster{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-cluster", + Namespace: "test-namespace", + Annotations: map[string]string{ + pNaming.AnnotationPatroniVersion: "4.1.0-beta.1", + }, + }, + Spec: v2.PerconaPGClusterSpec{ + CRVersion: version.Version(), + }, + }, + pods: []corev1.Pod{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: "test-cluster-primary-0", + Namespace: "test-namespace", + Labels: map[string]string{ + "app.kubernetes.io/instance": "test-cluster", + "postgres-operator.crunchydata.com/role": "primary", + }, + }, + }, + }, + expectedPod: "test-cluster-primary-0", + }, + "patroni 3.9.9 with master role (just before 4.0.0)": { + cr: &v2.PerconaPGCluster{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-cluster", + Namespace: "test-namespace", + }, + Status: v2.PerconaPGClusterStatus{ + PatroniVersion: "3.9.9", + }, + Spec: v2.PerconaPGClusterSpec{ + CRVersion: "2.7.0", + }, + }, + pods: []corev1.Pod{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: "test-cluster-master-0", + Namespace: "test-namespace", + Labels: map[string]string{ + "app.kubernetes.io/instance": "test-cluster", + "postgres-operator.crunchydata.com/role": "master", + }, + }, + }, + }, + expectedPod: "test-cluster-master-0", + }, + } + + for name, tt := range tests { + t.Run(name, func(t *testing.T) { + scheme := runtime.NewScheme() + err := corev1.AddToScheme(scheme) + assert.NilError(t, err) + err = v2.AddToScheme(scheme) + assert.NilError(t, err) + + objects := []runtime.Object{tt.cr} + for i := range tt.pods { + objects = append(objects, &tt.pods[i]) + } + + fakeClient := fake.NewClientBuilder(). + WithScheme(scheme). + WithRuntimeObjects(objects...). + Build() + + pod, err := GetPrimaryPod(ctx, fakeClient, tt.cr) + + if tt.expectedError != "" { + assert.ErrorContains(t, err, tt.expectedError) + assert.Assert(t, pod == nil) + } else { + assert.NilError(t, err) + assert.Assert(t, pod != nil) + assert.Equal(t, pod.Name, tt.expectedPod) + } + }) + } +} From 3f6d94f90dc2b59b6f1f81126f9242b452bcf332 Mon Sep 17 00:00:00 2001 From: George Kechagias Date: Mon, 27 Oct 2025 12:31:06 +0200 Subject: [PATCH 03/11] fix imports --- percona/controller/pgcluster/patroniversion.go | 11 ++++++----- percona/postgres/common_test.go | 2 +- 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/percona/controller/pgcluster/patroniversion.go b/percona/controller/pgcluster/patroniversion.go index e8d97d45c..cf5ff00bc 100644 --- a/percona/controller/pgcluster/patroniversion.go +++ b/percona/controller/pgcluster/patroniversion.go @@ -8,11 +8,6 @@ import ( "time" gover "github.com/hashicorp/go-version" - "github.com/percona/percona-postgresql-operator/v2/internal/initialize" - "github.com/percona/percona-postgresql-operator/v2/internal/naming" - "github.com/percona/percona-postgresql-operator/v2/percona/clientcmd" - pNaming "github.com/percona/percona-postgresql-operator/v2/percona/naming" - v2 "github.com/percona/percona-postgresql-operator/v2/pkg/apis/pgv2.percona.com/v2" "github.com/pkg/errors" corev1 "k8s.io/api/core/v1" k8serrors "k8s.io/apimachinery/pkg/api/errors" @@ -24,6 +19,12 @@ import ( "k8s.io/utils/ptr" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" + + "github.com/percona/percona-postgresql-operator/v2/internal/initialize" + "github.com/percona/percona-postgresql-operator/v2/internal/naming" + "github.com/percona/percona-postgresql-operator/v2/percona/clientcmd" + pNaming "github.com/percona/percona-postgresql-operator/v2/percona/naming" + v2 "github.com/percona/percona-postgresql-operator/v2/pkg/apis/pgv2.percona.com/v2" ) var errPatroniVersionCheckWait = errors.New("waiting for pod to initialize") diff --git a/percona/postgres/common_test.go b/percona/postgres/common_test.go index 72a7f0d0a..8570e9d41 100644 --- a/percona/postgres/common_test.go +++ b/percona/postgres/common_test.go @@ -4,7 +4,6 @@ import ( "context" "testing" - "github.com/percona/percona-postgresql-operator/v2/percona/version" "gotest.tools/v3/assert" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -12,6 +11,7 @@ import ( "sigs.k8s.io/controller-runtime/pkg/client/fake" pNaming "github.com/percona/percona-postgresql-operator/v2/percona/naming" + "github.com/percona/percona-postgresql-operator/v2/percona/version" v2 "github.com/percona/percona-postgresql-operator/v2/pkg/apis/pgv2.percona.com/v2" ) From b79d002f028df273fccf23722e48adb75190a0a7 Mon Sep 17 00:00:00 2001 From: George Kechagias Date: Mon, 27 Oct 2025 13:05:34 +0200 Subject: [PATCH 04/11] fix failed test --- percona/watcher/wal_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/percona/watcher/wal_test.go b/percona/watcher/wal_test.go index d50375a8e..f6442391b 100644 --- a/percona/watcher/wal_test.go +++ b/percona/watcher/wal_test.go @@ -348,7 +348,7 @@ func TestGetLatestCommitTimestamp(t *testing.T) { CRVersion: version.Version(), }, }, - expectedErr: errors.New("failed to get patroni version: Malformed version: error: primary pod not found"), + expectedErr: errors.New("primary pod not found"), }, } for name, tt := range tests { @@ -357,7 +357,7 @@ func TestGetLatestCommitTimestamp(t *testing.T) { _, err := GetLatestCommitTimestamp(ctx, c, nil, tt.cluster, tt.backup) - assert.EqualError(t, err, tt.expectedErr.Error()) + assert.ErrorContains(t, err, tt.expectedErr.Error()) }) } } From 134bdd701f1aecafdc2599e27fc3d811a5d6e1cf Mon Sep 17 00:00:00 2001 From: George Kechagias Date: Mon, 27 Oct 2025 16:30:23 +0200 Subject: [PATCH 05/11] increase timeout on test upgrade-minor when creating the cluster --- e2e-tests/tests/upgrade-minor/01-create-cluster.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/e2e-tests/tests/upgrade-minor/01-create-cluster.yaml b/e2e-tests/tests/upgrade-minor/01-create-cluster.yaml index 358a147da..9b96bfe43 100644 --- a/e2e-tests/tests/upgrade-minor/01-create-cluster.yaml +++ b/e2e-tests/tests/upgrade-minor/01-create-cluster.yaml @@ -1,6 +1,6 @@ apiVersion: kuttl.dev/v1beta1 kind: TestStep -timeout: 10 +timeout: 100 commands: - script: |- set -o errexit From 31171cdf88dfa32577a3cfe216ccc4de186122ac Mon Sep 17 00:00:00 2001 From: George Kechagias Date: Fri, 31 Oct 2025 10:34:37 +0200 Subject: [PATCH 06/11] make custom annotation working only for 2.7 and older --- .../controller/pgcluster/patroniversion.go | 82 +++++++++++-------- .../pgcluster/patroniversion_test.go | 56 ------------- percona/postgres/common.go | 10 +-- 3 files changed, 47 insertions(+), 101 deletions(-) diff --git a/percona/controller/pgcluster/patroniversion.go b/percona/controller/pgcluster/patroniversion.go index cf5ff00bc..7b7f19369 100644 --- a/percona/controller/pgcluster/patroniversion.go +++ b/percona/controller/pgcluster/patroniversion.go @@ -30,43 +30,11 @@ import ( var errPatroniVersionCheckWait = errors.New("waiting for pod to initialize") func (r *PGClusterReconciler) reconcilePatroniVersion(ctx context.Context, cr *v2.PerconaPGCluster) error { - if cr.Annotations == nil { - cr.Annotations = make(map[string]string) - } - - if patroniVersion, ok := cr.Annotations[pNaming.AnnotationCustomPatroniVersion]; ok { - patroniVersionUpdateFunc := func() error { - cluster := &v2.PerconaPGCluster{} - if err := r.Client.Get(ctx, types.NamespacedName{ - Name: cr.Name, - Namespace: cr.Namespace, - }, cluster); err != nil { - return errors.Wrap(err, "get PerconaPGCluster") - } - - orig := cluster.DeepCopy() - - cluster.Status.Patroni.Version = patroniVersion - cluster.Status.PatroniVersion = patroniVersion - - if err := r.Client.Status().Patch(ctx, cluster.DeepCopy(), client.MergeFrom(orig)); err != nil { - return errors.Wrap(err, "failed to patch patroni version") - } - - err := r.patchPatroniVersionAnnotation(ctx, cr, patroniVersion) - if err != nil { - return errors.Wrap(err, "failed to patch patroni version annotation") - } - - return nil - } - - // To ensure that the update was done given that conflicts can be caused by - // other code making unrelated updates to the same resource at the same time. - if err := retry.RetryOnConflict(retry.DefaultRetry, patroniVersionUpdateFunc); err != nil { - return errors.Wrap(err, "failed to patch patroni version") + if cr.CompareVersion("2.7.0") <= 0 { + err := r.handleCustomPatroniVersionAnnotation(ctx, cr) + if err != nil { + return errors.Wrap(err, "handle patroni annotation") } - return nil } // Starting from version 2.8.0, the patroni version check pod should not be executed. @@ -260,6 +228,48 @@ func (r *PGClusterReconciler) getPatroniVersion(ctx context.Context, pod *corev1 return patroniVersion, nil } +func (r *PGClusterReconciler) handleCustomPatroniVersionAnnotation(ctx context.Context, cr *v2.PerconaPGCluster) error { + if cr.Annotations == nil { + cr.Annotations = make(map[string]string) + } + + if patroniVersion, ok := cr.Annotations[pNaming.AnnotationCustomPatroniVersion]; ok { + patroniVersionUpdateFunc := func() error { + cluster := &v2.PerconaPGCluster{} + if err := r.Client.Get(ctx, types.NamespacedName{ + Name: cr.Name, + Namespace: cr.Namespace, + }, cluster); err != nil { + return errors.Wrap(err, "get PerconaPGCluster") + } + + orig := cluster.DeepCopy() + + cluster.Status.Patroni.Version = patroniVersion + cluster.Status.PatroniVersion = patroniVersion + + if err := r.Client.Status().Patch(ctx, cluster.DeepCopy(), client.MergeFrom(orig)); err != nil { + return errors.Wrap(err, "failed to patch patroni version") + } + + err := r.patchPatroniVersionAnnotation(ctx, cr, patroniVersion) + if err != nil { + return errors.Wrap(err, "failed to patch patroni version annotation") + } + + return nil + } + + // To ensure that the update was done given that conflicts can be caused by + // other code making unrelated updates to the same resource at the same time. + if err := retry.RetryOnConflict(retry.DefaultRetry, patroniVersionUpdateFunc); err != nil { + return errors.Wrap(err, "failed to patch patroni version") + } + return nil + } + return nil +} + func (r *PGClusterReconciler) patchPatroniVersionAnnotation(ctx context.Context, cr *v2.PerconaPGCluster, patroniVersion string) error { orig := cr.DeepCopy() cr.Annotations[pNaming.AnnotationPatroniVersion] = patroniVersion diff --git a/percona/controller/pgcluster/patroniversion_test.go b/percona/controller/pgcluster/patroniversion_test.go index 16bdde7a6..bcd876d3b 100644 --- a/percona/controller/pgcluster/patroniversion_test.go +++ b/percona/controller/pgcluster/patroniversion_test.go @@ -18,62 +18,6 @@ import ( var _ = Describe("patroni version check", Ordered, func() { ctx := context.Background() - const crName = "patroni-version-test" - const ns = crName - crNamespacedName := types.NamespacedName{Name: crName, Namespace: ns} - - namespace := &corev1.Namespace{ - ObjectMeta: metav1.ObjectMeta{ - Name: crName, - 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) - }) - - Context("With custom patroni version annotation", func() { - cr, err := readDefaultCR(crName, ns) - It("should read default cr.yaml", func() { - Expect(err).NotTo(HaveOccurred()) - }) - - It("should create PerconaPGCluster with custom patroni version", func() { - if cr.Annotations == nil { - cr.Annotations = make(map[string]string) - } - cr.Annotations[pNaming.AnnotationCustomPatroniVersion] = "3.2.1" - - status := cr.Status - Expect(k8sClient.Create(ctx, cr)).Should(Succeed()) - cr.Status = status - Expect(k8sClient.Status().Update(ctx, cr)).Should(Succeed()) - }) - - It("should successfully reconcile patroni version check", func() { - reconcilerInstance := reconciler(cr) - err := reconcilerInstance.reconcilePatroniVersion(ctx, cr) - Expect(err).NotTo(HaveOccurred()) - }) - - It("should copy custom patroni version to status", func() { - updatedCR := &v2.PerconaPGCluster{} - Expect(k8sClient.Get(ctx, crNamespacedName, updatedCR)).Should(Succeed()) - - Expect(updatedCR.Status.Patroni.Version).To(Equal("3.2.1")) - Expect(updatedCR.Status.PatroniVersion).To(Equal("3.2.1")) - Expect(updatedCR.Annotations[pNaming.AnnotationPatroniVersion]).To(Equal("3.2.1")) - }) - }) - Context("Without custom patroni version annotation for cr version <=2.7", func() { const crName2 = "patroni-version-test-2" const ns2 = crName2 diff --git a/percona/postgres/common.go b/percona/postgres/common.go index a6857f02e..41026ade4 100644 --- a/percona/postgres/common.go +++ b/percona/postgres/common.go @@ -9,7 +9,6 @@ import ( "k8s.io/apimachinery/pkg/labels" "sigs.k8s.io/controller-runtime/pkg/client" - pNaming "github.com/percona/percona-postgresql-operator/v2/percona/naming" v2 "github.com/percona/percona-postgresql-operator/v2/pkg/apis/pgv2.percona.com/v2" ) @@ -59,12 +58,5 @@ func determineVersion(cr *v2.PerconaPGCluster) string { if cr.CompareVersion("2.7.0") <= 0 { return cr.Status.PatroniVersion } - patroniVersion, ok := cr.Annotations[pNaming.AnnotationPatroniVersion] - if !ok { - // If the annotation is non-existing, the operator is assuming version 4.x.x by default - // in order to enforce the use of "primary" role. Patroni version after 4.x.x will also - // use "primary", so the operator will be compatible with them as well. - return patroniVersion4 - } - return patroniVersion + return patroniVersion4 } From c255890f90e4ed98ab25889a034367778c06ac1a Mon Sep 17 00:00:00 2001 From: George Kechagias Date: Fri, 31 Oct 2025 10:35:01 +0200 Subject: [PATCH 07/11] update cr --- deploy/cr.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/deploy/cr.yaml b/deploy/cr.yaml index ac61fd278..e055bc6f3 100644 --- a/deploy/cr.yaml +++ b/deploy/cr.yaml @@ -3,7 +3,7 @@ kind: PerconaPGCluster metadata: name: cluster1 # annotations: -# pgv2.percona.com/custom-patroni-version: "4" +# test-annotation: value # finalizers: # - percona.com/delete-pvc # - percona.com/delete-ssl From 0cfcb3e91b6ce58a98f5ec451cd5fa8c8bcef891 Mon Sep 17 00:00:00 2001 From: George Kechagias Date: Fri, 31 Oct 2025 13:48:55 +0200 Subject: [PATCH 08/11] early return --- .../controller/pgcluster/patroniversion.go | 67 +++++++++---------- 1 file changed, 33 insertions(+), 34 deletions(-) diff --git a/percona/controller/pgcluster/patroniversion.go b/percona/controller/pgcluster/patroniversion.go index 7b7f19369..50b582d9e 100644 --- a/percona/controller/pgcluster/patroniversion.go +++ b/percona/controller/pgcluster/patroniversion.go @@ -31,9 +31,15 @@ var errPatroniVersionCheckWait = errors.New("waiting for pod to initialize") func (r *PGClusterReconciler) reconcilePatroniVersion(ctx context.Context, cr *v2.PerconaPGCluster) error { if cr.CompareVersion("2.7.0") <= 0 { - err := r.handleCustomPatroniVersionAnnotation(ctx, cr) - if err != nil { - return errors.Wrap(err, "handle patroni annotation") + if cr.Annotations == nil { + cr.Annotations = make(map[string]string) + } + if patroniVersion, ok := cr.Annotations[pNaming.AnnotationCustomPatroniVersion]; ok { + err := r.handleCustomPatroniVersionAnnotation(ctx, cr, patroniVersion) + if err != nil { + return errors.Wrap(err, "handle patroni annotation") + } + return nil } } @@ -228,45 +234,38 @@ func (r *PGClusterReconciler) getPatroniVersion(ctx context.Context, pod *corev1 return patroniVersion, nil } -func (r *PGClusterReconciler) handleCustomPatroniVersionAnnotation(ctx context.Context, cr *v2.PerconaPGCluster) error { - if cr.Annotations == nil { - cr.Annotations = make(map[string]string) - } - - if patroniVersion, ok := cr.Annotations[pNaming.AnnotationCustomPatroniVersion]; ok { - patroniVersionUpdateFunc := func() error { - cluster := &v2.PerconaPGCluster{} - if err := r.Client.Get(ctx, types.NamespacedName{ - Name: cr.Name, - Namespace: cr.Namespace, - }, cluster); err != nil { - return errors.Wrap(err, "get PerconaPGCluster") - } - - orig := cluster.DeepCopy() - - cluster.Status.Patroni.Version = patroniVersion - cluster.Status.PatroniVersion = patroniVersion +func (r *PGClusterReconciler) handleCustomPatroniVersionAnnotation(ctx context.Context, cr *v2.PerconaPGCluster, patroniVersion string) error { + patroniVersionUpdateFunc := func() error { + cluster := &v2.PerconaPGCluster{} + if err := r.Client.Get(ctx, types.NamespacedName{ + Name: cr.Name, + Namespace: cr.Namespace, + }, cluster); err != nil { + return errors.Wrap(err, "get PerconaPGCluster") + } - if err := r.Client.Status().Patch(ctx, cluster.DeepCopy(), client.MergeFrom(orig)); err != nil { - return errors.Wrap(err, "failed to patch patroni version") - } + orig := cluster.DeepCopy() - err := r.patchPatroniVersionAnnotation(ctx, cr, patroniVersion) - if err != nil { - return errors.Wrap(err, "failed to patch patroni version annotation") - } + cluster.Status.Patroni.Version = patroniVersion + cluster.Status.PatroniVersion = patroniVersion - return nil + if err := r.Client.Status().Patch(ctx, cluster.DeepCopy(), client.MergeFrom(orig)); err != nil { + return errors.Wrap(err, "failed to patch patroni version") } - // To ensure that the update was done given that conflicts can be caused by - // other code making unrelated updates to the same resource at the same time. - if err := retry.RetryOnConflict(retry.DefaultRetry, patroniVersionUpdateFunc); err != nil { - return errors.Wrap(err, "failed to patch patroni version") + err := r.patchPatroniVersionAnnotation(ctx, cr, patroniVersion) + if err != nil { + return errors.Wrap(err, "failed to patch patroni version annotation") } + return nil } + + // To ensure that the update was done given that conflicts can be caused by + // other code making unrelated updates to the same resource at the same time. + if err := retry.RetryOnConflict(retry.DefaultRetry, patroniVersionUpdateFunc); err != nil { + return errors.Wrap(err, "failed to patch patroni version") + } return nil } From baeed56a176ed0da8a7e59a09a81056e0ac9cfc4 Mon Sep 17 00:00:00 2001 From: George Kechagias Date: Fri, 31 Oct 2025 17:13:57 +0200 Subject: [PATCH 09/11] make annotations great again --- percona/controller/pgcluster/patroniversion.go | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/percona/controller/pgcluster/patroniversion.go b/percona/controller/pgcluster/patroniversion.go index 50b582d9e..bb305cb99 100644 --- a/percona/controller/pgcluster/patroniversion.go +++ b/percona/controller/pgcluster/patroniversion.go @@ -30,10 +30,10 @@ import ( var errPatroniVersionCheckWait = errors.New("waiting for pod to initialize") func (r *PGClusterReconciler) reconcilePatroniVersion(ctx context.Context, cr *v2.PerconaPGCluster) error { + if cr.Annotations == nil { + cr.Annotations = make(map[string]string) + } if cr.CompareVersion("2.7.0") <= 0 { - if cr.Annotations == nil { - cr.Annotations = make(map[string]string) - } if patroniVersion, ok := cr.Annotations[pNaming.AnnotationCustomPatroniVersion]; ok { err := r.handleCustomPatroniVersionAnnotation(ctx, cr, patroniVersion) if err != nil { @@ -45,7 +45,6 @@ func (r *PGClusterReconciler) reconcilePatroniVersion(ctx context.Context, cr *v // Starting from version 2.8.0, the patroni version check pod should not be executed. if cr.CompareVersion("2.8.0") >= 0 { - pods, err := r.getInstancePods(ctx, cr) if err != nil { return errors.Wrap(err, "failed to get instance pods") From d24f06771c134e2eb9855a58b64ee7a9f1c63e5e Mon Sep 17 00:00:00 2001 From: George Kechagias Date: Mon, 3 Nov 2025 12:06:56 +0200 Subject: [PATCH 10/11] reconcile one more time --- percona/controller/pgcluster/controller_test.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/percona/controller/pgcluster/controller_test.go b/percona/controller/pgcluster/controller_test.go index 1ba73e632..66da4b442 100644 --- a/percona/controller/pgcluster/controller_test.go +++ b/percona/controller/pgcluster/controller_test.go @@ -2294,6 +2294,8 @@ var _ = Describe("Init Container", Ordered, func() { Expect(err).NotTo(HaveOccurred()) _, err = crunchyReconciler().Reconcile(ctx, ctrl.Request{NamespacedName: crNamespacedName}) Expect(err).NotTo(HaveOccurred()) + _, err = crunchyReconciler().Reconcile(ctx, ctrl.Request{NamespacedName: crNamespacedName}) + Expect(err).NotTo(HaveOccurred()) }) Context("check init containers", func() { From 898926bc1f61ba05202faf39c17b17dbb6892607 Mon Sep 17 00:00:00 2001 From: George Kechagias Date: Mon, 3 Nov 2025 12:47:33 +0200 Subject: [PATCH 11/11] fixes --- percona/controller/pgcluster/controller_test.go | 3 +-- percona/naming/prefix.go | 4 ---- 2 files changed, 1 insertion(+), 6 deletions(-) diff --git a/percona/controller/pgcluster/controller_test.go b/percona/controller/pgcluster/controller_test.go index 66da4b442..30de7aece 100644 --- a/percona/controller/pgcluster/controller_test.go +++ b/percona/controller/pgcluster/controller_test.go @@ -2266,6 +2266,7 @@ var _ = Describe("Init Container", Ordered, func() { }) cr, err := readDefaultCR(crName, ns) + cr.Spec.CRVersion = "2.7.0" It("should read defautl cr.yaml", func() { Expect(err).NotTo(HaveOccurred()) }) @@ -2294,8 +2295,6 @@ var _ = Describe("Init Container", Ordered, func() { Expect(err).NotTo(HaveOccurred()) _, err = crunchyReconciler().Reconcile(ctx, ctrl.Request{NamespacedName: crNamespacedName}) Expect(err).NotTo(HaveOccurred()) - _, err = crunchyReconciler().Reconcile(ctx, ctrl.Request{NamespacedName: crNamespacedName}) - Expect(err).NotTo(HaveOccurred()) }) Context("check init containers", func() { diff --git a/percona/naming/prefix.go b/percona/naming/prefix.go index 4d599b747..73b78f66a 100644 --- a/percona/naming/prefix.go +++ b/percona/naming/prefix.go @@ -13,10 +13,6 @@ func ToCrunchyAnnotation(annotation string) string { return replacePrefix(annotation, PrefixPerconaPGV2, PrefixCrunchy) } -func ToPerconaAnnotation(annotation string) string { - return replacePrefix(annotation, PrefixCrunchy, PrefixPerconaPGV2) -} - func replacePrefix(s, oldPrefix, newPrefix string) string { s, found := strings.CutPrefix(s, oldPrefix) if found {