Skip to content

Commit

Permalink
don't set False to ReadyToUse condition
Browse files Browse the repository at this point in the history 
Signed-off-by: Ryotaro Banno <[email protected]>
  • Loading branch information
@ushitora-anqou
ushitora-anqou committed Feb 25, 2025
1 parent 455e52a commit cf6915a
Show file tree
Hide file tree
Showing 2 changed files with 17 additions and 91 deletions.
34 changes: 3 additions & 31 deletions internal/controller/mantlebackup_controller.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -168,12 +168,7 @@ func (r *MantleBackupReconciler) createRBDSnapshot(ctx context.Context, poolName
snap, findErr := ceph.FindRBDSnapshot(r.ceph, poolName, imageName, backup.Name)
if findErr != nil {
logger.Error(errors.Join(createErr, findErr), "failed to find rbd snapshot")
updateStatusErr := r.updateStatusCondition(ctx, backup, metav1.Condition{
Type: mantlev1.BackupConditionReadyToUse,
Status: metav1.ConditionFalse,
Reason: mantlev1.BackupReasonFailedToCreateBackup,
})
return nil, errors.Join(createErr, findErr, updateStatusErr)
return nil, errors.Join(createErr, findErr)
}
return snap, nil
}
Expand All @@ -183,11 +178,6 @@ func (r *MantleBackupReconciler) checkPVCInManagedCluster(ctx context.Context, b
clusterID, err := getCephClusterIDFromPVC(ctx, r.Client, pvc)
if err != nil {
logger.Error(err, "failed to get clusterID from PVC", "namespace", pvc.Namespace, "name", pvc.Name)
err2 := r.updateStatusCondition(ctx, backup, metav1.Condition{Type: mantlev1.BackupConditionReadyToUse, Status: metav1.ConditionFalse, Reason: mantlev1.BackupReasonFailedToCreateBackup})
if err2 != nil {
return err2
}

return err
}
if clusterID != r.managedCephClusterID {
Expand All @@ -198,14 +188,9 @@ func (r *MantleBackupReconciler) checkPVCInManagedCluster(ctx context.Context, b
return nil
}

func (r *MantleBackupReconciler) isPVCBound(ctx context.Context, backup *mantlev1.MantleBackup, pvc *corev1.PersistentVolumeClaim) (bool, error) {
func (r *MantleBackupReconciler) isPVCBound(ctx context.Context, pvc *corev1.PersistentVolumeClaim) (bool, error) {
logger := log.FromContext(ctx)
if pvc.Status.Phase != corev1.ClaimBound {
err := r.updateStatusCondition(ctx, backup, metav1.Condition{Type: mantlev1.BackupConditionReadyToUse, Status: metav1.ConditionFalse, Reason: mantlev1.BackupReasonFailedToCreateBackup})
if err != nil {
return false, err
}

if pvc.Status.Phase == corev1.ClaimPending {
return false, nil
} else {
Expand Down Expand Up @@ -246,10 +231,6 @@ func (r *MantleBackupReconciler) getSnapshotTarget(ctx context.Context, backup *
err := r.Get(ctx, types.NamespacedName{Namespace: pvcNamespace, Name: pvcName}, &pvc)
if err != nil {
logger.Error(err, "failed to get PVC", "namespace", pvcNamespace, "name", pvcName)
err2 := r.updateStatusCondition(ctx, backup, metav1.Condition{Type: mantlev1.BackupConditionReadyToUse, Status: metav1.ConditionFalse, Reason: mantlev1.BackupReasonFailedToCreateBackup})
if err2 != nil {
return nil, ctrl.Result{}, err2
}
if aerrors.IsNotFound(err) {
return nil, ctrl.Result{}, errTargetPVCNotFound{err}
}
Expand All @@ -260,7 +241,7 @@ func (r *MantleBackupReconciler) getSnapshotTarget(ctx context.Context, backup *
return nil, ctrl.Result{}, err
}

ok, err := r.isPVCBound(ctx, backup, &pvc)
ok, err := r.isPVCBound(ctx, &pvc)
if err != nil {
return nil, ctrl.Result{}, err
}
Expand All @@ -274,11 +255,6 @@ func (r *MantleBackupReconciler) getSnapshotTarget(ctx context.Context, backup *
err = r.Get(ctx, types.NamespacedName{Name: pvName}, &pv)
if err != nil {
logger.Error(err, "failed to get PV", "name", pvName)
err2 := r.updateStatusCondition(ctx, backup, metav1.Condition{Type: mantlev1.BackupConditionReadyToUse, Status: metav1.ConditionFalse, Reason: mantlev1.BackupReasonFailedToCreateBackup})
if err2 != nil {
return nil, ctrl.Result{}, err2
}

return nil, ctrl.Result{}, err
}

Expand Down Expand Up @@ -415,10 +391,6 @@ func (r *MantleBackupReconciler) reconcileAsStandalone(ctx context.Context, back
logger.Error(err, "failed to add finalizer", "finalizer", MantleBackupFinalizerName)
return ctrl.Result{}, err
}
err := r.updateStatusCondition(ctx, backup, metav1.Condition{Type: mantlev1.BackupConditionReadyToUse, Status: metav1.ConditionFalse, Reason: mantlev1.BackupReasonNone})
if err != nil {
return ctrl.Result{}, err
}
return requeueReconciliation(), nil
}

Expand Down
74 changes: 14 additions & 60 deletions internal/controller/mantlebackup_controller_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ var _ = Describe("MantleBackup controller", func() {
var ns string
var lastExpireQueuedBackups sync.Map

ensureReadyToUseNotTrue := func(ctx context.Context, backup *mantlev1.MantleBackup) {
ensureBackupNotReadyToUse := func(ctx context.Context, backup *mantlev1.MantleBackup) {
GinkgoHelper()
Consistently(ctx, func(g Gomega, ctx context.Context) {
namespacedName := types.NamespacedName{
Expand All @@ -58,28 +58,12 @@ var _ = Describe("MantleBackup controller", func() {
}
err := k8sClient.Get(ctx, namespacedName, backup)
g.Expect(err).NotTo(HaveOccurred())
condition := meta.FindStatusCondition(backup.Status.Conditions, mantlev1.BackupConditionReadyToUse)
if condition != nil {
g.Expect(condition.Status).NotTo(Equal(metav1.ConditionTrue))
}
g.Expect(
meta.IsStatusConditionTrue(backup.Status.Conditions, mantlev1.BackupConditionReadyToUse),
).To(BeFalse())
}, "10s", "1s").Should(Succeed())
}

waitForBackupNotReady := func(ctx context.Context, backup *mantlev1.MantleBackup) {
EventuallyWithOffset(1, func(g Gomega, ctx context.Context) {
namespacedName := types.NamespacedName{
Namespace: backup.Namespace,
Name: backup.Name,
}
err := k8sClient.Get(ctx, namespacedName, backup)
g.Expect(err).NotTo(HaveOccurred())
condition := meta.FindStatusCondition(backup.Status.Conditions, mantlev1.BackupConditionReadyToUse)
g.Expect(condition).NotTo(BeNil())
g.Expect(condition.Status).To(Equal(metav1.ConditionFalse))
g.Expect(condition.Reason).NotTo(Equal(mantlev1.BackupReasonNone))
}).WithContext(ctx).Should(Succeed())
}

waitForHavingFinalizer := func(ctx context.Context, backup *mantlev1.MantleBackup) {
EventuallyWithOffset(1, func(g Gomega, ctx context.Context) {
namespacedName := types.NamespacedName{
Expand Down Expand Up @@ -327,7 +311,7 @@ var _ = Describe("MantleBackup controller", func() {
backup, err := resMgr.CreateUniqueBackupFor(ctx, pvc)
Expect(err).NotTo(HaveOccurred())

waitForBackupNotReady(ctx, backup)
ensureBackupNotReadyToUse(ctx, backup)
})

It("should not be ready to use if specified non-existent PVC name", func(ctx SpecContext) {
Expand All @@ -340,7 +324,7 @@ var _ = Describe("MantleBackup controller", func() {
})
Expect(err).NotTo(HaveOccurred())

waitForBackupNotReady(ctx, backup)
ensureBackupNotReadyToUse(ctx, backup)
})

It("should fail the resource creation the second time if the same MantleBackup is created twice", func(ctx SpecContext) {
Expand All @@ -360,7 +344,7 @@ var _ = Describe("MantleBackup controller", func() {

backup, err := resMgr.CreateUniqueBackupFor(ctx, pvc)
Expect(err).NotTo(HaveOccurred())
ensureReadyToUseNotTrue(ctx, backup)
ensureBackupNotReadyToUse(ctx, backup)
})

It("should fail to take a backup if the size of the taken snapshot is not equal to the PV size", func(ctx SpecContext) {
Expand All @@ -371,7 +355,7 @@ var _ = Describe("MantleBackup controller", func() {

backup, err := resMgr.CreateUniqueBackupFor(ctx, pvc)
Expect(err).NotTo(HaveOccurred())
ensureReadyToUseNotTrue(ctx, backup)
ensureBackupNotReadyToUse(ctx, backup)
})
})

Expand Down Expand Up @@ -702,13 +686,9 @@ var _ = Describe("searchDiffOriginMantleBackup", func() {
}
// Note that slices.Clone() does the shallow copy.
// ref. https://pkg.go.dev/slices#Clone
primaryBackupsWithConditionFalse := slices.Clone(basePrimaryBackups)
primaryBackupsWithConditionFalse[2] = *basePrimaryBackups[2].DeepCopy()
meta.SetStatusCondition(&primaryBackupsWithConditionFalse[2].Status.Conditions,
metav1.Condition{
Type: mantlev1.BackupConditionReadyToUse,
Status: metav1.ConditionFalse,
})
primaryBackupsWithBlankCondition := slices.Clone(basePrimaryBackups)
primaryBackupsWithBlankCondition[2] = *basePrimaryBackups[2].DeepCopy()
meta.RemoveStatusCondition(&primaryBackupsWithBlankCondition[2].Status.Conditions, mantlev1.BackupConditionReadyToUse)
primaryBackupsWithDeletionTimestamp := slices.Clone(basePrimaryBackups)
primaryBackupsWithDeletionTimestamp[2] = *basePrimaryBackups[2].DeepCopy()
now := metav1.Now()
Expand Down Expand Up @@ -743,7 +723,7 @@ var _ = Describe("searchDiffOriginMantleBackup", func() {
testMantleBackup, basePrimaryBackups, testSecondaryMantleBackups,
true, "test3"),
Entry("should skip the not-ready MantleBackup",
testMantleBackup, primaryBackupsWithConditionFalse, testSecondaryMantleBackups,
testMantleBackup, primaryBackupsWithBlankCondition, testSecondaryMantleBackups,
true, "test1"),
Entry("should skip the MantleBackup with the deletion timestamp",
testMantleBackup, primaryBackupsWithDeletionTimestamp, testSecondaryMantleBackups,
Expand Down Expand Up @@ -1081,16 +1061,7 @@ var _ = Describe("SetSynchronizing", func() {
DescribeTable("call SetSynchronizing once", doTestCallOnce,
Entry(
"a full backup should succeed",
&mantlev1.MantleBackup{
Status: mantlev1.MantleBackupStatus{
Conditions: []metav1.Condition{
{
Type: mantlev1.BackupConditionReadyToUse,
Status: metav1.ConditionFalse,
},
},
},
},
&mantlev1.MantleBackup{},
nil,
false,
func(target, source *mantlev1.MantleBackup) error {
Expand All @@ -1106,16 +1077,7 @@ var _ = Describe("SetSynchronizing", func() {
),
Entry(
"an incremental backup should succeed",
&mantlev1.MantleBackup{
Status: mantlev1.MantleBackupStatus{
Conditions: []metav1.Condition{
{
Type: mantlev1.BackupConditionReadyToUse,
Status: metav1.ConditionFalse,
},
},
},
},
&mantlev1.MantleBackup{},
&mantlev1.MantleBackup{},
false,
func(target, source *mantlev1.MantleBackup) error {
Expand Down Expand Up @@ -1168,14 +1130,6 @@ var _ = Describe("SetSynchronizing", func() {
ObjectMeta: metav1.ObjectMeta{
Name: name,
},
Status: mantlev1.MantleBackupStatus{
Conditions: []metav1.Condition{
{
Type: mantlev1.BackupConditionReadyToUse,
Status: metav1.ConditionFalse,
},
},
},
})
Expect(err).NotTo(HaveOccurred())
}
Expand Down

0 comments on commit cf6915a

Please sign in to comment.