Skip to content

Commit

Permalink
chore: review
Browse files Browse the repository at this point in the history
Signed-off-by: Armando Ruocco <[email protected]>
  • Loading branch information
armru committed Dec 18, 2024
1 parent ee525c5 commit 76915b8
Show file tree
Hide file tree
Showing 7 changed files with 34 additions and 35 deletions.
25 changes: 20 additions & 5 deletions internal/controller/backup_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)
Expand Down Expand Up @@ -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)")
}

Expand All @@ -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,
Expand All @@ -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")
}

Expand Down Expand Up @@ -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)
Expand Down
19 changes: 0 additions & 19 deletions pkg/conditions/doc.go

This file was deleted.

5 changes: 2 additions & 3 deletions pkg/management/postgres/backup.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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")
}
Expand Down
9 changes: 7 additions & 2 deletions pkg/management/postgres/webserver/local.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)

Expand Down Expand Up @@ -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(
Expand Down
5 changes: 2 additions & 3 deletions pkg/management/postgres/webserver/plugin_backup.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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")
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ See the License for the specific language governing permissions and
limitations under the License.
*/

package conditions
package status

import (
"context"
Expand All @@ -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,
Expand Down
File renamed without changes.

0 comments on commit 76915b8

Please sign in to comment.