Skip to content

Commit c62817b

Browse files
egeguneshorsgkech
authored
K8SPG-844: Don't modify archive_command if trackLatestRestorableTime is disabled (#1290)
* K8SPG-844: Don't modify archive_command if trackLatestRestorableTime is disabled * fix * compare version for updating the archive command * param ordering fix * remove unused params * fixes * fix linter --------- Co-authored-by: Viacheslav Sarzhan <[email protected]> Co-authored-by: George Kechagias <[email protected]>
1 parent 32cc445 commit c62817b

File tree

11 files changed

+234
-81
lines changed

11 files changed

+234
-81
lines changed

‎build/crd/crunchy/generated/postgres-operator.crunchydata.com_postgresclusters.yaml‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6938,6 +6938,9 @@ spec:
69386938
required:
69396939
- volumeSnapshotClassName
69406940
type: object
6941+
trackLatestRestorableTime:
6942+
description: Enable tracking latest restorable time
6943+
type: boolean
69416944
type: object
69426945
config:
69436946
properties:

‎config/crd/bases/postgres-operator.crunchydata.com_postgresclusters.yaml‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6928,6 +6928,9 @@ spec:
69286928
required:
69296929
- volumeSnapshotClassName
69306930
type: object
6931+
trackLatestRestorableTime:
6932+
description: Enable tracking latest restorable time
6933+
type: boolean
69316934
type: object
69326935
config:
69336936
properties:

‎deploy/bundle.yaml‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36721,6 +36721,9 @@ spec:
3672136721
required:
3672236722
- volumeSnapshotClassName
3672336723
type: object
36724+
trackLatestRestorableTime:
36725+
description: Enable tracking latest restorable time
36726+
type: boolean
3672436727
type: object
3672536728
config:
3672636729
properties:

‎deploy/crd.yaml‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36721,6 +36721,9 @@ spec:
3672136721
required:
3672236722
- volumeSnapshotClassName
3672336723
type: object
36724+
trackLatestRestorableTime:
36725+
description: Enable tracking latest restorable time
36726+
type: boolean
3672436727
type: object
3672536728
config:
3672636729
properties:

‎deploy/cw-bundle.yaml‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36721,6 +36721,9 @@ spec:
3672136721
required:
3672236722
- volumeSnapshotClassName
3672336723
type: object
36724+
trackLatestRestorableTime:
36725+
description: Enable tracking latest restorable time
36726+
type: boolean
3672436727
type: object
3672536728
config:
3672636729
properties:

‎internal/pgbackrest/postgres.go‎

