OAS-10007 Fix race condition in ArangoBackup

arangodb · Aug 28, 2024 · 3727351 · 3727351
1 parent 8264471
commit 3727351
Show file tree

Hide file tree

Showing 3 changed files with 56 additions and 0 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -18,6 +18,7 @@
 - (Improvement) Better panic handling
 - (Feature) PongV1 Integration Service
 - (Feature) Custom Gateway image
+- (Bugfix) Fix race condition in ArangoBackup
 
 ## [1.2.42](https://github.com/arangodb/kube-arangodb/tree/1.2.42) (2024-07-23)
 - (Maintenance) Go 1.22.4 & Kubernetes 1.29.6 libraries

diff --git a/pkg/handlers/backup/handler.go b/pkg/handlers/backup/handler.go
@@ -123,6 +123,14 @@ func (h *handler) refreshDeployment(deployment *database.ArangoDeployment) error
 		return err
 	}
 
+	for _, backup := range backups.Items {
+		switch backup.GetStatus().ArangoBackupState.State {
+		case backupApi.ArangoBackupStateCreate, backupApi.ArangoBackupStateCreating:
+			// Skip refreshing backups if they are in creation state
+			return nil
+		}
+	}
+
 	existingBackups, err := client.List()
 	if err != nil {
 		return err

diff --git a/pkg/handlers/backup/handler_test.go b/pkg/handlers/backup/handler_test.go
@@ -129,4 +129,51 @@ func Test_Refresh_Cleanup(t *testing.T) {
 		require.NoError(t, err)
 		require.Len(t, backups.Items, 0)
 	})
+
+	t.Run("Do not refresh if backup is creating", func(t *testing.T) {
+		// Arrange
+		fakeId := driver.BackupID(uuid.NewUUID())
+		createBackup := backupApi.ArangoBackup{
+
+			ObjectMeta: meta.ObjectMeta{
+				Name: "backup",
+			},
+			Status: backupApi.ArangoBackupStatus{
+				ArangoBackupState: backupApi.ArangoBackupState{
+					State: backupApi.ArangoBackupStateCreating,
+				},
+				Backup: &backupApi.ArangoBackupDetails{
+					ID: string(fakeId),
+				},
+			},
+		}
+		b, err := handler.client.BackupV1().ArangoBackups(tests.FakeNamespace).Create(context.Background(), &createBackup, meta.CreateOptions{})
+		require.NoError(t, err)
+		require.NotNil(t, b)
+		require.Equal(t, backupApi.ArangoBackupStateCreating, b.Status.State)
+
+		t.Run("Refresh should not happen if there is Backup in creation state", func(t *testing.T) {
+			require.NoError(t, handler.refreshDeployment(arangoDeployment))
+
+			backups, err := handler.client.BackupV1().ArangoBackups(tests.FakeNamespace).List(context.Background(), meta.ListOptions{})
+			require.NoError(t, err)
+			require.Len(t, backups.Items, 1)
+			require.NotNil(t, backups.Items[0].Status.Backup)
+			require.EqualValues(t, fakeId, backups.Items[0].Status.Backup.ID)
+		})
+
+		createBackup.Status.State = backupApi.ArangoBackupStateReady
+		b, err = handler.client.BackupV1().ArangoBackups(tests.FakeNamespace).UpdateStatus(context.Background(), &createBackup, meta.UpdateOptions{})
+		require.NoError(t, err)
+		require.NotNil(t, b)
+		require.Equal(t, backupApi.ArangoBackupStateReady, b.Status.State)
+
+		t.Run("Refresh should happen if there is Backup in ready state", func(t *testing.T) {
+			require.NoError(t, handler.refreshDeployment(arangoDeployment))
+
+			backups, err := handler.client.BackupV1().ArangoBackups(tests.FakeNamespace).List(context.Background(), meta.ListOptions{})
+			require.NoError(t, err)
+			require.Len(t, backups.Items, 2)
+		})
+	})
 }