Skip to content

Commit 32e847d

Browse files
committed
fix concurrency issue by checking the secret update
1 parent 7e7e05b commit 32e847d

File tree

1 file changed

+61
-6
lines changed

1 file changed

+61
-6
lines changed

internal/controller/postgrescluster/postgres.go

Lines changed: 61 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ import (
2525
"k8s.io/apimachinery/pkg/types"
2626
"k8s.io/apimachinery/pkg/util/sets"
2727
"k8s.io/apimachinery/pkg/util/validation/field"
28+
"k8s.io/client-go/util/retry"
2829
"sigs.k8s.io/controller-runtime/pkg/client"
2930

3031
"github.com/percona/percona-postgresql-operator/internal/feature"
@@ -496,6 +497,9 @@ func (r *Reconciler) reconcilePostgresUserSecrets(
496497
userSpecs[string(specUsers[i].Name)] = &specUsers[i]
497498
}
498499

500+
// K8SPG-570 for secrets that were created manually, update them
501+
// with the right labels so that the selector called next to track them
502+
// and utilize their data.
499503
for _, user := range specUsers {
500504
if user.SecretName != "" {
501505
if err := r.updateCustomSecretLabels(ctx, cluster, user); err != nil {
@@ -590,9 +594,10 @@ func (r *Reconciler) reconcilePostgresUserSecrets(
590594
return specUsers, userSecrets, err
591595
}
592596

593-
// updateCustomSecretLabels checks if a custom secret exists - can be created manually through kubectl apply
594-
// and updates it with required labels if they are missing that enabled the
595-
// naming.AsSelector(naming.ClusterPostgresUsers(cluster.Name)) to identify them.
597+
// K8SPG-570
598+
// updateCustomSecretLabels checks if a custom secret exists - can be created manually through
599+
// kubectl apply - and updates it with required labels if they are missing. This enables the
600+
// naming.AsSelector(naming.ClusterPostgresUsers(cluster.Name)) to identify these secrets.
596601
func (r *Reconciler) updateCustomSecretLabels(
597602
ctx context.Context, cluster *v1beta1.PostgresCluster, user v1beta1.PostgresUserSpec,
598603
) error {
@@ -611,11 +616,10 @@ func (r *Reconciler) updateCustomSecretLabels(
611616
return errors.Wrap(err, fmt.Sprintf("failed to get user %s secret %s", userName, secretName))
612617
}
613618

614-
orig := secret.DeepCopy()
615-
616619
requiredLabels := map[string]string{
617620
naming.LabelCluster: cluster.Name,
618621
naming.LabelPostgresUser: userName,
622+
naming.LabelRole: naming.RolePostgresUser,
619623
}
620624

621625
needsUpdate := false
@@ -631,7 +635,58 @@ func (r *Reconciler) updateCustomSecretLabels(
631635
}
632636

633637
if needsUpdate {
634-
return errors.WithStack(r.Client.Patch(ctx, secret.DeepCopy(), client.MergeFrom(orig)))
638+
err := retry.RetryOnConflict(retry.DefaultRetry, func() error {
639+
current := &corev1.Secret{}
640+
if err := r.Client.Get(ctx, types.NamespacedName{
641+
Name: secretName,
642+
Namespace: cluster.Namespace,
643+
}, current); err != nil {
644+
return err
645+
}
646+
647+
currentOrig := current.DeepCopy()
648+
if current.Labels == nil {
649+
current.Labels = make(map[string]string)
650+
}
651+
652+
updateNeeded := false
653+
for labelKey, labelValue := range requiredLabels {
654+
if existing, exists := current.Labels[labelKey]; !exists || existing != labelValue {
655+
current.Labels[labelKey] = labelValue
656+
updateNeeded = true
657+
}
658+
}
659+
660+
if !updateNeeded {
661+
return nil
662+
}
663+
664+
return r.Client.Patch(ctx, current, client.MergeFrom(currentOrig))
665+
})
666+
667+
if err != nil {
668+
return errors.Wrap(err, fmt.Sprintf("failed to update secret %s", secretName))
669+
}
670+
671+
verifyErr := retry.RetryOnConflict(retry.DefaultRetry, func() error {
672+
verifySecret := &corev1.Secret{}
673+
if err := r.Client.Get(ctx, types.NamespacedName{
674+
Name: secretName,
675+
Namespace: cluster.Namespace,
676+
}, verifySecret); err != nil {
677+
return err
678+
}
679+
680+
for labelKey, labelValue := range requiredLabels {
681+
if existing, exists := verifySecret.Labels[labelKey]; !exists || existing != labelValue {
682+
return errors.Errorf("secret %s label %s not yet propagated", secretName, labelKey)
683+
}
684+
}
685+
686+
return nil
687+
})
688+
689+
return errors.Wrap(verifyErr, "failed to update secret")
635690
}
636691

637692
return nil

0 commit comments

Comments
 (0)