Skip to content

Commit 8b9604a

Browse files
egeguneshors
egegunes
and
hors
authored
K8SPG-772: Fix panic if backup completedAt is nil (#1149)
* K8SPG-772: Fix panic if backup completedAt is nil * fix golangci-lint --------- Co-authored-by: Viacheslav Sarzhan <slava.sarzhan@percona.com>
1 parent c57cce3 commit 8b9604a

File tree

5 files changed

+377
-6
lines changed

5 files changed

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

0 commit comments

Comments
 (0)