Skip to content

Commit 1bc3e61

Browse files
egegunesegegunes
committed
K8SPG-772: Fix panic if backup completedAt is nil
1 parent 4689dad commit 1bc3e61

File tree

5 files changed

+374
-6
lines changed

5 files changed

+374
-6
lines changed

‎cmd/postgres-operator/main.go

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -194,11 +194,8 @@ func addControllersToManager(ctx context.Context, mgr manager.Manager) error {
194194
if err := mgr.GetFieldIndexer().IndexField(
195195
context.Background(),
196196
&v2.PerconaPGBackup{},
197-
"spec.pgCluster",
198-
func(rawObj client.Object) []string {
199-
backup := rawObj.(*v2.PerconaPGBackup)
200-
return []string{backup.Spec.PGCluster}
201-
},
197+
v2.IndexFieldPGCluster,
198+
v2.PGClusterIndexerFunc,
202199
); err != nil {
203200
return err
204201
}

‎percona/testutils/client.go

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
1+
package testutils
2+
3+
import (
4+
"k8s.io/apimachinery/pkg/runtime"
5+
"sigs.k8s.io/controller-runtime/pkg/client"
6+
"sigs.k8s.io/controller-runtime/pkg/client/fake"
7+
8+
pgv2 "github.com/percona/percona-postgresql-operator/pkg/apis/pgv2.percona.com/v2"
9+
crunchyv1beta1 "github.com/percona/percona-postgresql-operator/pkg/apis/postgres-operator.crunchydata.com/v1beta1"
10+
)
11+
12+
func BuildFakeClient(initObjs ...client.Object) client.Client {
13+
scheme := runtime.NewScheme()
14+
15+
perconaTypes := []runtime.Object{
16+
new(pgv2.PerconaPGCluster),
17+
new(pgv2.PerconaPGClusterList),
18+
new(pgv2.PerconaPGBackup),
19+
new(pgv2.PerconaPGBackupList),
20+
new(pgv2.PerconaPGRestore),
21+
new(pgv2.PerconaPGRestoreList),
22+
new(pgv2.PerconaPGUpgrade),
23+
new(pgv2.PerconaPGUpgradeList),
24+
}
25+
26+
crunchyTypes := []runtime.Object{
27+
new(crunchyv1beta1.PostgresCluster),
28+
new(crunchyv1beta1.PostgresClusterList),
29+
new(crunchyv1beta1.PGUpgrade),
30+
new(crunchyv1beta1.PGUpgradeList),
31+
}
32+
33+
scheme.AddKnownTypes(pgv2.GroupVersion, perconaTypes...)
34+
scheme.AddKnownTypes(crunchyv1beta1.GroupVersion, crunchyTypes...)
35+
36+
return fake.NewClientBuilder().
37+
WithScheme(scheme).
38+
WithObjects(initObjs...).
39+
WithIndex(new(pgv2.PerconaPGBackup), pgv2.IndexFieldPGCluster, pgv2.PGClusterIndexerFunc).
40+
Build()
41+
}

‎percona/watcher/wal.go

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -131,9 +131,19 @@ func getLatestBackup(ctx context.Context, cli client.Client, cr *pgv2.PerconaPGC
131131
runningBackupExists := false
132132
for _, backup := range backupList.Items {
133133
backup := backup
134+
134135
switch backup.Status.State {
135136
case pgv2.BackupSucceeded:
136-
if latest.Status.CompletedAt == nil || backup.Status.CompletedAt.After(latest.Status.CompletedAt.Time) {
137+
var completedAt *metav1.Time
138+
139+
if backup.Status.CompletedAt != nil {
140+
completedAt = backup.Status.CompletedAt
141+
}
142+
if completedAt == nil {
143+
completedAt = &backup.CreationTimestamp
144+
}
145+
146+
if latest.Status.CompletedAt == nil || completedAt.After(latest.Status.CompletedAt.Time) {
137147
latest = &backup
138148
}
139149
case pgv2.BackupFailed:

‎percona/watcher/wal_test.go

Lines changed: 309 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,309 @@
1+
package watcher
2+
3+
import (
4+
"testing"
5+
"time"
6+
7+
"github.com/stretchr/testify/assert"
8+
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
9+
"sigs.k8s.io/controller-runtime/pkg/client"
10+
11+
"github.com/percona/percona-postgresql-operator/percona/testutils"
12+
pgv2 "github.com/percona/percona-postgresql-operator/pkg/apis/pgv2.percona.com/v2"
13+
)
14+
15+
func mustParseTime(layout string, value string) time.Time {
16+
time, err := time.Parse(layout, value)
17+
if err != nil {
18+
panic(err)
19+
}
20+
return time
21+
}
22+
23+
func TestGetLatestBackup(t *testing.T) {
24+
tests := []struct {
25+
name string
26+
backups []client.Object
27+
latestBackupName string
28+
expectedErr error
29+
}{
30+
{
31+
name: "no backups",
32+
backups: []client.Object{},
33+
latestBackupName: "",
34+
expectedErr: errNoBackups,
35+
},
36+
{
37+
name: "single backup",
38+
backups: []client.Object{
39+
&pgv2.PerconaPGBackup{
40+
ObjectMeta: metav1.ObjectMeta{
41+
Name: "backup1",
42+
Namespace: "test-ns",
43+
CreationTimestamp: metav1.Time{
44+
Time: mustParseTime(time.RFC3339, "2024-02-04T21:00:57Z"),
45+
},
46+
},
47+
Spec: pgv2.PerconaPGBackupSpec{
48+
PGCluster: "test-cluster",
49+
},
50+
Status: pgv2.PerconaPGBackupStatus{
51+
State: pgv2.BackupSucceeded,
52+
CompletedAt: &metav1.Time{
53+
Time: mustParseTime(time.RFC3339, "2024-02-04T21:23:38Z"),
54+
},
55+
},
56+
},
57+
},
58+
latestBackupName: "backup1",
59+
expectedErr: nil,
60+
},
61+
{
62+
name: "multiple backups, same cluster",
63+
backups: []client.Object{
64+
&pgv2.PerconaPGBackup{
65+
ObjectMeta: metav1.ObjectMeta{
66+
Name: "backup1",
67+
Namespace: "test-ns",
68+
CreationTimestamp: metav1.Time{
69+
Time: mustParseTime(time.RFC3339, "2024-02-04T21:00:57Z"),
70+
},
71+
},
72+
Spec: pgv2.PerconaPGBackupSpec{
73+
PGCluster: "test-cluster",
74+
},
75+
Status: pgv2.PerconaPGBackupStatus{
76+
State: pgv2.BackupSucceeded,
77+
CompletedAt: &metav1.Time{
78+
Time: mustParseTime(time.RFC3339, "2024-02-04T21:23:38Z"),
79+
},
80+
},
81+
},
82+
&pgv2.PerconaPGBackup{
83+
ObjectMeta: metav1.ObjectMeta{
84+
Name: "backup2",
85+
Namespace: "test-ns",
86+
CreationTimestamp: metav1.Time{
87+
Time: mustParseTime(time.RFC3339, "2024-02-04T22:00:57Z"),
88+
},
89+
},
90+
Spec: pgv2.PerconaPGBackupSpec{
91+
PGCluster: "test-cluster",
92+
},
93+
Status: pgv2.PerconaPGBackupStatus{
94+
State: pgv2.BackupSucceeded,
95+
CompletedAt: &metav1.Time{
96+
Time: mustParseTime(time.RFC3339, "2024-02-04T22:24:12Z"),
97+
},
98+
},
99+
},
100+
},
101+
latestBackupName: "backup2",
102+
expectedErr: nil,
103+
},
104+
{
105+
name: "multiple backups, different clusters",
106+
backups: []client.Object{
107+
&pgv2.PerconaPGBackup{
108+
ObjectMeta: metav1.ObjectMeta{
109+
Name: "backup1",
110+
Namespace: "test-ns",
111+
CreationTimestamp: metav1.Time{
112+
Time: mustParseTime(time.RFC3339, "2024-02-04T21:00:57Z"),
113+
},
114+
},
115+
Spec: pgv2.PerconaPGBackupSpec{
116+
PGCluster: "test-cluster",
117+
},
118+
Status: pgv2.PerconaPGBackupStatus{
119+
State: pgv2.BackupSucceeded,
120+
CompletedAt: &metav1.Time{
121+
Time: mustParseTime(time.RFC3339, "2024-02-04T21:23:38Z"),
122+
},
123+
},
124+
},
125+
&pgv2.PerconaPGBackup{
126+
ObjectMeta: metav1.ObjectMeta{
127+
Name: "backup2",
128+
Namespace: "test-ns",
129+
CreationTimestamp: metav1.Time{
130+
Time: mustParseTime(time.RFC3339, "2024-02-04T22:00:57Z"),
131+
},
132+
},
133+
Spec: pgv2.PerconaPGBackupSpec{
134+
PGCluster: "test-cluster",
135+
},
136+
Status: pgv2.PerconaPGBackupStatus{
137+
State: pgv2.BackupSucceeded,
138+
CompletedAt: &metav1.Time{
139+
Time: mustParseTime(time.RFC3339, "2024-02-04T22:24:12Z"),
140+
},
141+
},
142+
},
143+
&pgv2.PerconaPGBackup{
144+
ObjectMeta: metav1.ObjectMeta{
145+
Name: "backup-from-different-cluster",
146+
Namespace: "test-ns",
147+
CreationTimestamp: metav1.Time{
148+
Time: mustParseTime(time.RFC3339, "2024-02-04T22:10:57Z"),
149+
},
150+
},
151+
Spec: pgv2.PerconaPGBackupSpec{
152+
PGCluster: "different-cluster",
153+
},
154+
Status: pgv2.PerconaPGBackupStatus{
155+
State: pgv2.BackupSucceeded,
156+
CompletedAt: &metav1.Time{
157+
Time: mustParseTime(time.RFC3339, "2024-02-04T22:14:12Z"),
158+
},
159+
},
160+
},
161+
},
162+
latestBackupName: "backup2",
163+
expectedErr: nil,
164+
},
165+
{
166+
name: "single running backup",
167+
backups: []client.Object{
168+
&pgv2.PerconaPGBackup{
169+
ObjectMeta: metav1.ObjectMeta{
170+
Name: "backup1",
171+
Namespace: "test-ns",
172+
CreationTimestamp: metav1.Time{
173+
Time: mustParseTime(time.RFC3339, "2024-02-04T21:00:57Z"),
174+
},
175+
},
176+
Spec: pgv2.PerconaPGBackupSpec{
177+
PGCluster: "test-cluster",
178+
},
179+
Status: pgv2.PerconaPGBackupStatus{
180+
State: pgv2.BackupRunning,
181+
},
182+
},
183+
},
184+
latestBackupName: "",
185+
expectedErr: errRunningBackup,
186+
},
187+
{
188+
name: "running backup but a backup is already succeeded",
189+
backups: []client.Object{
190+
&pgv2.PerconaPGBackup{
191+
ObjectMeta: metav1.ObjectMeta{
192+
Name: "backup1",
193+
Namespace: "test-ns",
194+
CreationTimestamp: metav1.Time{
195+
Time: mustParseTime(time.RFC3339, "2024-02-04T21:00:57Z"),
196+
},
197+
},
198+
Spec: pgv2.PerconaPGBackupSpec{
199+
PGCluster: "test-cluster",
200+
},
201+
Status: pgv2.PerconaPGBackupStatus{
202+
State: pgv2.BackupSucceeded,
203+
CompletedAt: &metav1.Time{
204+
Time: mustParseTime(time.RFC3339, "2024-02-04T21:23:38Z"),
205+
},
206+
},
207+
},
208+
&pgv2.PerconaPGBackup{
209+
ObjectMeta: metav1.ObjectMeta{
210+
Name: "backup2",
211+
Namespace: "test-ns",
212+
CreationTimestamp: metav1.Time{
213+
Time: mustParseTime(time.RFC3339, "2024-02-04T22:00:57Z"),
214+
},
215+
},
216+
Spec: pgv2.PerconaPGBackupSpec{
217+
PGCluster: "test-cluster",
218+
},
219+
Status: pgv2.PerconaPGBackupStatus{
220+
State: pgv2.BackupRunning,
221+
},
222+
},
223+
},
224+
latestBackupName: "backup1",
225+
expectedErr: nil,
226+
},
227+
{
228+
name: "K8SPG-772: multiple backups, some has no CompletedAt",
229+
backups: []client.Object{
230+
&pgv2.PerconaPGBackup{
231+
ObjectMeta: metav1.ObjectMeta{
232+
Name: "backup1",
233+
Namespace: "test-ns",
234+
CreationTimestamp: metav1.Time{
235+
Time: mustParseTime(time.RFC3339, "2024-02-04T21:00:57Z"),
236+
},
237+
},
238+
Spec: pgv2.PerconaPGBackupSpec{
239+
PGCluster: "test-cluster",
240+
},
241+
Status: pgv2.PerconaPGBackupStatus{
242+
State: pgv2.BackupSucceeded,
243+
CompletedAt: &metav1.Time{
244+
Time: mustParseTime(time.RFC3339, "2024-02-04T21:24:12Z"),
245+
},
246+
},
247+
},
248+
&pgv2.PerconaPGBackup{
249+
ObjectMeta: metav1.ObjectMeta{
250+
Name: "backup2",
251+
Namespace: "test-ns",
252+
CreationTimestamp: metav1.Time{
253+
Time: mustParseTime(time.RFC3339, "2024-02-04T22:00:57Z"),
254+
},
255+
},
256+
Spec: pgv2.PerconaPGBackupSpec{
257+
PGCluster: "test-cluster",
258+
},
259+
Status: pgv2.PerconaPGBackupStatus{
260+
State: pgv2.BackupSucceeded,
261+
},
262+
},
263+
&pgv2.PerconaPGBackup{
264+
ObjectMeta: metav1.ObjectMeta{
265+
Name: "backup3",
266+
Namespace: "test-ns",
267+
CreationTimestamp: metav1.Time{
268+
Time: mustParseTime(time.RFC3339, "2024-02-04T23:00:57Z"),
269+
},
270+
},
271+
Spec: pgv2.PerconaPGBackupSpec{
272+
PGCluster: "test-cluster",
273+
},
274+
Status: pgv2.PerconaPGBackupStatus{
275+
State: pgv2.BackupSucceeded,
276+
CompletedAt: &metav1.Time{
277+
Time: mustParseTime(time.RFC3339, "2024-02-04T23:24:12Z"),
278+
},
279+
},
280+
},
281+
},
282+
latestBackupName: "backup3",
283+
expectedErr: nil,
284+
},
285+
}
286+
287+
for _, tt := range tests {
288+
t.Run(tt.name, func(t *testing.T) {
289+
client := testutils.BuildFakeClient(tt.backups...)
290+
291+
cluster := &pgv2.PerconaPGCluster{
292+
ObjectMeta: metav1.ObjectMeta{
293+
Name: "test-cluster",
294+
Namespace: "test-ns",
295+
},
296+
}
297+
298+
latest, err := getLatestBackup(t.Context(), client, cluster)
299+
if tt.expectedErr != nil {
300+
assert.EqualError(t, err, tt.expectedErr.Error())
301+
assert.Nil(t, latest)
302+
} else {
303+
assert.NoError(t, err)
304+
assert.NotNil(t, latest)
305+
assert.Equal(t, latest.Name, tt.latestBackupName)
306+
}
307+
})
308+
}
309+
}

0 commit comments

Comments
 (0)