Skip to content

K8SPSMDB-1527: Cluster can't be ready before PBM is ready #2152

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
egegunes wants to merge 1 commit into
base: main
Choose a base branch
from
+515 −174
Draft

K8SPSMDB-1527: Cluster can't be ready before PBM is ready #2152

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
64 changes: 34 additions & 30 deletions pkg/apis/psmdb/v1/psmdb_types.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -339,9 +339,13 @@ const (
ConditionUnknown ConditionStatus = "Unknown"
)

// ConditionTypePendingSmartUpdate is a condition type set on PSMDBCluster when a smart update is required
// but has not yet started. For e.g., if a backup/restore is running at the same time as a smart update is triggered.
const ConditionTypePendingSmartUpdate AppState = "pendingSmartUpdate"
const (
// ConditionTypePendingSmartUpdate is a condition type set on PSMDBCluster when a smart update is required
// but has not yet started. For e.g., if a backup/restore is running at the same time as a smart update is triggered.
ConditionTypePendingSmartUpdate AppState = "pendingSmartUpdate"

ConditionTypePBMReady AppState = "PBMReady"
)

type ClusterCondition struct {
Status ConditionStatus `json:"status"`
Expand Down Expand Up @@ -369,6 +373,33 @@ func (s *PerconaServerMongoDBStatus) IsStatusConditionTrue(conditionType AppStat
return cond.Status == ConditionTrue
}

func (s *PerconaServerMongoDBStatus) AddCondition(c ClusterCondition) {
existingCondition := s.FindCondition(c.Type)
if existingCondition == nil {
if c.LastTransitionTime.IsZero() {
c.LastTransitionTime = metav1.NewTime(time.Now())
}
s.Conditions = append(s.Conditions, c)
return
}

if existingCondition.Status != c.Status {
existingCondition.Status = c.Status
if !c.LastTransitionTime.IsZero() {
existingCondition.LastTransitionTime = c.LastTransitionTime
} else {
existingCondition.LastTransitionTime = metav1.NewTime(time.Now())
}
}

if existingCondition.Reason != c.Reason {
existingCondition.Reason = c.Reason
}
if existingCondition.Message != c.Message {
existingCondition.Message = c.Message
}
}

type PMMSpec struct {
Enabled bool `json:"enabled,omitempty"`
ServerHost string `json:"serverHost,omitempty"`
Expand Down Expand Up @@ -1439,33 +1470,6 @@ func (s *PerconaServerMongoDBStatus) RemoveCondition(conditionType AppState) {
}
}

func (s *PerconaServerMongoDBStatus) AddCondition(c ClusterCondition) {
existingCondition := s.FindCondition(c.Type)
if existingCondition == nil {
if c.LastTransitionTime.IsZero() {
c.LastTransitionTime = metav1.NewTime(time.Now())
}
s.Conditions = append(s.Conditions, c)
return
}

if existingCondition.Status != c.Status {
existingCondition.Status = c.Status
if !c.LastTransitionTime.IsZero() {
existingCondition.LastTransitionTime = c.LastTransitionTime
} else {
existingCondition.LastTransitionTime = metav1.NewTime(time.Now())
}
}

if existingCondition.Reason != c.Reason {
existingCondition.Reason = c.Reason
}
if existingCondition.Message != c.Message {
existingCondition.Message = c.Message
}
}

// GetExternalNodes returns all external nodes for all replsets
func (cr *PerconaServerMongoDB) GetExternalNodes() []*ExternalNode {
extNodes := make([]*ExternalNode, 0)
Expand Down
119 changes: 0 additions & 119 deletions pkg/controller/perconaservermongodb/backup.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,10 +1,8 @@
package perconaservermongodb

import (
"bytes"
"container/heap"
"context"
"strings"
"time"

"github.com/pkg/errors"
Expand All @@ -19,12 +17,9 @@ import (
logf "sigs.k8s.io/controller-runtime/pkg/log"

"github.com/percona/percona-backup-mongodb/pbm/defs"
pbmVersion "github.com/percona/percona-backup-mongodb/pbm/version"

api "github.com/percona/percona-server-mongodb-operator/pkg/apis/psmdb/v1"
"github.com/percona/percona-server-mongodb-operator/pkg/k8s"
"github.com/percona/percona-server-mongodb-operator/pkg/naming"
"github.com/percona/percona-server-mongodb-operator/pkg/psmdb"
"github.com/percona/percona-server-mongodb-operator/pkg/psmdb/backup"
)

Expand Down Expand Up @@ -403,117 +398,3 @@ func isPodUpToDate(pod *corev1.Pod, stsRevision, image string) bool {

return true
}

func (r *ReconcilePerconaServerMongoDB) reconcileBackupVersion(ctx context.Context, cr *api.PerconaServerMongoDB) error {
log := logf.FromContext(ctx)

if !cr.Spec.Backup.Enabled {
return nil
}

if cr.Status.State != api.AppStateReady {
return nil
}

if cr.Status.BackupVersion != "" && cr.Status.BackupImage == cr.Spec.Backup.Image {
return nil
}

if len(cr.Spec.Replsets) < 1 {
return errors.New("no replsets found")
}

var rs *api.ReplsetSpec
for _, r := range cr.Spec.Replsets {
rs = r
break
}

stsName := naming.MongodStatefulSetName(cr, rs)
sts := psmdb.NewStatefulSet(stsName, cr.Namespace)
err := r.client.Get(ctx, client.ObjectKeyFromObject(sts), sts)
if err != nil {
return errors.Wrapf(err, "get statefulset/%s", stsName)
}

matchLabels := naming.RSLabels(cr, rs)
label, ok := sts.Labels[naming.LabelKubernetesComponent]
if ok {
matchLabels[naming.LabelKubernetesComponent] = label
}

podList := corev1.PodList{}
if err := r.client.List(ctx,
&podList,
&client.ListOptions{
Namespace: cr.Namespace,
LabelSelector: labels.SelectorFromSet(matchLabels),
},
); err != nil {
return errors.Wrap(err, "get pod list")
}

var pod *corev1.Pod
for _, p := range podList.Items {
if !k8s.IsPodReady(p) {
continue
}

if !isPodUpToDate(&p, sts.Status.UpdateRevision, cr.Spec.Backup.Image) {
continue
}

pod = &p
break
}
if pod == nil {
log.V(1).Error(nil, "no ready pods to get pbm-agent version")
return nil
}

stdout := &bytes.Buffer{}
stderr := &bytes.Buffer{}
cmd := []string{"pbm-agent", "version", "--short"}

err = r.clientcmd.Exec(ctx, pod, naming.ContainerBackupAgent, cmd, nil, stdout, stderr, false)
if err != nil {
return errors.Wrap(err, "get pbm-agent version")
}

// PBM v2.9.0 and above prints version to stderr, below prints it to stdout
stdoutStr := strings.TrimSpace(stdout.String())
stderrStr := strings.TrimSpace(stderr.String())
if stdoutStr != "" && stderrStr != "" {
log.V(1).Info("pbm-agent version found in both stdout and stderr; using stdout",
"stdout", stdoutStr, "stderr", stderrStr)
cr.Status.BackupVersion = stdoutStr
} else if stdoutStr != "" {
cr.Status.BackupVersion = stdoutStr
} else if stderrStr != "" {
cr.Status.BackupVersion = stderrStr
} else {
return errors.New("pbm-agent version not found in stdout or stderr")
}

cr.Status.BackupImage = cr.Spec.Backup.Image

log.Info("pbm-agent version",
"pod", pod.Name,
"image", cr.Status.BackupImage,
"version", cr.Status.BackupVersion)

pbmInfo := pbmVersion.Current()

compare, err := cr.ComparePBMAgentVersion(pbmInfo.Version)
if err != nil {
return errors.Wrap(err, "compare pbm-agent version with go module")
}

if compare != 0 {
log.Info("pbm-agent version is different than the go module, this might create problems",
"pbmAgentVersion", cr.Status.BackupVersion,
"goModuleVersion", pbmInfo.Version)
}

return nil
}
12 changes: 12 additions & 0 deletions pkg/controller/perconaservermongodb/conditions.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
package perconaservermongodb

import (
"context"

psmdbv1 "github.com/percona/percona-server-mongodb-operator/pkg/apis/psmdb/v1"
)

func (r *ReconcilePerconaServerMongoDB) updateCondition(ctx context.Context, cr *psmdbv1.PerconaServerMongoDB, c psmdbv1.ClusterCondition) error {
cr.Status.AddCondition(c)
return r.writeStatus(ctx, cr)
Copy link

Copilot AI Dec 18, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The updateCondition function calls writeStatus which immediately persists the status to the API server. This is called multiple times during reconciliation (lines 166 and 191 in pbm.go), but there's also a deferred updateStatus call at the end of the Reconcile function. While the CR is passed by reference so the conditions should be preserved, calling writeStatus multiple times during a single reconciliation loop is not ideal as it creates unnecessary API calls and potential race conditions. Consider accumulating condition changes and writing them only once at the end of reconciliation, similar to how other status fields are handled.

Suggested change
return r.writeStatus(ctx, cr)
return nil

Copilot uses AI. Check for mistakes.
}
Loading
Loading