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

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