OAS-10007 Fix race condition in ArangoBackup (#1708)

Author: jwierzbo

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

pkg/apis/deployment/v2alpha1/deployment_spec_gateway.go

@@ -23,10 +23,17 @@ package v2alpha1
 import "github.com/arangodb/kube-arangodb/pkg/util"
 
 type DeploymentSpecGateway struct {
-	Enabled *bool   `json:"enabled,omitempty"`
-	Image   *string `json:"image"`
+	// Enabled setting enables/disables support for gateway in the cluster.
+	// When enabled, the cluster will contain a number of `gateway` servers.
+	// +doc/default: false
+	Enabled *bool `json:"enabled,omitempty"`
+
+	// Image is the image to use for the gateway.
+	// By default, the image is determined by the operator.
+	Image *string `json:"image"`
 }
 
+// IsEnabled returns whether the gateway is enabled.
 func (d *DeploymentSpecGateway) IsEnabled() bool {
 	if d == nil || d.Enabled == nil {
 		return false
@@ -35,10 +42,12 @@ func (d *DeploymentSpecGateway) IsEnabled() bool {
 	return *d.Enabled
 }
 
+// Validate the given spec
 func (d *DeploymentSpecGateway) Validate() error {
 	return nil
 }
 
+// GetImage returns the image to use for the gateway.
 func (d *DeploymentSpecGateway) GetImage() string {
 	return util.TypeOrDefault[string](d.Image)
 }

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

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)
+		})
+	})
 }