Refactoring to use a single API call

zalando · Jan 3, 2025 · 2c9ceed · 2c9ceed
1 parent 024db78
commit 2c9ceed
Show file tree

Hide file tree

Showing 3 changed files with 48 additions and 36 deletions.
diff --git a/pkg/cluster/connection_pooler.go b/pkg/cluster/connection_pooler.go
@@ -1047,6 +1047,8 @@ func (c *Cluster) syncConnectionPoolerWorker(oldSpec, newSpec *acidv1.Postgresql
 			syncReason = append(syncReason, []string{"new connection pooler's pod template annotations do not match the current ones: " + reason}...)
 
 			if strings.Contains(reason, "Removed") {
+				annotationToRemove := `{"metadata":{"annotations":{`
+				annotationToRemoveTemplate := `{"spec":{"template":{"metadata":{"annotations":{`
 				for anno := range deployment.Spec.Template.Annotations {
 					if _, ok := newPodAnnotations[anno]; !ok {
 						// template annotation was removed
@@ -1055,23 +1057,24 @@ func (c *Cluster) syncConnectionPoolerWorker(oldSpec, newSpec *acidv1.Postgresql
 								continue
 							}
 						}
-						annotationToRemove := []byte(fmt.Sprintf(`{"metadata":{"annotations":{"%s":null}}}`, anno))
-						annotationToRemoveTemplate := []byte(fmt.Sprintf(`{"spec":{"template":{"metadata":{"annotations":{"%s":null}}}}}`, anno))
-						deployment, err = c.KubeClient.Deployments(c.Namespace).Patch(context.TODO(),
-							deployment.Name, types.StrategicMergePatchType, annotationToRemoveTemplate, metav1.PatchOptions{}, "")
-						if err != nil {
-							c.logger.Errorf("failed to remove annotation %s from %s connection pooler's pod template: %v",
-								anno, role, err)
-							return nil, err
-						}
-						for _, pod := range pods {
-							_, err = c.KubeClient.Pods(c.Namespace).Patch(context.TODO(), pod.Name,
-								types.StrategicMergePatchType, annotationToRemove, metav1.PatchOptions{})
-							if err != nil {
-								c.logger.Errorf("failed to remove annotation %s from pod %s: %v", anno, pod.Name, err)
-								return nil, err
-							}
-						}
+						annotationToRemove += fmt.Sprintf(`"%s":null,`, anno)
+						annotationToRemoveTemplate += fmt.Sprintf(`"%s":null,`, anno)
+					}
+				}
+				annotationToRemove = strings.TrimSuffix(annotationToRemove, ",") + `}}}`
+				annotationToRemoveTemplate = strings.TrimSuffix(annotationToRemoveTemplate, ",") + `}}}}}`
+				deployment, err = c.KubeClient.Deployments(c.Namespace).Patch(context.TODO(),
+					deployment.Name, types.StrategicMergePatchType, []byte(annotationToRemoveTemplate), metav1.PatchOptions{}, "")
+				if err != nil {
+					c.logger.Errorf("failed to remove annotations from %s connection pooler's pod template: %v", role, err)
+					return nil, err
+				}
+				for _, pod := range pods {
+					_, err = c.KubeClient.Pods(c.Namespace).Patch(context.TODO(), pod.Name,
+						types.StrategicMergePatchType, []byte(annotationToRemove), metav1.PatchOptions{})
+					if err != nil {
+						c.logger.Errorf("failed to remove annotations from pod %s: %v", pod.Name, err)
+						return nil, err
 					}
 				}
 			}

diff --git a/pkg/cluster/sync.go b/pkg/cluster/sync.go
@@ -573,6 +573,7 @@ func (c *Cluster) syncStatefulSet() error {
 					}
 				}
 			}
+			annotationToRemove := ""
 			for anno := range c.Statefulset.Spec.Template.Annotations {
 				if _, ok := desiredSts.Spec.Template.Annotations[anno]; !ok {
 					// template annotation was removed
@@ -581,14 +582,21 @@ func (c *Cluster) syncStatefulSet() error {
 							continue
 						}
 					}
-					annotationToRemove := []byte(fmt.Sprintf(`{"metadata":{"annotations":{"%s":null}}}`, anno))
-					for _, pod := range pods {
-						_, err = c.KubeClient.Pods(c.Namespace).Patch(context.Background(), pod.Name,
-							types.StrategicMergePatchType, annotationToRemove, metav1.PatchOptions{})
-						if err != nil {
-							c.logger.Errorf("failed to remove annotation %s from pod %s: %v", anno, pod.Name, err)
-							return err
-						}
+					if annotationToRemove != "" {
+						annotationToRemove = `{"metadata":{"annotations":{`
+					}
+					annotationToRemove += fmt.Sprintf(`"%s":null,`, anno)
+					// annotationToRemove := []byte(fmt.Sprintf(`{"metadata":{"annotations":{"%s":null}}}`, anno))
+				}
+			}
+			if annotationToRemove != "" {
+				annotationToRemove = strings.TrimSuffix(annotationToRemove, ",") + `}}}`
+				for _, pod := range pods {
+					_, err = c.KubeClient.Pods(c.Namespace).Patch(context.Background(), pod.Name,
+						types.StrategicMergePatchType, []byte(annotationToRemove), metav1.PatchOptions{})
+					if err != nil {
+						c.logger.Errorf("failed to remove annotations from pod %s: %v", pod.Name, err)
+						return err
 					}
 				}
 			}
