Skip to content

Commit faa8943

Browse files
mayankshah1607Copilothors
authored
K8SPSMDB-1455: avoid repeated delete calls (#2389)
* avoid repeated Delete calls Signed-off-by: Mayank Shah <mayank.shah@percona.com> * add back cached client Signed-off-by: Mayank Shah <mayank.shah@percona.com> * wrong error package imported Signed-off-by: Mayank Shah <mayank.shah@percona.com> * Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> * DeleteIfExists for hidden sts Signed-off-by: Mayank Shah <mayank.shah@percona.com> * fix RSPod ordering Signed-off-by: Mayank Shah <mayank.shah@percona.com> * Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> * Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> * copilot Signed-off-by: Mayank Shah <mayank.shah@percona.com> * index events Signed-off-by: Mayank Shah <mayank.shah@percona.com> * update e2e test Signed-off-by: Mayank Shah <mayank.shah@percona.com> * copilot Signed-off-by: Mayank Shah <mayank.shah@percona.com> --------- Signed-off-by: Mayank Shah <mayank.shah@percona.com> Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> Co-authored-by: Viacheslav Sarzhan <slava.sarzhan@percona.com>
1 parent f9bebbb commit faa8943

5 files changed

Lines changed: 103 additions & 53 deletions

File tree

cmd/manager/main.go

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,12 +16,14 @@ import (
1616
volumesnapshotv1 "github.com/kubernetes-csi/external-snapshotter/client/v8/apis/volumesnapshot/v1"
1717
uzap "go.uber.org/zap"
1818
"go.uber.org/zap/zapcore"
19+
corev1 "k8s.io/api/core/v1"
1920
k8sruntime "k8s.io/apimachinery/pkg/runtime"
2021
utilruntime "k8s.io/apimachinery/pkg/util/runtime"
2122
"k8s.io/client-go/discovery"
2223
clientgoscheme "k8s.io/client-go/kubernetes/scheme"
2324
ctrl "sigs.k8s.io/controller-runtime"
2425
"sigs.k8s.io/controller-runtime/pkg/cache"
26+
"sigs.k8s.io/controller-runtime/pkg/client"
2527
"sigs.k8s.io/controller-runtime/pkg/healthz"
2628
"sigs.k8s.io/controller-runtime/pkg/log/zap"
2729
metricsServer "sigs.k8s.io/controller-runtime/pkg/metrics/server"
@@ -104,6 +106,12 @@ func main() {
104106
WebhookServer: webhook.NewServer(webhook.Options{
105107
Port: 9443,
106108
}),
109+
Client: client.Options{
110+
Scheme: scheme,
111+
Cache: &client.CacheOptions{
112+
DisableFor: []client.Object{&corev1.Node{}},
113+
},
114+
},
107115
}
108116

109117
options.Controller.GroupKindConcurrency = map[string]int{

e2e-tests/pvc-resize/run

Lines changed: 25 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -68,6 +68,28 @@ function wait_cluster_status() {
6868
echo "psmdb/${cluster} status is ${expected}"
6969
}
7070

71+
function wait_pvc_request_revert() {
72+
local cluster=$1
73+
local expected=$2
74+
local retry=0
75+
76+
echo -n "Waiting for psmdb/${cluster} storage request to revert to ${expected}"
77+
until [[ $(kubectl_bin get psmdb "${cluster}" -o jsonpath='{.spec.replsets[0].volumeSpec.persistentVolumeClaim.resources.requests.storage}') == "${expected}" ]]; do
78+
if [[ $retry -ge 60 ]]; then
79+
echo
80+
echo "psmdb/${cluster} storage request did not revert to ${expected}, max retries exceeded"
81+
exit 1
82+
fi
83+
echo -n "."
84+
sleep 5
85+
86+
retry=$((retry + 1))
87+
done
88+
89+
echo
90+
echo "psmdb/${cluster} storage request reverted to ${expected}"
91+
}
92+
7193
set_debug
7294

7395
if [ "$EKS" == 1 ]; then
@@ -152,12 +174,10 @@ wait_all_pvc_resize "3Gi"
152174

153175
desc "test downscale"
154176

155-
# operator shouldn't try to downscale the PVCs and set status to error
177+
# operator shouldn't downscale the PVCs. instead it reverts the storage request
178+
# in the spec back to the configured size and keeps the cluster ready
156179
patch_pvc_request "${cluster}" "1G"
157-
wait_cluster_status ${cluster} "error"
158-
159-
# user should be able to restore to the previous size and make the cluster ready
160-
patch_pvc_request "${cluster}" "3G"
180+
wait_pvc_request_revert "${cluster}" "3G"
161181
wait_cluster_status ${cluster} "ready"
162182

163183
if [[ $EKS == 1 || -n ${OPENSHIFT} ]]; then

pkg/controller/perconaservermongodb/psmdb_controller.go

Lines changed: 42 additions & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ import (
1818
"github.com/robfig/cron/v3"
1919
appsv1 "k8s.io/api/apps/v1"
2020
corev1 "k8s.io/api/core/v1"
21+
eventsv1 "k8s.io/api/events/v1"
2122
k8serrors "k8s.io/apimachinery/pkg/api/errors"
2223
"k8s.io/apimachinery/pkg/api/meta"
2324
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -36,6 +37,7 @@ import (
3637

3738
"github.com/percona/percona-server-mongodb-operator/clientcmd"
3839
api "github.com/percona/percona-server-mongodb-operator/pkg/apis/psmdb/v1"
40+
k8sutils "github.com/percona/percona-server-mongodb-operator/pkg/k8s"
3941
"github.com/percona/percona-server-mongodb-operator/pkg/naming"
4042
"github.com/percona/percona-server-mongodb-operator/pkg/psmdb"
4143
"github.com/percona/percona-server-mongodb-operator/pkg/psmdb/backup"
@@ -51,6 +53,19 @@ import (
5153
"github.com/percona/percona-server-mongodb-operator/pkg/version"
5254
)
5355

56+
const eventRegardingNameIndex = "regarding.name"
57+
58+
func eventRegardingNameIndexer(o client.Object) []string {
59+
evt, ok := o.(*eventsv1.Event)
60+
if !ok {
61+
return nil
62+
}
63+
if evt.Regarding.Name == "" {
64+
return nil
65+
}
66+
return []string{evt.Regarding.Name}
67+
}
68+
5469
// Add creates a new PerconaServerMongoDB Controller and adds it to the Manager. The Manager will set fields on the Controller
5570
// and Start it when the Manager is Started.
5671
func Add(mgr manager.Manager) error {
@@ -59,6 +74,13 @@ func Add(mgr manager.Manager) error {
5974
return err
6075
}
6176

77+
// the volume resize logic lists PVC events through the cached client,
78+
// which requires the field to be indexed
79+
err = mgr.GetFieldIndexer().IndexField(context.TODO(), &eventsv1.Event{}, eventRegardingNameIndex, eventRegardingNameIndexer)
80+
if err != nil {
81+
return errors.Wrapf(err, "index events by %s", eventRegardingNameIndex)
82+
}
83+
6284
return add(mgr, r)
6385
}
6486

@@ -81,22 +103,12 @@ func newReconciler(mgr manager.Manager) (reconcile.Reconciler, error) {
81103
return nil, errors.Wrap(err, "failed to get operator pod image")
82104
}
83105

84-
client, err := client.New(mgr.GetConfig(), client.Options{
85-
Scheme: mgr.GetScheme(),
86-
Cache: &client.CacheOptions{
87-
DisableFor: []client.Object{&corev1.Node{}},
88-
},
89-
})
90-
if err != nil {
91-
return nil, errors.Wrap(err, "create client")
92-
}
93-
94106
secretProviders := []pkgSecret.Provider{
95107
new(vault.Provider),
96108
}
97109

98110
return &ReconcilePerconaServerMongoDB{
99-
client: client,
111+
client: mgr.GetClient(),
100112
scheme: mgr.GetScheme(),
101113
serverVersion: sv,
102114
reconcileIn: getReconcileInterval(),
@@ -532,10 +544,8 @@ func (r *ReconcilePerconaServerMongoDB) reconcileReplset(ctx context.Context, cr
532544
return err
533545
}
534546
} else {
535-
err := r.client.Delete(ctx, psmdb.NewStatefulSet(naming.ArbiterStatefulSetName(cr, replset), cr.Namespace))
536-
if err != nil && !k8serrors.IsNotFound(err) {
537-
err = errors.Errorf("delete arbiter in replset %s: %v", replset.Name, err)
538-
return err
547+
if err := k8sutils.DeleteIfExists(ctx, r.client, psmdb.NewStatefulSet(naming.ArbiterStatefulSetName(cr, replset), cr.Namespace)); err != nil {
548+
return errors.Wrapf(err, "failed to delete arbiter statefulset: %s", naming.ArbiterStatefulSetName(cr, replset))
539549
}
540550
}
541551

@@ -547,25 +557,21 @@ func (r *ReconcilePerconaServerMongoDB) reconcileReplset(ctx context.Context, cr
547557
return err
548558
}
549559
} else {
550-
err := r.client.Delete(ctx, psmdb.NewStatefulSet(naming.NonVotingStatefulSetName(cr, replset), cr.Namespace))
551-
if err != nil && !k8serrors.IsNotFound(err) {
552-
err = errors.Errorf("delete nonVoting statefulset %s: %v", replset.Name, err)
553-
return err
560+
if err := k8sutils.DeleteIfExists(ctx, r.client, psmdb.NewStatefulSet(naming.NonVotingStatefulSetName(cr, replset), cr.Namespace)); err != nil {
561+
return errors.Wrapf(err, "failed to delete non voting statefulset: %s", naming.NonVotingStatefulSetName(cr, replset))
554562
}
555563
}
556564

557565
if replset.Hidden.Enabled {
558566
matchLabels = naming.HiddenLabels(cr, replset)
559567
_, err := r.reconcileStatefulSet(ctx, cr, replset, matchLabels)
560568
if err != nil {
561-
err = errors.Errorf("reconcile nonVoting StatefulSet for %s: %v", replset.Name, err)
569+
err = errors.Errorf("reconcile hidden StatefulSet for %s: %v", replset.Name, err)
562570
return err
563571
}
564572
} else {
565-
err := r.client.Delete(ctx, psmdb.NewStatefulSet(naming.HiddenStatefulSetName(cr, replset), cr.Namespace))
566-
if err != nil && !k8serrors.IsNotFound(err) {
567-
err = errors.Errorf("delete hidden statefulset %s: %v", replset.Name, err)
568-
return err
573+
if err := k8sutils.DeleteIfExists(ctx, r.client, psmdb.NewStatefulSet(naming.HiddenStatefulSetName(cr, replset), cr.Namespace)); err != nil {
574+
return errors.Wrapf(err, "failed to delete hidden statefulset: %s", naming.HiddenStatefulSetName(cr, replset))
569575
}
570576
}
571577

@@ -1062,24 +1068,18 @@ func (r *ReconcilePerconaServerMongoDB) deleteCfgIfNeeded(ctx context.Context, c
10621068

10631069
sfsName := cr.Name + "-" + api.ConfigReplSetName
10641070
sfs := psmdb.NewStatefulSet(sfsName, cr.Namespace)
1065-
1066-
if err := r.client.Delete(ctx, sfs); err != nil && !k8serrors.IsNotFound(err) {
1071+
if err := k8sutils.DeleteIfExists(ctx, r.client, sfs); err != nil {
10671072
return errors.Wrapf(err, "failed to delete sfs: %s", sfs.Name)
10681073
}
10691074

1070-
svc := corev1.Service{}
1071-
err = r.client.Get(ctx, types.NamespacedName{Name: cr.Name + "-" + api.ConfigReplSetName, Namespace: cr.Namespace}, &svc)
1072-
if err != nil && !k8serrors.IsNotFound(err) {
1073-
return errors.Wrap(err, "failed to get config service")
1074-
}
1075-
1076-
if k8serrors.IsNotFound(err) {
1077-
return nil
1075+
svc := corev1.Service{
1076+
ObjectMeta: metav1.ObjectMeta{
1077+
Name: cr.Name + "-" + api.ConfigReplSetName,
1078+
Namespace: cr.Namespace,
1079+
},
10781080
}
1079-
1080-
err = r.client.Delete(ctx, &svc)
1081-
if err != nil {
1082-
return errors.Wrap(err, "failed to delete config service")
1081+
if err := k8sutils.DeleteIfExists(ctx, r.client, &svc); err != nil {
1082+
return errors.Wrapf(err, "failed to delete config service: %s", svc.Name)
10831083
}
10841084

10851085
return nil
@@ -1117,15 +1117,6 @@ func (r *ReconcilePerconaServerMongoDB) upgradeFCVIfNeeded(ctx context.Context,
11171117
return errors.Wrap(err, "failed to set FCV")
11181118
}
11191119

1120-
func (r *ReconcilePerconaServerMongoDB) deleteMongos(ctx context.Context, cr *api.PerconaServerMongoDB) error {
1121-
err := r.client.Delete(ctx, psmdb.MongosStatefulset(cr))
1122-
if err != nil && !k8serrors.IsNotFound(err) {
1123-
return errors.Wrap(err, "failed to delete mongos statefulset")
1124-
}
1125-
1126-
return nil
1127-
}
1128-
11291120
func (r *ReconcilePerconaServerMongoDB) deleteMongosIfNeeded(ctx context.Context, cr *api.PerconaServerMongoDB) error {
11301121
if cr.Spec.Sharding.Enabled {
11311122
return nil
@@ -1152,7 +1143,10 @@ func (r *ReconcilePerconaServerMongoDB) deleteMongosIfNeeded(ctx context.Context
11521143
}
11531144
}
11541145

1155-
return r.deleteMongos(ctx, cr)
1146+
if err := k8sutils.DeleteIfExists(ctx, r.client, psmdb.MongosStatefulset(cr)); err != nil {
1147+
return errors.Wrap(err, "failed to delete mongos statefulset")
1148+
}
1149+
return nil
11561150
}
11571151

11581152
func (r *ReconcilePerconaServerMongoDB) reconcileMongodConfigMaps(ctx context.Context, cr *api.PerconaServerMongoDB, repls []*api.ReplsetSpec) error {

pkg/k8s/utils.go

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,15 @@
11
package k8s
22

33
import (
4+
"context"
45
"fmt"
56
"os"
67
"strings"
78

9+
"github.com/pkg/errors"
810
corev1 "k8s.io/api/core/v1"
11+
k8serrors "k8s.io/apimachinery/pkg/api/errors"
12+
"sigs.k8s.io/controller-runtime/pkg/client"
913
)
1014

1115
const WatchNamespaceEnvVar = "WATCH_NAMESPACE"
@@ -41,3 +45,21 @@ func IsPodReady(pod corev1.Pod) bool {
4145

4246
return false
4347
}
48+
49+
func DeleteIfExists(ctx context.Context, c client.Client, obj client.Object) error {
50+
if err := c.Get(ctx, client.ObjectKeyFromObject(obj), obj); k8serrors.IsNotFound(err) {
51+
return nil
52+
} else if err != nil {
53+
return errors.Wrapf(err, "failed to get %T %s/%s", obj, obj.GetNamespace(), obj.GetName())
54+
}
55+
56+
// Deletion was already requested
57+
if obj.GetDeletionTimestamp() != nil {
58+
return nil
59+
}
60+
61+
if err := c.Delete(ctx, obj); client.IgnoreNotFound(err) != nil {
62+
return errors.Wrapf(err, "failed to delete %T %s/%s", obj, obj.GetNamespace(), obj.GetName())
63+
}
64+
return nil
65+
}

pkg/psmdb/getters.go

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,12 @@ func getRSPods(ctx context.Context, k8sclient client.Client, cr *api.PerconaServ
4444
return rsPods, errors.Wrapf(err, "failed to get statefulset list related to replset %s", rsName)
4545
}
4646

47+
// `client.List` doesn't guarantee ordering (the cache-backed client returns items
48+
// in map iteration order). Sort StatefulSets by name so iteration order is deterministic.
49+
sort.Slice(stsList.Items, func(i, j int) bool {
50+
return stsList.Items[i].Name < stsList.Items[j].Name
51+
})
52+
4753
for _, sts := range stsList.Items {
4854
rs := cr.Spec.Replset(rsName)
4955

0 commit comments

Comments
 (0)