Fix issue where the presence of --target-timeline was adding --target-action.

Adjust tests and add more test cases.

Author: dsessler7

internal/controller/postgrescluster/pgbackrest.go

@@ -1153,10 +1153,16 @@ func (r *Reconciler) reconcileRestoreJob(ctx context.Context,
 		"--pg1-path=" + pgdata,
 		"--repo=" + regexRepoIndex.FindString(repoName)}...)
 
+	// Look specifically for the "--target" flag, NOT flags that contain
+	// "--target" (e.g. "--target-timeline")
+	targetRegex, err := regexp.Compile("--target[ =]")
+	if err != nil {
+		return err
+	}
 	var deltaOptFound, foundTarget bool
 	for _, opt := range opts {
 		switch {
-		case strings.Contains(opt, "--target"):
+		case targetRegex.Match([]byte(opt)):
 			foundTarget = true
 		case strings.Contains(opt, "--delta"):
 			deltaOptFound = true

internal/controller/postgrescluster/pgbackrest_test.go

@@ -1777,6 +1777,9 @@ func TestReconcilePostgresClusterDataSource(t *testing.T) {
 		configCount, jobCount, pvcCount                         int
 		invalidSourceRepo, invalidSourceCluster, invalidOptions bool
 		expectedClusterCondition                                *metav1.Condition
+		expectedEventMessage                                    string
+		expectedCommandPieces                                   []string
+		missingCommandPieces                                    []string
 	}
 
 	for _, dedicated := range []bool{true, false} {
@@ -1799,6 +1802,8 @@ func TestReconcilePostgresClusterDataSource(t *testing.T) {
 				configCount: 1, jobCount: 1, pvcCount: 1,
 				invalidSourceRepo: false, invalidSourceCluster: false, invalidOptions: false,
 				expectedClusterCondition: nil,
+				expectedCommandPieces:    []string{"--stanza=", "--pg1-path=", "--repo=", "--delta"},
+				missingCommandPieces:     []string{"--target-action"},
 			},
 		}, {
 			desc: "invalid source cluster",
@@ -1812,6 +1817,7 @@ func TestReconcilePostgresClusterDataSource(t *testing.T) {
 				configCount: 0, jobCount: 0, pvcCount: 0,
 				invalidSourceRepo: false, invalidSourceCluster: true, invalidOptions: false,
 				expectedClusterCondition: nil,
+				expectedEventMessage:     "does not exist",
 			},
 		}, {
 			desc: "invalid source repo",
@@ -1825,6 +1831,7 @@ func TestReconcilePostgresClusterDataSource(t *testing.T) {
 				configCount: 1, jobCount: 0, pvcCount: 0,
 				invalidSourceRepo: true, invalidSourceCluster: false, invalidOptions: false,
 				expectedClusterCondition: nil,
+				expectedEventMessage:     "does not have a repo named",
 			},
 		}, {
 			desc: "invalid option: --repo=",
@@ -1839,6 +1846,7 @@ func TestReconcilePostgresClusterDataSource(t *testing.T) {
 				configCount: 1, jobCount: 0, pvcCount: 1,
 				invalidSourceRepo: false, invalidSourceCluster: false, invalidOptions: true,
 				expectedClusterCondition: nil,
+				expectedEventMessage:     "Option '--repo' is not allowed: please use the 'repoName' field instead.",
 			},
 		}, {
 			desc: "invalid option: --repo ",
@@ -1853,6 +1861,7 @@ func TestReconcilePostgresClusterDataSource(t *testing.T) {
 				configCount: 1, jobCount: 0, pvcCount: 1,
 				invalidSourceRepo: false, invalidSourceCluster: false, invalidOptions: true,
 				expectedClusterCondition: nil,
+				expectedEventMessage:     "Option '--repo' is not allowed: please use the 'repoName' field instead.",
 			},
 		}, {
 			desc: "invalid option: stanza",
@@ -1867,6 +1876,7 @@ func TestReconcilePostgresClusterDataSource(t *testing.T) {
 				configCount: 1, jobCount: 0, pvcCount: 1,
 				invalidSourceRepo: false, invalidSourceCluster: false, invalidOptions: true,
 				expectedClusterCondition: nil,
+				expectedEventMessage:     "Option '--stanza' is not allowed: the operator will automatically set this option",
 			},
 		}, {
 			desc: "invalid option: pg1-path",
@@ -1881,6 +1891,68 @@ func TestReconcilePostgresClusterDataSource(t *testing.T) {
 				configCount: 1, jobCount: 0, pvcCount: 1,
 				invalidSourceRepo: false, invalidSourceCluster: false, invalidOptions: true,
 				expectedClusterCondition: nil,
+				expectedEventMessage:     "Option '--pg1-path' is not allowed: the operator will automatically set this option",
+			},
+		}, {
+			desc: "invalid option: target-action",
+			dataSource: &v1beta1.DataSource{PostgresCluster: &v1beta1.PostgresClusterDataSource{
+				ClusterName: "invalid-target-action-option", RepoName: "repo1",
+				Options: []string{"--target-action"},
+			}},
+			clusterBootstrapped: false,
+			sourceClusterName:   "invalid-target-action-option",
+			sourceClusterRepos:  []v1beta1.PGBackRestRepo{{Name: "repo1"}},
+			result: testResult{
+				configCount: 1, jobCount: 0, pvcCount: 1,
+				invalidSourceRepo: false, invalidSourceCluster: false, invalidOptions: true,
+				expectedClusterCondition: nil,
+				expectedEventMessage:     "Option '--target-action' is not allowed: the operator will automatically set this option",
+			},
+		}, {
+			desc: "invalid option: link-map",
+			dataSource: &v1beta1.DataSource{PostgresCluster: &v1beta1.PostgresClusterDataSource{
+				ClusterName: "invalid-link-map-option", RepoName: "repo1",
+				Options: []string{"--link-map"},
+			}},
+			clusterBootstrapped: false,
+			sourceClusterName:   "invalid-link-map-option",
+			sourceClusterRepos:  []v1beta1.PGBackRestRepo{{Name: "repo1"}},
+			result: testResult{
+				configCount: 1, jobCount: 0, pvcCount: 1,
+				invalidSourceRepo: false, invalidSourceCluster: false, invalidOptions: true,
+				expectedClusterCondition: nil,
+				expectedEventMessage:     "Option '--link-map' is not allowed: the operator will automatically set this option",
+			},
+		}, {
+			desc: "valid option: target-timeline",
+			dataSource: &v1beta1.DataSource{PostgresCluster: &v1beta1.PostgresClusterDataSource{
+				ClusterName: "valid-target-timeline-option", RepoName: "repo1",
+				Options: []string{"--target-timeline=1"},
+			}},
+			clusterBootstrapped: false,
+			sourceClusterName:   "valid-target-timeline-option",
+			sourceClusterRepos:  []v1beta1.PGBackRestRepo{{Name: "repo1"}},
+			result: testResult{
+				configCount: 1, jobCount: 1, pvcCount: 1,
+				invalidSourceRepo: false, invalidSourceCluster: false, invalidOptions: false,
+				expectedClusterCondition: nil,
+				expectedCommandPieces:    []string{"--stanza=", "--pg1-path=", "--repo=", "--delta", "--target-timeline=1"},
+				missingCommandPieces:     []string{"--target=", "--target-action=promote"},
+			},
+		}, {
+			desc: "valid option: target",
+			dataSource: &v1beta1.DataSource{PostgresCluster: &v1beta1.PostgresClusterDataSource{
+				ClusterName: "valid-target-option", RepoName: "repo1",
+				Options: []string{"--target=some-date"},
+			}},
+			clusterBootstrapped: false,
+			sourceClusterName:   "valid-target-option",
+			sourceClusterRepos:  []v1beta1.PGBackRestRepo{{Name: "repo1"}},
+			result: testResult{
+				configCount: 1, jobCount: 1, pvcCount: 1,
+				invalidSourceRepo: false, invalidSourceCluster: false, invalidOptions: false,
+				expectedClusterCondition: nil,
+				expectedCommandPieces:    []string{"--stanza=", "--pg1-path=", "--repo=", "--delta", "--target=some-date", "--target-action=promote"},
 			},
 		}, {
 			desc: "cluster bootstrapped init condition missing",
@@ -2003,6 +2075,16 @@ func TestReconcilePostgresClusterDataSource(t *testing.T) {
 				if len(restoreJobs.Items) == 1 {
 					assert.Assert(t, restoreJobs.Items[0].Labels[naming.LabelStartupInstance] != "")
 					assert.Assert(t, restoreJobs.Items[0].Annotations[naming.PGBackRestConfigHash] != "")
+					for _, cmd := range tc.result.expectedCommandPieces {
+						assert.Assert(t, cmp.Contains(
+							strings.Join(restoreJobs.Items[0].Spec.Template.Spec.Containers[0].Command, " "),
+							cmd))
+					}
+					for _, cmd := range tc.result.missingCommandPieces {
+						assert.Assert(t, !strings.Contains(
+							strings.Join(restoreJobs.Items[0].Spec.Template.Spec.Containers[0].Command, " "),
+							cmd))
+					}
 				}
 
 				dataPVCs := &corev1.PersistentVolumeClaimList{}
@@ -2040,7 +2122,11 @@ func TestReconcilePostgresClusterDataSource(t *testing.T) {
 								"involvedObject.namespace": namespace,
 								"reason":                   "InvalidDataSource",
 							})
-							return len(events.Items) == 1, err
+							eventExists := len(events.Items) > 0
+							if eventExists {
+								assert.Assert(t, cmp.Contains(events.Items[0].Message, tc.result.expectedEventMessage))
+							}
+							return eventExists, err
 						}))
 				}
 			})