Skip to content

Commit d3f30fd

Browse files
nmarukovicheleo007mayankshah1607
authored
K8SPG-740 fix cleanupOutdatedBackups logs transient ERROR messages during pod restarts and image updates (#1514)
* K8SPG-740 fix --------- Co-authored-by: Eleonora Zinchenko <eleonora.zinchenko@percona.com> Co-authored-by: Mayank Shah <mayankshah1614@gmail.com>
1 parent 5fabdd8 commit d3f30fd

File tree

4 files changed

+198
-3
lines changed

4 files changed

+198
-3
lines changed

‎percona/controller/pgcluster/backup.go‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -79,6 +79,10 @@ func (r *PGClusterReconciler) cleanupOutdatedBackups(ctx context.Context, cr *v2
7979
log.Info("There are no info about backups in the pgbackrest", "repo", repo.Name)
8080
continue
8181
}
82+
if errors.Is(err, pgbackrest.ErrStanzaNotCreated) {
83+
log.Info("pgBackRest stanza not yet created, skipping backup cleanup", "repo", repo.Name)
84+
continue
85+
}
8286
return errors.Wrap(err, "get pgBackRest info")
8387
}
8488

‎percona/controller/utils.go‎

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ package controller
22

33
import (
44
"context"
5+
"slices"
56

67
"github.com/pkg/errors"
78
batchv1 "k8s.io/api/batch/v1"
@@ -71,11 +72,16 @@ func GetReadyInstancePod(ctx context.Context, c client.Client, clusterName, name
7172
if err := c.List(ctx, pods, client.InNamespace(namespace), client.MatchingLabelsSelector{Selector: selector}); err != nil {
7273
return nil, errors.Wrap(err, "list pods")
7374
}
74-
for _, pod := range pods.Items {
75+
for i := range pods.Items {
76+
pod := &pods.Items[i]
7577
if pod.Status.Phase != corev1.PodRunning {
7678
continue
7779
}
78-
return &pod, nil
80+
if slices.ContainsFunc(pod.Status.Conditions, func(c corev1.PodCondition) bool {
81+
return c.Type == corev1.PodReady && c.Status == corev1.ConditionTrue
82+
}) {
83+
return pod, nil
84+
}
7985
}
8086
return nil, errors.New("no running instance found")
8187
}

‎percona/controller/utils_test.go‎

Lines changed: 177 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,177 @@
1+
package controller
2+
3+
import (
4+
"context"
5+
"testing"
6+
7+
"github.com/stretchr/testify/assert"
8+
"github.com/stretchr/testify/require"
9+
corev1 "k8s.io/api/core/v1"
10+
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
11+
"sigs.k8s.io/controller-runtime/pkg/client/fake"
12+
13+
"github.com/percona/percona-postgresql-operator/v2/internal/naming"
14+
)
15+
16+
func TestGetReadyInstancePod(t *testing.T) {
17+
const (
18+
clusterName = "test-cluster"
19+
ns = "test-ns"
20+
)
21+
22+
instanceLabels := map[string]string{
23+
naming.LabelCluster: clusterName,
24+
naming.LabelInstance: "instance1",
25+
}
26+
27+
tests := []struct {
28+
name string
29+
pods []corev1.Pod
30+
wantPod string
31+
wantErr string
32+
}{
33+
{
34+
name: "no pods",
35+
pods: nil,
36+
wantErr: "no running instance found",
37+
},
38+
{
39+
name: "pod not running",
40+
pods: []corev1.Pod{
41+
{
42+
ObjectMeta: metav1.ObjectMeta{
43+
Name: "pod-pending",
44+
Namespace: ns,
45+
Labels: instanceLabels,
46+
},
47+
Status: corev1.PodStatus{
48+
Phase: corev1.PodPending,
49+
},
50+
},
51+
},
52+
wantErr: "no running instance found",
53+
},
54+
{
55+
name: "pod running but not ready",
56+
pods: []corev1.Pod{
57+
{
58+
ObjectMeta: metav1.ObjectMeta{
59+
Name: "pod-not-ready",
60+
Namespace: ns,
61+
Labels: instanceLabels,
62+
},
63+
Status: corev1.PodStatus{
64+
Phase: corev1.PodRunning,
65+
Conditions: []corev1.PodCondition{
66+
{
67+
Type: corev1.PodReady,
68+
Status: corev1.ConditionFalse,
69+
},
70+
},
71+
},
72+
},
73+
},
74+
wantErr: "no running instance found",
75+
},
76+
{
77+
name: "pod running and ready",
78+
pods: []corev1.Pod{
79+
{
80+
ObjectMeta: metav1.ObjectMeta{
81+
Name: "pod-ready",
82+
Namespace: ns,
83+
Labels: instanceLabels,
84+
},
85+
Status: corev1.PodStatus{
86+
Phase: corev1.PodRunning,
87+
Conditions: []corev1.PodCondition{
88+
{
89+
Type: corev1.PodReady,
90+
Status: corev1.ConditionTrue,
91+
},
92+
},
93+
},
94+
},
95+
},
96+
wantPod: "pod-ready",
97+
},
98+
{
99+
name: "skips not-ready pod and returns ready pod",
100+
pods: []corev1.Pod{
101+
{
102+
ObjectMeta: metav1.ObjectMeta{
103+
Name: "pod-not-ready",
104+
Namespace: ns,
105+
Labels: instanceLabels,
106+
},
107+
Status: corev1.PodStatus{
108+
Phase: corev1.PodRunning,
109+
Conditions: []corev1.PodCondition{
110+
{
111+
Type: corev1.PodReady,
112+
Status: corev1.ConditionFalse,
113+
},
114+
},
115+
},
116+
},
117+
{
118+
ObjectMeta: metav1.ObjectMeta{
119+
Name: "pod-ready",
120+
Namespace: ns,
121+
Labels: instanceLabels,
122+
},
123+
Status: corev1.PodStatus{
124+
Phase: corev1.PodRunning,
125+
Conditions: []corev1.PodCondition{
126+
{
127+
Type: corev1.PodReady,
128+
Status: corev1.ConditionTrue,
129+
},
130+
},
131+
},
132+
},
133+
},
134+
wantPod: "pod-ready",
135+
},
136+
{
137+
name: "pod running without ready condition",
138+
pods: []corev1.Pod{
139+
{
140+
ObjectMeta: metav1.ObjectMeta{
141+
Name: "pod-no-condition",
142+
Namespace: ns,
143+
Labels: instanceLabels,
144+
},
145+
Status: corev1.PodStatus{
146+
Phase: corev1.PodRunning,
147+
Conditions: []corev1.PodCondition{},
148+
},
149+
},
150+
},
151+
wantErr: "no running instance found",
152+
},
153+
}
154+
155+
for _, tt := range tests {
156+
t.Run(tt.name, func(t *testing.T) {
157+
ctx := context.Background()
158+
159+
builder := fake.NewClientBuilder()
160+
for i := range tt.pods {
161+
builder = builder.WithObjects(&tt.pods[i])
162+
}
163+
cl := builder.Build()
164+
165+
pod, err := GetReadyInstancePod(ctx, cl, clusterName, ns)
166+
if tt.wantErr != "" {
167+
require.Error(t, err)
168+
assert.Contains(t, err.Error(), tt.wantErr)
169+
assert.Nil(t, pod)
170+
} else {
171+
require.NoError(t, err)
172+
require.NotNil(t, pod)
173+
assert.Equal(t, tt.wantPod, pod.Name)
174+
}
175+
})
176+
}
177+
}

‎percona/pgbackrest/pgbackrest.go‎

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -41,12 +41,18 @@ type InfoStanza struct {
4141
} `json:"status,omitempty"`
4242
}
4343

44-
var ErrNoValidBackups = errors.New("no valid backups")
44+
var (
45+
ErrNoValidBackups = errors.New("no valid backups")
46+
ErrStanzaNotCreated = errors.New("pgBackRest stanza not created")
47+
)
4548

4649
const (
4750
statusOK = 0
4851
// statusNoValidBackups means that there are no backups in pgbackrest
4952
statusNoValidBackups = 2
53+
// statusOther indicates a transient issue, e.g. stanza not yet created
54+
// or temporarily unavailable during pod restarts
55+
statusOther = 99
5056
)
5157

5258
func GetInfo(ctx context.Context, pod *corev1.Pod, repoName string) (InfoOutput, error) {
@@ -77,6 +83,8 @@ func GetInfo(ctx context.Context, pod *corev1.Pod, repoName string) (InfoOutput,
7783
switch elem.Status.Code {
7884
case statusNoValidBackups:
7985
return InfoOutput{}, ErrNoValidBackups
86+
case statusOther:
87+
return InfoOutput{}, ErrStanzaNotCreated
8088
case statusOK:
8189
continue
8290
default:

0 commit comments

Comments
 (0)