@@ -1614,6 +1622,7 @@ func (c *Cluster) syncLogicalBackupJob() error {
 				c.logger.Infof("reason: %s", reason)
 			}
 			if strings.Contains(reason, "annotations do not match") {
+				annotationToRemoveTemplate := `{"spec":{"jobTemplate":{"spec":{"template":{"metadata":{"annotations":{`
 				for anno := range job.Spec.JobTemplate.Spec.Template.Annotations {
 					if _, ok := desiredJob.Spec.JobTemplate.Spec.Template.Annotations[anno]; !ok {
 						// template annotation was removed
@@ -1622,16 +1631,16 @@ func (c *Cluster) syncLogicalBackupJob() error {
 								continue
 							}
 						}
-						annotationToRemoveTemplate := []byte(fmt.Sprintf(
-							`{"spec":{"jobTemplate":{"spec":{"template":{"metadata":{"annotations":{"%s":null}}}}}}}`, anno))
-						job, err = c.KubeClient.CronJobs(c.Namespace).Patch(context.TODO(),
-							jobName, types.StrategicMergePatchType, annotationToRemoveTemplate, metav1.PatchOptions{}, "")
-						if err != nil {
-							c.logger.Errorf("failed to remove annotation %s from the logical backup job %q pod template: %v", anno, jobName, err)
-							return err
-						}
+						annotationToRemoveTemplate += fmt.Sprintf(`"%s":null,`, anno)
 					}
 				}
+				annotationToRemoveTemplate = strings.TrimSuffix(annotationToRemoveTemplate, ",") + `}}}}}}}`
+				job, err = c.KubeClient.CronJobs(c.Namespace).Patch(context.TODO(),
+					jobName, types.StrategicMergePatchType, []byte(annotationToRemoveTemplate), metav1.PatchOptions{}, "")
+				if err != nil {
+					c.logger.Errorf("failed to remove annotations from the logical backup job %q pod template: %v", jobName, err)
+					return err
+				}
 			}
 			if err = c.patchLogicalBackupJob(desiredJob); err != nil {
 				return fmt.Errorf("could not update logical backup job to match desired state: %v", err)

diff --git a/pkg/cluster/sync_test.go b/pkg/cluster/sync_test.go
@@ -146,7 +146,7 @@ func TestPodAnnotationsSync(t *testing.T) {
 	clusterName := "acid-test-cluster-2"
 	namespace := "default"
 	podAnnotation := "no-scale-down"
-	podAnnotations := map[string]string{"no-scale-down": "true"}
+	podAnnotations := map[string]string{podAnnotation: "true"}
 
 	ctrl := gomock.NewController(t)
 	defer ctrl.Finish()
@@ -271,7 +271,7 @@ func TestPodAnnotationsSync(t *testing.T) {
 	stsList, err = cluster.KubeClient.StatefulSets(namespace).List(context.TODO(), clusterOptions)
 	assert.NoError(t, err)
 	for _, sts := range stsList.Items {
-		assert.NotContains(t, sts.Spec.Template.Annotations, "no-scale-down")
+		assert.NotContains(t, sts.Spec.Template.Annotations, podAnnotation)
 	}
 
 	for _, role := range []PostgresRole{Master, Replica} {
@@ -286,7 +286,7 @@ func TestPodAnnotationsSync(t *testing.T) {
 	podList, err = cluster.KubeClient.Pods(namespace).List(context.TODO(), clusterOptions)
 	assert.NoError(t, err)
 	for _, pod := range podList.Items {
-		assert.NotContains(t, pod.Annotations, "no-scale-down",
+		assert.NotContains(t, pod.Annotations, podAnnotation,
 			fmt.Sprintf("pod %s should not contain annotation %s, found %#v", pod.Name, podAnnotation, pod.Annotations))
 	}