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

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

* fix cluster readiness when no storages

* fix pause

* fix replset-overrides

* fix liveness

Author: egegunes

e2e-tests/custom-users-roles/conf/some-name-rs0.yml

@@ -61,39 +61,6 @@ spec:
   backup:
     enabled: true
     image: perconalab/percona-server-mongodb-operator:1.1.0-backup
-    storages:
-      aws-s3:
-        main: true
-        type: s3
-        s3:
-          credentialsSecret: aws-s3-secret
-          region: us-east-1
-          bucket: operator-testing
-          prefix: psmdb
-          insecureSkipTLSVerify: false
-      minio:
-        type: s3
-        s3:
-          credentialsSecret: minio-secret
-          region: us-east-1
-          bucket: operator-testing
-          endpointUrl: http://minio-service:9000/
-          insecureSkipTLSVerify: false
-      gcp-cs:
-        type: s3
-        s3:
-          credentialsSecret: gcp-cs-secret
-          region: us-east-1
-          bucket: operator-testing
-          prefix: psmdb
-          endpointUrl: https://storage.googleapis.com
-          insecureSkipTLSVerify: false
-    tasks:
-    - name: weekly
-      enabled: true
-      schedule: "0 0 * * 0"
-      compressionType: gzip
-      storageName: aws-s3
   replsets:
   - name: rs0
     affinity:

e2e-tests/functions

@@ -2168,7 +2168,7 @@ wait_for_cluster_state() {
 		echo -n '.'
 		if [[ ${timeout} -gt 1500 ]]; then
 			echo
-			echo "Waiting timeout has been reached. Exiting..."
+			log "Waiting timeout has been reached. Exiting..."
 			exit 1
 		fi
 	done

e2e-tests/liveness/run

@@ -7,6 +7,7 @@ test_dir=$(realpath $(dirname $0))
 set_debug
 
 create_infra $namespace
+deploy_minio
 
 desc 'create secrets and start client'
 kubectl_bin apply -f $conf_dir/secrets.yml -f $conf_dir/client.yml -f $conf_dir/minio-secret.yml

e2e-tests/replset-overrides/conf/some-name-overridden.yml

@@ -24,7 +24,7 @@ spec:
     size: 3
     expose:
       enabled: false
-      exposeType: ClusterIP
+      type: ClusterIP
     replsetOverrides:
       some-name-rs0-0:
         host: external-rs0-0.NAMESPACE.svc:27017
@@ -58,7 +58,7 @@ spec:
         antiAffinityTopologyKey: none
       expose:
         enabled: false
-        exposeType: ClusterIP
+        type: ClusterIP
       podDisruptionBudget:
         maxUnavailable: 1
       resources:
@@ -79,7 +79,7 @@ spec:
       affinity:
         antiAffinityTopologyKey: none
       expose:
-        exposeType: ClusterIP
+        type: ClusterIP
       podDisruptionBudget:
         maxUnavailable: 1
       resources:

e2e-tests/replset-overrides/conf/some-name-override-priority.yml

@@ -24,7 +24,7 @@ spec:
     size: 3
     expose:
       enabled: false
-      exposeType: ClusterIP
+      type: ClusterIP
     replsetOverrides:
       some-name-rs0-0:
         priority: 1

e2e-tests/replset-overrides/conf/some-name.yml

@@ -24,7 +24,7 @@ spec:
     size: 3
     expose:
       enabled: false
-      exposeType: ClusterIP
+      type: ClusterIP
     resources:
       limits:
         cpu: 300m
@@ -45,7 +45,7 @@ spec:
         antiAffinityTopologyKey: none
       expose:
         enabled: false
-        exposeType: ClusterIP
+        type: ClusterIP
       podDisruptionBudget:
         maxUnavailable: 1
       resources:
@@ -66,7 +66,7 @@ spec:
       affinity:
         antiAffinityTopologyKey: none
       expose:
-        exposeType: ClusterIP
+        type: ClusterIP
       podDisruptionBudget:
         maxUnavailable: 1
       resources:

e2e-tests/replset-overrides/run

@@ -61,6 +61,7 @@ test_override_host_after_deploy() {
 	echo "patching PSMDB cluster with replsetOverrides: ${cluster}"
 	sed "s/NAMESPACE/${namespace}/g" ${test_dir}/conf/${cluster}-overridden.yml > ${tmp_dir}/${cluster}-overridden.yml
 	apply_cluster ${tmp_dir}/${cluster}-overridden.yml
+	sleep_with_log 10 "wait for replset reconfiguration"
 	wait_for_cluster_state ${cluster} "ready"
 
 	echo "checking if we can read existing data"

pkg/apis/psmdb/v1/psmdb_types.go

@@ -340,9 +340,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"`
@@ -370,6 +374,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"`
@@ -1498,6 +1529,15 @@ func (cr *PerconaServerMongoDB) MongosNamespacedName() types.NamespacedName {
 	return types.NamespacedName{Name: cr.Name + "-" + "mongos", Namespace: cr.Namespace}
 }
 
+func (cr *PerconaServerMongoDB) GetReplsets() []*ReplsetSpec {
+	replsets := make([]*ReplsetSpec, 0)
+	replsets = append(replsets, cr.Spec.Replsets...)
+	if cr.Spec.Sharding.Enabled {
+		replsets = append(replsets, cr.Spec.Sharding.ConfigsvrReplSet)
+	}
+	return replsets
+}
+
 func (cr *PerconaServerMongoDB) CanBackup(ctx context.Context) error {
 	log := logf.FromContext(ctx).V(1).WithValues("cluster", cr.Name, "namespace", cr.Namespace)
 	log.Info("checking if backup is allowed")
@@ -1547,33 +1587,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)

pkg/controller/perconaservermongodb/backup.go

@@ -1,10 +1,8 @@
 package perconaservermongodb
 
 import (
-	"bytes"
 	"container/heap"
 	"context"
-	"strings"
 	"time"
 
 	"github.com/pkg/errors"
@@ -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"
 )
 
@@ -444,117 +439,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
-}

pkg/controller/perconaservermongodb/conditions.go

@@ -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)
+}