K8SPSMDB-1425: Add pitr-to-new-cluster e2e test #2070

egegunes · 2025-09-29T05:54:05Z

CHANGE DESCRIPTION

Add e2e test for K8SPSMDB-1425.

Also I fix two problems that I encountered in e2e test:

After restore finishes, operator reverts main storage as it should but doesn't actually trigger resync.
Backoff configuration for backup validation retries sometimes are not enough.

CHECKLIST

Jira

Is the Jira ticket created and referenced properly?
Does the Jira ticket have the proper statuses for documentation (Needs Doc) and QA (Needs QA)?
Does the Jira ticket link to the proper milestone (Fix Version field)?

Tests

Is an E2E test/test case added for the new feature/change?
Are unit tests added where appropriate?
Are OpenShift compare files changed for E2E tests (compare/*-oc.yml)?

Config/Logging/Testability

Are all needed new/changed options added to default YAML files?
Are all needed new/changed options added to the Helm Chart?
Did we add proper logging messages for operator actions?
Did we ensure compatibility with the previous version or cluster upgrade process?
Does the change support oldest and newest supported MongoDB version?
Does the change support oldest and newest supported Kubernetes version?

e2e-tests/pitr-to-new-cluster/run

github-actions · 2025-10-01T08:59:12Z

e2e-tests/pitr-to-new-cluster/run

+		pbm_binary=/opt/percona/pbm
+	fi
+
+	kubectl_bin exec ${cluster}-rs0-0 -c ${container} -- ${pbm_binary} config > ${tmp_dir}/pbm_config.yml


[shfmt] _{reported by reviewdog 🐶}

Suggested change

kubectl_bin exec ${cluster}-rs0-0 -c ${container} -- ${pbm_binary} config > ${tmp_dir}/pbm_config.yml

kubectl_bin exec ${cluster}-rs0-0 -c ${container} -- ${pbm_binary} config >${tmp_dir}/pbm_config.yml

Copilot

Pull Request Overview

This PR adds an end-to-end test for K8SPSMDB-1425, specifically testing Point-in-Time Recovery (PITR) to a new cluster. The test validates the ability to restore backups from a source cluster to a completely separate target cluster using both logical and physical backup types.

Key changes:

Adds a new e2e test pitr-to-new-cluster that tests PITR functionality across different clusters
Updates test execution lists to include the new test in release, PR, and backup test suites
Implements test scenarios for both logical and physical restore operations with PBM configuration validation

Reviewed Changes

Copilot reviewed 17 out of 17 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
e2e-tests/run-release.csv	Adds pitr-to-new-cluster test to release test suite
e2e-tests/run-pr.csv	Adds pitr-to-new-cluster test to PR test suite
e2e-tests/run-backups.csv	Adds pitr-to-new-cluster and other backup tests to backup test suite
e2e-tests/pitr-to-new-cluster/run	Main test script implementing the PITR cross-cluster functionality
e2e-tests/pitr-to-new-cluster/conf/*	Configuration files for source/target clusters and backup/restore operations
e2e-tests/pitr-to-new-cluster/compare/*	Expected output files for test validation
e2e-tests/functions	Updates echo statements to include '.OK' suffix for consistency

_{Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.}

Copilot

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

Copilot

Pull Request Overview

Copilot reviewed 17 out of 17 changed files in this pull request and generated 2 comments.

_{Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.}

Copilot · 2025-10-01T09:13:32Z

pkg/controller/perconaservermongodb/pbm.go

 	if isResyncNeeded(currentCfg, &main) {
-		log.Info("resync storage", "storage", mainStgName)
-		log.V(1).Info("main storage changed", "old", currentCfg.Storage, "new", main.Storage)
+		log.Info("main storage changed. starting resync", "old", currentCfg.Storage, "new", main.Storage)


The variable currentCfg may be nil at this point since the code that ensures it's not nil was removed. This could cause a panic when accessing currentCfg.Storage in the log message.

Suggested change

log.Info("main storage changed. starting resync", "old", currentCfg.Storage, "new", main.Storage)

var oldStorage interface{}

if currentCfg != nil {

oldStorage = currentCfg.Storage

} else {

oldStorage = nil

}

log.Info("main storage changed. starting resync", "old", oldStorage, "new", main.Storage)

doesn't make sense, nil config already handled on line 118

Copilot · 2025-10-01T09:13:33Z

e2e-tests/pitr-to-new-cluster/conf/restore-backupsource.yml

+spec:
+  clusterName:
+  pitr:
+    pitrType: latest


The field pitrType is being used here but the sed replacement pattern on line 62 of the test script suggests this should be type. This inconsistency could cause the restore configuration to be malformed.

Suggested change

pitrType: latest

type: latest

gkech

tests are passing for me, leaving some minor comments

gkech · 2025-10-01T09:27:19Z

e2e-tests/pitr-to-new-cluster/run

+setup_target_cluster
+
+desc "Case 1: Logical restore"
+test_case_1


For the function names, I would rather have the purpose than the number of the case we are evaluating. So test_pbm_config_logical, test_pbm_config_physical pitr_backup_restore, or something like that.

gkech · 2025-10-01T09:37:22Z

e2e-tests/pitr-to-new-cluster/run

+	sleep_with_log 3 "wait for data to be replicated"
+
+	compare_mongo_cmd "find" "myApp:myPass@${target_cluster}-rs0.${namespace}" "-3rd" \
+		".svc.cluster.local" myApp test 'sort( { x: 1 } )'
+	log "Data is ready for PiTR: OK"


If we added retries for the find command, sleeping would be redundant, right?

K8SPSMDB-1425: Add pitr-to-new-cluster e2e test

e5bc64b

egegunes added this to the v1.21.0 milestone Sep 29, 2025

egegunes requested review from jvpasinatto, eleo007 and valmiranogueira as code owners September 29, 2025 05:54

pull-request-size bot added the size/L 100-499 lines label Sep 29, 2025

github-actions bot added the tests label Sep 29, 2025

eleo007 reviewed Sep 29, 2025

View reviewed changes

e2e-tests/pitr-to-new-cluster/run Outdated Show resolved Hide resolved

egegunes and others added 2 commits September 29, 2025 19:38

fix test

33a690d

Merge branch 'main' into K8SPSMDB-1425-e2e

252b9c8

eleo007 reviewed Sep 30, 2025

View reviewed changes

e2e-tests/pitr-to-new-cluster/run Outdated Show resolved Hide resolved

egegunes added 3 commits October 1, 2025 11:58

fix triggering resync after restore

e61b7da

increase retries for backup validation

8bc026e

improve test

bcc94f7

Copilot AI review requested due to automatic review settings October 1, 2025 08:58

egegunes requested review from hors, pooknull, nmarukovich and gkech as code owners October 1, 2025 08:58

pull-request-size bot added size/XL 500-999 lines and removed size/L 100-499 lines labels Oct 1, 2025

github-actions bot reviewed Oct 1, 2025

View reviewed changes

Merge branch 'main' into K8SPSMDB-1425-e2e

b7c9eaf

Copilot AI reviewed Oct 1, 2025

View reviewed changes

percona deleted a comment from Copilot AI Oct 1, 2025

hors requested a review from eleo007 October 1, 2025 09:03

Copilot AI reviewed Oct 1, 2025

View reviewed changes

egegunes requested a review from Copilot October 1, 2025 09:12

Copilot AI reviewed Oct 1, 2025

View reviewed changes

gkech approved these changes Oct 1, 2025

View reviewed changes

eleo007 approved these changes Oct 1, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

K8SPSMDB-1425: Add pitr-to-new-cluster e2e test #2070

K8SPSMDB-1425: Add pitr-to-new-cluster e2e test #2070

egegunes commented Sep 29, 2025 •

edited

Loading

Uh oh!

Uh oh!

Uh oh!

github-actions bot Oct 1, 2025

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Oct 1, 2025

Uh oh!

egegunes Oct 1, 2025

Uh oh!

Copilot AI Oct 1, 2025

Uh oh!

gkech left a comment

Uh oh!

gkech Oct 1, 2025

Uh oh!

gkech Oct 1, 2025

Uh oh!

Uh oh!

	kubectl_bin exec ${cluster}-rs0-0 -c ${container} -- ${pbm_binary} config > ${tmp_dir}/pbm_config.yml
	kubectl_bin exec ${cluster}-rs0-0 -c ${container} -- ${pbm_binary} config >${tmp_dir}/pbm_config.yml

-		log.Info("main storage changed. starting resync", "old", currentCfg.Storage, "new", main.Storage)
+		var oldStorage interface{}
+		if currentCfg != nil {
+			oldStorage = currentCfg.Storage
+		} else {
+			oldStorage = nil
+		}
+		log.Info("main storage changed. starting resync", "old", oldStorage, "new", main.Storage)

K8SPSMDB-1425: Add pitr-to-new-cluster e2e test #2070

Are you sure you want to change the base?

K8SPSMDB-1425: Add pitr-to-new-cluster e2e test #2070

Conversation

egegunes commented Sep 29, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

CHANGE DESCRIPTION

CHECKLIST

Uh oh!

Uh oh!

Uh oh!

github-actions bot Oct 1, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Uh oh!

Copilot AI Oct 1, 2025

Choose a reason for hiding this comment

Uh oh!

egegunes Oct 1, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Oct 1, 2025

Choose a reason for hiding this comment

Uh oh!

gkech left a comment

Choose a reason for hiding this comment

Uh oh!

gkech Oct 1, 2025

Choose a reason for hiding this comment

Uh oh!

gkech Oct 1, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

egegunes commented Sep 29, 2025 •

edited

Loading