Lines changed: 28 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -28,13 +28,20 @@ func PostgreSQL(
2828
// - https://pgbackrest.org/user-guide.html#quickstart/configure-archiving
2929
// - https://pgbackrest.org/command.html#command-archive-push
3030
// - https://www.postgresql.org/docs/current/runtime-config-wal.html
31-
32-
fixTimezone := `sed -E "s/([0-9]{4}-[0-9]{2}-[0-9]{2}) ([0-9]{2}:[0-9]{2}:[0-9]{2}\.[0-9]{6}) (UTC|[\\+\\-][0-9]{2})/\1T\2\3/" | sed "s/UTC/Z/"`
33-
extractCommitTime := `grep -oP "COMMIT \K[^;]+" | ` + fixTimezone + ``
34-
validateCommitTime := `grep -E "^[0-9]{4}-[0-9]{2}-[0-9]{2}T[0-9]{2}:[0-9]{2}:[0-9]{2}\.[0-9]{6}(Z|[\+\-][0-9]{2})$"`
3531
archive := `pgbackrest --stanza=` + DefaultStanzaName + ` archive-push "%p"`
36-
archive += ` && timestamp=$(pg_waldump "%p" | ` + extractCommitTime + ` | tail -n 1 | ` + validateCommitTime + `);`
37-
archive += ` if [ ! -z ${timestamp} ]; then echo ${timestamp} > /pgdata/latest_commit_timestamp.txt; fi`
32+
33+
// K8SPG-518
34+
if inCluster.CompareVersion("2.8.0") >= 0 {
35+
if trackRestorableTime := inCluster.Spec.Backups.TrackLatestRestorableTime; trackRestorableTime != nil && *trackRestorableTime {
36+
updateCommandRestorableTime(&archive)
37+
// K8SPG-518: This parameter is required to ensure that the commit timestamp is
38+
// included in the WAL file. This is necessary for the WAL watcher to
39+
// function correctly.
40+
outParameters.Mandatory.Add("track_commit_timestamp", "true")
41+
}
42+
} else {
43+
updateCommandRestorableTime(&archive)
44+
}
3845

3946
outParameters.Mandatory.Add("archive_mode", "on")
4047

@@ -46,10 +53,12 @@ func PostgreSQL(
4653
outParameters.Mandatory.Add("archive_command", `true`)
4754
}
4855

49-
// K8SPG-518: This parameter is required to ensure that the commit timestamp is
50-
// included in the WAL file. This is necessary for the WAL watcher to
51-
// function correctly.
52-
outParameters.Mandatory.Add("track_commit_timestamp", "true")
56+
if inCluster.CompareVersion("2.8.0") < 0 {
57+
// K8SPG-518: This parameter is required to ensure that the commit timestamp is
58+
// included in the WAL file. This is necessary for the WAL watcher to
59+
// function correctly.
60+
outParameters.Mandatory.Add("track_commit_timestamp", "true")
61+
}
5362

5463
// archive_timeout is used to determine at what point a WAL file is switched,
5564
// if the WAL archive has not reached its full size in # of transactions
@@ -94,3 +103,12 @@ func PostgreSQL(
94103
outParameters.Mandatory.Add("restore_command", restore)
95104
}
96105
}
106+
107+
func updateCommandRestorableTime(archive *string) {
108+
fixTimezone := `sed -E "s/([0-9]{4}-[0-9]{2}-[0-9]{2}) ([0-9]{2}:[0-9]{2}:[0-9]{2}\.[0-9]{6}) (UTC|[\\+\\-][0-9]{2})/\1T\2\3/" | sed "s/UTC/Z/"`
109+
extractCommitTime := `grep -oP "COMMIT \K[^;]+" | ` + fixTimezone + ``
110+
validateCommitTime := `grep -E "^[0-9]{4}-[0-9]{2}-[0-9]{2}T[0-9]{2}:[0-9]{2}:[0-9]{2}\.[0-9]{6}(Z|[\+\-][0-9]{2})$"`
111+
112+
*archive += ` && timestamp=$(pg_waldump "%p" | ` + extractCommitTime + ` | tail -n 1 | ` + validateCommitTime + `);`
113+
*archive += ` if [ ! -z ${timestamp} ]; then echo ${timestamp} > /pgdata/latest_commit_timestamp.txt; fi`
114+
}

‎internal/pgbackrest/postgres_test.go‎

Lines changed: 173 additions & 69 deletions
Original file line numberDiff line numberDiff line change
@@ -9,85 +9,189 @@ import (
99
"testing"
1010

1111
"gotest.tools/v3/assert"
12+
"k8s.io/utils/ptr"
1213

1314
"github.com/percona/percona-postgresql-operator/v2/internal/postgres"
15+
"github.com/percona/percona-postgresql-operator/v2/percona/version"
1416
"github.com/percona/percona-postgresql-operator/v2/pkg/apis/postgres-operator.crunchydata.com/v1beta1"
1517
)
1618

