Skip to content

Commit fffb532

Browse files
mikkeloscarmikkeloscar
committed
Use UpdateStatus instead of patch
Signed-off-by: Mikkel Oscar Lyderik Larsen <mikkel.larsen@zalando.de>
1 parent 9eaddae commit fffb532

File tree

5 files changed

+71
-61
lines changed

5 files changed

+71
-61
lines changed

‎pkg/cluster/cluster.go‎

Lines changed: 35 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@ import (
3232
v1 "k8s.io/api/core/v1"
3333
policyv1 "k8s.io/api/policy/v1"
3434
rbacv1 "k8s.io/api/rbac/v1"
35+
"k8s.io/apimachinery/pkg/api/equality"
3536
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
3637
"k8s.io/apimachinery/pkg/types"
3738
"k8s.io/client-go/rest"
@@ -271,26 +272,29 @@ func (c *Cluster) Create() (err error) {
271272
)
272273

273274
defer func() {
274-
var (
275-
pgUpdatedStatus *acidv1.Postgresql
276-
errStatus error
277-
)
278-
if err == nil {
279-
pgUpdatedStatus, errStatus = c.KubeClient.SetPostgresCRDStatus(c.clusterName(), acidv1.ClusterStatusRunning) //TODO: are you sure it's running?
280-
} else {
275+
currentStatus := c.Status.DeepCopy()
276+
pg := c.Postgresql.DeepCopy()
277+
pg.Status.PostgresClusterStatus = acidv1.ClusterStatusRunning
278+
279+
if err != nil {
281280
c.logger.Warningf("cluster created failed: %v", err)
282-
pgUpdatedStatus, errStatus = c.KubeClient.SetPostgresCRDStatus(c.clusterName(), acidv1.ClusterStatusAddFailed)
283-
}
284-
if errStatus != nil {
285-
c.logger.Warningf("could not set cluster status: %v", errStatus)
286-
return
281+
pg.Status.PostgresClusterStatus = acidv1.ClusterStatusAddFailed
287282
}
288-
if pgUpdatedStatus != nil {
283+
284+
if !equality.Semantic.DeepEqual(currentStatus, pg.Status) {
285+
pgUpdatedStatus, err := c.KubeClient.SetPostgresCRDStatus(c.clusterName(), pg)
286+
if err != nil {
287+
c.logger.Warningf("could not set cluster status: %v", err)
288+
return
289+
}
289290
c.setSpec(pgUpdatedStatus)
290291
}
291292
}()
292293

293-
pgCreateStatus, err = c.KubeClient.SetPostgresCRDStatus(c.clusterName(), acidv1.ClusterStatusCreating)
294+
pg := c.Postgresql.DeepCopy()
295+
pg.Status.PostgresClusterStatus = acidv1.ClusterStatusCreating
296+
297+
pgCreateStatus, err = c.KubeClient.SetPostgresCRDStatus(c.clusterName(), pg)
294298
if err != nil {
295299
return fmt.Errorf("could not set cluster status: %v", err)
296300
}
@@ -978,7 +982,12 @@ func (c *Cluster) Update(oldSpec, newSpec *acidv1.Postgresql) error {
978982
c.mu.Lock()
979983
defer c.mu.Unlock()
980984

981-
c.KubeClient.SetPostgresCRDStatus(c.clusterName(), acidv1.ClusterStatusUpdating)
985+
newSpec.Status.PostgresClusterStatus = acidv1.ClusterStatusUpdating
986+
987+
newSpec, err := c.KubeClient.SetPostgresCRDStatus(c.clusterName(), newSpec)
988+
if err != nil {
989+
return fmt.Errorf("could not set cluster status to updating: %w", err)
990+
}
982991

983992
if !isInMaintenanceWindow(newSpec.Spec.MaintenanceWindows) {
984993
// do not apply any major version related changes yet
@@ -987,20 +996,19 @@ func (c *Cluster) Update(oldSpec, newSpec *acidv1.Postgresql) error {
987996
c.setSpec(newSpec)
988997

989998
defer func() {
990-
var (
991-
pgUpdatedStatus *acidv1.Postgresql
992-
err error
993-
)
999+
currentStatus := newSpec.Status.DeepCopy()
1000+
newSpec.Status.PostgresClusterStatus = acidv1.ClusterStatusRunning
1001+
9941002
if updateFailed {
995-
pgUpdatedStatus, err = c.KubeClient.SetPostgresCRDStatus(c.clusterName(), acidv1.ClusterStatusUpdateFailed)
996-
} else {
997-
pgUpdatedStatus, err = c.KubeClient.SetPostgresCRDStatus(c.clusterName(), acidv1.ClusterStatusRunning)
998-
}
999-
if err != nil {
1000-
c.logger.Warningf("could not set cluster status: %v", err)
1001-
return
1003+
newSpec.Status.PostgresClusterStatus = acidv1.ClusterStatusUpdateFailed
10021004
}
1003-
if pgUpdatedStatus != nil {
1005+
1006+
if !equality.Semantic.DeepEqual(currentStatus, newSpec.Status) {
1007+
pgUpdatedStatus, err := c.KubeClient.SetPostgresCRDStatus(c.clusterName(), newSpec)
1008+
if err != nil {
1009+
c.logger.Warningf("could not set cluster status: %v", err)
1010+
return
1011+
}
10041012
c.setSpec(pgUpdatedStatus)
10051013
}
10061014
}()

‎pkg/cluster/sync.go‎

Lines changed: 10 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ import (
2020
batchv1 "k8s.io/api/batch/v1"
2121
v1 "k8s.io/api/core/v1"
2222
policyv1 "k8s.io/api/policy/v1"
23+
"k8s.io/apimachinery/pkg/api/equality"
2324
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
2425
"k8s.io/apimachinery/pkg/types"
2526
)
@@ -43,21 +44,19 @@ func (c *Cluster) Sync(newSpec *acidv1.Postgresql) error {
4344
c.setSpec(newSpec)
4445

4546
defer func() {
46-
var (
47-
pgUpdatedStatus *acidv1.Postgresql
48-
errStatus error
49-
)
5047
if err != nil {
5148
c.logger.Warningf("error while syncing cluster state: %v", err)
52-
pgUpdatedStatus, errStatus = c.KubeClient.SetPostgresCRDStatus(c.clusterName(), acidv1.ClusterStatusSyncFailed)
49+
newSpec.Status.PostgresClusterStatus = acidv1.ClusterStatusSyncFailed
5350
} else if !c.Status.Running() {
54-
pgUpdatedStatus, errStatus = c.KubeClient.SetPostgresCRDStatus(c.clusterName(), acidv1.ClusterStatusRunning)
51+
newSpec.Status.PostgresClusterStatus = acidv1.ClusterStatusRunning
5552
}
56-
if errStatus != nil {
57-
c.logger.Warningf("could not set cluster status: %v", errStatus)
58-
return
59-
}
60-
if pgUpdatedStatus != nil {
53+
54+
if !equality.Semantic.DeepEqual(oldSpec.Status, newSpec.Status) {
55+
pgUpdatedStatus, err := c.KubeClient.SetPostgresCRDStatus(c.clusterName(), newSpec)
56+
if err != nil {
57+
c.logger.Warningf("could not set cluster status: %v", err)
58+
return
59+
}
6160
c.setSpec(pgUpdatedStatus)
6261
}
6362
}()

‎pkg/cluster/util_test.go‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -288,6 +288,12 @@ func newInheritedAnnotationsCluster(client k8sutil.KubernetesClient) (*Cluster,
288288
},
289289
}
290290

291+
// add postgresql cluster to fake client
292+
_, err := client.PostgresqlsGetter.Postgresqls(namespace).Create(context.TODO(), &pg, metav1.CreateOptions{})
293+
if err != nil {
294+
return nil, err
295+
}
296+
291297
cluster := New(
292298
Config{
293299
OpConfig: config.Config{
@@ -321,7 +327,7 @@ func newInheritedAnnotationsCluster(client k8sutil.KubernetesClient) (*Cluster,
321327
}, client, pg, logger, eventRecorder)
322328
cluster.Name = clusterName
323329
cluster.Namespace = namespace
324-
_, err := cluster.createStatefulSet()
330+
_, err = cluster.createStatefulSet()
325331
if err != nil {
326332
return nil, err
327333
}

‎pkg/controller/postgresql.go‎

Lines changed: 17 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -161,7 +161,8 @@ func (c *Controller) acquireInitialListOfClusters() error {
161161
func (c *Controller) addCluster(lg *logrus.Entry, clusterName spec.NamespacedName, pgSpec *acidv1.Postgresql) (*cluster.Cluster, error) {
162162
if c.opConfig.EnableTeamIdClusternamePrefix {
163163
if _, err := acidv1.ExtractClusterName(clusterName.Name, pgSpec.Spec.TeamID); err != nil {
164-
c.KubeClient.SetPostgresCRDStatus(clusterName, acidv1.ClusterStatusInvalid)
164+
pgSpec.Status.PostgresClusterStatus = acidv1.ClusterStatusInvalid
165+
c.KubeClient.SetPostgresCRDStatus(clusterName, pgSpec)
165166
return nil, err
166167
}
167168
}
@@ -470,13 +471,25 @@ func (c *Controller) queueClusterEvent(informerOldSpec, informerNewSpec *acidv1.
470471

471472
switch eventType {
472473
case EventAdd:
473-
c.KubeClient.SetPostgresCRDStatus(clusterName, acidv1.ClusterStatusAddFailed)
474+
informerNewSpec.Status.PostgresClusterStatus = acidv1.ClusterStatusAddFailed
475+
_, err := c.KubeClient.SetPostgresCRDStatus(clusterName, informerNewSpec)
476+
if err != nil {
477+
c.logger.WithField("cluster-name", clusterName).Errorf("could not set PostgresCRD status: %v", err)
478+
}
474479
c.eventRecorder.Eventf(c.GetReference(informerNewSpec), v1.EventTypeWarning, "Create", "%v", clusterError)
475480
case EventUpdate:
476-
c.KubeClient.SetPostgresCRDStatus(clusterName, acidv1.ClusterStatusUpdateFailed)
481+
informerNewSpec.Status.PostgresClusterStatus = acidv1.ClusterStatusUpdateFailed
482+
_, err := c.KubeClient.SetPostgresCRDStatus(clusterName, informerNewSpec)
483+
if err != nil {
484+
c.logger.WithField("cluster-name", clusterName).Errorf("could not set PostgresCRD status: %v", err)
485+
}
477486
c.eventRecorder.Eventf(c.GetReference(informerNewSpec), v1.EventTypeWarning, "Update", "%v", clusterError)
478487
default:
479-
c.KubeClient.SetPostgresCRDStatus(clusterName, acidv1.ClusterStatusSyncFailed)
488+
informerNewSpec.Status.PostgresClusterStatus = acidv1.ClusterStatusSyncFailed
489+
_, err := c.KubeClient.SetPostgresCRDStatus(clusterName, informerNewSpec)
490+
if err != nil {
491+
c.logger.WithField("cluster-name", clusterName).Errorf("could not set PostgresCRD status: %v", err)
492+
}
480493
c.eventRecorder.Eventf(c.GetReference(informerNewSpec), v1.EventTypeWarning, "Sync", "%v", clusterError)
481494
}
482495

‎pkg/util/k8sutil/k8sutil.go‎

Lines changed: 2 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -191,24 +191,8 @@ func NewFromConfig(cfg *rest.Config) (KubernetesClient, error) {
191191
}
192192

193193
// SetPostgresCRDStatus of Postgres cluster
194-
func (client *KubernetesClient) SetPostgresCRDStatus(clusterName spec.NamespacedName, status string) (*apiacidv1.Postgresql, error) {
195-
var pg *apiacidv1.Postgresql
196-
var pgStatus apiacidv1.PostgresStatus
197-
pgStatus.PostgresClusterStatus = status
198-
199-
patch, err := json.Marshal(struct {
200-
PgStatus interface{} `json:"status"`
201-
}{&pgStatus})
202-
203-
if err != nil {
204-
return pg, fmt.Errorf("could not marshal status: %v", err)
205-
}
206-
207-
// we cannot do a full scale update here without fetching the previous manifest (as the resourceVersion may differ),
208-
// however, we could do patch without it. In the future, once /status subresource is there (starting Kubernetes 1.11)
209-
// we should take advantage of it.
210-
pg, err = client.PostgresqlsGetter.Postgresqls(clusterName.Namespace).Patch(
211-
context.TODO(), clusterName.Name, types.MergePatchType, patch, metav1.PatchOptions{}, "status")
194+
func (client *KubernetesClient) SetPostgresCRDStatus(clusterName spec.NamespacedName, pg *apiacidv1.Postgresql) (*apiacidv1.Postgresql, error) {
195+
pg, err := client.PostgresqlsGetter.Postgresqls(clusterName.Namespace).UpdateStatus(context.TODO(), pg, metav1.UpdateOptions{})
212196
if err != nil {
213197
return pg, fmt.Errorf("could not update status: %v", err)
214198
}

0 commit comments

Comments
 (0)