Skip to content

Commit 799c015

Browse files
hughcapethughcapet
committed
Ensure podAnnotations are removed from pods if reset in the config
1 parent 265f2a0 commit 799c015

File tree

3 files changed

+222
-9
lines changed

3 files changed

+222
-9
lines changed

‎pkg/cluster/connection_pooler.go‎

Lines changed: 45 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1060,7 +1060,6 @@ func (c *Cluster) syncConnectionPoolerWorker(oldSpec, newSpec *acidv1.Postgresql
10601060
if err != nil {
10611061
return syncReason, err
10621062
}
1063-
c.ConnectionPooler[role].Deployment = deployment
10641063
}
10651064

10661065
newAnnotations := c.AnnotationsToPropagate(c.annotationsSet(nil)) // including the downscaling annotations
@@ -1069,7 +1068,6 @@ func (c *Cluster) syncConnectionPoolerWorker(oldSpec, newSpec *acidv1.Postgresql
10691068
if err != nil {
10701069
return nil, err
10711070
}
1072-
c.ConnectionPooler[role].Deployment = deployment
10731071
}
10741072
}
10751073

@@ -1098,18 +1096,56 @@ func (c *Cluster) syncConnectionPoolerWorker(oldSpec, newSpec *acidv1.Postgresql
10981096
if err != nil {
10991097
return nil, fmt.Errorf("could not delete pooler pod: %v", err)
11001098
}
1101-
} else if changed, _ := c.compareAnnotations(pod.Annotations, deployment.Spec.Template.Annotations); changed {
1102-
patchData, err := metaAnnotationsPatch(deployment.Spec.Template.Annotations)
1103-
if err != nil {
1104-
return nil, fmt.Errorf("could not form patch for pooler's pod annotations: %v", err)
1099+
} else {
1100+
if changed, _ := c.compareAnnotations(pod.Annotations, deployment.Spec.Template.Annotations); changed {
1101+
patchData, err := metaAnnotationsPatch(deployment.Spec.Template.Annotations)
1102+
if err != nil {
1103+
return nil, fmt.Errorf("could not form patch for pooler's pod annotations: %v", err)
1104+
}
1105+
_, err = c.KubeClient.Pods(pod.Namespace).Patch(context.TODO(), pod.Name, types.MergePatchType, []byte(patchData), metav1.PatchOptions{})
1106+
if err != nil {
1107+
return nil, fmt.Errorf("could not patch annotations for pooler's pod %q: %v", pod.Name, err)
1108+
}
11051109
}
1106-
_, err = c.KubeClient.Pods(pod.Namespace).Patch(context.TODO(), pod.Name, types.MergePatchType, []byte(patchData), metav1.PatchOptions{})
1107-
if err != nil {
1108-
return nil, fmt.Errorf("could not patch annotations for pooler's pod %q: %v", pod.Name, err)
1110+
}
1111+
}
1112+
1113+
if oldSpec != nil {
1114+
for anno := range oldSpec.Spec.PodAnnotations {
1115+
if _, ok := newSpec.Spec.PodAnnotations[anno]; !ok {
1116+
// template annotation was removed
1117+
for _, ignore := range c.OpConfig.IgnoredAnnotations {
1118+
if anno == ignore {
1119+
continue
1120+
}
1121+
}
1122+
annotationToRemove := []byte(fmt.Sprintf(`{"metadata":{"annotations":{"%s":null}}}`, anno))
1123+
annotationToRemoveTemplate := []byte(fmt.Sprintf(`{"spec":{"template":{"metadata":{"annotations":{"%s":null}}}}}`, anno))
1124+
if err != nil {
1125+
c.logger.Errorf("could not form removal patch for pod annotations: %v", err)
1126+
return nil, err
1127+
}
1128+
deployment, err = c.KubeClient.Deployments(c.Namespace).Patch(context.TODO(),
1129+
deployment.Name, types.StrategicMergePatchType, annotationToRemoveTemplate, metav1.PatchOptions{}, "")
1130+
if err != nil {
1131+
c.logger.Errorf("failed to remove annotation %s from %s connection pooler's pod template: %v",
1132+
anno, role, err)
1133+
return nil, err
1134+
}
1135+
for _, pod := range pods {
1136+
_, err = c.KubeClient.Pods(c.Namespace).Patch(context.TODO(), pod.Name,
1137+
types.StrategicMergePatchType, annotationToRemove, metav1.PatchOptions{})
1138+
if err != nil {
1139+
c.logger.Errorf("failed to remove annotation %s from pod %s: %v", anno, pod.Name, err)
1140+
return nil, err
1141+
}
1142+
}
11091143
}
11101144
}
11111145
}
11121146

1147+
c.ConnectionPooler[role].Deployment = deployment
1148+
11131149
if service, err = c.KubeClient.Services(c.Namespace).Get(context.TODO(), c.connectionPoolerName(role), metav1.GetOptions{}); err == nil {
11141150
c.ConnectionPooler[role].Service = service
11151151
desiredSvc := c.generateConnectionPoolerService(c.ConnectionPooler[role])

‎pkg/cluster/sync.go‎

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -573,6 +573,29 @@ func (c *Cluster) syncStatefulSet() error {
573573
}
574574
}
575575
}
576+
for anno := range c.Statefulset.Spec.Template.Annotations {
577+
if _, ok := desiredSts.Spec.Template.Annotations[anno]; !ok {
578+
// template annotation was removed
579+
for _, ignore := range c.OpConfig.IgnoredAnnotations {
580+
if anno == ignore {
581+
continue
582+
}
583+
}
584+
annotationToRemove := []byte(fmt.Sprintf(`{"metadata":{"annotations":{"%s":null}}}`, anno))
585+
if err != nil {
586+
c.logger.Errorf("could not form removal patch for pod annotations: %v", err)
587+
return err
588+
}
589+
for _, pod := range pods {
590+
_, err = c.KubeClient.Pods(c.Namespace).Patch(context.Background(), pod.Name,
591+
types.StrategicMergePatchType, annotationToRemove, metav1.PatchOptions{})
592+
if err != nil {
593+
c.logger.Errorf("failed to remove annotation %s from pod %s: %v", anno, pod.Name, err)
594+
return err
595+
}
596+
}
597+
}
598+
}
576599
}
577600
if !cmp.match {
578601
if cmp.rollingUpdate {

‎pkg/cluster/sync_test.go‎

Lines changed: 154 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -142,6 +142,160 @@ func TestSyncStatefulSetsAnnotations(t *testing.T) {
142142
}
143143
}
144144

145+
func TestPodAnnotationsSync(t *testing.T) {
146+
// testName := "test pod annotations"
147+
clusterName := "acid-test-cluster-2"
148+
namespace := "default"
149+
podAnnotation := "no-scale-down"
150+
podAnnotations := map[string]string{"no-scale-down": "true"}
151+
152+
ctrl := gomock.NewController(t)
153+
defer ctrl.Finish()
154+
mockClient := mocks.NewMockHTTPClient(ctrl)
155+
client, _ := newFakeK8sAnnotationsClient()
156+
157+
pg := acidv1.Postgresql{
158+
ObjectMeta: metav1.ObjectMeta{
159+
Name: clusterName,
160+
Namespace: namespace,
161+
},
162+
Spec: acidv1.PostgresSpec{
163+
Volume: acidv1.Volume{
164+
Size: "1Gi",
165+
},
166+
EnableConnectionPooler: boolToPointer(true),
167+
EnableLogicalBackup: true,
168+
EnableReplicaConnectionPooler: boolToPointer(true),
169+
PodAnnotations: podAnnotations,
170+
NumberOfInstances: 2,
171+
},
172+
}
173+
174+
var cluster = New(
175+
Config{
176+
OpConfig: config.Config{
177+
PatroniAPICheckInterval: time.Duration(1),
178+
PatroniAPICheckTimeout: time.Duration(5),
179+
PodManagementPolicy: "ordered_ready",
180+
ConnectionPooler: config.ConnectionPooler{
181+
ConnectionPoolerDefaultCPURequest: "100m",
182+
ConnectionPoolerDefaultCPULimit: "100m",
183+
ConnectionPoolerDefaultMemoryRequest: "100Mi",
184+
ConnectionPoolerDefaultMemoryLimit: "100Mi",
185+
NumberOfInstances: k8sutil.Int32ToPointer(1),
186+
},
187+
Resources: config.Resources{
188+
ClusterLabels: map[string]string{"application": "spilo"},
189+
ClusterNameLabel: "cluster-name",
190+
DefaultCPURequest: "300m",
191+
DefaultCPULimit: "300m",
192+
DefaultMemoryRequest: "300Mi",
193+
DefaultMemoryLimit: "300Mi",
194+
PodRoleLabel: "spilo-role",
195+
ResourceCheckInterval: time.Duration(3),
196+
ResourceCheckTimeout: time.Duration(10),
197+
},
198+
},
199+
}, client, pg, logger, eventRecorder)
200+
201+
configJson := `{"postgresql": {"parameters": {"log_min_duration_statement": 200, "max_connections": 50}}}, "ttl": 20}`
202+
response := http.Response{
203+
StatusCode: 200,
204+
Body: io.NopCloser(bytes.NewReader([]byte(configJson))),
205+
}
206+
207+
mockClient.EXPECT().Do(gomock.Any()).Return(&response, nil).AnyTimes()
208+
cluster.patroni = patroni.New(patroniLogger, mockClient)
209+
cluster.Name = clusterName
210+
cluster.Namespace = namespace
211+
clusterOptions := clusterLabelsOptions(cluster)
212+
213+
// create a statefulset
214+
_, err := cluster.createStatefulSet()
215+
assert.NoError(t, err)
216+
// create a pods
217+
podsList := createPods(cluster)
218+
for _, pod := range podsList {
219+
_, err = cluster.KubeClient.Pods(namespace).Create(context.TODO(), &pod, metav1.CreateOptions{})
220+
assert.NoError(t, err)
221+
}
222+
// create connection pooler
223+
_, err = cluster.createConnectionPooler(mockInstallLookupFunction)
224+
assert.NoError(t, err)
225+
226+
annotateResources(cluster)
227+
err = cluster.Sync(&cluster.Postgresql)
228+
assert.NoError(t, err)
229+
230+
// 1. PodAnnotations set
231+
stsList, err := cluster.KubeClient.StatefulSets(namespace).List(context.TODO(), clusterOptions)
232+
assert.NoError(t, err)
233+
for _, sts := range stsList.Items {
234+
assert.Contains(t, sts.Spec.Template.Annotations, podAnnotation)
235+
}
236+
237+
for _, role := range []PostgresRole{Master, Replica} {
238+
deploy, err := cluster.KubeClient.Deployments(namespace).Get(context.TODO(), cluster.connectionPoolerName(role), metav1.GetOptions{})
239+
assert.NoError(t, err)
240+
assert.Contains(t, deploy.Spec.Template.Annotations, podAnnotation,
241+
fmt.Sprintf("pooler deployment pod template %s should contain annotation %s, found %#v",
242+
deploy.Name, podAnnotation, deploy.Spec.Template.Annotations))
243+
assert.NoError(t, err)
244+
}
245+
246+
podList, err := cluster.KubeClient.Pods(namespace).List(context.TODO(), clusterOptions)
247+
assert.NoError(t, err)
248+
for _, pod := range podList.Items {
249+
assert.Contains(t, pod.Annotations, podAnnotation,
250+
fmt.Sprintf("pod %s should contain annotation %s, found %#v", pod.Name, podAnnotation, pod.Annotations))
251+
assert.NoError(t, err)
252+
}
253+
254+
cronJobList, err := cluster.KubeClient.CronJobs(namespace).List(context.TODO(), clusterOptions)
255+
assert.NoError(t, err)
256+
for _, cronJob := range cronJobList.Items {
257+
assert.Contains(t, cronJob.Annotations, podAnnotation,
258+
fmt.Sprintf("logical backup cron job's pod template should contain annotation %s, found %#v",
259+
podAnnotation, cronJob.Spec.JobTemplate.Spec.Template.Annotations))
260+
}
261+
262+
// 2 PodAnnotations removed
263+
newSpec := cluster.Postgresql.DeepCopy()
264+
newSpec.Spec.PodAnnotations = nil
265+
err = cluster.Sync(newSpec)
266+
assert.NoError(t, err)
267+
268+
stsList, err = cluster.KubeClient.StatefulSets(namespace).List(context.TODO(), clusterOptions)
269+
assert.NoError(t, err)
270+
for _, sts := range stsList.Items {
271+
assert.NotContains(t, sts.Spec.Template.Annotations, "no-scale-down")
272+
}
273+
274+
for _, role := range []PostgresRole{Master, Replica} {
275+
deploy, err := cluster.KubeClient.Deployments(namespace).Get(context.TODO(), cluster.connectionPoolerName(role), metav1.GetOptions{})
276+
assert.NoError(t, err)
277+
assert.NotContains(t, deploy.Spec.Template.Annotations, podAnnotation,
278+
fmt.Sprintf("pooler deployment pod template %s should not contain annotation %s, found %#v",
279+
deploy.Name, podAnnotation, deploy.Spec.Template.Annotations))
280+
assert.NoError(t, err)
281+
}
282+
283+
podList, err = cluster.KubeClient.Pods(namespace).List(context.TODO(), clusterOptions)
284+
assert.NoError(t, err)
285+
for _, pod := range podList.Items {
286+
assert.NotContains(t, pod.Annotations, "no-scale-down",
287+
fmt.Sprintf("pod %s should not contain annotation %s, found %#v", pod.Name, podAnnotation, pod.Annotations))
288+
}
289+
290+
cronJobList, err = cluster.KubeClient.CronJobs(namespace).List(context.TODO(), clusterOptions)
291+
assert.NoError(t, err)
292+
for _, cronJob := range cronJobList.Items {
293+
assert.NotContains(t, cronJob.Annotations, podAnnotation,
294+
fmt.Sprintf("logical backup cron job's pod template should not contain annotation %s, found %#v",
295+
podAnnotation, cronJob.Spec.JobTemplate.Spec.Template.Annotations))
296+
}
297+
}
298+
145299
func TestCheckAndSetGlobalPostgreSQLConfiguration(t *testing.T) {
146300
testName := "test config comparison"
147301
client, _ := newFakeK8sSyncClient()

0 commit comments

Comments
 (0)