1719
func TestPostgreSQLParameters(t *testing.T) {
18-
cluster := new(v1beta1.PostgresCluster)
19-
parameters := new(postgres.Parameters)
20-
21-
PostgreSQL(cluster, parameters, true)
22-
assert.DeepEqual(t, parameters.Mandatory.AsMap(), map[string]string{
23-
"archive_mode": "on",
24-
"archive_command": strings.Join([]string{
25-
`pgbackrest --stanza=db archive-push "%p" `,
26-
`&& timestamp=$(pg_waldump "%p" | `,
27-
`grep -oP "COMMIT \K[^;]+" | `,
28-
`sed -E "s/([0-9]{4}-[0-9]{2}-[0-9]{2}) ([0-9]{2}:[0-9]{2}:[0-9]{2}\.[0-9]{6}) (UTC|[\\+\\-][0-9]{2})/\1T\2\3/" | `,
29-
`sed "s/UTC/Z/" | `,
30-
"tail -n 1 | ",
31-
`grep -E "^[0-9]{4}-[0-9]{2}-[0-9]{2}T[0-9]{2}:[0-9]{2}:[0-9]{2}\.[0-9]{6}(Z|[\+\-][0-9]{2})$"); `,
32-
"if [ ! -z ${timestamp} ]; then echo ${timestamp} > /pgdata/latest_commit_timestamp.txt; fi",
33-
}, ""),
34-
"restore_command": `pgbackrest --stanza=db archive-get %f "%p"`,
35-
"track_commit_timestamp": "true",
36-
})
20+
t.Run("latest CR version", func(t *testing.T) {
21+
cluster := new(v1beta1.PostgresCluster)
22+
parameters := new(postgres.Parameters)
3723

38-
assert.DeepEqual(t, parameters.Default.AsMap(), map[string]string{
39-
"archive_timeout": "60s",
40-
})
24+
if cluster.Labels == nil {
25+
cluster.Labels = make(map[string]string)
26+
}
27+
cluster.Labels["pgv2.percona.com/version"] = version.Version()
28+
29+
PostgreSQL(cluster, parameters, true)
30+
assert.DeepEqual(t, parameters.Mandatory.AsMap(), map[string]string{
31+
"archive_mode": "on",
32+
"archive_command": `pgbackrest --stanza=db archive-push "%p"`,
33+
"restore_command": `pgbackrest --stanza=db archive-get %f "%p"`,
34+
})
4135

42-
dynamic := map[string]any{
43-
"postgresql": map[string]any{
44-
"parameters": map[string]any{
45-
"restore_command": "/bin/true",
36+
assert.DeepEqual(t, parameters.Default.AsMap(), map[string]string{
37+
"archive_timeout": "60s",
38+
})
39+
40+
dynamic := map[string]any{
41+
"postgresql": map[string]any{
42+
"parameters": map[string]any{
43+
"restore_command": "/bin/true",
44+
},
4645
},
47-
},
48-
}
49-
if cluster.Spec.Patroni == nil {
50-
cluster.Spec.Patroni = &v1beta1.PatroniSpec{}
51-
}
52-
cluster.Spec.Patroni.DynamicConfiguration = dynamic
53-
54-
PostgreSQL(cluster, parameters, true)
55-
assert.DeepEqual(t, parameters.Mandatory.AsMap(), map[string]string{
56-
"archive_mode": "on",
57-
"archive_command": strings.Join([]string{
58-
`pgbackrest --stanza=db archive-push "%p" `,
59-
`&& timestamp=$(pg_waldump "%p" | `,
60-
`grep -oP "COMMIT \K[^;]+" | `,
61-
`sed -E "s/([0-9]{4}-[0-9]{2}-[0-9]{2}) ([0-9]{2}:[0-9]{2}:[0-9]{2}\.[0-9]{6}) (UTC|[\\+\\-][0-9]{2})/\1T\2\3/" | `,
62-
`sed "s/UTC/Z/" | `,
63-
"tail -n 1 | ",
64-
`grep -E "^[0-9]{4}-[0-9]{2}-[0-9]{2}T[0-9]{2}:[0-9]{2}:[0-9]{2}\.[0-9]{6}(Z|[\+\-][0-9]{2})$"); `,
65-
"if [ ! -z ${timestamp} ]; then echo ${timestamp} > /pgdata/latest_commit_timestamp.txt; fi",
66-
}, ""),
67-
"restore_command": "/bin/true",
68-
"track_commit_timestamp": "true",
46+
}
47+
if cluster.Spec.Patroni == nil {
48+
cluster.Spec.Patroni = &v1beta1.PatroniSpec{}
49+
}
50+
cluster.Spec.Patroni.DynamicConfiguration = dynamic
51+
52+
PostgreSQL(cluster, parameters, true)
53+
assert.DeepEqual(t, parameters.Mandatory.AsMap(), map[string]string{
54+
"archive_mode": "on",
55+
"archive_command": `pgbackrest --stanza=db archive-push "%p"`,
56+
"restore_command": "/bin/true",
57+
})
58+
59+
cluster.Spec.Standby = &v1beta1.PostgresStandbySpec{
60+
Enabled: true,
61+
RepoName: "repo99",
62+
}
63+
cluster.Spec.Patroni.DynamicConfiguration = nil
64+
65+
PostgreSQL(cluster, parameters, true)
66+
assert.DeepEqual(t, parameters.Mandatory.AsMap(), map[string]string{
67+
"archive_mode": "on",
68+
"archive_command": `pgbackrest --stanza=db archive-push "%p"`,
69+
"restore_command": `pgbackrest --stanza=db archive-get %f "%p" --repo=99`,
70+
})
71+
72+
cluster.Spec.Standby = nil
73+
cluster.Spec.Patroni.DynamicConfiguration = nil
74+
cluster.Spec.Backups.TrackLatestRestorableTime = ptr.To(true)
75+
76+
PostgreSQL(cluster, parameters, true)
77+
assert.DeepEqual(t, parameters.Mandatory.AsMap(), map[string]string{
78+
"archive_mode": "on",
79+
"archive_command": strings.Join([]string{
80+
`pgbackrest --stanza=db archive-push "%p" `,
81+
`&& timestamp=$(pg_waldump "%p" | `,
82+
`grep -oP "COMMIT \K[^;]+" | `,
83+
`sed -E "s/([0-9]{4}-[0-9]{2}-[0-9]{2}) ([0-9]{2}:[0-9]{2}:[0-9]{2}\.[0-9]{6}) (UTC|[\\+\\-][0-9]{2})/\1T\2\3/" | `,
84+
`sed "s/UTC/Z/" | `,
85+
"tail -n 1 | ",
86+
`grep -E "^[0-9]{4}-[0-9]{2}-[0-9]{2}T[0-9]{2}:[0-9]{2}:[0-9]{2}\.[0-9]{6}(Z|[\+\-][0-9]{2})$"); `,
87+
"if [ ! -z ${timestamp} ]; then echo ${timestamp} > /pgdata/latest_commit_timestamp.txt; fi",
88+
}, ""),
89+
"restore_command": `pgbackrest --stanza=db archive-get %f "%p"`,
90+
"track_commit_timestamp": "true",
91+
})
6992
})
7093

71-
cluster.Spec.Standby = &v1beta1.PostgresStandbySpec{
72-
Enabled: true,
73-
RepoName: "repo99",
74-
}
75-
cluster.Spec.Patroni.DynamicConfiguration = nil
76-
77-
PostgreSQL(cluster, parameters, true)
78-
assert.DeepEqual(t, parameters.Mandatory.AsMap(), map[string]string{
79-
"archive_mode": "on",
80-
"archive_command": strings.Join([]string{
81-
`pgbackrest --stanza=db archive-push "%p" `,
82-
`&& timestamp=$(pg_waldump "%p" | `,
83-
`grep -oP "COMMIT \K[^;]+" | `,
84-
`sed -E "s/([0-9]{4}-[0-9]{2}-[0-9]{2}) ([0-9]{2}:[0-9]{2}:[0-9]{2}\.[0-9]{6}) (UTC|[\\+\\-][0-9]{2})/\1T\2\3/" | `,
85-
`sed "s/UTC/Z/" | `,
86-
"tail -n 1 | ",
87-
`grep -E "^[0-9]{4}-[0-9]{2}-[0-9]{2}T[0-9]{2}:[0-9]{2}:[0-9]{2}\.[0-9]{6}(Z|[\+\-][0-9]{2})$"); `,
88-
"if [ ! -z ${timestamp} ]; then echo ${timestamp} > /pgdata/latest_commit_timestamp.txt; fi",
89-
}, ""),
90-
"restore_command": `pgbackrest --stanza=db archive-get %f "%p" --repo=99`,
91-
"track_commit_timestamp": "true",
94+
t.Run("2.8.0< version", func(t *testing.T) {
95+
cluster := new(v1beta1.PostgresCluster)
96+
parameters := new(postgres.Parameters)
97+
98+
if cluster.Labels == nil {
99+
cluster.Labels = make(map[string]string)
100+
}
101+
cluster.Labels["pgv2.percona.com/version"] = "2.7.0"
102+
103+
PostgreSQL(cluster, parameters, true)
104+
assert.DeepEqual(t, parameters.Mandatory.AsMap(), map[string]string{
105+
"archive_mode": "on",
106+
"archive_command": strings.Join([]string{
107+
`pgbackrest --stanza=db archive-push "%p" `,
108+
`&& timestamp=$(pg_waldump "%p" | `,
109+
`grep -oP "COMMIT \K[^;]+" | `,
110+
`sed -E "s/([0-9]{4}-[0-9]{2}-[0-9]{2}) ([0-9]{2}:[0-9]{2}:[0-9]{2}\.[0-9]{6}) (UTC|[\\+\\-][0-9]{2})/\1T\2\3/" | `,
111+
`sed "s/UTC/Z/" | `,
112+
"tail -n 1 | ",
113+
`grep -E "^[0-9]{4}-[0-9]{2}-[0-9]{2}T[0-9]{2}:[0-9]{2}:[0-9]{2}\.[0-9]{6}(Z|[\+\-][0-9]{2})$"); `,
114+
"if [ ! -z ${timestamp} ]; then echo ${timestamp} > /pgdata/latest_commit_timestamp.txt; fi",
115+
}, ""),
116+
"restore_command": `pgbackrest --stanza=db archive-get %f "%p"`,
117+
"track_commit_timestamp": "true",
118+
})
119+
120+
assert.DeepEqual(t, parameters.Default.AsMap(), map[string]string{
121+
"archive_timeout": "60s",
122+
})
123+
124+
dynamic := map[string]any{
125+
"postgresql": map[string]any{
126+
"parameters": map[string]any{
127+
"restore_command": "/bin/true",
128+
},
129+
},
130+
}
131+
if cluster.Spec.Patroni == nil {
132+
cluster.Spec.Patroni = &v1beta1.PatroniSpec{}
133+
}
134+
cluster.Spec.Patroni.DynamicConfiguration = dynamic
135+
136+
PostgreSQL(cluster, parameters, true)
137+
assert.DeepEqual(t, parameters.Mandatory.AsMap(), map[string]string{
138+
"archive_mode": "on",
139+
"archive_command": strings.Join([]string{
140+
`pgbackrest --stanza=db archive-push "%p" `,
141+
`&& timestamp=$(pg_waldump "%p" | `,
142+
`grep -oP "COMMIT \K[^;]+" | `,
143+
`sed -E "s/([0-9]{4}-[0-9]{2}-[0-9]{2}) ([0-9]{2}:[0-9]{2}:[0-9]{2}\.[0-9]{6}) (UTC|[\\+\\-][0-9]{2})/\1T\2\3/" | `,
144+
`sed "s/UTC/Z/" | `,
145+
"tail -n 1 | ",
146+
`grep -E "^[0-9]{4}-[0-9]{2}-[0-9]{2}T[0-9]{2}:[0-9]{2}:[0-9]{2}\.[0-9]{6}(Z|[\+\-][0-9]{2})$"); `,
147+
"if [ ! -z ${timestamp} ]; then echo ${timestamp} > /pgdata/latest_commit_timestamp.txt; fi",
148+
}, ""),
149+
"restore_command": "/bin/true",
150+
"track_commit_timestamp": "true",
151+
})
152+
153+
cluster.Spec.Standby = &v1beta1.PostgresStandbySpec{
154+
Enabled: true,
155+
RepoName: "repo99",
156+
}
157+
cluster.Spec.Patroni.DynamicConfiguration = nil
158+
159+
PostgreSQL(cluster, parameters, true)
160+
assert.DeepEqual(t, parameters.Mandatory.AsMap(), map[string]string{
161+
"archive_mode": "on",
162+
"archive_command": strings.Join([]string{
163+
`pgbackrest --stanza=db archive-push "%p" `,
164+
`&& timestamp=$(pg_waldump "%p" | `,
165+
`grep -oP "COMMIT \K[^;]+" | `,
166+
`sed -E "s/([0-9]{4}-[0-9]{2}-[0-9]{2}) ([0-9]{2}:[0-9]{2}:[0-9]{2}\.[0-9]{6}) (UTC|[\\+\\-][0-9]{2})/\1T\2\3/" | `,
167+
`sed "s/UTC/Z/" | `,
168+
"tail -n 1 | ",
169+
`grep -E "^[0-9]{4}-[0-9]{2}-[0-9]{2}T[0-9]{2}:[0-9]{2}:[0-9]{2}\.[0-9]{6}(Z|[\+\-][0-9]{2})$"); `,
170+
"if [ ! -z ${timestamp} ]; then echo ${timestamp} > /pgdata/latest_commit_timestamp.txt; fi",
171+
}, ""),
172+
"restore_command": `pgbackrest --stanza=db archive-get %f "%p" --repo=99`,
173+
"track_commit_timestamp": "true",
174+
})
175+
176+
cluster.Spec.Standby = nil
177+
cluster.Spec.Patroni.DynamicConfiguration = nil
178+
cluster.Spec.Backups.TrackLatestRestorableTime = ptr.To(true)
179+
180+
PostgreSQL(cluster, parameters, true)
181+
assert.DeepEqual(t, parameters.Mandatory.AsMap(), map[string]string{
182+
"archive_mode": "on",
183+
"archive_command": strings.Join([]string{
184+
`pgbackrest --stanza=db archive-push "%p" `,
185+
`&& timestamp=$(pg_waldump "%p" | `,
186+
`grep -oP "COMMIT \K[^;]+" | `,
187+
`sed -E "s/([0-9]{4}-[0-9]{2}-[0-9]{2}) ([0-9]{2}:[0-9]{2}:[0-9]{2}\.[0-9]{6}) (UTC|[\\+\\-][0-9]{2})/\1T\2\3/" | `,
188+
`sed "s/UTC/Z/" | `,
189+
"tail -n 1 | ",
190+
`grep -E "^[0-9]{4}-[0-9]{2}-[0-9]{2}T[0-9]{2}:[0-9]{2}:[0-9]{2}\.[0-9]{6}(Z|[\+\-][0-9]{2})$"); `,
191+
"if [ ! -z ${timestamp} ]; then echo ${timestamp} > /pgdata/latest_commit_timestamp.txt; fi",
192+
}, ""),
193+
"restore_command": `pgbackrest --stanza=db archive-get %f "%p"`,
194+
"track_commit_timestamp": "true",
195+
})
92196
})
93197
}

‎pkg/apis/pgv2.percona.com/v2/perconapgcluster_types.go‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -505,7 +505,7 @@ func (b Backups) ToCrunchy(version string) crunchyv1beta1.Backups {
505505
sc = b.PGBackRest.Sidecars
506506
}
507507

508-
return crunchyv1beta1.Backups{
508+
backups := crunchyv1beta1.Backups{
509509
PGBackRest: crunchyv1beta1.PGBackRestArchive{
510510
Metadata: b.PGBackRest.Metadata,
511511
Configuration: b.PGBackRest.Configuration,
@@ -522,6 +522,12 @@ func (b Backups) ToCrunchy(version string) crunchyv1beta1.Backups {
522522
EnvFrom: b.PGBackRest.EnvFrom,
523523
},
524524
}
525+
526+
if currVersion != nil && currVersion.GreaterThanOrEqual(gover.Must(gover.NewVersion("2.8.0"))) {
527+
backups.TrackLatestRestorableTime = b.TrackLatestRestorableTime
528+
}
529+
530+
return backups
525531
}
526532

527533
type PGBackRestArchive struct {

0 commit comments

Comments
 (0)