From 76915b8b5d09dc4e3086a428198c19ba3cf14ced Mon Sep 17 00:00:00 2001 From: Armando Ruocco Date: Wed, 18 Dec 2024 11:30:20 +0100 Subject: [PATCH] chore: review Signed-off-by: Armando Ruocco --- internal/controller/backup_controller.go | 25 +++++++++++++++---- pkg/conditions/doc.go | 19 -------------- pkg/management/postgres/backup.go | 5 ++-- pkg/management/postgres/webserver/local.go | 9 +++++-- .../postgres/webserver/plugin_backup.go | 5 ++-- .../status}/conditions.go | 6 ++--- pkg/resources/status/{tx.go => update.go} | 0 7 files changed, 34 insertions(+), 35 deletions(-) delete mode 100644 pkg/conditions/doc.go rename pkg/{conditions => resources/status}/conditions.go (93%) rename pkg/resources/status/{tx.go => update.go} (100%) diff --git a/internal/controller/backup_controller.go b/internal/controller/backup_controller.go index d95d839fa5..29c6aea6f9 100644 --- a/internal/controller/backup_controller.go +++ b/internal/controller/backup_controller.go @@ -45,11 +45,11 @@ import ( cnpgiClient "github.com/cloudnative-pg/cloudnative-pg/internal/cnpi/plugin/client" "github.com/cloudnative-pg/cloudnative-pg/internal/cnpi/plugin/repository" "github.com/cloudnative-pg/cloudnative-pg/pkg/certs" - "github.com/cloudnative-pg/cloudnative-pg/pkg/conditions" "github.com/cloudnative-pg/cloudnative-pg/pkg/management/postgres" "github.com/cloudnative-pg/cloudnative-pg/pkg/management/postgres/webserver/client/remote" "github.com/cloudnative-pg/cloudnative-pg/pkg/reconciler/backup/volumesnapshot" "github.com/cloudnative-pg/cloudnative-pg/pkg/reconciler/persistentvolumeclaim" + resourcestatus "github.com/cloudnative-pg/cloudnative-pg/pkg/resources/status" "github.com/cloudnative-pg/cloudnative-pg/pkg/specs" "github.com/cloudnative-pg/cloudnative-pg/pkg/utils" ) @@ -417,7 +417,12 @@ func (r *BackupReconciler) reconcileSnapshotBackup( } } - if errCond := conditions.OptimisticLockPatch(ctx, r.Client, cluster, apiv1.BackupStartingCondition); errCond != nil { + if errCond := resourcestatus.PatchConditionsWithOptimisticLock( + ctx, + r.Client, + cluster, + apiv1.BackupStartingCondition, + ); errCond != nil { contextLogger.Error(errCond, "Error while updating backup condition (backup starting)") } @@ -440,7 +445,7 @@ func (r *BackupReconciler) reconcileSnapshotBackup( // and un-fence the Pod contextLogger.Error(err, "while executing snapshot backup") // Update backup status in cluster conditions - if errCond := conditions.OptimisticLockPatch( + if errCond := resourcestatus.PatchConditionsWithOptimisticLock( ctx, r.Client, cluster, @@ -458,7 +463,12 @@ func (r *BackupReconciler) reconcileSnapshotBackup( return res, nil } - if err := conditions.OptimisticLockPatch(ctx, r.Client, cluster, apiv1.BackupSucceededCondition); err != nil { + if err := resourcestatus.PatchConditionsWithOptimisticLock( + ctx, + r.Client, + cluster, + apiv1.BackupSucceededCondition, + ); err != nil { contextLogger.Error(err, "Can't update the cluster with the completed snapshot backup data") } @@ -638,7 +648,12 @@ func startInstanceManagerBackup( status.CommandError = stdout // Update backup status in cluster conditions - if errCond := conditions.OptimisticLockPatch(ctx, client, cluster, apiv1.BuildClusterBackupFailedCondition(err)); errCond != nil { + if errCond := resourcestatus.PatchConditionsWithOptimisticLock( + ctx, + client, + cluster, + apiv1.BuildClusterBackupFailedCondition(err), + ); errCond != nil { log.FromContext(ctx).Error(errCond, "Error while updating backup condition (backup failed)") } return postgres.PatchBackupStatusAndRetry(ctx, client, backup) diff --git a/pkg/conditions/doc.go b/pkg/conditions/doc.go deleted file mode 100644 index acecc6fc10..0000000000 --- a/pkg/conditions/doc.go +++ /dev/null @@ -1,19 +0,0 @@ -/* -Copyright The CloudNativePG Contributors - -Licensed under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. -You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. -*/ - -// Package conditions contains functions useful to update the conditions -// on the resources managed by the operator -package conditions diff --git a/pkg/management/postgres/backup.go b/pkg/management/postgres/backup.go index 77e130eb61..6552e73344 100644 --- a/pkg/management/postgres/backup.go +++ b/pkg/management/postgres/backup.go @@ -42,7 +42,6 @@ import ( "sigs.k8s.io/controller-runtime/pkg/client" apiv1 "github.com/cloudnative-pg/cloudnative-pg/api/v1" - "github.com/cloudnative-pg/cloudnative-pg/pkg/conditions" "github.com/cloudnative-pg/cloudnative-pg/pkg/postgres" "github.com/cloudnative-pg/cloudnative-pg/pkg/resources" "github.com/cloudnative-pg/cloudnative-pg/pkg/resources/status" @@ -215,7 +214,7 @@ func (b *BackupCommand) takeBackup(ctx context.Context) error { // Update backup status in cluster conditions on startup if err := b.retryWithRefreshedCluster(ctx, func() error { - return conditions.OptimisticLockPatch(ctx, b.Client, b.Cluster, apiv1.BackupStartingCondition) + return status.PatchConditionsWithOptimisticLock(ctx, b.Client, b.Cluster, apiv1.BackupStartingCondition) }); err != nil { b.Log.Error(err, "Error changing backup condition (backup started)") // We do not terminate here because we could still have a good backup @@ -261,7 +260,7 @@ func (b *BackupCommand) takeBackup(ctx context.Context) error { // Update backup status in cluster conditions on backup completion if err := b.retryWithRefreshedCluster(ctx, func() error { - return conditions.OptimisticLockPatch(ctx, b.Client, b.Cluster, apiv1.BackupSucceededCondition) + return status.PatchConditionsWithOptimisticLock(ctx, b.Client, b.Cluster, apiv1.BackupSucceededCondition) }); err != nil { b.Log.Error(err, "Can't update the cluster with the completed backup data") } diff --git a/pkg/management/postgres/webserver/local.go b/pkg/management/postgres/webserver/local.go index 0b0bcd0c6d..27b15db9e1 100644 --- a/pkg/management/postgres/webserver/local.go +++ b/pkg/management/postgres/webserver/local.go @@ -33,9 +33,9 @@ import ( apiv1 "github.com/cloudnative-pg/cloudnative-pg/api/v1" "github.com/cloudnative-pg/cloudnative-pg/internal/management/cache" - "github.com/cloudnative-pg/cloudnative-pg/pkg/conditions" "github.com/cloudnative-pg/cloudnative-pg/pkg/management/postgres" "github.com/cloudnative-pg/cloudnative-pg/pkg/management/url" + "github.com/cloudnative-pg/cloudnative-pg/pkg/resources/status" "github.com/cloudnative-pg/cloudnative-pg/pkg/utils" ) @@ -283,7 +283,12 @@ func (ws *localWebserverEndpoints) setWALArchiveStatusCondition(w http.ResponseW return } - if errCond := conditions.OptimisticLockPatch(ctx, ws.typedClient, cluster, asr.getContinuousArchivingCondition()); errCond != nil { + if errCond := status.PatchConditionsWithOptimisticLock( + ctx, + ws.typedClient, + cluster, + asr.getContinuousArchivingCondition(), + ); errCond != nil { contextLogger.Error(errCond, "Error changing wal archiving condition", "condition", asr.getContinuousArchivingCondition()) http.Error( diff --git a/pkg/management/postgres/webserver/plugin_backup.go b/pkg/management/postgres/webserver/plugin_backup.go index fea072902c..5c10140282 100644 --- a/pkg/management/postgres/webserver/plugin_backup.go +++ b/pkg/management/postgres/webserver/plugin_backup.go @@ -32,7 +32,6 @@ import ( pluginClient "github.com/cloudnative-pg/cloudnative-pg/internal/cnpi/plugin/client" "github.com/cloudnative-pg/cloudnative-pg/internal/cnpi/plugin/repository" "github.com/cloudnative-pg/cloudnative-pg/internal/configuration" - "github.com/cloudnative-pg/cloudnative-pg/pkg/conditions" "github.com/cloudnative-pg/cloudnative-pg/pkg/management/postgres" "github.com/cloudnative-pg/cloudnative-pg/pkg/resources" "github.com/cloudnative-pg/cloudnative-pg/pkg/resources/status" @@ -103,7 +102,7 @@ func (b *PluginBackupCommand) invokeStart(ctx context.Context) { // Update backup status in cluster conditions on startup if err := b.retryWithRefreshedCluster(ctx, func() error { - return conditions.OptimisticLockPatch(ctx, b.Client, b.Cluster, apiv1.BackupStartingCondition) + return status.PatchConditionsWithOptimisticLock(ctx, b.Client, b.Cluster, apiv1.BackupStartingCondition) }); err != nil { contextLogger.Error(err, "Error changing backup condition (backup started)") // We do not terminate here because we could still have a good backup @@ -153,7 +152,7 @@ func (b *PluginBackupCommand) invokeStart(ctx context.Context) { // Update backup status in cluster conditions on backup completion if err := b.retryWithRefreshedCluster(ctx, func() error { - return conditions.OptimisticLockPatch(ctx, b.Client, b.Cluster, apiv1.BackupSucceededCondition) + return status.PatchConditionsWithOptimisticLock(ctx, b.Client, b.Cluster, apiv1.BackupSucceededCondition) }); err != nil { contextLogger.Error(err, "Can't update the cluster with the completed backup data") } diff --git a/pkg/conditions/conditions.go b/pkg/resources/status/conditions.go similarity index 93% rename from pkg/conditions/conditions.go rename to pkg/resources/status/conditions.go index cd852c9b60..54b09a056b 100644 --- a/pkg/conditions/conditions.go +++ b/pkg/resources/status/conditions.go @@ -14,7 +14,7 @@ See the License for the specific language governing permissions and limitations under the License. */ -package conditions +package status import ( "context" @@ -28,12 +28,12 @@ import ( apiv1 "github.com/cloudnative-pg/cloudnative-pg/api/v1" ) -// OptimisticLockPatch will update a particular condition in cluster status. +// PatchConditionsWithOptimisticLock will update a particular condition in cluster status. // This function may update the conditions in the passed cluster // with the latest ones that were found from the API server. // This function is needed because Kubernetes still doesn't support strategic merge // for CRDs (see https://kubernetes.io/docs/concepts/extend-kubernetes/api-extension/custom-resources/). -func OptimisticLockPatch( +func PatchConditionsWithOptimisticLock( ctx context.Context, c client.Client, cluster *apiv1.Cluster, diff --git a/pkg/resources/status/tx.go b/pkg/resources/status/update.go similarity index 100% rename from pkg/resources/status/tx.go rename to pkg/resources/status/update.go