Skip to content

Commit ae5a498

Browse files
Jonathan S. Katzjkatz
authored andcommitted
Change label updates to PATCH requests on non-Postgres Deployments
This avoids any race conditions that can occur if a full update occurs on a Deployment while we are going through and making updates.
1 parent a4e350b commit ae5a498

File tree

1 file changed

+38
-20
lines changed

1 file changed

+38
-20
lines changed

internal/controller/pgcluster/pgclustercontroller.go

Lines changed: 38 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -738,37 +738,55 @@ func updateLabelsForDeployments(c *Controller, cluster *crv1.Pgcluster, labels m
738738
).String(),
739739
}
740740

741-
items, err := c.Client.AppsV1().Deployments(cluster.Namespace).List(ctx, options)
741+
// create the contents of the merge patch for updating the labels on the
742+
// Deployments. We're going to create two merge patches: one for just Postgres
743+
// instances, and one for every other instance. We're not updating the
744+
// template for the Postgres Deployments as that will trigger a restart
745+
mergePatchDeployments := kubeapi.NewMergePatch()
746+
mergePatchPostgresDeployments := kubeapi.NewMergePatch()
747+
748+
// first set the patch for labels that need to be removed.
749+
for i := range labelsToRemove {
750+
mergePatchDeployments.Remove("metadata", "labels", labelsToRemove[i])
751+
mergePatchDeployments.Remove("spec", "template", "metadata", "labels", labelsToRemove[i])
752+
mergePatchPostgresDeployments.Remove("metadata", "labels", labelsToRemove[i])
753+
}
742754

755+
// now, set the patch for labels that need to be added/updated
756+
for k, v := range labels {
757+
mergePatchDeployments.Add("metadata", "labels", k)(v)
758+
mergePatchDeployments.Add("spec", "template", "metadata", "labels", k)(v)
759+
mergePatchPostgresDeployments.Add("metadata", "labels", k)(v)
760+
}
761+
762+
// generate the bytes for the two patches
763+
patchDeployments, err := mergePatchDeployments.Bytes()
743764
if err != nil {
744765
return err
745766
}
746767

747-
for i := range items.Items {
748-
item := &items.Items[i]
768+
patchPostgresDeployments, err := mergePatchPostgresDeployments.Bytes()
769+
if err != nil {
770+
return err
771+
}
749772

750-
for j := range labelsToRemove {
751-
delete(item.ObjectMeta.Labels, labelsToRemove[j])
773+
items, err := c.Client.AppsV1().Deployments(cluster.Namespace).List(ctx, options)
752774

753-
// only remove the labels on the template if this is not a Postgres
754-
// instance
755-
if _, ok := item.ObjectMeta.Labels[config.LABEL_PG_DATABASE]; !ok {
756-
delete(item.Spec.Template.ObjectMeta.Labels, labelsToRemove[j])
757-
}
758-
}
775+
if err != nil {
776+
return err
777+
}
759778

760-
for k, v := range labels {
761-
item.ObjectMeta.Labels[k] = v
779+
for i := range items.Items {
780+
item := &items.Items[i]
762781

763-
// only update the labels on the template if this is not a Postgres
764-
// instance
765-
if _, ok := item.ObjectMeta.Labels[config.LABEL_PG_DATABASE]; !ok {
766-
item.Spec.Template.ObjectMeta.Labels[k] = v
767-
}
782+
patch := patchDeployments
783+
if _, isPostgresInstance := item.ObjectMeta.Labels[config.LABEL_PG_DATABASE]; isPostgresInstance {
784+
patch = patchPostgresDeployments
768785
}
769786

770-
if _, err := c.Client.AppsV1().Deployments(cluster.Namespace).Update(ctx,
771-
item, metav1.UpdateOptions{}); err != nil {
787+
// and patch!
788+
if _, err := c.Client.AppsV1().Deployments(cluster.Namespace).Patch(ctx,
789+
item.Name, types.MergePatchType, patch, metav1.PatchOptions{}); err != nil {
772790
return err
773791
}
774792
}

0 commit comments

Comments
 (0)