Always keep the last completed BackupSession (when `backupHistoryLimi…

…t > 0`) even if outside of history limit (#1413)

Signed-off-by: Emruz Hossain <emruz@appscode.com>

pkg/controller/backup_session.go

@@ -897,10 +897,16 @@ func (c *StashController) cleanupBackupHistory(backupInvokerRef api_v1beta1.Back
 		return bsList[i].CreationTimestamp.After(bsList[j].CreationTimestamp.Time)
 	})
 
+	var lastCompletedSession string
+	for i := range bsList {
+		if bsList[i].Status.Phase == api_v1beta1.BackupSessionSucceeded || bsList[i].Status.Phase == api_v1beta1.BackupSessionFailed {
+			lastCompletedSession = bsList[i].Name
+			break
+		}
+	}
 	// delete the BackupSession that does not fit within the history limit
 	for i := int(historyLimit); i < len(bsList); i++ {
-		// delete only the BackupSessions that has completed its backup
-		if backupCompleted(bsList[i].Status.Phase) {
+		if backupCompleted(bsList[i].Status.Phase) && !(bsList[i].Name == lastCompletedSession && historyLimit > 0) {
 			err = c.stashClient.StashV1beta1().BackupSessions(namespace).Delete(context.TODO(), bsList[i].Name, meta.DeleteInBackground())
 			if err != nil && !(kerr.IsNotFound(err) || kerr.IsGone(err)) {
 				return err

test/e2e/framework/backup_session.go

@@ -168,16 +168,6 @@ func (fi *Invocation) EventuallyRunningBackupCompleted(invokerMeta metav1.Object
 	}, WaitTimeOut, PullInterval)
 }
 
-func (fi *Invocation) EventuallyBackupSessionCount(invokerMeta metav1.ObjectMeta, invokerKind string) GomegaAsyncAssertion {
-	return Eventually(func() int {
-		bsList, err := fi.GetBackupSessionsForInvoker(invokerMeta, invokerKind)
-		if err != nil {
-			return 0
-		}
-		return len(bsList.Items)
-	}, WaitTimeOut, PullInterval)
-}
-
 func (fi *Invocation) GetBackupSessionsForInvoker(invokerMeta metav1.ObjectMeta, invokerKind string) (*v1beta1.BackupSessionList, error) {
 	return fi.StashClient.StashV1beta1().BackupSessions(fi.namespace).List(context.TODO(), metav1.ListOptions{
 		LabelSelector: labels.SelectorFromSet(map[string]string{

test/e2e/misc/backupsession.go

@@ -17,7 +17,6 @@ limitations under the License.
 package misc
 
 import (
-	"fmt"
 	"time"
 
 	"stash.appscode.dev/apimachinery/apis"
@@ -71,7 +70,7 @@ var _ = Describe("BackupSession", func() {
 			})
 			Expect(err).NotTo(HaveOccurred())
 
-			var lastXBS []string
+			var keepCandidates []string
 			totalBS := 5
 			By("Triggering multiple backups")
 			for i := 0; i < totalBS; i++ {
@@ -82,23 +81,20 @@ var _ = Describe("BackupSession", func() {
 				Expect(err).NotTo(HaveOccurred())
 				f.AppendToCleanupList(backupSession)
 				// store only last X BackupSession name
-				if i >= totalBS-int(historyLimit) {
-					lastXBS = append(lastXBS, backupSession.Name)
+				if i == 0 || i >= totalBS-int(historyLimit) {
+					keepCandidates = append(keepCandidates, backupSession.Name)
 				}
 				time.Sleep(1 * time.Second)
 			}
 
 			By("Waiting for all BackupSession to complete")
 			f.EventuallyRunningBackupCompleted(backupConfig.ObjectMeta, v1beta1.ResourceKindBackupConfiguration).Should(BeTrue())
 
-			By(fmt.Sprintf("Verifying that only %d BackupSession remaining", historyLimit))
-			f.EventuallyBackupSessionCount(backupConfig.ObjectMeta, v1beta1.ResourceKindBackupConfiguration).Should(BeNumerically("==", 2))
-
-			By("Verifying that the remaining BackupSessions are the most recent BackupSessions")
+			By("Verifying that the remaining BackupSessions are the desired ones")
 			remainingBS, err := f.GetBackupSessionsForInvoker(backupConfig.ObjectMeta, v1beta1.ResourceKindBackupConfiguration)
 			Expect(err).NotTo(HaveOccurred())
 			for _, bs := range remainingBS.Items {
-				exist, _ := arrays.Contains(lastXBS, bs.Name)
+				exist, _ := arrays.Contains(keepCandidates, bs.Name)
 				Expect(exist).To(BeTrue())
 			}
 		})